Skip to content

feat: publish & subscribe arbitrary custom tracks within a broadcast#1748

Merged
kixelated merged 7 commits into
mainfrom
claude/moq-arbitrary-track-insert-846893
Jun 16, 2026
Merged

feat: publish & subscribe arbitrary custom tracks within a broadcast#1748
kixelated merged 7 commits into
mainfrom
claude/moq-arbitrary-track-insert-846893

Conversation

@kixelated

@kixelated kixelated commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Lets applications carry arbitrary tracks (e.g. a meta.json metadata track, or an SCTE-35 event track) within an existing broadcast alongside audio/video, and advertise them with their own catalog section. MoQ stays generic: no per-application support and no catalog-format change. Reachable from the <moq-publish> / <moq-watch> web components via their .broadcast property.

The catalog root is already a loose object, so an app adds its own typed section (the existing scte35 pattern). What was missing was a generic track hook on the publish/watch components, which always handled tracks dynamically and rejected anything that wasn't catalog/audio/video. This adds those hooks and lets the app encode payloads with @moq/json directly.

Fully additive and backwards-compatible (wire format unchanged). Counterpart to the breaking redesign on dev.

What's new

@moq/jsonProducer<T> now publishes a JSON value to one track or many:

  • new Producer(track, config) — unchanged (writes directly to that track; byte-for-byte identical, so the wire-format vectors still pass).
  • new Producer(config) — track-less: retains the value, fans it out to subscribers attached with the new serve(track, effect), seeding late joiners. This is the fan-out producer the catalog uses, now reusable by applications.

Publish (js/publish)

  • Broadcast.publishTrack(name, serve) — registers a per-subscriber serve handler for a custom track; rejects the built-in track names (catalog/audio/video). For JSON, serve each track from a track-less Json.Producer.
  • CatalogProducer is now a type alias for Producer<Catalog.Root> (the broadcast seeds it empty).

Watch (js/watch)

  • Broadcast.subscribeTrack(name, priority, consume) — runs consume(track, effect) for the active broadcast, following it across reconnects. For JSON, wrap the track in a Json.Consumer.

Both packages re-export @moq/json as Json, so an app has one import for the JSON track tooling. The app reads its own catalog section straight back from broadcast.catalog (unknown sections pass through the loose RootSchema).

import { Json } from "@moq/publish";

// publish: a custom scte35 section + track, no hang-specific support
const scte35 = new Json.Producer<{ splices: number[] }>({ initial: { splices: [] } });
el.broadcast.publishTrack("scte35.json", (track, effect) => scte35.serve(track, effect));
el.broadcast.catalog.mutate((c) => { c.scte35 = { track: "scte35.json" }; });
scte35.update({ splices: [42] });

// watch
const section = el.broadcast.catalog.peek()?.scte35;
el.broadcast.subscribeTrack("scte35.json", 100, (track, effect) => {
  const consumer = new Json.Consumer<{ splices: number[] }>(track);
  effect.spawn(async () => {
    for (;;) {
      const next = await Promise.race([effect.cancel, consumer.next()]);
      if (next === undefined) break;
      // use next
    }
  });
});

Public API changes (additive)

  • js/json: Producer gains a track-less constructor overload, serve(track, effect), and a value getter. The existing new Producer(track, config) path is unchanged. Per-subscriber failures during fan-out are now isolated.
  • js/publish: Broadcast.publishTrack; ServeTrack type; Json namespace re-export. CatalogProducer is now a type alias for Producer<Catalog.Root> (its mutate/serve/update surface is unchanged; only direct new CatalogProducer() construction is affected, which the broadcast owns).
  • js/watch: Broadcast.subscribeTrack; ConsumeTrack type; Json namespace re-export.
  • No catalog schema change (js/hang / rs/hang untouched).

Cross-package sync

  • No rs/hang / js/hang catalog change, so no Rust mirror needed.
  • Docs: doc/concept/layer/hang.md ("Custom tracks" under Extensions), doc/lib/js/@moq/publish.md, doc/lib/js/@moq/watch.md.
  • demo/web: not updated; opt-in feature, existing media flows unchanged.

