[kixelated]

Finishes quest/m2/api-ack-one-primitive.md.

Problem

moq_net::transport::poll::SendStream is public and unsealed, and it carried three ways to report acknowledgment: acknowledged(), delivered() with poll_delivered(), and the offset()/poll_acked pair moq-dev/moq#8739 added. Every method that ships at the cut is one every out-of-tree transport implements for good. The maintainer decided on 2026-11-15 to keep offset/poll_acked and collapse the rest.

Approach

• Trait: acknowledged, delivered, and poll_delivered are gone; Offset, offset(), and poll_acked remain.
• Egress billing (coding::Writer): a private Billing keeps a queue of minted offsets, each with the bytes the writer had written when it was minted. A write that ends a frame mints one. A park with nothing outstanding mints one for a frame still being written. Parks poll the oldest offset and bill through each one acknowledged. Acked::Unsupported falls back to billing the bytes taken, and an error stops billing. Offsets never outnumber the waiting frames plus one, and stats::Delivery already holds an entry per waiting frame.
• Open-stream log (transport/open.rs): it counts acknowledged streams by asking poll_acked at each stream's current offset. OpenStream is gone. The listing hands out (Rank, Arc<Listed>), and make_room reads only the rank, so it no longer asks the transport about acknowledgment.
• QUIC adapters (moq-tokio and moq-uring quic::web::send::Stream): type Offset is the driver's send::Offset, and poll_acked forwards to new crate-private driver methods. Offsets count the WebTransport header, and the writer maps them through its own byte count, so nothing needs adjusting. Once reset, a stream answers from the offset and acknowledged prefix it kept at the reset (Ended).
• Drivers: send::Stream::poll_acked, send::Finished::poll_acked, and Finished::offset are added, all crate-private. Bytes the peer acknowledged stay acknowledged; the rest fail with the peer's stop code or the connection's loss. A reset consumes the handle, so a handle never waits past one. moq-tokio checks for a foreign offset before taking the lock, and releases the lock before its one unreachable!.
• Memory transport: type Offset = u64 counts the bytes written. An offset is delivered once the reader has taken those bytes. A reset or stop fails the bytes not yet taken, and bytes taken before the reset still count.
• qmux: drops the three methods and keeps Acked::Unsupported, so its egress bills the bytes it accepted (decided 2026-11-15).
• Quests: deletes this quest. quic-ack-hook keeps only the public driver API and its trap tests. lag-histogram no longer requires quic-ack-hook, since the adapters now answer poll_acked, and it points at Billing's per-frame offsets.

Impact

• moq-net (breaking): transport::poll::SendStream::{acknowledged, delivered, poll_delivered} are removed. Migration:
  • An implementor deletes them. To report acknowledgments, it implements type Offset, offset(), and poll_acked. Otherwise it keeps type Offset = () and the default, which reports Acked::Unsupported.
  • A caller that read delivered() mints offset() after writing and awaits poll_acked(waiter, &offset).
  • acknowledged() becomes poll_acked at the current offset(), ready with Acked::Delivered.
• moq-net (breaking): transport::memory::SendStream's Offset changes from () to u64, and it implements poll_acked.
• moq-tokio, moq-uring (breaking): quic::web::send::Stream loses the three methods. Its Offset changes from () to the driver's send::Offset, and it implements poll_acked.
• qmux (breaking): SendStream loses the three methods.
• Behavior: egress bytes/frames bill as before once the stream finishes. A reset bills each frame whose end the peer acknowledged before it, plus the acknowledged part of a frame still being written when a park asked about it. Bytes acknowledged after the last offset asked about stay unbilled; that is at most one frame, measured below. A STOP_SENDING bills, the same way, what the peer acknowledged before it: the drivers answer poll_acked from the acknowledged prefix even though moq-quic fails the wait once the peer stops reading. Over qmux, egress bills the bytes the stream accepted, not what the link took.
• Wire: none.

Alternatives

• One outstanding offset per stream, re-minted after each acknowledgment, as the quest's guidance suggested. It mints less while acknowledgments lag. But a reset loses everything acknowledged past the last offset asked about, which can be a whole buffered group (measured below). It also saves no memory, since Delivery already keeps one entry per waiting frame.
• Asking the transport on every write instead of at frame ends gives byte-precise billing on a reset. The cost is one offset and one boxed allocation per write, and a peer could make the offsets outgrow the frames by granting credit a byte at a time.

