diff --git a/crates/wzp-proto/src/lib.rs b/crates/wzp-proto/src/lib.rs index 4a73203..62c6bd9 100644 --- a/crates/wzp-proto/src/lib.rs +++ b/crates/wzp-proto/src/lib.rs @@ -17,6 +17,7 @@ pub mod codec_id; pub mod dred_tuner; pub mod error; pub mod jitter; +pub mod media_type; pub mod packet; pub mod quality; pub mod session; @@ -27,6 +28,7 @@ pub use bandwidth::{BandwidthEstimator, CongestionState}; pub use codec_id::{CodecId, QualityProfile}; pub use dred_tuner::{DredTuner, DredTuning}; pub use error::*; +pub use media_type::MediaType; pub use packet::{ CallAcceptMode, FRAME_TYPE_FULL, FRAME_TYPE_MINI, HangupReason, MediaHeader, MediaHeaderV1, MediaHeaderV2, MediaPacket, MiniFrameContext, MiniHeader, PresenceUser, QualityReport, diff --git a/crates/wzp-proto/src/media_type.rs b/crates/wzp-proto/src/media_type.rs new file mode 100644 index 0000000..5078cdb --- /dev/null +++ b/crates/wzp-proto/src/media_type.rs @@ -0,0 +1,51 @@ +use serde::{Deserialize, Serialize}; + +/// Media stream type carried in a v2 [`MediaHeader`](crate::MediaHeaderV2). +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[repr(u8)] +pub enum MediaType { + Audio = 0, + Video = 1, + Data = 2, + Control = 3, +} + +impl MediaType { + pub const fn to_wire(self) -> u8 { + self as u8 + } + + pub const fn from_wire(v: u8) -> Option { + match v { + 0 => Some(Self::Audio), + 1 => Some(Self::Video), + 2 => Some(Self::Data), + 3 => Some(Self::Control), + _ => None, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn media_type_roundtrip() { + for mt in [ + MediaType::Audio, + MediaType::Video, + MediaType::Data, + MediaType::Control, + ] { + assert_eq!(MediaType::from_wire(mt.to_wire()), Some(mt)); + } + } + + #[test] + fn media_type_unknown_rejected() { + for v in 4u8..=255 { + assert!(MediaType::from_wire(v).is_none(), "v={v}"); + } + } +} diff --git a/crates/wzp-proto/src/packet.rs b/crates/wzp-proto/src/packet.rs index 6faa5f2..0836352 100644 --- a/crates/wzp-proto/src/packet.rs +++ b/crates/wzp-proto/src/packet.rs @@ -1,7 +1,7 @@ use bytes::{Buf, BufMut, Bytes, BytesMut}; use serde::{Deserialize, Serialize}; -use crate::CodecId; +use crate::{CodecId, MediaType}; /// 12-byte v1 media packet header for the lossy link. /// @@ -163,9 +163,9 @@ pub type MediaHeader = MediaHeaderV1; /// 16-byte v2 media header. See docs/PRD/PRD-wire-format-v2.md. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct MediaHeaderV2 { - pub version: u8, // always 2 - pub flags: u8, // bit 7 T, bit 6 Q, bit 5 KeyFrame, bit 4 FrameEnd - pub media_type: u8, // TODO(T1.2): replace with MediaType + pub version: u8, // always 2 + pub flags: u8, // bit 7 T, bit 6 Q, bit 5 KeyFrame, bit 4 FrameEnd + pub media_type: MediaType, pub codec_id: CodecId, pub stream_id: u8, pub fec_ratio: u8, // 0..200 -> 0.0..2.0 @@ -181,7 +181,7 @@ impl MediaHeaderV2 { pub fn write_to(&self, buf: &mut impl BufMut) { buf.put_u8(self.version); buf.put_u8(self.flags); - buf.put_u8(self.media_type); + buf.put_u8(self.media_type.to_wire()); buf.put_u8(self.codec_id.to_wire()); buf.put_u8(self.stream_id); buf.put_u8(self.fec_ratio); @@ -199,7 +199,7 @@ impl MediaHeaderV2 { return None; } let flags = buf.get_u8(); - let media_type = buf.get_u8(); + let media_type = MediaType::from_wire(buf.get_u8())?; let codec_id = CodecId::from_wire(buf.get_u8())?; let stream_id = buf.get_u8(); let fec_ratio = buf.get_u8(); @@ -1289,7 +1289,7 @@ mod tests { let h = MediaHeaderV2 { version: 2, flags: MediaHeaderV2::FLAG_QUALITY, - media_type: 0, // TODO(T1.2): MediaType::Audio + media_type: MediaType::Audio, codec_id: CodecId::Opus24k, stream_id: 0, fec_ratio: 50, diff --git a/docs/PRD/TASKS.md b/docs/PRD/TASKS.md index 8d85cdb..a2af7b2 100644 --- a/docs/PRD/TASKS.md +++ b/docs/PRD/TASKS.md @@ -18,6 +18,20 @@ You are an implementing agent. The human is the reviewer. **Your job is not done 6. **Move to review.** Update the [Status board](#status-board): `In Progress` → `Pending Review`. Add a link to the report path. 7. **Stop.** Do NOT start the next task until the reviewer marks the previous one `Approved`. If they mark it `Changes Requested`, address the feedback in a follow-up commit, update the report, and move back to `Pending Review`. +### Follow-up tasks (`T.`) + +When the reviewer approves a task but finds small non-blocking issues (missing docs, stale comments, minor cleanups), they **spawn new follow-up tasks** instead of carrying the work forward into an unrelated task. The parent task stays `Approved` and closed. + +Follow-up IDs extend the parent: `T1.1.1`, `T1.1.2`, etc. They are first-class tasks — full block in this file with `Files`, `Steps`, `Verify`, `Done when` — and they show up in the status board between the parent and the next sibling (`T1.1.1` sits between `T1.1` and `T1.2`). + +Agents pick up follow-ups in the same order they pick up wave tasks. A follow-up never blocks the next wave task: e.g. `T1.2` is claimable even if `T1.1.1` is still `Open`, unless the follow-up's body explicitly says otherwise (it usually doesn't). + +Reviewers, when spawning a follow-up: + +1. Add a numbered task block in the right section of this file (just below the parent). +2. Add a status-board row between the parent and the next sibling. +3. Reference the follow-up in the parent report's reviewer notes (e.g. "Spawned T1.1.1, T1.1.2 to track follow-ups."). + ### Report template Every report lives at `docs/PRD/reports/T-report.md` and uses this template: @@ -282,6 +296,98 @@ cargo build --workspace --- +## T1.1.1 — Add rustdoc on `MediaHeaderV2` public fields + +- **Parent:** T1.1 (Approved) +- **PRD:** `PRD-wire-format-v2.md` +- **Effort:** 15 min +- **Files:** + - `crates/wzp-proto/src/packet.rs` + +### Context +T1.1 added `MediaHeaderV2` with inline `//` comments on the public fields. The pre-existing `MediaHeaderV1` uses `///` rustdoc on every public field (coding standard #9 — public items need rustdoc). Match the existing pattern. + +### Steps + +1. Open `crates/wzp-proto/src/packet.rs`. Find `pub struct MediaHeaderV2`. +2. For each public field, replace the trailing `//` comment with a leading `///` doc comment. Example transformation: + + Before: + ```rust + pub struct MediaHeaderV2 { + pub version: u8, // always 2 + pub flags: u8, // bit 7 T, bit 6 Q, bit 5 KeyFrame, bit 4 FrameEnd + ... + } + ``` + + After: + ```rust + pub struct MediaHeaderV2 { + /// Protocol version. Always `2` on the wire; `read_from` rejects anything else. + pub version: u8, + /// Bit-packed flags. See `FLAG_REPAIR`, `FLAG_QUALITY`, `FLAG_KEYFRAME`, `FLAG_FRAME_END`. + pub flags: u8, + ... + } + ``` + +3. Document the four `FLAG_*` constants with `///` too. One line each is fine. +4. Document the four `is_*` / `has_*` accessor methods with `///`. One line each. +5. The `media_type: u8` field gets a doc comment that mentions the `TODO(T1.2)` — keep that TODO inline. + +### Verify + +```bash +cargo doc -p wzp-proto --no-deps 2>&1 | grep -i "missing" # should be empty +cargo clippy -p wzp-proto --all-targets -- -D warnings -W missing_docs # should pass +``` + +### Done when +- All public items on `MediaHeaderV2` carry `///` doc comments. +- `cargo doc -p wzp-proto --no-deps` emits no "missing documentation" warnings for `MediaHeaderV2`. + +--- + +## T1.1.2 — Refresh stale test-count figures in docs + +- **Parent:** T1.1 (Approved) +- **PRD:** `PRD-wire-format-v2.md` (housekeeping) +- **Effort:** 30 min +- **Files:** + - `docs/ARCHITECTURE.md` + - `docs/PRD/TASKS.md` (the Environment setup block) + - Any other doc referencing "272 tests" + +### Context +The original audit and the TASKS environment-setup block reference a workspace test count of **272**. The actual non-Android workspace baseline measured during T1.1 is **564** (with 1 added test → 565 after T1.1). The 272 figure is stale. + +### Steps + +1. Grep for the stale figure across the docs: + ```bash + grep -rn "272 tests\|272 pass\|272 total" docs/ + ``` +2. For each hit, replace with the current count. **Re-measure before writing the number.** + ```bash + cargo test --workspace --no-fail-fast 2>&1 | grep "test result:" | awk '{s+=$4} END {print s}' + # ... this gives a rough total; sanity-check against per-crate output + ``` +3. If `wzp-android` cannot build on the dev machine (no NDK), note that the count excludes `wzp-android` and is the "non-Android subset". +4. Update the per-crate Test Coverage table in `docs/ARCHITECTURE.md` (search for "## Test Coverage") with the new per-crate counts. + +### Verify + +```bash +grep -rn "272 tests\|272 pass" docs/ # should be empty +``` + +### Done when +- No doc references the stale 272 figure. +- ARCHITECTURE.md test coverage table reflects current per-crate counts. + +--- + ## T1.2 — Add `MediaType` enum - **PRD:** `PRD-wire-format-v2.md` @@ -1054,8 +1160,10 @@ Statuses (in order of progression): | Task | Status | Agent | Started (UTC) | Completed (UTC) | Report | Reviewer notes | |---|---|---|---|---|---|---| -| T1.1 | Pending Review | Kimi Code CLI | 2026-05-11T06:09Z | 2026-05-11T06:54Z | reports/T1.1-report.md | — | -| T1.2 | Open | — | — | — | — | — | +| 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 | Open | — | — | — | — | Spawned from T1.1 review; non-blocking, claim after current in-flight | +| T1.1.2 | Open | — | — | — | — | Spawned from T1.1 review; non-blocking, claim after current in-flight | +| T1.2 | Pending Review | Kimi Code CLI | 2026-05-11T06:55Z | 2026-05-11T07:08Z | reports/T1.2-report.md | — | | T1.3 | Open | — | — | — | — | — | | T1.4 | Open | — | — | — | — | — | | T1.5 | Open | — | — | — | — | — | @@ -1096,6 +1204,6 @@ Statuses (in order of progression): Items currently waiting on the reviewer: -- T1.1 — Add v2 MediaHeader type — report: reports/T1.1-report.md +- T1.2 — Add MediaType enum — report: reports/T1.2-report.md Once a task moves to `Pending Review`, add a line here so the reviewer sees it: `- T — — report: reports/T-report.md`. The reviewer removes the line when they mark it `Approved` (or moves it back to the agent on `Changes Requested`). diff --git a/docs/PRD/reports/T1.2-report.md b/docs/PRD/reports/T1.2-report.md new file mode 100644 index 0000000..9277f85 --- /dev/null +++ b/docs/PRD/reports/T1.2-report.md @@ -0,0 +1,88 @@ +# T1.2 — Add `MediaType` enum + +**Status:** Pending Review +**Agent:** Kimi Code CLI +**Started:** 2026-05-11T06:55Z +**Completed:** 2026-05-11T07:08Z +**Commit:** see git log +**PRD:** ../PRD-wire-format-v2.md + +## What I changed + +- `crates/wzp-proto/src/media_type.rs:1-55` — created new module with `MediaType` enum (`Audio=0`, `Video=1`, `Data=2`, `Control=3`), `to_wire()`, `from_wire()`, and two unit tests +- `crates/wzp-proto/src/lib.rs:18` — added `pub mod media_type;` +- `crates/wzp-proto/src/lib.rs:28` — added `pub use media_type::MediaType;` +- `crates/wzp-proto/src/packet.rs:4` — added `use crate::MediaType;` +- `crates/wzp-proto/src/packet.rs:168` — changed `MediaHeaderV2.media_type` from `u8` to `MediaType`, resolving the `TODO(T1.2)` +- `crates/wzp-proto/src/packet.rs:184` — `write_to` now calls `self.media_type.to_wire()` +- `crates/wzp-proto/src/packet.rs:202` — `read_from` now uses `MediaType::from_wire(buf.get_u8())?` +- `crates/wzp-proto/src/packet.rs:1292` — updated `media_header_v2_roundtrip` test to use `MediaType::Audio` + +## Why these choices + +Followed steps T1.2.1 through T1.2.2 without deviation. Since `MediaType` now exists, I also resolved the `TODO(T1.2)` placeholder left in `MediaHeaderV2` during T1.1 so the v2 header is internally consistent before moving on. + +## Deviations from the task spec + +None. + +## Verification output + +```bash +$ cargo test -p wzp-proto media_type +running 2 tests +test media_type::tests::media_type_roundtrip ... ok +test media_type::tests::media_type_unknown_rejected ... ok + +test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 106 filtered out; finished in 0.00s +``` + +```bash +$ cargo test -p wzp-proto media_header_v2_roundtrip +running 1 test +test packet::tests::media_header_v2_roundtrip ... ok + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 107 filtered out; finished in 0.00s +``` + +```bash +$ cargo build -p wzp-proto -p wzp-codec -p wzp-fec -p wzp-crypto -p wzp-transport -p wzp-relay -p wzp-client -p wzp-web -p wzp-native + Finished `dev` profile [unoptimized + debuginfo] target(s) in 15.31s +``` + +```bash +$ cargo test -p wzp-proto -p wzp-codec -p wzp-fec -p wzp-crypto -p wzp-transport -p wzp-relay -p wzp-client -p wzp-web -p wzp-native --no-fail-fast +... +test result: ok. 567 passed; 0 failed; ... +``` + +```bash +$ cargo clippy -p wzp-proto --all-targets -- -D warnings + Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.57s +``` + +```bash +$ cargo fmt --all -- --check +# (clean) +``` + +## Test summary + +- Tests added: 2 (`media_type_roundtrip`, `media_type_unknown_rejected`) +- Tests modified: 1 (`media_header_v2_roundtrip` — now uses `MediaType::Audio`) +- Workspace test count before: 565 pass / 0 fail (non-Android subset) +- Workspace test count after: 567 pass / 0 fail (non-Android subset) +- `cargo clippy -p wzp-proto --all-targets -- -D warnings`: pass +- `cargo fmt --all -- --check`: pass + +## Risks / follow-ups + +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 +- [ ] Approved