Test plan

  • @moq/json Producer round-trips and wire-format vectors pass unchanged; track-less fan-out seeds late/reconnecting subscribers, composes via mutate, and serve() rejects a track-bound producer.
  • biome clean; tsc typecheck passes for json/publish/watch; existing hang/moq-mux suites still pass.

(Written by Claude)

…roadcast

Add a backwards-compatible way to carry arbitrary application tracks (e.g. a
meta.json metadata track) within an existing broadcast, alongside audio/video,
without spinning up a separate broadcast.

Catalog: new optional `data` section, a flat map of track name to a small
descriptor (mime, description). Omitted from the wire when empty so existing
catalogs are byte-identical. Mirrored in js/hang and rs/hang.

Publish: `Broadcast.publishJson(name, opts)` serves a fan-out JSON track
(seeding late joiners with the latest value) and adds the catalog `data` entry;
`Broadcast.publishTrack(name, serve)` is the low-level escape hatch for arbitrary
payloads. The unknown-track branch now consults this registry instead of always
rejecting. Extracted a reusable `JsonProducer<T>` that `CatalogProducer` reuses.

Watch: `Broadcast.subscribeJson(name, opts)` returns a `JsonConsumer` whose
latest value is a signal, following the active broadcast across reconnects.

Both web components already expose `.broadcast`, so this is reachable as
`el.broadcast.publishJson(...)` / `el.broadcast.subscribeJson(...)`.

Docs: concept/layer/hang plus the @moq/publish and @moq/watch lib pages.
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This PR introduces a Source<T> class in @moq/json that maintains a stable JSON value and fans it out to per-subscription Producer<T> instances, seeding late joiners and supporting update()/mutate() operations. CatalogProducer in @moq/publish is refactored from a standalone class to a type alias over Source<Catalog.Root>. Both the publish and watch Broadcast classes gain new APIs: publishTrack(name, serve) registers custom track handlers with reserved-name protection, while subscribeTrack(name, priority, consume) follows the active broadcast across reconnects. Both packages now re-export @moq/json as the Json namespace. Documentation is added to the hang concept and both library API references.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main feature: adding support for publishing and subscribing to arbitrary custom tracks within broadcasts, which aligns with all documented changes and objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Description check ✅ Passed The PR description clearly describes the changeset: adding support for custom tracks in broadcasts with catalog sections and providing APIs to publish/subscribe to them.

✏️ 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/moq-arbitrary-track-insert-846893

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.

@coderabbitai coderabbitai 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.

Actionable comments posted: 5

🧹 Nitpick comments (3)
js/publish/src/json.test.ts (1)

7-27: ⚡ Quick win

Add a teardown-path unit test for effect.close() cleanup.

The new class’s lifecycle contract includes unregister/finish on cleanup; adding one assertion-driven test here would harden regressions on subscriber cleanup.

As per coding guidelines: “Write unit tests for critical functionality.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/publish/src/json.test.ts` around lines 7 - 27, The test does not verify
the teardown and cleanup behavior of the subscriber when effect.close() is
called. After the effect.close() statement in the test, add assertions to verify
that the consumer has been properly unregistered and cleaned up. For example,
verify that subsequent producer updates are no longer received by the consumer,
or that the consumer behaves as expected in a closed state. This ensures the
subscription cleanup contract is correctly enforced.

Source: Coding guidelines

js/watch/src/index.ts (1)

9-9: ⚡ Quick win

Add a doc comment for the new public re-export.

The new ./json re-export is public API surface; add a brief /** ... */ comment above it for consistency with exported-symbol docs.

As per coding guidelines, “Document every exported JavaScript/TypeScript symbol with doc comments (/** */).”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/watch/src/index.ts` at line 9, The `export * from "./json"` re-export
statement lacks a documentation comment, which violates the requirement that all
exported symbols must be documented. Add a brief `/** */` doc comment above the
`export * from "./json"` line to document the re-exported symbols and their
purpose, ensuring consistency with the codebase's documentation standards.

Source: Coding guidelines

