moq-lite-04: explicit hop IDs and exclude hop filter#21
moq-lite-04: explicit hop IDs and exclude hop filter#21
Conversation
- Replace ANNOUNCE `Hops` count with explicit `Hop Count` + `Hop ID` list for relay loop detection. - Add `Exclude Hop` field to ANNOUNCE_PLEASE so relays can filter out announces that passed through themselves. - Hop ID 0 reserved for unknown hops (bridging from older versions). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughThe draft updates MOQ-Lite protocol text (clarifying Track contains Groups and Group contains Frames and fixing a Flow reference) and changes discovery/announce messaging. 🚥 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)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
draft-lcurley-moq-lite.md (1)
388-394: 🛠️ Refactor suggestion | 🟠 MajorSpecify validation requirement for Hop Count.
The wire format shows
Hop Count (i)followed byHop ID (i) ..., but the specification doesn't explicitly state that an implementation MUST validate that the Hop Count matches the actual number of Hop ID values received. This could lead to protocol ambiguity.🛡️ Suggested validation requirement
Add after line 394 or in line 407's description:
A receiver MUST close the stream with a PROTOCOL_VIOLATION if the number of Hop ID values does not match the Hop Count.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@draft-lcurley-moq-lite.md` around lines 388 - 394, Add a MUST-level validation that the numeric field "Hop Count (i)" in the ANNOUNCE Message exactly matches the number of subsequent "Hop ID (i)" entries; if they differ the receiver MUST close the stream with a PROTOCOL_VIOLATION. Update the ANNOUNCE Message description (or insert after the current wire-format block) to state this requirement and reference the fields "Hop Count" and "Hop ID" so implementers know to count Hop ID entries and enforce the check.
🧹 Nitpick comments (1)
draft-lcurley-moq-lite.md (1)
371-373: Minor terminology inconsistency.The description uses "hop list" but the wire format defines
Hop Count (i)followed byHop ID (i) .... Consider using more precise terminology like "broadcasts whose Hop IDs include this value" to align with the wire format field names.📝 Suggested clarification
**Exclude Hop**: -If non-zero, the publisher SHOULD skip ANNOUNCE messages for broadcasts whose hop list contains this value. +If non-zero, the publisher SHOULD skip ANNOUNCE messages for broadcasts whose Hop ID list contains this value. This is used by relays to avoid routing loops in a cluster.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@draft-lcurley-moq-lite.md` around lines 371 - 373, The terminology in "Exclude Hop" is inconsistent with the wire format: replace "hop list" with the precise field names used in the wire format (e.g., refer to "Hop ID (i)" or "Hop IDs") so the sentence reads that the publisher SHOULD skip ANNOUNCE messages for broadcasts whose Hop IDs include this value; reference the related fields "Hop Count (i)" and "Hop ID (i)" to make the mapping explicit and ensure the wording aligns with the defined wire format.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@draft-lcurley-moq-lite.md`:
- Around line 406-413: Clarify that when a relay forwards an announcement it
MUST append its own Hop ID and increment the Hop Count field (i.e., Hop Count is
the numeric count of Hop ID entries present); explicitly state that "Hop Count"
refers to the value in the Hop Count field (the number of Hop ID entries) and
may differ from non-zero Hop ID values if unknown hops are represented; define
that a Hop ID value of 0 indicates an unknown/bridged relay hop (not that the
Hop Count is unknown), and that a Hop Count value of 0 means there are no Hop ID
entries present (fully unknown path), plus note expected behavior when receiving
messages from older protocol peers (relay SHOULD append a new Hop ID and
increment Hop Count even if prior IDs are absent).
---
Outside diff comments:
In `@draft-lcurley-moq-lite.md`:
- Around line 388-394: Add a MUST-level validation that the numeric field "Hop
Count (i)" in the ANNOUNCE Message exactly matches the number of subsequent "Hop
ID (i)" entries; if they differ the receiver MUST close the stream with a
PROTOCOL_VIOLATION. Update the ANNOUNCE Message description (or insert after the
current wire-format block) to state this requirement and reference the fields
"Hop Count" and "Hop ID" so implementers know to count Hop ID entries and
enforce the check.
---
Nitpick comments:
In `@draft-lcurley-moq-lite.md`:
- Around line 371-373: The terminology in "Exclude Hop" is inconsistent with the
wire format: replace "hop list" with the precise field names used in the wire
format (e.g., refer to "Hop ID (i)" or "Hop IDs") so the sentence reads that the
publisher SHOULD skip ANNOUNCE messages for broadcasts whose Hop IDs include
this value; reference the related fields "Hop Count (i)" and "Hop ID (i)" to
make the mapping explicit and ensure the wording aligns with the defined wire
format.
🪄 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: 65e24dc8-09e7-42f0-bc36-199e2f015524
📒 Files selected for processing (1)
draft-lcurley-moq-lite.md
Strengthen ANNOUNCE hop tracking: add MUST-level validation for Hop Count
matching Hop ID entries, require relays to append Hop IDs when forwarding,
and clarify Hop ID value 0 vs Hop Count value 0 semantics. Also fix
grammar ("An series" → "A series"), a duplicate "section" word, and
invalid BCP 14 keyword "SHOULD BE".
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@draft-lcurley-moq-lite.md`:
- Line 623: The heading "moq-lite-04" currently uses ATX style ("##
moq-lite-04") which violates markdownlint MD003; replace the ATX line with a
setext-style heading by removing the leading "##" and adding a setext underline
on the following line (for an H2 use a line of three or more hyphens, e.g. place
"moq-lite-04" on one line and "---" on the next). Ensure no extra trailing
spaces so the new setext heading (moq-lite-04 + ---) replaces the original ATX
heading.
🪄 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: 0486fb5a-8459-4f6a-8cdd-a0f1ad9b6ff0
📒 Files selected for processing (1)
draft-lcurley-moq-lite.md
|
|
||
| # Appendix A: Changelog | ||
|
|
||
| ## moq-lite-04 |
There was a problem hiding this comment.
Fix heading style at Line 623 to satisfy markdownlint MD003.
## moq-lite-04 is ATX style, but the configured rule expects setext style. Please convert this heading to setext (or align rule config if ATX is intended globally).
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 623-623: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@draft-lcurley-moq-lite.md` at line 623, The heading "moq-lite-04" currently
uses ATX style ("## moq-lite-04") which violates markdownlint MD003; replace the
ATX line with a setext-style heading by removing the leading "##" and adding a
setext underline on the following line (for an H2 use a line of three or more
hyphens, e.g. place "moq-lite-04" on one line and "---" on the next). Ensure no
extra trailing spaces so the new setext heading (moq-lite-04 + ---) replaces the
original ATX heading.
1 participant
Summary
Hopscount with explicitHop Count+Hop IDlist for relay loop detectionExclude Hopfield to ANNOUNCE_PLEASE so relays can filter out announces that passed through themselvesImplementation
Corresponding code changes: https://github.com/moq-dev/moq/tree/explicit-hops
🤖 Generated with Claude Code