connor/news
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Initial commit -- did OMO do a good job?
Check / check (push) Failing after 1m27s
Details
Check / guardrails (push) Failing after 39s
Details
Check / bundle (push) Successful in 1m4s
Details

Browse Source
This commit is contained in:
connor
2026-09-01 18:29:46 -04:00
commit 20a6c26f7c
179 changed files with 21704 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.omo/notepads/news-triage/learnings.md
+666
View File
@@ -0,0 +1,666 @@
# Learnings — news-triage
Conventions, patterns, and successful approaches discovered during work on this plan.
_Auto-scaffolded by /start-work. Append new entries below - never overwrite._
---
## Todo 1 — workspace scaffold
- **Toolchain pin**: `rust-toolchain.toml` pinned to `channel = "1.89.0"` (exact
`rustc --version` on this box), with `components = ["rustfmt", "clippy"]` and
`targets = ["wasm32-unknown-unknown"]`. First `cargo build` auto-downloaded
the 1.89.0 toolchain via rustup (~35s one-time cost) even though 1.98.0 was
already the machine's default (matches runway's pin) — this is expected and
correct: rust-toolchain.toml overrides the ambient default per-directory.
- **cargo-machete false "unused" at scaffold stage**: with only stub
`lib.rs`/`main.rs` files (doc comments only, no code using `serde`/
`thiserror`/`news-core`), `cargo machete` correctly flags every declared dep
as unused and exits 1. This is EXPECTED, not a bug — it will self-resolve
once todo 2 adds `serde`-derived types to `news-core`. Do not "fix" this by
removing the deps; they're scoped per this todo's explicit allowance
(serde + thiserror only).
- **cargo-deny/cargo-machete already installed** on this box at
`~/.cargo/bin/` — no `cargo install --locked` needed locally (CI still
installs them fresh per the workflow, matching runway's pattern).
- **`cargo deny check` emits `license-not-encountered` warnings** (not
failures) for licenses listed in `deny.toml`'s allow-list that don't appear
yet in the tiny dependency tree (Apache-2.0 WITH LLVM-exception,
BSD-2-Clause, BSD-3-Clause, ISC, Zlib) — harmless, will resolve as more deps
land; exit code stays 0.
- **news-web is a plain native binary stub** (no Leptos dep yet) per the
todo's explicit allowance ("minimal placeholder `fn main() {}` is fine").
It still passes `cargo check -p news-web --target wasm32-unknown-unknown`
since it has zero non-wasm-safe dependencies — so the CI `check` job's wasm
step already works even before Leptos is wired in (Wave 5 / todo 19).
- Workspace-level `[workspace.dependencies]` kept to `serde` (derive feature)
+ `thiserror` per the todo's explicit "must not" constraint — every crate's
own `Cargo.toml` has `[lints] workspace = true` to inherit the deny/forbid
lint set, mirroring runway's per-crate convention exactly.
## Todo 2 — news-core domain types
- **`unwrap_used`/`expect_used` deny is workspace-wide, including tests** —
confirmed against runway's convention: runway does NOT scope the lint
exception to `#[cfg(test)] mod tests` blocks; it uses
`#![allow(clippy::unwrap_used, clippy::expect_used)]` at the top of
standalone `tests/*.rs` integration test files. For inline `#[cfg(test)]
mod tests` inside `src/model.rs` (this todo's case, since the plan asked
for the test in `model.rs` itself), the cleanest option that needs NO
`#[allow]` at all is: write `#[test]` functions that return
`Result<(), serde_json::Error>` and use the `?` operator on the
fallible `serde_json::to_string`/`from_str` calls, then `assert_eq!` for
the actual behavioral check (which never panics via unwrap/expect — it
uses `assert_eq!`'s own panic machinery, which is not linted). This
avoids `.unwrap()`/`.expect()` entirely and keeps the workspace-wide deny
intact with zero `#[allow]` anywhere. Use this pattern for all future
crates' inline unit tests; reserve runway's
`#![allow(clippy::unwrap_used, clippy::expect_used)]` file-level escape
hatch only if a future test genuinely cannot be expressed as a
`Result`-returning test (e.g. asserting a panic via `#[should_panic]`,
or needing `Uuid::new_v4()` fallibility that never actually fails).
- **`uuid`/`chrono` workspace deps added**, mirroring runway's exact feature
sets (`uuid = { version = "1", features = ["v4", "serde"] }`,
`chrono = { version = "0.4", default-features = false, features =
["serde", "clock", "std"] }`). Also added `serde_json` and
`pretty_assertions` to `[workspace.dependencies]` as dev-dependencies for
round-trip tests — every crate with domain types will need these, so
hoisting them to the workspace level now avoids repeated version
decisions later.
- This resolves the `cargo-machete` "unused serde" false positive flagged
in todo 1: `news-core` now has real `serde`-derived types, so `serde` is
genuinely used.
## Todo 5 — FTS5 external-content table sync triggers (exact working SQL)
The standard SQLite FTS5 "external content table" trigger recipe
(sqlite.org/fts5.html section 4.4.3), confirmed working against
`sqlite::memory:` via sqlx 0.8.6 with three passing tests (insert-then-MATCH,
delete-then-MATCH-absent, and the migrate-twice idempotency check):
```sql
CREATE VIRTUAL TABLE raw_items_fts USING fts5(
title,
summary,
content = 'raw_items',
content_rowid = 'rowid',
tokenize = 'porter'
);
CREATE TRIGGER raw_items_ai AFTER INSERT ON raw_items BEGIN
INSERT INTO raw_items_fts (rowid, title, summary)
VALUES (new.rowid, new.title, new.summary);
END;
CREATE TRIGGER raw_items_ad AFTER DELETE ON raw_items BEGIN
INSERT INTO raw_items_fts (raw_items_fts, rowid, title, summary)
VALUES ('delete', old.rowid, old.title, old.summary);
END;
CREATE TRIGGER raw_items_au AFTER UPDATE ON raw_items BEGIN
INSERT INTO raw_items_fts (raw_items_fts, rowid, title, summary)
VALUES ('delete', old.rowid, old.title, old.summary);
INSERT INTO raw_items_fts (rowid, title, summary)
VALUES (new.rowid, new.title, new.summary);
END;
```
Key gotchas that would otherwise silently corrupt the index:
- The `'delete'` command is issued as an **INSERT into the fts5 table
itself**, not a `DELETE FROM raw_items_fts`. The first column value
must literally be the string `'delete'` and the second must be the
`rowid` to remove — this special form is documented in section 6.3
("The 'delete' Command") and is what makes it different from a normal
content mutation.
- The delete/update triggers MUST pass `old.title`/`old.summary` (the
values being removed) alongside `old.rowid` — omitting the column
values or passing `new.*` there is the single most common mistake and
either throws an FTS5 "constraint failed" error or leaves stale index
entries.
- UPDATE is delete-then-insert (two separate `INSERT INTO raw_items_fts`
statements in one trigger body), not a single "update" special command
— FTS5 has no special update verb; the delete+reinsert pattern is the
documented approach.
- `PRAGMA journal_mode=WAL;` has no effect on `sqlite::memory:` — it
silently returns `"memory"` instead of `"wal"` from some drivers. Do not
assert on the PRAGMA's return value in a test that runs against
`sqlite::memory:`; `init_db`'s test suite intentionally does not check
it (per the plan's explicit note on this).
- sqlx 0.8 (not 0.9) is required under this workspace's `rust-toolchain.toml`
pin of `1.89.0` — see decisions.md for the MSRV details. `cargo test`
fails fast with a clear "rustc X is not supported by sqlx@0.9.0" message
if this is gotten wrong, so it's an easy fix to spot.
## Todo 7 — feed poller XML namespace handling and op-ed heuristic
- **Namespace handling: literal tag-string matching, not `NsReader`.**
Both parsers (`rss_atom.rs`, `news_sitemap.rs`) use plain
`quick_xml::reader::Reader` (not `NsReader`) and match element names as
`&str` via `QName`/`LocalName`'s `AsRef<str>` (quick-xml 0.42's `QName`
wraps `&str`, not `&[u8]` — the API changed since older quick-xml
versions that some examples online still show as byte-string `b"tag"`
matching). For the AP news-sitemap, `news:title` and
`news:publication_date` are matched as the literal strings
`"news:title"`/`"news:publication_date"` (prefix included), which is
correct and sufficient because AP's sitemap always declares the prefix
as exactly `news:` — no namespace-aware resolution needed. If a future
source ever used a different prefix for the same namespace URI, this
approach would silently miss it; `NsReader::read_resolved_event_into`
would be the fix at that point, resolving by namespace URI
(`http://www.google.com/schemas/sitemap-news/0.9`) rather than prefix
string. Chose literal matching over `NsReader` because it's simpler and
every real AP/BBC/AJ sample uses the same fixed prefixes.
- **Op-ed heuristic (todo 9/12 should know this)**: `ItemKind::Opinion` is
assigned in `rss_atom.rs`'s `item_kind_from_category()` when the RSS
`<category>` text (case-folded) contains the substring `"opinion"` or
`"comment"`; everything else (including a missing `<category>`)
defaults to `ItemKind::News`. Atom `<category term="...">` values feed
the same heuristic via the `term` attribute. The news-sitemap dialect
has no per-item category field at all, so `parse_news_sitemap()` always
emits `ItemKind::News` — AP items can never be classified `Opinion` by
this crate; if AP op-ed detection matters later, it would need a
title/URL-path heuristic (e.g. `/opinion/` in the `<loc>` path) added at
that time, since the sitemap protocol carries no signal for it today.
This is a coarse heuristic (substring match on one field), not a
content classifier — todo 9 (clustering) and later scoring todos should
treat `ItemKind::Opinion` as a soft/approximate signal, not authoritative.
- **`quick_xml` 0.42 API notes** (differs from older docs found online):
`BytesText` has no `.unescape()` method in this version — use
`quick_xml::escape::unescape(text.into_inner().as_ref())` directly.
`Attribute::unescape_value()` is deprecated in favor of
`Attribute::normalized_value(XmlVersion::Explicit1_0)`. `Attributes`
iterates `Result<Attribute, AttrError>`, and `AttrError` converts to
`quick_xml::Error` via `From` but not directly to a custom error type,
so call sites need `.map_err(quick_xml::Error::from)?` before the `?`
propagates into a `thiserror`-derived `IngestError` that only has
`#[from] quick_xml::Error`.
- **Retry-After**: only the integer-seconds form is parsed
(`parse_retry_after` in `lib.rs`); the HTTP-date form
(`Retry-After: Wed, 21 Oct 2026 07:28:00 GMT`) is intentionally left
unparsed (falls back to `None`) per the plan's explicit allowance to
"document if you skip HTTP-date parsing" — todo 18's scheduler should
not assume `RateLimited(None)` means "no rate limit signal was sent",
since it also covers this unparsed-date case.
- **Live smoke test result**: `curl -sI` against
`https://www.aljazeera.com/xml/rss/all.xml` with the exact honest UA
returned `HTTP/2 200` with `etag`/`last-modified` response headers
present — network access was available in this sandbox, so this is a
real (not skipped) verification.
## Todo 8 — ItemRepo + FTS5 BM25 search
- **Unique-violation detection**: the clean dedupe path is `match` on the
`sqlx::query(...).execute(...).await` result and map
`Err(sqlx::Error::Database(db_err)) if db_err.is_unique_violation()` to
`InsertOutcome::AlreadyExists`. Do not pre-SELECT — it is race-prone and
slower, and the SQLite `UNIQUE(source_id, link)` constraint is the
authoritative guard.
- **FTS5 auto-sync**: never insert/update `raw_items_fts` directly. The
external-content triggers in `0001_init.sql` (`raw_items_ai`,
`raw_items_ad`, `raw_items_au`) mirror `raw_items` into the FTS5 table
automatically. Manual inserts would duplicate entries and corrupt ranking.
- **MATCH with a bound parameter is safe**: `WHERE raw_items_fts MATCH ?1`
with `sqlx::query(...).bind(query)` treats the bound value as the literal
query text, so arbitrary user input cannot break FTS5 syntax. Do not
string-interpolate the query.
- **bm25() sign convention**: SQLite's `bm25()` returns negative or small
positive values where **lower is better** (e.g. a strong match can be
`-2.3`, a weaker match closer to `0`). `ORDER BY rank` therefore yields
best-first. Callers that want a descending "higher is better" score
(e.g. todo 10) must negate or otherwise invert the value.
- **External-content join**: because `raw_items_fts` is declared with
`content = 'raw_items'` and `content_rowid = 'rowid'`, the FTS5 `rowid`
is the same as `raw_items.rowid`. Join is `JOIN raw_items r ON r.rowid =
f.rowid`; `SELECT r.*` then maps cleanly to the domain `RawItem`.
- **`chrono` must be declared in `news-store/Cargo.toml`** even though the
`RawItem` type lives in `news-core`. Serialization to RFC3339 strings
(`published_at.to_rfc3339()`) and parsing back (`DateTime::parse_from_rfc3339`)
happen inside `news-store`, so the dependency is required there.
## Todo 9 — title-similarity clustering + ClusterRepo
- **Threshold chosen for tests: `0.5`**. The Jaccard token-set score between
"Israel and Hamas agree ceasefire deal" and "Israel and Hamas agree ceasefire
deal, mediators say" is `5/7 ≈ 0.714`, well above `0.5`; a looser title
variant like "Israel and Hamas reach ceasefire agreement" scored only
`3/7 ≈ 0.429` and therefore started its own cluster. `0.5` is a serviceable
first cut for the breaking-bypass corroboration rule, but real-world
evaluation may need calibration.
- **Window is anchored to the incoming item's timestamp, not `Utc::now()`**.
The candidate query uses `sc.last_seen_at <= ?1` (the new item's
`published_at` as RFC3339) and
`julianday(?1) - julianday(sc.last_seen_at) < 45.0 / 1440.0`. Using the
item's own timestamp makes the 46-minute failure-path test deterministic.
- **Strict inequality matters**: `< 45.0 / 1440.0` ensures a 46-minute gap is
excluded; a `<=` would incorrectly merge across the boundary.
- **`source_count` must be `COUNT(DISTINCT raw_items.source_id)`**, not a
count of member rows. Two items from the same source (e.g. AJ repeating the
same event) must yield `source_count == 1` until a different source joins.
- **All assignment writes belong in one sqlx transaction** (`pool.begin()`,
updates, `tx.commit()`). This prevents a crash between inserting a
`cluster_members` row and updating `story_clusters.source_count`.
- **Clippy `manual_contains`**: for a `&[&str]` slice, prefer
`STOPWORDS.contains(&token)` over `iter().any(|&s| s == token)`.
## Todo 10 — RelevanceScorer: FTS5 BM25 + persisted Naive Bayes + blocklist veto
- **BM25 sign flip**: SQLite's `bm25()` returns ascending-better ranks that are
typically negative, so the scorer negates the raw rank to produce a
descending higher-is-better score. Items not returned by the FTS query (empty
interests or no match) are floored at `0.0`. This keeps the BM25 term
non-negative while preserving the ordering from `ItemRepo::search_fts`.
- **Tokenizer sharing**: `news_core::clustering::tokenize` is now public and
returns `Vec<String>` using the same rules as title-similarity clustering.
`similarity` converts the vector to a `HashSet` so its behavior is unchanged;
the scorer keeps duplicates so repeated words contribute multiple times to
the Bayes log-odds sum.
- **Model persistence**: the `bayes_model` table's singleton row (`id = 1`) is
read on `RelevanceScorer::load` and written with `INSERT OR REPLACE` after
every `train`. `serde_json` round-trips the model because `NaiveBayesModel`
now derives `Serialize` and `Deserialize`.
- **Do not implement `news_core::scoring::relevance::Bm25Source`** here. The
trait is synchronous, but `news-store`/`sqlx` access is async. The scorer
takes its BM25 signal directly from `ItemRepo::search_fts`; a sync adapter
would require blocking an async runtime and is the wrong seam for todos 13/18.
- **Direct SQL needs a pool handle**: `ItemRepo` keeps its `SqlitePool` field
private, so `RelevanceScorer` retains its own `pool: SqlitePool` clone for
the singleton-row read/write in `load` and `train`.
- **Blocklist veto is first and unconditional**: case-insensitive substring
match against `title + " " + summary` returns `f64::NEG_INFINITY` before
BM25 or Bayes are computed. Training the model heavily on the item's own
tokens as `Interested` does not override the veto.
- **Test naming**: placing `#[tokio::test]` functions at module level (with
`#[cfg(test)]` on each helper and test) yields paths like
`scoring::blocklist_veto_returns_neg_infinity`, matching the expected test
filter exactly.
## Task 11 — PercentileTracker + ImportanceScorer
- **Keep distributions strictly separate**: `percentile_stats.metric_kind`
stores `"importance"` and `"relevance"` as TEXT discriminants, and every
query filters on `metric_kind`. Todo 12 reads Importance@p99 and
Relevance@p90 from independent samples; mixing them would break the gate.
- **RFC 3339 TEXT consistency**: both writes and the cutoff comparison use
`Utc::now().to_rfc3339()` (no `to_rfc3339_opts`). SQLite TEXT comparison then
orders ISO timestamps correctly, so the `computed_at >= cutoff` filter is
deterministic and precision-matched on both sides.
- **Window edge case**: rows with `computed_at` older than `window_days` are
excluded by the query, not by post-filtering. Inserting a row directly via SQL
with an 8-day-old timestamp and querying with `window_days = 7` proves the
filter excludes it.
- **Empty window must return `StoreError::InsufficientData`**, not a default
`0.0` and not a panic. The `InsufficientData` variant already existed with
`needed`/`available` fields; the tracker uses `{ needed: 1, available: 0 }`.
- **Percentile indexing**: with `count` sorted values, the value at index
`ceil(p / 100.0 * count) - 1` (clamped to `count - 1`) gives the expected
exact results: 100 values `1.0..=100.0` at p90 returns `90.0`, and p100
returns the maximum.
- **ImportanceScorer trust term**: use the **maximum** `Source.weight` among
the cluster's distinct member sources. The cluster may contain multiple items
from the same source; `COUNT(DISTINCT raw_items.source_id)` is already
`cluster.source_count`, and `MAX(sources.weight)` captures the strongest
source in the corroboration set. v1 hard-codes prominence to `0` because RSS
feeds carry no editorial rank signal.
- **ImportanceScorer does not record samples**: recording cadence is the
scheduler's job (todo 18). The scorer only computes the arithmetic score via
`news_core::scoring::importance::importance_score`.
- **Direct SQL in news-server needs `StoreError` conversion**: `sqlx::Error`
must be mapped with `.map_err(news_store::StoreError::from)?` because
`ServerError` only implements `From<StoreError>`, not `From<sqlx::Error>`.
- **Age parameter**: the importance formula accepts `corroboration_window_minutes`
for API completeness but does not use it numerically in v1. `score_cluster`
still passes the cluster's age in minutes to keep the call site honest for
future window-aware variants.
## Todo 12 — NotificationGate
- **The plan sketch omitted the pool, but the gate needs it**: `NotificationGate`
must hold `SqlitePool` because every `evaluate()` call reads from and writes
to `notification_log` (already-notified check, bypass ceiling count, and
success logging). The struct is `NotificationGate { pool, budget, config }`.
- **Bypass lane is fully independent of the normal token bucket**: bypass
decisions neither consume nor refill `TokenBucket.tokens`; the acceptance
test `bypass_ceiling_is_independent_of_normal_budget` proves this by
pre-exhausting the bucket to `0.0` and still allowing 4 bypass `Notify`
decisions before the 5th falls through to `Suppress(BudgetExhausted)`.
- **In-memory token bucket is an accepted v1 limitation**: `TokenBucket` stores
`tokens` and `last_refill` in memory, so a process restart resets the budget.
Persistence is intentionally out of scope for todo 12.
- **Corroboration bypass inference**: `cluster.source_count >= 2` qualifies a
story for the bypass lane because `StoryCluster` members are grouped within a
45-minute window (clustering logic in news-ingest), so two sources means
2-of-N corroboration within that window.
- **Percentiles are 0-100 scale in the gate**: `notify_percentile = 90.0`,
`digest_floor_percentile = 75.0`, `bypass_importance_percentile = 99.0`.
Tests use values like `95.0` and `80.0` directly; this matches the config
defaults and avoids the 0-1 scale used elsewhere in sample data.
- **Quiet-hours window uses Louisville local hour**: `hour >= 22 || hour < 7`
on `now.with_timezone(&America::Louisville)`. Relevance in the digest band
(`75 <= r < 90`) still produces `Digest` even during quiet hours; higher
relevance during quiet hours produces `Suppress(QuietHours)`.
- **Already-notified check runs before bypass**: the latest
`notification_log` row for the cluster is fetched first. If
`cluster.source_count < source_count_at_notify + material_update_source_delta`,
the evaluation returns `Suppress(AlreadyNotified)` before any bypass or
normal-path logic.
- **RFC 3339 TEXT for DB reads/writes and cutoff comparison**: `created_at` is
stored with `now_utc.to_rfc3339()` and the 24-hour bypass cutoff uses
`(now_utc - Duration::hours(24)).to_rfc3339()`. SQLite TEXT ordering is
lexicographically correct for RFC 3339 timestamps.
- **No unwrap/expect in tests**: `chrono_tz::Tz::with_ymd_and_hms()` returns a
`LocalResult`, so tests use `.single().ok_or_else(|| ServerError::DateTime(...))?`.
This required adding a `ServerError::DateTime(String)` variant.
- **Clippy `collapsible_if` on `let`/`if` chain**: the already-notified check
was cleaner as `if let Some(...) = ...? && cluster.source_count < ...`.
- **Digest is a decision only**: `GateDecision::Digest` does not persist digest
content or queue entries; todo 16 owns digest content selection.
## Todo 13 — POST /api/feedback + FeedbackRepo + incremental Naive Bayes retraining
- **`FeedbackRepo` mirrors `sources.rs`/`items.rs`**: `pub struct FeedbackRepo(SqlitePool)`
with `new(pool)`, `insert`, and query helpers. `kind_to_str` maps
`FeedbackKind` to snake_case TEXT exactly like `ItemKind`/`SourceKind`.
- **`canonical_item_text` uses `fetch_optional`**: returns `None` when the cluster
is missing, letting the handler return the same 400 as an unknown cluster id.
- **`Json<FeedbackRequest>` gives 422 for free**: `FeedbackKind` already has
`#[serde(rename_all = "snake_case")]`; an unknown variant like `"bogus"`
fails deserialization and axum returns 422 `JsonDataError` with no custom
parsing code.
- **`RelevanceScorer::load(..., Vec::new(), Vec::new())` is fine for training**:
`load()` reads the persisted `bayes_model` row; `train()` only increments
counts and writes the row back. Empty interests/blocklist are irrelevant
because `train()` never consults them.
- **Incremental training only**: each request calls `scorer.train(&tokens, kind)`
once. The model is never rebuilt from the `feedback` table.
- **`tower::ServiceExt::oneshot` for in-process tests**: build the `Router`, then
`app.oneshot(Request::builder()...body(...)?)` with no TCP listener. In axum
0.8 `oneshot` returns `Result<Response, Infallible>`; handle the `Infallible`
with an exhaustive `match` to satisfy the `unwrap_used`/`expect_used` deny.
- **Map HTTP/builder errors into `ServerError` in tests**: `Request::builder().body()`
and `axum::body::to_bytes()` can fail; map them to `ServerError::DateTime`
(or propagate via `?`) so tests return `Result<(), ServerError>` without
unwrap/expect.
- **Keep axum/tower imports tidy**: `Body` and `Request` are test-only, so they
live inside `#[cfg(test)] mod tests`; otherwise `dead_code`/`unused_imports`
warnings fire and `-D warnings` fails.
## Todo 14 — NewsConfig TOML loader with SIGHUP hot-reload
- **Schema must match the draft character-for-character** (`.omo/drafts/news-triage.md:47`):
top-level `[[sources]]`, `[notify]`, `[topics]`; required source keys `url`,
`kind`, `weight`, `enabled`; only `poll_interval_secs` is optional via
`#[serde(default)]`. Missing required keys produce a hard TOML parse error.
- **Naming mismatches with `gate::NotifyConfig`** (to resolve when wiring the
gate in a later todo):
- Config has `quiet_hours_start`/`quiet_hours_end` as `"HH:MM"` strings;
`NotifyConfig` has `quiet_start_hour`/`quiet_end_hour` as `u32`. A parser
will be needed to translate `"22:00"` → `22` and `"07:00"` → `7`.
- Config has `budget_refill_per_day`/`budget_burst` (`f64`) and
`bypass_ceiling_per_day` (`u32`); `NotifyConfig` lacks budget fields
(they feed `TokenBucket::new`) and calls the bypass limit
`bypass_daily_ceiling: u32`.
- `NotifyConfig` carries computed defaults not present in TOML:
`notify_percentile`, `digest_floor_percentile`,
`bypass_importance_percentile`, `material_update_source_delta`. These will
likely stay hard-coded or move to a separate defaults module.
- **All-or-nothing reload**: `reload_once` parses the file outside the lock,
then acquires the write lock and replaces the value. A parse error returns
`Err` without touching the in-memory config; the SIGHUP watcher logs the
error and continues.
- **`tracing_test::traced_test` + `logs_contain`** captures real logs in async
tests. `reload_once` itself returns `Err`; the test logs that error with
`tracing::error!` and asserts `logs_contain("toml parse error")`, mirroring
what `watch_sighup` does in production.
- **No unwrap/expect in tests or production**: `tempfile::tempdir()?`,
`std::fs::write(&path, ...)?`, and `load(&path)?` all leverage the new
`ServerError::Io`/`ServerError::Toml` `#[from]` conversions. The SIGHUP
handler installation failure is handled with `match` rather than `.expect()`
because the workspace denies `expect_used`.
- **Cargo workspace inheritance with additive features**: promoting `tokio` to
a regular dependency in `news-server/Cargo.toml` uses
`tokio = { workspace = true, features = ["macros", "rt", "signal", "sync"] }`.
Cargo adds these features to the workspace features (`rt-multi-thread`,
`macros`) rather than replacing them.
- **Added to root `[workspace.dependencies]` only**: `toml`, `tracing`,
`tracing-test`, `tempfile`. No other root-table changes were made.
## Todo 15 — NtfyPublisher
- **Action-button JSON bodies must be single-quoted in the `Actions` header**:
ntfy's simple action parser splits parameters on commas and actions on
semicolons. A JSON body like `{"story_cluster_id":"...","kind":"interested"}`
contains both, so it has to be wrapped in single quotes:
`body='{"story_cluster_id":"...","kind":"interested"}'`.
- **Use reqwest's built-in auth helpers**: `.bearer_auth(token)` and
`.basic_auth(user, Some(pass))` avoid hand-rolled base64 and
`InvalidHeaderValue` errors.
- **Retry-once logic should be explicit, not a loop**: one `match` for the
first attempt, a 500 ms `tokio::time::sleep`, then a second attempt. This
guarantees exactly one retry and no infinite loops.
- **`NEWS_API_BASE_URL` default belongs in the publisher**: the ntfy
notification needs a concrete API base URL for its `http` feedback actions.
`from_env` reads `NEWS_API_BASE_URL` and falls back to `http://localhost:3000`.
- **Wiremock `Mock::expect(n)` verifies call counts on guard drop**: hold the
returned `MockGuard` until after the assertion so the expectation is not
unregistered early. `expect(2)` plus `respond_with(500)` cleanly asserts the
retry-test "exactly 2 requests" contract.
- **`#[allow(dead_code)]` on `NtfyPublisher` is acceptable when the spec
requires stored fields that have no current reader**: the struct must keep
`base_url` and `topic` per the todo, even though only `topic_url` is used at
runtime. The override is narrower than disabling the workspace lint globally.
## Todo 16 — daily digest
- **`PercentileTracker::percentile(metric_kind, p, days)` returns the *value* at
percentile `p`, not the percentile rank of a value**: the task's pseudo-call
`percentile(MetricKind::Relevance, score)` is semantically backwards against
the existing API. The correct band check is to fetch the 75th and 90th
percentile values once and test `p75 <= score && score < p90`, which is
equivalent to checking that the score's rank falls in `[75, 90)`.
- **DateTime with a generic `TimeZone` does not implement `Display`**: the
`Tz: TimeZone` generic used in `build_and_send_digest` prevents `tracing::debug!`
with `{}`. Use structured debug fields (`tracing::debug!(?now, "...")`) or
convert to `DateTime<Utc>` before formatting.
- **Digest idempotency relies on `notification_log.lane = 'digest'`**: a single
row with a nil UUID cluster ID and `source_count_at_notify` set to the number
of stories is sufficient; no token-bucket or cluster-specific log rows are
needed.
- **`NtfyPublisher` retry logic can be factored without changing existing tests**:
extracting `send_with_retry` lets `publish_digest` share the one-retry behavior
of `publish` while keeping the original tests passing unchanged.
- **`RawItem.kind` / `raw_items.kind` already distinguishes opinion pieces**:
no schema change was required to honor `notify_opinions`; the digest joins
through to the canonical `raw_items` row and filters on `ItemKind::Opinion`.
## Todo 17 — replay harness and server dry-run
- **`SqlitePool::connect` in this sqlx/sqlite build does not create the database
file**: the default `NEWS_DB_PATH=./news.db` must be pre-created as an empty
file (and its parent directories) before `init_db` opens it; in-memory pools
(`sqlite::memory:`) are unaffected.
- **Dry-run suppression isolation is enforced in SQL, not Rust**: every
suppression-relevant read of `notification_log` must include `AND dry_run = 0`.
Missing it in even one subquery (e.g. `bypass_count_last_24h`) lets a dry-run
row suppress a real evaluation.
- **Keep public wrapper APIs unchanged**: adding `evaluate_with_mode` and
`build_and_send_digest_with_mode` lets the new code use `dry_run` while all
existing tests and call sites compile without modification.
- **`FeedPoller` can be exercised against a local `python3 -m http.server`**:
this gives fast, deterministic live QA for `news-server --dry-run` without
relying on remote feeds or ntfy.
- **Replay and dry-run share the same formatter but not the same code path**:
`DecisionRow` + `format_decision_row` lives in `news-server` and is reused by
`news-cli replay`; the replay harness stays isolated with an in-memory DB and
a fixed `--now`.
## Todo 18 — axum wiring, health/metrics, scheduler, retention pruning
- **Keep the binary thin**: `main.rs` should only parse CLI args, load config,
build the runtime, decide between one-shot and server mode, and start the
Axum listener. Runtime construction, source conversion, default config, and
ntfy setup belong in a `bin_support` module so both files stay under the 250
pure-LOC ceiling.
- **Move orchestration into `main.rs` if it keeps `bin_support` under LOC**:
`run_once` and `start_background_loops` live next to `main()` because they
are the binary's specific flow; `bin_support` keeps reusable setup helpers.
- **Split oversized test modules into their own files**: `digest.rs` was ~550
total LOC (mostly tests). Moving tests into `digest/tests.rs` and declaring
`#[cfg(test)] mod tests;` in `digest.rs` drops the core module to ~170 LOC
without changing test behavior. Avoid `mod tests { ... }` inside a file
already named `tests.rs` — clippy flags it as module-inception.
- **Prometheus metrics need a stable label set from the start**: `Metrics`
registers `news_poll_total`, `news_notify_total`, `news_suppress_total`, and
`news_scrape_duration_seconds` at startup with empty label values, then
increments via typed helpers. This guarantees `/metrics` always emits the
expected series even before the first poll.
- **Per-source scheduling must be per-source, not global**: each enabled source
gets its own `tokio::time::interval` task. A `RateLimited` result on one
source only affects that source's next poll via a local backoff variable;
other sources continue on their normal cadence.
- **Use `spawn_blocking` only for CPU-heavy work; DB writes stay async**: the
scheduler's poll pipeline (`poll` -> `insert_if_new` -> `assign_or_create` ->
score -> gate -> notify) is fully async and runs in the source task. No
blocking calls are introduced.
- **Pruning deletes orphan `cluster_members` first**: the 90-day retention job
deletes `raw_items` rows older than the cutoff, then removes any
`cluster_members` whose `item_id` no longer exists. `story_clusters`,
`feedback`, `bayes_model`, and `percentile_stats` are retained indefinitely.
- **`duration_until_03_00` must handle the current-hour case**: if the time is
already past 03:00, target tomorrow at 03:00; if before, target today. A
naive `+1 day` would delay the first pruning run by up to 24 hours.
- **Default config in the binary is helpful for local dev**: when
`NEWS_CONFIG_PATH` (default `/config/config.toml`) is missing, the server
falls back to a built-in default with the three v1 sources. This matches the
seed-source defaults and lets `cargo run` work outside the container.
- **Dry-run --once exits cleanly without real notifications**: `run_once`
polls every enabled source once, runs the digest path with a dummy ntfy
publisher, and returns. It never publishes to the real ntfy topic and never
starts the background Axum server.
## Todo 19a — notification-gate peek + read-only HTTP API
- **Side-effect-free peek should share one private decision function with
evaluate**: extract the threshold/bypass/budget logic into a private
`decide()` method that returns an internal `DecisionOutcome`. `peek()` clones
the token bucket, refills it in memory, and calls `decide()` without writing
`notification_log` or decrementing `budget.tokens`. This guarantees `peek`
and `evaluate` never diverge.
- **`PercentileTracker` has no rank API**: the frozen `news-store` crate exposes
only `record()` and `percentile()`. To show percentile ranks in the UI, query
`percentile_stats` directly with a window filter and compute the empirical
CDF in application code.
- **`pub(crate)` is the least-invasive way to let sibling-module tests observe
private state**: `NotificationGate.budget` was made `pub(crate)` so
`api::stories::tests` can assert that `peek` leaves `budget.tokens` unchanged.
It does not appear in the crate's public API.
- **RFC3339 query parameters containing `+` must be URL-encoded or use `Z`
format**: `to_rfc3339()` emits `+00:00`, which query-string parsers treat as
a space. In tests, use `to_rfc3339_opts(SecondsFormat::Secs, true)` to get the
`Z` suffix and avoid encoding entirely.
- **Clippy 1.89 supports collapsing nested `if let` guards**: the lint
`collapsible_if` now suggests `if let A = a && let B = b { ... }`, which is
stable in this toolchain. Prefer this over nested `if let` blocks.
## Todo 19b — news-web Leptos 0.8 CSR UI
- **Mirror `runway-web`'s Trunk + Tailwind v4 setup exactly**: `Trunk.toml`
uses a `pre_build` hook that invokes `npx --prefix ../.. @tailwindcss/cli`,
`index.html` links `styles/generated.css`, and `styles/main.css` is a Tailwind
v4 source file. This keeps the build self-contained and avoids Trunk's bundled
Tailwind v3.
- **Keep the WASM bundle lean by cutting `news-core`**: `news-web` does not need
domain types from `news-core`; defining small DTOs locally (or reusing
`serde_json::Value`) keeps the release `.wasm` under 600 KiB, leaving headroom
for the todo 20 1.8 MiB budget.
- **Use relative `/api/...` URLs and let Trunk proxy in dev**: `gloo-net`
requests go to `/api/stories`, `/api/feedback`, and `/api/config`. In
production the same-origin request reaches the server directly; in dev,
`Trunk.toml` proxies `/api` to the backend. No absolute URLs are baked into
the WASM.
- **On `wasm32-unknown-unknown`, use `Action::new_local`**: Leptos 0.8's
`Action::new` requires a `Send` future, which JS futures are not. Use
`Action::new_local` for feedback actions backed by `gloo_net`.
- **Stories and Config views need matching loading and error states**: a
`Suspense`/`Transition` wrapper and a dedicated error panel give visible
feedback when `/api/stories` or `/api/config` fail.
- **Playwright MCP expects Google Chrome at `/opt/google/chrome/chrome`**: on
Arch this required installing the system `chromium` package and symlinking
`/opt/google/chrome/chrome -> /usr/bin/chromium` before the MCP could launch
the browser.
## Todo 20 — multi-stage Dockerfile, WASM bundle gate, NEWS_DATABASE_URL
- **Mirror `runway/Dockerfile` stage-for-stage, adapting only names and the Rust
pin**: `web` builds the frontend with Node copied from a pinned image and Trunk
installed as a released binary; `server` builds the release binary inside a
cache mount and copies it out; `runtime` is `debian:bookworm-slim` with only
`ca-certificates` and `tzdata`. No toolchain remains in the final image.
- **Use the pinned toolchain in the Docker base image**: `rust-toolchain.toml`
says `1.89.0`, so the Dockerfile uses `rust:1.89-slim-bookworm`, not a floating
`rust:slim-bookworm`.
- **`NEWS_DATABASE_URL` must be parsed, not passed straight to sqlx**: the task
requires stripping the `sqlite://` prefix to get the filesystem path, because
the existing create-if-missing logic works on filesystem paths, not URLs.
`url.strip_prefix("sqlite://")` gives `/db/news.db`; the URL is then rebuilt
as `sqlite:{path}` for `init_db`.
- **A clap `version` flag is the cheapest container smoke test**: adding
`#[command(version)]` exposes `--version`/`-V` using the Cargo.toml version,
avoiding the need to start the server or hit the database inside the image.
- **The CI bundle gate should run the exact same shell logic locally**: the QA
failure path rebuilt the frontend and ran `find ... stat ... budget=1800000`,
proving the gate catches real overages and prints the byte overage message.
- **A 2 MB static array reliably inflates a WASM bundle for gate testing**: it
is also trivial to revert completely, which `git diff` confirmed.
## Todo 21: deploy docs, update script, and systemd timer
- **Mirror runway's deploy README structure closely, but adapt the count**.
Runway has six setup steps (image, signing key, compose, Caddy, frontend dir,
timer). news-triage has no signing key, but adds config-file creation, ntfy
ACL provisioning, and Prometheus scraping. That makes eight steps. Call that
out explicitly so the reader does not look for a missing seventh step.
- **The config-file example lives in `crates/news-server/src/config.rs`**.
There is no standalone `config.example.toml` in the repo; the working schema
is the test fixture inside `config.rs` (lines 127-153). The README tells the
operator to copy from there. If a separate example file is added later, the
README path should be updated.
- **ntfy auth uses env vars, not a mounted secret file**, matching every other
containerized service in the estate. The publishing user needs `rw` access on
the `news-triage` topic because `auth-default-access` is `"deny-all"`. The
exact command is in `deploy/ntfy-access-snippet.md` and repeated in the README.
- **Caddy validation against the scratch copy needs root on this host**.
`caddy validate` opens `/var/log/caddy/access.log` to set up the file writer,
and `/var/log/caddy` is `caddy:caddy` `drwxr-x---`. The non-root command fails
with a permission error that is unrelated to config syntax. `caddy adapt`
succeeds without root and proves the file parses; `sudo caddy validate`
succeeds and proves the full runtime config is valid. Record both in evidence.
- **podman-compose config passes cleanly** against a scratch copy after pasting
the compose service block and the Caddy volume mount. The scratch file must
include the Caddy volume addition (`./data/news/web:/srv/news:ro`) as well as
the new service, because the README documents both under step 4.
- **Atomic rename discipline is the same as runway**: mount the holder
(`./data/news/web`) at `/srv/news`, set `root * /srv/news/dist`, and extract
the image's `dist` into the holder. A deploy renames `dist` to
`dist.previous` and the staged directory to `dist`, so the old inode is
preserved for rollback and Caddy sees the swap immediately.
- **The update script mirrors runway-update nearly line-for-line**, with
`NEWS_*` environment variables and `news-backend` defaults. The same three
failure modes runway documents (rootless deploy, half-deploy, stale bind
mount) apply here, so the comments were kept.
- **Protected-file proof is mandatory**. The four estate files
(`~/compose.yml`, `~/Caddyfile`, `~/data/ntfy/etc/server.yml`,
`~/config/prometheus/prometheus.yml`) must be stat-ted before and after.
Access times may shift on reads, but Modify and Change timestamps must be
unchanged. All four passed.
- **Do not auto-apply snippets**. The README is operator-facing; the QA used
scratch copies in `/tmp/opencode`. No real estate file was touched.