Auto-detect catalog format from broadcast name extension#1394
Open
kixelated wants to merge 4 commits into
Open
Auto-detect catalog format from broadcast name extension#1394kixelated wants to merge 4 commits into
kixelated wants to merge 4 commits into
Conversation
Introduce a filename-style convention (`.hang`, `.msf`) on broadcast names so the catalog format is self-describing. Producers append `.hang` when no extension is given; consumers parse the suffix to seed `catalogFormat`, falling back to `hang` for legacy names. - `moq_mux::format` exposes `CatalogFormat`, `detect`, and `ensure` helpers (plus unit tests). `moq-cli` runs every `--name` through `ensure` for serve, publish, and subscribe so the wire name is consistent. - `@moq/hang/catalog` mirrors the Rust helper. `@moq/publish` auto-appends before calling `connection.publish`. `@moq/watch` makes `catalogFormat` optional (undefined = auto-detect) and resolves the name on subscribe so watcher and publisher stay symmetric without forcing the user to type the extension on both sides. - Both catalog tracks (`catalog.json` and `catalog`) continue to be published per broadcast, so the extension is a discovery hint rather than a track filter, keeping legacy consumers working.
Contributor
WalkthroughThis change introduces catalog format auto-detection and normalization across Rust and JavaScript runtimes. A new 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/js/`@moq/watch.md:
- Line 72: The line describing `catalog-format` uses an em dash; remove the em
dash and replace it with standard punctuation—e.g., change "`catalog-format` —
Catalog format: ...`" to "`catalog-format`: Catalog format: ...", or split into
two sentences so there's no em dash; update the `catalog-format` description
accordingly and ensure surrounding punctuation and spacing are correct.
- Around line 77-79: Replace the em dash in the sentence "The format is detected
from the broadcast name extension by default — `room/alice.hang` uses hang,
`room/alice.msf` uses MSF." in doc/js/@moq/watch.md: restructure it to avoid the
em dash by using a period or colon and make two clear sentences (e.g., "The
format is detected from the broadcast name extension by default.
`room/alice.hang` uses hang, `room/alice.msf` uses MSF.") and ensure the
surrounding guidance for `catalog-format` remains unchanged.
🪄 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: 7df78584-ff3f-4812-8ff9-cd0ce9331c7b
📒 Files selected for processing (9)
doc/js/@moq/watch.mdjs/hang/src/catalog/format.tsjs/hang/src/catalog/index.tsjs/publish/src/broadcast.tsjs/watch/src/broadcast.tsjs/watch/src/element.tsrs/moq-cli/src/main.rsrs/moq-mux/src/format.rsrs/moq-mux/src/lib.rs
kixelated
commented
May 9, 2026
| // An explicit `catalogFormat` of `"msf"` chooses `.msf`; everything else | ||
| // (including `"manual"` and the `undefined` auto-detect default) chooses | ||
| // `.hang`. | ||
| #resolveName(name: Moq.Path.Valid, explicit: CatalogFormat | undefined): Moq.Path.Valid { |
Collaborator
Author
There was a problem hiding this comment.
IMO move the effect.get(this.name) into this function, since we do it in both callers.
| } | ||
|
|
||
| const name = effect.get(this.name); | ||
| const explicit = effect.get(this.catalogFormat); |
Collaborator
Author
There was a problem hiding this comment.
let catalogFormat = effect.get(this.catalogFormat);
I don't think explicit is descriptive enough.
|
|
||
| function parseCatalogFormat(value: string | null): CatalogFormat { | ||
| return CATALOG_FORMATS.find((f) => f === value) ?? DEFAULT_CATALOG_FORMAT; | ||
| function parseCatalogFormat(value: string | null): CatalogFormat | undefined { |
Collaborator
Author
There was a problem hiding this comment.
Move this to the catalog module?
|
|
||
| /// Return `name` unchanged if it already has a recognized extension, | ||
| /// otherwise append `default.extension()`. | ||
| pub fn ensure(name: &str, default: CatalogFormat) -> String { |
Collaborator
Author
There was a problem hiding this comment.
And should detect/ensure be static methods on CatalogFormat? dunno.
| pub mod import; | ||
|
|
||
| pub use error::*; | ||
| pub use format::CatalogFormat; |
Collaborator
Author
There was a problem hiding this comment.
I don't like promoting this.
| pub mod container; | ||
| mod error; | ||
| pub mod export; | ||
| pub mod format; |
Collaborator
Author
There was a problem hiding this comment.
IMO format.rs should be in catalog, not a new module.
- Move format module into catalog (per kixelated review) - Drop publish-side auto-append. Mutating the publisher's chosen name was asymmetric and surprising. Just warn if the name has no recognized catalog format extension; receivers detect from suffix only. - Receiver no longer rewrites the broadcast name either; #resolveName removed. - Rename JS exports for the flat catalog namespace: CATALOG_FORMATS -> FORMATS, DEFAULT_CATALOG_FORMAT -> DEFAULT_FORMAT, CatalogFormat type -> Format. Drop extensionFor/EXTENSIONS (the format value already is the extension stem). - Extract parseCatalogFormat into js/watch/broadcast. - Fix em dashes per CLAUDE.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
js/watch/src/broadcast.ts (1)
14-17: ⚡ Quick winAdd API doc comment for
parseCatalogFormat().This is a public export with behavior that matters (
nulland unrecognized values both map toundefined), so it should have a short doc comment like the other public catalog helpers.As per coding guidelines, "Document public APIs with clear docstrings or 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/broadcast.ts` around lines 14 - 17, Add a short API doc comment above the exported function parseCatalogFormat describing its signature and behavior: it accepts a string or null and returns a CatalogFormat or undefined, explicitly noting that null input and unrecognized values both yield undefined; reference that it checks membership against CATALOG_FORMATS and mention any examples or edge cases (e.g., null -> undefined, unknown string -> undefined) consistent with the style used for other public catalog helpers.rs/moq-cli/src/main.rs (1)
139-146: ⚡ Quick winConsolidate missing-format warnings into one code path.
The same detect-and-warn behavior exists in two places with different message text. Centralizing this avoids drift and keeps CLI guidance consistent.
♻️ Suggested diff
Command::Publish { config, url, name, format, } => { - if moq_mux::catalog::CatalogFormat::detect(&name).is_none() { - tracing::warn!( - name, - "broadcast name has no catalog format extension; consumers will fall back to {:?}. Append `{}` to make the format explicit.", - moq_mux::catalog::CatalogFormat::DEFAULT, - moq_mux::catalog::CatalogFormat::DEFAULT.extension(), - ); - } + warn_if_missing_format(&name); let publish = Publish::new(&format)?; let client = config.init()?; @@ fn warn_if_missing_format(name: &str) { if moq_mux::catalog::CatalogFormat::detect(name).is_none() { tracing::warn!( name, - "broadcast name has no catalog format extension. Append `{}` to make the format explicit.", + "broadcast name has no catalog format extension; consumers will fall back to {:?}. Append `{}` to make the format explicit.", + moq_mux::catalog::CatalogFormat::DEFAULT, moq_mux::catalog::CatalogFormat::DEFAULT.extension(), ); } }Also applies to: 171-179
🤖 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 `@rs/moq-cli/src/main.rs` around lines 139 - 146, There are duplicate detect-and-warn blocks using moq_mux::catalog::CatalogFormat::detect and emitting slightly different messages; extract this into a single helper (e.g., fn warn_missing_catalog_format(name: &str) or similar) that calls moq_mux::catalog::CatalogFormat::detect(&name) and, if None, emits a consistent tracing::warn! message referencing moq_mux::catalog::CatalogFormat::DEFAULT and DEFAULT.extension(); replace both original blocks (the ones using detect and the warn macro) with calls to this helper so the guidance text is centralized and identical.
🤖 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.
Nitpick comments:
In `@js/watch/src/broadcast.ts`:
- Around line 14-17: Add a short API doc comment above the exported function
parseCatalogFormat describing its signature and behavior: it accepts a string or
null and returns a CatalogFormat or undefined, explicitly noting that null input
and unrecognized values both yield undefined; reference that it checks
membership against CATALOG_FORMATS and mention any examples or edge cases (e.g.,
null -> undefined, unknown string -> undefined) consistent with the style used
for other public catalog helpers.
In `@rs/moq-cli/src/main.rs`:
- Around line 139-146: There are duplicate detect-and-warn blocks using
moq_mux::catalog::CatalogFormat::detect and emitting slightly different
messages; extract this into a single helper (e.g., fn
warn_missing_catalog_format(name: &str) or similar) that calls
moq_mux::catalog::CatalogFormat::detect(&name) and, if None, emits a consistent
tracing::warn! message referencing moq_mux::catalog::CatalogFormat::DEFAULT and
DEFAULT.extension(); replace both original blocks (the ones using detect and the
warn macro) with calls to this helper so the guidance text is centralized and
identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eae11a77-eea9-4b70-8bf3-6a3c1958bbac
📒 Files selected for processing (9)
doc/js/@moq/watch.mdjs/hang/src/catalog/format.tsjs/publish/src/broadcast.tsjs/watch/src/broadcast.tsjs/watch/src/element.tsrs/moq-cli/src/main.rsrs/moq-mux/src/catalog/format.rsrs/moq-mux/src/catalog/mod.rsrs/moq-mux/src/lib.rs
✅ Files skipped from review due to trivial changes (3)
- doc/js/@moq/watch.md
- rs/moq-mux/src/lib.rs
- js/publish/src/broadcast.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- js/watch/src/element.ts
The auto-append in moq-cli now warns more directly: "append .hang to your broadcast name to make the catalog format explicit" Update the demo justfiles, web demos, doc site examples, and package READMEs to use the `.hang` extension on broadcast names so the convention is visible without relying on the auto-append.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@demo/web/src/publish.html`:
- Line 26: Update the nearby tips text to reflect the new default broadcast name
"me.hang": find the explanatory copy that references the default "me" and the
example query parameter "broadcast=notme" (the nearby tips section tied to the
<moq-publish> usage) and change it to mention "me.hang" as the default and
update any example links/parameters to use "broadcast=notme" only if still
correct — otherwise make the example consistent with "me.hang" (e.g., use
broadcast=notme with context showing it differs from the default "me.hang").
Ensure the text and example link mention the new default name "me.hang" wherever
the old "me" appeared.
In `@rs/moq-cli/src/main.rs`:
- Around line 117-118: The code currently calls warn_if_missing_format(&name)
but continues using the original name, which can cause mismatches; update the
usage to normalize and return a canonical name from warn_if_missing_format (or
introduce a new normalize_broadcast_name function) and assign that back to the
local variable before creating Publish::new(&format) and other
consumers/producers; e.g., have warn_if_missing_format/normalize_broadcast_name
return a String and then replace occurrences of name used in Publish::new and
the related command execution sites so both warning and execution use the
normalized name.
🪄 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: 750f9f4e-d507-4100-b27a-ffc2734b8081
📒 Files selected for processing (15)
demo/pub/justfiledemo/web/src/index.htmldemo/web/src/manual.htmldemo/web/src/mse.htmldemo/web/src/publish.htmldoc/js/@moq/hang/publish.mddoc/js/@moq/hang/watch.mddoc/js/@moq/publish.mddoc/js/@moq/watch.mddoc/js/env/web.mddoc/js/index.mdjs/publish/README.mdjs/publish/src/broadcast.tsjs/watch/README.mdrs/moq-cli/src/main.rs
✅ Files skipped from review due to trivial changes (6)
- demo/web/src/manual.html
- js/publish/README.md
- js/watch/README.md
- doc/js/@moq/hang/watch.md
- doc/js/@moq/publish.md
- doc/js/index.md
🚧 Files skipped from review as they are similar to previous changes (2)
- js/publish/src/broadcast.ts
- doc/js/@moq/watch.md
| --> | ||
| <moq-publish-ui> | ||
| <moq-publish url="%VITE_RELAY_URL%" name="me"> | ||
| <moq-publish url="%VITE_RELAY_URL%" name="me.hang"> |
Contributor
There was a problem hiding this comment.
Update nearby tips text to match the new default name
Line 26 changed the default broadcast to me.hang, but the tips section still says the default is me and links to broadcast=notme. Please update that copy so examples stay consistent.
Suggested doc tweak
- This page creates a broadcast called `me` by default.
- You can use <a href="index.html?broadcast=notme">query parameters</a> to use a different broadcast name and create
+ This page creates a broadcast called `me.hang` by default.
+ You can use <a href="index.html?broadcast=notme.hang">query parameters</a> to use a different broadcast name and createAlso applies to: 33-33
🤖 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 `@demo/web/src/publish.html` at line 26, Update the nearby tips text to reflect
the new default broadcast name "me.hang": find the explanatory copy that
references the default "me" and the example query parameter "broadcast=notme"
(the nearby tips section tied to the <moq-publish> usage) and change it to
mention "me.hang" as the default and update any example links/parameters to use
"broadcast=notme" only if still correct — otherwise make the example consistent
with "me.hang" (e.g., use broadcast=notme with context showing it differs from
the default "me.hang"). Ensure the text and example link mention the new default
name "me.hang" wherever the old "me" appeared.
Comment on lines
+117
to
118
| warn_if_missing_format(&name); | ||
| let publish = Publish::new(&format)?; |
Contributor
There was a problem hiding this comment.
Normalize broadcast names instead of warning only
Line 117, Line 139, and Line 164 currently warn but keep name unchanged. That can break producer and consumer alignment when one side relies on explicit .hang naming. Return a normalized name and use it for command execution.
Suggested fix
-fn warn_if_missing_format(name: &str) {
- if moq_mux::catalog::CatalogFormat::detect(name).is_none() {
- tracing::warn!(name, "append .hang to your broadcast name to make the catalog format explicit");
- }
+fn ensure_format_extension(name: &str) -> String {
+ if moq_mux::catalog::CatalogFormat::detect(name).is_none() {
+ let resolved = moq_mux::catalog::CatalogFormat::DEFAULT.ensure(name);
+ tracing::warn!(name, resolved, "broadcast name has no catalog extension, using resolved name");
+ resolved
+ } else {
+ name.to_string()
+ }
}- warn_if_missing_format(&name);
+ let name = ensure_format_extension(&name);
let publish = Publish::new(&format)?;
...
- res = run_server(server, name, publish.consume()) => res,
+ res = run_server(server, name, publish.consume()) => res,- warn_if_missing_format(&name);
+ let name = ensure_format_extension(&name);
let publish = Publish::new(&format)?;
...
- run_client(client, url, name, publish).await
+ run_client(client, url, name, publish).awaitAlso applies to: 139-140, 164-168
🤖 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 `@rs/moq-cli/src/main.rs` around lines 117 - 118, The code currently calls
warn_if_missing_format(&name) but continues using the original name, which can
cause mismatches; update the usage to normalize and return a canonical name from
warn_if_missing_format (or introduce a new normalize_broadcast_name function)
and assign that back to the local variable before creating Publish::new(&format)
and other consumers/producers; e.g., have
warn_if_missing_format/normalize_broadcast_name return a String and then replace
occurrences of name used in Publish::new and the related command execution sites
so both warning and execution use the normalized name.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add support for auto-detecting catalog format (hang vs. MSF) from broadcast name extensions (
.hangor.msf), allowing producers and consumers to stay in sync without explicit configuration.Key Changes
New
formatmodule inrs/moq-mux: ProvidesCatalogFormatenum,detect()function to parse extensions, andensure()function to append default extensions to broadcast names. Includes comprehensive tests.New
formatmodule injs/hang: TypeScript equivalent withdetectFormat(),ensureExtension(), and format constants for use across JavaScript packages.Updated
js/watchBroadcast:catalogFormatsignal now acceptsundefinedto enable auto-detection#resolveName()method to append format extensions to broadcast namesUpdated
js/publishBroadcast: Auto-appends.hangextension when publishing so consumers can detect the format from the broadcast name.Updated
js/watchelement:parseCatalogFormat()now returnsundefinedwhen attribute is omitted, enabling auto-detection instead of defaulting to "hang".Updated
rs/moq-cli: Addedensure_format_extension()helper to append.hangextension to broadcast names in publish and subscribe commands.Documentation: Updated
doc/js/@moq/watch.mdto explain the auto-detection behavior and how to override it.Implementation Details
#resolveName()method in watch ensures both announced and consumed broadcasts use the same resolved name.catalogFormatsettings always take precedence over name-derived detection..hang) when resolving names.https://claude.ai/code/session_01CMy4MWdJzvCgy3LdJA6WFy