Repository navigation
fix(json): preserve prototype keys and property-test snapshot codecs - #5544
Conversation
Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
A fresh nightly checkout has no node_modules, so bun could not resolve fast-check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 13 minutes. View limit details
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Merge summary:
Public API and wire: unchanged. (Written by Claude Opus 5.5) |
Problem
A generated snapshot sequence reduced to
{"__proto__":0} -> {}leaves the JS decoder with a stale field. Object membership sees inherited properties, and assignment to__proto__invokes its inherited setter. These are valid JSON keys.Approach
Use own-property membership and safe data-property writes in the JS diff/merge/equality implementation. Four shared Rust/JS vectors cover prototype-key addition, deletion, nesting, and inherited equality. A JS assertion also checks the result retains an ordinary object prototype.
Add fast-check 4.10.2 and proptest 1.12.0 as dev dependencies. Mirrored bounded JSON generators check every reconstructed value across snapshots, deltas, resets, ratios, and compression, including updates emitting no frame. PR runs use 256 fixed-seed cases.
just test json-propertiesprints a fresh seed and runs 4096 cases in each language nightly. Failures shrink and report replay details; minimized regressions stay in shared vectors. A codec benchmark sweeps document size and changed-key count with both compression modes.Impact
Public API shape and wire format are unchanged. JS behavior now preserves valid JSON prototype-named fields. Rust production code is unchanged. The nightly row is independent of #5527's workload split and needs no landing dependency. No package versions change.
Validation
just checkpassed 7468 default Rust tests, crypto identity, and 460 capture tests, then hit the existing CLI testexport::the_same_instance_within_the_linger_continues(export wrote nothing). The same failure is documented in fix: count HTTP refusals and preserve replaced routes at the final sequence #5526 and tracked byquest/m1/ts-same-instance-resume.md; this PR does not change CLI or Rust production codec logic.just checkcommand therefore is not green.Alternatives and follow-up
Using prototype-less output objects would change observable object shape. The private safe-write helper preserves that shape and only handles the special setter key differently. Keep generated counterexamples as deterministic shared vectors, and resolve the separately tracked CLI resume regression before treating the complete suite as green.
(Written by GPT-6)
Need help on this PR? Tag
@codesmith-botwith what you need.