[kixelated]

Completes and deletes quest/m2/quic-realtime/gcc.md, and removes its links from the realtime uplink epic and the WebRTC epic.

Summary

libwebrtc's Google Congestion Control now runs as moq_congestion::Gcc, driven by receive timestamps (draft-ietf-quic-receive-ts). It is the GCC candidate in the realtime uplink bake-off.

• Vendored. goog_cc 0.1.4 (WebRTC da2a3d6c4d, 2024-12-09) lives in rs/moq-congestion/src/gcc/ with WebRTC's BSD LICENSE, PATENTS and AUTHORS. The first commit imports it verbatim, so later commits show the refresh and the trim as diffs.
• Refreshed to WebRTC 556f7eec27 (2026-11-01). The revision is recorded in the module docs. Changes taken from upstream:
  • LossBasedBweV2's new defaults (hold, byte loss rate, bounded best candidate, use in start phase, padding duration) and its tie-break.
  • The 1.1 pacing factor once transport feedback arrives.
  • ALR probing gated on that feedback, and allocation probes outside ALR.
  • The ALR detector on Timestamps.
  • Removal of the link capacity tracker, LossBasedBandwidthEstimation and GetExpectedBwePeriod.
  • A background agent ported every upstream unit test change for the code that remains.
• Trimmed to what QUIC has. Removed: RTCP receiver reports and REMB, route changes, the network state estimate, separate audio detection, RTP sequence numbers, ECN feedback, padding, the video encoder settings, and the AcknowledgedBitrateEstimator that only a disabled field trial selects. Each file's docs say what it lost. The port's ALR detector stamped the wall clock; it now uses send times, so tests run on mocked time.
• Adapter (gcc/mod.rs).
  • Each ACK becomes transport feedback carrying the peer's receive times, mapped onto the local clock through i128 and clamped within an hour of it. Each loss becomes feedback of lost packets.
  • GoogCC's 25ms process timer runs from the controller's calls.
  • The window is GoogCC's congestion window, which its congestion-window pushback works from: target × (RTT + 350ms). pacing_rate is GoogCC's pacing rate and bandwidth_estimate is its target rate.
  • A port of WebRTC's BitrateProber paces each probe cluster and reports what the cluster still needs, in whole packets, as probe_inflight, so moq-quic's native probing fills it. The cluster id rides in Packet's padding: size_of::<Packet>() stays 40.
• Native probing fills a cluster at GoogCC's pace, and aims GoogCC at the target.
  • A new Controller::paces_probe (default false, GCC true) tells moq-quic that the controller sizes and paces its own probe. moq-quic then fills it at the controller's pace instead of holding the duplicates to the session's probe target. GoogCC's clusters (15ms or 5 packets at twice the estimate) bound the overhead. BBR3 keeps the target cap.
  • A new Controller::on_probe_target (default no-op) hands the controller the session's probe target. moq-quic reports it on every set_probe_target change, to the previous path's controller while a migration validates, and to each controller it builds on migration. GCC feeds it to OnMaxTotalAllocatedBitrate through a port of upstream's OnStreamsConfig, so GoogCC probes toward the target and caps its probes at twice it.
  • GoogCC's maximum rate is 1 Gbit/s when the transport sets no pacing limit, the default SendSideBandwidthEstimation already used, so ProbeController no longer caps every probe at 5 Mbit/s.
  • GoogCC starts at 1 Mbit/s rather than WebRTC's 300 kbit/s, so its initial probes go out at 3 and 6 Mbit/s. "Start" below says why.
• Tests.
  • Every vendored unit test that still applies, plus gcc::alr_detector::test::sub_millisecond_sends_refill_the_budget for the review fix below.
  • Adapter tests on mocked time over a simulated uplink: finds the link and keeps the queue short; backs off a drop to a quarter; falls back to ACK arrival without timestamps; filled probes raise the estimate far past what media alone reads; probes_aim_at_the_target, where a 4 Mbit/s target holds filled probes between the target and twice it on a link where they otherwise reach 18.7 Mbit/s; hostile receive times. The hostile-time test caught an overflow panic, now fixed.
  • A per-packet benchmark next to BBR3's.
  • Nightly tests::probe::rises_to_target_under_gcc (renamed from rises_toward_target_under_gcc), at 4 and 8 Mbit/s targets: asserts that every seed's estimate reaches the target and that the probe's own budgets hold, and reports the rise time.
  • tests::probe::paced_probe_fills_past_the_target: a controller that paces its own probe gets its probe_inflight filled at once, while one that does not is held to the target. gcc::tests::probe_inflight_rounds_up_to_a_packet covers the whole-packet remainder.