js/watch/src/json.test.ts (1)

18-60: ⚡ Quick win

Add a reconnect-switch test for JsonConsumer follow behavior.

The suite validates initial read and teardown-to-undefined, but it doesn’t explicitly assert that switching active to a new broadcast resumes on the new track and updates value from that new source.

As per coding guidelines, “Write unit tests for critical functionality.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/watch/src/json.test.ts` around lines 18 - 60, Add a new test case after
the existing "clears the value when the broadcast goes away" test that validates
JsonConsumer's reconnect-switch behavior. The test should create an initial
MoqBroadcast and JsonConsumer, then switch the active Signal to a new
MoqBroadcast instance by calling active.set() with the new broadcast. Verify
that the consumer correctly resumes on the new track by handling the request on
the new broadcast side, updating a new Json.Producer instance with new data, and
asserting that consumer.value reflects the updated data from the new source.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@js/hang/src/catalog/root.ts`:
- Line 24: The doc comment for the Root object at line 24 is incomplete and does
not reflect the current RootSchema definition. Update the comment describing
Root to include the optional data section alongside the existing video and audio
sections and app extensions, ensuring the documentation accurately reflects all
the optional sections now present in RootSchema.

In `@js/publish/src/broadcast.ts`:
- Around line 146-151: Add validation in the publishTrack method to reject
reserved track names that conflict with built-in tracks (such as catalog.json
and audio/video names). When a reserved name is detected, throw an error to fail
fast rather than silently registering a handler that will never run due to
built-in tracks being matched first. Apply the same validation to the
publishJson method as indicated by the comment reference to lines 166-173. This
ensures users get immediate feedback when attempting to register handlers for
reserved track names.

In `@js/publish/src/json.ts`:
- Around line 34-37: The update method iterates over subscribers in the `#outputs`
collection and calls update on each one, but if any subscriber throws an
exception, the loop terminates and remaining subscribers don't receive the
update. Wrap each output.update(value) call within the loop in a try-catch block
to handle exceptions independently for each subscriber, ensuring that one failed
subscriber doesn't prevent others from receiving the update notification.

In `@rs/hang/src/catalog/data.rs`:
- Around line 11-13: The DataTrack struct is a public config-like struct that
needs the #[non_exhaustive] attribute to allow for future non-breaking additions
of optional fields. Add the #[non_exhaustive] attribute to the DataTrack struct
definition, placing it before the existing derive and serde attributes, so that
consumers cannot exhaustively construct instances and future field additions
remain compatible.

In `@rs/hang/src/catalog/root.rs`:
- Around line 32-35: The new `data` field in the `Catalog` struct requires all
external code constructing instances via struct literals to provide a value,
breaking source compatibility. To maintain additive compatibility, provide a
constructor function or builder pattern for `Catalog` that handles
initialization with sensible defaults for new optional fields, or alternatively
make the `data` field use a default implementation by providing an appropriate
default constructor pattern. This allows existing code to continue compiling
while new code can opt into the new field through a dedicated construction
method rather than direct struct literal syntax.

---

Nitpick comments:
In `@js/publish/src/json.test.ts`:
- Around line 7-27: The test does not verify the teardown and cleanup behavior
of the subscriber when effect.close() is called. After the effect.close()
statement in the test, add assertions to verify that the consumer has been
properly unregistered and cleaned up. For example, verify that subsequent
producer updates are no longer received by the consumer, or that the consumer
behaves as expected in a closed state. This ensures the subscription cleanup
contract is correctly enforced.

In `@js/watch/src/index.ts`:
- Line 9: The `export * from "./json"` re-export statement lacks a documentation
comment, which violates the requirement that all exported symbols must be
documented. Add a brief `/** */` doc comment above the `export * from "./json"`
line to document the re-exported symbols and their purpose, ensuring consistency
with the codebase's documentation standards.