Decisions

Taken while the maintainer was away; each can be undone in review.

• Offsets are minted per frame end rather than one outstanding offset per stream, based on the measurements under Validation.
• A frame still being written bills its acknowledged part only through an offset minted at a park while nothing else is outstanding. Otherwise it bills once its end is acknowledged.
• Each web adapter mints its stream's starting offset when it opens, so a reset stream always has an Ended to answer from without an Option or expect. That is one extra driver lock per opened stream.
• The driver methods stay crate-private. Making them public is left to quic-ack-hook, which now carries a Waiting decision on whether to do it at all.
• The open-stream log counts Acked::Unsupported as not acknowledged, as the old == Some(true) did.
• The JS OpenStream.acknowledged() stays. It belongs to JS's own open listing, not a transport trait, and is now only read by tests.

Validation

• just check passed, including test rs: moq-net 3385 lib tests plus integration, moq-tokio, moq-uring, and qmux.
• New tests:
  • writer::egress_bills_acknowledged_payload, and writer::egress_bills_taken_payload_without_acknowledgments for the unsupported fallback.
  • memory::an_offset_is_acknowledged_once_the_reader_takes_it.
  • moq-tokio web::session::an_offset_stays_acknowledged_through_a_reset and an_offset_stays_acknowledged_through_a_stop, over raw QUIC and WebTransport. The stop test fails without the review fix (Ready(Err(Stopped(3)))).
  • moq-uring integration web::an_offset_stays_acknowledged_through_a_reset, which ran on this host rather than skipping.
• Real surface: a throwaway moq-tokio test (not committed) ran a server session with stats against a subscriber over real localhost QUIC, 40 frames of 10 kB in one group. When the subscriber had read every frame, 21 of the 40 frames were billed (210000 B). All 40 were billed 6.6 ms later, over both moqt:// and https://. So QUIC egress bills on acknowledgment; billing the bytes taken would have billed all 40 at once.
• Reset accuracy, per frame end vs one outstanding offset: a throwaway link model drove the real Writer (bottleneck rate, RTT, unbounded send buffer, 20.8 kB frames) and reset the stream at 68 points across each run. The table reports payload acknowledged before the reset but not billed:

Scenario	Per frame end: mean / max	One outstanding: mean / max
5 Mbps live over 10 Mbps, 50 ms RTT	3.9 kB (0.5%) / 20.0 kB	11.9 kB (1.6%) / 40.8 kB
5 Mbps live over 4 Mbps (queueing), 50 ms	10.1 kB (1.7%) / 20.8 kB	79.0 kB (13.2%) / 238 kB
1.25 MB catch-up group over 10 Mbps, 100 ms	4.2 kB (0.4%) / 20.8 kB	247 kB (26.1%) / 1.22 MB

• Cost: a throwaway release-mode loop pushed 200k frames of 1 kB through a boxed memory stream, with the reader draining and the writer settling after each frame. It took about 167 ns/frame on this branch vs about 131 ns/frame on main. The extra is the minted offset: one transport lock plus one Box in the boxed layer per frame. One outstanding offset costs the same in this loop, because acknowledgments keep up and both policies mint once per frame; per-frame minting adds offsets only while acknowledgments lag.

Follow-ups

• quic-ack-hook is rewritten to what remains, the public driver API and its trap tests. Open decision: whether the drivers' wait should go public at all, since nothing in the tree needs it public. Recommendation: drop the public half and keep only the trap tests, on the crate-private methods, as an XS quest.

Models

Size: M
Implementer: Claude Code, claude-opus-5-5, unknown effort (arm B)

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

Review

Rewritten by quest review after each round.

• Head: 663c204

• Base: origin/main at 0e6e469

• Risk: high, The change affects customer billing accounting, where incorrect delivery counts could lose billable usage that a revert cannot recover.

• Classified: medium by default

• Reviewer: Codex, gpt-6.1-sol, high effort