Measurements

These were measured before this round's 1 Mbit/s start and 1 Gbit/s maximum, so GCC's rows may shift; the shared comparison in the epic re-runs every candidate. They come from the sibling quest's harness, tests::realtime in moq-dev/moq#7544: sim replicas of cellular-step, cellular-bursty and cellular-deep, with the uplink dropping at 10s and restoring at 20s, and a bulk upload averaged over 8 seeds. I added a GCC candidate in a local copy. That file is not on main, so this PR does not carry the row; whichever of the two PRs lands second adds its candidate. The local patch swaps DELAY_BOUND for:

const GCC: Candidate = Candidate {
	name: "GCC",
	transport: || transport::Config {
		congestion: Arc::new(gcc::Config::default()),
		receive_timestamps: NonZeroU32::new(TIMESTAMPS),
		..transport::Config::default()
	},
};

preset	controller	queue p50	p95	p99	p99 before the drop	utilization	after the drop	peak	reaction	drained	dropped	share beside CUBIC
cellular-step	BBR3	47ms	384ms	445ms	78ms	95%	96%	482ms	9463ms, never 5	7718ms, never 6	3	18%
cellular-step	GCC	0ms	198ms	339ms	13ms	59%	98%	500ms	1226ms	4034ms	24	12%
cellular-bursty	BBR3	66ms	155ms	182ms	88ms	96%	96%	208ms	4571ms	4656ms	0	25%
cellular-bursty	GCC	0ms	265ms	388ms	27ms	77%	98%	463ms	1131ms	4603ms, never 1	0	13%
cellular-deep	BBR3	114ms	530ms	658ms	125ms	94%	98%	685ms	never	never	0	11%
cellular-deep	GCC	0ms	73ms	728ms	21ms	65%	89%	1047ms	1158ms	1385ms	0	3%

What the table shows:

• Steady queue. Before the drop, GCC holds the queue near empty: its p99 is 13–27ms where BBR3's is 78–125ms.
• Reaction. GCC paces under the dropped rate in about 1.2s. BBR3 takes 4.5–9.5s on step and bursty, and never does on deep.
• Utilization. The cost is utilization, 59–77% against BBR3's 94–96%. After the 2s initial probing, GoogCC climbs additively near its link-capacity estimate and holds within 1.5× the acknowledged rate, so after the drop the restore takes many seconds.
• The drop itself. The drop still costs a 460ms–1s peak, because the congestion window allows the target rate times 350ms of queue.
• Share beside CUBIC. GCC gives way to the CUBIC flow: 3–13%.
• Sender model. This harness uploads in bulk. GoogCC is built for an encoder that follows its target rate, so these numbers understate it there. The shared comparison runs both sender models (decided 2026-11-01).

Stopped rung under native probing, re-measured on this head. This is rises_to_target_under_gcc, BBR3's rises_to_target scenario on 8 seeds: constant 2 Mbit/s media, a 3 Mbit/s path that opens to 12 Mbit/s at 5s, and 20s runs.

target	estimate at the end, probing	without a target	rise after the path opens	overhead	late frames (baseline)	p99 (baseline)
4 Mbit/s	6.2 Mbit/s on every seed	3.1 Mbit/s	5.0s on every seed	0.38%	1 (1)	73.7ms (72.8ms)
8 Mbit/s	12.0 Mbit/s on every seed	3.1 Mbit/s	9.0s on every seed	0.87%	1 (1)	73.7ms (72.8ms)

A stopped rung now recovers at both targets, so the quest's goal is met. The previous head reached 4 Mbit/s 7.9s after the path opened and never reached 8 Mbit/s. The rise follows GoogCC's schedule: once the target is above the estimate, GoogCC probes from the application-limited region five seconds after it last probed or entered that region, at twice its estimate but no more than twice the target, and the 8 Mbit/s target takes a second such probe. As decided, the test asserts only that the target is reached, not BBR3's 5s budget.