In `@js/watch/src/json.test.ts`:
- Around line 18-60: Add a new test case after the existing "clears the value
when the broadcast goes away" test that validates JsonConsumer's
reconnect-switch behavior. The test should create an initial MoqBroadcast and
JsonConsumer, then switch the active Signal to a new MoqBroadcast instance by
calling active.set() with the new broadcast. Verify that the consumer correctly
resumes on the new track by handling the request on the new broadcast side,
updating a new Json.Producer instance with new data, and asserting that
consumer.value reflects the updated data from the new source.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 30826f6c-ad77-4fe6-9886-71650ae4fc9f

📥 Commits

Reviewing files that changed from the base of the PR and between c924280 and e606486.

📒 Files selected for processing (21)
  • doc/concept/layer/hang.md
  • doc/lib/js/@moq/publish.md
  • doc/lib/js/@moq/watch.md
  • js/hang/src/catalog/data.ts
  • js/hang/src/catalog/index.ts
  • js/hang/src/catalog/root.test.ts
  • js/hang/src/catalog/root.ts
  • js/publish/src/broadcast.ts
  • js/publish/src/catalog.ts
  • js/publish/src/index.ts
  • js/publish/src/json.test.ts
  • js/publish/src/json.ts
  • js/watch/src/broadcast.ts
  • js/watch/src/index.ts
  • js/watch/src/json.test.ts
  • js/watch/src/json.ts
  • rs/hang/src/catalog/data.rs
  • rs/hang/src/catalog/mod.rs
  • rs/hang/src/catalog/root.rs
  • rs/moq-mux/src/catalog/hang/ext.rs
  • rs/moq-mux/src/catalog/producer.rs

Comment thread js/hang/src/catalog/root.ts
Comment thread js/publish/src/broadcast.ts
Comment thread js/json/src/source.ts Outdated
Comment thread rs/hang/src/catalog/data.rs Outdated
Comment thread rs/hang/src/catalog/root.rs Outdated
claude added 2 commits June 16, 2026 00:04
…verage

Address CodeRabbit review feedback on #1748:
- publish Broadcast.publishTrack now rejects reserved built-in track names
  (catalog/audio/video) so a handler that could never run fails fast.
- rs/hang DataTrack gains #[non_exhaustive] so future optional fields stay
  additive for external constructors.
- js/hang Root type doc comment now mentions the data section.
- Add a teardown assertion to the publish JsonProducer test and a
  resubscribe-on-broadcast-switch test to the watch JsonConsumer test.
Reframe per review: applications add their own typed catalog sections (like
scte35) through the existing loose catalog root, and publish/subscribe arbitrary
tracks through generic helpers. MoQ stays generic with no per-application
support and no catalog-format change.

- Revert the typed `data` catalog section in js/hang and rs/hang (and the
  moq-mux construction sites). The loose RootSchema already lets an app add its
  own sections (e.g. scte35) that pass through to consumers untouched.
- publishJson no longer writes a catalog entry; the app advertises the track by
  mutating its own section via broadcast.catalog.mutate(...).
- Add watch Broadcast.subscribeTrack for arbitrary (non-JSON) tracks, mirroring
  publish Broadcast.publishTrack.
- Update docs to show the app-driven pattern: a JS app supports scte35 with no
  explicit support in hang / @moq/publish / @moq/watch.
@kixelated kixelated changed the title feat: publish & subscribe arbitrary tracks within a broadcast feat: publish & subscribe arbitrary custom tracks within a broadcast Jun 16, 2026

@coderabbitai coderabbitai 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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
js/publish/src/broadcast.ts (1)

136-143: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix contradictory publishTrack JSDoc about publishJson catalog behavior.

Line 142 says publishJson “also advertises itself in the catalog”, but Lines 165-167 document the opposite behavior. Please align this to avoid API misuse.

Suggested patch
-	 * {`@link` publishJson} for a JSON track that also advertises itself in the catalog.
+	 * {`@link` publishJson} for a JSON-track helper.

