Skip to content

test(mux): skip truncated spliced groups as aborted - #4707

Merged
kixelated merged 4 commits into
mainfrom
quest/m1/mux-truncated-group
Oct 3, 2026
Merged

kixelated merged 4 commits into
mainfrom
quest/m1/mux-truncated-group

Conversation

@kixelated

@kixelated kixelated commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

A truncated spliced group must be skipped as aborted instead of emitting a short clean group boundary. The mux consumer had no regression covering this behavior after #4689.

Approach

Exercise the public origin route takeover path with a paused clock. After the continuation is pruned, assert that the mux skips directly to the next retained group without GroupEnd, then still emits GroupEnd for a complete successor.

Impact

  • No public API or wire changes; test only.

Alternatives

Use the public route path so moq-net resume internals stay private.

Validation

  • Focused Nix nextest regression passes.
  • Temporarily restoring pre-fix(net): a truncated spliced group fails instead of ending cleanly #4689 resume.rs makes the new mux assertion fail: the truncated group emits a clean boundary and advances only to sequence 1. Current source restored afterward.
  • Exact-head GitHub CI succeeds: Check, Test, Android, Windows, and macOS.
  • Local CARGO_BUILD_JOBS=2 nix develop --command just check completed with 719 passing tests and one failure in unchanged CLI integration test import_delivers_the_catalog_finish_at_eof at its 10-second announcement deadline; fail-fast left 1611 tests unrun. No blind rerun.
  • Static comparison against exact base 48d89430d confirms identical CLI, relay, Tokio, and moq-net trees, identical relay fixture and media input, and identical mux production code. The only Rust delta is this cfg(test) unit test, which is absent from the CLI integration binary. This establishes source equivalence; the timeout was not reproduced on a separate base checkout.

Follow-ups

No mux design follow-ups. Investigate the CLI announcement deadline separately. The quest remains tracked: automatic approval review rejected removal because the local broad check failed, despite successful exact-head CI and the unrelated source-equivalent fixture.

(written by GPT-6)

kixelated and others added 2 commits October 1, 2026 21:51
Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Implemented the public-path mux regression. Focused Nix nextest passes; restoring the pre-#4689 resume implementation makes this mux assertion fail when the truncated group emits a clean boundary. The source is restored, and the PR contains only the regression.

Full CARGO_BUILD_JOBS=2 nix develop --command just check remains running in a detached persistent process. No API or wire changes. The quest remains tracked pending that validation and its completion cleanup.

(written by GPT-6)

@kixelated

Copy link
Copy Markdown
Collaborator Author

Completion audit at head 47855682c41a98f84a2f0d0508a9b19459e9d294:

  • Focused mux regression passes and fails against temporarily restored pre-fix(net): a truncated spliced group fails instead of ending cleanly #4689 behavior.
  • GitHub Check, Test, Android, Windows, and macOS all succeed at this exact head.
  • Local full check finished: 719 passed, 1 failed, 1611 unrun. Failure is moq-cli::import import_delivers_the_catalog_finish_at_eof, announcement timeout at rs/moq-cli/tests/import.rs:63.
  • Exact-base 48d89430d comparison shows identical CLI, relay, Tokio, and moq-net source trees, identical relay fixture/input, and identical mux production source. This unit-test addition is not compiled into the failing CLI integration binary. No baseline runtime reproduction or blind rerun was performed.
  • Quest cleanup remains deferred: automatic approval review rejected removal because the local broad check did not pass. The draft remains open with no API/wire changes; investigate the CLI deadline separately.

(written by GPT-6)

@kixelated
kixelated changed the base branch from release to main October 2, 2026 22:03
@kixelated
kixelated marked this pull request as ready for review October 3, 2026 16:00
@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

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 36 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
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c7440611-e7fa-45dd-9aab-7f04d3f78fe3
📥 Commits

Reviewing files that changed from the base of the PR and between 41d132f and 2917349.

📒 Files selected for processing (3)
  • quest/m1/README.md
  • quest/m1/mux-truncated-group.md
  • rs/moq-mux/src/container/consumer.rs
  • Autopilot · 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.

kixelated and others added 2 commits October 3, 2026 09:15
Main dropped track::Producer::used(); the regression test now awaits
demand().used(). The mux truncated group quest is done, so delete it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Merge prep at 2917349ba:

  • Merged main in. Main dropped track::Producer::used() (8c62e88), so the test now awaits track.demand().used(). Focused test passes locally.
  • Deleted the completed quest /quest/m1/mux-truncated-group.md and its quest/m1/README.md entry, per the maintainer. quest check passes.
  • The PR description's note that the quest stays tracked is now stale; the quest is removed in this PR.
  • The local moq-cli import_delivers_the_catalog_finish_at_eof timeout is unrelated to this test-only change; left as a follow-up.

Enabling auto-merge on this head once CI is green.

(Written by Claude Opus 5.5)

@kixelated
kixelated enabled auto-merge (squash) October 3, 2026 16:18
@kixelated

Copy link
Copy Markdown
Collaborator Author

Review (head 2917349bafd84e5ac880faece410c9b7e4aff344)

Test-only regression for the #4689 truncated-splice path, plus quest cleanup. No production code changes.

Blocking

None.

Non-blocking

  1. rs/moq-mux/src/container/consumer.rs (loop 1..=4) — The four cheaper takeovers are load-bearing against moq-net’s private MAX_SEGMENTS = 3 (original + 4 segments → prune the first two, so group 0 aborts and group 1 never reaches the mux). A one-line comment tying the bound to “enough takeovers to prune past the seam” would keep the next MAX_SEGMENTS bump from looking like a random flake.
  2. CI — Check/Test/Android/Windows/macOS/Quest were still pending on this head at review time; prior head was green per the PR notes.

The paused-clock origin takeover path is the right public surface: after the seam is pruned, poll_event yields the successor frame (ts(200_000), current == 2) with no intervening GroupEnd, then still emits GroupEnd for the complete group. demand().used() matches main after Producer::used() went away. Quest file + README entry removal matches a completed XS.

MERGE

This is an automated review, not the maintainer's decision
(Written by Grok)

@kixelated
kixelated merged commit 73c08de into main Oct 3, 2026
7 checks passed
@kixelated
kixelated deleted the quest/m1/mux-truncated-group branch October 3, 2026 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant