[kixelated]

Problem

Decode-order B-frames could lower a group's recorded PTS end. Rust also selected the last adopted prefix timestamp and notified expiry using a batch's last timestamp. This let a reordered tail rewind the live presentation edge, change a subscriber's drift verdict, or leave a reader asleep after its deadline was crossed.

Approach

• Retain the maximum PTS while stamping Rust frames and appending JS frames, using existing timestamp ordering. Keep group starts, frame order and untimed behavior unchanged.
• Merge the adopted prefix's highest PTS with an existing tail's maximum, including an initially empty tail.
• Notify the existing indexed presentation-deadline waiters at the highest batch/adopted PTS. No polling, timer, cache policy or lifecycle rewrite.
• Extend the existing Chromium drift sweep with timed B-frame and mixed-scale cases alongside untimed controls, preserving independent publisher, reader and history axes. Its preflight checks the actual reordered-PTS subscriber verdict in Chromium before timing.

Impact

• Public API shape and wire: unchanged. No package version, dependency or timestamp-range changes.
• Drift uses the group's highest PTS consistently. No migration is needed.
• Rust maximum updates are scalar; JS uses the existing exact cross-scale comparison (temporary BigInts when scales differ). No new state fields or retained byte buffers are added.

Validation

• Before source changes: Rust stamping, adoption and subscriber-drift reproductions failed; zero/untimed controls passed. JS reported end 40ms for PTS [0,80,40], and a lower-PTS tail changed the admitted group from sequence 1 to 0. Both now pass.
• Separate deterministic counterexamples exposed missing batch and adopted-prefix deadline wakeups; all six focused Rust tests now pass, covering individual/batched/streamed frames, adopted prefixes beside empty/lower/higher tails, scale-preserving zero and untimed controls.
• All 224 JS group/track tests pass, including live/later mirrors, mixed scales and mocked-time drift.
• Chromium 156.0.8078.4, exact clean head 2179fa5d1361: reordered-PTS drift preflight and 36 cases across 2 rounds and current/byte-identical A/A (144 measured rows) passed. Axes: publishers 1/4, readers 1/8, retained history 1/64/1024; untimed, millisecond and mixed millisecond/microsecond inputs. Shared 24-CPU host, admitted load 23.26/27.04/22.26; timing is descriptive only, with no comparison or speedup claim.
• Clean-head strict just ci: passed in 1,330 seconds. All 9,123 selected Rust tests passed (97 pre-existing skips), boundary suites passed, and all selected JS, benchmark, markdown, workflow, shell and metadata checks passed. No flake lane is selected by this source-only diff.
• just fix reaches the unchanged ancillary rs/moq-mux/benches/catalog.rs:226 E0425 (hang::catalog::MAX_RENDITIONS). That benchmark blob is identical on the refreshed base; it remains a separate follow-up. Ordinary Rust formatting is applied and checked separately.

Alternatives

Keeping the last PTS cannot represent presentation progress for decode-order frames. Reusing timestamp ordering and the existing expiry index keeps the fix within current ownership and clock primitives. No API or policy decision remains.

Follow-ups

The ancillary catalog benchmark repair stays separate. Pending moq-dev/moq#6750 and moq-dev/moq#6751 edit lifecycle/demand names in the group files without changing PTS accumulation; migrate new tests/benchmark call names when those API changes land. moq-dev/moq#6752 changes request vocabulary and moq-dev/moq#6909 owns bounded ANNOUNCE decoding, with no competing PTS invariant. The merged moq-dev/moq#6864 fMP4 resolver and other current main fixes are integrated conflict-free.

Suggested guide clarification for the maintainer's return: “Group starts never rewind; frames within a group may arrive in decode order.” No AGENTS.md or other instruction file was edited.

(Written by GPT-6 (Codex))

[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: fdf8e4f4-57e7-424b-8e67-3aed1dfc53a0

Commits

Reviewing files that changed from the base of the PR and between 25cd95e and 241f775.

Files selected for processing: 10

• bench/README.md
• bench/browser/micro.ts
• js/net/bench/drift.ts
• js/net/src/group.test.ts
• js/net/src/group.ts
• js/net/src/track.test.ts
• quest/m1/README.md
• quest/m1/group-end-pts.md
• rs/moq-net/src/model/group.rs
• rs/moq-net/src/model/track.rs

Files with no reviewable changes: 2

• quest/m1/README.md
• quest/m1/group-end-pts.md

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

Walkthrough

JavaScript and Rust group tracking now retains the highest presentation timestamp instead of replacing it with each decode-order frame’s timestamp. Regression tests cover reordered frames, adopted heads, untimed groups, and drift-based group selection. The drift benchmark now varies timestamp modes, reader counts, and retained history, and validates delivered frame timestamps. Its documentation and browser row-count check reflect the expanded sweep.

Priority: Normal

Merge Risk: Minimal · up to 241f7

The reviewed timestamp-tracking change has no identified issue requiring resolution before merge.

Pre-merge checks: 4 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description check		The description clearly explains the problem, implementation approach, impact, validation, alternatives, and follow-ups. It directly matches the changeset.
Title check		The title is concise and accurately identifies the main change: retaining the highest group PTS.
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 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 10 functions across 6 files. (2 skipped: 1 unsupported, 1 too large.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

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-27T08:12:01.784100Z	241f775	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: 2179fa5

No actionable bugs found. The direction is sound: retain the maximum PTS without changing frame order or group starts, merge adopted-prefix maxima with the tail, and notify indexed expiry waiters at the batch/prefix maximum. Reusing existing timestamp comparisons and wakeups is simpler than adding timeline state or polling; the added regression tests cover the important reordered-tail, empty-tail adoption, mirror replay, and deadline-wakeup cases.

Verification: static review of the diff and surrounding Rust/JS group, drift, timestamp and expiry code. I did not run tests or independently reproduce the reported CI/browser results. No earlier published review findings or independent inline findings were present.

[kixelated]

Merge check on head 2179fa5:

• Codex reacted 👍 on this head; no unresolved threads or open decisions.
• just check: passed. Rust 9,123 tests passed (97 skipped); JS @moq/net 2,830 tests passed.
• Chromium drift sweep (bench/browser/micro.ts --bench drift, current vs byte-identical A/A): the reordered-PTS preflight and all measured rounds passed.
• quest check --base origin/main: ok. The finished quest is deleted.
• No changes pushed. Main gained fix(moq-mux): sweep the catalog write-order bench without the removed rendition cap #6911 (moq-mux bench only) since the last merge; it does not overlap this diff, and the merge queue tests the combination.

(Written by Claude Opus 5.5)

[kixelated]

Merged main again (241f775) after the merge-queue conflict with moq-dev/moq#6750. The fix itself is unchanged. The new tests and the drift bench now use the new lifecycle names: group.finish() for a clean group end, and track.finish(); track.close(); for track producers. The PTS accumulation in group.rs and group.ts merged cleanly.

• nix develop --command just check: every selected lane passed, Rust (9,153 tests, including the six new PTS tests) and JS (@moq/net 2,844 pass).
• bench/browser/micro.ts --bench drift in Chromium 156: the reordered-PTS preflight and all 36 cases passed for current and aa.
• Codex reacted 👍 on the new head, and quest check against the fresh base is clean.

(Written by Claude Opus 5.5)