[kixelated]

Finishes quest/m1/js-cache-window.md, a follow-up of moq-dev/moq#7219.

Problem

The JS track cache evicts a group after 30 s with no writes (CACHE_WINDOW in js/net/src/track.ts, matching Rust cache::DEFAULT_EXPIRY). Track.Info.maxAge doesn't change that. It only decides which cached groups a reader still receives. Several tests, comments, and docs still called maxAge "retention" or relied on it to keep or drop cached groups.

Approach

Search. I grepped the JS tests, doc/, and @moq/net's docs for maxAge used as retention. As a mutation check, I disabled time-based eviction (#prune keeps every entry) and ran every JS test file that mentions maxAge, eviction, or retention. Every test that checks eviction failed under the mutation, including all the idle-retention tests in track.test.ts and the parked-expiry and broadcast eviction tests. The tests that still passed check staleness or survival, so they also pass when nothing is evicted. None of them relied on maxAge for eviction. The ones below only named or described maxAge as retention.

Fixes

• js/watch/src/retention.test.ts: removed maxAge: Milli(1). Its comment claimed it kept "the track's own replay cache" flat, but the cache keeps idle groups for its fixed window whatever maxAge says. The test passes unchanged without it. A comment now says why it sets no maxAge.
• js/net/src/track.test.ts:
  • Renamed "a group newly outside the retention window" to "newly past the publisher's maxAge".
  • Renamed "clamped to the publisher's window" to "clamped to the publisher's maxAge".
  • Renamed "a stalled track keeps only its newest group" to "serves only". Its comment now says it checks staleness, not eviction: two seconds is well inside the cache window.
  • Reworded the comment on the refused-subscriber test.
• js/net/src/broadcast.test.ts: renamed "tightens publisher retention" to "tightens the publisher's maxAge".
• doc/lib/js/net.md: added a bullet on the 30 s idle window, with the live-edge exemption. Neither maxAge nor delay.max changes the window. The delay bullet now calls maxAge the publisher's staleness bound, not "retention".
• doc/lib/js/publish.md:
  • The encoding section no longer says video "retains up to maxAge". The cache keeps an idle group for 30 s, and maxAge bounds how far back a reader may reach. Raw PCM audio really is bounded by maxAge, and that sentence stays.
  • The meta.json example dropped its day-long maxAge, which claimed to make a late viewer replay the last value. A live track's newest group is never stale or evicted, so it already does.
• js/net/MIGRATE.md, js/net/src/lite/publisher.ts: call maxAge a staleness bound, not retention. The unlinked heading "Publisher retention is optional" is now "Track.Info.maxAge is optional".
• Deleted the quest and its link in quest/m1/README.md.

Choices made without asking

• The Plan said to check that each fixed test fails with eviction disabled. None of the fixed tests checks eviction: they check staleness or a leak. So that check doesn't apply. The mutation run above checked the eviction tests themselves instead.
• I edited doc/lib/js/publish.md, not just the @moq/net pages, because it is under doc/ and made the same claim. That includes removing maxAge from the meta.json example code.
• I left alone uses of "retention" for maxAge in @moq/hang and @moq/publish source docs and tests, such as Container.trackInfo and Broadcast.maxAge. They describe how far back a fetch may reach, which is accurate. See the follow-ups.

Impact

Tests, docs, and comments only. Public API: none. Wire: none.

Validation