• Session: 01a1dfd9-b5ee-7d93-bb9f-a5de65df51a2, round 2

• Score: 10/10, Both adapters now preserve acknowledged offsets after STOP_SENDING, and the incremental changes introduce no further findings; builds and tests were not run.

• Raised the risk to high: The change affects customer billing accounting, where incorrect delivery counts could lose billable usage that a revert cannot recover.

• Rounds: 2; first round 1 (medium 1); findings 1 (medium 1): fixed 1, refuted 0, open 0

• Findings: none

• The last round's findings, at 86dc9bd:

  • Fixed: medium, rs/moq-net/src/coding/writer.rs:425: Preserve acknowledged payload when the peer stops the stream; Both drivers now return success when acked_len() covers the requested offset despite a failed acknowledgment wait, preserving the acknowledged prefix after STOP_SENDING.

[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 15 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: a160d1e3-c05f-471c-a363-39bccc46f3d1

Commits

Reviewing files that changed from the base of the PR and between 505a86f and 663c204.

Files selected for processing: 22

• quest/m2/api-ack-one-primitive.md
• quest/m3/qos/lag-histogram.md
• quest/m3/quic-ack-hook.md
• rs/moq-net/src/coding/writer.rs
• rs/moq-net/src/fuzz/session.rs
• rs/moq-net/src/ietf/publisher.rs
• rs/moq-net/src/lite/test_transport.rs
• rs/moq-net/src/stats.rs
• rs/moq-net/src/transport.rs
• rs/moq-net/src/transport/boxed.rs
• rs/moq-net/src/transport/memory.rs
• rs/moq-net/src/transport/open.rs
• rs/moq-net/tests/integration/support/mock.rs
• rs/moq-tokio/src/quic/driver/send.rs
• rs/moq-tokio/src/quic/web/send.rs
• rs/moq-tokio/src/quic/web/session.rs
• rs/moq-uring/src/quic/driver/send.rs
• rs/moq-uring/src/quic/web/send.rs
• rs/moq-uring/src/transport/local.rs
• rs/moq-uring/tests/integration/web.rs
• rs/qmux/src/session.rs
• rs/qmux/src/session/tests.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

[pullfrog]

✅ No new issues found.

Reviewed changes

This review covered the full PR: SendStream now reports acknowledgment only through Offset/offset()/poll_acked, and egress billing, the open-stream log, and every backend move onto it.

• Trait collapse — acknowledged(), delivered(), and poll_delivered() are removed from transport::poll::SendStream and from every wrapper (boxed, ietf Ranked, fuzz Recorded, MockSendStream, moq-uring Local). The poll_acked docs now promise that acknowledged bytes stay acknowledged after a reset or stop.
• Per-frame egress billing — coding::Writer's new Billing mints an offset after each write that ends a frame, plus one idle offset at a park when none is outstanding. It polls them oldest first and falls back to billing bytes taken on Acked::Unsupported. It stops billing on error. Only tracked meters create a Billing, so untracked streams mint nothing.
• Open-stream log — OpenStream is gone. make_room now reads only Listed::rank(), and the DEBUG-gated Line::start count asks poll_acked at each stream's current offset with a noop waiter, which registers nothing.
• Memory transport — Offset = u64. An offset counts as delivered once the reader has taken every byte before it. Reset and stop fail only the bytes not yet taken, and the session-close check runs first so earlier reads still count.
• QUIC drivers and web adapters — moq-tokio and moq-uring add crate-private poll_acked methods. These keep the acknowledged prefix through STOP_SENDING and map other failures to the peer's stop code or the connection's loss. The unreachable! arm holds because every moq-quic move to a closed state calls lose() first, and a held handle is never released. The web adapters forward the driver's send::Offset, which already counts the header. After a reset they answer from the saved Ended { offset, acked }.
• qmux — drops the three methods and stays Acked::Unsupported, so its egress bills the bytes the stream accepted. This is the maintainer decision recorded in the Impact section.
• Quests — deletes the finished quest, cuts quic-ack-hook down to the public driver API and its trap tests, and removes lag-histogram's dependency on quic-ack-hook.

  ^{｜ View workflow run ｜ Using claude-opus-5-5 ｜ 𝕏}