connor/news

Fork 0

Files

T

connor 4242373576 Dumb OMO commit

2026-09-01 18:44:42 -04:00

42 KiB

Raw Blame History

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):

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.

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<f64> 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.

42 KiB Raw Blame History