Repository navigation
Conversation
Quest-Claim: 8dc91da8afd60b0c
…moq-congestion-lab GCC and SCReAMv2 move to the workspace-only moq-congestion-lab crate, BBR3's delay bound gets a hidden setter, and moq-ffi's MoqCongestionControl becomes an object with constructors so a new algorithm is additive in every binding. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ort feature doc(hidden) is banned, so the pacer, a packet's GCC probe cluster, and BBR3's delay bound sit behind moq-congestion's documented lab feature, as test support does elsewhere. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…w the move Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
ℹ️ No blocking issues. One small doc gap is noted below.
Reviewed changes
I reviewed all four commits: the candidates move into a workspace-only crate, the lab test-support feature, and the conversion of the FFI congestion selector from an enum to an object in every binding.
moq-congestion-labcrate: GCC and SCReAMv2 move over unchanged except for imports and theprobe_clusteraccessors. The crate ispublish = false, and moq-quic and moq-tokio take it only as a path dev-dependency. It has its own large-MTU window test.labfeature onmoq-congestion: this feature gatespub mod pacer, thePacketprobe-cluster field and its accessors, and thebbr3::Config::delay_boundsetter. The license dropsBSD-3-Clause, and the unusedtracingdependency is removed.MoqCongestionControlas a uniffi object: it hasloss(),delay()andnone()constructors and exportsDebug/Eq/Hash.MoqQuicConfig.congestion_controlbecomesOption<Arc<_>>. Dart is regenerated. Go gets builder functions and a genericoptional[T]. Kotlin, Python, Swift, OBS C++ and the tests switch to the constructors.- Selector docs:
moq_tokio::quic::CongestionControlnow derivesHash. The docs in the relay config, the C API and the bindings saydelayis BBR v3 and the default. - Nightly and quests: the
rust-testgroup adds-p moq-congestion-lab, so the GCCbenchmark_per_packetstill runs and still matches its filter. The completed quest is removed, and the vendor-sync andquic-realtimequests point at the new paths.
I checked locally: cargo check --all-targets passes with no warnings for moq-congestion without lab, moq-congestion with --features lab, and moq-congestion-lab. A grep finds the old enum spellings (CongestionControl.DELAY, kLoss, CongestionControlLoss, and so on) only in historical MIGRATE.md entries.
ℹ️ doc/setup/repository.md doesn't list moq-congestion-lab
AGENTS.md sends contributors and agents to doc/setup/repository.md to find where a feature lives. Its "Congestion controllers and controller tests" row still lists only rs/moq-congestion. After this PR, someone looking for GCC or SCReAMv2 there will look in the wrong crate.
Technical details
# repository.md omits the new lab crate
## Affected sites
- doc/setup/repository.md:21 — the "Congestion controllers and controller tests" row links only `rs/moq-congestion`
## Required outcome
- The row also points to `rs/moq-congestion-lab` as the home of the candidate controllers under test (GCC, SCReAMv2)claude-opus-5-5 | 𝕏
|
No actionable comments were generated in the recent review. WalkthroughThe changes add Merge Risk:
|

