[kixelated]

Finishes quest/m2/bus-minus-several.md.

Summary

Mix::minus takes any number of strips, so a guest with a camera and a screen share gets a return feed without either copy of their own sound (moq-dev/studio#37).

• minus<'a>(&self, strips: impl IntoIterator<Item = &'a Strip>) -> Minus, changed in place (no second method).
• Strips are deduplicated when the mix-minus is built, so listing one twice removes it once.
• Driver::advance subtracts each removed source the parent routes: one subtraction per removed source per block, still cheaper than a second sum.
• The unit test now covers two removed sources, a duplicate, a routing change, and a removed source the parent stops routing, on the bus driver's own clock (no sleeps).
• minus collects its strips before locking the mixer, so an iterator that reads the bus (such as one filtered by Mix::routes) cannot deadlock.
• Deletes the quest and its link in quest/m2/README.md.

Impact

Breaking for moq_audio::bus, which is new since release, so this costs no extra major.

• Migrate: mix.minus(&strip) becomes mix.minus([&strip]); pass every strip to remove, e.g. mix.minus([&camera, &screen]).
• No wire impact; no FFI or JS counterpart exists.

Choices made without asking

• impl IntoIterator<Item = &Strip> over &[&Strip]: callers write [&a, &b] either way, and the iterator also takes &Vec<Strip> or an iterator of a guest's strips without collecting.
• The removed set is fixed at construction; drop and rebuild the Minus to change it. No consumer needs a mutable set yet.
• An empty set is accepted and equals the parent; it is not malformed.
• rs/moq-audio/MIGRATE.md still says mix.minus(&strip): AGENTS.md reserves that file for the release cut, so the migration note is above. The CHANGELOG line now says the mix-minus drops "any set of its sources".

Overlap

moq-dev/moq#8273 also edits rs/moq-audio/src/bus.rs and CHANGELOG.md; this PR touches only minus, its state, its test, and a separate CHANGELOG line, so the two should merge cleanly.

Checks

just check passes locally.

Caused by

No single PR: minus was built for one source from the start.

Models

Size: S
Implementer: Claude Code, Claude Opus 5.5, medium effort
Reviewer: Codex, gpt-6.1-sol, high effort

Review

Rewritten by quest review after each round.

• Reviewer: Codex, gpt-6.1-sol, medium effort
• Session: 01a1bc43-9003-78c1-a2eb-0653c9353c86, round 2
• Head: 04cac92
• Base: origin/main at 9f51fa0
• Score: 9/10, The new head only merges base changes and leaves the previously reviewed implementation unchanged, with no actionable findings; builds and tests were not run as instructed.
• Findings: none

🤖 Generated with Claude Code

(Written by Claude Opus 5.5 with high effort)

[coderabbitai]

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 21 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

• 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: 07873f34-a6cd-4887-a04a-5f6e32a1935a

Commits

Reviewing files that changed from the base of the PR and between aabd6d7 and 04cac92.

Files selected for processing: 5

• quest/m2/README.md
• quest/m2/bus-minus-several.md
• rs/moq-audio/CHANGELOG.md
• rs/moq-audio/benches/bus.rs
• rs/moq-audio/src/bus.rs

• 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

[kixelated]

Automated review by review (OpenAI)

Reviewed head b2df562. No actionable findings.

Exclusions are deduplicated by strip identity before taking the mixer lock, so caller-provided iterators can query routing without deadlocking. Each block subtracts only sources still routed by the parent, preserving routing changes and parent lifetime. The expanded regression covers multiple exclusions, duplicates and an iterator that reads the bus.

Direction: retain the single iterator-based minus API and collect before locking. Verification limits: GitHub source/diff review only; no builds, tests or audio runs performed.

[pullfrog]

ℹ️ No critical issues — one minor measurement suggestion inline.

Reviewed changes

This run reviewed the full PR at b2df562a0 (4 commits against main at 0f14fee), and all 24 bus::tests pass locally.

• Mix::minus takes many strips — minus<'a>(&self, strips: impl IntoIterator<Item = &'a Strip>) replaces the single-strip form, deduplicating by Arc identity before allocating a Minus.
• Strips collected before the lock — the iterator is consumed and deduplicated outside Mixer, so an iterator that calls Mix::routes cannot deadlock on the mixer mutex.
• Driver::advance subtracts each removed source — MinusState::excluded is now a Vec<Strip>; each block subtracts every excluded source the parent still routes, so routing changes and the "parent stopped routing" case still hold.
• Test, bench, changelog, quest — the unit test now covers two removals, a duplicate, a routing change, an unrouted removal, and an iterator that reads the bus; the bench call site is updated; the changelog bullet says "any set of its sources"; the finished quest and its milestone-README line are deleted with no leftover references.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Automated review by review (OpenAI)

Reviewed head e17121d incrementally from b2df562, accounting for main's positioned/live channel changes. No new actionable findings.

The mix-minus behavior remains intact after the merge. The updated benchmark now sweeps 1, 4, and 16 excluded sources across 16 and 64 source mixes, using distinct wrapped strip selections and retaining the minus handles during measurement.

Verification limits: GitHub-only static review. No builds, tests, audio runs, or benchmarks executed; scaling claims were not measured.

[pullfrog]

✅ No new issues found.

Reviewed changes

This run reviewed the delta since Pullfrog's review at b2df562a0: the reworked bus_minus benchmark, its module docs, and the main merge.

• bus_minus sweeps the removed set — the benchmark now builds each guest's mix-minus from (0..removed) wrapped strips and sweeps removed ∈ {1, 4, 16} across 16 and 64 sources, holding the Minus handles until after each measurement, so the per-removed-source subtraction loop in Driver::advance is actually exercised (including the case where a minus removes every source).
• Bench docs describe both axes — the module header now states the sweep and the expected scaling, replacing the single-axis description.
• main merged, minus semantics preserved — the merge brings #8273's Live/Positioned split into the module header and the test helpers, but leaves minus, MinusState::excluded, and the subtraction loop untouched; no conflict markers, the quest and its milestone line are still gone with no stale references, and rustfmt --check on the bench passes. CI on this head is green, and its test action compiles benches via --benches --all-targets.

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