Start. Lifting the 5 Mbit/s cap exposed a crawling start, in the baseline as much as while probing: the estimate fell to 0.7 Mbit/s within a second and climbed back at 8% a second, with 264 of 450 frames late on every seed. A trace showed why. From 300 kbit/s the initial probes took several round trips to reach the path, while 2 Mbit/s of media queued behind the pacer. Draining that backlog, once a probe found the path, queued it at the bottleneck, and the delay-based estimate cut the rate to 85% of the throughput acknowledged so far, which was still the startup rate. The media then held GoogCC out of the application-limited region, so no probe followed. Under the 5 Mbit/s cap, the capped probe happened to land after the cut and restore the estimate.

I compared candidate retunes on the same scenario, sweeping the first path's rate from 2 to 4.8 Mbit/s:

retune	late frames on the 3 Mbit/s path	rise at 4 / 8 Mbit/s	elsewhere in the sweep
none (1 Gbit/s maximum alone)	264	never / never	also crawls on a 3.6 Mbit/s path
end the initial probing at the first probe the path saturates (further_probe_threshold 0.875)	146	never / never	still crawls on a 3.6 Mbit/s path
initial probes stepping 1.5x rather than 2x	1	0.8s / 3.7s	166–276 late on 2.4 and 2.7 Mbit/s paths, worse than the old cap
start at 1 Mbit/s (chosen)	1	5.0s / 9.0s	1 late on every path from 2.4 to 4.8 Mbit/s
the previous head's 5 Mbit/s cap, for reference	8	7.9s / never	52 late on a 2.4 Mbit/s path, 16 on 2.7

The 1 Mbit/s start is the only retune that held across the sweep, and it is one constant. On a 2 Mbit/s path, equal to the media, the 300 kbit/s variants had 380 late frames probing or not; the 1 Mbit/s start has 1 in the baseline but 59 while probing (see Follow-ups).

Per-packet cost, from gcc::bench, the median of three runs with an ACK per packet:

in flight	GCC	BBR3
64	410ns	200ns
1000	1.5µs	200ns
16000	1.3µs	200ns

Most of the cost is RobustThroughputEstimator scanning its window of up to 500 packets on every ACK. At 32 packets per ACK GCC costs about 85ns per packet.

Public API and wire

• New Controller::paces_probe, a provided method returning false, and Controller::on_probe_target, a provided no-op. Existing controllers are unaffected, and the change is additive.
• New moq_congestion::gcc module, with gcc::Config as its factory, and a new moq_congestion::Gcc. This follows the namespacing of moq-dev/moq#7258, which has landed; the merge ports GCC to its Factory, Metrics, and the removal of into_any.
• Nothing selects GCC yet. CongestionControl::Realtime is the epic's job once a candidate wins.
• The crate's license becomes (MIT OR Apache-2.0) AND BSD-3-Clause for the vendored code. BSD-3-Clause is already on deny.toml's allow list.
• No wire change. Packet gains a private field that fits its existing padding.

Choices made without asking

• Vendored dependencies. Dropped goog_cc's approx and test-trace dev-dependencies (a local macro replaces approx) and added tracing to moq-congestion.
• ALR probing on (requests_alr_probing). This lets an application-limited sender probe, the mechanism by which native probing can raise the estimate.
• Network becomes available at the first ACK, and a probe cluster starts on a sent packet of 200 bytes or more (WebRTC starts one on queued media). Otherwise the initial probes ran during the handshake and were discarded across the idle round trip after it.
• A probe cluster the sender runs out of data for restarts under a new id with its next packet. WebRTC instead drops a cluster whose pacer falls 10ms behind, but WebRTC pads; here only the transport's duplicates can.
• Packets acknowledged without a receive time. Until the peer reports one, the ACK's arrival stands in for every packet's. Once it has, a packet without one still counts as received for the loss-based estimate, but is marked with upstream's ambiguous_receive_time so the delay-based estimate and the RTT skip it.
• burst() is the pacer's own budget, or one packet during a probe cluster. moq-quic checks a datagram's pace while the packet before it is still being built; under 2 Mbit/s the budget is one packet, so a batch of two left on one packet's credit, at twice the pacing rate. The shared pacer's fix is quest/m1/pacer-low-rate.md.
• The target rate keeps WebRTC's DropFrame:true. bandwidth_estimate is the loss-based target, and the pushback's frame-drop ratio has no consumer.
• The paced fill is a Controller method. moq-quic must know which controllers bound their own probe, since BBR3 relies on the target cap, so a provided paces_probe() -> bool carries that.
• The start retune is the starting rate, 1 Mbit/s, chosen over the probe-threshold and step-size knobs by the sweep above. gcc::Config still has no settings.
• The maximum is 1 Gbit/s, the default GoogCC's loss-based estimate already used. A transport's max_outgoing_bytes_per_second still replaces it.
• A new target waits for the controller's next call that knows the time, since on_probe_target takes none and GoogCC timestamps its probes.
• rises_to_target_under_gcc runs 20s rather than 15s, so the 8 Mbit/s rise, 14s in, has margin.
• The ALR detector departs from upstream on intervals. At 556f7eec27 upstream rounds each interval between sends to whole milliseconds, so sends under half a millisecond apart never refill its budget. The port rounds each send time instead, as upstream did before its send times became Timestamps, so the intervals add up to the elapsed time (sub_millisecond_sends_refill_the_budget).
• Probe cluster short ids are split in half. Packet stores 16 bits, so GoogCC's ids wrap within 1–32767 and the prober's restart ids within 32768–65535, and the two never alias.

Decisions

Settled by the maintainer on 2026-11-01:

1. How native probing raises GCC's estimate to the probe target.
   • ✅ A. The transport fills a probe cluster at the controller's own pace rather than capping the duplicates at the target (Controller::paces_probe).
   • B. Add a Controller hook carrying the session's probe target, fed to OnMaxTotalAllocatedBitrate.
   • C. Accept that GCC cannot recover a stopped rung.
2. The sender model for the shared comparison.
   • ✅ The PR's recommendation: an encoder retuned to bandwidth_estimate, as moq-cli and moq-ffi do, alongside bulk. The epic README's remaining work already runs both sender models in one harness.
3. GoogCC's 5 Mbit/s probe cap, the limit on recovering a rung above 5 Mbit/s.
   • A. Configure GoogCC's maximum rate and tune its initial probing around it.
   • ✅ B. A, plus hand GoogCC the session's probe target through a new Controller hook fed to OnMaxTotalAllocatedBitrate. Implemented as Controller::on_probe_target, a 1 Gbit/s maximum, and a 1 Mbit/s start.
   • C. Accept the cap.
4. Whether BBR3's 5s rise budget applies to GCC.
   • A. Hold GCC to 5s by shortening GoogCC's ALR probe interval.
   • ✅ B. Assert only that the estimate reaches the target, and report the rise time in the bake-off. Implemented in rises_to_target_under_gcc: 5.0s at 4 Mbit/s, 9.0s at 8 Mbit/s.
   • C. Keep the test as a measurement.

Follow-ups

• GoogCC's per-ACK cost (RobustThroughputEstimator's window scan, and the feedback vectors it sorts and copies) if GCC wins.
• Real-socket runs over the moq-shaper presets, beside the near-zero noise floor, once the shared comparison has a real-socket harness.
• If GCC wins, tune WebRTC-CongestionWindow's 350ms queue for the uplink presets.
• Without a target, a probe cluster that no frame fills restarts under a new id with each frame, so it never ends. WebRTC pads such a cluster, or drops it once its pacer falls behind. Worth bounding the restarts, for example at PROBE_CLUSTER_TIMEOUT, if GCC wins.
• GCC under an unreachable target. In the start sweep, on a 2 Mbit/s path equal to the media, the probing run had 59 late frames against the baseline's 1. BBR3 backs off a target that misses (backs_off_unreachable_target); GoogCC's ALR probes keep their own cadence. Worth a GCC case beside unreachable_target if GCC wins.

Review

quest review --deep --base origin/main: Codex, gpt-6-astra, medium effort, in a new session for this round's design.

• Round 1, head f5c1af4: one finding, fixed. The ALR detector rounded each interval between sends to whole milliseconds, so sends under a millisecond apart never refilled its budget (alr_detector.rs:86), a behavior upstream shares since its own move onto Timestamps. Fixed in d09d1dc by subtracting whole-millisecond send times, as upstream's integer times did before that move, and covered by sub_millisecond_sends_refill_the_budget, which fails on the old rounding.
• Round 2, head d09d1dc: no findings.

Online reviewers (Pullfrog) on cfb42fd, two threads, both addressed in 88dad74:

