Skip to content

Use Catalog.Consumer in native JS smoke client#10

Merged
kixelated merged 2 commits into
mainfrom
claude/epic-grothendieck-ffc90d
Jun 16, 2026
Merged

Use Catalog.Consumer in native JS smoke client#10
kixelated merged 2 commits into
mainfrom
claude/epic-grothendieck-ffc90d

Conversation

@kixelated

Copy link
Copy Markdown
Contributor

What

Updates the native JS smoke subscriber (clients/js-native/subscribe.ts) to resolve the .hang catalog via Catalog.Consumer instead of the removed Catalog.fetch helper.

const catalog = new Catalog.Consumer(bc.subscribe("catalog.json", Catalog.PRIORITY.catalog));
let videoTrack: string | undefined;
while (!videoTrack) {
    const root = await catalog.next();
    if (!root) throw new Error("catalog ended without a video track");
    const renditions = root.video?.renditions;
    if (renditions) videoTrack = Object.keys(renditions)[0];
}

Why

The scheduled Smoke run went red on every channel with Catalog.fetch is not a function. The harness pins @moq/hang to latest, which floated to 0.2.10 — that release made @moq/hang/catalog schema-only and dropped the fetch/decode/encode helpers. The native subscriber was the sole consumer of Catalog.fetch, so it was the only client that broke (the others use native bindings with their own catalog APIs). This is exactly the upstream-API drift the freshness test exists to catch.

@moq/hang 0.2.11 (moq#1767) re-adds Catalog.Consumer, a thin wrapper over @moq/json's snapshot+delta Consumer pre-wired with RootSchema. Switching to it also picks up catalog deltas, which a one-shot frame read would miss.

No new dependency is needed: 0.2.11 declares @moq/json as a dependency, so it's pulled transitively.

Verification

Ran the real smoke harness locally against the published @moq/hang@0.2.11 (no patching/shims) with a moq-relay + rust ffmpeg publisher:

rust -> js-native-node   PASS
rust -> js-native-bun    PASS
rust -> rust             PASS
smoke: all checks passed

Notes for reviewers

  • Requires @moq/hang >= 0.2.11 on npm (now live). The next scheduled smoke run will pick it up.
  • The first commit in this branch was an interim manual frame-parse fix that worked against 0.2.10; the second commit replaces it with the supported Catalog.Consumer API now that 0.2.11 is out.

🤖 Generated with Claude Code

kixelated and others added 2 commits June 16, 2026 11:29
@moq/hang 0.2.10 dropped the catalog fetch/decode helpers; the catalog
module is now schema-only. The native subscriber was the sole user of
Catalog.fetch and broke with "Catalog.fetch is not a function" once the
latest-pinned package floated to 0.2.10.

Read the catalog frame directly and validate it against RootSchema via
zod/mini (which 0.2.10's schema now targets), replacing the removed
helper. Verified locally against moq-relay: rust -> js-native-{node,bun}
both pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@moq/hang re-adds Catalog.Consumer (moq#1767), a thin wrapper over
@moq/json's snapshot+delta Consumer pre-wired with RootSchema. Switch the
native subscriber from the manual frame-parse stopgap to the supported
API: construct a Catalog.Consumer over the catalog.json track and pull
catalog updates via next() until one carries a video track. This also
picks up catalog deltas, which the one-shot frame read ignored.

Verified the runtime path against a local moq-relay + rust publisher
using @moq/json's Consumer + RootSchema directly (what Catalog.Consumer
wraps): catalog resolved, video track found, frames received.

NOTE: requires the @moq/hang release that ships Catalog.Consumer to be
published to npm; until then the latest-pinned install (0.2.10) has no
Consumer and the smoke run stays red.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 72ca3b15-e4e8-45cb-9034-f1a58c1abb2c

📥 Commits

Reviewing files that changed from the base of the PR and between 6fecbeb and 88faa42.

📒 Files selected for processing (1)
  • clients/js-native/subscribe.ts

Walkthrough

In clients/js-native/subscribe.ts, the catalog discovery logic is refactored. The previous approach called Catalog.fetch(...) in a loop; the new approach creates a Catalog.Consumer from a bc.subscribe("catalog.json", ...) subscription and advances it with catalog.next() on each iteration until a root containing a video rendition track is found. Surrounding comments were updated to match the consumer-driven approach.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically describes the main change: using Catalog.Consumer API in the native JS smoke client to replace the deprecated Catalog.fetch.
Description check ✅ Passed The PR description is comprehensive and directly related to the changeset, explaining the what, why, verification, and notes for the Catalog.Consumer API migration.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/epic-grothendieck-ffc90d

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@kixelated kixelated merged commit ff9bd39 into main Jun 16, 2026
6 of 8 checks passed
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