[kixelated]

Problem

When finish() refused truncated MKV input, it returned before ending tracks. Subscribers could wait indefinitely while the importer remained held.

Approach

Keep EOF draining's result, fail the cut-short frame with its reason, release the initial catalog reservation, and finish every track before returning the first error. Completed groups stay readable. Completes and removes quest/m1/mkv-truncated-finish.md and its milestone link.

Impact

• Public API: no signature or type changes. finish() now ends accepted content even when it returns Mkv(Truncated).
• Wire: none.

Alternatives

Aborting all tracks was rejected by the settled quest plan because it would discard accepted groups and differ from a refused decode().

Choices made without asking

• Finish every track even if another track's finish fails, preserving the original EOF error when present.
• Cover truncation inside a real Opus packet and its trailing metadata at four chunk sizes, plus two empty tracks before the first block. Use paused time and immediate polls to assert completion while holding the importer.

Validation

• Reproduced the held-importer hang before the fix: the new fixture test failed immediately at “finish must end the track while the importer is held”.
• quest gate just rs test -p moq-mux: 1,957 passed, 3 skipped. Includes the new lifecycle tests and the cut-short frame's StreamError::Internal reason.
• quest check --base origin/main: 624 documents OK.
• quest gate just ci origin/main: failed. mbx could not spawn cargo (E2BIG) in Rust docs and the lint-enforcement fixture. Other selected checks passed. This run did not reach a namespace-denial failure; no bypass was used.

Follow-ups

No new follow-ups. The validation limit is already tracked by moq-dev/moq#7594.

Review

Claude Code through T3 delegation, Claude Opus 5.5 (claude-opus-5-5), medium configured (reviewer self-reported low effort). Reviewed head e7942391c0f158e957a5f82355dcf9c591f86875 against origin/main: no findings. The reviewer confirmed the source fix preserves the EOF error, accepted groups, and the cut-short frame failure.

(Written by GPT-6.1 Sol)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: e794239

No actionable bugs found. Keeping the EOF error while releasing the catalog reservation and finishing every track fixes the lifecycle problem at its source. The eager Result::and call still visits every track, and aborting only the cut-short frame preserves completed groups without adding a second teardown mechanism. The held-importer tests cover payload truncation, trailing metadata, and empty tracks.

Verification limits: static inspection of the importer, producer lifecycle, and tests only. I did not run tests or builds, or independently reproduce the validation reported in the PR.

(Written by OpenAI)

[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: 84cdb528-8125-4927-9b40-354dc2c2cec6

Commits

Reviewing files that changed from the base of the PR and between e3bd4b2 and e794239.

Files selected for processing: 3

• quest/m1/README.md
• quest/m1/mkv-truncated-finish.md
• rs/moq-mux/src/container/mkv/import.rs

Files with no reviewable changes: 2

• quest/m1/README.md
• quest/m1/mkv-truncated-finish.md

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

Import::finish now retains drain errors, aborts an incomplete block, releases the initial reservation, and attempts to finish every track while preserving the first error. Tests cover truncated input at multiple cut positions and before any block. The README entry and planning document for this behavior were removed.

Merge Risk: Minimal · up to e7942

No merge-blocking risk is established; the reported full CI run did not complete.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: finishing accepted MKV tracks after truncated input.
Description check		The description directly explains the truncated MKV track-completion problem, the implementation approach, impact, alternatives, and validation results.
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 33.33% which is insufficient. 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 3 functions across 1 files.

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

[pullfrog]

✅ No new issues found.

Reviewed changes

• finish() ends tracks after truncated input: Import::finish keeps drain_eof's result, aborts the cut-short frame's group, releases initial_reservation, and finishes every track before returning the first error, so a refused truncation no longer leaves subscribers waiting while the importer is held (rs/moq-mux/src/container/mkv/import.rs:838).
• Return value preserved: drain_eof's error wins, otherwise the first track-finish error; the change only alters side effects, so no caller sees a different result.
• Tests: truncation_fails_the_frame_cut_short now asserts the cut-short frame fails synchronously with StreamError::Internal; two paused-time tests cover truncation inside an Opus packet and its trailing BlockGroup metadata across four chunk sizes, and empty tracks releasing the catalog reservation before ending.
• Quest completion: removes quest/m1/mkv-truncated-finish.md and its quest/m1/README.md entry.

I built and ran the MKV import module (cargo test -p moq-mux --lib container::mkv::import::tests): 14 passed, 0 failed.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}