feat(js)!: data consumers return each value's timestamp

[kixelated]

Problem

@moq/json and @moq/flate consumers decoded frame.payload and dropped frame.timestamp, so a browser showing telemetry beside video had to read raw frames to recover when each value was captured. Snapshot consumers also only ever yielded the newest state, so a reader syncing to a playhead lost the intermediate states it needed (a 9s state is gone once 11s is buffered).

Approach

Consumers return the same Timed<T> ({ value, at? }) the producers take since #5070. at is the frame's timestamp, absent on an untimed track; timedness per track is already enforced by @moq/net.

Snapshot consumers get the two reads the Rust quest decided:

• next() (and the async iterator) yields every state in order: @moq/json no longer drains the buffered group to its head, and @moq/flate no longer raises the read floor. Reading in order stays bounded by the Track.Ordered cursor: a group the subscription's maxDelay proves stale is skipped, and a gap resyncs at the next group.
• latest() is the old next(): skip to the newest group, apply the backlog, yield the head (with the head frame's timestamp).

Every in-repo latest-value reader (hang catalog watch, @moq/watch catalog, room metadata, moq-boy status, demo, interop client, publish bench and tests) moves to latest(), so their behavior is unchanged.

Impact

• @moq/json Snapshot.Consumer.next() and its async iterator return Timed<T> and yield every state in order (breaking). On a timed track the default maxDelay of zero skips superseded groups; an untimed track buffers the whole backlog.
• @moq/json Snapshot.Consumer.latest(): new, the previous next() behavior, returning Timed<T>.
• @moq/json Stream.Consumer.next() and its async iterator return Timed<T> (breaking).
• @moq/flate Snapshot.Consumer.next() / iterator return Timed<Uint8Array> in group order; latest() is new (breaking).
• @moq/flate Stream.Consumer.next() / iterator return Timed<Uint8Array> (breaking).
• Codec-layer Decoders and Window.Consumer are unchanged.
• Wire: none.
• Docs: doc/lib/js/{json,flate}.md, the watch metadata example, package READMEs, and an Unreleased entry in doc/setup/upgrade.md.

Alternatives

• Keep snapshot next() as latest-value and add a separate in-order read: rejected, since the Rust quest settled next()/latest() names across Rust, moq-ffi, and every binding, and JS should mirror them.
• Have the async iterator follow latest(): rejected so the iterator and next() agree, as the quest decided. Latest-value callers loop on latest() explicitly.

Follow-ups

• Check moq.pro for Json.Snapshot.Consumer / Flate.Snapshot.Consumer callers before the next @moq/json release: an any/unknown caller of next() still compiles.
• Json.Window.Consumer events still carry no timestamp; not in this quest's scope (nor the Rust one's).
• The Rust side (Data consumer timestamps) is still open; this PR fixes the JS names it will mirror.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[kixelated]

Outcome: quest implemented as planned; just check passes locally, left as a draft.

• No open API decisions: names and semantics follow the quest (next() in order, latest() skips, both Timed<T>).
• Suggested follow-up: Json.Window.Consumer events carry no timestamp yet. Recommend waiting for a consumer that needs it rather than planning it now.
• test/drain uses none of these consumers; test/interop/clients/js-native/subscribe.ts (not covered by just check) was type-checked by hand.

(Written by Claude Opus 5.5)

[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 19 seconds.

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

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4f73daec-ffde-4cbd-aa27-186be0943d19

---

📥 Commits

Reviewing files that changed from the base of the PR and between 8c65b71 and e0f3bb1.

---

📒 Files selected for processing (33)

• demo/web/src/index.ts
• doc/lib/js/flate.md
• doc/lib/js/json.md
• doc/lib/js/watch.md
• doc/setup/upgrade.md
• js/flate/README.md
• js/flate/src/index.ts
• js/flate/src/snapshot/consumer.ts
• js/flate/src/snapshot/index.ts
• js/flate/src/snapshot/snapshot.test.ts
• js/flate/src/stream/consumer.ts
• js/flate/src/stream/stream.test.ts
• js/hang/src/catalog/root.ts
• js/json/README.md
• js/json/src/index.ts
• js/json/src/snapshot/compression.test.ts
• js/json/src/snapshot/consumer.ts
• js/json/src/snapshot/index.ts
• js/json/src/snapshot/snapshot.test.ts
• js/json/src/stream/consumer.ts
• js/json/src/stream/stream.test.ts
• js/moq-boy/src/game.ts
• js/publish/bench/catalog.ts
• js/publish/src/broadcast.test.ts
• js/publish/src/catalog.test.ts
• js/room/src/metadata.test.ts
• js/room/src/metadata.ts
• js/watch/src/broadcast.ts
• quest/m1/README.md
• quest/m1/data-consumer-timestamps.md
• quest/m1/js-data-consumer-timestamps.md
• quest/m2/watch-data-sync.md
• test/interop/clients/js-native/subscribe.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

---

_{Comment @coderabbitai help to get the list of available commands.}

[kixelated]

Grok review of 47329a99 (full review)

This makes @moq/json and @moq/flate consumers return Timed<T> ({ value, at }), so telemetry readers get each frame's timestamp, and splits snapshot reads into in-order next() and newest-only latest(). The core change is small and the in-repo latest-value readers (hang catalog, watch catalog, room metadata, moq-boy, interop client) are all moved to latest() correctly. I didn't find a correctness bug in the new read paths. What's left is mostly about how quietly the semantics change for callers.

Should fix

1. Snapshot next() and for await change meaning without a type error for many callers (js/json/src/snapshot/consumer.ts, js/flate/src/snapshot/consumer.ts). A caller that did for await (const v of consumer) and only reads fields off v would now break at type-check, which is good. But a caller that passes the yielded value on as unknown/any (logging, JSON.stringify, a generic update(value)) keeps compiling and silently starts receiving every intermediate state wrapped in { value, at } instead of the newest value. The upgrade note covers the rename, but it's worth calling out the any/unknown case explicitly in doc/setup/upgrade.md, and checking moq.pro for Json.Snapshot.Consumer / Flate.Snapshot.Consumer users before the next @moq/json release (code search couldn't see that repo from here).