Also applies to: 165-167

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/publish/src/broadcast.ts` around lines 136 - 143, The JSDoc for the
publishTrack method at lines 136-143 in js/publish/src/broadcast.ts contains a
statement that publishJson "also advertises itself in the catalog", but the
documentation at lines 165-167 in the same file describes the opposite behavior.
Review both JSDoc comment blocks to determine the actual behavior of
publishJson, then update the incorrect documentation to align with the true
behavior and eliminate the contradiction that could lead to API misuse.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@js/publish/src/broadcast.ts`:
- Around line 136-143: The JSDoc for the publishTrack method at lines 136-143 in
js/publish/src/broadcast.ts contains a statement that publishJson "also
advertises itself in the catalog", but the documentation at lines 165-167 in the
same file describes the opposite behavior. Review both JSDoc comment blocks to
determine the actual behavior of publishJson, then update the incorrect
documentation to align with the true behavior and eliminate the contradiction
that could lead to API misuse.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 572d5252-956d-4715-923d-98c911336ca0

📥 Commits

Reviewing files that changed from the base of the PR and between 610a8ab and 32f8a28.

📒 Files selected for processing (5)
  • doc/concept/layer/hang.md
  • doc/lib/js/@moq/publish.md
  • doc/lib/js/@moq/watch.md
  • js/publish/src/broadcast.ts
  • js/watch/src/broadcast.ts
✅ Files skipped from review due to trivial changes (2)
  • doc/lib/js/@moq/publish.md
  • doc/lib/js/@moq/watch.md

claude added 2 commits June 16, 2026 00:32
publishJson no longer writes a catalog entry, so the cross-reference in
publishTrack's doc was contradictory.
Per review: drop the @moq/publish/@moq/watch JSON-specific wrappers and have
applications use @moq/json directly on the generic track hooks.

- Promote the fan-out producer into @moq/json as `Source<T>` (holds a value,
  seeds late joiners, fans out to every subscriber). This is the primitive the
  catalog was already using.
- `CatalogProducer` becomes a thin type alias for `Json.Source<Catalog.Root>`;
  the broadcast constructs it with `initial: {}`. Its fan-out tests move to
  js/json/src/source.test.ts.
- Remove publishJson/subscribeJson and the JsonProducer/JsonConsumer classes.
  publishTrack/subscribeTrack remain as the generic hooks; the app wraps the
  track with @moq/json Source (publish) or Consumer (watch).
- Re-export @moq/json as `Json` from @moq/publish and @moq/watch so apps have a
  single import for the JSON track tooling.
- Docs updated to the @moq/json-direct pattern.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

♻️ Duplicate comments (1)
js/json/src/source.ts (1)

35-38: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Isolate per-subscriber failures during fan-out updates.

One throwing subscriber can terminate the loop, so later subscribers miss updates. This remains the same unresolved fan-out failure path previously reported.

