[kixelated]

Problem

js/net bounded its cache only by time: each track's #prune aged groups out on a 30 s idle window, and group.ts capped one group's frame tail at 32 MiB. Nothing billed bytes across a broadcast's groups or tracks, so JS could not bound a broadcast in bytes the way Rust's cache::Pool does, and the Drop set quest had no pool to bill its runs to.

Closes the quest quest/m1/js-cache-pool.md.

Approach

js/net/src/cache.ts ports Rust's rs/moq-net/src/model/cache.rs, keeping its names and shape:

• Pool: one per broadcast (a track with no broadcast gets its own). It holds a byte capacity, the idle expiry (DEFAULT_EXPIRY, 30 s), the access-stamp mean, and the sweep timer. It defaults as broadcast::Info::default does in Rust: no capacity and 30 s expiry.
• Track: a track's account. While the pool is over capacity, writes accrue eviction debt (written * used / capacity, doubled when the oldest group is stale, capped at used). The account pays it by evicting its own oldest groups with Evicted. A group read after the pool's mean rotates to the back instead, and a group bigger than the remaining debt carries the debt over. The live edge is protected; with nothing evictable, the debt sheds the fronts of fresh groups and of the live edge.
• Charge: a group's bill. It covers frame bytes plus a fixed per-group overhead, and stamps access on write (boost 1) and read (boost 2) in 100 ms ticks.
• Idle expiry: a bounded scan on each write, a full scan before a new subscriber is mirrored, and a pool-wide sweep every half window. These replace #prune's per-track timer. The sweep holds only tracks with an evictable group and never holds a process open.

Wiring: track.ts bills each cached group through its account, evicts through the pool, and charges a fixed overhead per subscriber mirror. group.ts bills frame writes and tail trims, sheds its front on demand, and refreshes access on frame reads. A subscriber taking a group refreshes it too, as Rust's take does. broadcast.ts gives every track of a broadcast the same pool. Producer.#cache is now an insertion-ordered Set, so evicting a run of groups is linear.

Impact

• Public API: none. cache.ts and the new hooks entries are package-internal. CACHE_EXPIRY left the internal ledger.ts for cache.DEFAULT_EXPIRY. js/net/api/index.api.md is unchanged.
• Wire: none.
• Behavior:
  • Taking a group, reading a frame, or a FETCH hit now restarts a group's idle window, as in Rust; before, only writes did.
  • Expiry lags by up to half a window (the Rust sweep cadence) when nothing writes or subscribes, instead of one eighth.
  • A broadcast's pool has no capacity by default, as in Rust, so byte eviction runs only for a pool given one. Today only tests do that.

Choices made without asking

Each one is open to review; my recommendation is to keep it.

• Idle expiry still ends a group with TooFarBehind; byte eviction uses Evicted. Rust ends idle groups differently. Keeping JS's existing error stays inside this quest's scope.
• Closing a track restarts its former live edge's idle window. This keeps JS's existing behavior, so close never reclaims a group its subscribers have yet to drain. Rust does not do this.
• Each subscriber mirror bills 4 KiB, and each group bills 3 KiB. Mirrors are JS-only, and they are what a slow subscriber holds. Both constants were measured with Bun 1.4 heap stats (about 2.9 KB per group and 3.9 KB per mirror).
• An aborted group ages out like any other instead of being reclaimed eagerly. It is no longer the protected live edge.
• Access stamps read the clock on each touch rather than caching a coarse tick, since now() is cheap in JS.
• No public setter for the capacity. The quest's plan says to mirror Rust's API only once a JS consumer needs it.

Alternatives

• Keep #prune and add a byte cap per track: rejected by the quest's 2026-11-03 decision, which picked a shared per-broadcast pool so tracks of one broadcast compete for one budget, as in Rust.
• Bill the drop set's runs here: left to Drop set, per the split recorded in both quests. That quest now bills its runs to cache.ts.

Validation