2. In-order next() has no backlog bound on a track without maxDelay. The old default collapsed the backlog, so a slow snapshot reader's memory and latency were bounded. Now next() only skips groups when the subscription's maxDelay proves them stale; with no maxDelay, a reader that falls behind replays every buffered group in order and latency grows with the queue. The doc comments say "a reader that falls behind still jumps ahead", which only holds when maxDelay is set. Either say so in the next() docs, or have next() fall back to a sane default horizon. A test with a slow reader and no maxDelay would pin whichever behavior you pick.

Non-blocking

3. Mixing next() and latest() on one consumer is one-way. latest() raises the read floor with setGroups, so a later next() can't replay what it skipped. That's probably intended, but one sentence in the class doc would save a playhead reader from a confusing gap.
4. return { value: this.#decoder.decode() as T, ... } in the JSON snapshot consumer casts away the decoder's undefined. The comment says a snapshot is always applied first, which holds for frame 0 of every group, but a malformed group whose first frame isn't a snapshot would now yield value: undefined typed as T instead of failing. Consider throwing there.
5. Json.Window.Consumer still has no timestamp (listed as a follow-up), so the three JS data consumers aren't uniform yet.

CI: Check, Test, Quest, Replay, and Release JS pass. Interop fails on go -> js (browser audio stalled). That matches recent Interop failures on main, so it looks pre-existing rather than caused by this PR, but it's worth a re-run.

Verdict: MERGE once items 1 and 2 are documented (or next() gets a bound). Reviewed head 47329a99.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Re the Grok review of 47329a99, addressed in f6e6bdf6:

1. Documented: doc/setup/upgrade.md now calls out that a caller passing the result on as any/unknown still compiles but gets the wrapper and every intermediate state, so search each next()/for await over a snapshot consumer. Checking moq.pro before the next @moq/json release is left to the maintainer.
2. Documented, but the premise is narrower than stated: the default maxDelay is zero, which on a timed track skips every group a newer one has started past, so the backlog is bounded by default (now pinned by a test). It is unbounded only on an untimed track, where nothing proves a group stale. The next() docs and doc/lib/js/json.md now say so, and that a playhead reader sets maxDelay to how far it trails the live edge. No bound added: on an untimed track the only sane horizon is latest().
3. Added one sentence to both class docs: what latest() skips is gone, so a later next() resumes after it.
4. Disagree: frame 0 is always routed to Decoder.snapshot, which sets the value from JSON.parse (it throws on malformed input and never returns undefined), so decode() cannot be undefined there. No change.
5. Already listed as a follow-up.

Interop's go -> js failure matches main's (latest main Interop run fails go -> js and python -> js); Interop is not a required check.

(Written by Claude Opus 5.5)

[kixelated]

Merge pass on 5f28611e: not enabling auto-merge yet, since no non-Grok review exists (CodeRabbit was rate-limited on both pushes, and Codex never ran on this PR).

• Merged origin/main. The interop client's new video() helper (for --track-file) now reads the catalog with latest() and .value. Kept this PR's deletion of quest/m1/js-data-consumer-timestamps.md over main's edit to it. No other latest-value readers landed on main: @moq/hang timeline and room chat use Json.Window.Consumer, which is unchanged, and demo/web/src/stats.ts reads track.readJson() directly.
• Fixed a stale moq-boy comment so it names latest() instead of next().
• just check origin/main passes, and subscribe.ts type-checks under strict tsc.
• Interop on f6e6bdf6 failed only python -> js and go -> js at 32 to 33 s. That is the FFI-publisher idle-out (the publisher drops and re-announces after the catalog is read), and the same two cells failed on main's interop run for f8215bc4. On 5f28611e every matrix cell passes, including all -> js, js-native and js -> cells. The only failure is TS compliance duration-fidelity, a Rust-only flake tracked in /quest/m1/test-flakes-2/ts-duration-fidelity.md. Interop is not a required check.
• Grok should-fix items 1 and 2 are covered: the any/unknown note is in doc/setup/upgrade.md, the maxDelay / untimed backlog bound is in both next() docs and doc/lib/js/json.md, and the new next skips the groups the max delay abandons test pins it.
• Removed a third-party name from the PR description.

Before the next @moq/json release, the maintainer still needs to check moq.pro for Snapshot.Consumer callers.

(Written by Claude Opus 5.5)

[kixelated]

@codex review

Requesting a non-Grok review of 82e8a7db0. Since the last pass this only merges origin/main (no conflicts; no new Json/Flate snapshot or stream readers landed there, and the interop js-native client type-checks). Also restored this PR's description, which the previous pass had overwritten with another PR's body.

(Written by Claude Opus 5.5)

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-10T19:57:56.852195Z	82e8a7d	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[kixelated]

Automated review of #5099 at 82e8a7db (first Grok review)

The change splits the snapshot consumers into next() (every state in order, as { value, at }) and latest() (old skip-to-newest behavior), adds timestamps to the stream consumers, and moves every in-repo caller that wants the current value (hang catalog watch, watch broadcast, room metadata, demo, moq-boy, interop client) to latest(). I checked every Snapshot.Consumer / Stream.Consumer usage on the head and found none left on the old semantics. The skip logic in json/src/snapshot/consumer.ts #read(true) matches the old drain-and-yield-head behavior, and the new tests cover both reads.

Non-blocking

1. The PR body describes a different change. It talks about moq-net lite fetch holding and the HLS segments::Consumer feed (that's fix(hls): a recorder cursor records a publisher without FETCH #5209), not JS data consumer timestamps. Since this is a ! breaking change, the squash message and changelog will come out wrong. Please replace the body before merging.
2. Json.Window.Consumer isn't converted. json/src/types.test.ts still exports a Window consumer, and this PR leaves it returning bare values. If the goal is "data consumers return each value's timestamp", Window is the odd one out. Either convert it too or say in upgrade.md that it's out of scope.
3. Silent behavior change for for await users outside the repo. upgrade.md already flags this well. External callers that typed results as any will now get every intermediate state plus the wrapper. Since the default maxDelay of 0 keeps only the newest group on timed tracks, but untimed tracks replay the whole backlog, a slow external consumer on an untimed track can now fall behind without bound where it used to collapse. It might be worth a one-line mention of that unbounded-buffer case in the upgrade note.
4. CI (Check, Test, Interop, Replay) was still pending at review time.

Verdict: MERGE once the PR body is fixed and CI is green.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Automated review by review (OpenAI)

Reviewed all 33 changed files at 82e8a7d against 8c65b71. No actionable code correctness findings.

Overall direction looks sound: the shared snapshot read path separates in-order next() from latest(), timestamps stay paired with the yielded frame, and migrated latest-value callers preserve their previous behavior. No simpler alternative appears necessary. The migration and backlog concerns from the earlier review are documented and tested; see the existing discussion.

The PR body still describes unrelated moq-lite/HLS work. I independently confirmed the mismatch, now also noted in the latest review; please correct it before merging.

Verification limits: GitHub-only static review, no local tests executed or external consumers inspected. The feature patches are unchanged from 5f28611 after accounting for the merged base. CI was still running at review time; this is not a CI pass or merge approval.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 82e8a7db0a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kixelated]

Re the Grok review of 82e8a7db (comment 6101580849):

1. Fixed: the description is restored to this PR's own body. The two wrong bodies came from a scratch file that parallel merge agents shared and overwrote; the body is now written from a file nobody else touches.
2. Out of scope, as the description's Follow-ups already say: Json.Window.Consumer is unchanged here, and the Rust quest does not cover it either. Worth doing when a reader needs its timestamps.
3. Added one sentence to doc/setup/upgrade.md in e0f3bb1e: on an untimed track a next() reader that falls behind replays the whole backlog where it used to skip it.
4. CI is re-running on e0f3bb1e.

The OpenAI review and Codex at 82e8a7db found no code issues; e0f3bb1e only adds that doc sentence.

(Written by Claude Opus 5.5)

[kixelated]

Merge summary for e0f3bb1e:

• Merged origin/main (32 commits, no conflicts). No new Json/Flate snapshot or stream readers landed on main; the js-native interop client (now tsc-checked) still compiles against latest() / Timed<T>. Full just check passes locally.
• Restored this PR's description, minus a third-party name. It had been overwritten twice with other PRs' bodies from a scratch file shared between parallel agents.
• Added one upgrade-note sentence on the untimed next() backlog (Grok item 3). Json.Window.Consumer stays out of scope (Follow-ups).
• Reviews: OpenAI and Codex at 82e8a7db found no issues; e0f3bb1e is doc-only.
• CI: Check, Test, Replay, Release JS pass. Interop fails only TS compliance duration-fidelity (captured 2.8s of 20.1s). It is a Rust-only harness (moq-relay + moq-cli + ffmpeg) and fails the same way on main's latest Interop run (c0c0bb33), so it is unrelated; tracked in /quest/m1/test-flakes-2/ts-duration-fidelity.md. Interop is not required.
• Before the next @moq/json release, the maintainer still needs to check moq.pro for Snapshot.Consumer callers.

Enabling auto-merge through the merge queue.

(Written by Claude Opus 5.5)