feat(json): a public snapshot group budget, refusing an oversized snapshot before parsing by kixelated · Pull Request #7072 · moq-dev/moq

kixelated · 2026-10-28T09:37:43Z

Problem

quest/m0/ci/stats-oversize.md, found by the profile in moq-dev/moq#6987. The moq-stats oversize tests publish a stats frame past moq-net's 32 MiB group cache and expect FrameTooLarge. Most of their cost was building and serializing 32 MiB of stats in a debug build, before the encoder ever saw the bytes. The snapshot encoder then parsed the oversized snapshot back into its baseline before anything refused it.

Changes

This PR was widened at the maintainer's request (see Decisions). It was also rebuilt on main after moq-dev/moq#7046 removed per-record compression, which most of its first version was about.

moq_json::snapshot::Config::max_group_bytes (Rust)

A new public field with a with_max_group_bytes builder. It defaults to moq_net::group::MAX_CACHE_BYTES, and Encoder::new clamps it to that limit, since moq-net refuses a larger group anyway.
It replaces the encoder's private max_group_bytes field, which only in-crate tests could set.
The encoder refuses any snapshot past the budget with FrameTooLarge before parsing it into the baseline. This covers update and the snapshot() restate. Before, it parsed first and left the refusal to moq-net on write.

@moq/json snapshot Config.maxGroupBytes (JS): the existing option is no longer @internal. It is clamped the same way, and a snapshot past it throws FrameTooLarge. The API report is regenerated.

moq-stats

One config() builds every stats track's snapshot config. Under cfg(test) it lowers the budget to tests::MAX_GROUP_BYTES (64 KiB). oversized_paths sizes itself to that budget, so it builds about 17 paths instead of about 8,200.
The payload-length guard in Snapshot::update is removed. The encoder now refuses past the budget itself, and there is no compression left for the guard to cover. Two comments about compressed frames are updated.

Tests: a_snapshot_past_the_budget_is_refused (Rust and JS, with deltas on and off), and the_budget_is_clamped_to_the_cache_limit (Rust).

Decisions

