Repository navigation
feat(js/net): head fills and time lookups - #8698
Conversation
Quest-Claim: 4c4499b1ac7b7a83
… new ranges, quest done Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…up an answer gone unstarted, walk a time end from the reach - Index every cached group's frame-0 start beside the checked bounds, counting one accepted below the eviction watermark where a local handler serves the history, as Rust's `TrackState::starts` and `index_start` do. A lookup answered there now starts the range at the answer instead of stalling or reaching past the missing groups. - Wait on a lookup's answer only while a cached copy from frame 0 may still write it, as Rust's `Slot::may_start`: an answer evicted unstarted falls back to the groups held. - A time end resolved after an unanswered lookup walks from the oldest group held, as Rust's `Window::found` seeds its trail at the reach. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou'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 35 minutes. View limit details
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. |
There was a problem hiding this comment.
Important
A head fill that a local handler answers with dropGroups, or closes unanswered, gets asked again on every poll. The loop never ends and the bounded reader never finishes. Reproduced with a scratch test (details inline).
Reviewed changes
I reviewed the full diff: JS head fills, Fill.time lookups, the missable floor, and the FetchFrom plumbing, checked against the Rust Cursor::ask_head/give_up_heads, Window::found and unresolved.
- Head fills: a bounded reader that finds a cached group starting past frame 0 asks a one-group fill. A copy from frame 0 replaces the headless one in the cache, and the old copy keeps feeding the readers already holding it.
- Fetch frame start: the lite publisher passes a FETCH's
startFramethrough the internalFetchFrom, so#lacksHeadmeasures a fetch against the frame it asked for. - Time lookups: a time start the held groups can't place opens a
Fillwithtimeset. It owes no group, ends at its firstaccept, and falls back to the oldest group held if it closes unanswered or its answer leaves the cache before it starts. #startsindex: unverified starts below the eviction watermark now count for time resolution when a local handler serves the history, as Rust'sTrackState::startsdoes.- Missable floor: a time start reaching back past every held group asks no miss below the oldest of them.
Things I checked that hold up: calling refuseGroups, dropGroups or close on a lookup drops nothing, because unresolved() is empty for it. An arrival reader wakes when an answer's first frame lands, through dst.readable() → timelineChanged. A headless copy never fires admitGroupStart, so a replaced copy cannot leave a stale bound behind. Over the wire (lite-06 and lite-07, upstream NotFound), the session settles a missing head after one FETCH, so the loop below only happens with a local handler.
claude-opus-5-5 | 𝕏
…ds without one A head fill a local handler answered with `dropGroups`, or closed unanswered, left the group neither refused nor covered, so every poll asked a new fill for it without end. Each reader now keeps the heads it asked (`#heads`), clipped to what it still reads, and gives up one met again with no fill bringing it, as Rust's `Heads` and `give_up_heads` do. The lite-07 end-to-end test now waits for the publisher's SUBSCRIBE_DROP of the groups it refused before asking the head: that verdict predates the archive holding group 2, and arriving after the ask it ends the head's fill, as in Rust. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`FillState.dropped` resolved every fill overlapping a track drop, so a SUBSCRIBE_DROP the feed sent before a head was asked, landing after, ended the head's FETCH, and the reader gave the group up though its source could still bring frame 0. It now skips the one-group fill of a group the cache holds headless, as Rust's `drop_groups` touches no fill. The lite-07 end-to-end test drops its wait for that drop: it now covers that ordering. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ No new issues found. The head re-ask loop from the prior review is fixed, and the follow-up drop fix is scoped correctly.
Reviewed changes
Reviewed the two commits since the prior Pullfrog review: per-reader head tracking, and leaving a held group's head to its own fill.
- Heads asked once:
Subscriber.#headsrecords each head a reader asked. A head met again with no fill bringing it is refused asNotFoundinstead of asked again. It is clipped and cleared at the same sites as#givenUp, so widening back over a head asks it again. - Track drops leave head fills alone:
FillState.droppedskips a one-group live part whose sequence the cache holds as"headless", so a late SUBSCRIBE_DROP no longer resolves the head's FETCH. The fill's owndropGroupsstill resolves it throughfillOps.drop→#resolve, and an unanswered close still finishes it through#settle. BecauseProducercallsfills.arrivedon every cache insert, a miss never has a live part on a held sequence, so the skip only affects head fills.holds()returnsfalsefor an aborted copy, so an aborted headless copy's drop still settles its head. - Tests: added coverage for drop and close in both cursor modes, plus the late-drop ordering. The lite-07 end-to-end test no longer waits for the drop.
claude-opus-5-5 | 𝕏
# Conflicts: # js/net/src/lite/copy_fill.test.ts # quest/m3/subscribe-fills-js.md
There was a problem hiding this comment.
✅ No new issues found. The only change since the last approval is a merge of
main, and it changes no behavior.
Reviewed changes
I reviewed 969b556f0..169908e0a, the merge of origin/main.
- Test imports:
fill.test.tsandlite/copy_fill.test.tsnow importManualfrom@moq/clock/test, which is wheremainmoved it (js/clock/src/test.ts).copy_fill.test.tsstill imports and usesacceptSession/connectSession. - Quest file: the PR now deletes the quest at its new path,
quest/m3/subscribe-fills-js.md, aftermainmoved it there.
The head fill, lookup, and FillState.dropped logic is the same as what was approved at 969b556f0.
claude-opus-5-5 | 𝕏