Suggested patch
 update(value: T): void {
 	this.#value = value;
-	for (const output of this.#outputs) output.update(value);
+	for (const output of this.#outputs) {
+		try {
+			output.update(value);
+		} catch (err) {
+			this.#outputs.delete(output);
+			try {
+				output.finish();
+			} catch {
+				// ignore secondary close errors
+			}
+			console.warn("dropping failed json subscriber during fan-out", err);
+		}
+	}
 }
#!/bin/bash
# Verify unguarded fan-out call and throwable paths in Producer.update.
rg -n -C2 'for \(const output of this.#outputs\) output\.update\(value\);' js/json/src/source.ts
rg -n -C3 'class Producer|update\(value: T\)|throw new Error' js/json/src/producer.ts

Expected: first command shows the unguarded loop at Line 37; second shows potential throw sites reachable via Producer.update.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/json/src/source.ts` around lines 35 - 38, The `update(value: T)` method in
the source class iterates through subscribers in `this.#outputs` and calls
`output.update(value)` without error handling. If any subscriber throws an
error, the loop terminates and remaining subscribers never receive the update.
Wrap the `output.update(value)` call in a try-catch block to isolate
per-subscriber failures, ensuring that all subscribers are processed even if one
throws an error. Consider logging or handling caught errors appropriately to aid
debugging.
🧹 Nitpick comments (3)
js/json/src/index.ts (1)

1-4: ⚡ Quick win

Add module/export doc comments for the new public entrypoint export.

The new Source re-export is public API surface but has no doc comment, and this entrypoint also lacks a module-level block.

Suggested patch
+/** `@module` */
 export { Consumer } from "./consumer.ts";
 export { type Diff, deepEqual, diff, merge } from "./diff.ts";
 export { type Config, Producer } from "./producer.ts";
+/** Stable retained-value JSON source for fan-out publishing. */
 export { Source } from "./source.ts";

As per coding guidelines: "Document every exported JavaScript/TypeScript symbol with doc comments (/** */)..." and "Add a module-level doc comment to every entrypoint."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/json/src/index.ts` around lines 1 - 4, Add a module-level doc comment at
the top of the index.ts entrypoint file explaining its purpose, and add
individual doc comments for each exported symbol. Specifically, the Source
export on line 4 is missing documentation. Add a doc comment block above the
Source export line that explains what it is and its purpose, and add a
module-level comment block at the very beginning of the file that describes the
overall purpose of this public API entrypoint module.

Source: Coding guidelines

js/publish/src/broadcast.ts (1)

159-167: ⚡ Quick win

Add unit tests for publishTrack contract edges.

Please add focused tests for reserved-name rejection and unregister behavior (unregister only removing the same handler) to lock down this new public API path.

As per coding guidelines: "Write unit tests for critical functionality."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/publish/src/broadcast.ts` around lines 159 - 167, The publishTrack method
in the Broadcast class has two important contract edges that need unit test
coverage. First, add a test that verifies the method throws an error when a
reserved track name (one that exists in Broadcast.#RESERVED_TRACKS) is passed to
publishTrack. Second, add a test for the unregister function behavior that
verifies the returned function only removes the track from this.#tracks when the
stored ServeTrack reference matches the exact same serve parameter that was
originally registered, ensuring that registering the same track name with
different handlers and then unregistering one handler does not affect the other.

Source: Coding guidelines

js/watch/src/index.ts (1)

1-3: ⚡ Quick win

Add module/export doc comments for the new Json re-export.

This new public export should include a doc comment, and this entrypoint should have a module-level @module block.

As per coding guidelines: "Document every exported JavaScript/TypeScript symbol with doc comments (/** */)..." and "Add a module-level doc comment to every entrypoint."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/watch/src/index.ts` around lines 1 - 3, Add documentation comments to the
entrypoint file to comply with coding guidelines. First, add a module-level doc
comment block at the very beginning of the file before any imports/exports,
including a `@module` tag that describes this as the watch module entrypoint.
Then, add individual doc comments above each of the three export statements (the
Hang, Json, and Net re-exports) that briefly describe what each module provides.
The Json re-export should particularly be documented as part of the new public
API.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@doc/lib/js/`@moq/publish.md:
- Around line 136-137: The markdown link for `@moq/json` in the documentation is
pointing to the incorrect path `/lib/js/@moq/hang` instead of the correct
target. Fix the link destination for the `@moq/json` reference by changing the
link target from `/lib/js/@moq/hang` to the correct path `/lib/js/@moq/json` so
users are directed to the proper documentation page.

---

Duplicate comments:
In `@js/json/src/source.ts`:
- Around line 35-38: The `update(value: T)` method in the source class iterates
through subscribers in `this.#outputs` and calls `output.update(value)` without
error handling. If any subscriber throws an error, the loop terminates and
remaining subscribers never receive the update. Wrap the `output.update(value)`
call in a try-catch block to isolate per-subscriber failures, ensuring that all
subscribers are processed even if one throws an error. Consider logging or
handling caught errors appropriately to aid debugging.

---

Nitpick comments:
In `@js/json/src/index.ts`:
- Around line 1-4: Add a module-level doc comment at the top of the index.ts
entrypoint file explaining its purpose, and add individual doc comments for each
exported symbol. Specifically, the Source export on line 4 is missing
documentation. Add a doc comment block above the Source export line that
explains what it is and its purpose, and add a module-level comment block at the
very beginning of the file that describes the overall purpose of this public API
entrypoint module.

In `@js/publish/src/broadcast.ts`:
- Around line 159-167: The publishTrack method in the Broadcast class has two
important contract edges that need unit test coverage. First, add a test that
verifies the method throws an error when a reserved track name (one that exists
in Broadcast.#RESERVED_TRACKS) is passed to publishTrack. Second, add a test for
the unregister function behavior that verifies the returned function only
removes the track from this.#tracks when the stored ServeTrack reference matches
the exact same serve parameter that was originally registered, ensuring that
registering the same track name with different handlers and then unregistering
one handler does not affect the other.

In `@js/watch/src/index.ts`:
- Around line 1-3: Add documentation comments to the entrypoint file to comply
with coding guidelines. First, add a module-level doc comment block at the very
beginning of the file before any imports/exports, including a `@module` tag that
describes this as the watch module entrypoint. Then, add individual doc comments
above each of the three export statements (the Hang, Json, and Net re-exports)
that briefly describe what each module provides. The Json re-export should
particularly be documented as part of the new public API.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b46a3dbf-8ac9-4746-bc4c-92719ce1b8f4

📥 Commits

Reviewing files that changed from the base of the PR and between 4803788 and 02619f7.

📒 Files selected for processing (12)
  • doc/concept/layer/hang.md
  • doc/lib/js/@moq/publish.md
  • doc/lib/js/@moq/watch.md
  • js/json/src/index.ts
  • js/json/src/source.test.ts
  • js/json/src/source.ts
  • js/publish/src/broadcast.ts
  • js/publish/src/catalog.test.ts
  • js/publish/src/catalog.ts
  • js/publish/src/index.ts
  • js/watch/src/broadcast.ts
  • js/watch/src/index.ts
💤 Files with no reviewable changes (1)
  • js/publish/src/catalog.test.ts
✅ Files skipped from review due to trivial changes (2)
  • doc/concept/layer/hang.md
  • doc/lib/js/@moq/watch.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • js/publish/src/index.ts

Comment thread doc/lib/js/@moq/publish.md Outdated
claude added 2 commits June 16, 2026 01:15
- CI: the publish package has no headless-testable units left (Broadcast
  transitively imports an AudioWorklet bun can't load), so `bun test` exited 1
  on "no tests found". Use `--pass-with-no-tests`.
- @moq/json Source.update now isolates per-subscriber failures during fan-out so
  one broken track doesn't stop updates to the others.
- Add a module-level doc to the @moq/json entrypoint.
- Drop dead `@moq/json` doc links (there is no @moq/json doc page); keep it as
  inline code.
A Producer constructed with a track is unchanged (the leaf encoder, byte-for-byte
identical, so the wire-format vectors still pass). A Producer constructed without
a track is the fan-out source: it retains the value, seeds late joiners, and fans
updates out to subscribers attached with the new `serve(track, effect)`. The
constructor overloads on `instanceof Track`, so `new Producer(track, config)`
(existing) and `new Producer(config)` (fan-out) both read cleanly.

- Remove the separate `Source` class; `CatalogProducer` aliases `Producer<Root>`
  and the broadcast constructs `new Producer({ initial: {} })`.
- Move the fan-out tests to producer.test.ts and add a serve()-on-leaf guard test.
- Docs use a track-less `Json.Producer` instead of `Json.Source`.
@kixelated
kixelated enabled auto-merge (squash) June 16, 2026 03:20
@kixelated
kixelated merged commit db6de19 into main Jun 16, 2026
1 check passed
@kixelated
kixelated deleted the claude/moq-arbitrary-track-insert-846893 branch June 16, 2026 03:20
kixelated added a commit that referenced this pull request Jun 16, 2026
CatalogProducer was a real fan-out class in #1658, collapsed to a thin type
alias in #1748 once @moq/json's Producer absorbed the fan-out/mutate/serve
logic. With this PR it aliased Catalog.Producer, so it added an indirection
without information. Annotate the broadcast field as Catalog.Producer directly,
delete js/publish/src/catalog.ts, and stop re-exporting it from the package.

Removes the public `CatalogProducer` type from @moq/publish; consumers use
`Catalog.Producer` from @moq/hang/catalog instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <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.

2 participants