Skip to content

feat(js/net): head fills and time lookups - #8698

Merged
kixelated merged 9 commits into
mainfrom
quest/m2/subscribe-ranges/js-fills
Nov 14, 2026
Merged

kixelated merged 9 commits into
mainfrom
quest/m2/subscribe-ranges/js-fills

Conversation

@kixelated

@kixelated kixelated commented Nov 14, 2026 •

Copy link
Copy Markdown
Collaborator

Claims quest/m3/subscribe-fills-js.md (moved from quest/m2/subscribe-ranges/js-fills.md on main) and deletes it.

Problem

JS lagged Rust in two places:

  • A bounded JS reader handed out a cached group that was missing its first frames.
  • A time start that no held group could place reached back to group 0 without asking the track's Requests handler.

Rust's cursor waits on a head fill (Heads, lacks_head) and looks the time up through Fill::time (Window::looking).

Approach

  • Head fill (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.
    • A fill's 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.
    • A refused head, or one asked with no handler open, is refused to that reader as NotFound, and the reader moves past it.
    • A head the reader's bounds no longer admit is withdrawn by the existing fills.moved. A stale one is written off by the reader's own budget through the existing #askFills covered loop.
  • Fetch from frame 0: A JS fetchGroup of a headless cached group now asks a head fill, as Rust's cached_or_queued does. The lite publisher serving a FETCH passes its start frame through an internal FetchFrom option, so a FETCH from frame 5 still takes a copy that starts at 5.
  • Received copy: The session needed no change. A head fill is one group, which #runFills already FETCHes from frame 0 on every lite version.
  • Fill.time (window.ts, fill.ts, track.ts): Ports Rust's unresolved, Window::lookups, Window::found and Cursor::look_up.
    • A span whose time start the held groups can't place reads nothing until its lookup answers.
    • A lookup owes no group: wanted() is undefined, and it ends at its first accept. If it closes unanswered, the start reaches back to the oldest group held.
    • Only a non-wire copy with a handler open looks times up (Holdings.looksUp), as Rust's looks_up excludes received copies.
  • Missable floor: Ports Rust's 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 JS Fill for now (rejected: adding frameStart(sequence): number that 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's Fill::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):

  • Head ask with no handler gives up at once, as Rust's ask_head resolves to Err and give_up_heads. Before this PR, JS handed the headless group out.
  • A handler's dropGroups on a head records a track drop, as Rust's drop_groups does, even though a headless copy stays cached.
  • A reader asks each head once (#heads, as Rust's Heads): a head met again with no fill bringing it, as after a handler's dropGroups or an unanswered close, is given up as NotFound instead of asked again on every poll.
  • A track drop leaves a held group's head to its own fill, as Rust's drop_groups touches 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.
  • Any copy from frame 0 replaces a headless one, not just a fill's. A lite-06 re-offer or a lite-07 range widening can bring the whole group on the subscription while the head fill is out, and before this PR that arrival threw duplicate group.

Impact

  • Public API: Track.Fill.time?: Timestamp is added, mirroring Rust's Fill::time. Not breaking. FetchGroupOptions is unchanged; FetchFrom is @internal.
  • Behavior:
    • A bounded reader no longer hands out a group missing its head.
    • fetchGroup of a headless cached group asks a head fill, or fails NotFound with no handler.
    • A time start reaching back past every held group asks no fills below the oldest held group.
  • Wire: none.

Alternatives

  • Keeping several copies per cached sequence, as Rust's Slot does, instead of replacing the headless copy. Rejected: replacement reuses the existing failed-copy path, and only readers already holding the old copy need it.
  • A separate head-only Pending kind 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 src in js/net: 3291 pass. Repeated 4 times with no failure.
  • just check: clean.
  • New tests (fill.test.ts, window.test.ts, lite/copy_fill.test.ts):
    • Bounded arrival and sequence readers get a headless group whole from a local handler, and an open reader then reads the whole copy.
    • A refused head and a missing handler give the group up, while an open reader still reads the headless copy.
    • fetchGroup from frame 0 asks the head.
    • A lookup answered, a lookup closed unanswered, and no lookup when the held groups place the time.
    • Replacing the ranges withdraws a lookup that is still out. This test fails without the #resolve fix.
    • Window lookups, found, and missable.
  • End to end, on lite-05, -06 and -07: a JS client copy holding group 2 from frame 3 serves a bounded reader the whole group. The copy's session FETCHes group 2 exactly once (counted at the publisher's 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.
    • An earlier version counted every client bidi stream, and lite-07 once saw 2. The count now covers only FETCHes of group 2.
  • Codex review round 1 found three lookup gaps against Rust, fixed in 520c6aa, each with a test that hangs or fails without its fix:
    • A lookup answered below the eviction watermark: the answer is accepted unverified, so it never entered the start index and the range stalled or reached past the missing groups. Producer now keeps #starts beside #bounds, as Rust's TrackState::starts, counting an unverified start where a local handler serves the history; Holdings.nextHeld reads it too, as Rust's next_held does.
    • An answer evicted before its first frame landed kept the reader waiting. It now waits only while a cached copy from frame 0 may still start (Rust's Slot::may_start).
    • A time end after an unanswered lookup walked from group 0, below the reach no miss asks for. It now walks from the reach, as Rust's Window::found.
  • Pullfrog found a head re-asked forever once its fill ended without a copy, reproduced in all four drop/close and arrival/sequence combinations (4 asks, capped by the test). Fixed in 28f8fa9. That made the lite-07 end-to-end test fail deterministically: the publisher's SUBSCRIBE_DROP of groups 0 to 2, sent while the archive still refused group 2, lands after the head ask and resolved the head's fill. Codex round 3 flagged the same ordering, and 969b556 fixes it at the root in 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.
  • Codex round 3 reached the design checkpoint; see Decisions.
  • After the fixes: bun test src in js/net, 3298 pass; just check clean.
  • Merged main (169908e) for the moved quest file and copy_fill.test.ts imports (Manual now comes from @moq/clock/test); subscriber.ts from 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 src in js/net, 3326 pass; just check clean.
  • Not run: 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 review after each round.

  • Head: 169908e

  • Base: origin/main at 99d789e

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

  • Session: 01a1d7f8-f3d8-75f3-9518-e2902db10d54, round 5

  • Score: 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

@kixelated kixelated changed the title JS head fills and time lookups feat(js/net): head fills and time lookups Nov 14, 2026
…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>
@kixelated
kixelated marked this pull request as ready for review November 14, 2026 03:08
@coderabbitai

coderabbitai Bot commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

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 35 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: e1811ead-b4c8-4cb0-861a-1f5efeab554b
Commits

Reviewing files that changed from the base of the PR and between 99d789e and 169908e.

Files selected for processing: 12

  • js/net/api/index.api.md
  • js/net/src/broadcast.ts
  • js/net/src/fill.test.ts
  • js/net/src/fill.ts
  • js/net/src/group.ts
  • js/net/src/internal.ts
  • js/net/src/lite/copy_fill.test.ts
  • js/net/src/lite/publisher.ts
  • js/net/src/track.ts
  • js/net/src/window.test.ts
  • js/net/src/window.ts
  • quest/m3/subscribe-fills-js.md
  • 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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 startFrame through the internal FetchFrom, so #lacksHead measures a fetch against the frame it asked for.
  • Time lookups: a time start the held groups can't place opens a Fill with time set. It owes no group, ends at its first accept, and falls back to the oldest group held if it closes unanswered or its answer leaves the cache before it starts.
  • #starts index: unverified starts below the eviction watermark now count for time resolution when a local handler serves the history, as Rust's TrackState::starts does.
  • 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.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using claude-opus-5-5 | 𝕏

Comment thread js/net/src/track.ts Resolved
kixelated and others added 2 commits November 13, 2026 19:28
…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>
pullfrog[bot]
pullfrog Bot previously approved these changes Nov 14, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.#heads records each head a reader asked. A head met again with no fill bringing it is refused as NotFound instead 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.dropped skips 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 own dropGroups still resolves it through fillOps.drop → #resolve, and an unanswered close still finishes it through #settle. Because Producer calls fills.arrived on every cache insert, a miss never has a live part on a held sequence, so the skip only affects head fills. holds() returns false for 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.

Pullfrog  | View workflow run | Using claude-opus-5-5 | 𝕏

# Conflicts:
#	js/net/src/lite/copy_fill.test.ts
#	quest/m3/subscribe-fills-js.md

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.ts and lite/copy_fill.test.ts now import Manual from @moq/clock/test, which is where main moved it (js/clock/src/test.ts). copy_fill.test.ts still imports and uses acceptSession/connectSession.
  • Quest file: the PR now deletes the quest at its new path, quest/m3/subscribe-fills-js.md, after main moved it there.

The head fill, lookup, and FillState.dropped logic is the same as what was approved at 969b556f0.

Pullfrog  | View workflow run | Using claude-opus-5-5 | 𝕏

@kixelated
kixelated added this pull request to the merge queue Nov 14, 2026
Merged via the queue into main with commit 4c79c6a Nov 14, 2026
8 checks passed
@kixelated
kixelated deleted the quest/m2/subscribe-ranges/js-fills branch November 14, 2026 20:13
kixelated added a commit that referenced this pull request Nov 14, 2026
Resolves the conflict with main's VA-API reorder test (#8636), which sorts
timestamps that have no total order here, and gives main's new JS head-fill
test frames their required timestamps (#8698).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant