Conversation
Dunno what I'm going to do about publishing to IETF.
WalkthroughThis pull request adds a new 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
doc/spec/draft-lcurley-moq-lite.md (1)
231-231: Fix markdown fence style/language to satisfy lint.These fenced blocks use
~~~and omit a language, which triggers MD048/MD040 repeatedly. Converting to backticks with a language (e.g.,text) will clear the warnings.Proposed pattern
-~~~ +```text STREAM_TYPE { Stream Type (i) } -~~~ +```Also applies to: 238-238, 246-246, 315-315, 329-329, 351-351, 378-378, 427-427, 446-446, 476-476, 502-502, 531-531, 545-545, 566-566
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/spec/draft-lcurley-moq-lite.md` at line 231, Replace all markdown code fences that use tilde fences "~~~" and have no language with backtick fences "```" and specify a language (use "text" for these examples). Update each occurrence indicated in doc/spec/draft-lcurley-moq-lite.md where a fenced block currently starts or ends with "~~~" (e.g., the STREAM_TYPE block and the other listed blocks) so the opening fence becomes "```text" and the closing fence becomes "```"; ensure matching fence pairs for each block to satisfy MD048/MD040 lint rules.doc/spec/draft-lcurley-moq-hang.md (1)
38-42: Consider standardizing fenced blocks for lint/readability consistency.These sections use
~~~without language tags; aligning on backticks + explicit language (e.g.,text,json,typescript) would match common markdown tooling expectations.Also applies to: 49-53, 58-60, 77-83, 97-108, 116-142, 148-153, 161-179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/spec/draft-lcurley-moq-hang.md` around lines 38 - 42, Replace the ad-hoc fenced blocks that use "~~~" with standardized triple-backtick fences and add an explicit language tag (for example, use ```text or ```json as appropriate) so markdown linters and tooling can correctly parse them; update each occurrence shown (the block starting with /room123/alice.hang and similar blocks referenced) to use backticks and a language token for consistency across the document.doc/spec/draft-lcurley-moq-use-cases.md (1)
5-5: Section headings use#(H1) after the page's own# Use Casestitle.VitePress builds the on-page outline from H2/H3 headings. Keeping every section at H1 means none of the sections (
Introduction,Video,Audio, …) will appear in the outline sidebar, reducing navigability. Consider promoting them to##and their sub-sections to###.This is common in IETF draft style, so it may be intentional, but worth confirming.
Also applies to: 12-12, 23-23, 91-91, 124-124, 161-161, 303-303, 361-361, 402-402, 406-406
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/spec/draft-lcurley-moq-use-cases.md` at line 5, The document uses multiple top-level H1 headings (e.g., "Use Cases", "Introduction", "Video", "Audio") which prevents VitePress from building the page outline; update those section headings to H2 (prefix with "##") and promote their sub-sections to H3 ("###") consistently (for example change "## Introduction" from "# Introduction", "## Video" from "# Video", etc.), and apply the same change to the other occurrences noted (sections around the headings referenced in the comment) so the on-page outline sidebar is populated correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@doc/spec/draft-lcurley-moq-hang.md`:
- Line 55: Replace the incorrect word "participant" with the verb "participate"
in the sentence shown in the diff ("If a publisher no longer wants to
participant, or is disconnected somehow, their presence will be unannounced."),
updating it to "If a publisher no longer wants to participate, or is
disconnected somehow, their presence will be unannounced." to fix the verb
choice in the participant lifecycle text.
- Line 64: Fix the duplicated word in the sentence containing "extends the the
W3C WebCodecs specification." — replace "extends the the W3C WebCodecs
specification." with "extends the W3C WebCodecs specification." so the phrase is
singular and grammatically correct.
- Around line 17-18: In the terminology bullets for "Track" and "Group" replace
the incorrect article "An series" with "A series" so the lines read "A series of
Groups..." for Track and "A series of Frames..." for Group; update the text in
the Track and Group bullets accordingly.
In `@doc/spec/draft-lcurley-moq-lite.md`:
- Around line 43-45: Fix the article usage in the terminology list: change “An
series of Groups” and “An series of Frames” to “A series of Groups” and “A
series of Frames” respectively. Update the lines defining Track and Group (the
entries labeled "Track" and "Group") so both use "A series" before the noun,
leaving the rest of the wording (e.g., "each of which can be delivered and
decoded out-of-order" and "each of which must be delivered and decoded
in-order") unchanged.
- Around line 111-112: The sentence "See the section for Messages section for
the specific encoding." contains a duplicated word; update it (e.g., change the
text in the draft-lcurley-moq-lite.md where that sentence appears) to "See the
Messages section for the specific encoding." so the duplicated "section" is
removed and the phrasing is clear.
In `@doc/spec/draft-lcurley-moq-use-cases.md`:
- Line 180: Several fenced code blocks use bare "~~~" without a language
specifier (markdownlint MD040); update each protocol pseudo-code fence that
currently reads "~~~" to include a language tag such as "text" (e.g., change
"~~~" to "~~~text") so the markdown parser/linter recognizes the block; search
for all occurrences of the plain "~~~" fence in the document (the ones cited in
the review) and apply the same change consistently.
- Line 121: Fix three small prose errors in draft-lcurley-moq-use-cases.md:
change "retransmittted" to "retransmitted" (line with "retransmittted audio FEC
packets"), change "use form of clustering" to "a form of clustering" (line with
"use form of clustering"), and remove the duplicate article in "as the a higher
level application" to read "as the higher level application". Locate these exact
phrases to update them.
- Line 113: The sentence contains a double negative: locate the line containing
"this is not currently not supported and FEC is left to the application." and
change it to a clear phrase such as "this is not currently supported and FEC is
left to the application." or "this is currently not supported and FEC is left to
the application." to convey the intended meaning without the double negative.
---
Nitpick comments:
In `@doc/spec/draft-lcurley-moq-hang.md`:
- Around line 38-42: Replace the ad-hoc fenced blocks that use "~~~" with
standardized triple-backtick fences and add an explicit language tag (for
example, use ```text or ```json as appropriate) so markdown linters and tooling
can correctly parse them; update each occurrence shown (the block starting with
/room123/alice.hang and similar blocks referenced) to use backticks and a
language token for consistency across the document.
In `@doc/spec/draft-lcurley-moq-lite.md`:
- Line 231: Replace all markdown code fences that use tilde fences "~~~" and
have no language with backtick fences "```" and specify a language (use "text"
for these examples). Update each occurrence indicated in
doc/spec/draft-lcurley-moq-lite.md where a fenced block currently starts or ends
with "~~~" (e.g., the STREAM_TYPE block and the other listed blocks) so the
opening fence becomes "```text" and the closing fence becomes "```"; ensure
matching fence pairs for each block to satisfy MD048/MD040 lint rules.
In `@doc/spec/draft-lcurley-moq-use-cases.md`:
- Line 5: The document uses multiple top-level H1 headings (e.g., "Use Cases",
"Introduction", "Video", "Audio") which prevents VitePress from building the
page outline; update those section headings to H2 (prefix with "##") and promote
their sub-sections to H3 ("###") consistently (for example change "##
Introduction" from "# Introduction", "## Video" from "# Video", etc.), and apply
the same change to the other occurrences noted (sections around the headings
referenced in the comment) so the on-page outline sidebar is populated
correctly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
doc/.vitepress/config.tsdoc/concept/layer/hang.mddoc/concept/layer/moq-lite.mddoc/js/@moq/hang/index.mddoc/js/@moq/lite.mddoc/js/index.mddoc/rs/crate/hang.mddoc/rs/crate/moq-lite.mddoc/rs/index.mddoc/spec/draft-lcurley-moq-hang.mddoc/spec/draft-lcurley-moq-lite.mddoc/spec/draft-lcurley-moq-use-cases.mddoc/spec/index.md
Fix grammar issues (article/verb errors, typos, double negatives, duplicated words) across all three spec documents. Standardize code fences from ~~~ to backtick fences with language tags. Demote heading levels in use-cases doc for proper VitePress outline. Add doc/spec to CLAUDE.md project structure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@doc/spec/draft-lcurley-moq-lite.md`:
- Around line 231-251: The three fenced example blocks containing the lines
starting "ali/audio + bob/audio: subscriber_priority=2 publisher_priority=2",
"bob/audio: subscriber_priority=2 publisher_priority=3" and "ali/audio
subscriber_priority=4 publisher_priority=2" are missing language tags; fix by
adding a language identifier (e.g., ```text) to each opening fence so the three
code blocks become explicitly fenced (```text) and eliminate the MD040 warnings.
- Around line 114-116: The sentence "moq-lite runs on top of WebTransport"
incorrectly implies a protocol-wide requirement; update that sentence (and any
adjacent lines mentioning WebTransport/QUIC) to clarify scope by saying moq-lite
uses WebTransport for web clients only (e.g., "For web clients, moq-lite runs on
top of WebTransport"), and retain the earlier note that a bare QUIC mode is
supported so there is no contradiction.
In `@doc/spec/draft-lcurley-moq-use-cases.md`:
- Around line 272-280: The draft incorrectly uses an INFO message for track
priority/order (e.g., "INFO track=...") which is not defined in moq-lite; update
the text to use the currently specified mechanism by removing the INFO-based
examples and instead reference and show the publisher fields carried in
SUBSCRIBE_OK for Track Priority and Group Order (adjust the prose and examples
to show SUBSCRIBE_OK publisher fields and their values rather than an INFO
message).
- Around line 322-323: Update the stale SIGNAL name: replace the reference to
SESSION_UPDATE with PROBE in the sentence that currently reads "The viewer
learns about the estimated bandwidth via the SESSION_UPDATE message..." and
adjust wording to state that bitrate signaling is provided via the PROBE message
(per moq-lite-03); ensure the sentence still mentions local measurement as an
alternative and, if present elsewhere, update any other occurrences of
SESSION_UPDATE to PROBE to keep the doc consistent with moq-lite-03.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CLAUDE.mddoc/spec/draft-lcurley-moq-hang.mddoc/spec/draft-lcurley-moq-lite.mddoc/spec/draft-lcurley-moq-use-cases.md
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
| moq-lite runs on top of WebTransport. | ||
| WebTransport is a layer on top of QUIC and HTTP/3, required for web support. | ||
| The API is nearly identical to QUIC with the exception of stream IDs. |
There was a problem hiding this comment.
Clarify transport scope to avoid contradicting earlier QUIC support.
This section states moq-lite runs on WebTransport, but earlier lines explicitly define a bare QUIC mode. Please reword this as “for web clients” rather than a protocol-wide requirement.
Proposed edit
-moq-lite runs on top of WebTransport.
-WebTransport is a layer on top of QUIC and HTTP/3, required for web support.
-The API is nearly identical to QUIC with the exception of stream IDs.
+moq-lite can run over bare QUIC or over WebTransport.
+For web clients, WebTransport (over HTTP/3) is required.
+The WebTransport API is nearly identical to QUIC, except for stream IDs.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| moq-lite runs on top of WebTransport. | |
| WebTransport is a layer on top of QUIC and HTTP/3, required for web support. | |
| The API is nearly identical to QUIC with the exception of stream IDs. | |
| moq-lite can run over bare QUIC or over WebTransport. | |
| For web clients, WebTransport (over HTTP/3) is required. | |
| The WebTransport API is nearly identical to QUIC, except for stream IDs. |
🧰 Tools
🪛 LanguageTool
[style] ~116-~116: ‘with the exception of’ might be wordy. Consider a shorter alternative.
Context: ...rt. The API is nearly identical to QUIC with the exception of stream IDs. How the WebTransport conne...
(EN_WORDINESS_PREMIUM_WITH_THE_EXCEPTION_OF)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@doc/spec/draft-lcurley-moq-lite.md` around lines 114 - 116, The sentence
"moq-lite runs on top of WebTransport" incorrectly implies a protocol-wide
requirement; update that sentence (and any adjacent lines mentioning
WebTransport/QUIC) to clarify scope by saying moq-lite uses WebTransport for web
clients only (e.g., "For web clients, moq-lite runs on top of WebTransport"),
and retain the earlier note that a bare QUIC mode is supported so there is no
contradiction.
| ``` | ||
| ali/audio + bob/audio: subscriber_priority=2 publisher_priority=2 | ||
| ali/video + bob/video: subscriber_priority=1 publisher_priority=1 | ||
| ``` | ||
|
|
||
| If Bob starts actively speaking, they can bump their publisher priority via a SUBSCRIBE_OK message. | ||
| This would cause tracks be delivered in this order: | ||
| ``` | ||
| bob/audio: subscriber_priority=2 publisher_priority=3 | ||
| ali/audio: subscriber_priority=2 publisher_priority=2 | ||
| bob/video: subscriber_priority=1 publisher_priority=2 | ||
| ali/video: subscriber_priority=1 publisher_priority=1 | ||
| ``` | ||
|
|
||
| The subscriber priority takes precedence, so we could override it if we decided to full-screen Ali's window: | ||
| ``` | ||
| ali/audio subscriber_priority=4 publisher_priority=2 | ||
| ali/video subscriber_priority=3 publisher_priority=1 | ||
| bob/audio subscriber_priority=2 publisher_priority=3 | ||
| bob/video subscriber_priority=1 publisher_priority=2 | ||
| ``` |
There was a problem hiding this comment.
Add language tags to fenced example blocks (MD040).
These code fences are missing language identifiers, which will keep markdownlint warnings active.
Proposed edit
-```
+```text
ali/audio + bob/audio: subscriber_priority=2 publisher_priority=2
ali/video + bob/video: subscriber_priority=1 publisher_priority=1- +text
bob/audio: subscriber_priority=2 publisher_priority=3
ali/audio: subscriber_priority=2 publisher_priority=2
bob/video: subscriber_priority=1 publisher_priority=2
ali/video: subscriber_priority=1 publisher_priority=1
-```
+```text
ali/audio subscriber_priority=4 publisher_priority=2
ali/video subscriber_priority=3 publisher_priority=1
bob/audio subscriber_priority=2 publisher_priority=3
bob/video subscriber_priority=1 publisher_priority=2
</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.21.0)</summary>
[warning] 231-231: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
---
[warning] 238-238: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
---
[warning] 246-246: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @doc/spec/draft-lcurley-moq-lite.md around lines 231 - 251, The three fenced
example blocks containing the lines starting "ali/audio + bob/audio:
subscriber_priority=2 publisher_priority=2", "bob/audio: subscriber_priority=2
publisher_priority=3" and "ali/audio subscriber_priority=4 publisher_priority=2"
are missing language tags; fix by adding a language identifier (e.g., text) to each opening fence so the three code blocks become explicitly fenced (text) and eliminate the MD040 warnings.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
| moq-lite addresses this by providing the publisher's Track Priority and Group Order in the INFO message. | ||
| This is the intended behavior for the first hop and dictates which viewers are preferred. | ||
|
|
||
| For example, suppose the producer chooses: | ||
|
|
||
| ```text | ||
| INFO track=audio priority=1 order=DESC | ||
| INFO track=video priority=0 order=DESC | ||
| ``` |
There was a problem hiding this comment.
INFO message usage is inconsistent with the moq-lite spec in this PR.
This section relies on INFO track=..., but the moq-lite draft here does not define an INFO message for track priority/order signaling. Please update this flow to the currently defined mechanism (e.g., publisher fields in SUBSCRIBE_OK).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@doc/spec/draft-lcurley-moq-use-cases.md` around lines 272 - 280, The draft
incorrectly uses an INFO message for track priority/order (e.g., "INFO
track=...") which is not defined in moq-lite; update the text to use the
currently specified mechanism by removing the INFO-based examples and instead
reference and show the publisher fields carried in SUBSCRIBE_OK for Track
Priority and Group Order (adjust the prose and examples to show SUBSCRIBE_OK
publisher fields and their values rather than an INFO message).
| The viewer learns about the estimated bandwidth via the SESSION_UPDATE message, or by measuring network traffic and can then choose the appropriate track based on bitrate. | ||
|
|
There was a problem hiding this comment.
SESSION_UPDATE reference appears stale relative to moq-lite-03.
The moq-lite changelog in this PR says SESSION_UPDATE bitrate signaling was replaced by PROBE. Keeping SESSION_UPDATE here will confuse implementers following both docs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@doc/spec/draft-lcurley-moq-use-cases.md` around lines 322 - 323, Update the
stale SIGNAL name: replace the reference to SESSION_UPDATE with PROBE in the
sentence that currently reads "The viewer learns about the estimated bandwidth
via the SESSION_UPDATE message..." and adjust wording to state that bitrate
signaling is provided via the PROBE message (per moq-lite-03); ensure the
sentence still mentions local measurement as an alternative and, if present
elsewhere, update any other occurrences of SESSION_UPDATE to PROBE to keep the
doc consistent with moq-lite-03.
1 participant
Dunno what I'm going to do about publishing to IETF.