Problem
moq-congestion 0.1.0 has never been published, yet its public API carried three candidates no session can select: GCC (
gcc, about 16k vendored WebRTC lines under BSD), SCReAMv2 (scream), and BBR3's delay bound (bbr3::Config::delay_bound). The maintainer's pick on moq-dev/moq#8764: ship BBR through one selector, keep the candidates off the published API until tested, and make new algorithms additive. moq-ffi'sMoqCongestionControlwas a closed enum, so a new algorithm would break every Swift, Kotlin, and Dart caller that switches over it.Completes
quest/m2/congestion-candidates.md.Approach
rs/moq-congestion-lab(publish = false). moq-quic's sim comparisons and moq-tokio's real-socket SCReAM tests take it as a path dev-dependency, whichcargo publishdrops. It gets its own tests (the large-MTU window check), and the nightlyrust-testbench group now includes it.#[doc(hidden)]is banned (sh/text.sh), so what the lab needs from moq-congestion sits behind a documented test-support feature,lab, the waysimandfuzzdo elsewhere: the pacer module, a packet's GCC probe-cluster accessors, and the delay-bound setter. Withoutlab, the probe-cluster field isn't compiled.moq_tokio::quic::CongestionControl(#[non_exhaustive], relay/CLIcongestion_control). Its docs now say plainly thatdelayis BBR v3 and the default, and that a new algorithm is added as a variant named after it.MoqCongestionControlbecomes an immutable uniffi object withloss(),delay(), andnone()constructors, following theMoqAudioCodecprecedent, with Eq/Hash/Debug exported. A new algorithm is a new constructor, which breaks no binding. Wrappers, tests, and docs are updated in Swift, Kotlin, Dart (bindings regenerated), Python, Go, C++ (OBS plugin), and moq-ffi.scream/webrtc,webrtcREADME,quic-realtime).quic-realtime's stale "addRealtime" naming item is aligned with the newer decision to name each winner after its algorithm.Impact
moq-congestion(unpublished): removesgccandscream(nowmoq_congestion_lab::{gcc, scream}, workspace-only).bbr3::Config::delay_boundnow requires the newlabfeature, which also exposespacerandPacket::{probe_cluster, set_probe_cluster}as test support. The license is nowMIT OR Apache-2.0(the BSD-3-Clause code left with GCC).moq-tokio:quic::CongestionControlalso derivesHash(additive). Docs only otherwise.moq-ffi(breaking):MoqCongestionControlis an object instead of an enum.MoqQuicConfig.congestion_controlisOption<Arc<MoqCongestionControl>>. Migration:.delay/.loss/.none->CongestionControl.delay()/.loss()/.none()CongestionControl.DELAY->CongestionControl.delay()CongestionControl.delay->CongestionControl.delay()CongestionControl.DELAY->CongestionControl.delay()CongestionControlLoss/Delay/Noneconstants are gone; usemoq.LossCongestionControl(),moq.DelayCongestionControl(),moq.NoneCongestionControl(), assigned directly (noptr(...))moq::CongestionControl::kDelay->moq::CongestionControl::delay(); compare witheq()loss/delay/nonestring)delay,loss,none).Choices made without the maintainer (for review)
moq-congestion-lab.labfeature, not#[doc(hidden)](banned): the Goal's "not reachable from any published crate's public API" is met for applications, but the setter, pacer, and probe-cluster accessors can still be reached through an opt-in test-support feature, the same rule moq-dev/moq#8392 set forsim/fuzz. Alternatives rejected: deleting the delay-bound sim tests (loses the bake-off's harness), or compilingpacer.rsinto the lab via#[path](still needs a feature for the other two).BSD-3-Clausefrom moq-congestion's license: the vendored WebRTC files and theirLICENSE/PATENTSmoved to the lab.bbr3/delay.rsreimplements GoogCC's trend detector in its own structure, keeping only its constants. If a reviewer reads it as a derivative work, restore the expression and copy the notice beside it.MoqCongestionControlas an object (theMoqAudioCodecshape) rather than wrapper-only types in Swift/Kotlin/Dart, so every binding grows the same way and the API names match across languages.rust-test/moq-congestion/gcc/...torust-test/moq-congestion-lab/gcc/..., so they start a new history.Validation
just check(changed scope: rs incl. tests, go, kt, dart, cpp, obs_compile, c, python, text, nix, quest): passes.just rs test -p moq-congestion -p moq-congestion-lab -p moq-quic: 1134 passed, includingdelay_bound_keeps_a_shorter_queue,rises_to_target_under_delay_bound, and the lab's GCC/SCReAM tests; moq-tokio SCReAM and moq-ffi config tests (client_connects_with_every_knob_set, rung) pass.just py test(128 passed),just go check(wrapper tests incl. reconnect withLossCongestionControl()),just kt check,just dart check, and the OBS settings test, all on the new constructors. Swift wasn't run here (no toolchain on Linux); CI's macOS lane covers it.cargo package --list -p moq-congestioncontains nogcc/orscream.rs;cargo package -p moq-congestion-labrefuses (publish = false).just rs test --release -p moq-congestion-lab -E 'test(benchmark_per_packet)' --run-ignored only).cargo docwith-D warningspasses for moq-congestion with and withoutlab.Alternatives
Follow-ups
None.
Models
Size: S
Implementer: Claude Code, claude-opus-5-5, unknown effort (arm A)
🤖 Generated with Claude Code
(Written by Claude Opus 5.5)
Review
Rewritten by
quest reviewafter each round.Head: 7678090
Base: origin/main at d09ca93
Risk: medium,
the base has no RISK.md to name what is always high or customer-facing, so every change is mediumClassified: medium by default
Reviewer: Codex, gpt-6.1-sol, medium effort
Session:
01a1dfcc-cfd7-75e1-8793-d48ad864e19e, round 1Score: 9/10,
The controller extraction and binding updates preserve behavior with no actionable defects identified, though the source-only review did not include builds or tests.Rounds: 1; first round 0; findings 0: fixed 0, refuted 0, open 0
Findings: none