diff --git a/crates/wzp-proto/src/packet.rs b/crates/wzp-proto/src/packet.rs index f76f488..0c420af 100644 --- a/crates/wzp-proto/src/packet.rs +++ b/crates/wzp-proto/src/packet.rs @@ -184,9 +184,12 @@ pub struct MediaHeaderV2 { } impl MediaHeaderV2 { + /// Header size in bytes on the wire (16 for v2). pub const WIRE_SIZE: usize = 16; + /// Protocol version byte (always 2). pub const VERSION: u8 = 2; + /// Serialize the header to a buffer in big-endian wire format. pub fn write_to(&self, buf: &mut impl BufMut) { buf.put_u8(self.version); buf.put_u8(self.flags); @@ -199,6 +202,8 @@ impl MediaHeaderV2 { buf.put_u16(self.fec_block); } + /// Deserialize from a buffer. Returns `None` if the buffer is too short + /// or the version byte is not 2. pub fn read_from(buf: &mut impl Buf) -> Option { if buf.remaining() < Self::WIRE_SIZE { return None; @@ -228,20 +233,28 @@ impl MediaHeaderV2 { }) } + /// Bit 7: set when this packet is an FEC repair packet, not source media. pub const FLAG_REPAIR: u8 = 0b1000_0000; + /// Bit 6: set when a [`QualityReport`] trailer is appended to the payload. pub const FLAG_QUALITY: u8 = 0b0100_0000; + /// Bit 5: set for video keyframes (reserved for future video use). pub const FLAG_KEYFRAME: u8 = 0b0010_0000; + /// Bit 4: set when this packet is the final fragment of a frame. pub const FLAG_FRAME_END: u8 = 0b0001_0000; + /// Returns true if the repair flag is set. pub fn is_repair(&self) -> bool { self.flags & Self::FLAG_REPAIR != 0 } + /// Returns true if the quality-report flag is set. pub fn has_quality(&self) -> bool { self.flags & Self::FLAG_QUALITY != 0 } + /// Returns true if the keyframe flag is set. pub fn is_keyframe(&self) -> bool { self.flags & Self::FLAG_KEYFRAME != 0 } + /// Returns true if the frame-end flag is set. pub fn is_frame_end(&self) -> bool { self.flags & Self::FLAG_FRAME_END != 0 } diff --git a/docs/PRD/TASKS.md b/docs/PRD/TASKS.md index 41bfbf7..b99a797 100644 --- a/docs/PRD/TASKS.md +++ b/docs/PRD/TASKS.md @@ -1235,8 +1235,8 @@ Statuses (in order of progression): | Task | Status | Agent | Started (UTC) | Completed (UTC) | Report | Reviewer notes | |---|---|---|---|---|---|---| | T1.1 | Approved | Kimi Code CLI | 2026-05-11T06:09Z | 2026-05-11T06:54Z | [report](reports/T1.1-report.md) | Approved 2026-05-11. Spawned T1.1.1 (field rustdoc) and T1.1.2 (refresh stale test-count). | -| T1.1.1 | Changes Requested | Kimi Code CLI | 2026-05-11T07:17Z | — | [report](reports/T1.1.1-report.md) | Field docs OK; missing on 6 consts (WIRE_SIZE/VERSION + 4 FLAG_*) and 6 methods (write_to/read_from + 4 accessors). Skipped second Verify command (`clippy -W missing_docs`). See report for rework instructions. | -| T1.1.2 | In Progress | Kimi Code CLI | 2026-05-11T07:19Z | — | — | — | +| T1.1.1 | Pending Review | Kimi Code CLI | 2026-05-11T07:17Z | 2026-05-11T07:22Z | [report](reports/T1.1.1-report.md) | Rework submitted: added rustdoc on 6 consts + 6 methods in impl MediaHeaderV2. Re-ran both Verify commands. | +| T1.1.2 | Pending Review | Kimi Code CLI | 2026-05-11T07:19Z | 2026-05-11T07:21Z | reports/T1.1.2-report.md | — | | T1.2 | Approved | Kimi Code CLI | 2026-05-11T06:55Z | 2026-05-11T07:08Z | [report](reports/T1.2-report.md) | Approved 2026-05-11. Spawned T1.2.1 (rustdoc on MediaType variants/methods). Agent also resolved the T1.2 TODO inside MediaHeaderV2 — good call. | | T1.2.1 | Open | — | — | — | — | Spawned from T1.2 review; non-blocking | | T1.3 | Approved | Kimi Code CLI | 2026-05-11T07:10Z | 2026-05-11T07:11Z | [report](reports/T1.3-report.md) | Approved 2026-05-11. No follow-ups; docs-and-test-only change. | diff --git a/docs/PRD/reports/T1.1.1-report.md b/docs/PRD/reports/T1.1.1-report.md index 98ecf9f..62a4d65 100644 --- a/docs/PRD/reports/T1.1.1-report.md +++ b/docs/PRD/reports/T1.1.1-report.md @@ -1,6 +1,6 @@ # T1.1.1 — Add rustdoc on `MediaHeaderV2` fields -**Status:** Pending Review +**Status:** Changes Requested **Agent:** Kimi Code CLI **Started:** 2026-05-11T07:17Z **Completed:** 2026-05-11T07:18Z @@ -55,8 +55,58 @@ None. ## Reviewer checklist (filled in by reviewer) -- [ ] Code matches PRD intent -- [ ] Verification output is real (re-run if suspicious) -- [ ] No backward-incompat surprises -- [ ] Tests cover the new behavior +- [x] Field-level rustdoc complete and well-written +- [ ] **Step 3 of the task spec not completed: the four `FLAG_*` constants have no `///` doc.** +- [ ] **Step 4 of the task spec not completed: the four `is_*` / `has_*` accessor methods have no `///` doc.** +- [ ] **`WIRE_SIZE`, `VERSION`, `write_to`, `read_from` also lack `///` doc** — the spec phrased "Done when" as "All public items on `MediaHeaderV2` carry `///` doc comments", which means all of these qualify. +- [ ] Second `Verify` command (`cargo clippy ... -W missing_docs`) was skipped — that command would have caught the gaps. The first command (`cargo doc | grep missing`) returned empty only because `missing_docs` is not currently a crate-level deny. - [ ] Approved + +### Reviewer notes (2026-05-11) — Changes Requested + +The 9 field docs are good and stay. What's missing: + +**1. Constants on `impl MediaHeaderV2`** (lines 187, 188, 231–234 in current `packet.rs`): +- `WIRE_SIZE` +- `VERSION` +- `FLAG_REPAIR` +- `FLAG_QUALITY` +- `FLAG_KEYFRAME` +- `FLAG_FRAME_END` + +**2. Methods on `impl MediaHeaderV2`** (lines 190, 202, 236+): +- `write_to` +- `read_from` (note: returns `None` on short buffer or wrong version) +- `is_repair` +- `has_quality` +- `is_keyframe` +- `is_frame_end` + +One short `///` line per item is sufficient. For the `FLAG_*` consts, paraphrase what each bit means (e.g. `/// Bit 7: set when this packet is an FEC repair packet, not source media.`). + +**Re-verify with both commands the task spec lists**, especially the clippy one: + +```bash +cargo doc -p wzp-proto --no-deps 2>&1 | grep -i "missing" || echo "no missing-doc warnings" +cargo clippy -p wzp-proto --all-targets -- -D warnings -W missing_docs 2>&1 | grep "packet.rs:" | head -30 +``` + +The clippy output should not list any line inside `impl MediaHeaderV2` (lines 186–250-ish in current packet.rs). Note: the rest of `wzp-proto` has many pre-existing missing-doc gaps — those are not in scope. Filter your output to `packet.rs:1[6-9][0-9]` to see only the v2 region. + +**Append a "Rework" section to this report** rather than creating a new file (see `reports/README.md`). Move the status back to `Pending Review` when re-submitted. + +**Process note for future tasks:** running every `Verify` command listed in the task is mandatory. If a command produces output you don't understand, file a `Blocked` report instead of skipping it. The agreement is: spec says do X and verify with Y; skipping Y is the same as not having done X for review purposes. + +## Rework (2026-05-11) + +Addressed reviewer feedback: + +- Added `///` doc comments to all 6 constants in `impl MediaHeaderV2`: + - `WIRE_SIZE`, `VERSION`, `FLAG_REPAIR`, `FLAG_QUALITY`, `FLAG_KEYFRAME`, `FLAG_FRAME_END` +- Added `///` doc comments to all 6 methods in `impl MediaHeaderV2`: + - `write_to`, `read_from`, `is_repair`, `has_quality`, `is_keyframe`, `is_frame_end` +- Re-ran both Verify commands from the task spec: + - `cargo doc -p wzp-proto --no-deps` → no missing-doc warnings + - `cargo clippy -p wzp-proto --all-targets -- -D warnings -W missing_docs` → no `packet.rs:1[6-9][0-9]` hits (the only missing-doc lines are pre-existing gaps in the 1189–1245 range, outside `MediaHeaderV2`) + +**Status moved back to Pending Review.**