• just ci on b351e24: everything passes except one @moq/net test, a connection samples the transport only while a reservation reads the estimate (js/net/src/connection/bandwidth.test.ts), with 3140 pass and 1 fail. That failure is not from this PR. It depends on file order: Bun runs the files in directory-read order, and in this checkout bandwidth.test.ts runs after stats.test.ts. origin/main's js/net fails the same way in that order, and this PR's head passes the full suite in a fresh worktree, where the order differs. No open PR fixes it yet.
• New tests for the review fixes, each confirmed to fail without its fix: a debt shed keeps the newest committed frame behind one being written and a debt shed of a finished group ends the copies that lost a frame.
• Array.shift cost (.tmp/ probe, not committed): 100k rotate-and-restore steps took about 10 ms in Bun at every size from 1k to 1M entries. In Node (V8) they took 22 ms at 1k, 95 ms at 10k, and 20.3 s at 100k. The account's eviction order now uses a head index instead.
• Earlier, on b5f1e11: just relay-fill passed, bun net/bench/track.ts passed its slope guard, mutation checks on cache.test.ts held, and the idle-expiry scaling probe showed a linear sweep (2k, 20k, and 100k groups in 4.7, 39.3, and 169.4 ms).

Follow-ups

• Drop set is now unblocked and bills its runs to this pool.
• moq-dev/moq#8165 moves js/net tests onto a mocked clock and touches track.test.ts, parked-expiry.test.ts, and others edited here. It does not conflict with main today. Whichever lands second merges the other.

Review feedback addressed

• Thread on group.ts, shedding the committed frame behind one still being written: fixed in e93ce5c. It now keeps the newest committed frame, as Rust's GroupState::shed does.
• Thread on track.ts, mirror overhead on a closed track: declined. entry.mirrors really does retain those mirrors, so the bill is accurate, and #unlink refunds it when the entry is evicted or expires.
• OpenAI review [P2], a finished group's readers on shed: fixed in e93ce5c. A finished group no longer tees into its copies, so a debt shed now ends the copies its track holds that had not taken the shed frames, with Evicted. Whole-group eviction already ended them this way.
• OpenAI review [P2], #order.shift() / unshift(): fixed in e93ce5c and 73ab959. The order reads from a head index, and each group holds a slot that removal empties, so the order never pins an evicted group's frames. I did not add a capacity-limited row to bench/track.ts, because that bench runs on Bun, where shift is already O(1). It could not catch this cost, which shows up only in V8.
• Pullfrog, "the byte budget has no way to be enabled": deliberate. The quest's Goal is "Rust and JS mirror each other under Rust's default budget, with no new setting". Rust's default has no capacity, and the plan adds a setter only once a JS consumer needs it.

Models

• Implementer: Claude Code, claude-opus-5-5 (iterate, S)

Review

• Reviewer: Codex, gpt-6.1-sol, medium effort, via quest review --base origin/main.
• Head b351e24 (new session 01a1b8ab-7935-77e2-9caf-adf6b69d9b3b, after origin/main moved): no findings.
• Session 01a1b877-c8b3-7721-9b12-ce4f75057b32:
  • Round 2, head 73ab959: no findings.
  • Round 1, head e93ce5c: one finding, fixed in 73ab959.
    • js/net/src/cache.ts:411 [high] "Release evicted entries instead of retaining their payloads": with the head index from e93ce5c, slots before #head still referenced evicted groups. Each group now holds a slot that remove empties. A WeakRef test for this was dropped in b351e24, because JSC's conservative GC made it depend on test order.
