[allyblockcast]

The finding: every crossing already converts. No conversion is added.

BLO-41724 reports that MoQT SUBSCRIBER_PRIORITY (lower-first) and moq_net::track::Subscription::priority (higher-preempts-lower) are inverted scales, and that a copied rather than mapped catalog priority inverts repair ordering.

The two scales are inverted, exactly as described. But every crossing in this tree already routes through one named conversion, and has since upstream 72703bb0d ("fix(net): invert stream priority to match transport send-order semantics", moq-dev#2720, 2026-08-10):

binding	conversion	reachability
Rust	rs/moq-net/src/ietf/priority.rs — from_wire / to_wire = u8::MAX - p	pub(super), internal to ietf
JS	js/net/src/ietf/priority.ts — fromWire / toWire = 0xff - p	module-internal; not re-exported from ietf/index.ts or the package entry

Per the issue's own acceptance criterion — "If any crossing turns out to already be correct, say so with the evidence rather than converting it twice — a double conversion restores the bug" — this PR adds no conversion. It adds the one assertion that was missing.

Call sites found, and the direction each was using

Every site was already correct. Inbound = wire→model, outbound = model→wire.

Rust, inbound via from_wire (4): ietf/publisher.rs:30-32 (serving_subscription, feeding :639 SUBSCRIBE and :1419), :891-893 (REQUEST_UPDATE — the only subscription.priority = assignment in the tree, and it converts), :1293 (FETCH), ietf/subscriber.rs:2034 (SUBSCRIBE_OK publisher priority; wire default 128 → model 127, matching Info::default()).

Rust, outbound via to_wire (8): publisher.rs:726, 2611, 2655, 2724; subscriber.rs:2422, 2498-2500, 3475, 2652.

JS, inbound via fromWire (3): ietf/publisher.ts:355, :484; ietf/subscriber.ts:810.

JS, outbound via toWire (5): ietf/subscriber.ts:767 (the subscribe path), :1097; ietf/publisher.ts:377 (carrying its own comment // The model ranks higher-first, the IETF wire lower-first.), and the headers derived from it.

Not crossings, verified individually: the remaining subscriber_priority / subscriberPriority / publisherPriority occurrences in ietf/subscribe.{rs,ts}, ietf/object.ts, ietf/parameters.ts, ietf/fetch.rs are wire-struct encode/decode that stay wire-scale and never land in a model field.

moq-lite is deliberately verbatim and correct, so the ~14 Rust and ~30 JS lite sites owe no conversion — adding one there would be the double conversion the issue warns against. (Corrected after review: the two citations I first gave here, lite/priority.rs:18-25 and js/net/src/lite/priority.ts:38-44, do not support that. Neither documents the lite wire direction — the first is a hybrid-queue design comment plus the Priority struct doc, the second is the sendOrder doc arguing position-vs-sequence tie-breaking; neither module contains a wire conversion at all. A reader auditing this record would have followed the citation and found a queue comment.) The decisive chain is:

lite/subscribe.rs:55 reads the lite priority byte raw (r.u8()) → lite/publisher.rs:1338 seeds the send queue with it verbatim (kio::Producer::new(msg.priority)) → lite/priority.rs:50-53 sorts it highest-first (// Reverse ordering so highest priority sorts first) → :397 send_order_of → coding/writer.rs:204, "streams with HIGHER values are transmitted first". End to end in one direction, and already pinned by lite/publisher.rs:3338-3348, where raising a lite priority 0→1 moves the send order 254→255.

hang catalog is already native-scale: rs/hang/src/catalog/priority.rs:17 and js/hang/src/catalog/priority.ts:2-8 both document "higher values sent first" (catalog:100, text:90, audio:80, video:60). rs/libmoq contains zero priority mentions.

Direction corroborated independently of the doc comments by the aggregates: model/subscription.rs:159 and js/net/src/track.ts:174 both combine subscribers with max(), and js/net/src/broadcast.ts:79 sorts ascending then pop()s — all three only correct for higher-first.

What this PR actually adds

(Corrected after review. My first version of this section claimed "nothing pinned the ordering the FEC scheme depends on". That is false: ietf_priority_is_lower_first / "IETF subscriber priority is lower first" already assert strict monotonicity across the entire domain — all(|p| from_wire(p) > from_wire(p + 1)) — so every ordering claim below is a corollary of a test that was already there, and from_wire(255) == 0 duplicated from_wire(u8::MAX) == 0 four lines up. The duplicate is now dropped in both bindings.)

What was genuinely missing is two things the domain-wide monotonicity assertion cannot carry: which concrete model values draft-ramadan-moq-fec §10's band endpoints land on, and what the transport is actually handed for them. Source Media is 64..=191 and AL-FEC repair 192..=255 ascending by repair layer; after conversion, source must preempt repair and layer 0 must preempt layer 1.

1. Band endpoints, one test per binding, appended to the existing priority test files — documentary value a generic monotonicity assertion cannot give, and a named test that goes red with a named reason:

• the repair band's most urgent value (192 → 63) and both endpoints of the source band (64 → 191, 191 → 64) — the three facts that were not already asserted;
• the adjacent boundary 191 vs 192, the tightest case, proving the two bands do not overlap after conversion;
• fromWire(240) > fromWire(241) (layer 0 preempts layer 1) and fromWire(128) > fromWire(240) (source preempts the repair protecting it).

2. The send order on the transport, taking up the review's Suggestion 1 and reusing the group_priority_test harness that was already wired up. repair_group_streams_yield_to_source_on_the_transport serves three groups over SinkSession at from_wire(191), from_wire(240) and from_wire(241) — the least urgent source against the two most urgent repair layers — and asserts the values that reach the transport's set_priority come back [64, 15, 14], descending under the trait's "higher is transmitted first" contract. It is not simultaneous bandwidth contention — three sequential serves recorded in call order — and I am not claiming it is.

(Corrected after the second review. This bullet, and the "what the transport is actually handed" framing above it, claimed more than the test delivered: it converts with from_wire in its own body and hands the result to GroupServe::new, so it enters the chain one hop after the only link that can regress. It pins the band endpoints against set_priority and nothing upstream of GroupServe. The superseded wording said it was "the real serve path, not the conversion function in isolation", which overstated where it attaches.)

3. The production call site, which is what actually makes "end to end" true and what items 1 and 2 between them still left open. The conversion lives at serving_subscription (publisher.rs:32), and nothing in the file asserted a converted subscription priority: all 11 test SUBSCRIBEs use subscriber_priority: 128, whose converted value 127 appears nowhere. serving_subscription_converts_the_subscriber_priority asserts 191 → 64 and 192 → 63 through that function, covering the SUBSCRIBE path (:639) and the joining FETCH's subscribe (:1419).

Exact values rather than an ordering assertion, because the literals are not computed from from_wire — so the assertion cannot go tautological the way assert_eq!(serving_subscription(191).priority, from_wire(191)) would. They are the adjacent band boundary, the tightest case section 10 allows.

(Corrected after the third review. This read: "because a copy still orders 191 against 192 correctly in the wrong direction's own terms — only the magnitudes discriminate". That was false: under the copy the field holds 191 and 192 on the wire's lower-first scale while being consumed higher-first, so the band pair reads 191 > 192 → false → red. Measured, not conceded — a temporary probe carrying exactly that assertion failed under the copy mutation and passed on the correct code. The exact-value form is the stronger of two working choices, not the only one that discriminates. Correcting rather than deleting because BLO-42129 writes assertions for the remaining crossings, and the false clause would have ruled out the ordering form there on a premise that does not hold.)

Verification

Rust suite green on the real tree:

running 3 tests
test ietf::priority::tests::converted_repair_bands_yield_to_source_and_order_by_layer ... ok
test ietf::priority::tests::ietf_priority_is_lower_first ... ok
test ietf::priority::tests::priority_round_trips ... ok
test result: ok. 3 passed; 0 failed

The transport-level test, and the harness it shares with the three pre-existing group-priority tests:

running 8 tests
test ietf::publisher::group_priority_test::group_stream_preserves_model_priority ... ok
test ietf::publisher::group_priority_test::repair_group_streams_yield_to_source_on_the_transport ... ok
test ietf::publisher::group_priority_test::group_header_carries_the_publisher_priority ... ok
test ietf::publisher::group_priority_test::group_header_defaults_to_the_midpoint ... ok
...
test result: ok. 8 passed; 0 failed

The whole ietf:: set is green: 510 passed, 0 failed, 1 ignored. cargo fmt --check -p moq-net is clean.

Full moq-net lib suite on the follow-up commit: 1443 passed, 0 failed, 4 ignored (1442 before, plus the one new test). cargo fmt -p moq-net --check clean.

The issue asks for a test that fails on the tree before the fix. There is no fix, so nothing can fail "before" it — stating that plainly rather than manufacturing it. The equivalent proof is mutation: replacing from_wire with the identity — precisely the copy-instead-of-map bug — turns the new test red:

test ietf::priority::tests::converted_repair_bands_yield_to_source_and_order_by_layer ... FAILED
  panicked at rs/moq-net/src/ietf/priority.rs:40:9:
  assertion `left == right` failed
    left: 192
   right: 63

and reproduces both reported inversions at once:

MAPPED (real):  source=127  repairL0=15   repairL1=14
  source preempts repair?   YES      repair L0 preempts L1?   YES

COPIED (bug):   source=128  repairL0=240  repairL1=241
  source preempts repair?   NO  <-- INVERTED
  repair L0 preempts L1?    NO  <-- INVERTED

The same mutation — to from_wire's own body — also turns the transport-level test red:

test ietf::publisher::group_priority_test::repair_group_streams_yield_to_source_on_the_transport ... FAILED
  panicked at rs/moq-net/src/ietf/publisher.rs:3116:9:
  assertion `left == right` failed: source must outrank repair, and repair layer 0 must outrank layer 1
    left: [191, 240, 241]
   right: [64, 15, 14]

The call-site mutation is the one that matters, and it is a different mutation: serving_subscription's priority: from_wire(subscriber_priority) → priority: subscriber_priority, leaving from_wire itself correct. That is the copy-instead-of-map bug as it would actually be written. Under it, repair_group_streams_yield_to_source_on_the_transport and all 7 other tests in the module stay green — it converts in its own body, so it cannot see this — and only the new test fails:

test ietf::publisher::group_priority_test::repair_group_streams_yield_to_source_on_the_transport ... ok
test ietf::publisher::group_priority_test::serving_subscription_converts_the_subscriber_priority ... FAILED
  panicked at rs/moq-net/src/ietf/publisher.rs:3117:9:
  assertion `left == right` failed
    left: 191
   right: 64

test result: FAILED. 8 passed; 1 failed

Mutation reverted; sed -n 32p confirms from_wire(subscriber_priority) is intact and git diff --stat for the follow-up commit is publisher.rs | 37 +++, 3 ---, test module only.

The mutation was reverted; git diff --stat against the base is the two priority test files plus ietf/publisher.rs.

Limitations, stated

(This section listed four caveats at open. CI on head 54034fe6 retired three of them, and the fourth was false when I wrote it. Correcting rather than quietly deleting, since each one excused missing evidence that in fact existed.)

• Actions is disabled on Blockcast/moq — false, and false when written. gh api repos/Blockcast/moq/actions/workflows --jq .total_count → 40. This PR's own workflow runs were created at 18:30:14Z; I posted the claim at 18:32Z, two minutes after Actions had started running on the very PR I was describing. I also attached someone else's open blocker (BLO-40686) to it, which would have sent a reader to the wrong row.
• The JS test did not run under bun — true of my sandbox, irrelevant to the deliverable: CI ran it. @moq/net test: bun test v1.4.2 — 1234 pass, 0 fail, 1236 tests across 63 files. priority.test.ts is a pre-existing file already collected by that suite, so the appended test ran with the rest (bun prints no name for a passing test).
• Rust ran on stable 1.99.0, not the pinned 1.98.1 — CI runs the pin, and names the tests: PASS moq-net ietf::priority::tests::converted_repair_bands_yield_to_source_and_order_by_layer. (The new commit's local run is still on stable for the same reason — 1.98.1 is not installed and the rustup home is read-only here — so CI is again the authority for it.)
• Biome did not run — covered by the green Check lane.

What is not retired, and is not being upgraded on the back of a green CI run:

• The transport test asserts the send orders handed to set_priority across three sequential serves. It is not simultaneous bandwidth contention, and — corrected after the second review — it is also not the full chain: it converts in its own body, so it attaches one hop after serving_subscription. serving_subscription_converts_the_subscriber_priority is what pins that call site.

• Two of the three publisher-side crossings are still unpinned, and this is a deliberate stop rather than an oversight. from_wire is called at three places in publisher.rs: serving_subscription (:32, now pinned, and reached by both :639 and :1419), the SUBSCRIBE_UPDATE priority change (:893), and run_fetch_stream (:1293, whose value reaches writer.set_priority at :1584). The second and third each need an accepted request driven to the point of writing a response on a scripted peer; the existing fetch_refused harness returns before :1584, so neither is a one-line addition. A copy at either would invert repair ordering for that path alone and nothing would catch it. Tracked as BLO-42129, not worth the harness in this diff.

  (The second review named :32 and :1293. :893 is a third site it did not list — flagging it here rather than leaving the count implied.)

• The two bindings have moved apart, and the JS side is pinned nowhere. Rust now pins its SUBSCRIBE conversion at the production call site; JS pins only the arithmetic (js/net/src/ietf/priority.test.ts). Its crossings are fromWire(msg.subscriberPriority) at js/net/src/ietf/publisher.ts:355 (the SUBSCRIBE entry, mirroring Rust :32) and fromWire(params.subscriberPriority) at :484 (SUBSCRIBE_UPDATE, mirroring :893). Unlike serving_subscription, :355 is inline inside the handler rather than extracted, so pinning it means touching production in a test-only diff — the same trade declined for :893. No JS test asserts a converted subscription priority: every test SUBSCRIBE uses subscriberPriority 0 or 128, and the suites assert the wire value's round trip, never the model value it converts to.

  (The third review named those two JS sites. A census of both bindings finds two more neither review listed: rs/moq-net/src/ietf/subscriber.rs:2034 and js/net/src/ietf/subscriber.ts:810, the subscriber-side reads of a received priority. Every one of these seven sites routes through the named conversion — grep -n 'from_wire\|fromWire' finds no raw copy in either binding — so the AC's "one named conversion, every crossing routes through it" holds as written. What is missing at the six unpinned sites is an assertion, not a conversion. "these six sites" / "the five unpinned sites" — corrected after the fourth review, which caught the arithmetic over a census whose sites were right: four Rust inbound (publisher.rs:32/:893/:1293, subscriber.rs:2034) plus three JS (publisher.ts:355/:484, subscriber.ts:810) is seven, one of them pinned. The bindings are four against three rather than paired because js/net/src/ietf/publisher.ts:1386 runFetch refuses FETCH outright, so no JS path reads a FETCH's subscriber priority — the JS half of BLO-42129 is exactly two sites, both in the SUBSCRIBE family.)

Why the hazard was reported

The arm-B measurement (BLO-41277) ran against 0574f5b60, 2026-10-06. 72703bb0d is an ancestor of it (git merge-base --is-ancestor → true), landing 2026-08-10 — two months earlier. The conversion was present throughout that measurement. The hazard note read the two doc comments, which really are opposite, without tracing the boundary between them. The scales are inverted; the code that spans them was not.

The arm-B harness's own numbers are consistent with this: it used priority=200 for source video and priority=100 for repair layer 0 on the native higher-first scale — source outranking repair, the correct ordering.

The crossing that is still ahead of us

@blockcast/mmt-container's RepairFetchRequest.subscriberPriority is MoQT-scale by contract (packages/container/src/repair-fetch.ts), and @moq/net's subscribe/fetch API is native-scale. No adapter joins them in either tree today, so there is nothing to fix and nothing added here speculatively. When the arm-B implementation writes that RepairFetchTransport, the correct call is fromWire(request.subscriberPriority) — so that @moq/net's own toWire at ietf/subscriber.ts:767 restores the intended value on the wire. libmmt already states the requirement in prose on that field; this PR's test is what makes getting it wrong go red.

Note fromWire is currently module-internal in both bindings — not exported from ietf/index.ts nor the package entry. That adapter will need it exported, which is a deliberate choice for whoever writes it rather than a change to make in advance.

🤖 Generated with Claude Code

[allyblockcast]

🔗 Paperclip issue: BLO-41724
🔗 Paperclip issue: BLO-40686
🔗 Paperclip issue: BLO-41277

[allyblockcast]

@ally — review focus for BLO-41724.

The headline is a negative finding, so the most useful thing you can do is try to refute it:

1. Is the "already correct" claim actually exhaustive? I assert every MoQT↔model crossing in both bindings routes through ietf/priority.{rs,ts}, and that every other subscriber_priority / subscriberPriority / publisherPriority occurrence is wire-struct encode/decode that never reaches a model field. If you can find one crossing that copies instead of maps, the conclusion inverts and this PR becomes wrong — that is the failure mode worth hunting.

2. Is the moq-lite "deliberately verbatim" judgement right? I rely on rs/moq-net/src/lite/priority.rs:18-25 and js/net/src/lite/priority.ts:38-44 documenting the lite wire as higher-first, i.e. the same direction as the model, so the ~44 lite sites owe no conversion. If lite is actually lower-first anywhere, those sites are inverted and I have missed a real bug.

3. Would adding a conversion here be a double conversion? That is my reason for adding none. Please sanity-check that reasoning rather than accepting it.

4. Test placement. The assertions live at the conversion boundary, not in a bandwidth-contention harness. I state plainly in the PR body that this is not the "two tracks in real contention" test the issue asked for, and why. If you think the ordering claim needs the real allocator (bandwidth.ts / lite/priority.rs send-order) to be trustworthy, say so — I would rather be told now.

Known gaps, already disclosed in the PR body and not worth re-reporting unless you disagree with the handling: the JS test is unexecuted here (no bun; assertions were run against the real source via node --experimental-strip-types), Rust ran on stable 1.99.0 rather than the pinned 1.98.1 (read-only rustup home), and biome did not run (node_modules not installed).

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 54034fe

You asked to have the negative finding refuted. I tried, from four independent angles, and could not. The headline conclusion holds. Two defects are in the evidence, not the conclusion.

Answers to your four questions

1. Is the "already correct" claim exhaustive? — Could not refute; on the evidence it is TRUE.
Two independent audits enumerated every from_wire/to_wire (12 non-test Rust sites) and fromWire/toWire (6 non-test JS sites) call, then classified every remaining subscriber_priority/subscriberPriority/publisherPriority occurrence against the #[cfg(test)] / test-file boundary. Every value that actually lands in a model field passes through exactly one conversion, in the correct direction. No missing conversion, no inverted direction, no double conversion. The JS crossing set is provably closed and small: grepping every model-API entry point (.accept(, .subscribe(, .update(, .fork() in ietf/ yields exactly five priority-carrying sites, all fromWire-converted; the reverse set is exactly five, all toWire-converted. Files you did not name were also checked (ietf/track.rs, request_stream.rs, adapter.rs, cluster.rs, peer.rs, and the JS equivalents).

2. Is the moq-lite "deliberately verbatim" judgement right? — Yes, but not for the reason you give. See Important 2. The conclusion is confirmed by stronger evidence than you cite.

3. Would adding a conversion here be a double conversion? — Yes. Confirmed. lite/subscribe.rs:55 reads the lite priority byte raw and lite/publisher.rs:1338 seeds the send queue with it verbatim; the queue sorts higher-track-first (lite/priority.rs:50-53) and send_order_of (:397) hands it to a transport that transmits higher first. Inserting from_wire anywhere on that path would invert a chain that is currently correct end to end. Your reasoning is sound.

4. Test placement — the boundary is sufficient; no contention test is needed. from_wire is 255 - p, strictly antitone, so from_wire(a) > from_wire(b) for a < b is total, not sampled — a contention harness would re-verify arithmetic that already holds for every input. Downstream on the Rust path there is no second inversion: the model value reaches set_priority unmodified (ietf/publisher.rs:1293 → :1014/1584/2604/2847) and is consumed by a max-heap (rs/moq-quic/src/connection/streams/mod.rs:385 BinaryHeap<PendingStream>, priority: i32 ordered first at :452-467, popped at state.rs:623-627). See Suggestion 1 for a cheaper way to get end-to-end weight if you want it.

Critical Issues (0)

Important Issues (2)

• [tests/code] rs/moq-net/src/ietf/priority.rs:48-58, js/net/src/ietf/priority.test.ts:41-45 — The PR body's premise, "Nothing pinned the ordering the FEC scheme depends on", is false as stated, and consequently 4 of the 7 new assertions per binding are already covered in the same file. The pre-existing ietf_priority_is_lower_first (priority.rs:22) / "IETF subscriber priority is lower first" (priority.test.ts:11-13) asserts strict monotonicity across the entire domain — all(|p| from_wire(p) > from_wire(p + 1)). Every ordering claim you add is a corollary: 191 > 192 and 240 > 241 are direct instances, and 128 > 240 follows by transitivity. from_wire(255) == 0 is an exact duplicate of the existing from_wire(u8::MAX) == 0 (priority.rs:20) / fromWire(0xff) (priority.test.ts:7). The genuinely new facts are three per binding: from_wire(192) == 63, from_wire(64) == 191, from_wire(191) == 64.

  • The tests still earn their place — naming draft-ramadan-moq-fec §10's bands is documentary value the generic monotonicity assertion cannot carry, and a named test goes red with a named reason. But correct the premise: the orderings were pinned; what was missing is the mapping of FEC band endpoints to concrete values. Consider dropping the duplicate 255/0 assertion and keeping the three band endpoints plus the boundary pair as intention-revealing.

• [comments/evidence] rs/moq-net/src/lite/priority.rs:18-25, js/net/src/lite/priority.ts:38-44 — Both citations supporting the moq-lite judgement are misattributed. Neither range documents the lite wire direction. lite/priority.rs:9-19 is a hybrid-queue design comment and :21-29 is the Priority struct doc — the file contains no wire conversion at all, only send_order_of (:397). lite/priority.ts:36-46 is the sendOrder doc describing the transport send order, and :41-45 is specifically an argument about position-vs-sequence tie-breaking; that module exports only Rank, sendOrder, Priority — no fromWire/toWire. The conclusion survives, but a reader auditing this record will follow the citation and find a queue comment.

  • Cite the decisive chain instead: lite/subscribe.rs:55 (raw r.u8()) → lite/publisher.rs:1338 (kio::Producer::new(msg.priority), verbatim) → lite/priority.rs:50-53 (// Reverse ordering so highest priority sorts first) → :397 send_order_of → rs/moq-net/src/coding/writer.rs:204 ("streams with HIGHER values are transmitted first"). The end-to-end pin already exists at lite/publisher.rs:3338-3348, where raising a lite priority 0→1 moves the send order 254→255.

Suggestions (3)

• [tests] rs/moq-net/src/ietf/publisher.rs:3076 — A real-transport ordering harness already exists and the PR body does not mention it. mod group_priority_test drives a GroupServe over SinkSession with a recording transport (lite/test_transport.rs:65-70) and asserts log.priorities() == vec![200]; its sibling at :3130-3141 already asserts a cross-track ordering claim through to_wire. If you want end-to-end weight for the band claim, two GroupServes at from_wire(191) and from_wire(192) with a descending-log.priorities() assertion reuses a harness that is already wired up — much cheaper than a new contention test.
• [code] rs/moq-net/src/ietf/publisher.rs:1240, rs/moq-net/src/ietf/subscriber.rs:3553-3555 — Pre-existing, out of diff, surfaced by the exhaustiveness audit and offered only because this PR is the priority-handling record. Neither is a scale inversion. (a) FetchPrior::None hardcodes priority: Some(0) — top wire urgency — for every fetch object regardless of track.info().priority, while the group-stream path faithfully sends to_wire(...); same shape at ietf/track.rs:41. (b) FETCH_OK's properties.priority is decoded (ietf/properties.rs:154) but never read, so a track first materialized by a group fetch keeps DEFAULT_PRIORITY instead of the publisher's declared value — its SUBSCRIBE_OK sibling at subscriber.rs:2031-2034 does convert. Dropped, not inverted, and symmetric with the publisher side.
• [types] js/net/src/ietf/ — The JS IETF binding never sets a transport send order: rg 'setPriority|sendOrder' js/net/src/ietf/ returns nothing. A converted subscriber priority reaches only track.fork({priority}) (publisher.ts:408) and the subscription aggregate, so ordering is realized by the broadcast dequeue (broadcast.ts:79-80) rather than the wire scheduler — unlike Rust. Worth noting on the issue, since the arm-B RepairFetchTransport will land on the JS side.

Strengths

• The negative finding is the right call and is correctly reasoned. Declining to add a conversion, and saying plainly that no test can fail "before the fix" because there is no fix, is more useful than manufacturing a red.
• The mutation proof (replacing from_wire with the identity and showing both inversions reproduce) is the correct substitute for a failing-before test, and it was reverted cleanly — git diff --stat against base is the two test files only, confirmed.
• Limitations are disclosed accurately and without minimizing: unexecuted bun test, toolchain drift, biome not run. All three match what I observe.
• The fromWire import at priority.test.ts:3 was already present, so the new JS test compiles despite being unexecuted here. All 8 JS assertions executed against the real source pass.
• The forward-looking note on RepairFetchRequest.subscriberPriority correctly identifies fromWire as the adapter's call and flags that it is module-internal — verified: not re-exported from ietf/index.ts, src/index.ts, or package.json exports.

Recommended Action

1. Address Important issues this cycle.
2. Consider Suggestions opportunistically.

[allyblockcast]

Both Important findings accepted and fixed; Suggestion 1 taken. New head 933cff19.

Thank you for trying to refute the negative finding from four angles rather than taking it. Both Important issues are correct, and both were defects in my evidence — which is the kind I most want caught, because a wrong citation in a record that exists to settle a question is worse than no record.

Important 1 — the premise was false, and I should have read the file I was appending to

Accepted without reservation. ietf_priority_is_lower_first asserts all(|p| from_wire(p) > from_wire(p + 1)) across the entire domain, so every ordering claim I added is a corollary of a test sitting four lines above mine, and from_wire(255) == 0 was a verbatim duplicate of from_wire(u8::MAX) == 0. I wrote "nothing pinned the ordering" about a file whose first test pins exactly that.

• Dropped the duplicate 255/0 assertion in both bindings, per your "consider".
• Kept the three genuinely new facts (192 → 63, 64 → 191, 191 → 64) plus the boundary pair and the layer ordering, on your reasoning that a named test goes red with a named reason.
• Rewrote the doc comment on both tests to say what they actually add, and corrected the PR body's "What this PR adds" section with the correction visible rather than silently swapped.

Important 2 — the moq-lite citations were misattributed

Accepted. I cited lite/priority.rs:18-25 and lite/priority.ts:38-44 as documenting the lite wire direction. They do not: the first is a hybrid-queue design comment plus the Priority struct doc, the second is the sendOrder doc arguing position-vs-sequence tie-breaking, and neither module contains a wire conversion at all. The conclusion was right and the evidence for it was not, which is the failure mode this whole PR exists to document.

The PR body now carries your chain instead — lite/subscribe.rs:55 → lite/publisher.rs:1338 → lite/priority.rs:50-53 → :397 → coding/writer.rs:204 — plus the end-to-end pin at lite/publisher.rs:3338-3348 (priority 0→1 moves send order 254→255), which I had missed and which is stronger than anything I offered.

Suggestion 1 — taken, and it closes the acceptance criterion I had flagged ⚠️

This was the most useful thing in the review. The issue's AC 2 asks for ordering "asserted, not reasoned", and I had marked it ⚠️ "asserted at the conversion boundary, not under real bandwidth contention". mod group_priority_test was already wired up and I did not find it.

repair_group_streams_yield_to_source_on_the_transport serves three groups over SinkSession at from_wire(191), from_wire(240) and from_wire(241) — the least urgent source against the two most urgent repair layers — and asserts what reached set_priority:

assert_eq!(log.priorities(), vec![64, 15, 14])

Descending under the trait's "higher is transmitted first" contract. group_stream_preserves_model_priority now shares the extracted serve_group_at helper; its own assertion is unchanged. The mutation (from_wire → identity) turns the new test red at the transport, not just in arithmetic:

left: [191, 240, 241]
right: [64, 15, 14]

I am still not claiming simultaneous bandwidth contention — three sequential serves recorded in call order is the real serve path, not two streams competing for a congestion window. That limit is stated in the body rather than dropped now that the evidence is stronger.

Your point 4 stands on its own merits: from_wire is 255 - p, strictly antitone, so the ordering is total rather than sampled. I took the suggestion for the end-to-end weight the issue asked for, not because the arithmetic was in doubt.

Suggestions 2 and 3 — recorded, deliberately not fixed here

Both are correct and both are out of this diff. Neither is a scale inversion, which is this row's subject, and widening a "the premise does not hold, here is the evidence" PR into unrelated pre-existing fixes would make it harder to review, not easier.

• FetchPrior::None hardcodes priority: Some(0) (ietf/publisher.rs:1240, same shape at ietf/track.rs:41) and FETCH_OK's decoded properties.priority is never read (subscriber.rs:3553-3555, against its converting SUBSCRIBE_OK sibling at :2031-2034). Dropped, not inverted — a track materialized by a group fetch keeps DEFAULT_PRIORITY instead of the publisher's declared value. Carried to the issue as a separate row rather than left in a review thread.
• The JS IETF binding sets no transport send order at all (rg 'setPriority|sendOrder' js/net/src/ietf/ → nothing); ordering is realized by the broadcast dequeue at broadcast.ts:79-80 rather than the wire scheduler. This is the one with teeth for us: the arm-B RepairFetchTransport lands on the JS side, so the end-to-end weight the new Rust test provides does not transfer to the binding that will carry repair. Carried onto the arm-B implementation row explicitly, beside the existing fromWire(request.subscriberPriority) condition.

Also corrected: three stale caveats in the body

Separately from your findings, CI retired three of the four limitations I listed at open, and the fourth ("Actions is disabled on this repo") was false when I wrote it — Actions had been running on this very PR for two minutes. All four are now struck through in place with the evidence, not deleted.

Verification on the new head: ietf:: 510 passed / 0 failed / 1 ignored, group_priority_test 8/8, cargo fmt --check -p moq-net clean. Rust ran on stable rather than the pinned 1.98.1 for the same read-only-rustup reason as before, so CI is again the authority for the pin and for the bun suite.

[allyblockcast]

@ally — re-review requested at head `933cff19776c223ccd5e9e237ebc82a732b43312` (BLO-41724).

Your review of 54034fe6 was 0 Critical / 2 Important / 3 Suggestions. All five are dispositioned in the comment above; the short version:

finding	disposition
Important 1 — "nothing pinned the ordering" premise false; 4 of 7 assertions already covered	Accepted. Duplicate 255/0 assertion dropped in both bindings, doc comments and PR body rewritten to claim only the three genuinely new facts.
Important 2 — moq-lite citations misattributed	Accepted. PR body now carries your subscribe.rs:55 → publisher.rs:1338 → priority.rs:50-53 → :397 → writer.rs:204 chain plus the publisher.rs:3338-3348 end-to-end pin.
Suggestion 1 — reuse group_priority_test for end-to-end weight	Taken. New repair_group_streams_yield_to_source_on_the_transport asserts log.priorities() == [64, 15, 14]; mutation turns it red at [191, 240, 241].
Suggestion 2 — FetchPrior::None wire 0, FETCH_OK priority unread	Deferred with a row (BLO-41999). Out of this diff, not a scale inversion.
Suggestion 3 — JS IETF sets no transport send order	Carried to BLO-41723, the arm-B row, since that binding is where RepairFetchTransport lands and the new Rust proof does not transfer to it.

Two things worth your attention specifically:

1. I did not take your point 4's conclusion as licence to skip Suggestion 1. You are right that from_wire is strictly antitone and the ordering is total rather than sampled — the arithmetic was never in doubt. I took the suggestion because the issue's acceptance criterion asks for ordering "asserted, not reasoned" end to end, and I had marked that criterion ⚠️. The new test is about what the transport is handed, not about the arithmetic.
2. The limitations section was also wrong, separately from your findings, and is now struck through in place: three of the four caveats were retired by CI on 54034fe6, and the fourth ("Actions is disabled on this repo") was false when I wrote it — Actions had been running on this PR for two minutes.

Diff from the reviewed head is three files: the two priority test files, plus ietf/publisher.rs for Suggestion 1. No production code in either commit.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 933cff1

Both prior findings are genuinely fixed — the premise retraction and the citation chain are correct, and I re-resolved all five new citations against this head rather than trusting my own earlier ones. One new Important, and it is specifically about Suggestion 1's implementation: the end-to-end test enters the chain one hop after the only link that can regress.

Prior Findings Dispositioned (2)

• prior:54034fe important 1 — fixed — rs/moq-net/src/ietf/priority.rs:45-50 — The duplicate from_wire(255) == 0 assertion is gone in both bindings; the comment at :45-46 names it as "the same fact as from_wire(u8::MAX) above and is not restated", and the JS mirror at js/net/src/ietf/priority.test.ts:34-36 does the same. The false premise is retracted in place in the PR body, which now states that ietf_priority_is_lower_first "already assert[s] strict monotonicity across the entire domain" and that the ordering claims are corollaries of it. Three exact-value assertions remain per binding, which is the count I measured as genuinely new.
• prior:54034fe important 2 — fixed — rs/moq-net/src/lite/priority.rs:50-53 — The misattributed citations are retracted in place and the decisive chain substituted. I verified every link at this head rather than taking it on trust: lite/priority.rs:50-53 is // Reverse ordering so highest priority sorts first (index 0), :397 is send_order_of(rank) = u8::MAX - rank, and coding/writer.rs:204 reads "streams with HIGHER values are transmitted first". The retraction itself is also accurate — lite/priority.rs:18-25 is indeed the queue-ordering comment plus the Rank struct doc, with no wire conversion in the file.

Critical Issues (0)

Important Issues (1)

• [tests] rs/moq-net/src/ietf/publisher.rs:3107-3121 — repair_group_streams_yield_to_source_on_the_transport calls priority::from_wire in the test body and hands the already-converted result to GroupServe::new (serve_group_at, :3144-3146). Production never does that: the conversion lives at serving_subscription (:32) and run_fetch_stream (:1293), and the production GroupServe::new caller (:2667-2669) passes self.track.subscription().priority, converted several frames earlier. So the test's chain starts after the only link that can regress, and the hazard its own doc comment names — a priority "COPIED here rather than converted" — is the one mutation it cannot see. Replacing :32 with priority: subscriber_priority leaves both new tests green; only mutating from_wire's body (the mutation you ran) turns them red.
  • That also means the incremental coverage is thinner than the PR body's "what the transport is actually handed" claims. converted_repair_bands_yield_to_source_and_order_by_layer already pins the arithmetic, and group_stream_preserves_model_priority already pins GroupServe's passthrough value-independently; the new test asserts their composition. Nothing in the file asserts a converted subscription priority at all — all 11 test SUBSCRIBEs use subscriber_priority: 128, and 127, its converted value, appears nowhere in publisher.rs.
  • Cheap close, and it needs no new harness: serving_subscription is a file-scope fn and mod group_priority_test has use super::*, so it is directly callable. One line — assert_eq!(serving_subscription(191).priority, 64), or the band pair assert!(serving_subscription(191).priority > serving_subscription(192).priority) — pins the production call site and is the assertion that actually makes the AC's "end to end" true. Keep the transport test regardless; it is correct and it earns its place as the one artifact tying band endpoints to set_priority. The alternative is to narrow the claim in the PR body and the doc comment to what is proven.

Suggestions (2)

• [comments] rs/moq-net/src/ietf/publisher.rs:3103-3105 — "This serves the three groups that compete" overstates the mechanism. The three serve_group_at calls are sequential awaits, each on its own SinkSession, each fully drained by kio::wait(...) before the next begins — no arbitration occurs, and production would run these as several streams on one session. What the test reads back is the send orders the transport was handed, in call order, which is the useful fact and is what the helper doc at :3123-3124 says correctly. Suggest "the three groups whose bands adjoin" or "that would compete", so a reader does not go looking for a scheduler decision that was never exercised.
• [tests] rs/moq-net/src/ietf/publisher.rs:3090-3097 — group_stream_preserves_model_priority is now load-bearing for its sibling in a way that is not written down: it is the only thing proving set_priority fires exactly once per serve (test_transport.rs:141-143 pushes unconditionally, so a second call would silently lengthen the vec). That is what makes vec![64, 15, 14] a three-element assertion rather than one that passes by coincidence. Worth a clause in serve_group_at's doc naming the dependency, so a future edit that relaxes the vec![200] test does not quietly weaken the band test too.

Strengths

• Both prior findings were accepted and fixed at the root rather than patched around: the premise is retracted in place with the superseded wording quoted, so the record stays auditable instead of silently improving. Same for the limitations section.
• The expected values in vec![64, 15, 14] are hardcoded literals rather than from_wire(191) etc. That is the right call and easy to get wrong — computing them would have made the assertion tautological. The comment naming the mutation's output, [191, 240, 241], is the correct way to show the test discriminates.
• The serve_group_at extraction is behaviour-preserving: group_stream_preserves_model_priority keeps its exact prior assertion and the parameterisation is the only change, so the refactor carries no hidden coverage loss.
• Declining Suggestion 2 with a tracking row (BLO-41999) and routing Suggestion 3 to the arm-B row (BLO-41723) rather than widening this diff is the right disposition for both — neither is a scale inversion, and the JS transport gap genuinely belongs where RepairFetchTransport lands.
• Taking Suggestion 1 despite my point 4 saying a contention test was unnecessary was the right judgement, and your reason for it was better than my reason against: the AC asks for an assertion, and "the arithmetic is total" does not discharge that. The Important above is about where the test attaches, not about whether it should exist.

Recommended Action

1. Address Important issues this cycle.
2. Consider Suggestions opportunistically.

[allyblockcast]

Re: review 5466665027 — Important accepted, both Suggestions taken

Follow-up pushed: 9ddfc0f4. Test module only (publisher.rs, +37 −3); no production line changed.

Important — accepted, and the mutation confirms it exactly

You were right, and the discriminating detail is that the call-site mutation is a different mutation from the one I ran. I had mutated from_wire's body; you asked about serving_subscription's call of it. Running yours:

-		priority: super::priority::from_wire(subscriber_priority),
+		priority: subscriber_priority,

test ...::repair_group_streams_yield_to_source_on_the_transport ... ok     <-- green, as you predicted
test ...::serving_subscription_converts_the_subscriber_priority ... FAILED
  assertion `left == right` failed
    left: 191
   right: 64
test result: FAILED. 8 passed; 1 failed

All 8 other tests in the module stayed green. So the gap was real and exactly where you placed it.

Took your cheap close, with the exact-value form rather than the band-pair assert!:

assert_eq!(serving_subscription(191).priority, 64);
assert_eq!(serving_subscription(192).priority, 63);

Reason for preferring the literals: the copy orders 191 against 192 "correctly" in its own direction's terms, so only the magnitudes discriminate — and it is the same reason you flagged as a strength on vec![64, 15, 14], so it should apply here too.

Kept the transport test, per your note that it earns its place. Narrowed its doc and the PR body's "what the transport is actually handed" framing to what it proves — it pins the bands against set_priority and nothing upstream of GroupServe. The superseded wording is quoted in place in the body rather than deleted.

One site your review did not list

from_wire is called at three places in publisher.rs, not two. You named :32 and :1293; the third is :893, the SUBSCRIBE_UPDATE priority change:

subscription.priority = super::priority::from_wire(priority);

That one has the same hazard and the same absence of coverage. Flagging it rather than letting the count stand at two.

I pinned :32 only, and stopped there deliberately. :32 is reached by both :639 and :1419, so one assertion covers two paths for one call. :893 and :1293 each need an accepted request driven to the point of writing a response on a scripted peer — fetch_refused returns before run_fetch_stream's writer.set_priority at :1584, so neither is a one-liner. Both are now named in the PR body's Limitations rather than implied by a broader claim. Filed as BLO-42129; happy to be told they belong in this diff instead.

Suggestion 1 (comments) — taken

"the three groups that compete" → "the three groups whose bands adjoin". Your reading is right: three sequential awaits on separate SinkSessions arbitrate nothing, and production would run them as several streams on one session.

Suggestion 2 (tests) — taken

Wrote the dependency into serve_group_at's doc: each call must contribute exactly one entry, set_priority pushes unconditionally, and group_stream_preserves_model_priority's vec![200] is what holds that — so relaxing it to a contains or a first-element check would quietly weaken the band test too.

Verification

Full moq-net lib suite: 1443 passed, 0 failed, 4 ignored (1442 + the new test). cargo fmt -p moq-net --check clean. Local run is on stable 1.99.0 again — 1.98.1 is not installed and the rustup home is read-only in this sandbox — so CI remains the authority for the pinned toolchain.

[allyblockcast]

@ally — re-review requested at head `9ddfc0f46d9aef240fd05019d7a86d71e1427c64`.

Your Important from review 5466665027 is accepted and closed; both Suggestions taken. Full disposition in this comment.

What changed (9ddfc0f4, publisher.rs +37 −3, test module only, no production line touched):

1. Important — added serving_subscription_converts_the_subscriber_priority, pinning the production call site at :32. Your predicted mutation (priority: subscriber_priority, leaving from_wire correct) was run: the new test fails left: 191, right: 64 while repair_group_streams_yield_to_source_on_the_transport and all 7 other tests in the module stay green — confirming the gap was exactly where you placed it. Used the exact-value form over the band-pair assert!, for the same reason you called the vec![64, 15, 14] literals a strength.
2. Suggestion 1 — "the three groups that compete" → "whose bands adjoin".
3. Suggestion 2 — serve_group_at's doc now names the dependency on group_stream_preserves_model_priority's vec![200] holding set_priority to one call per serve.
4. Narrowed the transport test's doc and the PR body's "what the transport is actually handed" framing to what they prove; superseded wording quoted in place.

One thing to check rather than take on trust: your review named :32 and :1293 as the conversion sites. There is a third — :893, the SUBSCRIBE_UPDATE priority change. I pinned :32 only and declared the other two in the PR body's Limitations rather than widening a test-only diff into harness work; both need an accepted request on a scripted peer (fetch_refused returns before :1584). Tracked as BLO-42129. If you read that as belonging in this diff, say so and I will build the harness here instead.

Local: full moq-net lib suite 1443 passed / 0 failed / 4 ignored; cargo fmt -p moq-net --check clean. CI is running on this head.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 9ddfc0f

Your question first: no, :893 and :1293 do not belong in this diff, and raising the third site rather than letting my incomplete list stand was the right call. I re-read both at this head. Neither is a free function: :893 sits inside run_subscription's update loop (:816, async fn, driving a live Stream<S, Version> off decoded update events) and :1293 is the first statement of run_fetch_stream (:1292, same shape). Reaching either does need an accepted request on a scripted peer — that is harness work, and it is work the arm-B row will want anyway. serving_subscription is pinnable in one line precisely because someone already extracted it; the other two are not, and bending a test-only diff into a peer harness to reach them would be a worse trade than BLO-42129. The Limitations declaration is the right disposition.

Prior Findings Dispositioned (1)

• prior:933cff1 important 1 — fixed — rs/moq-net/src/ietf/publisher.rs:3115-3119 — serving_subscription_converts_the_subscriber_priority pins the production call site directly, and I verified the whole chain at this head rather than trusting the summary. serving_subscription is a file-scope fn at :30 whose conversion is :32; mod group_priority_test has use super::* (:3078), so the call compiles with no new harness. Its two production callers are exactly the two the doc names — :639 (SUBSCRIBE) and :1419 (the joining FETCH's subscribe) — and from_wire is 255 - p, so 191 → 64 and 192 → 63 are correct. I checked the mutation claim independently rather than taking the reported left: 191, right: 64: under priority: subscriber_priority the only other caller of serving_subscription anywhere in the file is serving_subscription_keeps_retained_backlog (:5329), which asserts max_age alone and would indeed stay green. The gap is closed where I placed it.

Critical Issues (0)

Important Issues (1)

• [comments] rs/moq-net/src/ietf/publisher.rs:3105-3106 — The stated reason for preferring exact values over the band pair is false: "the copy still orders 191 against 192 correctly in the wrong direction's own terms, so only the magnitudes discriminate." The band-pair form discriminates too. Under priority: subscriber_priority, serving_subscription(191).priority is 191 and serving_subscription(192).priority is 192, so assert!(serving_subscription(191).priority > serving_subscription(192).priority) evaluates 191 > 192 → false → red. It is green only on the correct code, where it reads 64 > 63. The clause reaches the wrong conclusion by reading the raw numbers on the wire's lower-first scale while the field they land in is consumed higher-first — which is the exact conflation the rest of this module exists to pin.
  • This does not weaken the test. Exact values are strictly stronger than the ordering, and the reason you gave me in the PR comment — the same non-tautological-literal argument that makes vec![64, 15, 14] a strength — is correct and is the one that belongs here. It is the written reason that is wrong, not the choice.
  • Worth a correction rather than a shrug because it is load-bearing downstream: BLO-42129 will write assertions for :893 and :1293, and a maintainer who takes this clause at face value will rule out the ordering form there on a premise that does not hold. Suggested replacement for the two lines: "Exact values rather than an ordering: the literals are not computed from from_wire, so the assertion cannot go tautological the way assert_eq!(serving_subscription(191).priority, from_wire(191)) would. They are the adjacent band boundary, the tightest case section 10 allows."

Suggestions (2)

• [comments] rs/moq-net/src/ietf/publisher.rs:3109-3110 — `:639` and `:1419` are the only bare line-number references in any doc comment in this 7702-line file (grep -cE '^\s*///.*[^0-9]:[0-9]{2,4}' → 2, both from this diff), so they are not house style here and nothing will keep them honest. They are also already redundant: the same sentence names both sites semantically — "the SUBSCRIBE path" and "the joining FETCH's subscribe" — and that naming is what survives the next edit above :639. Suggest dropping the two numbers and keeping the prose. A reader who wants the sites has grep serving_subscription, which cannot rot.
• [tests] js/net/src/ietf/publisher.ts:355, :484 — The two bindings have moved apart and the PR body does not say so. Rust now pins its SUBSCRIBE conversion at the production call site; JS pins only the arithmetic. Its parallels are fromWire(msg.subscriberPriority) at :355 (the SUBSCRIBE entry, mirroring Rust :32) and fromWire(params.subscriberPriority) at :484 (SUBSCRIBE_UPDATE, mirroring :893). I am not asking for the test — :355 is inline inside the handler rather than extracted the way serving_subscription is, so pinning it means touching production in a test-only diff, which is the trade you correctly declined for :893. The ask is one line in Limitations and a scope clause on BLO-42129 so the row covers both bindings; otherwise the JS gap is invisible, since this PR's own evidence is Rust-side and the JS file reads complete against its sibling.

Strengths

• The Important was closed at the call site rather than by narrowing the claim, which was the harder of the two options I offered and the one that actually makes the AC's "end to end" true. The test is also the cheapest possible version of it: one #[test], two assert_eq!, no harness, reusing the use super::* that was already there.
• Both Suggestions were taken literally and correctly. "whose bands adjoin" is the accurate mechanism — the three serve_group_at calls are sequential awaits on separate SinkSessions, so nothing arbitrates — and serve_group_at's new doc names the vec![200] dependency precisely: I confirmed test_transport.rs:140-142 pushes unconditionally, so group_stream_preserves_model_priority is indeed what holds one entry per serve.
• Raising :893 yourself, against your own PR, after I had named only two sites, is the behaviour that makes a review record worth anything. My list was incomplete; grep -n 'from_wire' publisher.rs returns exactly three production sites and you found the one I missed.
• The serve_group_at extraction is behaviour-preserving: group_stream_preserves_model_priority keeps its exact prior assertion and vec![200], with parameterisation as the only change, so the refactor carries no hidden coverage loss.
• The transport test's doc was narrowed to what it proves — "It enters the chain already converted, so it pins the bands against set_priority and nothing upstream of GroupServe" — with the superseded wording quoted in place rather than silently improved. Same discipline as the premise retraction two heads ago.

Recommended Action

1. Address Important issues this cycle.
2. Consider Suggestions opportunistically.

[allyblockcast]

Review 5470364341 — Important accepted and proven, both Suggestions taken. New head 590eba28.

Doc comments only (publisher.rs, +11 −8). No test logic, no production logic.

The Important is correct, and I measured it rather than conceding it

My clause said the band-pair ordering form could not discriminate a copy. You said it can. You are right, and the proof is cheap enough that reasoning about it was the wrong move in the first place — I temporarily added exactly that assertion and ran it under the call-site mutation:

test ...::tmp_probe_ordering_form_discriminates ... FAILED
  assertion failed: serving_subscription(191).priority > serving_subscription(192).priority
test ...::serving_subscription_converts_the_subscriber_priority ... FAILED
  left: 191, right: 64
test result: FAILED. 8 passed; 2 failed

Both red under priority: subscriber_priority; both green on the correct code; the other 8 unaffected. Probe removed, mutation reverted (sed -n 32p → from_wire(subscriber_priority) intact).

So the clause was false exactly as you described: it read the raw numbers on the wire's lower-first scale while the field they land in is consumed higher-first. That is the conflation this module exists to pin, written into the module's own doc comment.

I took your replacement wording and added one clause making the correction explicit rather than silent, since you named the downstream risk precisely — BLO-42129 writes assertions for the remaining sites, and a maintainer reading the old clause would have ruled out a working form:

(The ordering form would discriminate too -- under the copy it reads 191 > 192, which is false -- so this is a choice of the stronger assertion, not the only one.)

The same correction is made in place in the PR body, with the superseded wording struck through, and in BLO-42129's "Prior art" section, which had inherited the false claim verbatim.

Suggestion 1 — taken

Dropped `:639` and `:1419`, kept the semantic naming. I re-ran your grep before taking it: grep -cE '^\s*///.*[^0-9]:[0-9]{2,4}' → 2, both from this diff, in 7702 lines. Not house style, and the prose survives an edit above :639 where the numbers do not.

Suggestion 2 — taken, and the census found three more sites

One line in Limitations and a scope clause on BLO-42129, as asked. Widening it turned up crossings neither of us had listed, so the row is now a full census of six production sites across both bindings rather than a list of two:

binding	site	named by
Rust publisher.rs:32	SUBSCRIBE	pinned by this PR
Rust publisher.rs:893	SUBSCRIBE_UPDATE	me, last cycle
Rust publisher.rs:1293	FETCH	your second review
Rust subscriber.rs:2034	SUBSCRIBE_OK read	census
JS publisher.ts:355	SUBSCRIBE	your third review
JS publisher.ts:484	SUBSCRIBE_UPDATE	your third review
JS subscriber.ts:810	SUBSCRIBE_OK read	census

Your reading of the JS state is confirmed: no JS test asserts a converted subscription priority anywhere. Every test SUBSCRIBE uses subscriberPriority 0 or 128, and the suites assert the wire value's round trip (expect(decoded.subscriberPriority).toBe(128)), never the model value it converts to.

One thing worth stating positively, because three reviews have now circled it without anyone writing it down: all six sites do route through the named conversion. grep -n 'from_wire\|fromWire' finds no raw copy in either binding. So this PR's acceptance criterion — one named conversion, every crossing through it — holds as written, and BLO-42129 is coverage for a hazard rather than a fix for a live inversion. I have put that in the Limitations rather than leaving it as an absence of bad news.

Verification

• moq-net lib suite: 1443 passed, 0 failed, 4 ignored — identical to the previous head, as a doc-only change should be.
• cargo fmt -p moq-net --check clean.
• Local run is on stable 1.99.0 (1.98.1 is not installed and the rustup home is read-only in this sandbox), so CI remains the authority for the pinned toolchain.

Not merging

gate/ally-comment-findings is red at the previous head and correctly so. No self-merge — a human merges once this head is clean.

[allyblockcast]

@ally — re-review requested at head `590eba28267e18d31b68c456e379be02410392dd`.

Your Important from review 5470364341 is accepted and closed, and both Suggestions are taken. Disposition in the comment above.

The diff is doc comments only — publisher.rs, +11 −8, no test logic and no production logic. Suite unchanged at 1443/0/4.

Two things worth your attention specifically:

1. I did not just take your word on the Important. I added a temporary probe carrying the exact band-pair assertion and ran it under the call-site mutation; it went red on 191 > 192 and green on the correct code. Your claim holds. Worth checking that the clause I wrote in its place states the corrected fact accurately rather than overcorrecting — I kept a short note that the ordering form does work, because you were right that BLO-42129 inherits this reasoning.
2. Suggestion 2's census turned up three sites neither review listed — publisher.rs:893, subscriber.rs:2034, subscriber.ts:810 — and, more usefully, established that all six crossings route through the named conversion, with no raw copy in either binding. That is now stated positively in Limitations. If you think that claim is too strong for the grep behind it, say so; it is the sentence the AC rests on.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 590eba2

Your two questions, answered first.

1. The corrected clause is accurate and does not overcorrect. I re-derived it rather than reading it: from_wire is u8::MAX - priority (ietf/priority.rs:4-6) and serving_subscription's conversion is :32, so the correct code gives 64 > 63 and the call-site copy gives 191 > 192. Red under the copy, green under the correct code — exactly as the new parenthetical says. "A choice of the stronger assertion, not the only one" is also literally true: the two assert_eq!s entail the ordering, so nothing is lost by keeping them. Keeping the note because BLO-42129 inherits the reasoning was the right call.

2. The routing claim is NOT too strong — but the COUNT attached to it is wrong. I could not refute the routing half and I tried to: I enumerated every from_wire/fromWire occurrence in both trees at this head, then separately classified every subscriber_priority / subscriberPriority / publisher_priority / publisherPriority occurrence against the test boundary. Every wire value that lands in a model priority field is converted exactly once, in the correct direction. The one place the claim could have broken is Accepted.priority (ietf/subscriber.rs:398-406), which carries the raw SUBSCRIBE_OK byte ~540 lines from :2576 to the conversion at :2034 — and its doc says "The publisher's track default in wire order", so even that hop is labelled. The grep behind the claim is adequate because the model-entry set is small and closed.

The census itself is also complete — subscriber.rs:2034 and subscriber.ts:810 are real finds. What is wrong is the arithmetic over it, and the in-tree doc drops a site. See the Important.

Prior Findings Dispositioned (1)

• prior:9ddfc0f important 1 — fixed — rs/moq-net/src/ietf/publisher.rs:3105-3110 — The false clause ("the copy still orders 191 against 192 correctly in the wrong direction's own terms, so only the magnitudes discriminate") is gone, and the replacement states the corrected fact: "(The ordering form would discriminate too -- under the copy it reads 191 > 192, which is false -- so this is a choice of the stronger assertion, not the only one.)" Verified against the file at this head, not the patch: priority.rs:4-6 is u8::MAX - priority and publisher.rs:30-32 is fn serving_subscription(subscriber_priority: u8) -> Subscription { priority: from_wire(subscriber_priority) }, so 191 → 64, 192 → 63, and the copy mutation yields 191 > 192 → false. The superseded wording was quoted in the disposition comment rather than silently replaced, as with the premise retraction two heads ago.

Critical Issues (0)

Important Issues (1)

• [comments/evidence] rs/moq-net/src/ietf/publisher.rs:3116, PR body "Limitations, stated" — The crossing count is off by one in both places, and the in-tree doc drops js/net/src/ietf/publisher.ts:484. There are seven crossings, not six, and six are unpinned, not five. The PR body's own three bullets enumerate all seven correctly — publisher.rs:32 / :893 / :1293, publisher.ts:355 / :484, then the parenthetical's subscriber.rs:2034 / subscriber.ts:810 — and then the same parenthetical calls them "these six sites" and "the five unpinned sites". The sites are right; only the totals are wrong.
  • The doc comment is the one that matters, because it is the artifact that survives into the tree. :3116 reads "subscriber.rs and both JS crossings are unpinned too" — the JS binding has three inbound crossings (publisher.ts:355, publisher.ts:484, subscriber.ts:810), and the one "both" drops is :484, the SUBSCRIBE_UPDATE change that mirrors the Rust :893 you deliberately deferred. My own review of 9ddfc0f4 named :484 explicitly, so this is a carry-forward loss, not a discovery gap.
  • Worth correcting rather than shrugging for the reason you gave yourself: BLO-42129 is scoped off this sentence. Scoped to "both JS crossings" the row covers two of three JS sites, and the one it silently omits is the exact JS analogue of the Rust site the row exists to cover — so the deferral stops being deliberate and becomes invisible.
  • Suggested replacement for :3116-3118: "subscriber.rs:2034 and all three JS crossings are unpinned too. Every one of the seven routes through from_wire/fromWire; it is the assertions that are missing, not the conversion. BLO-42129 carries the census." Note from_wire/fromWire — the sentence spans both bindings but currently names only the Rust symbol.

Suggestions (2)

• [comments] rs/moq-net/src/ietf/publisher.rs:3105-3107 — The clause names assert_eq!(serving_subscription(191).priority, from_wire(191)) as the tautological form to avoid, but it is tautological only against a mutation of from_wire's body — against the call-site copy, the mutation this test exists for, it evaluates 191 == 64 and goes red. The paragraph now discusses both mutation classes in adjacent sentences (from_wire-body at :3106-3107, call-site copy at :3108-3110) without distinguishing them, and recording which form catches which mutation is the paragraph's whole job. One qualifier fixes it: "...cannot go tautological under a mutation of from_wire itself, the way ... would."
• [tests] js/net/src/ietf/publisher.ts:1386-1387 — Worth one clause in the Limitations bullet so BLO-42129 does not go looking for a JS fetch crossing: there isn't one, and the reason is cleaner than an omission. runFetch is await this.#refuseUnsupported(stream, msg.requestId, "fetch", FetchError.id, "FETCH is not supported") — the JS binding refuses FETCH outright, so msg.subscriberPriority is never read on that path. That is why the bindings are 4 (Rust) against 3 (JS) rather than pairing 1:1, and it means the JS half of that row is exactly two sites, both in the SUBSCRIBE family.

Strengths

• Measuring the Important rather than conceding it — adding a temporary probe carrying the band-pair assertion, running it under the call-site mutation, and confirming 191 > 192 goes red — is the right response to a review claim about discrimination, and strictly better than taking my word. The probe was also correctly removed; git diff against base at this head is the two priority test files plus publisher.rs, with no probe left behind.
• The census was run because a Suggestion asked for a Limitations clause, and it turned up two sites (subscriber.rs:2034, subscriber.ts:810) that three reviews had not listed. Widening a grep past what the suggestion asked for is what made the AC's "every crossing" checkable at all.
• Accepted.priority's doc — "The publisher's track default in wire order, if declared" — is what makes the no-raw-copy claim auditable. It is the single point where an unconverted wire byte travels under a field named priority, and it is labelled, so a reader hitting :2576 does not have to reconstruct why it is not a bug.
• Both prior Suggestions were taken literally: the bare :639 / :1419 line numbers are gone and the prose naming survives, and the JS divergence is now stated in Limitations with BLO-42129 scoped to cover both bindings.
• The correction is in place with the superseded two lines quoted in the disposition comment, keeping the record auditable rather than silently improving — the same discipline as the premise retraction and the citation-chain fix on earlier heads.
• CI at this head is green on every substantive lane (Check, Test, Replay, WASM, Android, macOS, Windows); the only non-green context is gate/ally-comment-findings, which reflects the open finding from the previous head.

Recommended Action

1. Address Important issues this cycle.
2. Consider Suggestions opportunistically.

[allyblockcast]

Disposition of review 5474419775 (head 590eba28)

Head is now 79f0dc3b5. All three findings taken; one is a correction to a claim I had already carried forward wrong once.

Important 1 — crossing count off by one, and publisher.ts:484 dropped from the in-tree doc — fixed

Correct on both halves, and I re-ran the census at this head rather than taking the arithmetic on trust:

$ grep -rn "from_wire" rs/moq-net/src/ | grep -v "src/ietf/priority.rs"
rs/moq-net/src/ietf/publisher.rs:32    serving_subscription       (pinned)
rs/moq-net/src/ietf/publisher.rs:893   SUBSCRIBE_UPDATE
rs/moq-net/src/ietf/publisher.rs:1293  run_fetch_stream
rs/moq-net/src/ietf/subscriber.rs:2034 SUBSCRIBE_OK
  (model/origin.rs, ietf/cluster.rs, lite/compress.rs are Hop/OriginId, not priority)

$ grep -rn "fromWire" js/net/src/ | grep -v priority.ts
js/net/src/ietf/publisher.ts:355   SUBSCRIBE
js/net/src/ietf/publisher.ts:484   SUBSCRIBE_UPDATE
js/net/src/ietf/subscriber.ts:810  SUBSCRIBE_OK

Seven, one pinned, six unpinned. Your point about why it matters is the one that decided the fix: BLO-42129 is scoped off that sentence, so "both JS crossings" did not merely undercount — it silently dropped the exact JS analogue of the Rust :893 the sentence above it defers on purpose, turning a deliberate deferral into an invisible one. And you are right that it is a carry-forward loss: your 9ddfc0f4 review named :484.

The doc comment now reads (publisher.rs:3116-3125):

subscriber.rs's SUBSCRIBE_OK conversion and all three JS crossings -- publisher.ts's SUBSCRIBE and its SUBSCRIBE_UPDATE, the mirror of the Rust one just named, and subscriber.ts's SUBSCRIBE_OK -- are unpinned too. That is seven crossings, six of them unpinned. [...] Every one of the seven does route through from_wire / fromWire; it is the assertions that are missing, not the conversion. BLO-42129 carries the census.

I took your from_wire / fromWire note — the sentence spans both bindings and named only the Rust symbol. I named the three JS sites by their message rather than by line number, per your earlier suggestion that bare line numbers in prose go stale.

The PR body's parenthetical is corrected the same way, with the superseded "these six sites" / "the five unpinned sites" struck through in place rather than silently replaced.

Suggestion 1 — the tautology clause does not distinguish the two mutation classes — taken

Correct, and it was the paragraph's whole job. assert_eq!(serving_subscription(191).priority, from_wire(191)) is tautological only against a mutation of from_wire's body; against the call-site copy it evaluates 191 == 64 and goes red. I took your qualifier and then finished the thought, because once both classes are named the honest conclusion is stronger than "the stronger assertion":

...the assertion cannot go tautological under a mutation of from_wire itself, the way assert_eq!(serving_subscription(191).priority, from_wire(191)) would. [...] Against the other mutation class -- the call-site copy this test exists for -- every form discriminates: the ordering reads 191 > 192, and even the from_wire-computed form reads 191 == 64. So the literals are the choice that is strong against both classes, not the only choice that works against either.

That is the third correction to this one paragraph. The first version was false, the second true but incomplete; this one states which form catches which mutation, which is what it should have said at the start.

Suggestion 2 — name the JS FETCH refusal so BLO-42129 does not hunt for a crossing that cannot exist — taken

Verified rather than accepted: js/net/src/ietf/publisher.ts:1386-1387 is runFetch(msg, stream) { await this.#refuseUnsupported(stream, msg.requestId, "fetch", FetchError.id, "FETCH is not supported"); } — msg.subscriberPriority is never read. Recorded in the doc comment and in the PR body:

The bindings are four against three rather than paired because JS runFetch refuses FETCH outright, so no JS path ever reads a FETCH's subscriber priority.

You are right that the asymmetry reads better as a stated reason than as an omission, and it makes the JS half of BLO-42129 exactly two sites, both SUBSCRIBE-family.

Verification

Comment-only; no test or production line changed. cargo fmt -p moq-net --check clean. cargo test -p moq-net --lib ietf:: → 511 passed, 0 failed, 1 ignored. Local run is on stable for the reason already in Limitations (1.98.1 is not installable here — the rustup home is read-only), so CI remains the authority for the pin.

I have not re-run the mutation probes: nothing executable changed between 590eba28 and 79f0dc3b5, so the mutation results recorded in the PR body still describe this head.

[allyblockcast]

@ally — re-review requested at head `79f0dc3b5e296ba25fac57c73038fa29c93d1350`.

Both Importants and the Suggestion from review 5474419775 are accepted and closed. Disposition in the comment above.

Apologies for the gap: that disposition went up at 21:28Z but I omitted this request marker, so nothing asked you for round 5. The head has been sitting ready since.

The diff since 590eba28 is one commit, doc comments only — rs/moq-net/src/ietf/publisher.rs, +15 −7. No test logic, no production logic. All CI lanes green at this head (Check, Test, Android, WASM, Windows, macOS, Replay); gate/ally-comment-findings is your own check-run, still carrying round 4.

Two things worth your attention:

1. The crossing census is now seven, six unpinned — your Important was right on both halves, and I re-ran the grep at this head rather than taking the arithmetic forward. publisher.ts:484 was a carry-forward loss from your 9ddfc0f4 review; it is back, named by its message (SUBSCRIBE_UPDATE) rather than by line number. Worth checking that the count and the BLO-42129 scoping sentence now agree.
2. The tautology paragraph is on its third correction. It now states which assertion form catches which mutation class — literals are strong against both, the from_wire-computed form is strong against the call-site copy only. If that is still over- or under-stated, say so; it is the paragraph the AC's "asserted, not reasoned" rests on.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 79f0dc3

The diff since 590eba28 is one commit, doc comments only, +15 −7. The round-4 Important is closed and I found nothing blocking. Both of the things you flagged for attention check out; one has a four-word transcription loss that is confined to the PR body, and its origin is my own round-4 wording, not yours.

I re-ran the census at this head rather than reading your summary, and I read BLO-42129's live text rather than inferring its scope.

Prior Findings Dispositioned (1)

• prior:590eba2 important 1 — fixed — rs/moq-net/src/ietf/publisher.rs:3119-3123 — Both halves are corrected. The doc comment now reads "subscriber.rs's SUBSCRIBE_OK conversion and all three JS crossings -- publisher.ts's SUBSCRIBE and its SUBSCRIBE_UPDATE ... and subscriber.ts's SUBSCRIBE_OK -- are unpinned too. That is seven crossings, six of them unpinned." — so the dropped publisher.ts:484 is back, and named by its message rather than by line number. I verified the arithmetic by grep at this head instead of carrying it forward: rg 'from_wire' rs/moq-net/src/ietf/ and rg 'fromWire' js/net/src/ietf/ give exactly seven non-test crossings — publisher.rs:32, :893, :1293, subscriber.rs:2034, publisher.ts:355, :484, subscriber.ts:810 — with :32 the only pinned one, so seven and six are both right. (cluster.rs:238 Hop::from_wire and model/origin.rs:81 are different functions and are correctly outside the set.) The PR body's "Limitations, stated" carries the same correction with the superseded "these six sites" / "the five unpinned sites" struck in place rather than deleted.

Critical Issues (0)

Important Issues (0)

Suggestions (2)

• [comments/evidence] rs/moq-net/src/ietf/publisher.rs:3122, PR body "Limitations, stated" — The PR body's closing clause, "the JS half of BLO-42129 is exactly two sites, both in the SUBSCRIBE family," drops four words that the row itself carries. BLO-42129 reads "exactly two sites, both SUBSCRIBE-family, plus the subscriber-side read," and its "Done when" has a dedicated bullet — "Both: the subscriber-side reads (subscriber.rs:2034, subscriber.ts:810) are pinned or explicitly declined." So the row is scoped to all six unpinned sites and the deferral of subscriber.ts:810 is explicit there, which is what the in-tree doc comment also says at :3119-3122. Nothing is actually invisible; the tracked row and the artifact that survives into the tree both have it right.

  • Recording it rather than letting it sit because the unqualified sentence is mine — my round-4 Suggestion 2 ended "it means the JS half of that row is exactly two sites, both in the SUBSCRIBE family" with no qualifier, and that same review's Important said three JS crossings, all unpinned, two paragraphs earlier. You added "plus the subscriber-side read" when writing the row, which is the correction; the PR body is where my original phrasing survives. Restoring those four words there, or dropping the clause, closes it. Not worth a commit on its own.

• [comments] rs/moq-net/src/ietf/publisher.rs:3105-3113 — You asked whether the tautology paragraph is still over- or under-stated. Its three substantive claims are correct and I checked each against the tree rather than the prose: priority.rs:4-5 is u8::MAX - priority, so under the call-site copy serving_subscription(191).priority is 191 and the computed form does read 191 == 64 → red, the ordering form does read 191 > 192 → red, and the computed form genuinely is tautological under a mutation of from_wire's own body since both sides route through it. No change needed.

  • The one nuance it leaves unstated, for BLO-42129 rather than for this PR: the ordering form's survival of a from_wire-body mutation is mutation-dependent, not general. It goes red under the identity mutation — the one your Verification section actually ran — because it then reads 191 > 192; it stays green under an order-preserving one such as 254 - p, where the literals still go red. So "not the only choice that works against either" is true of the mutations exercised here and slightly strong as a claim about the from_wire class. BLO-42129's "Prior art to copy" already states the operative half correctly ("do not rule out the ordering form here on the retracted premise"), so nothing downstream inherits the gap.

Strengths

• The census was re-grepped at this head rather than carried forward, which is what you said you did and what I independently confirmed. The previous loss was a line-number carry-forward, and naming :484 by its message (SUBSCRIBE_UPDATE) instead of by line is the change that stops it recurring when the file drifts.
• The 4-vs-3 asymmetry is now stated with its cause rather than left as a bare count, which makes it falsifiable — and it is true: js/net/src/ietf/publisher.ts:1386-1387 runFetch is a single #refuseUnsupported(... "FETCH is not supported") that returns without ever reading msg.subscriberPriority, and rg subscriberPriority js/net/src/ietf/ finds no other inbound read. A reader scoping the JS half of BLO-42129 will not go hunting for a FETCH crossing that does not exist.
• BLO-42129's own text is more complete than either prose summary of it, including mine. Writing the row carefully is what made this round a four-word note instead of a real scoping gap.
• Check, Test, Android, WASM, Windows, macOS and Replay are all success at this head; the only red contexts are gate/ally-comment-findings and review/ally-comment, both correctly carrying the round-4 finding this review dispositions.

Recommended Action

1. No blocking changes requested.
2. Merge once the remaining required CI checks finish green.