212 lines
12 KiB
Markdown
212 lines
12 KiB
Markdown
# Decisions — news-triage
|
|
|
|
Architectural choices and rationales discovered during work on this plan.
|
|
|
|
_Auto-scaffolded by /start-work. Append new entries below - never overwrite._
|
|
|
|
---
|
|
|
|
## Todo 2 — serde discriminant wire format
|
|
|
|
- **All enum discriminants use `#[serde(rename_all = "snake_case")]`**, not
|
|
derive-default `PascalCase`. Applies to `SourceKind` (`rss_atom`,
|
|
`news_sitemap`), `ItemKind` (`news`, `opinion`), `FeedbackKind`
|
|
(`interested`, `not_interested`). This is deliberate and locked by a test
|
|
(`opinion_item_serializes_kind_as_snake_case_discriminant` in
|
|
`news-core/src/model.rs`) — the plan's QA section explicitly requires
|
|
`kind: ItemKind::Opinion` to serialize as `"opinion"`.
|
|
- **Downstream todos matching on these discriminants in SQL or JSON must
|
|
use the snake_case string form**: `"rss_atom"`, `"news_sitemap"`,
|
|
`"news"`, `"opinion"`, `"interested"`, `"not_interested"`. This affects
|
|
todo 8 (source registry storage), any SQLite column that stores
|
|
`kind` as TEXT, and any frontend/JSON consumer matching on the
|
|
discriminant.
|
|
|
|
## Todo 4 — NaiveBayesModel Laplace-smoothing formula (locked, downstream contract)
|
|
|
|
- **Exact formula** used by `NaiveBayesModel::score(tokens)` in
|
|
`crates/news-core/src/scoring/relevance.rs` (per-token, summed across
|
|
`tokens`):
|
|
|
|
```text
|
|
score(tokens) = sum over token in tokens of:
|
|
ln((count_interested(token) + 1) / (total_interested + 2))
|
|
- ln((count_not_interested(token) + 1) / (total_not_interested + 2))
|
|
```
|
|
|
|
Additive (Laplace) smoothing with `+1` on counts and `+2` on the class
|
|
totals avoids `ln(0)`. A token never seen in training still contributes
|
|
a nonzero term derived purely from `total_interested`/`total_not_interested`
|
|
(not `0.0`) — only the cold-start gate in `relevance_score` produces an
|
|
exact `0.0` Bayes contribution.
|
|
- **Cold-start gate**: `relevance_score(bm25, bayes, tokens)` returns `bm25`
|
|
bit-identically (Bayes term is exactly `0.0`, not merely small) whenever
|
|
`bayes.total_interested + bayes.total_not_interested < 20`; at `>= 20`
|
|
total samples it returns `bm25 + bayes.score(tokens)`. Locked by
|
|
`cold_start_boundary_at_20_samples` (19 vs 20 exact boundary).
|
|
- **`score()` CAN return negative values** — this happens when a token's
|
|
training history skews toward `NotInterested` (see
|
|
`score_is_negative_when_token_skews_not_interested`). This is a
|
|
legitimate, finite relevance signal, distinct from and must NEVER be
|
|
confused with `f64::NEG_INFINITY`, which todo 10's blocklist-veto logic
|
|
reserves as a separate hard-suppression sentinel. Callers combining the
|
|
two (todo 10, todo 13) must check for the veto sentinel explicitly
|
|
(e.g. `is_infinite() && is_sign_negative()`) rather than assuming "any
|
|
negative relevance score" means "blocked".
|
|
- No persistence in `news-core`: `NaiveBayesModel`'s fields
|
|
(`feature_counts`, `total_interested`, `total_not_interested`) are
|
|
private; `news-store` (a later todo) owns serializing/loading this state
|
|
to/from the `bayes_model` table (see plan todo 5).
|
|
|
|
## Todo 3 — importance scoring formula
|
|
|
|
- **Formula (additive, unweighted sum of three terms)**, implemented in
|
|
`crates/news-core/src/scoring/importance.rs`:
|
|
```
|
|
importance_score(prominence_rank, source_trust_weight, corroboration_count, _window)
|
|
= prominence_term + trust_term + corroboration_term
|
|
where
|
|
prominence_term = 1.0 / (1.0 + prominence_rank)
|
|
trust_term = source_trust_weight (assumed in [0.0, 1.0])
|
|
corroboration_term = 1.0 - 1.0 / (1.0 + corroboration_count) (saturating, 0 -> 0.0, asymptotic -> 1.0)
|
|
```
|
|
Worked example (asserted by test `worked_example_matches_doc_comment`):
|
|
`importance_score(0, 0.8, 3, 45) == 2.55` (`1.0 + 0.8 + 0.75`).
|
|
- **Numeric range**: with `source_trust_weight` in `[0.0, 1.0]` (the
|
|
assumed/documented range for `Source.weight`), the score ranges from
|
|
`0.0` (rank -> infinity, trust 0, zero corroboration) up toward but never
|
|
reaching `3.0` (rank 0 gives prominence_term 1.0, trust 1.0, corroboration
|
|
approaching but never reaching 1.0 as corroboration_count grows). Todo 12
|
|
(notification gate, importance percentile >= 99th) should treat this as
|
|
an open-ended-but-bounded-near-3.0 range, not a normalized `[0,1]` score —
|
|
percentile ranking (not a fixed numeric threshold) is what the plan
|
|
specifies, so the exact ceiling doesn't need to be a round number.
|
|
- **`corroboration_window_minutes` is accepted but NOT used in the
|
|
formula** — it's a pure-function parameter for API completeness/future
|
|
use. Window enforcement (deciding which raw items count toward
|
|
`corroboration_count` in the first place, e.g. "sources within 45
|
|
minutes") is the CALLER's responsibility in news-store's `ClusterRepo`
|
|
(todo 9). `importance_score` trusts the `corroboration_count` it's given
|
|
and does not re-derive timing. Todo 11 (importance scorer wiring) must
|
|
compute `corroboration_count` correctly at the call site before invoking
|
|
this function — passing an un-windowed raw source count would be wrong,
|
|
but that bug would be in the caller, not in this pure function.
|
|
- **Saturation curve is `1.0 - 1.0/(1.0 + n)`** (same family as the
|
|
prominence term, inverted) rather than e.g. `log(1+n)` or `tanh(n)` —
|
|
chosen for symmetry with the prominence term and because it has a clean
|
|
worked-example fraction (`3` sources -> `0.75`, easy to hand-verify).
|
|
Marginal gain shrinks monotonically: 0->1 adds 0.5, 1->2 adds ~0.167,
|
|
2->3 adds 0.083, 9->10 adds ~0.009 — confirmed by test
|
|
`corroboration_saturates_so_tenth_source_adds_less_than_third`.
|
|
- Six tests total (4 required by the plan + 2 extra: the worked-example
|
|
doc-comment assertion and an explicit saturation-margin test), all in
|
|
`crates/news-core/src/scoring/importance.rs`'s inline `#[cfg(test)] mod
|
|
tests`, using plain `assert!`/`assert_eq!` (no `.unwrap()`/`.expect()`),
|
|
per todo 2's established pattern.
|
|
- Ran into a transient race with the parallel todo 4 agent: it added `pub
|
|
mod relevance;` to `crates/news-core/src/scoring/mod.rs` (shared file)
|
|
slightly before creating `scoring/relevance.rs`, causing a momentary
|
|
`E0583 file not found for module` compile error. Resolved itself ~15s
|
|
later once the other agent finished writing `relevance.rs` — not a bug in
|
|
this todo's work, just a heads-up for future same-file parallel edits.
|
|
|
|
## Todo 5 — news-store SQLite schema (locked, downstream contract for 6/8/9/11)
|
|
|
|
- **All ids are `TEXT` primary keys** storing `Uuid::to_string()`
|
|
(hyphenated lowercase form, sqlx's default `Display` for `uuid::Uuid`).
|
|
No `BLOB`/16-byte-uuid storage — plain text, matching runway's
|
|
convention (see `stamp`/`parse_stamp` note below) and keeping the schema
|
|
greppable with the sqlite3 CLI.
|
|
- **All timestamps are `TEXT`** storing RFC3339 (`DateTime<Utc>::to_rfc3339()`
|
|
or equivalent). No SQLite `DATETIME`/`INTEGER` epoch columns — every
|
|
`*_at` column (`published_at`, `first_seen_at`, `last_seen_at`,
|
|
`created_at`, `computed_at`) uses this same TEXT/RFC3339 representation.
|
|
Todo 6/8/9's repositories must parse with `DateTime::parse_from_rfc3339`
|
|
(mirrors runway's `db::parse_stamp` helper) rather than inventing a
|
|
second timestamp format.
|
|
- **Enum-like columns (`kind` on `sources`/`raw_items`, `lane` on
|
|
`notification_log`, `kind` on `feedback`) are `TEXT`** storing the
|
|
snake_case discriminant strings locked by todo 2's decision above
|
|
(`"rss_atom"`, `"news_sitemap"`, `"news"`, `"opinion"`, `"interested"`,
|
|
`"not_interested"`) — no separate `CHECK (kind IN (...))` constraint was
|
|
added in this todo (kept out of scope; a later todo can add one without
|
|
a schema-shape change since the column stays `TEXT`).
|
|
- **Booleans are `INTEGER`** (`sources.enabled`, `notification_log.dry_run`)
|
|
— SQLite has no native boolean type; sqlx maps `bool` to `INTEGER`
|
|
(0/1) transparently via the `sqlite` feature, so repositories bind/read
|
|
plain `bool` values without manual `i64` conversion.
|
|
- **`bayes_model` is a singleton row** (`id INTEGER PRIMARY KEY CHECK (id
|
|
= 1)`, one column `serialized_json TEXT`) holding the entire
|
|
`NaiveBayesModel` (todo 4) as one JSON blob via `serde_json`. Todo 4's
|
|
`feature_counts`/`total_interested`/`total_not_interested` fields are
|
|
private but the type derives `Serialize`/`Deserialize` per todo 4's
|
|
notepad — todo 6/11's bayes-model repo should `UPSERT` (`INSERT ...
|
|
ON CONFLICT(id) DO UPDATE`) this single row rather than
|
|
delete-then-insert.
|
|
- **`StoreError` added an `InsufficientData { needed: u32, available: u32
|
|
}` variant now** (forward-looking, unused until the percentile tracker
|
|
todo), documented in its doc comment as unused — todo 11 should reuse
|
|
this variant rather than adding a second "not enough data" error type.
|
|
- **sqlx pinned to the `0.8` line, not runway's `0.9`**: this workspace's
|
|
`rust-toolchain.toml` is pinned to `1.89.0`, but `sqlx 0.9.0` requires
|
|
rustc `1.94.0` (confirmed via `cargo test` failing with an explicit
|
|
MSRV error naming `sqlx@0.9.0`/`sqlx-core@0.9.0`/etc.). `sqlx = "0.8"`
|
|
(resolved to `0.8.6`) has no such floor and builds cleanly under
|
|
`1.89.0`. Any future todo bumping sqlx must re-check this MSRV
|
|
constraint against `rust-toolchain.toml` before bumping the major/minor
|
|
version — do not blindly copy runway's `0.9` pin.
|
|
|
|
## Todo 6 — SourceRepo fixed seed ids and weights (locked, reusable by todo 9+)
|
|
|
|
- **Fixed, stable UUIDs** for the 3 v1 seed sources, defined as constants
|
|
in `crates/news-store/src/sources.rs`. Any later todo (e.g. todo 9's
|
|
clustering tests) referencing "the BBC source" by id should reuse these
|
|
literals rather than re-deriving or re-seeding:
|
|
- Al Jazeera: `6f6a0e3e-9d1a-4b7a-8b8a-000000000001`
|
|
- BBC World: `6f6a0e3e-9d1a-4b7a-8b8a-000000000002`
|
|
- AP: `6f6a0e3e-9d1a-4b7a-8b8a-000000000003`
|
|
- **Weights** (in `[0.0, 1.0]`, per todo 3's assumed `Source.weight`
|
|
range): Al Jazeera `0.7`, BBC World `0.9`, AP `0.9`. Rationale: BBC and
|
|
AP get the higher trust weight as established wire-service/broadcast
|
|
sources with long corroboration track records; Al Jazeera is reputable
|
|
but set slightly lower to reflect more contested editorial framing on
|
|
some regional coverage. These are subjective starting defaults, not a
|
|
derived formula — a future todo could recalibrate them from feedback
|
|
data without a schema change (the column is a plain `REAL`).
|
|
`Source.weight` values are chosen defaults, not derived from any
|
|
formula in this todo.
|
|
- **`upsert` is the sole idempotency mechanism for
|
|
`seed_default_sources`**: `INSERT ... ON CONFLICT(id) DO UPDATE SET
|
|
...` on the fixed ids above, so calling it any number of times leaves
|
|
exactly 3 rows. No separate "does this row exist" pre-check was added.
|
|
- **Reuters is deliberately absent** from this crate's code (no seeded
|
|
row, enabled or disabled) — per the plan, it belongs only in the config
|
|
template (todo 18/21) as a disabled, commented-out example.
|
|
- **`kind` TEXT values match todo 2's snake_case convention exactly**:
|
|
`"rss_atom"` / `"news_sitemap"`, via an explicit `match` in
|
|
`kind_to_str`/`kind_from_str` (no `serde_json` round-trip through the
|
|
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<f64>`,
|
|
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).
|