feat(json): default delta_ratio to 8, count only delta bytes#1765
Merged
Conversation
Make the snapshot/delta JSON producer emit deltas by default instead of requiring an explicit opt-in. The delta_ratio config goes from Option<f64>/number-or-undefined (default disabled) to a plain number defaulting to 8, where 0 now means disabled. Rust uses u32 since the ratio is a coarse knob with no need for fractional precision; JS keeps number. The budget now measures only the accumulated delta bytes against ratio x snapshot size, excluding the snapshot frame. Previously the snapshot counted toward the budget, which made a ratio of 1 behave identically to 0 (the snapshot alone filled the budget, so no delta ever fit). Now ratio 1 means "deltas may total up to one snapshot before rolling", genuinely distinct from disabled. The catalog producers (rs/moq-mux and js/publish) explicitly pass 0 for now to stay byte-compatible with consumers that only read snapshots; the delta rollout is a follow-up. Adds tests documenting the byte-budget threshold on both sides. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
WalkthroughThe delta ratio configuration for JSON producers is changed in both the Rust ( 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@rs/moq-json/src/lib.rs`:
- Around line 56-75: The Config struct is public and allows direct construction
by consumers using struct-literal syntax. To make future field additions
non-breaking, add the `#[non_exhaustive]` attribute to the Config struct before
the existing `#[derive(Debug, Clone)]` attribute. This ensures that adding new
optional fields to Config in the future will not break existing code that
constructs it, since callers will be required to use the Default implementation
or an explicit constructor rather than direct struct literals.
🪄 Autofix (Beta)
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: CHILL
Plan: Pro
Run ID: 58f15f08-5811-438b-9aba-3b21dd1ac850
📒 Files selected for processing (5)
js/json/src/json.test.tsjs/json/src/producer.tsjs/publish/src/broadcast.tsrs/moq-json/src/lib.rsrs/moq-mux/src/catalog/producer.rs
Comment on lines
+56
to
75
| #[derive(Debug, Clone)] | ||
| pub struct Config { | ||
| /// Controls whether the producer emits deltas (merge patches) instead of full snapshots. | ||
| /// Controls how aggressively the producer emits deltas (merge patches) instead of full snapshots. | ||
| /// | ||
| /// `None` disables deltas: every change is published as a new snapshot group. | ||
| /// A ratio of `0` disables deltas: every change is published as a new snapshot group. | ||
| /// | ||
| /// `Some(ratio)` enables deltas. A delta is appended to the current group as long as the | ||
| /// group's total size stays within `ratio` times the size of a fresh snapshot; otherwise a | ||
| /// new snapshot group is started. A larger ratio tolerates bigger groups before snapshotting. | ||
| pub delta_ratio: Option<f64>, | ||
| /// A positive ratio enables deltas. A delta is appended to the current group as long as the | ||
| /// accumulated deltas (excluding the snapshot frame) stay within `ratio` times the size of a | ||
| /// fresh snapshot; otherwise a new snapshot group is started. So `1` allows deltas totalling up | ||
| /// to one snapshot before rolling, and a larger ratio tolerates more deltas per snapshot. | ||
| /// | ||
| /// Defaults to `8`. | ||
| pub delta_ratio: u32, | ||
| } | ||
|
|
||
| impl Default for Config { | ||
| fn default() -> Self { | ||
| Self { delta_ratio: 8 } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add #[non_exhaustive] to the Config struct.
The Config struct is public and constructed by consumers. Without #[non_exhaustive], adding new optional fields later becomes a breaking change for callers using struct-literal syntax.
Proposed fix
/// Configuration for a [`Producer`].
+#[non_exhaustive]
#[derive(Debug, Clone)]
pub struct Config {Based on coding guidelines: "Add #[non_exhaustive] to config structs that consumers construct, and provide a Default or constructor so new optional fields stay additive".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #[derive(Debug, Clone)] | |
| pub struct Config { | |
| /// Controls whether the producer emits deltas (merge patches) instead of full snapshots. | |
| /// Controls how aggressively the producer emits deltas (merge patches) instead of full snapshots. | |
| /// | |
| /// `None` disables deltas: every change is published as a new snapshot group. | |
| /// A ratio of `0` disables deltas: every change is published as a new snapshot group. | |
| /// | |
| /// `Some(ratio)` enables deltas. A delta is appended to the current group as long as the | |
| /// group's total size stays within `ratio` times the size of a fresh snapshot; otherwise a | |
| /// new snapshot group is started. A larger ratio tolerates bigger groups before snapshotting. | |
| pub delta_ratio: Option<f64>, | |
| /// A positive ratio enables deltas. A delta is appended to the current group as long as the | |
| /// accumulated deltas (excluding the snapshot frame) stay within `ratio` times the size of a | |
| /// fresh snapshot; otherwise a new snapshot group is started. So `1` allows deltas totalling up | |
| /// to one snapshot before rolling, and a larger ratio tolerates more deltas per snapshot. | |
| /// | |
| /// Defaults to `8`. | |
| pub delta_ratio: u32, | |
| } | |
| impl Default for Config { | |
| fn default() -> Self { | |
| Self { delta_ratio: 8 } | |
| } | |
| } | |
| #[non_exhaustive] | |
| #[derive(Debug, Clone)] | |
| pub struct Config { | |
| /// Controls how aggressively the producer emits deltas (merge patches) instead of full snapshots. | |
| /// | |
| /// A ratio of `0` disables deltas: every change is published as a new snapshot group. | |
| /// | |
| /// A positive ratio enables deltas. A delta is appended to the current group as long as the | |
| /// accumulated deltas (excluding the snapshot frame) stay within `ratio` times the size of a | |
| /// fresh snapshot; otherwise a new snapshot group is started. So `1` allows deltas totalling up | |
| /// to one snapshot before rolling, and a larger ratio tolerates more deltas per snapshot. | |
| /// | |
| /// Defaults to `8`. | |
| pub delta_ratio: u32, | |
| } | |
| impl Default for Config { | |
| fn default() -> Self { | |
| Self { delta_ratio: 8 } | |
| } | |
| } |
🤖 Prompt for 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.
In `@rs/moq-json/src/lib.rs` around lines 56 - 75, The Config struct is public and
allows direct construction by consumers using struct-literal syntax. To make
future field additions non-breaking, add the `#[non_exhaustive]` attribute to
the Config struct before the existing `#[derive(Debug, Clone)]` attribute. This
ensures that adding new optional fields to Config in the future will not break
existing code that constructs it, since callers will be required to use the
Default implementation or an explicit constructor rather than direct struct
literals.
Source: Coding guidelines
kixelated
enabled auto-merge (squash)
June 16, 2026 18:24
5 tasks
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.
Summary
Make the snapshot/delta JSON producer (
rs/moq-jsonand@moq/json) emit deltas by default instead of requiring an explicit opt-in, and fix the budget so it counts only the delta bytes.delta_ratiodefault is now8(was disabled). The config changes fromOption<f64>/number | undefined(default disabled) to a plain number defaulting to8, where0now means disabled. Rust usesu32(the ratio is a coarse knob with no need for fractional precision); JS keepsnumber. Deltas are never even encoded when the ratio is0.accumulated_delta_bytes + new_delta > ratio × snapshot_size. Previously the snapshot counted toward the budget, which made a ratio of1behave identically to0(the snapshot alone filled the budget, so no delta ever fit). Now1means "deltas may total up to one snapshot before rolling", genuinely distinct from disabled.delta_ratio: 0for now (rs/moq-mux/src/catalog/producer.rsandjs/publish/src/broadcast.ts) to stay byte-compatible with consumers that only read snapshots. Turning deltas on for the catalog is a follow-up.Semantics
With default
8: a group keeps appending deltas until they total more than 8× the current snapshot size, then it rolls a fresh snapshot. A late joiner reads the snapshot at frame 0 plus the deltas (still also bounded by the existing 256-frame cap).Public API
moq_json::Config::delta_ratio:Option<f64>→u32(breaking, butmoq-jsonhas no external consumers yet).@moq/jsonConfig.deltaRatio: stillnumber?, semantics changed (0disables, default8).Tests
deltas_stay_within_ratio_times_snapshot(Rust) / "deltas stay within ratio times snapshot" (JS) documenting the exact byte-budget threshold: with 7-byte frames, ratio 8 admits 8 deltas on top of the snapshot (group 0 = 9 frames) before the 9th delta rolls.tight_ratio_rolls_snapshotsto show ratio 1 now lands one delta per group then rolls (distinct from 0), and the deltas-off tests to pass0explicitly.Test plan
cargo test -p moq-json(23 pass)cargo build -p moq-mux, clippy + fmt clean (via nix)bun testinjs/json(37 pass), biome clean(Written by Claude)