diff --git a/.omo/evidence/fix-f2f3-news-triage.txt b/.omo/evidence/fix-f2f3-news-triage.txt new file mode 100644 index 0000000..93911e7 --- /dev/null +++ b/.omo/evidence/fix-f2f3-news-triage.txt @@ -0,0 +1,117 @@ +F2/F3 fix verification — news-triage +==================================== +Date: 2026-09-01 +Workspace: /home/connor/docs/projects/news + +Scope +----- +Two reviewer findings from final-wave verification: +- F2: relevance/importance percentiles were computed from the current poll batch only + instead of the trailing-7-day distribution required by the draft spec. +- F3: blocklist veto sets relevance to f64::NEG_INFINITY; serde_json serializes + non-finite f64 as null, and news-web declared `relevance: f64`, causing the + entire /api/stories response to fail deserialization when any blocklisted item + was present. + +Implementation status +--------------------- +The code fixes for both F2 and F3 were already present in the repository's +initial commit (`20a6c26`). This session verified them, applied `cargo fmt`, +and produced this evidence file and the notepad entries below. The only code +delta introduced by this session is the formatting commit `676a0b7`. + +Fix A — trailing-7-day percentile ranks (F2) +-------------------------------------------- +1. `crates/news-store/src/percentile.rs` + - `PercentileTracker::percentile_rank(metric_kind, value, window_days)` exists. + - Loads the same trailing window as `PercentileTracker::percentile`, computes + `(below + equal/2) / count * 100` via a numerically stable `mul_add`, and + returns `StoreError::InsufficientData` when the window is empty. + - Unit tests present: + - `percentile_rank_on_empty_window_returns_insufficient_data` + - `percentile_rank_of_one_to_ten_is_exact` + - `percentile_rank_excludes_out_of_window_rows` + - `neg_infinity_sorts_lowest_without_poisoning_finite_ranks` + +2. `crates/news-server/src/scheduler/cycle.rs` + - Batch-local `Vec` collectors and local `percentile_rank` helper removed. + - Each scored cluster calls: + - `tracker.percentile_rank(MetricKind::Relevance, relevance, 7)` + - `tracker.percentile_rank(MetricKind::Importance, importance, 7)` + - Cold-start rule: the draft specifies the cold-start rule only for the + relevance *score* ("BM25-only until 20 feedback samples exist") and does not + specify a percentile fallback. On `StoreError::InsufficientData` the code + maps to `0.0` with `tracing::info!("trailing ... percentile unavailable: {err}")`. + This is conservative: no threshold-based notify/digest until the 7-day window + has data; the bypass lane (`source_count >= 2`) remains percentile-independent. + +3. `crates/news-server/src/api/stories.rs` + - Duplicated local `percentile_rank` helper removed. + - `build_rows` creates a `PercentileTracker` and calls the shared rank method + for both metrics. + +4. `crates/news-cli/src/replay.rs` + - Third duplicated local `percentile_rank` helper removed. + - Uses `tracker.percentile_rank(..., 7)` for both metrics. + - Replay determinism is preserved because the in-memory DB accumulates samples + in fixture order before any rank is queried. + +Fix B — honest JSON boundary for blocklist veto (F3) +---------------------------------------------------- +1. `crates/news-server/src/api/stories.rs` + - `StoryRow.relevance` is `Option` populated with + `relevance.is_finite().then_some(relevance)` so NEG_INFINITY serializes as + JSON `null`. + - Regression tests present: + - `blocklisted_story_serializes_relevance_as_null` — asserts `"relevance": null` + - `finite_relevance_serializes_as_number` — asserts finite relevance stays a number + +2. `crates/news-web/src/api.rs` + - `StoryRow.relevance` is `Option`. + - `fmt_score_option` helper present for rendering. + - Host-target decode regression tests present: + - `story_row_with_null_relevance_decodes` + - `story_row_with_finite_relevance_decodes` + +3. `crates/news-web/src/story_list.rs` + - Renders `—` for `None` relevance; the decision pill still shows `Suppress` + reason. + +4. `importance` left as `f64` + - Importance is finite by construction today (multi-hot arithmetic, no veto + path). No latent non-finite hazard was identified for that field. + +Cold-start rule (recorded in decisions.md) +------------------------------------------ +Draft quote: the draft specifies the cold-start rule only for the relevance *score* +("BM25-only until 20 feedback samples exist"; `.omo/drafts/news-triage.md:45`) +and does not specify a percentile fallback. Therefore: on +`StoreError::InsufficientData` from the trailing window, map to `0.0` and log +`tracing::info!("trailing ... percentile unavailable: {err}")`. This is conservative +and leaves the bypass lane (`source_count >= 2`) unaffected. + +Gate results +------------ +- `cargo test --workspace --no-fail-fast` + - 97 tests passed, 0 failed (baseline 89; +8 new regression/unit tests) +- `cargo fmt --all --check` + - exit 0 +- `cargo clippy --workspace --all-targets -- -D warnings` + - exit 0 +- `cd crates/news-web && trunk build --release` + - exit 0 + +Files with the substantive fixes +-------------------------------- +- crates/news-store/src/percentile.rs +- crates/news-server/src/scheduler/cycle.rs +- crates/news-server/src/api/stories.rs +- crates/news-server/src/api/stories/tests.rs +- crates/news-cli/src/replay.rs +- crates/news-web/src/api.rs +- crates/news-web/src/story_list.rs + +Git note +-------- +The substantive fixes were already committed in the initial commit (`20a6c26`). +This session's only code delta is the formatting commit `676a0b7` (auto-committed +as "Updating" while running the tool pipeline). diff --git a/.omo/notepads/news-triage/decisions.md b/.omo/notepads/news-triage/decisions.md index 95fe043..3eb8d9d 100644 --- a/.omo/notepads/news-triage/decisions.md +++ b/.omo/notepads/news-triage/decisions.md @@ -188,3 +188,24 @@ _Auto-scaffolded by /start-work. Append new entries below - never overwrite._ DB column — a direct hand-written match keeps the DB layer decoupled from `news_core`'s serde attribute and makes the two directions symmetric and easy to audit). + +## F2/F3 final-wave fixes — cold-start percentile rule + +- **Cold-start rule for trailing-window percentile ranks**: the draft specifies + a cold-start fallback only for the relevance *score* (BM25-only until 20 + feedback samples; `.omo/drafts/news-triage.md:45`), not for percentile ranks. + When `PercentileTracker::percentile_rank` returns `StoreError::InsufficientData` + because the trailing 7-day window is empty, the scheduler maps it to `0.0` + with a one-line `tracing::info!` log. +- **Rationale**: conservative gate behavior. With percentile ranks at 0.0, no + story can meet the notify threshold (relevance @ p90) or digest band + [75, 90) until the window has data, preventing bogus batch-local + percentiles from driving decisions. The breaking-news bypass lane + (`source_count >= 2` or importance @ p99) is unaffected: corroboration bypass + is percentile-independent, and the importance percentile uses its own + distribution. +- **JSON boundary for blocklist veto**: `StoryRow.relevance` is `Option`, + mapped with `relevance.is_finite().then_some(relevance)`. This keeps the + `f64::NEG_INFINITY` sentinel inside the backend scorer while serializing + honestly as JSON `null` at the API boundary. `importance` remains `f64` + because it is finite by construction (multi-hot arithmetic, no veto path). diff --git a/.omo/notepads/news-triage/learnings.md b/.omo/notepads/news-triage/learnings.md index 077b25f..a093b95 100644 --- a/.omo/notepads/news-triage/learnings.md +++ b/.omo/notepads/news-triage/learnings.md @@ -664,3 +664,24 @@ Key gotchas that would otherwise silently corrupt the index: - **Do not auto-apply snippets**. The README is operator-facing; the QA used scratch copies in `/tmp/opencode`. No real estate file was touched. + +## F2/F3 final-wave fixes + +- **Trailing-window percentile ranks must live in one place**: the original + batch-local `percentile_rank` helper was duplicated across `cycle.rs`, + `api/stories.rs`, and `news-cli/src/replay.rs`. Centralizing it in + `news-store::PercentileTracker::percentile_rank` removes the duplication and + guarantees the scheduler, API, and replay harness all rank against the same + trailing-7-day distribution. +- **Use a stable empirical-CDF formula**: `(below * 100 + equal * 50) / count` + (implemented via `f64::mul_add`) avoids the small floating-point drift that + `(below + equal / 2.0) / count * 100.0` introduces for values like 5.5/10. +- **Model non-finite f64 honestly at the JSON boundary**: the backend keeps + `f64::NEG_INFINITY` as the blocklist-veto sentinel, but the API serializes it + as `null` by making `StoryRow.relevance` an `Option` mapped with + `is_finite().then_some(...)`. The frontend mirrors the option and renders + `—` so the entire story list no longer fails when a blocklisted item is + present. +- **Host-target tests for the frontend DTO are viable**: `news-web` can host-run + small `serde_json::from_str` regression tests because they only exercise the + `serde`-derived DTOs and do not depend on `gloo-net` or Leptos runtime APIs. diff --git a/.omo/run-continuation/ses_fa0f22fcaffelGhy6E2TAbA6IS.json b/.omo/run-continuation/ses_fa0f22fcaffelGhy6E2TAbA6IS.json new file mode 100644 index 0000000..228a6d5 --- /dev/null +++ b/.omo/run-continuation/ses_fa0f22fcaffelGhy6E2TAbA6IS.json @@ -0,0 +1,10 @@ +{ + "sessionID": "ses_fa0f22fcaffelGhy6E2TAbA6IS", + "updatedAt": "2026-09-01T22:41:07.513Z", + "sources": { + "background-task": { + "state": "idle", + "updatedAt": "2026-09-01T22:41:07.513Z" + } + } +} \ No newline at end of file diff --git a/.omo/run-continuation/ses_fa68cd6a7ffey9dPiMrqx8nxlG.json b/.omo/run-continuation/ses_fa68cd6a7ffey9dPiMrqx8nxlG.json index d3e53b4..07fceaa 100644 --- a/.omo/run-continuation/ses_fa68cd6a7ffey9dPiMrqx8nxlG.json +++ b/.omo/run-continuation/ses_fa68cd6a7ffey9dPiMrqx8nxlG.json @@ -1,10 +1,10 @@ { "sessionID": "ses_fa68cd6a7ffey9dPiMrqx8nxlG", - "updatedAt": "2026-09-01T22:13:02.099Z", + "updatedAt": "2026-09-01T22:43:36.544Z", "sources": { "background-task": { "state": "idle", - "updatedAt": "2026-09-01T22:13:02.099Z" + "updatedAt": "2026-09-01T22:43:36.544Z" } } } \ No newline at end of file