• Earlier session 01a1b763-77ca-7fc1-9b41-bc2fe1d52ec1, before this iteration: round 3 on b5f1e11, no findings. Round 1 had one finding (a quadratic idle-expiry splice), fixed in e46ff98.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[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 10 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: fb467b54-3484-4f79-9492-430c9c5cff43

Commits

Reviewing files that changed from the base of the PR and between 9920a3f and b351e24.

Files selected for processing: 19

• js/net/bench/track.ts
• js/net/src/broadcast.test.ts
• js/net/src/broadcast.ts
• js/net/src/cache.test.ts
• js/net/src/cache.ts
• js/net/src/group.test.ts
• js/net/src/group.ts
• js/net/src/ietf/fetch.test.ts
• js/net/src/internal.ts
• js/net/src/ledger.ts
• js/net/src/lite/copy_fill.test.ts
• js/net/src/lite/subscriber.test.ts
• js/net/src/parked-expiry.test.ts
• js/net/src/track.test.ts
• js/net/src/track.ts
• quest/m1/README.md
• quest/m1/drop-set.md
• quest/m1/js-cache-pool.md
• test/relay-fill/relay.ts

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[kixelated]

Automated review by review (OpenAI)

Reviewed head b5f1e11.

[P2] Keep finished readers accounted for when shedding frames (js/net/src/group.ts:703–713). Finished groups can now shed every frame, but finish removes their readers from #mirrors, and readers created after finish never enter that map. #shedMirrors then subtracts payload bytes from the pool without releasing the unread copies in those readers' queues or ending them with Evicted. With a small configured capacity, retain an unread mirror of a finished live-edge group and append another group: debt shedding can credit its payload while that mirror still retains and reads it. Propagate shedding through the retained track-level mirrors, or otherwise keep those bytes charged until actually released, and cover this finished-group case.

[P2] Avoid linear array shifts in the bounded eviction scan (js/net/src/cache.ts:337–355). #order.shift()/unshift() move the retained array tail on each candidate. Under byte pressure, even a four-candidate scan costs O(retained groups), and draining a large backlog can become quadratic. The Set change fixes idle removal but not this debt-payment path. Use a deque or head-index queue with amortized compaction and exercise a capacity-limited pool in the scaling benchmark.

Direction: the shared pool/account/charge split is sound, but byte-pressure reclamation should release what it credits and keep scan costs bounded. These findings concern explicitly capacity-limited internal pools; the default remains unbounded. Verification limits: GitHub-only static review of all 19 changed files, surrounding JS group/track code and the Rust cache implementation. No tests, benchmarks or memory measurements run.

[pullfrog]

ℹ️ Minor suggestions only — all three rough edges live in the new byte-budget paths, which are currently reachable only with a configured capacity.

Reviewed changes

• Byte pool port (js/net/src/cache.ts): Pool/Track/Charge mirror Rust's cache module plus the debt/eviction and expiry walks from model/track.rs, including the access-mean ordering, stale doubling, carried debt, and the bounded scan.
• Pool wiring: BroadcastState owns one pool and createProducer puts every track of a broadcast on it (broadcast.ts); a track with no broadcast keeps its own.
• Group billing: group.ts bills frame writes, compression repacking, and tail trims to the group's Charge, sheds fronts on demand, and refreshes access on frame reads; mirrors share the source charge.
• Track cache: #cache became an insertion-ordered Set, #prune and its timer left for the account's bounded scan plus the pool's half-window sweep, and byte eviction aborts Evicted while idle expiry keeps TooFarBehind.
• Delivery refresh: handing a group to a subscriber now restarts its idle window.
• Quest cleanup: the shipped quest is deleted from quest/m1/, and drop-set.md points at cache.ts.

Both P2 findings in the existing OpenAI review still stand at this head; I reproduced the finished-group shed one (a shed with frames=1, closed=null credited ~10 KiB while the reader still returned its 10,240 bytes).

ℹ️ The byte budget has no way to be enabled

Every production pool is created with an expiry and no capacity, and Pool.resize has no caller outside tests, so accrue, #payDebt, and #shed never run in production. The only live behavior change is the sweep-driven idle expiry; the byte half is covered only by cache.test.ts. The body records the missing setter as deliberate, but the deleted quest's Goal and the drop-set follow-up both assume a pool that can actually be bounded, and nothing in the tree owns that wiring yet.

Technical details

# The byte budget has no way to be enabled

## Affected sites
- `js/net/src/broadcast.ts:60` — every broadcast pool is created with `{ expiry }` only
- `js/net/src/track.ts:1071` — a standalone track's pool likewise
- `js/net/src/cache.ts:107` — `Pool.resize` is package-internal and called only from tests

## Required outcome
- Confirm which follow-up lets a consumer set a capacity, and that it is tracked, before this becomes the base the drop-set billing counts on.
- If the eviction half is meant to land with that wiring, consider splitting it out: today the debt path's cost has no guard either, since `js/net/bench/track.ts` uses the default unbounded pool and never reaches `#payDebt`.

## Open questions for the human
- Is the capacity source an origin/broadcast config in a later quest, or should `resize` be reachable now?

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

[kixelated]

Responses to the review-body findings at b5f1e11, now on b351e24:

• OpenAI [P2], a finished group's readers on shed: fixed. A debt shed now ends the track-held copies of a finished group that had not taken the shed frames, with Evicted.
• OpenAI [P2], #order.shift() / unshift(): fixed. The order uses a head index plus slots that removal empties. I added no bench row: bench/track.ts runs on Bun, where shift is O(1) (measured flat from 1k to 1M entries). V8 is what goes quadratic (20 s for 100k steps at 100k entries).
• Pullfrog, no way to enable the budget: deliberate, per the quest's Goal of "no new setting" under Rust's default, which has no capacity.

Details are in the body's ## Review feedback addressed.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed b351e24 incrementally from b5f1e11.

Both prior P2 findings are addressed. Finished-group shedding now visits the track-held mirrors, clears unread queues and reports Evicted to readers that lost frames, while a drained reader keeps its clean end. The regression covers both readers. The eviction order now advances a head index with amortized compaction instead of shifting the array; removing a group empties its shared slot, including references before the head, so the queue does not retain evicted payloads.

No new actionable findings. Keeping the newest committed frame behind an unfinished frame also aligns shedding with the intended readable-tail rule. The pool/account/charge direction remains sound. Removing a GC-order-dependent assertion is reasonable; the retention correction is visible in the ownership path, though I did not verify collection experimentally.

Verification limits: GitHub-only static review of the substantive follow-up commits, complete cache implementation and surrounding group/track mirror and eviction paths. No tests, memory probes, browser/V8 benchmarks or reported failing-suite reproduction run. Default pools remain unbounded; these byte-pressure fixes concern explicitly configured internal capacities. Rechecked open/non-draft state, full head and published reviews immediately before this COMMENT.

[pullfrog]

✅ No new issues found.

Reviewed changes

This run covered the three commits since the prior pullfrog review (b5f1e11..b351e24), which address the two OpenAI P2s and both prior pullfrog threads.

• Shed keeps the newest committed frame (group.ts): #shed now computes keep = closed === null ? 0 : writing ? 2 : 1, so an open group never sheds the frame a reader can still take while a newer one is being filled, matching Rust's last = committed - 1.
• Shedding a finished group ends its lagging copies (group.ts, track.ts, internal.ts): #shed takes the track's entry.mirrors as readers and, for a finished group, fails each copy whose #state.start is behind the shed front with Evicted. A copy that already took every shed frame keeps reading to its clean end, which the new test asserts.
• Eviction order is a head-indexed ring (cache.ts): #live maps each entry to a Slot, remove empties the slot, and #payDebt advances a #head index instead of shift()/unshift(). I traced the ring invariants: every examined live slot is re-pushed or restored through #head--, an emptied account resets the order and the loop breaks, and #compact only slices off pre-head dead duplicates, so no live entry can be dropped and no evicted group is pinned.
• Regression tests: a debt shed keeps the newest committed frame behind one being written and a debt shed of a finished group ends the copies that lost a frame, both exercising the new paths; the order-dependent WeakRef retention test was removed with its cause documented.
• Hook signatures: shedGroup gained readers and evictGroup takes the loss Error, with track.ts supplying both.

bun test in js/net passes here (3141 pass, 0 fail; the body's order-dependent bandwidth.test.ts failure did not reproduce in this checkout) and tsc --noEmit is clean. The finished-group shed keeps the pool accounting symmetric: the source Charge is credited on #shift, and mirrors bill only MIRROR_OVERHEAD, so clearing a behind mirror's queue does not double credit.

The prior thread declining a refund for mirror overhead on a closed track stands: entry.mirrors genuinely retains those mirrors and #unlink refunds them when the entry leaves. The missing capacity setter remains the body's recorded decision and is owned by the drop-set follow-up, so I am not re-raising it.

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