test(fuzz): let the generative decode target see the codec gaps it draws - #54
Merged
Conversation
Deep mode fails `proto:decode-parity` with 11 divergences on `main`, and every one is a bridge codec gap the registry already documents — reached from a target the entries did not list. Seven are `BotAvatarMetadata`, the type the bridge does not implement. The finite sweeps meet it through a field that holds one, where the bridge silently omits it; the generative target hands the type's own bytes to both decoders and the bridge throws. One missing codec, two ways of meeting it, so `proto-unknown-type-dropped` covers the target — with the outcome pinned there, since decode-parity reports every kind of disagreement and the entry must not excuse wrong data for the same type. Four are a rename beside `businessBroadcastAssociationAction`, which the bridge never writes. Both halves have entries; neither explained the pair, because the finite sweeps vary one field at a time and never saw them together. The rename predicate now composes with the enumerated not-encoded fields: only those eleven may be absent, only from the bridge's side, and any value both sides carry must still match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtvVVWQs8AeBqSzS1JpCg
📝 WalkthroughWalkthroughThe divergence harness now supports path-qualified bridge omissions, composed field-rename matching, and restricted unknown-type handling for protobuf decode parity. Tests cover valid documented divergences and reject unrelated omissions, value changes, reverse additions, and invalid decode outcomes. ChangesDivergence allowlist validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c47e17053
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two review findings, both on the leaf-name set, both real. `NOT_ENCODED_FIELDS` is the *encoder* gap list, and three of its entries are decoded under a renamed property rather than dropped. Taking its leaves let one valid rename stand in for a key genuinely missing somewhere else: a SyncActionValue with the expected AgentAction.deviceId rename and a newly absent ChatAssignmentAction.deviceAgentID passed, because `undoRenames` restored the first and the leaf set forgave the second. Measured before and after — excused went from true to false. A leaf name is also not unique. `messageParamsJson` is unwritten on Message.PaymentExtendedMetadata, while the schema declares another on Message.InteractiveMessage.NativeFlowMessage; the bare leaf excused a new drop of the second as though it were the documented first. Measured the same way. The set is now keyed by decoded type and key path, and holds only what the decode targets actually produced, so neither shape is reachable. The first draft of both regression tests was vacuous — one made a whole holder missing instead of the renamed leaf, the other carried no rename and never reached the composition; both were rebuilt and verified to fail against the previous predicate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtvVVWQs8AeBqSzS1JpCg
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__fuzz__/harness/__tests__/harness.test.ts`:
- Around line 549-559: Extend the local excused helper and its assertions to
cover a divergence whose input omits the path field, exercising the branch where
inputPath returns undefined. Keep the existing path-based rename case unchanged
and add a focused assertion that verifies the expected allowlist result for the
missing-path input.
In `@src/__fuzz__/harness/divergence.ts`:
- Around line 1260-1263: Update the nearby composition comment to state that
only the single field path enumerated by DECODE_OMITTED_PATHS may be absent,
replacing the incorrect count of eleven while preserving the remaining
constraints.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 632108a1-3436-4ee9-8c68-0879aa908c43
📒 Files selected for processing (2)
src/__fuzz__/harness/__tests__/harness.test.tssrc/__fuzz__/harness/divergence.ts
Two review notes. The composition comment still said "the eleven enumerated fields" from the leaf-name draft; the set it consults now enumerates one path, so it names the set instead of a number that will drift again. The `inputPath` lookup had no test. Adding one exposed a defect in the test helper rather than the predicate: `path` was a defaulted parameter, so passing `undefined` took the default and the case asserted the exact opposite of what it claimed. The helper now takes the input whole, and the assertion fails against a predicate that does not check the root. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtvVVWQs8AeBqSzS1JpCg
This was referenced Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
proto:decode-parityfails in deep mode onmainwith 11 divergences. Every one is a bridge codec gap the registry already documents — reached from a target the entries did not list. Two entries, two shapes:BotAvatarMetadata— bridge throwsunknown proto type, protobufjs returns{}proto-unknown-type-droppeddeviceID→deviceId,deviceAgentID→deviceAgentId) besidebusinessBroadcastAssociationAction, which the bridge never writesproto-field-renamed-and-dropped+proto-field-not-encodedNothing here is a baileyrs defect and nothing here is new — the codec is the bridge's TypeScript, and all three fields plus the missing type are in
KNOWN_WIRE_GAPS/KNOWN_UNSUPPORTED_CODECSinscripts/compatibility/proto-runtime-audit.ts. What changes is that the nightly stops being red on findings it can already explain.The unimplemented type
The finite sweeps reach
BotAvatarMetadatathrough a field that holds one, where the bridge silently omits it. The generative target hands the type's own bytes to both decoders, and the bridge throws instead. One missing codec, two ways of meeting it.Widening the target alone would have been too loose: the sweeps' classifier has already established the shape, so naming the type is the whole test there, but
decode-parityreports every kind of disagreement. Without the outcome pinned, the entry would excuse the bridge returning wrong data for a type it does not implement just as readily as refusing to decode it. On that target it now requires a throw naming a probed-unknown type on one side and a decoded object on the other.The composed pair
proto-field-renamed-and-droppeddeliberately demands the rename be the whole difference — undo it and the two sides must agree, or the finding carries a second defect the entry cannot explain. That guard is why the four composed records failed, and it is worth keeping.The second defect here is not unexplained, though: it is
businessBroadcastAssociationAction, one of the eleven fields inNOT_ENCODED_FIELDS. The finite sweeps never saw the two together because they vary one field at a time; a generative draw ofSyncActionValuecarries both at once.So the comparison composes, and stays exact:
The leaf-name set is derived from
NOT_ENCODED_FIELDSrather than written out again, so the qualified list stays the single source of truth.Validation
Two new harness tests, one per entry, each with its counter-cases:
Negative test. Reverting only
divergence.ts:Performance
No runtime code is touched — the diff is two files under
src/__fuzz__/:Nothing in the published package changes, so allocations, memory and CPU are identical by construction. Both predicates run only after a fuzz run has already found a divergence to classify, and
sameExceptUnwrittenFieldsis a depth-bounded walk of two decoded objects — the same order of work thesameShapecall it replaces already did.Still red in deep mode, not this PR
Two of the four causes remain, both uncharacterised and both bridge-side:
proto:mutation-agreement, 28 divergences — the bridge drops sub-fields on mutated payloads (PollCreationMessage.correctAnswer.optionHash,SyncActionValue.timestamp,DeviceListMetadata.senderKeyIndexes).proto:mutation-stability, 1 —deviceListMetadata.senderKeyIndexesis lost on re-encode, so decode → encode → decode never reaches a fixed point.Both verified identical on
main. Next, separately — and they need investigation rather than a registry edit, since no existing entry describes them.Generated by Claude Code
Summary by cubic
Teach the generative
proto:decode-paritytarget to recognize documented bridge codec gaps and qualify decode-side omissions by decoded type and key path. This removes 11 false divergences in deep mode without touching runtime code.proto-unknown-type-droppedto coverproto:decode-parity, requiring a bridge-side throw that names the probed unknown type and an upstream object decode.proto-field-renamed-and-droppedwith measured omissions viasameExceptUnwrittenFields, keyed by<decoded type>.<key path>and rooted at the decoded type, so renames don’t excuse unrelated missing keys or similarly named leaves elsewhere.pathand inverted the no-path case; updated comments to remove a stale count.Written for commit aeb01ee. Summary will update on new commits.