Accept the smaller saving, or widen this PR with a test budget.
- Accept the smaller saving and plan the knob as a follow-up.
- ✅ Widen this PR: add the public test-budget knob, mirror it in JS, and use it in the moq-stats oversize tests. (Maintainer's choice.)

Chosen without asking, listed for review:

Knob shape: a snapshot::Config field. Config is #[non_exhaustive] and already carries deltas with a with_ builder, and JS already had maxGroupBytes on its snapshot Config. So this is the API that matches both. The window and stream encoders have no budget knob on main (moq-dev/moq#6987, which adds a private one, is still open), so there was nothing there to match.
Fully public in both languages, not #[doc(hidden)] or @internal. A caller may want a smaller group budget for reasons beyond tests, and the semantics are simple.
Clamped to MAX_CACHE_BYTES, not rejected. A larger budget would let deltas grow a group past what moq-net holds.
moq-stats uses a cfg(test) constant, not a field threaded through Task. Threading one through would touch every Task literal and the producer constructors for a test-only value. Every moq-stats test runs under the 64 KiB budget, and the whole suite passes with it.
moq-json's Producer keeps its own payload check. It also guards frames that did not come from this encoder's budget logic, so it is left as is.

Result

User+sys CPU of the debug moq-stats test binary, three standalone runs each:

Test	main	this branch
`oversized_frame_ends_the_track`	1.19–1.29 s	< 0.01 s
`an_oversize_group_leaves_other_groups_readable`	1.97–2.01 s	0.01 s

That saves about 3.2 s of CPU, close to the quest Goal's 4.5 s estimate for the whole moq-stats cohort.

Impact

Public API (additive): Rust snapshot::Config::max_group_bytes and Config::with_max_group_bytes. JS Snapshot.Config.maxGroupBytes moves from internal to public.
Behavior: snapshot::Encoder::update and snapshot() (Rust and JS) now return or throw FrameTooLarge for a snapshot past the budget. Before, they handed the frame back and moq-net refused it on write. The error and the published state are the same: the previous value stays readable.
Wire: none.

Validation

just check origin/main passed.
just ci origin/main passed: 4,945 Rust tests plus the JS, lint and docs gates.
cargo test -p moq-json -p moq-stats --lib passed (182 + 74 tests). bun test src/snapshot in js/json passed (69 tests).

Review

Harness: T3 Code delegate (Codex). Model: GPT-6.1 Sol, high reasoning effort.
Reviewed head: 5887398a3b941047da5bc92fe7cace8af32b7850.
Findings: none significant. It checked refusal-state recovery, delta rolls, Rust/JS parity, the moq-stats test budget, Config callers, and comments.
Head 2353028885 merges main into the reviewed head without conflicts, so it needs no new review. just check origin/main passed on it.

Follow-ups

moq-ffi's MoqJsonSnapshotConfig does not expose max_group_bytes. Add it if an FFI caller needs a smaller budget.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated · 2026-10-30T03:07:25Z

Design change: at the maintainer's request this PR is widened. moq_json::snapshot::Config gets a public max_group_bytes (JS maxGroupBytes, no longer internal), clamped to moq-net's cache limit. moq-stats lowers it to 64 KiB under cfg(test), so the oversize tests stop building 32 MiB of stats. The branch was rebuilt on main after #7046 removed per-record compression, so the compression-specific half of the first version is gone. The decisions are recorded in the PR body.

(Written by Claude Opus 5.5)

coderabbitai · 2026-10-30T03:07:37Z

Warning

Review limit reached

You'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 44 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 15b0a085-1d31-459e-88ad-a8fe1cf2486c

Commits

Reviewing files that changed from the base of the PR and between d33b591 and 2353028.

Files selected for processing: 7

js/json/api/index.api.md
js/json/src/snapshot/encoder.ts
js/json/src/snapshot/snapshot.test.ts
quest/m0/ci/README.md
quest/m0/ci/stats-oversize.md
rs/moq-json/src/snapshot/encoder.rs
rs/moq-stats/src/produce.rs

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

chatgpt-codex-connector · 2026-10-30T03:07:40Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-30T04:26:55.514425Z	`2353028`	New commits

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

kixelated

Automated review by review (OpenAI)

Reviewed commit 5887398a3b941047da5bc92fe7cace8af32b7850.

No actionable bugs found in the inspected changes. Rust rejects oversized snapshots before baseline parsing/replacement, and both implementations check restated snapshots as well as new ones. Delta admission retains the group budget and rollover behavior. The stats test-only limit still exercises the production FrameTooLarge abort path rather than replacing it with a test-specific error.

Direction: exposing the existing snapshot budget consistently is a reasonable, small alternative to constructing transport-sized test payloads or threading a test-only setting through every stats task. The public limit remains capped by the transport cache ceiling.

Verification: GitHub-only static review of Rust/JS encoders, producer integration and changed tests, including an independent Rust-side pass. I did not execute tests, type checks or benchmarks, and did not reproduce the reported CPU savings.

kixelated · 2026-10-30T04:10:15Z

Landing via /quest-merge.

Head 5887398a3b941047da5bc92fe7cace8af32b7850 is covered by the PR body's ## Review record (Codex, GPT-6.1 Sol), and the online reviews on it (Codex, OpenAI) found nothing actionable. No unresolved threads.
Required checks passed; only the optional CodeQL Analyze (rust) was still running.
quest check --base origin/main --branch quest/m0/ci/stats-oversize passed on the head; the PR deletes the finished quest and its link.
No code changes in this pass. Follow-up for /quest-plan: expose max_group_bytes on moq-ffi's MoqJsonSnapshotConfig if an FFI caller needs a smaller budget.

(Written by Claude Opus 5.5)

* chore(quest): claim quest/m1/resume-model-test.md Quest-Claim: 7bff411939571787 * test(net): check resume against a model of one logical track A seeded proptest drives an origin's fronts through route flips, route failures, give-ups, ranges, cache expiry, a newer epoch, and reader joins and leaves on the simulated clock, and checks every reader's groups, frames, drops, and end against a plain cursor on the logical track. Fixes what it found: - An ordered reader never ended once a finished track left nothing in its range, or the rest of the range was given up. - A drop poll with local bounds below the subscription's floor built a reversed span and panicked. - A resumed reader held a drop back for a replaced copy kept only to back a group still read, and was not woken once that read finished. * test(net): check JS resume against the logical-track model Mirror rs/moq-net's resume model as a fast-check property over the same ops, constants and checks, seeded by MOQ_RESUME_SEED and sized by MOQ_RESUME_CASES (default 64). It found three bugs, each fixed with a regression case: - A replacement copy's refusal ended a group the serving route still delivered; it is a verdict only once that route won't land it either. - A failed copy's cache eviction released a group recovering from another copy; only the copy a mirror reads from reclaims it. - A reported drop was final, so a group a later copy delivered was never handed out; the report is now once and not final. * ci: sweep the resume model nightly from a fresh seed * test(net): answer route fetches in the resume model A real route forwards a fetch for a group it lacks to the publisher. The model's routes refused every fetch, so a group continued across a failover read NotFound although the instance carried it. * test(net): let model routes hold a tail fill beside the live copy A frame-bounded fetch caches a partial copy of a group, so a later whole-group fetch must still be answered. A group a reader stitched from two copies is complete once any copy wrote its last frame. * fix(net): keep a recovered group when its old copy is let go Releasing a copy's subscription released every mirror it cached, including a group recovery had already moved to another copy, which then ended cleanly short of its frames. The JS model's routes now answer fetches, which exposed it. * quest: finish resume-model-test, plan delivering late groups below the resume point The maintainer kept the resume point policy on #6947; delivering the late lower groups is a nice-to-have follow-up in m2. * test(net): request the model's broadcast through the renamed origin API * test(net): resume readers only on a route Up or Down actually changes * test(net): restore Recover's refusal wait, and resume Rust readers only when the cheapest route changes The main merge took main's whole resume.ts and dropped the PR's Recover hunk. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

* chore(quest): claim quest/m1/quic-api/streams.md Quest-Claim: 3067217ff870abc5 * wip(quic): namespace moq-quic streams as send, recv, and stream send::{Stream, Offset, End, Options, Group, Groups, Error}, recv::{Stream, Lease, Chunks, Error}, and stream::{Id, Dir}. Writes take Buf, a lease moves its chunks out and holds no borrow, send ends stay queryable until released, and datagrams chain chunks without copying. * wip(qmux): use moq-quic send ends and Buf writes * wip(tokio): namespace the QUIC driver as send, recv, and stream send::{Stream, Finished, Group, Options, Offset, Error} with finish(self) typestate and consuming resets, recv::{Stream, Lease, Error} with a lease that owns its bytes and holds no lock, Buf writes and BufMut reads, and datagrams that chain a header. quic::web collapses onto one error per direction. moq-quic only reruns the stuck pass on restore when an out lease held it off. * wip(uring): namespace the QUIC driver as send, recv, and stream * wip(uring): port tests and moq-relay to the namespaced driver * wip(uring): format * wip(quic): name the namespaced stream API in the fork notes * wip(quic): fix a stale doc link * chore(quest): finish quic-api/streams * fix(quic): keep terminal stream state across the review's lifecycle gaps - moq-tokio and moq-uring cache a reset `received_reset` reports, since moq-quic frees the stream then, so later reads and leases fail with it instead of panicking. - moq-uring's web `poll_closed` observes a clean FIN again through an empty read, instead of a reset-only query that waits forever. - moq-uring's web `finish` keeps a finished stream's handle when called again. - qmux resets an unfinished stream on drop before releasing its ID, which refused the reset. * fix(quic): size a datagram before taking its payload Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

… on moq-dev pairs (#7073) * chore(quest): claim quest/m0/smoke-released.md Quest-Claim: 1a9318c095d636e0 * fix(smoke): hold the Cloudflare probe's track reader The probe dropped its TrackReader, so when the relay rejected an early SUBSCRIBE with DoesNotExist, cloudflare/moq-rs failed to close the track and resolved the subscription as done. The harness then failed the cell instead of retrying the readiness race. Also rewrite the smoke-released quest to the classified runner failures. * fix(smoke): gate the upstream runner on moq-dev pairs only Only moq-dev-rs and moq-dev-js against this checkout's relay fail the Upstream Docker interoperability job; every other pair is reported, as relays.sh treats non-moq-dev relays. A required pair the registry stops pairing fails too. With the Cloudflare probe fix this finishes the quest, so delete it. * fix(smoke): annotate optional runner failures; record the moq-rs close bug Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

* chore(quest): claim quest/m0/nightly/quic-rustdoc.md Quest-Claim: 0a0d799823f82f7f * fix(quic): unbreak moq-quic's all-features rustdoc The fuzz-gated bench map linked to the private SentPackets type, which fails rustdoc under -D warnings. Nightly's features lane already builds workspace rustdoc with --all-features, so no new lane is needed. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

* chore(quest): claim quest/m0/ci/tracing-filter.md Quest-Claim: a26a0b73687228f9 * wip: stop forcing tracing-test's no-env-filter * chore(quest): finish quest/m0/ci/tracing-filter.md * test: keep captured spans enabled beside parallel tests With one dispatcher registered, tracing-core caches a callsite's interest from the thread that first hits it. A parallel test without a subscriber then disabled the `srt` ingest span for the capturing test, so `publisher_reports_a_silent_pid` failed about two runs in three under `cargo test`. A second, no-op dispatcher makes every callsite ask the current thread's subscriber. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

* chore: point quest and rust2 references at moq-dev Both repositories moved from the kixelated account to the moq-dev organization. The flake input keeps the same revision and hash. Generated quest skills refresh with the next quest bump. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

* chore(quest): claim quest/m0/ci/mbx-r2-verify.md Quest-Claim: 9a556a77ba203b39 * quest(m0/ci): record the first mbx 1.28 Nightly in the R2 verify quest The bump's checks passed: every hosted Nightly job writes, macOS refused only an already oversized manifest, and the merge queue shows no E0463. Reuse is still absent on every platform; record the lockfile-identity evidence and narrow the quest to what remains. * ci(platform): install uniffi-bindgen-cpp after the Rust build The Nightly Windows build loaded the install's 124-prediction mbx manifest under its own identity on 2026-10-27 and 2026-10-28, which blocked inheriting the previous lockfile's manifest. Installing after the build lets the build inherit first. Reported upstream as jdx/mr-boxington#869. * quest(m0/ci): record upstream's CRLF cause for the Windows cache miss jdx/mr-boxington#870 traced the Windows miss to CRLF lockfile digests, not the uniffi-bindgen-cpp install, and the macOS build did inherit. * quest(m0/ci): revert the uniffi-bindgen-cpp step move, record settled decisions #870 showed the Windows miss comes from CRLF lockfile digests, not the install order, so platform.yml keeps main's order. Windows now waits on an mbx release with #870 and a pin bump; Linux full history waits on Windows hitting; lockfile churn past 8 states counts under the rebuild item. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

…pshot before parsing (#7072) * chore(quest): claim quest/m0/ci/stats-oversize.md Quest-Claim: 4a6d2df5874a8c48 * perf(json): refuse an oversized snapshot before parsing or compressing it * chore(quest): finish quest/m0/ci/stats-oversize.md * feat(json): a public snapshot group budget, refusing a snapshot past it before parsing snapshot::Config gains max_group_bytes (JS maxGroupBytes, no longer internal), clamped to moq-net's cache limit. The encoder refuses a snapshot past it before the baseline parse, so moq-stats' oversize tests lower it to 64 KiB under test instead of building 32 MiB of stats. * chore(json): regenerate the API report for maxGroupBytes Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

kixelated changed the title ~~moq-stats refuses an oversized plain snapshot early~~ perf(json): refuse an oversized snapshot before parsing or compressing it Oct 28, 2026

kixelated added quest:iterate and removed quest:iterate labels Oct 29, 2026

kixelated changed the title ~~perf(json): refuse an oversized snapshot before parsing or compressing it~~ feat(json): a public snapshot group budget, refusing an oversized snapshot before parsing Oct 30, 2026

kixelated added quest:ready and removed quest:iterate labels Oct 30, 2026

kixelated marked this pull request as ready for review October 30, 2026 03:07

kixelated removed the quest:ready label Oct 30, 2026

kixelated commented Oct 30, 2026

View reviewed changes

kixelated added the quest:merge label Oct 30, 2026

kixelated enabled auto-merge October 30, 2026 04:25

kixelated added this pull request to the merge queue Oct 30, 2026

kixelated added quest:merge and removed quest:merge labels Oct 30, 2026

kixelated enabled auto-merge (squash) October 30, 2026 06:28

kixelated and others added 8 commits October 30, 2026 08:32

moq-bot Bot force-pushed the quest/m0/ci/stats-oversize branch from 2353028 to 652ba00 Compare October 30, 2026 08:33

moq-bot Bot merged commit 652ba00 into main Oct 30, 2026
9 checks passed

moq-bot Bot deleted the quest/m0/ci/stats-oversize branch October 30, 2026 08:33

kixelated mentioned this pull request Nov 1, 2026

test(net): check refusal state cheaply and benchmark its scaling #7549

Merged

Navigation Menu

Repository navigation

feat(json): a public snapshot group budget, refusing an oversized snapshot before parsing - #7072

feat(json): a public snapshot group budget, refusing an oversized snapshot before parsing#7072
moq-bot[bot] merged 8 commits into
mainfrom
quest/m0/ci/stats-oversize

kixelated commented Oct 28, 2026 •

edited

Loading

Uh oh!

kixelated commented Oct 30, 2026

Uh oh!

coderabbitai Bot commented Oct 30, 2026 •

edited

Loading

Review limit reached

Uh oh!

chatgpt-codex-connector Bot commented Oct 30, 2026 •

edited

Loading

Uh oh!

kixelated left a comment

Uh oh!

kixelated commented Oct 30, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

kixelated commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

Decisions

Result

Impact

Validation

Review

Follow-ups

Uh oh!

kixelated commented Oct 30, 2026

Uh oh!

coderabbitai Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

chatgpt-codex-connector Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

kixelated left a comment

Choose a reason for hiding this comment

Uh oh!

kixelated commented Oct 30, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

kixelated commented Oct 28, 2026 •

edited

Loading

coderabbitai Bot commented Oct 30, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 30, 2026 •

edited

Loading