• gcc/mod.rs:532: restart ids (1 << 30 plus a count) aliased GoogCC's cluster 16384 plus that count in the 16-bit short id. Fixed: GoogCC's ids wrap within the lower half and restarts within the upper half, covered by gcc::tests::restart_ids_never_alias_googcc_ids; the Packet.probe_cluster docs in rate.rs say so.
• gcc/alr_detector.rs:84: the comment claimed upstream equivalence, but upstream rounds the interval. Fixed the comment to state the deliberate departure. Behavior kept, since upstream's rounding never refills the budget for sends under half a millisecond apart.

Local review skipped for 88dad74 under the simple-review rule (moq-dev/quest#364): a fix of a few dozen lines in private code, touching no public API, wire format, security code, or unsafe. The head adds a conflict-free merge of origin/main that touches quests alone.

Local checks on 88dad74: cargo nextest run -p moq-congestion --lib (348 passed), cargo clippy -p moq-congestion --all-targets -D warnings, cargo fmt --check, and quest check.

Earlier local checks on d09d1dc, after merging origin/main: cargo nextest run -p moq-congestion -p moq-quic --lib (all passed), cargo clippy -p moq-congestion -p moq-quic --all-targets -D warnings, cargo fmt --check, cargo doc with -D warnings, quest check, and the probe suite in release with its nightly cases (cargo test --release -p moq-quic --lib tests::probe:: -- --include-ignored, 22 passed, BBR3's unchanged). The full just check was not run, to keep the host's nearly full disk clear.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[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 4 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: 104d0668-92b5-4ee0-9455-82a458111920

Commits

Reviewing files that changed from the base of the PR and between 88e6990 and 7cb1e4a.

Files ignored due to path filters: 1

• Cargo.lock is excluded by !**/*.lock

Files selected for processing: 51

• quest/m2/quic-realtime/README.md
• quest/m2/quic-realtime/gcc.md
• quest/m2/webrtc/README.md
• rs/moq-congestion/Cargo.toml
• rs/moq-congestion/README.md
• rs/moq-congestion/src/gcc/AUTHORS
• rs/moq-congestion/src/gcc/LICENSE
• rs/moq-congestion/src/gcc/PATENTS
• rs/moq-congestion/src/gcc/alr_detector.rs
• rs/moq-congestion/src/gcc/api/mod.rs
• rs/moq-congestion/src/gcc/api/transport/bandwidth_usage.rs
• rs/moq-congestion/src/gcc/api/transport/mod.rs
• rs/moq-congestion/src/gcc/api/transport/network_types.rs
• rs/moq-congestion/src/gcc/api/units/data_rate.rs
• rs/moq-congestion/src/gcc/api/units/data_size.rs
• rs/moq-congestion/src/gcc/api/units/frequency.rs
• rs/moq-congestion/src/gcc/api/units/mod.rs
• rs/moq-congestion/src/gcc/api/units/time_delta.rs
• rs/moq-congestion/src/gcc/api/units/timestamp.rs
• rs/moq-congestion/src/gcc/api/units/unit_base.rs
• rs/moq-congestion/src/gcc/bench.rs
• rs/moq-congestion/src/gcc/congestion_window_pushback_controller.rs
• rs/moq-congestion/src/gcc/delay_based_bwe.rs
• rs/moq-congestion/src/gcc/delay_increase_detector_interface.rs
• rs/moq-congestion/src/gcc/experiments/field_trials.rs
• rs/moq-congestion/src/gcc/experiments/mod.rs
• rs/moq-congestion/src/gcc/experiments/rate_control.rs
• rs/moq-congestion/src/gcc/goog_cc_network_control.rs
• rs/moq-congestion/src/gcc/inter_arrival_delta.rs
• rs/moq-congestion/src/gcc/link_capacity_estimator.rs
• rs/moq-congestion/src/gcc/loss_based_bwe_v2.rs
• rs/moq-congestion/src/gcc/mod.rs
• rs/moq-congestion/src/gcc/pacing/bitrate_prober.rs
• rs/moq-congestion/src/gcc/pacing/interval_budget.rs
• rs/moq-congestion/src/gcc/pacing/mod.rs
• rs/moq-congestion/src/gcc/probe_bitrate_estimator.rs
• rs/moq-congestion/src/gcc/probe_controller.rs
• rs/moq-congestion/src/gcc/remote_bitrate_estimator/aimd_rate_control.rs
• rs/moq-congestion/src/gcc/remote_bitrate_estimator/bwe_defines.rs
• rs/moq-congestion/src/gcc/remote_bitrate_estimator/mod.rs
• rs/moq-congestion/src/gcc/robust_throughput_estimator.rs
• rs/moq-congestion/src/gcc/send_side_bandwidth_estimation.rs
• rs/moq-congestion/src/gcc/tests.rs
• rs/moq-congestion/src/gcc/trendline_estimator.rs
• rs/moq-congestion/src/lib.rs
• rs/moq-congestion/src/pacer.rs
• rs/moq-congestion/src/rate.rs
• rs/moq-quic/README.md
• rs/moq-quic/src/connection/bandwidth_probe.rs
• rs/moq-quic/src/connection/mod.rs
• rs/moq-quic/src/tests/probe.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 commit: cfb42fd2b3e73bd70937434cd91ccbc77a5581d0.

No actionable bugs found in the inspected adapter, feedback estimators and native-probing integration. Keeping GCC opt-in for the comparison is appropriate. The provided probe hooks preserve other controllers' behavior and reuse transport-owned duplication rather than introducing a second sending path. The documented startup/probing tradeoffs need the planned real-socket and encoder-following comparison before choosing a default.

Verification: static inspection of integration paths, estimator logic and regression tests. Tests, benchmarks and fuzzing were not run; the full vendored port's equivalence to upstream and the reported performance measurements were not independently verified.

(Written by gpt-6.1-sol)

[pullfrog]

ℹ️ No critical issues. Two minor findings inline, both in code nothing selects yet.

Reviewed changes

• Vendored GoogCC (rs/moq-congestion/src/gcc/). goog_cc 0.1.4 imported, refreshed to WebRTC 556f7eec27, trimmed to a QUIC sender (no RTCP/REMB, ECN, padding, AcknowledgedBitrateEstimator), keeping WebRTC's BSD LICENSE/PATENTS/AUTHORS; the crate license becomes (MIT OR Apache-2.0) AND BSD-3-Clause.
• Adapter (gcc/mod.rs, Gcc). ACKs become transport feedback with peer receive times mapped onto the local clock through a fixed receive_basis and the RECEIVE_SKEW clamp, losses become lost packets, and a 25ms process timer drives GoogCC. Window is GoogCC's congestion window, pacing_rate its pace, bandwidth_estimate its target. A ported BitrateProber paces each probe cluster and reports probe_inflight.
• Controller API. Two additive provided methods, paces_probe() -> bool (default false) and on_probe_target(u64) (default no-op), plus the gcc::Config factory and pub use gcc::Gcc. Packet gains a private probe_cluster that fits its existing padding.
• moq-quic probing. set_probe_target forwards the target to the current, previous, and newly built path controllers, and the native probe skips its own target bucket when the controller paces its own probe.
• Tests, bench, quests. Vendored unit tests, mocked-time adapter sim tests over a simulated uplink, a per-packet bench, nightly tests::probe::rises_to_target_under_gcc, and the quest bookkeeping that deletes gcc.md.

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

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 7cb1e4a68215e7dc683d127824377d68e5c76de5.

No new actionable bugs found in the changes since my previous review. The compact-ID split is a simple, appropriate fix for the independently reported restart/GoogCC collision: both packet tagging and history lookup use the disjoint ranges, with regression coverage for boundaries and wrap. The ALR documentation finding is also addressed by explicitly documenting the intentional upstream departure; that behavior is unchanged.

Excluded the base branch's shared-version planning changes. Verification was static inspection of the changed mapping, its callers and tests; tests were not run. The earlier opt-in direction and performance-verification limits still apply.

(Written by gpt-6.1-sol)

[pullfrog]

✅ No new issues found.

Reviewed changes

The two commits since the prior pullfrog review (cfb42fd) apply the fixes to both inline findings, with no behavior beyond them. I re-verified the id-splitting arithmetic and ran the affected tests (cargo test -p moq-congestion --lib, both new tests pass).

• Restart cluster ids kept off GoogCC's. short_id now maps GoogCC's ids into 1..=32767 and the prober's restart ids (BitrateProber::RESTART_ID_BASE, now pub) into 32768..=65535, so a restarted cluster never shares a short id with one of GoogCC's. gcc::tests::restart_ids_never_alias_googcc_ids covers both ranges and the wrap boundaries, and rate.rs's probe_cluster docs describe the split instead of the old "far fewer than wrap" premise.
• ALR comment corrected. alr_detector.rs now documents send_time.ms() - last_send_time.ms() as a deliberate departure from upstream's rounded interval rather than claiming equivalence. Behavior unchanged.

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