• Mutation (time-based eviction disabled in #prune): every eviction test in the touched files fails; the renamed tests pass, as staleness tests should.
• Unmutated:
  • bun test src/track.test.ts src/broadcast.test.ts src/lite/publisher.test.ts in js/net: 401 pass.
  • bun test src/retention.test.ts in js/watch: passes.
  • The full net, hang, publish, and watch suites passed on the base before the edits.
• biome check on changed files, tsc --noEmit for js/net and js/watch, quest check --base origin/main: clean.

Review

Codex gpt-6.1-sol, medium effort (quest review, round 1), reviewed head cdef3a5db304525313fc3f48fe2083d42d595b21 against origin/main: no findings.
Since then, head 5137a69132 only adds a conflict-free merge of main (review-exempt). The touched test files and quest check pass on it.

Suggested follow-ups

• js/publish/src/broadcast.ts creates the catalog track with maxAge: Milli(Number.MAX_SAFE_INTEGER), commented as keeping the sole snapshot replayable "after the ordinary media retention window". A live track's newest group is never stale or evicted, in JS or Rust, so this probably does nothing for that goal. It does advertise an effectively unbounded maxAge in TRACK_INFO to relays. Decide whether to drop it or document what it is for.
• @moq/hang (Container.trackInfo, MAX_AGE_MS) and @moq/publish (Broadcast.maxAge) call maxAge a "retention" budget, for example "how long relays keep a non-latest group … fetchable". It is the bound past which a cached group is no longer served, not how long anything keeps it. Align the wording with @moq/net's "staleness bound", or settle on one term repo-wide, Rust included.
• Several track.test.ts tests check that state survives eviction, for example "an evicted successor with unread frames still starts the wall clock" and "a mid-group end survives eviction". They pass with eviction disabled because they never assert that the eviction happened. Adding that assertion would make them catch a broken eviction path.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 5137a69

The revised distinction between delivery staleness and idle cache retention matches the implementation, and simplifying the leak test avoids implying that maxAge controls eviction. No actionable bugs found in this static review. I did not run the tests or mutation check.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The changes clarify that Track.Info.maxAge limits reader reach, waiting, and fetches, while the group cache uses a separate 30-second idle eviction window. The documentation also distinguishes video and audio retention and updates a custom-track example. Comments and test names now use the same terminology. The test logic and runtime behavior are unchanged.

Merge Risk: Minimal · up to 5137a

The changes distinguish reader staleness limits from cache eviction without changing public APIs or runtime behavior. No concrete issue requiring a merge hold is identified.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly identifies the main change: clarifying that the fixed cache window, rather than maxAge, keeps groups cached.
Description check		The description directly explains the cache-window and maxAge distinction, lists the affected tests and documentation, and reports validation results.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 4 files. (3 skipped: 3 unsupported.)

Finishing Touches

Simplify code

• Commit to this branch
• Create a new PR

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

Warning

Some tools did not complete. Review the errors below.

Biome (2.5.15)

js/net/src/broadcast.test.ts

Biome could not lint this file: configuration resulted in errors. Check the repository's Biome configuration and plugins.

js/net/src/lite/publisher.ts

Biome could not lint this file: configuration resulted in errors. Check the repository's Biome configuration and plugins.

js/net/src/track.test.ts

Biome could not lint this file: configuration resulted in errors. Check the repository's Biome configuration and plugins.

• 1 others

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

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

Recent review info

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: dbf795a2-3207-45b0-9e2f-cec3d1e67f25

Commits

Reviewing files that changed from the base of the PR and between 686644d and 5137a69.

Files selected for processing: 9

• doc/lib/js/net.md
• doc/lib/js/publish.md
• js/net/MIGRATE.md
• js/net/src/broadcast.test.ts
• js/net/src/lite/publisher.ts
• js/net/src/track.test.ts
• js/watch/src/retention.test.ts
• quest/m1/README.md
• quest/m1/js-cache-window.md

Files with no reviewable changes: 1

• quest/m1/js-cache-window.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

[pullfrog]

✅ No new issues found.

Reviewed changes

• JS docs align with the fixed cache window. doc/lib/js/net.md gains a bullet on the 30 s idle window and its live-edge exemption, and now calls Track.Info.maxAge the publisher's staleness bound that caps delay.max and fetches rather than "retention".
• publish.md encoding section corrected. The video cache keeps an idle group for 30 s regardless of maxAge; raw PCM history is the part actually bounded by maxAge (30 s fallback). The meta.json example drops its day-long maxAge and the now-unused Milli import.
• MIGRATE.md and lite/publisher.ts reworded. "Publisher retention is optional" heading becomes "Track.Info.maxAge is optional", and "retention bound" becomes "staleness bound".
• Tests renamed / one option dropped. track.test.ts and broadcast.test.ts names now say maxAge where they mean staleness; js/watch/src/retention.test.ts drops maxAge: Milli(1) since the fixed window keeps the groups regardless.
• Quest retired. quest/m1/js-cache-window.md and its README.md line are deleted.

I verified the load-bearing claims against source: CACHE_WINDOW/CACHE_EXPIRY is 30 s (js/net/src/ledger.ts:22) matching Rust cache::DEFAULT_EXPIRY (rs/moq-net/src/model/cache.rs:89), #prune evicts on write-idle activity independent of maxAge, maxAge only clamps subscriber/fetch budgets, and the "unless it failed" edge exemption matches #edge(). The renamed heading has no TOC anchor, no references to js-cache-window remain, and the removed Milli import is unused in that snippet. I could not run bun test here because dependency installation fails in this environment (js/libav needs Nix), so the test-pass claims rest on the author's runs.

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