Claims quest/m3/subscribe-fills-js.md (moved from
quest/m2/subscribe-ranges/js-fills.mdon main) and deletes it.Problem
JS lagged Rust in two places:
Requestshandler.Rust's cursor waits on a head fill (
Heads,lacks_head) and looks the time up throughFill::time(Window::looking).Approach
track.ts,fill.ts): A bounded reader that finds a group starting past frame 0 asks a one-group fill and holds the group until it is answered. This works in arrival and sequence order, and the group still goes out in its turn.accept, or any copy arriving from frame 0, replaces a cached copy that can't serve frame 0 (it started past it, or shed it). The old copy leaves the cache but keeps feeding the readers already holding it.NotFound, and the reader moves past it.fills.moved. A stale one is written off by the reader's own budget through the existing#askFillscovered loop.fetchGroupof a headless cached group now asks a head fill, as Rust'scached_or_queueddoes. The lite publisher serving a FETCH passes its start frame through an internalFetchFromoption, so a FETCH from frame 5 still takes a copy that starts at 5.#runFillsalready FETCHes from frame 0 on every lite version.Fill.time(window.ts,fill.ts,track.ts): Ports Rust'sunresolved,Window::lookups,Window::foundandCursor::look_up.wanted()isundefined, and it ends at its first accept. If it closes unanswered, the start reaches back to the oldest group held.Holdings.looksUp), as Rust'slooks_upexcludes received copies.Window::missable. A time start reaching back past every held group asks no miss below the oldest of them. JS asked fills from group 0 here before this PR, even without lookups.Decisions
No frame start on
Fill(maintainer, 2026-11-13, via /quest-complete): ✅ leave it out of JSFillfor now (rejected: addingframeStart(sequence): numberthat always returns 0). JS fills always start at frame 0 and nothing reads it; adding it when JS gets flights is additive. The quest's Plan line asked for it, matching Rust'sFill::start; this PR deletes that quest, and no quest owns JS flights yet. The frame start that does matter is a FETCH's, which stays internal (FetchFrom).Design checkpoint at Codex round 3 (taken by the agent under /quest-afk, for the maintainer's review): ✅ continue with the current approach and fix the finding narrowly in
FillState.dropped(rejected: rethink fill settlement so that, as in Rust, only a fill's own source ever settles it; merge as is; drop the PR). The PR already follows Rust except at the track-drop path, and round 4 came back clean. Decided 2026-11-14 (maintainer, /quest-complete): ✅ merge with this approach.Choices I made without asking (the maintainer couldn't be prompted):
ask_headresolves toErrandgive_up_heads. Before this PR, JS handed the headless group out.dropGroupson a head records a track drop, as Rust'sdrop_groupsdoes, even though a headless copy stays cached.#heads, as Rust'sHeads): a head met again with no fill bringing it, as after a handler'sdropGroupsor an unanswered close, is given up asNotFoundinstead of asked again on every poll.drop_groupstouches no fill: a SUBSCRIBE_DROP the feed sent before the head was asked says nothing of whether the FETCH can bring frame 0. Misses are unchanged: a drop still resolves a fill for a group the cache lacks.duplicate group.Impact
Track.Fill.time?: Timestampis added, mirroring Rust'sFill::time. Not breaking.FetchGroupOptionsis unchanged;FetchFromis@internal.fetchGroupof a headless cached group asks a head fill, or failsNotFoundwith no handler.Alternatives
Slotdoes, instead of replacing the headless copy. Rejected: replacement reuses the existing failed-copy path, and only readers already holding the old copy need it.Pendingkind that misses can't join. Unneeded: a miss and a head never cover the same sequence, since a headless copy is cached and so is no miss.Caused by
No single PR. JS never had head fills, lookups, or the missable floor; moq-dev/moq#7845 deferred the first two here on purpose.
Validation
bun test srcinjs/net: 3291 pass. Repeated 4 times with no failure.just check: clean.fill.test.ts,window.test.ts,lite/copy_fill.test.ts):fetchGroupfrom frame 0 asks the head.#resolvefix.Windowlookups,found, andmissable.runFetch), and the JS publisher, holding no group 2, fills it from its archive through the same frame-0 fetch path. 12 isolated runs passed.Producernow keeps#startsbeside#bounds, as Rust'sTrackState::starts, counting an unverified start where a local handler serves the history;Holdings.nextHeldreads it too, as Rust'snext_helddoes.Slot::may_start).Window::found.FillState.dropped. The end-to-end test needs no wait for that drop, and passed 5 of 5 runs with exactly one FETCH of group 2. Both new tests fail without their fixes.bun test srcinjs/net, 3298 pass;just checkclean.copy_fill.test.tsimports (Manualnow comes from@moq/clock/test);subscriber.tsfrom feat(net): a refused forwarded route drops the route, not the session #8672 and fix(net): a lite-07 reader survives browsers losing a group stream's reset #8673 merged cleanly. After the merge:bun test srcinjs/net, 3326 pass;just checkclean.just ci(whole tree).Follow-ups
None.
Models
Size: M
Implementer: Claude, claude-opus-5-5, unknown effort
Implementer: Claude Code, claude-opus-5-5, unknown effort (iterate, M)
Implementer: Claude Code, claude-opus-5-5, unknown effort (iterate, XS)
🤖 Generated with Claude Code
(Written by Claude Opus 5.5)
Review
Rewritten by
quest reviewafter each round.Head: 169908e
Base: origin/main at 99d789e
Reviewer: Codex, gpt-6.1-sol, high effort
Session:
01a1d7f8-f3d8-75f3-9518-e2902db10d54, round 5Score: 10/10,
The incremental change only merges the updated base, with compatible import resolutions and no new issues found in the branch’s integration; no builds or tests were run.Rounds: 5; findings 4 (high 1, medium 3): fixed 4, refuted 0, open 0
Findings: none