test(diff): cover empty-transcript edge cases and first-occurrence deduplication - #33
Merged
Conversation
…duplication diffTranscripts uses indexPairs to build a (method, volatile-stripped-params) keyed map, keeping only the first ReplayPair per key. Previously there were no tests for: (a) empty transcripts on either or both sides, and (b) the first-occurrence deduplication guarantee when a recording contains duplicate (method, params) entries. The new suite covers both scenarios, including the interaction between deduplication and volatile-key stripping (timestamp, requestId, etc.) that lets two recorded calls with the same non-volatile params collide to one comparison slot.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
1 participant
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.
Why
diffTranscriptsbuilds its comparison index viaindexPairs, which maps eachReplayPairto a(method, volatile-stripped-params)key and keeps only thefirst occurrence when duplicates appear. Two important behavioral guarantees
had no test coverage:
Empty-transcript edge cases - one or both sides empty. These feed
directly into
diffTranscriptsin real fixture-update workflows when aserver adds or removes an entire method.
First-occurrence deduplication - when a recording contains the same
(method, params)more than once (common in long sessions), only the firstresponse is used for comparison. A test author relying on "the second call's
response" being compared would get the wrong result silently.
Volatile-key collision - two calls with identical non-volatile params but
differing
timestamp/requestIdvalues collide to the same slot, meaningthe second is silently discarded. This is the correct behavior (it mirrors
the
normalizedMatchsemantics used everywhere else) but was untested.What
test/diff.dedup.test.tswith 9 tests across 4describeblocks:empty transcripts: both empty, A populated/B empty, A empty/B populatedfirst-occurrence deduplication in A: second duplicate ignored when itmatches B, when first matches B, three duplicates
first-occurrence deduplication in B: second duplicate in B ignoreddeduplication interacts with volatile key stripping: timestamp andrequestId variants collide to one slot
Tests
npm run lint- clean (tsc --noEmit)npm run build- cleannpm test- 227 tests pass (218 before this PR, 9 new)test/diff.dedup.test.ts; no source changesSelf-merge gate
Generated by Claude Code