[kixelated]

Problem

Whole-frame batch writes used to clone every payload into a temporary entry array. moq-dev/moq#7139 has since landed that fix on main: write_frames now moves each payload, and only groups with shared canonical content stage the batch. What main still lacks is coverage at the batch boundary and for refusals partway through a batch.

Changes

• Bench: group_write_frames also sweeps 32 and 33 frames, so the sweep crosses the 32-frame write-batch boundary.
• Tests:
  • a_mismatched_frame_is_refused checks that a timestamp refusal on a batch's tail leaves the whole buffer in place, publishes no prefix, and keeps the group alive. It runs for both timed and untimed groups.
  • a_refused_shared_write_fails_the_group (renamed from a_refused_compressed_write_fails_the_group) checks that a shared-content conflict after a matching prefix fails the group. It runs for both plain and compressed groups.

Public API: none. Wire: none.

Merge resolution

The branch conflicted with moq-dev/moq#7139 in write_frames. I kept main's version: it already moves payloads, and it also skips canonicalize for groups without shared content. This PR's single-pass version would only have dropped the staging step for shared-content groups, a small gain on a rare path that wasn't worth a second rewrite. Both carried-over tests pass against main's code.

Validation

• cargo test -p moq-net --lib -- group:: passed (93 tests).
• cargo clippy -p moq-net --all-targets and cargo fmt --check are clean.

The earlier benchmark table measured this branch's original rewrite against 53c0dcd685. It no longer applies, because main now carries the equivalent fix.

Follow-ups

The independent codec benchmark-composition effect is handled by moq-dev/moq#7153.

(Written by GPT-6.1 Sol, merge resolution by Claude Opus 5.5)

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

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: b8d1d264-d252-475e-9b78-bcbb2acdd00a

Commits

Reviewing files that changed from the base of the PR and between 66bdbb9 and 6aab648.

Files selected for processing: 2

• rs/moq-net/benches/group.rs
• rs/moq-net/src/model/group.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

Walkthrough

The group write tests now cover batched timestamp mismatches for timed and untimed groups, and shared-content conflicts for compressed and uncompressed groups. The benchmark frame-count sweep adds 32 and 33.

Priority: Low

Merge Risk: Minimal · up to 6aab6

The changes expand test and benchmark coverage without an identified merge-blocking risk.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly identifies the test changes for batch-boundary coverage and mid-batch write refusals.
Description check		The description directly explains the benchmark and test changes, their purpose, validation, and lack of public API or wire changes.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 2 files.

Finishing Touches

✨ Simplify code

• Commit to this branch
• Create a new PR

• 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

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-29T15:55:27.132156Z	6aab648	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: 00e1cba.

No actionable bugs found, including an independent review of the batch-write failure path. Recoverable validation remains before draining; canonical frame indices stay anchored to the unchanged next_index until the batch commits. A shared-content conflict remains group-fatal, with the partial stored prefix and remaining drained inputs released through the existing abort/drain contracts.

Moving payloads directly is simpler than maintaining separate staged/shared-content implementations. The 32/33-frame benchmark boundary and valid-prefix/conflicting-tail tests target the important risks. The reported performance results appropriately distinguish the noisy small case from the larger batch improvement.

Verification: static review of the diff and surrounding validation, frame-buffer, canonicalization, metadata conversion, and abort/accounting code. I did not run tests or reproduce benchmarks.

[kixelated]

Merge summary: no changes needed from the merge pass.

• Head 00e1cbaeba79e778da0fc662d70022c9f054e043: all 11 checks pass and the merge state is clean against main.
• Reviews: Codex reacted 👍 (no findings), and the automated review from the maintainer's account found no actionable bugs. Both count under CONTRIBUTING.md.
• quest check --base origin/main passes. This isn't a quest branch, so no quest file needed to change.
• No open decisions. The parallel delivery-path PR moq-dev/moq#7141 is still open. Whichever of the two lands second will merge main in if they conflict.

Landing with a squash merge through the queue.

(Written by Claude Opus 5.5)

[kixelated]

Design change: main's moq-dev/moq#7139 already moves batched payloads into group storage. The merge with main keeps its write_frames, so this PR now adds only the 32/33-frame bench sweep and the mid-batch refusal tests. The PR body records why.

(Written by Claude Opus 5.5)

[kixelated]

Landing summary: I merged main into this branch and resolved the write_frames conflict by keeping main's version from moq-dev/moq#7139, which already moves payloads. This PR now adds the 32/33-frame bench sweep and the mid-batch refusal tests. Codex reviewed the new head with no findings, quest check passes, and the merge with main is clean.

(Written by Claude Opus 5.5)