Add personalized curation v2 plan; archive superseded plan and reviews

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A1rCLQeKBgnBo3oTgHuTMe
This commit is contained in:
2026-09-02 02:45:11 +00:00
co-authored by Claude Fable 5.1
parent a5997454ec
commit d0560afa1a
12 changed files with 7081 additions and 0 deletions
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
@@ -0,0 +1,273 @@
# Review — Personalized Ranking, Embeddings, Facets, and Feedback
**Plan:** `docs/plans/2026-08-17-personalized-ranking-and-facets.md`
**Reviewer:** independent blind review (no other review consulted)
**Date:** 2026-08-18
**Method:** plan read end to end, then checked against `src/pipeline.rs`, `src/curate/{mod,prefilter,score,select,llm}.rs`, `src/curate/profile/mod.rs`, `src/types.rs`, `src/db.rs`, `src/config.rs`, `src/main.rs`, `migrations/0001_init.sql`, `data/scour-interests.opml`. Voyage AI claims in §3 were independently verified against the live docs.
---
## Verdict
**Not ready to execute as written — but close, and the architecture is right.** The diagnosis in §1 is accurate and well evidenced against the code: `prefilter.rs` really does make the first irreversible cut on word count, social proof, and a max-over-feeds prior (`PrefilterContext::prior_for`, `prefilter.rs:118-129`), and `select.rs:556-579` really does pad the lineup back up to `target - 5` in direct contradiction of the profile's stated editorial philosophy. The union-of-retrievers direction, the quality/reader-fit split, `candidate_rankings` as a durable feature snapshot, and "no forced filler" are all the correct calls, and I would not relitigate them. The Voyage facts in §3 are accurate (I verified `voyage-4-lite` exists at 32K context, dims 256/512/1024/2048, 1,000 inputs and 1M tokens per request, `$0.02`/Mtok with 200M free) — a genuinely unusual level of rigor for a plan.
What blocks execution is a small number of concrete numerical-design defects, not the architecture. Three of them cause the new system to *look* like it is working while producing garbage: percentile normalization with distinct tie-ranks injects article-ID order as ranking signal whenever a signal is sparse or absent (which is the day-one state); the sparse-evidence confidence damping in §11.2 is mathematically cancelled by the very normalization step that consumes it; and the facet vocabulary is roughly 84 values estimated from roughly 63 ratings, so `facet_preference` will be a near-constant for months while holding 22% of the pre-Stage-A blend. Separately, §16's split of `LlmScore.score` silently kills the churn-suppression rule, and the plan doubles the number of sequential DeepSeek round-trips in a job that has a publish deadline without ever mentioning concurrency. Fix Critical and High below — most are a paragraph of spec each — and this is ready.
---
## Critical
### C1. Percentile normalization with ID tie-breaking turns sparse signals into article-ID bias
> §14.1: "convert the daily candidate values to deterministic percentile ranks in `[0,1]` … Tie breaking must be stable by article ID."
Stable-by-ID tie breaking assigns **distinct** percentile ranks to **equal** raw values. That is correct for determinism of output order and wrong for normalization. Consider the two signals most likely to be degenerate:
- **`embedding_preference` on any day with no ratings in the lookback window** (i.e. day one of rollout, and any 90-day gap): §11.2 gives `positive_similarity_or_0 - 0.75 * negative_similarity_or_0` = `0.0` for every candidate. Percentile-ranking 400 tied zeros with an ID tiebreak produces a perfect ascending-article-ID ramp from 0.0 to 1.0. §14.2 then weights that ramp at **0.28** of the pre-Stage-A score and §17 at **0.15** of utility.
- **`social_score`**: `composite_social_score` returns exactly `0.0` for every article with no `social` rows — the majority on any given day. Same ramp, 0.05 / 0.04 weight.
Article IDs are `AUTOINCREMENT` (`migrations/0001_init.sql:26`) and assigned in `persist_articles` iteration order, so low IDs are systematically older articles and articles from feeds that happened to sort earlier. The failure is silent: rankings look plausible, `candidate_rankings` looks populated, and the reader sees a subtly wrong paper.
**Fix (specify explicitly in §14.1):** equal raw values must receive **equal** normalized values — use mid-rank / average-rank percentiles (`(#below + (#equal + 1)/2) / n`). Article ID may break ties only in the *final output ordering*, never inside the normalizer. Add a unit test: "a signal that is constant across all candidates normalizes to 0.5 for every candidate," alongside the existing §27.6 "percentile normalization stable with ties" case, which as worded would pass with the buggy behavior.
### C2. Splitting `LlmScore.score` silently disables the churn-suppression rule
§16.1 replaces `score` with `quality_score` + `reader_fit_score`. §13.1 says the recently-rejected rule is preserved:
> §13.1: "recently rejected churn rule (LLM < 3 within the configured lookback), except always-includes."
That rule is implemented as `db.recently_low_scored_ids(STALE_LOW_SCORE, since)` (`prefilter.rs:104`), which reads `scores.llm_score` (`db.rs:325-339`), which is written only by `db.upsert_score` from `llm.score` (`db.rs:402`). If Stage A stops producing a field named `score`, nothing writes `scores.llm_score`, `recently_low_scored_ids` returns empty forever, and the rule dies with no error and no test failure. The consequence is not cosmetic: yesterday's rejects re-enter the recall pool every day, they consume facet-extraction and Stage A tokens, and `PENALTY_TITLE_PATTERNS`-class churn recirculates indefinitely.
The plan gestures at this — §16.1 says "Prefer a new type if changing `LlmScore` would make existing persisted `scores.llm_score` ambiguous" — but never states what writes the column going forward. Worth noting that `LlmScore` is **not** persisted as JSON anywhere (I checked `report.rs`, `server.rs`, `publish.rs`; `issue_articles` stores only section/position/summary), so the only real compatibility surface is this one column.
**Fix:** state in §16.1 and §26 that `scores.llm_score` continues to be written with `quality_score`, and add `llm_reader_fit_score` as a new column in `0002`. Add the regression test: "an article with `quality_score < 3` yesterday does not appear in today's recall pool."
### C3. The sparse-evidence confidence damping in §11.2 is a no-op
> §11.2: `confidence = total_decayed_rating_weight / (total_decayed_rating_weight + 6.0)`; `embedding_preference = embedding_preference_raw * confidence`
`confidence` is a **single scalar for the whole run** — it depends only on the rating history, not on the candidate. Multiplying every candidate's raw score by the same positive constant is a strictly monotone transform. It is therefore erased by:
- §14.1's percentile normalization (rank-invariant), which is what consumes `embedding_preference` in the §14.2 and §17 blends; and
- §13.3's "top `recall_embedding_preference_top` by learned embedding preference" (a pure top-K, also rank-invariant).
So the damping has no effect anywhere it is used. With three upvotes total, the embedding preference signal still contributes its full 0.28 / 0.15 weight, ranking on noise. This is the exact failure the paragraph was written to prevent.
**Fix:** damping must act on the **blend weight**, not the score. Replace with: compute `w_embed_effective = w_embed * confidence`, redistribute the freed weight proportionally across the remaining present signals, and record both in `explanation_json`. Alternatively blend the normalized signal toward the pool mean: `norm' = 0.5 + confidence * (norm - 0.5)`. Either works; the current formulation cannot. Same audit is needed for any other place the plan multiplies a whole-run constant into a per-candidate score.
---
## High
### H1. Signals with no evidence must be dropped from the blend and the weights renormalized
The plan does exactly the right thing *inside* facet preference — §11.3: "Normalize by the sum of weights actually present" — and then does not do it for the **outer** blends in §14.2 and §17. On a cold-start day:
- `embedding_preference` has no evidence (C3) → 0.28
- `facet_preference` has no evidence (H2) → 0.22, mapped through `(x+1)/2` to a constant 0.5
That is **50% of the pre-Stage-A score** that is either constant or ID-noise, silently compressing the dynamic range of the heuristic, interest, and social signals to half their intended influence. §17's utility has the same problem at 25%.
**Fix:** make the blend a weighted mean over *present* signals with per-signal presence tests (`has_positive_centroid`, `facet_dimensions_with_support > 0`, `social_rows > 0`), renormalizing to the weights actually present, and persist the effective weight vector into `explanation_json` so `explain` and `evaluate` can see it. This is ~15 lines and it is the difference between the system degrading gracefully and degrading invisibly.
### H2. The facet vocabulary is far too large to estimate from the available ratings
§10.1 defines 11 dimensions over roughly **84 controlled values** (14 topic groups + 18 formats + 4 depths + 5 technicality + 4 audience + 10 tones + 8 stances + 9 evidence modes + 4 temporal + 4 locality + 4 commerciality). §11.3 estimates a Beta rate per value from decayed ratings. The plan's own example telemetry in §24 says:
> `personalization: 63 recent ratings, 55 embedding-backed, 51 facet-backed`
63 ratings across 84 values, further split by up/down and diluted by multi-value averaging, means the median facet value will have **zero or one** observation. With `support = (u+d)/(u+d+4)`, a single observation gives `support = 0.2` and `effect = (0.67-0.5)*2*0.2 ≈ 0.067` — indistinguishable from noise. `facet_preference` will hover near zero for months while consuming 0.22 of the pre-Stage-A blend and one DeepSeek call per 12 recall-pool articles.
The plan is aware of the cardinality/evidence tradeoff — §10.1: "Keep controlled enums small enough that ratings accumulate statistical support" — and then does not follow its own rule.
**Fix, pick one:**
- **(Recommended)** Ship V1 with **four** dimensions and ~5 values each: `format` (collapse 18 → `reported | analysis_essay | tutorial_technical | postmortem_case_study | announcement_roundup`), `depth`, `evidence_mode` (collapse 9 → `first_hand | original_reporting | data_or_experiment | synthesis | speculative`), `commerciality`. That is ~20 parameters against 63 ratings — estimable. Keep the full vocabulary as `schema_version = 2` once there are ~300 ratings. The full enums are still worth extracting into `facets_json` for explanation and for the §12 profile prompt; just don't *score* on the sparse ones.
- Or gate the whole facet-preference contribution behind a minimum-evidence threshold and let H1's renormalization carry the weight elsewhere until then.
Note this also affects §12: "Enrich the rebuild prompt with saved facet data" is valuable at *any* cardinality, because the LLM is doing pattern recognition, not parameter estimation. That part should ship regardless.
### H3. The high-recall union has no quality floor on three of its four retrievers, and dense retrieval has a strong short-document bias
§13.3 admits candidates by "top 80 by semantic-interest score", "top 80 by learned embedding preference", "top 30 by feed affinity". Only exploration gets a floor (§15: "still above a minimal heuristic-quality floor so the system does not explore obvious junk").
Cosine similarity against a short query concentrates on short documents. A 60-word "Rust 1.94.0 released" changelog stub whose `embedding_document` (§8.1) is `Title: … / Source: … / <60 words>` will score *higher* against the interest query "Rust" than a 3,000-word essay that discusses Rust among other things — because the essay's vector is diluted across many topics. The same holds for the positive centroid. So the two semantic retrievers will systematically over-admit exactly the class §16.2 tells Stage A to punish ("announcements/roundups/vendor marketing are generally low quality"), and which `PENALTY_TITLE_PATTERNS` already exists to catch.
§13.4's "top 20 from each major retriever" protection bounds the damage to ~40 pool slots, but those slots cost facet-extraction tokens and displace real candidates.
**Fix:** apply the cheap existing hygiene to the semantic paths — require `word_count >= ~250` and `!looks_like_roundup(title)` for admission *via the semantic-interest or embedding-preference retrievers only* (an article can still enter via heuristic or auto-include). None of the plan's own motivating examples are affected: §32's "quiet 900-word post" clears 250 comfortably. Add the inverse regression test alongside §27.5's: "a 60-word release-note stub with very high interest similarity does **not** enter the recall pool."
### H4. The plan roughly doubles sequential LLM round-trips and never mentions concurrency
`score_all` batches serially (`score.rs:344`, a plain `for` over `chunks`). The new pipeline adds facet extraction over the ~240-article recall pool at `facet_batch_size` 1216 — **15 to 20 additional sequential DeepSeek calls** — and simultaneously grows every Stage A prompt from a 200-word excerpt (`score.rs:21`) to a ~450-word beginning/middle/end sample (§10.2), which lengthens each call. Voyage adds ~13 more sequential calls at `batch_size = 32` over ~400 articles.
Token *cost* is not the issue (~240 × 600 tokens ≈ 145K input for facets, well inside `max_daily_usd = 2.0`). **Wall clock is.** This job runs on a 05:30 America/New_York timer and has to produce an EPUB before breakfast; adding 30+ sequential API round-trips to a stage that is already the slowest is a real delivery risk, and the plan's §24 stage-timing list implicitly acknowledges the concern without addressing it.
**Fix:** specify bounded concurrency (`futures::stream::iter(batches).buffer_unordered(4)`; `futures` is already a dependency) for facet extraction, Stage A, and Voyage batching, with the `UsageMeter::check_budget` gate evaluated before each spawn rather than between batches. Add the budget-trip semantics under concurrency to §23 — currently "Once tripped, remaining calls for that provider are skipped for the run" is written assuming a serial loop.
### H5. `candidate_rankings` reruns will interleave two runs' state unless the write is a full replace
> §5.4: "Persist rows incrementally as stages complete. A rerun for the same date should replace/update the day's rows deterministically."
"Replace/update" is ambiguous, and the existing house idiom is the opposite of what is needed here: `db.upsert_score` uses `COALESCE(excluded.x, scores.x)` (`db.rs:394-397`), which deliberately *preserves* prior values. If `candidate_rankings` copies it, a rerun that trips the budget at the recall stage will leave yesterday's `llm_quality_score`, `stage_a_candidate = 1`, and `selected = 1` attached to rows the current run never scored. This table is the ground truth for §21's entire evaluation program and for acceptance criterion 12; silently corrupt evaluation data is worse than no evaluation data.
**Fix:** mandate `DELETE FROM candidate_rankings WHERE run_date = ?` at the start of the recall stage, inside the same transaction as the first batch of inserts — matching `replace_issue_articles` (`db.rs:475-497`), which is the correct existing precedent. Add to §27.8: "rerunning a date that previously reached Stage B, but which now trips the budget at recall, leaves no stale Stage A/B flags."
### H6. `explain` and acceptance criterion 10 cannot be satisfied by the proposed schema
> §30, criterion 10: "`candidate_rankings` records why every eligible daily article did or did not survive each funnel stage."
But §5.4 persists "every **post-hygiene** candidate," and the most common exclusions happen *before* that: already-published, blocked domain, recently-rejected churn (`prefilter.rs:271-287`). Those articles get no row at all, so the single most frequent answer to "why did this article not show up?" is unanswerable from the table. The boolean flag set also cannot distinguish "did not make the recall union" from "made the union but was cut by the cap in §13.4."
**Fix:** add `excluded_reason TEXT` (nullable; `published | blocked | churn | not_recalled | recall_cap | stage_a_cut | mmr_cut | not_selected`) and write a row for every article the run considered, hygiene-excluded ones included, with only that column and the identifying keys populated. Cost is ~400 thin rows/day. Then criterion 10 is actually testable.
---
## Medium
### M1. Backfill has no cost estimate or guard, and embedding storage grows unbounded
§20.1 step 3: "Embed recent articles (e.g. last 90 days) for historical replay/exploration if desired." Nothing prunes `articles``publish::prune` only removes EPUB/XTC *files* (`publish.rs:491-547`), and `retention_days = 21` is a file policy. At ~400 articles/day, a system that has been running 90 days holds ~36,000 article rows; embedding all of them is ~47M tokens — a quarter of the lifetime 200M free allocation spent by one command with no confirmation, across ~1,125 requests.
Storage compounds: 1024 × f32 = 4,096 bytes plus row overhead, ~1.7 MB/day, **~600 MB/year** of SQLite BLOB on a VPS, with no retention policy anywhere in the plan.
**Fix:** (a) require `features backfill` to print an estimated token count and USD cost and require `--yes` above a threshold; default `--days` to 30 and default to `--rated-only`. (b) Add a retention rule to §5.1: drop embeddings for articles that are neither rated nor published and are older than N days. (c) Reconsider `output_dimension = 512` as the **default** rather than 1024 — Voyage's Matryoshka training makes 512 near-lossless for retrieval, it halves storage and dot-product cost, and the plan already requires the dimension to be configurable (§3). For a single-reader system on a small host, 512 is the better default and 1024 is the thing you evaluate into.
### M2. `Source: <feed title>` in the embedding document contradicts §8.1's own rule and degrades MMR
§8.1 is emphatic — "Do not include social score, ratings, feed prior, LLM rationale, or other ranking metadata … The vector should represent the article itself" — and then includes `Source: <feed title>` in the document format. Feed title *is* provenance metadata. Two consequences:
1. The positive centroid partly encodes "feeds the reader upvotes," double-counting with the separate `feed_affinity` signal (0.08 pre-Stage-A, 0.05 utility) that the plan went to some trouble to de-bias in §5.5.
2. **MMR degrades**: two unrelated posts from the same blog become artificially similar, so §18's diversification will suppress the second post from a favored feed as "redundant" when it is not. This is the one calculation where topical purity actually matters.
The effect is small for a 2,000-word article and material for a 200-word one — compounding with H3.
**Fix:** drop `Source:` (and probably `Author:`) from `embedding_document` v1. Keep the `EMBEDDING_DOCUMENT_VERSION` constant so this is an easy A/B later.
### M3. Max-similarity over 230 standing interests will not discriminate, because most interests are broad single words
I parsed `data/scour-interests.opml`: 230 interests, dominated by short generic terms — `Nature`, `History`, `Space`, `Engineering`, `Science`, alongside specific ones like `Gaussian Splatting`, `Writerdeck`, `tmux`. §9.2 takes `0.70 * top1 + 0.30 * mean(top3)` of raw cosine similarity across all of them.
Broad terms have high *average* similarity to everything. So `top1_similarity` will almost always be one of the generic interests, at a value that varies little between articles, and the score mostly measures "how generic is this article" rather than "does this match a stated interest." The genuinely valuable signal — "this article is *unusually* close to Gaussian Splatting" — is exactly what max-of-raw-cosine destroys.
**Fix:** z-score each interest's similarity **across the day's candidate pool** before taking top-1/top-3: `z_i(a) = (sim_i(a) - mean_a sim_i(a)) / std_a sim_i(a)`. This is free — you have already computed the full 230 × 400 matrix — and it converts "close to a broad term" into "unusually close to *this* term," which is what you want for both the score and the top-3 explanation shown to Stage B. Persist raw similarity too, as §9.2 already requires.
Relatedly: `input_type = "query"` already causes Voyage to prepend "Represent the query for retrieving supporting documents" (verified in the live API reference), so the `"Articles about: "` prefix is a second, redundant instruction that is identical across all 230 interests — it pulls all interest vectors toward each other and further compresses the `top1 top3` gap. Worth testing the bare interest name as v2 of the interest text format.
### M4. Exploration is unbounded at exactly the moment it does the most damage
§15's V1 definition admits candidates "from a feed with low rating evidence **or** semantically outside the dense region of recent positive ratings." On day one there are no ratings, so *every* feed has low evidence and there is no positive region — the predicate is universally true, and 20 slots in the recall pool plus a guaranteed shortlist reservation (§18.2 rule 4) plus Stage B exposure go to articles chosen by `hash(run_date, article_id)`. That is a lot of deliberate noise injected during the phase where you are trying to measure whether the new ranker beats the old one.
"Semantically outside the dense region" is also the one place the plan drops below implementation grade — no definition, no threshold.
**Fix:** make `recall_exploration` scale to zero when `total_decayed_rating_weight` is below a threshold (~15), and define "outside the dense region" concretely as `positive_similarity < 25th percentile of the day's candidate distribution`. Default the reservation to 8, not 20, until Phase D.
### M5. Phase A shadow mode cannot shadow what the plan implies it shadows
> §28 Phase A: "New ranker computes in shadow mode and persists what it _would_ have done. Compare old vs new selections for several days."
The new utility score (§17) is 40% LLM quality + 15% reader fit, and those fields do not exist until Phase C enables the new Stage A. So the Phase A shadow can only compute the non-LLM 45% of utility, and "compare old vs new selections" is not achievable — the shadow shortlist would be ranked on less than half its intended signal.
That is fine, and the honest framing is more useful anyway: **Phase A should shadow the recall and pre-Stage-A stages only**, which is precisely §21.2's metric 6 ("count historical upvoted articles that would have been lost at each proposed stage") — the plan's own "one of the most important metrics." That comparison is fully computable in Phase A and is the single best evidence for whether the recall redesign is justified.
**Fix:** rewrite Phase A's exit criterion as "recall-boundary diagnostics show the union recovers upvoted articles the current top-120 would have dropped," and move selection comparison to Phase C.
### M6. Voyage's daily ceiling will not survive a rerun, unlike DeepSeek's
§23 promises "DeepSeek and Voyage meters trip independently," but §7.4 says "Database columns for Voyage tokens/cost are optional in the first migration if the JSON report is sufficient." DeepSeek's ceiling is day-scoped because `pipeline.rs:379-386` preloads `db.spend_for_date(date)` from the `runs` table. Without an equivalent column, `voyage.max_daily_usd` is per-*invocation*, and `generate --date X` reruns are a normal, documented workflow (idempotency is a stated invariant).
Impact is genuinely low — the embedding cache makes reruns nearly free — but the asymmetry is a trap for whoever debugs a budget trip later.
**Fix:** add `voyage_input_tokens` / `voyage_cost_usd` columns to `runs` in `0002` and preload them the same way. It is four lines and removes a whole class of confusion.
### M7. Replay is reproducible for scalars but not for vectors, and the plan overstates it
> §21.1: "For exact future replay, `candidate_rankings` becomes authoritative."
`article_embeddings`'s primary key is `(article_id, model, dimension)` with `input_hash` as a *non-key* column, so a re-extraction overwrites the vector in place. `upsert_article` overwrites `content_html` on every re-ingest of the same `canonical_url` (`db.rs:272-279`), which happens routinely because the 26h lookback window overlaps consecutive days. So the vectors used for MMR and for §21.2's metric 5 (shortlist diversity) are not recoverable for a past date.
Additionally, percentile normalization is **day-relative**: re-tuning weights on historical data requires recomputing percentiles from the full day's candidate set, which requires a `candidate_rankings` row for every eligible article. §5.4 provides that (and H6 would complete it), so the scalar path works — but only if the evaluator recomputes percentiles from stored *raw* values rather than trusting stored normalized ones.
**Fix:** state explicitly in §21.1 that (a) `evaluate` recomputes normalization from raw columns and never trusts persisted normalized values across code changes, and (b) vector-dependent metrics (diversity, MMR replay) are approximate for historical dates. Do not add embedding versioning to fix this — the storage cost is not worth it; just stop claiming exactness.
### M8. `--max-articles` is not currently a hard ceiling, and the plan assumes it is
> §19.2: "`--max-articles N` should **remain** a hard ceiling/override, not a target that forces filling."
It is not one today. `pipeline.rs:188` assigns `--max-articles` to `target`, and `select.rs:187-193` derives `(target - 5, target + 5)`. So `--max-articles 10` today permits **15** picks and forces a floor of 5. An agent reading "remain" will assume the behavior already exists and not fix it.
Also unaddressed: how `--max-articles` composes with the new `max_article_count = 25`. Presumably `effective_max = min(max_article_count, --max-articles)` with no floor at all.
**Fix:** reword to "`--max-articles N` must **become** a hard ceiling" and specify the composition rule.
---
## Low
- **L1 — `assemble()` loses its sort key.** §26 says to replace `ScoredArticle::combined_score()` with the `rank.rs` utility, but `assemble` uses `combined_score()` in three places (`select.rs:545`, `:609`, and via `sort_by_combined` at `:563`) for oversize trim and intra-section ordering. §19.2 says "keep the max-size trim" without saying what it sorts by. Specify: trim and order by `utility_score`, falling back to `prefilter_score` when utility is absent.
- **L2 — `select_without_llm` ordering not updated.** §23 says the DeepSeek-unavailable path should "use the enhanced deterministic ranking … rather than reverting all the way to old prefilter order," but §26's `select.rs` bullets don't mention `select_without_llm`, which calls `sort_by_prefilter` directly (`select.rs:660`). Cross-reference the two sections.
- **L3 — MMR seed rule looks like a slip.** §18.2 rule 1: "Seed with the highest-utility **non-auto** candidate." If an auto-include is the day's best article, it should seed. Also unstated: whether the rule-5 force-preserved top-20 count as `already_selected` for the max-similarity term (they must, or MMR will re-select near-duplicates of them).
- **L4 — "exploration/novelty bonus" has no definition.** §14.2 gives it 0.05 of the pre-Stage-A blend, but §15 defines exploration as boolean set membership. Either make it a flat additive bonus for `exploration_candidate` articles, or define the continuous novelty measure (e.g. `1 - max similarity to the positive centroid`).
- **L5 — `article_facets` omits `model` from its primary key** while `article_embeddings` includes `model`. Switching DeepSeek models silently reuses facets extracted by the previous one. Either add `model` to the key or state that facets are deliberately model-agnostic and that `prompt_version` is the invalidation lever.
- **L6 — `--skip-llm` / `--skip-embeddings` are inconsistent.** §23 recommends that `--skip-llm` also disable Voyage generation "and add `--skip-embeddings` later only if a real operator need appears" — but §20 already specifies `features backfill --embeddings-only`, which *is* that need. Cleaner: `--skip-llm` gates DeepSeek only, `--skip-embeddings` gates Voyage, both shipped in the same commit. One extra boolean.
- **L7 — `prefilter_keep` validation must move.** `config.rs:348` enforces `prefilter_keep >= target_article_count`. If §25 deprecates or aliases it to `stage_a_keep`, that check needs relocating, and the new constraints (`recall_pool_keep >= stage_a_keep >= shortlist_keep`, `0 <= diversity_lambda <= 1`) need adding. §26's config bullet lists some of this; add the ordering constraints explicitly.
- **L8 — Correlated "independent" signals.** §16.3 correctly forbids showing numeric preference scores to the reader-fit rubric, but reader-fit *is* shown the taste profile, whose learned-adjustments section is now (§12) enriched with facet data derived from the same ratings that produce `facet_preference`. §17's claim that "30% of the score is direct rating-derived preference" understates the true rating-derived share (~40%) and, more importantly, those components share error. Not a blocker — just note it as a correlation to watch in §21.2 rather than asserting independence.
- **L9 — Dry-run behavior with `candidate_rankings` unstated.** Articles are persisted even under `--dry-run` (`pipeline.rs:320-332`), so ranking rows will be written on dry runs. Probably desirable for Phase A shadow work; say so.
---
## Nits
- **N1 — Stale file references.** §"read the current curation implementation" lists `src/curate/profile.rs`; it is `src/curate/profile/mod.rs` plus `themes.rs`. §6's module layout repeats the flat `profile.rs` and omits `editorial.rs`, which exists. An agent following §6 literally might collapse the profile module. For a plan that is explicitly "implementation-grade," these should be exact.
- **N2 — "configured lookback" for the churn rule doesn't exist.** §13.1 says "within the configured lookback"; `STALE_LOOKBACK_DAYS` is a `const` (`prefilter.rs:59`), not config. Either make it config as part of this work or drop the word.
- **N3 — Acceptance criterion 11 is nearly vacuous.** "`features backfill` can populate at least all historical rated articles **without network calls in tests**" — every test is offline per notes §6. The meaningful criterion is "backfill is resumable and idempotent: re-running it makes zero API calls when the cache is warm."
- **N4 — Root config section is not typo-protected.** Nested config structs use `#[serde(deny_unknown_fields)]`, but `Config` itself deliberately does not (`config.rs:41-43`, so that bare `DAILY_EPUB_SECRET` passes through). A `[voyages]` typo will therefore be silently ignored and defaults used. Worth a line in §7.1 telling the operator to verify via the startup log rather than assuming.
- **N5 — `voyage.max_daily_usd = 0.25` is a runaway guard, not a cost ceiling.** At `$0.02`/Mtok that trips at 12.5M tokens/day, ~25× expected volume, and the meter cannot know whether the 200M free allocation is exhausted. Fine as designed — just describe it as a runaway guard so nobody tunes it as if it were a bill.
- **N6 — `PENALTY_TITLE_PATTERNS` becomes redundant once facets exist.** `ArticleFormat::{roundup, release_notes, announcement}` subsumes the 22-pattern title list (`prefilter.rs:27-50`) with far better recall. §26 says to preserve current heuristic values for evaluation, which is right for V1; add a note that retiring the title-pattern penalty is a Phase E cleanup candidate.
- **N7 — Verified-facts section deserves a re-verification date.** §3's Voyage facts are correct as of today (I confirmed model, context, dimensions, dtypes, 1,000-input/1M-token request limits, `$0.02`/Mtok, and the 200M free allocation against the live docs). Add a note that this block should be re-checked whenever `model` changes, in the same spirit as the 2026-08-15 notes.
---
## Alternatives
### A1. Skip facets in V1; fit a regularized linear probe on the embeddings instead *(strongest alternative)*
The plan's own §11.2 already computes a positive and a negative centroid. The difference of two class centroids is the closed-form solution of a specific naive classifier — it weights every embedding dimension equally. A **ridge-regularized logistic regression** on the same labels is the same idea done properly: it learns *which* dimensions discriminate, it is convex, it needs no new dependency (a few hundred lines of gradient descent over `Vec<f32>`, or closed-form ridge on 256-d), and it handles the correlated-positives problem in §11.2 that the centroid difference cannot.
Crucially, it addresses H2's evidence problem head-on: with 63 labels you cannot estimate 84 facet parameters, but you *can* fit a heavily-regularized 256-dimensional linear model, because regularization is exactly the tool for the low-n regime — and it costs **zero DeepSeek tokens**.
"Likes first-hand postmortems, dislikes vendor announcements" is substantially linearly separable in a good embedding space; that is what embeddings are for. Facets buy explainability and the §12 profile-prompt enrichment, both real, but neither requires facets to be in the *scoring* path.
**Prefer this when:** rating volume is under ~200 and you want the immediate-feedback property (§30 criterion 4) working on day one. **Prefer the plan's approach when:** rating volume is high enough to estimate facet stats, or when the explainability of "you downvote `commerciality = product_marketing`" is worth more than ranking accuracy — which for a single-reader system it genuinely might be.
**Concrete middle path:** ship facets for extraction, storage, `explain`, and the §12 profile prompt (all cheap and all valuable), but have the *numeric* scoring path use a linear probe on embeddings until facet evidence crosses a threshold. That gets both properties and lets §21 compare them empirically.
### A2. Threshold clustering instead of MMR for diversification
§18's MMR introduces `lambda = 0.82`, a magic number whose meaning is not interpretable in isolation, and it composes awkwardly with rule 5 ("preserve the top ~20 by raw utility regardless of MMR" — at which point you are no longer running MMR, you are running force-include-then-MMR).
**Alternative:** single-linkage cluster the shortlist candidates at cosine > ~0.85, then take the top-N by utility with a cap of 2 per cluster. One interpretable parameter (the similarity threshold, which you can eyeball against real article pairs), trivially composable with auto-includes and the top-20 preservation rule, and dramatically easier to render in `explain` ("suppressed: 3rd article in the cluster led by #4821").
**Prefer MMR when** you want a smooth relevance/diversity tradeoff across the whole ranking. **Prefer clustering when** the actual problem is the one §32 describes — "six articles about the same AI news cycle" — which is a discrete cluster-cap problem, not a continuous one. I would ship clustering first and reach for MMR only if it proves too blunt.
### A3. Two vectors per article: title+lead for interest matching, full body for preference and MMR
Addresses H3 and M2 at the root rather than by filtering. The short-document bias in interest matching is not really a bug — it reflects that interest matching *should* be title-like. The problem is using one vector for two jobs with opposite length preferences.
Embed `Title + first ~100 words` with `input_type = "document"` for standing-interest matching, and the full capped body for the rating centroids and MMR. Cost is one extra Voyage call per batch (~13/day, negligible against a 200M free allocation), storage doubles (mitigated by A4), and both jobs get a vector shaped for them. The `article_embeddings` PK already accommodates this if you add a `kind` column or fold it into `model`.
### A4. Default to `output_dimension = 512`
Voyage's Matryoshka training makes 512-d near-lossless for retrieval on general text. It halves BLOB storage (~300 MB/year instead of ~600), halves every dot product (230 interests × 400 articles × 2 = 184K dot products/day either way — not a bottleneck, but MMR is O(n²)), and the plan already mandates configurability. For a single-reader system where the DB shares a VPS with the EPUB output directory, 512 is the better *default*; 1024 is the thing you evaluate into if §21 shows a measurable difference. This also makes A3's second vector free in storage terms.
---
## Open Questions
These block confident approval; each is answerable quickly and each changes what gets built.
1. **What writes `scores.llm_score` after the Stage A split?** (C2) Without an answer the churn rule dies silently. If the answer is "nothing, migrate `recently_low_scored_ids` to `candidate_rankings`," the `0002` migration and `prefilter.rs` change scope.
2. **How many ratings exist in the production database today?** If the count is under ~100, H2 stops being a tuning concern and becomes blocking: the entire facet-scoring path would be dead weight for the first several months, and A1 becomes the right V1.
3. **How many rows are in the production `articles` table?** Determines backfill token spend (M1) and whether the ~600 MB/year embedding storage projection is tolerable on the host. Answerable with one `SELECT COUNT(*)`.
4. **What is the current wall-clock runtime of `generate`, and what is the hard publish deadline?** (H4) If the run currently takes 4 minutes against a 30-minute window, serial batching is fine and H4 downgrades to a nit. If it takes 20 minutes, concurrency is mandatory before any of this ships.
5. **Does `[curation.personalization] enabled = false` (§28) disable embedding *generation*, or only the new ranking path?** If it disables generation, Phase A collects no data and the shadow phase is impossible. If it does not, the flag name is misleading and the config docs need to say so.
6. **Is the reader willing to accept a visibly noisier paper during Phase AB?** M4's exploration reservation plus H3's short-document admissions will both be most visible exactly when the system has the least evidence. If the answer is no, exploration should start at zero and ramp with rating volume.
@@ -0,0 +1,173 @@
# Review: Personalized Ranking, Embeddings, Facets, and Feedback
The plan has a strong overall direction—union-based recall, separate descriptive facets, explicit quality/fit scores, pre-editor diversification, and removal of forced filler are all sensible—but it is not ready to execute as written. The principal blockers are that the proposed persistence model cannot provide the promised per-run/exact replay guarantees, facet cache invalidation contradicts its schema, historical runs can leak future feedback, the single positive/negative centroid is too lossy for the stated multi-interest personalization goal, and the rollout/budget and target/ceiling semantics are internally inconsistent. Resolve the High findings before implementation; the remaining items can be handled during staged delivery.
## Critical
No critical findings.
## High
### 1. The facet cache key cannot honor the stated invalidation rules
Evidence: §5.3 defines `PRIMARY KEY (article_id, schema_version)` while also storing `model`, `prompt_version`, and `input_hash`; it says “`prompt_version` changes when instructions change” and “Reuse a facet row only if schema version and content hash match.” §10.4 then describes the cache as `(article_id, schema_version, input_hash)`.
With the proposed primary key, a prompt or model change either reuses stale output or overwrites the previous observation. Worse, §10.3 proposes placing the mutable reader taste profile in the system prompt for a supposedly descriptive extraction task, but neither the profile version nor its text hash participates in invalidation. Facets would therefore be reader/profile-dependent while appearing globally reusable and stable.
Recommendation:
- Remove the taste profile entirely from facet extraction. Use a stable, reader-independent descriptive system prompt and treat article text as untrusted quoted data.
- Make the cache identity explicit, for example `(article_id, schema_version, model, prompt_version, input_hash)`, or use a surrogate `facet_observation_id` plus a uniqueness constraint over those fields.
- Define `input_hash` over the exact effective facet input: excerpt-format version, title/author/source fields actually sent, and the representative text. Do not call it merely a “content hash.”
- Decide whether old observations are retained for replay or superseded; do not silently overwrite them if exact replay remains a goal.
### 2. `candidate_rankings` is neither per-run nor sufficient for exact replay
Evidence: goal 7 promises “replayable from persisted per-run features,” §5.4 calls the table essential, and §21.1 says it becomes authoritative “For exact future replay.” Yet its primary key is `(run_date, article_id)`, despite the existing `runs` table allowing multiple invocations per date. The plan explicitly says reruns replace the dates rows.
This loses shadow-versus-live results, failed/partial attempts, changed configurations, and the exact feature versions used. It also omits the ranking configuration/profile/model/prompt versions and per-retriever membership/cut reason. Mutable `article_embeddings` and `article_facets` rows can be overwritten, so a later MMR replay cannot reconstruct the original pairwise similarities. `explanation_json` is not a substitute unless its required schema and versioning are specified.
Recommendation:
- Key snapshots by `run_id` (foreign key to `runs`) and `article_id`; keep `run_date` as an indexed denormalization.
- Add a run-level immutable ranking manifest containing personalization mode (`shadow`/`live`), all normalized weights and thresholds, model/dimension versions, facet schema/prompt version, embedding/excerpt format versions, profile version/hash, feature availability, and deterministic algorithm version/seed.
- Persist retriever membership and explicit terminal reason (`blocked`, `published_before_cutoff`, `not_in_union`, `recall_cap`, `stage_a_cut`, `mmr_cut`, etc.). The current booleans cannot explain why an article failed to enter a stage.
- Either retain versioned embedding/facet observations referenced by the run or persist enough pairwise/MMR inputs to reproduce the shortlist. Narrow the claim from “exact replay” to “score diagnostics” if that storage is not desired.
- Write a run snapshot transactionally or mark its lifecycle (`running`, `complete`, `failed`) so evaluation does not treat a partial run as authoritative.
### 3. Historical generation/evaluation has undefined “as-of” semantics and can leak future data
Evidence: §11 says to build preference state “at the beginning of every generate run” using recent ratings, §20 adds historical backfill/evaluation, and §21 proposes historical replay. The plan does not say that ratings must be bounded by the simulated run time. It also says recall should reuse current prefilter history logic (§13.1/§26), but the current repositorys `previously_published_ids()` returns articles from every issue, including issues after a historical target date, and the current profile code anchors its rating window to `Timestamp::now()`.
A replay of August 1 performed on August 18 could train on August 218 votes, use the latest prose profile, and exclude an article because it was published on August 10. That produces optimistic evaluation and non-reproducible historical runs.
Recommendation:
- Define one `as_of` timestamp for every run/evaluation and require all ratings, issue history, feed priors, profile state, and candidate publication/history queries to use it.
- For an issue dated `D`, exclude only articles published in issues before the chosen cutoff; never use future issue rows.
- Store or reconstruct profile versions by effective interval. If historical prose profiles are unavailable, explicitly disable that feature in replay and label the limitation.
- Separate “regenerate an old issue using knowledge available today” from “historical replay as of that day” as distinct CLI modes.
- Add leakage tests in which future ratings/issues exist but do not affect an as-of replay.
### 4. One global positive and negative centroid collapses the readers multi-modal taste
Evidence: §9 describes roughly 220 standing interests, while §11.2 reduces all recent upvotes to one unit-normalized positive centroid and all downvotes to one negative centroid. §32 expects a highly favored niche to be rescued early.
A single average vector is a poor representation of a reader who likes unrelated clusters such as Rust, local Boston reporting, books, and e-ink. Niche vectors may be only weakly similar to the global mean. The negative centroid also conflates topic rejection with format/quality rejection; the plan acknowledges this caveat but still gives the combined embedding preference 1528% of ranking weight. This can work against the main recall goal before facets or Stage A can rescue the article.
Recommendation: evaluate a signed, time-decayed top-k neighbor signal as the V1 baseline (`top/mean similarity to recent upvotes` minus a configurable downvote term), optionally grouped by topic cluster. At this scale it is simpler than centroid maintenance and preserves multiple modes. If centroids remain, keep several clusters or combine centroid and nearest-neighbor signals, and make a synthetic multi-interest recall test an acceptance criterion—not just a one-topic centroid test.
### 5. Shadow mode can consume or trip the same DeepSeek budget needed by the production selector
Evidence: §28 Phase A says facets and new snapshots run in shadow while “Existing production selection remains authoritative.” §10.3 adds facet extraction for about 240 articles, and §23 says DeepSeeks meter trips independently once its ceiling is reached. The current pipeline uses a single per-day DeepSeek meter for scoring, selection, profile, and editorial work.
If shadow facet calls run before the existing production stages, they can exhaust the shared daily ceiling and force the supposedly authoritative path into fallback. This is a behavior change, not passive shadowing. Persisting Voyage cost only in report JSON also cannot reliably preload/enforce a provider-specific daily ceiling across reruns or dry runs.
Recommendation:
- Run authoritative production calls first, then shadow work from a separately configured shadow budget, or reserve explicit provider/stage budget slices before shadow calls.
- Persist provider usage by `run_id` in queryable columns or a `run_provider_usage` table and preload same-date spend, as the current DeepSeek path does. “JSON is sufficient” is not compatible with a durable daily guardrail.
- State whether failed requests, retries, dry runs, and concurrent runs count toward the budget, and make reservation/accounting atomic enough to prevent two runs overspending simultaneously.
### 6. Soft target, hard ceiling, auto-includes, and `--max-articles` have contradictory precedence
Evidence: §19.2 says target approximately 20, never exceed `max_article_count` 25 “plus unavoidable auto-includes,” then says `--max-articles N` is a “hard ceiling/override.” §18.2 also allows auto-includes and other protected sets to exceed nominal shortlist size. In current code, `--max-articles` replaces `target_article_count`, so merely reusing the existing plumbing would tell Stage B to aim for the ceiling rather than keep the normal soft target.
Recommendation:
- Carry `soft_target` and `hard_max` as separate values through the pipeline and Stage B prompt.
- Define one precedence rule for auto-includes. Either they can exceed the hard max (then call it a normal-content ceiling and report the exception) or the CLI ceiling is truly hard (then reject conflicting configuration or specify which auto-includes win).
- Define the exact shortlist-cap precedence among raw-utility preservation, exploration reservation, auto-includes, and MMR; specify whether protected items seed MMR similarity calculations.
- Add tests for `--max-articles` below, equal to, and above the soft target, including excess auto-includes.
## Medium
### 1. Missing-signal normalization is underspecified and can turn degradation into a penalty
Evidence: §14 percentile-normalizes daily signals, while §23 says missing embedding signals become “neutral, not zero-quality penalties.” The plan does not define the empirical population, neutral value, behavior for all-equal/singleton inputs, or whether missing values participate in percentile ranking.
Recommendation: define a typed normalization contract. Exclude missing values from the empirical CDF, assign missing signals an explicit neutral value (normally 0.5), return 0.5 for degenerate/all-tied distributions, and store availability flags alongside raw and normalized values. Test mixed cached/missing embeddings so an outage does not systematically demote uncached articles.
### 2. The preference evidence model needs vote-time and exposure semantics tightened
Evidence: §11 decays ratings by `age_days` without saying whether age is based on `rated_at` or `issue_date`; §5.5 keeps `included` exposure metadata but §11.3s support uses only explicit votes. A vote can arrive long after publication, and repeated flips update `rated_at` in the current schema.
Recommendation: use `rated_at` for behavioral recency but document that a flip resets recency, or preserve the initial and last-changed timestamps separately. Keep exposure out of the label, as planned, but persist whether/when an item was shown so evaluation can distinguish unshown, shown-unrated, and rated items. Anchor all calculations to the runs `as_of` timestamp.
### 3. Feed-prior rebuilding needs an atomic, fully specified source policy
Evidence: §5.5 says split credit among direct `Feed` sources, fall back to `best_entry_id`, then use a “weighted mean” of future source priors without defining weights. §22 allows either recomputation or transactional updates. The current rebuild path performs independent upserts and does not clear obsolete rows.
Recommendation: define deduplication by `feed_id`, the exact candidate-affinity weights, and what happens when the best-entry fallback is itself a discovery feed. Recompute the complete v2 table in one transaction (temporary table plus replace, or delete/upsert under a transaction) so generation never reads a partial rebuild and stale zero-evidence rows disappear.
### 4. LLM inputs need an explicit prompt-injection and data-handling policy
Evidence: §§10.3, 16.4, and 19.1 send extracted third-party article text to DeepSeek, and §8 sends up to 60,000 characters to Voyage. The plan discusses API-key secrecy but not untrusted instructions embedded in article text, provider data handling, or operator opt-out for private/authenticated feeds.
Recommendation: delimit article content as data, explicitly instruct the model to ignore instructions found inside it, validate outputs only against offered IDs/enums, and cap/escape metadata consistently. Document that article text is sent to external providers and add a feed/domain-level “local only / do not send” policy if private feeds are possible. Confirm the providers retention/training terms before rollout.
### 5. The new facet stage is expensive but its necessity is not tested against cheaper alternatives
Evidence: §10 adds a DeepSeek call for roughly 240 articles every day before Stage A, while §27 tests parsing and caching but not whether facets are stable or improve ranking. Many proposed fields (depth, technicality, temporal orientation, commerciality) may be derivable cheaply or bundled into Stage A for only 120 candidates.
Recommendation: in shadow mode, measure inter-run facet stability and incremental ranking value. Consider extracting cheap deterministic facets before recall, asking Stage A for descriptive facets alongside quality/fit for its 120 candidates, or using the embedding provider only for topic retrieval and delaying facets until enough ratings justify them. Keep the separate 240-item call only if it measurably rescues candidates at the pre-Stage-A cut.
### 6. The migration and rollout plan needs explicit compatibility behavior for partially deployed code
Evidence: §5 says keep `scores` temporarily, §26 says split `LlmScore`, and §28 phases behavior over several deployments, but the plan does not define which binary versions can safely run against migration 0002 or how Stage A v1/v2 values coexist. `scores.llm_score` becomes semantically ambiguous during Phase B/C.
Recommendation: version the assessment (`assessment_version`, model, prompt version), keep v1 and v2 writes distinguishable, and specify read precedence during each phase. Add forward/backward deployment tests or explicitly require a stop-the-world binary migration for this single-operator service.
## Low
### 1. The provider facts are current, but pricing and compatibility should remain metadata
The choices in §3 match the current official Voyage documentation: `voyage-4-lite` supports the stated context length/dimensions and request limits, and the listed price is currently correct. The provider also states that Voyage 4-series embeddings are mutually compatible, which means the plans blanket “Never compare vectors with different model” rule is conservative rather than technically required. Conservative isolation is reasonable for reproducibility; record model IDs in the run manifest and only relax compatibility after an explicit evaluation. See [Voyage embeddings](https://docs.voyageai.com/docs/embeddings), [API reference](https://docs.voyageai.com/reference/embeddings-api), and [pricing](https://docs.voyageai.com/docs/pricing).
### 2. Character caps must be Unicode-safe and do not guarantee the claimed token margin
Evidence: §7.3 uses 60,000 normalized characters per article as an aggregate-token safety proxy, while §8.1 says to cap text but only §10.2 explicitly warns against unsafe UTF-8 splitting.
Recommendation: make every cap Unicode-safe, enforce both per-input and aggregate character budgets before batching, and handle server truncation/token-limit errors explicitly. Log truncation counts without article text.
### 3. MMR should clamp/validate similarities and define the utility scale
Evidence: §18 uses `normalized_utility` but §17 stores utility on a 0100 scale. Floating-point embeddings loaded from storage are only length-checked, not checked for finite values or unit norm.
Recommendation: define whether MMR uses utility divided by 100 or a percentile, reject non-finite vector values, and normalize or verify vector norms within tolerance before dot products. Clamp small floating-point overshoots when using cosine-like scores.
## Nits
- §5.4 says “every post-hygiene candidate,” while acceptance criterion 10 says “every eligible daily article.” Define whether blocked, previously published, and recently rejected articles receive rows with terminal reasons. Logging them is necessary to explain hard exclusions.
- Use SQLite integer `0/1` plus `CHECK` constraints for ranking flags if strictness matters; SQLite does not enforce a separate Boolean storage class.
- `specific_topics: Vec<String> // 0..=4` and other cardinality comments require explicit validation after deserialization; Serde alone will not enforce them.
- The exploration hash should include an algorithm/version salt in the run manifest so an implementation change does not masquerade as reproducible behavior.
- Replace approximate terms such as “top ~20” and “small number of exploration candidates” with configuration fields and deterministic defaults before handing the plan to an implementation agent.
## Alternatives
### Alternative A: signed nearest-neighbor preference instead of one centroid
Keep recent rated document embeddings and compute a time-decayed top-k positive similarity and top-k negative similarity for each candidate. This preserves unrelated interest clusters, is explainable (“similar to these three upvotes”), and is trivial at the repositorys scale. Prefer this for V1 when ratings are sparse and multi-modal. Add clustered centroids later if the rating history becomes large enough for neighbor scans to matter.
### Alternative B: immutable feature observations plus run manifests
Store embeddings and facets as immutable observations keyed by model/prompt/input versions, and let each `candidate_ranking` reference the exact observation IDs plus a run manifest. Prefer this when reproducibility and offline tuning are real product requirements. If storage simplicity matters more, retain mutable caches but explicitly downgrade the promise to diagnostic snapshots rather than exact replay.
### Alternative C: fold facets into Stage A initially
Ask the existing 120-candidate Stage A call to return descriptive facets alongside quality and reader fit, then use those facets in final utility and future preference learning. Prefer this during early shadowing to avoid doubling DeepSeek candidate volume. A separate 240-candidate facet stage is preferable only if shadow evaluation shows facet preference materially improves the 240-to-120 cut.
### Alternative D: production-first shadow execution
Run the unchanged authoritative curation path first, reserve enough budget for all publication-critical calls, and execute new facet/ranking work afterward with its own cap. Prefer this during Phase A because it makes “shadow” genuinely non-interfering. Once the new path becomes authoritative, consolidate stage budgets under the provider-level ledger.
## Open Questions
1. Is a historical `generate --date D` supposed to reproduce knowledge available on day D, or intentionally re-curate D using todays ratings/profile? The database queries and CLI need separate semantics for these two operations.
2. Is `--max-articles N` an absolute ceiling even when there are more than N auto-includes? Which requirement wins?
3. Should facet labels be globally descriptive and reader-independent? If yes, confirm that the taste profile will be removed from the facet system prompt.
4. Is exact replay a hard requirement? If so, is retaining immutable feature observations and run manifests acceptable, including the extra storage?
5. How much of the DeepSeek daily budget is reserved for publication-critical Stage A/B/editorial work versus shadow facets, profile rebuilds, and evaluation/backfill?
6. Are any Miniflux feeds private, authenticated, or otherwise unsuitable for sending article text to Voyage/DeepSeek?
7. What minimum evidence must the shadow evaluation meet before moving between rollout phases (number of runs/ratings and explicit pass/fail thresholds), rather than “several days” or “monitor”?
@@ -0,0 +1,184 @@
# Re-review — Personalized Ranking, Embeddings, Facets, and Feedback (v3)
**Plan:** `docs/plans/2026-08-17-personalized-ranking-and-facets.md`
**Date:** 2026-08-19
**Scope:** revision 3, after the R3 amendments
## Verdict
Revision 3 resolves every R3 finding thoughtfully, and the main architecture is now coherent: the provider boundary is explicit, run eligibility uses the real lifecycle, profile text is versioned, facets are post-admission, and leader clustering is specified rather than mislabeled. I still would **not begin the full implementation unchanged**. Two remaining correctness defects sit below those amendments: the single global evidence weight can fully activate a learned signal backed by almost no compatible observations, and the stated `as_of`/backfill contract cannot be implemented with the proposed mutable feed-prior and date-keyed score stores. The generation lease also needs fencing and an active heartbeat before it can provide the mutual exclusion the plan claims. These are bounded amendments; the union-admission, utility, facet, and diversification decisions do not need to be revisited.
## Critical
### C1. One global evidence weight does not measure evidence for each learned signal
Evidence:
- §14 defines one `W` as the decayed weight of **all** ratings and uses it to gate `embedding_preference`, `facet_preference`, and `feed_affinity`.
- §13.4 says a rating is useful for embedding/facet preference only when its article has the corresponding features and that missing historical features “reduce evidence.”
- §25.1 guarantees protected articles receive no embedding or facets, but then says their ratings still update both `feed_priors_v2` **and the kNN preference state**.
Those statements cannot all hold. A newly protected article has no embedding, so its rating cannot enter kNN. More generally, 19 ratings without compatible embeddings plus one compatible rating produce `W = 20`, fully activating an embedding-preference signal learned from one example. The same issue applies independently to facets. Presence-aware blending does not fix it: once one compatible example makes the signal `Present`, the unrelated global `W` gives it full configured weight.
This recreates the sparse-evidence failure the ladder was designed to prevent, especially after provider opt-outs, partial backfills, model/dimension changes, or facet parse failures.
**Required amendment:** maintain evidence per learned signal:
```text
W_embedding = decayed weight of ratings with a compatible article embedding
W_facet = decayed weight of ratings with usable scored facets
W_feed = decayed weight successfully attributable to a feed
W_global = decayed weight of all ratings (telemetry/exploration maturity only)
```
Compute each gate from its own `W_signal`; facet value support remains an additional inner gate. Persist all four values in the finalized manifest/explanation rather than one `rating_evidence_weight`. A protected rating may update feed affinity and `W_global`, but it cannot update kNN or facet preference unless a locally generated compatible feature exists. Add a regression test where 20 total ratings but only one embedding-backed rating leave the embedding gate near its floor rather than fully open.
### C2. The `as_of` and historical-backfill contracts remain unsatisfiable with the proposed stores
Three plan requirements conflict:
1. **Feed priors:** §6.1 requires feed priors recomputed from ratings bounded by `as_of`, while §7.7 defines one singleton `feed_priors_v2` table rebuilt by `DELETE; INSERT`. A historical replay would overwrite todays materialized priors with a past snapshot. Meanwhile the rating server can rebuild the same table while generation holds its lease, because `serve` is not lease-protected. Atomic replacement prevents partial reads but not “wrong snapshot won the race” or future-data leakage relative to a runs fixed `as_of`.
2. **Churn scores:** §6.1 says churn is as-of bounded, but §18.4 keeps it on `scores.llm_score`. That table is keyed only by `(article_id, run_date)` and has no `run_id` or creation timestamp. A recuration performed on August 19 for an August 1 issue writes an August 1 score; a replay as of August 5 cannot tell that the score was created in its future. The proposed `recently_low_scored_ids(threshold, since, until)` bounds the nominal date, not observation time.
3. **Backfilled features:** §6.1 says replay ignores embedding/facet rows created after `as_of`, while §27.1 says to backfill embeddings/facets now and then replay historical dates. Those newly backfilled rows are, by definition, created after the historical `as_of`, so the replay must ignore the very features the backfill was intended to supply.
Acceptance criterion 13s leakage test cannot make all three behaviors correct without choosing stronger semantics.
**Required amendment:** separate two operations that v3 still calls `replay`:
- **Fidelity replay:** reconstruct only information actually available then. Ignore later-created features; derive feed affinity in memory from canonical ratings bounded by `as_of`; read churn assessments from run-scoped observations joined to `runs.started_at <= as_of`. Legacy `scores` rows without observation time must be excluded or explicitly treated as unverifiable.
- **Counterfactual evaluation:** ask how the new algorithm would rank a historical candidate set using features computed later. Permit marked post-hoc embeddings/facets, record `feature_time_policy = counterfactual`, and label results approximate because `content_html` is mutable.
Do not rebuild the global `feed_priors_v2` table for replay. Either compute a run-local `HashMap<FeedId, FeedPriorV2>` directly from bounded ratings, or persist a run-scoped snapshot. Keep the singleton table only as a current/live cache if the rating endpoint still needs it. Move the churn rule to `candidate_rankings.llm_quality_score` joined through eligible runs/manifests, or add `run_id`/actual `scored_at` provenance to `scores`; nominal `run_date` is insufficient.
## High
### H1. The expiring generation lease is not fenced and can expire during a live stage
Evidence:
- §7.4c gives the lease a 30-minute default TTL and refreshes it only “at each stage boundary.”
- §24.1 applies the same lease to `features backfill`, which may run for many batches, and acknowledges that Stage A wall clock is currently unmeasured.
- §31.11 tests racing acquisition and expired recovery, but not a live operation lasting longer than the TTL or an old holder acting after reclamation.
If one stage lasts longer than 30 minutes, a second process may reclaim the lease while the first is still running. Both then proceed. Worse, an RAII guard from the original process can later delete or refresh the replacement owners row unless every operation is conditional on an unforgeable ownership token. The claimed guarantee—“two concurrent `generate` invocations cannot both proceed”—does not hold.
**Required amendment:**
- assign each acquisition a random fencing token/generation;
- refresh and release only with `WHERE name = ? AND token = ?`;
- run a background heartbeat at a fraction of the TTL (for example TTL/3), not only at stage boundaries;
- if heartbeat/refresh loses ownership, abort before the next external call or persistent side effect;
- make a stale holder unable to publish even after another process has reclaimed the lease.
Add tests for a stage exceeding one TTL, reclamation followed by the old guard dropping, and a stale holder attempting to refresh/release/publish. An OS file lock is a simpler alternative on one host (§ Alternatives).
### H2. Phase Bs interleave rule depends on Phase C utility and does not guarantee exposure
Evidence:
- §32 says Phase B enables new admission but keeps the existing final selector.
- The Stage A quality/fit split and new utility do not become authoritative until Phase C.
- Phase B nevertheless reserves a slot for the “highest-utility” union-only candidate.
- The same sentence calls it an “issue slot” while preserving “Stage Bs right to refuse.”
At Phase B there is no v3 utility score to rank this cohort unless Phase C work has silently moved earlier. If Stage B may refuse, the slot is not an issue slot and the seven-run exit window can yield zero interleaved exposures, defeating the stated purpose of collecting real labels.
**Required amendment:** choose one exact Phase B behavior:
- rank candidates using an available Phase B score (preliminary blend or the legacy combined Stage A score), apply an explicit minimum quality threshold, and deterministically reinsert one qualified union-only candidate after Stage B; or
- call it an interleave nomination, let Stage B refuse, and require a minimum number of actual exposures before the phase can exit rather than “7 runs.”
If v3 utility is computed in Phase B shadow solely to choose the interleave, state which Stage A response fields exist then and move the necessary implementation work out of Phase C.
### H3. Migration `0002` cannot seed a hashed profile row as ordinary SQL without a bootstrap design
§7.4b and §31.13 require migration `0002` to seed `taste_profile_versions` from three existing `kv` values, including:
- parsing the JSON `profile_version` payload to obtain `version` and `built_at`;
- hashing the existing profile text with SHA-256 for non-null `profile_hash`;
- preserving learned text when present.
SQLite has no built-in SHA-256 function, and the repositorys migrations are plain SQL. Even if JSON extraction is available, the hash cannot be produced by the shown migration alone. A fake/empty hash would violate the manifest identity contract.
**Required amendment:** specify a Rust bootstrap immediately after schema migration:
1. open one transaction;
2. read and parse the current `kv` values;
3. compute the canonical hash in Rust;
4. insert the first history row idempotently;
5. commit before any profile load/rebuild.
Alternatively register an explicit SQLite hash function, but that is more machinery for a one-row migration. Test absent, malformed, and already-seeded `kv` states, not only the happy path.
## Medium
### M1. Adjudications are not tied to the run or algorithm that produced the sample
The §7.4d primary key is `(run_date, article_id)`, while all ranking snapshots correctly use `run_id`. A date can have live, shadow, dry-run, and rerun manifests with different candidate sets/configurations. `evaluate --adjudicate --date D` therefore cannot prove which run defined “union-only” and “control,” and a later rerun can change the explanation behind an existing verdict.
The text also says the table ensures an article is “never re-scored twice,” but including `run_date` permits the same overlapping article to be adjudicated again the next day.
**Recommendation:** introduce an adjudication batch keyed to `run_id`, algorithm version, and a persisted deterministic sample seed. Store randomized display order separately from hidden arm, and decide whether deduplication is per run, per article globally, or after a cooldown. The CLI should print the selected run ID before collecting labels.
### M2. A valid zero-candidate run has no “first ranking rows” with which to finalize its manifest
§7.4 finalizes the manifest in the same transaction as the first `candidate_rankings` inserts. Empty ingest windows and all-hygiene-excluded days are valid degraded/empty outcomes, but supply no first row. Such a run remains provisional forever and is excluded from every diagnostic even though its ranking configuration and zero-candidate outcome are meaningful.
**Recommendation:** finalize in one transaction that inserts zero or more initial rows; row existence must not be the trigger. Add a zero-eligible-candidate integration test.
### M3. `stage_completeness_json` is authoritative but nullable and structurally unversioned
Per-metric eligibility now depends on fields inside `stage_completeness_json`, yet the column is nullable, has no schema version, and no final-manifest invariant requires valid stage coverage. A malformed or old-shape JSON object can silently change metric denominators.
**Recommendation:** define a versioned typed structure, require it when `manifest_status = 'final'` in application logic, and treat parse/unknown-version failures as ineligible with an explicit diagnostic. Consider normal columns for the few coverage counts most often queried; JSON is reasonable only if all filtering occurs after typed decoding rather than ad hoc SQL JSON paths.
## Low
### L1. `explain` still says “latest complete run”
§26.3 says `explain` defaults to the “latest complete run for the date,” reintroducing the nonexistent lifecycle term fixed in §7.6. Say “latest eligible run under the typed predicate” and specify whether dry-run/shadow runs are excluded by default.
### L2. The proposed `QueryFragment` return type is not part of the current SQLx design
§7.6 sketches `fn evaluable_runs(kind: EvalKind) -> QueryFragment`, but this repository uses runtime `sqlx::query` and has no query-fragment abstraction. Keep the typed single-source requirement, but specify an implementable shape: a DB method that executes the full query, a `QueryBuilder<Sqlite>` helper, or a typed status/stage predicate applied after loading rows.
### L3. “Failed requests count actual tokens spent” is not always observable
§24 requires failed requests and retries to count actual provider tokens. For transport failures and some 5xx responses, no usage payload exists even if a provider ultimately bills work. Reserve-before-send is still correct, but reconciliation cannot always know “actual.”
Document the conservative rule: retain the estimate when actual usage is unavailable, reconcile only from trustworthy usage responses, and report estimated versus provider-reported usage separately.
## Nits
- Acceptance criteria number `18` appears twice; the final API-key/vector criterion should be 23.
- The R1 resolution table still points leakage tests to §31.9; they moved to §31.12.
- §24.1s parenthetical calls `--dry-run` read-only and then immediately says it persists data. State simply that dry runs acquire the lease.
- §15.1 says a dedicated facet stage is triggered by “metric 6 restricted to facet-driven admissions,” but facet preference is now forbidden in admission until that stage exists. Frame the trigger as an offline counterfactual evaluation, not an existing admission metric.
## Alternatives
### Alternative A: Per-signal evidence gates
Keep the current linear ramp but instantiate it independently for embedding, facets, and feed affinity using only compatible observations. Use global `W` solely for exploration maturity and general telemetry. Prefer this because it preserves the plans simple mathematics while making feature outages, opt-outs, and model migrations honest.
### Alternative B: Run-local derived preference snapshots
Treat `ratings` as canonical and derive kNN examples, facet statistics, and feed priors into one immutable in-memory `PreferenceState` per run, all bounded by `as_of`. Persist only the resulting raw candidate signals and evidence counts in run snapshots. Keep `feed_priors_v2` as an optional current-serving cache, never as replay input. Prefer this over versioning every aggregate table at the projects scale.
### Alternative C: Separate fidelity replay from counterfactual evaluation
Use `replay` only for “what information was available then?” and add an explicit `evaluate --counterfactual-features` mode for post-hoc embeddings/facets. Prefer this when offline algorithm comparison matters more than exact historical feature provenance. The manifest must record the feature-time policy so results cannot be mixed.
### Alternative D: OS-backed process lock
On one Linux host, an advisory file lock held by an open file descriptor naturally releases on process death and cannot be deleted by a stale RAII guard after another process acquires it. Prefer it if generation never needs to coordinate across hosts. Keep the SQLite fenced lease only if holder identity, waiting, and future multi-host operation justify the extra heartbeat/fencing machinery.
## Open Questions
1. Is `replay` meant to reproduce only features available at `as_of`, or to evaluate the new algorithm using features computed later? Both are useful, but they require different cache rules and labels.
2. Should protected/missing-feature ratings count toward exploration maturity while remaining excluded from embedding/facet evidence?
3. Is `feed_priors_v2` an online current-state cache or an input to every run? Historical runs cannot safely mutate or trust one global snapshot.
4. Will churn move to run-scoped `candidate_rankings`, or must `scores` gain actual observation-time provenance?
5. Is the Phase B interleave a guaranteed exposure or merely a Stage B nomination? What score exists to choose it before Phase C?
6. Can any lease-protected stage or backfill exceed 30 minutes? If yes, fencing and an active heartbeat are mandatory; stage-boundary refresh is insufficient.
@@ -0,0 +1,110 @@
# Plan Review: Personalized Ranking, Embeddings, Facets, and Feedback (v4)
**Reviewed:** 2026-08-19
**Plan:** `docs/plans/2026-08-17-personalized-ranking-and-facets.md` (revision 4)
**Verdict:** **Not ready for implementation.** Revision 4 resolves the prior review's mathematical, privacy, and lease defects, but two fidelity guarantees are still built on overwrite-in-place tables and therefore cannot hold. The remaining High findings should also be made explicit before implementation because they affect schema shape, provider accounting, process coordination, and rollout behavior.
## Critical findings
### C1. Adding `run_id` and `scored_at` does not preserve score observation history
Section 7.8 correctly identifies that nominal `run_date` is not an observation timestamp, but the proposed migration leaves the existing primary key unchanged: `scores` remains keyed by `(article_id, run_date)` (plan lines 646-661; `migrations/0001_init.sql:58-63`). The current `db::upsert_score` writes through that key (`src/db.rs:386-402`). Consequently, a recuration of the same nominal date still overwrites the earlier observation; it merely replaces it with a row containing a newer `run_id` and `scored_at`.
Example:
1. Article A receives score 2.0 for the August 1 run, observed August 1.
2. On August 19, `generate --date 2026-08-01` scores A as 7.0 and overwrites the same `(A, 2026-08-01)` row.
3. A fidelity replay as of August 5 excludes the surviving row because `scored_at = August 19`, but the valid August 1 score is gone. The churn result has changed because of a future recuration.
This directly contradicts the churn observation-time test in §31.12 and the stated purpose of the v4 fix. `run_id` is provenance only if it participates in an append-only identity.
**Required amendment:** create an append-only score-observation relation keyed at least by `(run_id, article_id)` (or rebuild `scores` with that primary key), then have the churn query select the latest compatible observation with `scored_at <= as_of`. If the legacy date-keyed `scores` table must remain for phased compatibility, treat it as a current-value projection and add a separate authoritative `score_observations` table. Add the destructive case above to §31.12; merely inserting one future score row is not sufficient.
### C2. The broader fidelity contract still queries mutable snapshots as though they were histories
The same issue exists outside `scores`:
- `ratings` is keyed by `(issue_date, article_id)`, and a vote flip overwrites both `vote` and `rated_at` (`migrations/0001_init.sql:100-107`, `src/db.rs:526-543`). Section 13.1 calls the overwrite acceptable for decay (plan line 952), but after a future flip a replay before the flip loses the original vote entirely. Filtering `rated_at <= as_of` cannot recover it.
- `issues` is keyed only by date, `upsert_issue` overwrites `generated_at`, and `replace_issue_articles` deletes and replaces the lineup for that date (`src/db.rs:441-486`). Republishing an old nominal date therefore erases the publication fact and lineup that existed at an earlier `as_of`. The §6.1 predicate `issues.generated_at <= as_of` then excludes the replacement without restoring the original.
- Feed-prior reconstruction joins ratings to the article's current `sources_json`. That field is overwritten on re-ingest (`src/db.rs:270-279`), so future source expansion can change historical feed attribution even if the rating row itself did not change.
Persisted `candidate_rankings` makes an original run's scalar output inspectable, but it does not repair `generate --as-of-date`, preference-state reconstruction, churn reconstruction, or previously-published exclusion. The acceptance criterion that replay is unaffected by future ratings or issues is therefore stronger than the schema can satisfy.
**Required amendment:** choose one of these contracts before implementation:
1. **Recommended:** add append-only `rating_events` and publication/issue-version tables, selecting the latest event at or before `as_of`; preserve the feed-attribution/source snapshot needed by each rating event. Together with C1, this makes fidelity a real temporal query.
2. **Narrower alternative:** remove `generate --as-of-date` and stop promising state reconstruction. Define “fidelity” as inspection/reweighting of already-persisted run snapshots only, explicitly excluding vote state before a later flip, historical publication reconstruction, and historical source attribution.
Whichever contract is chosen, §31.12 needs mutation tests: flip an existing rating in the future, republish the same issue date in the future, rescore the same article/date in the future, and verify that the earlier result is unchanged.
## High findings
### H1. “Daily” provider accounting still omits billing-day identity, non-generate commands, and crash persistence
Sections 7.6 and 24 preload spend from `runs` “for the date” (plan lines 588-589 and 1507 onward), following the existing `spend_for_date(date)` query over `runs.date` (`src/db.rs:688-694`). That date is the nominal issue date, not the provider billing day:
- recuration of August 1 on August 19 charges the August 1 bucket;
- recurations of several historical dates on one real day each receive a fresh “daily” ceiling;
- `features backfill` and standalone `profile rebuild` make provider calls but are not specified to create/finalize `runs` rows, so their spend has no durable bucket;
- reservations and conservative failure estimates live only in process memory until the run is finished. A crash after a request but before `finish_run` leaves zero persisted spend; the kernel correctly releases the file lock, and a retry starts from the understated balance.
Serialization prevents concurrent overspend, but it does not fix omitted or crash-lost spend. This is particularly important because the limits are described as runaway guards.
**Required amendment:** account by actual request/reservation time (with a documented UTC/provider-day boundary), across every provider-using command. Persist the estimate before dispatch and reconcile it afterward, so a crash leaves the conservative reservation rather than zero. A small append-only `provider_usage`/`provider_reservations` table keyed by provider, operation, optional `run_id`, and `reserved_at` is the cleanest design. If the plan intentionally keeps only successful-generate, nominal-date accounting, rename and weaken the guarantee accordingly; it is not a daily provider ceiling.
### H2. The lock scope omits the standalone profile rebuild, which spends budget and races profile versioning
Section 24.1 says “Every mutating command” takes the lock, but lists only `generate`, `features backfill`, and `features prune` (plan lines 1517-1523). `profile rebuild` also calls DeepSeek and writes both `taste_profile_versions` and the `kv` current pointer. Without the same lock, it can overlap generation's weekly rebuild, duplicate spend, choose the same next version, or change the profile pointer while a run is establishing its manifest.
There is also a placement mismatch: §30 says `pipeline.rs` takes the lock, but the current CLI calls `Db::open_and_migrate` before entering `pipeline::generate` (`src/main.rs:105-111`). Thus “before doing any work” does not include startup migration/bootstrap, and different commands are likely to acquire at inconsistent points.
**Required amendment:** define the exact command matrix instead of calling `serve` read-only (the rating endpoint writes `ratings`, though its post-`as_of` writes can safely remain unlocked). At minimum, standalone profile rebuild must take the generation/provider lock. Specify whether the lock is acquired in `main` before command-specific DB work or whether migration/bootstrap has its own short critical section. Add a concurrent generate/profile-rebuild test, including profile-version allocation and provider reservations.
### H3. A manifest becomes `final` before its required stage-completeness data exists
Section 7.4 says the manifest is finalized once preference state and profile selection complete, transactionally with the initial ranking rows (plan lines 386-394). In the target pipeline this is before admission, Stage A, facets, Stage B, and publication. Yet the same section requires every final manifest to contain a complete `stage_completeness_json`, and §7.6 uses that object as authoritative per-metric eligibility.
At that early point the implementation can only write placeholder zeroes and mutate a supposedly final authority later. The plan does not specify those later updates or make them atomic with `runs.status`. A crash or error between the independent writes can therefore leave an `ok`/`degraded` run with stale completeness. In addition, the proposed completeness schema contains `embeddings`, `stage_a`, `facets`, and `stage_b`, but §7.6 says admission metrics require the admission stage to have completed; there is no admission field from which to decide that.
**Required amendment:** separate “ranking definition captured” from “run finalized,” or keep the manifest provisional until `finish_run`. Write final stage completeness and the final `runs.status` in one transaction; candidate rows do not need to be coupled to manifest finalization. Include at least admission, utility/diversification, selection, and publication completion states wherever metrics depend on them. Zero-candidate runs still finalize normally at end of run, so the R4 fix is preserved.
### H4. Guaranteed post-Stage-B insertion and a hard maximum still lack a total precedence rule
Revision 4 says protected auto-includes and the Phase B interleave pick are reinserted after Stage B, both subject to `hard_max` (plan lines 1562 and 2015-2023). It simultaneously calls the interleave a reserved slot and guaranteed exposure. If Stage B returns exactly `hard_max` articles, insertion must either exceed the hard maximum, evict a Stage B pick, or fail to expose the interleave. The same ambiguity appears when protected auto-includes and an interleave compete for the last slot, or when Stage B already selected the intended interleave naturally.
Section 21.2 settles only the case where auto-includes alone exceed the ceiling; it does not define precedence among ordinary auto-includes, protected auto-includes, interleave exposure, and editor picks.
**Required amendment:** specify one deterministic merge order and test it at capacity. For example: deduplicate natural Stage-B selections first; reserve/insert mandatory auto-includes (trimming among auto-includes only if they alone exceed the ceiling); insert the interleave by evicting the lowest-ranked non-mandatory editor pick; then fill remaining slots from Stage B. If auto-includes consume all capacity, explicitly decide whether the interleave is not guaranteed that day or may displace an auto-include. Count `interleave_selected` only for a real final exposure.
## Medium findings
### M1. The malformed-profile bootstrap repairs history but leaves the current version pointer malformed
For malformed `kv[profile_version]`, §7.4b seeds `taste_profile_versions.version = 1` and does `ON CONFLICT DO NOTHING` (plan lines 427-440), but it does not repair the malformed `kv` value. The current `stored_version` treats that value as absent (`src/curate/profile/mod.rs:203-220`), so the next rebuild chooses version 1 again. An append then conflicts with the seeded row; an upsert would overwrite the very history the table is meant to preserve.
**Required amendment:** either repair `kv[profile_version]` to the canonical seeded metadata in the same bootstrap transaction, or allocate the next version from `MAX(taste_profile_versions.version) + 1` and update the pointer transactionally. Extend the malformed-state test from “bootstrap succeeds” to “bootstrap followed by rebuild creates version 2 and preserves version 1.”
## Alternatives worth considering
### One append-only observation layer
Instead of solving scores, ratings, issue publication, and provider spend with separate ad hoc exceptions, introduce a small family of append-only event/observation tables and keep the existing tables as current projections. SQLite is well suited to this volume. It gives `as_of` one consistent meaning and makes crash-safe accounting natural.
### Snapshot-only evaluation
If temporal event storage is judged too much scope, keep per-run `candidate_rankings` and manifests as immutable snapshots and constrain evaluation to those snapshots. This is materially simpler, but the plan must then drop claims that it can reconstruct arbitrary historical state or rerun generation faithfully after mutable inputs have changed.
### Pre-reserved final-selection capacity
Rather than post-selection eviction, calculate Stage B's available capacity after mandatory auto-includes and the active interleave reservation. This makes the prompt ceiling truthful and reduces surprising removal of editor choices, at the cost of giving Stage B a slightly smaller slate on those days.
## Open questions
1. Must fidelity remain stable after an existing vote is flipped and after the same nominal issue date is republished? If yes, append-only rating and publication history is mandatory.
2. Does “daily provider ceiling” mean the provider's real UTC billing day across generation, backfill, profile rebuild, retries, and crashes? If not, what narrower operational guarantee is intended?
3. When `hard_max` is full, which has priority: protected auto-includes, ordinary auto-includes, the Phase B interleave exposure, or Stage B's lowest-ranked choice?
4. Is `manifest_status = final` intended to mean “ranking inputs fixed” or “run outcome complete”? The current plan requires it to mean both at different times.
## Bottom line
The ranking, evidence-gating, privacy wrapper, and single-host lock choices are now implementation-worthy. Implementation should still wait for C1 and C2 because they determine whether migration `0002` needs append-only temporal tables. H1-H4 should be resolved in the same amendment so provider limits, manifest eligibility, locking, and rollout exposure have testable semantics rather than being decided piecemeal during coding.
@@ -0,0 +1,172 @@
# Plan Review: Personalized Ranking, Embeddings, Facets, and Feedback (v5)
**Reviewed:** 2026-08-19
**Plan:** `docs/plans/2026-08-17-personalized-ranking-and-facets.md` (revision 5)
**Verdict:** **Nearly ready, but not yet safe to implement verbatim.** Revision 5 fixes the prior temporal-history, billing-day, lock-scope, manifest-lifecycle, capacity, and profile-bootstrap blockers. No new architectural rewrite is needed. Three High-severity execution defects remain: the proposed churn query does not implement its stated “latest observation wins” rule, the plan sometimes bypasses its new event authority in live/recurate modes, and the provider ledger cannot enforce the promised shadow budget or conservatively account for retries. Several stale v4 instructions should also be removed so an implementation agent is not given two incompatible authorities.
## Critical
No remaining Critical finding. The append-only observation layer, UTC provider ledger, three-state manifest, expanded lock matrix, and pre-reserved Stage B capacity are sound architectural responses to the previous review.
## High
### H1. The shown churn SQL neither selects the latest observation nor uses observation time for the lookback window
Section 7.8 says “the latest observation per article wins,” but the displayed query returns every qualifying low-score row:
```sql
SELECT cr.article_id
FROM candidate_rankings cr
JOIN runs r ON r.id = cr.run_id
WHERE cr.llm_quality_score IS NOT NULL
AND r.started_at <= :as_of
AND cr.run_date >= :since
AND cr.llm_quality_score < :floor
AND r.status IN ('ok', 'degraded')
```
There is no grouping, window function, correlated `NOT EXISTS`, or maximum observation selection. If an article scored 2.0 and was later rescored 7.0, the old 2.0 row still satisfies this query and suppresses the article. The new mutation test can therefore pass only if the implementation diverges from the SQL the plan tells it to write.
The lookback also remains anchored to `cr.run_date`, the nominal issue date, even though §7.8 calls `runs.started_at` the “true observation time.” A low score observed today while recurating an old nominal date is immediately outside a seven-day nominal-date window; conversely, a future nominal issue date could enter the window despite when it was observed. Pruning ranking snapshots by nominal date would reproduce the same defect.
**Required amendment:** define whether churn recency means observation recency or nominal issue recency. The v5 rationale strongly implies observation recency, so rank eligible observations with something equivalent to:
```sql
WITH ranked AS (
SELECT cr.article_id,
cr.llm_quality_score,
ROW_NUMBER() OVER (
PARTITION BY cr.article_id
ORDER BY r.started_at DESC, cr.run_id DESC
) AS rn
FROM candidate_rankings cr
JOIN runs r ON r.id = cr.run_id
WHERE cr.llm_quality_score IS NOT NULL
AND r.started_at >= :observation_since
AND r.started_at <= :as_of
AND r.status IN ('ok', 'degraded')
)
SELECT article_id
FROM ranked
WHERE rn = 1 AND llm_quality_score < :floor;
```
Prune `candidate_rankings` by `runs.started_at`, not `candidate_rankings.run_date`. Add both directions to the regression suite: low→high must not suppress; high→low must suppress. Include a historical-date recuration observed today so the time-axis choice is tested rather than inferred.
### H2. `rating_events` and `publication_events` are declared authoritative, then bypassed in live/recurate modes
Section 6.1 correctly says temporal reads always use the event tables. Section 7.9 then says “`live` and `recurate` may read the projections directly.” That is not equivalent to asking the event layer for the latest state now:
- `ratings` is keyed by `(issue_date, article_id)`. If the same article is republished and rated in two issues, reading the projection can count it twice, while “latest event per article” counts it once.
- `issue_articles` contains only the current lineup for each nominal date. If a republish removes an article, a live projection read says it was never published, while `publication_events` correctly says it was published earlier. The article can then re-enter the paper despite the hard “already published” exclusion.
- Maintaining separate projection and event queries gives live and fidelity subtly different product semantics, not merely different time bounds.
There is also an incomplete attribution snapshot. `rating_events.source_feeds_json` stores only distinct **direct** feed IDs, while §7.7 explicitly falls back to the `best_entry_id` feed when no direct feed exists. The event does not store that fallback feed, so a discovery-only rating cannot be reproduced without consulting mutable current article state—the exact dependency the event row was introduced to remove.
**Required amendment:** use the event tables for preference and previously-published reads in **all** modes; live/recurate simply pass `as_of = now`. Keep projections only for serving the current issue/current vote UI. Store the exact local attribution result on each rating event—preferably a versioned `feed_credits_json` map of feed ID to weight, or at minimum the post-fallback attributed feed set—not merely the pre-fallback direct set. Specify `ORDER BY event_at DESC, id DESC` so equal timestamps are deterministic.
Add regressions for:
1. an article removed by same-date republication remains “previously published” in a subsequent live run;
2. an article rated through two issue dates contributes only its latest vote once;
3. a discovery-only rating retains its vote-time fallback feed after current article provenance changes.
### H3. The provider ledger lacks the dimensions and attempt semantics required by §24
The append-only `provider_usage` ledger fixes UTC bucketing, cross-command spend, and crash persistence, but two promised controls are not representable yet.
First, §24 says shadow work draws from `shadow_max_daily_usd` and “can never consume the production slice.” `provider_usage` has provider, operation, and optional `run_id`, but no `budget_class`/`slice`. Production and shadow calls can occur within the same run, so joining through `run_manifests.shadow` cannot classify individual requests. The ledger can enforce one provider-wide ceiling or a shadow ceiling, but not both the stated production and shadow slices.
Second, retries are under-specified. The plan reserves before “the request,” retries network/429/5xx failures, and keeps an estimate when a failed attempt has no usage payload. If one ledger row covers a logical request, a failed possibly-billed attempt followed by a successful retry will usually settle that row to the final attempt's actual usage, erasing the failed attempt's conservative estimate. This violates the rationale in §24 precisely on the retry path most likely to lack usage metadata.
**Required amendment:**
- Add a `budget_class` such as `production | shadow | backfill` (or an equivalent explicit allocation key) and define whether the provider-wide ceiling also caps the sum of all classes. State whether `shadow_max_daily_usd` is inside the production provider maximum or additive to it.
- Reserve and reconcile **per outbound HTTP attempt**, linking attempts with a logical request ID, or reserve the worst-case cost of all allowed attempts and decrement safely as attempts become known not to have been billed. The per-attempt model is easier to audit.
- Define the conservative estimate formula. For DeepSeek it should include input plus the request's maximum possible output tokens at their respective prices, assuming no cache discount unless known; Voyage is input-only.
- Add a test where attempt 1 returns a 5xx with no usage and attempt 2 succeeds: the day total must contain attempt 1's standing estimate plus attempt 2's actual usage. Add a mixed production/shadow run proving each slice and the aggregate ceiling.
## Medium
### M1. The new authorities were not propagated through the normative file-by-file instructions
Several later sections still instruct an implementer to build the v4 design:
- §9.4 says Voyage is “preloaded per date” and to keep DeepSeek budget semantics unchanged.
- §23 says feed priors and flips derive from canonical `ratings`; §25.1 says protected ratings derive from `ratings` and current `sources_json`.
- §24 says meters preload from `runs` by date, despite §7.6 making `provider_usage` authoritative, and says “rather than build a cross-process reservation ledger” immediately after adding one.
- §30's `src/db.rs` list still requests `voyage_spend_for_date` and “finalize [the manifest] transactionally with the first ranking rows.” Its `src/pipeline.rs` entry likewise says to “finalize” after preference loading instead of transition to `ranking_fixed`.
- The `PreferenceState` type comment and `preference.rs` entry still call `ratings` canonical.
- `RunReport`'s listed completeness block still omits admission, utility, diversification, selection, and publication.
- §31.8 says every mid-run failure leaves a provisional manifest, although a failure after the new transition must leave at least `ranking_fixed` (or deliberately finalize a failed outcome).
- Phase A's adjudication paragraph describes the superseded date-keyed table rather than the `adjudication_batches` schema.
These are not cosmetic in an “implementation-grade” plan: §30 is exactly where an implementation agent will derive its worklist.
**Required amendment:** run one terminology/authority pass and replace every stale use with `rating_events`, `publication_events`, `provider_usage`, `ranking_fixed`, and the full completeness schema. Reserve “projection” for explicitly non-temporal UI/compatibility paths. Update the R1/R2/R3 resolution tables where they still describe superseded mechanisms, or label those cells as historical resolutions superseded by R5.
### M2. Observation seeding is coupled to profile bootstrap and does not have an explicit one-time marker
Section 7.4b makes `bootstrap_profile_history()` also seed rating and publication events. Its earlier step says that an absent/empty taste profile does nothing, making it unclear whether event seeding still runs on a database that has issue/rating projections but no profile. These are unrelated migrations and should not share an early-return condition.
“Seed if the event table is empty” is also a state heuristic, not a migration marker. Every command runs the bootstrap after migration, while an already-running `serve` process does not hold the file lock and can append rating events concurrently. At cutover, an emptiness check plus projection copy can race a legitimate first event or make retry behavior ambiguous.
**Required amendment:** split this into `bootstrap_profile_history()` and `bootstrap_observation_history()`. Record completion in a durable bootstrap/migration marker inside the same transaction as seeding, and make the copy idempotent by a stable seed identity. Event seeding must run independently of whether a taste profile exists. Test a projection-only database with no profile, and test restart after a partially completed/rolled-back seed.
### M3. Failure-state manifest semantics remain contradictory
Section 7.4 defines `final` as the state written in the same transaction as `finish_run`, and the current pipeline calls `finish_run` on errors. The eligibility table even permits `explain --run-id` over failed runs with `ranking_fixed` **or final**. But §31.8 requires every run that fails mid-way to retain a provisional manifest.
That cannot hold for failures after the run reached `ranking_fixed`, and it discards useful completeness data if all failed runs are deliberately kept provisional.
**Required amendment:** specify transitions by failure point. A coherent rule would be: failure before preference/profile capture remains `provisional`; failure after it remains `ranking_fixed` or transitions to `final` with terminal completeness in the same status transaction; evaluation always excludes `failed`, while `explain --run-id` accepts all three states with whatever data exists. Add one test for failure before and one after `ranking_fixed`.
### M4. Publication-event authority stops just short of the actual publish boundary
The plan appends `publication_events` transactionally with `replace_issue_articles`, which is necessary, but current execution publishes files first and records the issue afterward (`src/pipeline.rs:513-528`, `557-582`). A crash or SQLite error after the atomic file copy but before the database transaction leaves an issue visible through the publish directory/OPDS without a publication event. Also, current `upsert_issue` and `replace_issue_articles` are separate database transactions; adding events only to the latter can leave the projections half-updated.
**Required amendment:** at minimum, put `issues`, `issue_articles`, and `publication_events` in one database transaction after file publication and define a recovery check for “files published, DB commit missing” on rerun/startup. If strict exposure-time fidelity is not required for that crash window, state that `publication_events` means “successfully published and recorded,” not every instant a file may have been externally visible.
## Low
### L1. The migration-lock release/reacquire introduces an avoidable race
Section 24.1 has mutating commands acquire the file lock for migration, release it, then immediately reacquire it for the command. Another command can win the gap, causing a generation that completed startup successfully to fail before doing work. This is safe but surprising.
For a lock-holding command, retain the same file descriptor after migration/bootstrap and upgrade the guard's diagnostic state once the database is available. Only non-lock-holding commands such as `serve` need to release after the migration section.
### L2. The deferred-options table still says a provider ledger is deferred
Section 35 lists “Fenced SQLite lease or a cross-process provider ledger instead of the file lock” as deferred, but v5 now includes a cross-process provider ledger. The actual deferred choice is a distributed generation lock/fenced multi-host coordinator; rename the row so it does not suggest removing or postponing `provider_usage`.
## Nits
- `source_feeds_json` should have a versioned typed schema and validation just like the other authoritative JSON fields; storing the final credit map makes this natural.
- Add `CHECK (estimated_usd >= 0)` and `CHECK (actual_usd IS NULL OR actual_usd >= 0)` to `provider_usage`, and validate that `billing_day` is derived internally from `reserved_at` rather than independently supplied by callers.
- The Stage B merge rule says editor picks are admitted in model order and excess picks are trimmed by `ordering_score`; choose one ordering rule for malformed over-cap responses.
- §23 says `serve` “mutates nothing that a run reads mid-flight.” It does mutate the event authority; the correct statement is that timestamp-bounded reads make concurrent later events invisible to the run.
## Alternatives
### Always read the observation layer (recommended)
Use `rating_events` and `publication_events` for every ranking/history query, with `as_of = now` for live/recurate. This produces one tested semantic path. The projection tables remain valuable for the current issue page and current vote state, but never decide ranking history.
### Maintain query-equivalent projections
If event scans ever become measurably expensive, introduce purpose-built current projections such as one row per article's latest rating and a durable “ever published” set, updated transactionally from events. Those projections must be defined and tested as exact query-equivalent caches; the existing `ratings` and `issue_articles` shapes are not equivalent.
### Retry budget alternatives
Per-attempt ledger rows are the most auditable solution. Reserving the maximum cost of every possible retry up front is simpler but needlessly blocks budget during transient failures and requires careful release semantics; use it only if per-attempt IDs are judged too invasive.
## Open Questions
1. Does “recent rejection” mean recently **observed** by an LLM or associated with a recent nominal issue date? The v5 prose and event-time rationale imply the former, but the SQL implements the latter.
2. Is `shadow_max_daily_usd` a sub-limit within each provider's overall daily ceiling, or an additive allowance beyond it? Which budget class owns feature collection performed during a shadow run but reusable by production?
3. Should a failed run after `ranking_fixed` be finalized with partial completeness, or remain `ranking_fixed`? Either is workable; “always provisional” is not.
4. Does a publication event represent external file visibility or the later successful database commit? What recovery behavior is expected if those diverge?
## Bottom line
The plan's architecture is now fundamentally sound. Resolve H1-H3 before implementation because they affect the correctness of the churn rule and the schemas for `rating_events` and `provider_usage`. The Medium findings should be amended in the same pass; most are consistency work, but leaving them in an implementation-grade document would cause the code to regress toward mechanisms revision 5 explicitly replaced.
@@ -0,0 +1,118 @@
# Plan Review: Personalized Ranking, Embeddings, Facets, and Feedback (v6)
**Reviewed:** 2026-08-19
**Plan:** `docs/plans/2026-08-17-personalized-ranking-and-facets.md` (revision 6)
**Verdict:** **Close, but not yet safe to implement verbatim.** Revision 6 correctly repairs the churn query, makes the event layer authoritative in every mode, snapshots final feed attribution, adds attempt-level budget accounting, and closes the migration/publication consistency gaps from the previous review. No architectural rewrite remains. Four High-severity contracts still need to be made implementable: protected-feed classification is not actually based on feed identity, the shadow sub-limit does not reserve production capacity, concurrent ledger reservations have no specified SQLite atomicity mechanism, and the legacy churn fallback cannot implement the new observation-time/latest-value semantics. A final normative consistency pass is also still required.
## Critical
No remaining Critical finding. The ranking, observation-history, manifest-lifecycle, and publication designs are now internally sound in their primary paths.
## High
### H1. The provider-policy type prevents bypasses only after classification; its host matcher can misclassify a private feed
Section 25.1 defines `no_external_ai_feeds` as feed IDs or host substrings “matched exactly like `always_include_feeds`” and then gives `externally_processable` only an `Article` and `CurationConfig` (§25.1, §30). That existing matcher does not inspect the feed URL. In `src/curate/prefilter.rs:136-165`, numeric values match any source feed ID, but string values are searched as case-insensitive substrings of `article.url` and `article.canonical_url`. `SourceRef` contains `feed_id`, title, category, and kind, but no feed URL (`src/types.rs:62-70`).
That is insufficient for the stated threat model. A private Miniflux feed may live at `reader.internal/private.xml` while its entries link to public sites. Configuring `reader.internal` appears valid but does not protect those articles; `externally_processable` constructs the wrapper and the strong type then faithfully sends the protected data. Substring matching also has the wrong security properties for a deny policy: it can match unrelated hosts and does not define exact-host/subdomain behavior.
This is especially easy to miss in a deduplicated cluster whose best article URL is public but one secondary `SourceRef.feed_id` is protected. Numeric IDs can classify that case correctly, but the advertised host form cannot.
**Required amendment:** make the privacy configuration identity-safe before relying on the type-level guarantee. The safest V1 is a typed `no_external_ai_feed_ids: Vec<FeedId>` and a startup error for nonnumeric entries; Miniflux feed IDs are already present on every source and survive deduplication. If host policies must remain, separate them into explicitly named fields and pass actual Miniflux feed URL/site metadata into the policy gate. Parse URLs and compare normalized hosts (with a documented exact-host/subdomain rule), never arbitrary substrings of article URLs.
Add tests where:
1. a protected feed URL host differs from the linked article host;
2. the best source is public but a secondary source feed is protected;
3. a lookalike hostname does not accidentally match an unrelated protected hostname.
The full-run recording mock should use one of these adversarial classifications rather than only a numeric best-feed match.
### H2. A shadow sub-limit inside a shared ceiling does not preserve a production slice
Section 7.6 says all classes share the provider's `max_daily_usd`, while `shadow_max_daily_usd` merely places an additional cap on shadow (§7.6 lines 672-687). Section 24 then claims shadow “can never consume the production slice” (§24 line 1734). Those statements are not equivalent.
With the documented Voyage defaults, the provider ceiling is $0.25 and the shadow sub-limit is $0.20. A shadow invocation that spends $0.20 first leaves only $0.05 for a later production run. Production-first ordering protects calls only within one invocation; it does nothing across runs or across the UTC day. The same issue arises if a standalone non-publication command runs before the timer. The ledger accurately records the depletion, but it does not reserve capacity for the newspaper.
**Required amendment:** choose and state one of these contracts:
- If publication capacity is guaranteed, add a per-provider `production_reserve_daily_usd` (or explicit class allocations) and admit shadow/backfill only when `provider_total + estimate <= provider_max - production_reserve`. Define which operations may consume the reserve and what happens after the publication run completes.
- If the ceiling is only an account-wide runaway guard, remove “production slice” and state plainly that shadow is bounded but may reduce capacity available to later production.
The Phase A promise that shadowing does not affect the paper favors the first option. Add an order-sensitive test: spend shadow first, then prove the configured production reserve is still dispatchable. A mixed-class test inside one run, which §31.11b currently requires, does not cover this defect.
### H3. “Atomic check-and-reserve” needs an explicit SQLite serialization mechanism
The plan correctly requires checking and reserving before each concurrently spawned request, but it never defines the transaction primitive that makes the read-sum-insert sequence atomic. `buffer_unordered` can run sibling reservations concurrently inside one process. The process-wide `flock` serializes commands, not async tasks or SQLite connections within that command.
A naive SQLx transaction is deferred in SQLite: two tasks can both read the same pre-reservation total, both decide they fit, and then contend when writing. Depending on timing, that either admits estimates beyond the ceiling or produces `SQLITE_BUSY` at a point the plan currently treats like ordinary provider degradation. A unique key on `(request_id, attempt)` does not serialize different requests, and the spend index does not enforce a sum constraint.
**Required amendment:** specify one implementation-grade reservation path. Viable choices are:
- a provider-scoped in-process async mutex around a short `BEGIN IMMEDIATE` transaction that re-sums and inserts before commit; or
- a single reservation actor/connection that serializes all check-and-insert operations.
Because every provider-spending command holds the OS lock, an in-process mutex plus `BEGIN IMMEDIATE` is sufficient for the declared single-host deployment. Specify busy-timeout/retry behavior for this short transaction and keep external HTTP work outside it.
Add a barrier-based concurrency test that releases many reservation tasks simultaneously near the ceiling and asserts that the sum of admitted estimates never exceeds either the provider cap or the applicable class cap. Also assert that refusal occurs before the corresponding mock HTTP dispatch.
### H4. The legacy `scores` fallback cannot satisfy the churn rule unless its precedence and lifetime are bounded
Section 7.8 now gives `candidate_rankings` a correct latest-observation query over `runs.started_at`, but then says pre-`0002` dates fall back to `scores` in live/recurate (line 795). `scores` is exactly the mutable, nominal-date-keyed projection the section rejected: it has no observation timestamp and may contain multiple rows for an article across nominal dates. The plan gives no query, merge precedence, or retirement point for that fallback.
A straightforward union of low IDs recreates the v5 defect: one legacy low row can suppress an article despite a newer high `candidate_rankings` observation. Using `scores.run_date` as recency recreates the wrong-time-axis defect. Because new runs continue writing the compatibility projection, “pre-migration score” also cannot be inferred merely from the row's existence.
**Required amendment:** either delete the fallback—the database is effectively at cold start, so this is the cleanest option—or define it as a strictly temporary compatibility bridge:
- capture a migration timestamp/marker;
- consider legacy `scores` only for articles with no `candidate_rankings` observation at all;
- document the nominal-date approximation explicitly;
- disable the fallback after one `recent_rejection_lookback_days` interval from migration, so an unverifiable projection cannot suppress forever.
Do not let a projection row compete with an observed candidate row. Add tests for legacy-low → new-high, legacy-high → new-low, two legacy nominal dates for one article, and fallback expiry. Fidelity should continue to exclude these unverifiable rows.
## Medium
### M1. The normative worklist still contains v5 authorities and one lifecycle contradiction
Revision 6's core sections are clear, but the later implementation instructions still tell an agent to build several superseded forms:
- §23 line 1720 and §30 `src/server.rs` line 2065 say to capture distinct direct-feed IDs. The authoritative event schema requires the completed, versioned `feed_credits_json` after fallback.
- §25.1 line 1810 says protected feed affinity and `W_global` derive from `ratings` and `sources_json`. They must derive from latest `rating_events` and stored feed credits; using current sources reintroduces the temporal bug §7.9 removes.
- §30 `src/db.rs` lines 2007-2008 says to load/derive from ratings joined to current sources/facets. The normative source is latest rating events, with article/facet joins only for compatible local signals.
- §30 `src/db.rs` lists `bootstrap_profile_history()` but omits `bootstrap_observation_history()`.
- §30 `src/publish.rs` line 2069 says publication events commit with `replace_issue_articles`; the actual contract is one transaction containing `upsert_issue`, `replace_issue_articles`, and events.
- §30 `src/main.rs` line 2079 still releases and reacquires the lock and runs only the profile bootstrap, contradicting §24.1's same-file-descriptor rule and the two-bootstrap contract.
- The eligibility table says `explain` accepts only `ranking_fixed`/`final`, while the failure semantics and §31.8 say `explain --run-id` accepts `provisional` too.
These are execution instructions, not historical review tables, and several directly reintroduce bugs v6 says are fixed.
**Required amendment:** update §23, §25.1, §30, and the eligibility table so each has one authority. Add `bootstrap_observation_history` to both the startup sequence and migration concurrency test. Search the normative text for `distinct direct-feed`, `ratings and sources_json`, `load ratings`, `transactionally with replace_issue_articles`, and `release → ... re-acquire`; none should remain except in explicitly labelled historical discussion.
## Low
No separate Low-severity finding. The remaining cleanup belongs in the normative consistency pass above.
## Alternatives
### Prefer typed feed IDs for provider opt-out
Feed IDs are the least ambiguous privacy boundary in this codebase: every clustered source carries one, and the Miniflux account owns the mapping. A separate article-domain rule can be added later for public-domain policy, but it should not masquerade as feed identity.
### Retire legacy churn state at migration
Given the near-empty production history, accepting at most one week without legacy churn suppression is lower risk than maintaining a second, semantically weaker query. If temporary continuity is still desired, snapshot the legacy low set once with an explicit expiry instead of continuing to consult the mutable `scores` projection.
### Serialize ledger admission, parallelize HTTP only
Reservation transactions are tiny. Serialize those locally, commit, then allow HTTP attempts to proceed concurrently. This preserves throughput where it matters while making the monetary invariant easy to state and test.
## Open Questions
1. Is `no_external_ai_feeds` intended to identify Miniflux feed subscriptions, article domains, or both? If both, what are the exact matching semantics for each namespace?
2. Must Phase A/shadow work be incapable of reducing a later production run's available budget, or is `shadow_max_daily_usd` only an additional runaway cap?
3. Is preserving pre-migration churn suppression worth a temporary second semantic path, given the production store has approximately one issue of history?
## Bottom line
The main ranking and temporal architecture is ready. Amend H1-H4 before implementation because each concerns a guarantee the current schema/API description cannot actually enforce: privacy classification, production budget availability, atomic budget admission, and latest-observation churn semantics. M1 should be fixed in the same edit so §30 becomes a reliable implementation checklist rather than a source of regressions.
@@ -0,0 +1,127 @@
# Plan Review: Personalized Ranking, Embeddings, Facets, and Feedback (v7)
**Reviewed:** 2026-08-19
**Plan:** `docs/plans/2026-08-17-personalized-ranking-and-facets.md` (revision 7)
**Verdict:** **The ranking architecture is ready, but the plan is not yet safe to execute verbatim.** Revision 7 resolves all four v6 High findings and the prior normative inconsistencies. Three narrower High-severity guarantees remain: provider protection is not durable across article re-ingestion, non-publication work can still consume the new production reserve by being labelled `production`, and the claimed conservative reservation uses an estimator that is not an upper bound. Two migration/test consistency issues should be corrected in the same pass. These amendments are localized; no redesign of the ranking pipeline is needed.
## Critical
No Critical finding.
## High
### H1. Protected-feed classification is correct for one ingest cluster but is not durable across re-ingestion
Section 25.1 now correctly checks every `SourceRef.feed_id` in the current deduplicated cluster. The plan also explicitly recognizes elsewhere that `db::upsert_article` overwrites `sources_json` on every re-ingest (§7.9, §27.2). The current implementation does exactly that in `src/db.rs:266-279`.
Those facts leave a temporal privacy hole. Consider this sequence:
1. Day 1 ingests canonical article A from protected feed 42. A is correctly withheld from providers but is not selected.
2. Day 2's overlapping window sees the same canonical URL only through a public mirror/feed. `upsert_article` replaces A's `sources_json`; feed 42 disappears.
3. The provider-policy constructor sees only the new public `Article.sources`, constructs `ExternallyProcessable`, and sends A.
The type-level gate is again faithfully enforcing an incomplete classification. The same issue affects a later profile rebuild, which reloads current article state. A `rating_events.feed_credits_json` row may preserve old provenance for rated articles, but it does not protect unrated articles and the wrapper does not consult it anyway.
This is not solved by merging historical sources into `articles.sources_json`: §7.7 deliberately uses current provenance for current candidate feed affinity. Privacy provenance and ranking provenance answer different questions.
**Required amendment:** persist durable source membership separately from the current cluster. For example:
```sql
CREATE TABLE article_feed_observations (
article_id INTEGER NOT NULL REFERENCES articles(id) ON DELETE CASCADE,
feed_id INTEGER NOT NULL,
first_seen TEXT NOT NULL,
last_seen TEXT NOT NULL,
PRIMARY KEY (article_id, feed_id)
);
```
Upsert every observed source during article persistence without deleting older observations. Build `ProviderPolicy` from the intersection of this table and the current configured protected IDs, then let the wrapper constructor consult the resulting protected-article set. Removing a feed ID from configuration can deliberately unprotect its articles; merely failing to observe that feed on a later day cannot.
Bootstrap the table from current `sources_json` at migration, state that older already-overwritten provenance is unrecoverable, and require the operator to verify the configured private subscriptions before rollout. Add a two-run regression: ingest A through a protected source, re-ingest the same canonical URL through public sources only, then prove every provider path still rejects A.
### H2. The production reserve is bypassed by classifying reusable shadow work as `production`
The reserve formulas in §7.6 are now correct, but their protection is only as strong as `budget_class`. Immediately after defining the reserve, the plan says embeddings produced during a shadow run are classed `production` because the cache is reusable (line 695). `profile rebuild` is also unconditionally listed as production (line 674), and dry-run classification is not specified.
That reopens the exact cross-invocation failure the reserve was introduced to close:
- Phase A is explicitly a shadow feature-collection phase and primarily performs embeddings.
- Those embedding attempts are labelled `production`, so they may consume the entire provider ceiling, including the reserve.
- A later publication run can then be refused despite the claim that Phase A cannot affect the paper.
Cache reuse may make a later run cheaper, but it does not make an evaluation request publication-critical. A shadow run over a different date, a broad feature collection, or an interrupted partial cache fill can spend the reserve without producing the exact artifacts the 05:30 run needs. Likewise, a standalone profile rebuild or dry run can consume DeepSeek's reserve before Stage A/B even though neither invocation publishes an issue.
**Required amendment:** classify by the purpose of the HTTP attempt, not by whether its output might someday be reusable. Prefer naming the privileged class `publication` to make the invariant explicit:
- provider calls required by an active issue-producing `generate` invocation: `publication`;
- calls made only for a shadow/dry run: `shadow`, including embeddings;
- standalone profile rebuild and feature backfill: `maintenance`/`backfill`, unless the profile rebuild is an in-run prerequisite for the issue currently being produced.
Only the issue-producing class may use `production_reserve_daily_usd`. Thread an explicit execution/budget context into provider orchestration; do not infer it from operation name or cacheability. Add order-sensitive tests for shadow embeddings, a dry run, and standalone profile rebuild before a live generation. Each must leave the reserve dispatchable.
### H3. The “conservative” reservation is based on a token estimate that can underestimate actual input
Section 7.6 calls the reservation conservative but says input tokens use the existing character approximation (lines 661-670). That helper is `text.len().div_ceil(4)` in `src/curate/mod.rs:159-163`, documented as a crude English-prose average. It is not an upper bound. Punctuation-heavy text, code, unusual Unicode, and provider tokenization can all use materially more than one token per four bytes/characters.
`max_output_tokens` safely bounds the output side, but an underestimated input reservation can be admitted just below the ceiling and then settle to an `actual_usd` above the estimate. At that point the provider call already happened and the day's ledger exceeds a ceiling acceptance criterion 22 says is enforced. The atomic transaction prevents races; it cannot repair an underestimated reservation.
**Required amendment:** either make reservation amounts genuine upper bounds or weaken the contract to a best-effort threshold with a stated maximum overshoot. For the strict contract currently promised, reserve input at a tokenizer-independent upper bound over the exact assembled payload (for example, one token per UTF-8 byte, if verified safe for both providers), plus maximum output at output price. Settle downward only from trustworthy usage. A provider-specific tokenizer is also acceptable if it is available locally and versioned with the model, but an English average plus a safety factor is still not a proof.
Add tests using adversarial prompt text and a mock response whose actual input usage exceeds `approx_tokens`. The admitted reservation must already cover that usage; settlement must never turn an under-ceiling admitted total into an over-ceiling total. If a strict upper bound is operationally too conservative, change the wording and acceptance criterion rather than claiming a hard ceiling.
## Medium
### M1. The observation bootstrap assumes an already-running `serve` process is the new dual-writing binary
Section 7.4b says a concurrent live vote is harmless because it appends an event newer than the seeded rows (line 469). That is true only after `serve` has been upgraded. During the `0002` rollout, an already-running old binary writes only the `ratings` projection. A new `generate` or migration command can acquire the new file lock, seed the event table, and commit; an old `serve` process can then accept a vote into `ratings` without appending `rating_events`. The durable bootstrap marker prevents any later repair, so the new event authority permanently misses that vote.
The generation lock cannot solve this because `serve` intentionally does not take it.
**Required amendment:** add an explicit cutover protocol: stop/drain the existing serve unit, install/start the new binary so migration and both bootstraps complete, then reopen the rating endpoint. Alternatively ship a compatibility release that dual-writes after the new tables exist before making events authoritative, but that is unnecessary complexity for this single-host service. Document the brief downtime and test the migration from a projection-only database; do not claim an old concurrent writer is safe.
### M2. Two migration tests still require the deleted legacy churn fallback
Section 7.8 and §18.4 correctly say pre-`0002` `scores` rows never suppress in any mode. Two later requirements say the opposite:
- §31.12: “Legacy pre-`0002` rows ... are excluded from fidelity and used in `recurate`.”
- §31.13: confirm “a v1 `scores` row is still usable by the churn rule.”
An implementation cannot satisfy those tests and the no-fallback contract simultaneously.
**Required amendment:** change both tests to assert that the v1 row remains readable as a compatibility projection but is never consulted by churn. The migration test should prove the old row survives schema migration, while a separate churn test proves it does not suppress in live, recurate, or fidelity modes.
## Low
### L1. The normative `VoyageConfig` shape omits the new reserve field
Section 9.1's Rust struct lists `max_daily_usd` and `embedding_retention_days` but not `production_reserve_daily_usd`, while §38 places that field under `[voyage]`. Add it to the struct and §30's file-by-file list. Also state where the shared `shadow_max_daily_usd` lives if it intentionally applies identically to both providers.
## Nits
- Section 24.0 is a subsection placed after section 24; `24.1`/`24.2` would read more naturally, though this has no implementation impact.
- Historical resolution tables still use `no_external_ai_feeds` in some older-review rows. They are clearly historical, so this is harmless, but adding “superseded by R7-H1” would reduce search noise.
## Alternatives
### Sticky Boolean restriction
Instead of a general source-observation table, add an `external_ai_protected` bit to `articles` and set it monotonically when any protected source is observed. This is smaller, but changing the configured feed list cannot automatically unprotect previously marked articles and loses the audit trail explaining why the bit was set. It is acceptable only if unprotection is intentionally a manual operation.
### Purpose-based budget classes (recommended)
Rename `production` to `publication` and pass a typed `BudgetContext` from the top-level command. This makes misuse difficult: cache code cannot promote itself merely because its output is reusable. The alternative is per-operation allowlists, which are easier to forget when adding a provider call.
### Best-effort budget threshold
If a byte-level upper bound rejects too much useful work, retain the existing estimator but explicitly define the limit as an advisory threshold, reserve with a documented safety factor, trip immediately when settlement exceeds it, and report maximum observed estimation error. This is operationally reasonable, but it is a different guarantee from “ceilings are enforced before dispatch.”
## Open Questions
1. Does an article remain protected after it was ever observed through a protected feed, until the operator removes that feed ID from configuration? The plan's “no field ever leaves” wording implies yes.
2. Which exact invocations may consume the production reserve: only issue-producing generation, or also dry runs, standalone profile rebuilds, and shadow cache warming?
3. Is the daily budget intended as a strict pre-dispatch ceiling or a best-effort runaway threshold? The estimator and acceptance criterion currently answer differently.
## Bottom line
Revision 7 resolves the prior review and leaves the core recommendation design in good shape. Amend H1-H3 before implementation because they affect the two strongest operational guarantees: private content never leaves the host, and shadow/maintenance work cannot exhaust publication capacity. M1 is a deployment-order requirement, while M2 and L1 are quick consistency fixes. After those changes, the plan should be ready to implement.
@@ -0,0 +1,211 @@
# Re-review — Personalized Ranking, Embeddings, Facets, and Feedback (v2)
**Plan:** `docs/plans/2026-08-17-personalized-ranking-and-facets.md`
**Date:** 2026-08-19
**Scope:** revision 2, after both 2026-08-18 reviews
## Verdict
The revised plan is substantially better: the evidence ladder, presence-aware normalization, union admission, Stage A facet folding, signed nearest neighbors, run-scoped telemetry, as-of semantics, and cluster-cap direction resolve the important defects in both earlier reviews. The core architecture is sound. It is **not ready to execute unchanged**, however. Two issues are blocking: the evaluator filters on a run status that the application never writes, and the promised provider opt-out still leaks opted-out articles through later DeepSeek stages. Before implementation, the plan also needs to repair an impossible Phase A success criterion, persist historical taste-profile versions if replay is meant to use them, and remove cached-only facet preference from admission. These are localized amendments; they do not require redesigning the overall approach.
## Critical
### C1. Evaluation filters on a nonexistent `complete` run status
Evidence:
- §7.5 says evaluation “must ignore rows whose `runs.status != 'complete'`.”
- §27.1 repeats that `evaluate` “must ignore runs whose `status != 'complete'`.”
- The current `RunStatus` vocabulary is `running | ok | degraded | failed | dry_run` (`src/report.rs:19-41`), and `Db::finish_run` stores those exact values.
- Migration `0002` adds no `complete` status and the plan does not change `RunStatus`.
Implemented literally, every run is excluded from evaluation. Phase A can therefore never accumulate its required completed runs, and failed/truncated-run filtering cannot be tested meaningfully.
**Required amendment:** define evaluation eligibility in terms of the real lifecycle. A reasonable default is:
- production outcome metrics: `status IN ('ok', 'degraded')`;
- shadow diagnostics: the same statuses plus the manifests shadow marker;
- dry-run diagnostics: included only when explicitly requested;
- always exclude `running` and `failed`.
Use one typed helper/query predicate everywhere rather than copying SQL strings. Add tests covering all five current statuses. If the intent is instead to rename `ok` to `complete`, specify the migration, enum change, compatibility behavior, and existing-row rewrite.
### C2. `no_external_content_feeds` does not actually keep articles away from DeepSeek
Evidence: §25 promises that matching articles are “never sent to Voyage or DeepSeek,” then only specifies “no embedding, no facets, and no Stage A score.” But such an article remains ranked on heuristic signals and can still:
1. enter the Stage B prompt, whose current renderer includes title, feed, and body-derived blurb (`src/curate/select.rs:157`);
2. be selected and have its body sent to the per-article summary call (`src/curate/editorial.rs:119-170`);
3. contribute title/feed/facet metadata to a later weekly profile rebuild (§22).
This breaks the explicit privacy contract for the private/authenticated-feed use case that motivated the setting.
**Required amendment:** centralize provider eligibility and apply it at every provider call, not only embedding and Stage A orchestration. Define whether the restriction covers article body only or all article-derived metadata. Under the strict meaning currently documented:
- omit protected candidates from Stage B and reinsert any mandatory protected articles deterministically afterward, subject to `hard_max`;
- always use local excerpt summaries for protected picks;
- exclude their rating-history metadata from the DeepSeek profile-rebuild prompt;
- assert with a recording mock that no Voyage or DeepSeek request contains any protected article field.
If titles/metadata may be sent while bodies may not, rename and document the policy accordingly; the current “never sent” wording is broader.
## High
### H1. Phase A requires a counterfactual upvote that admission-only shadowing cannot observe
Evidence: §32 correctly says Phase A shadows admission only while the old selector remains authoritative. Its exit gate nevertheless requires the union to “admit at least one upvoted article per week that the prefilter would have dropped.”
An article dropped by the authoritative prefilter is not shown in the issue, so it cannot receive an upvote. Historical selected/upvoted articles necessarily survived the old funnel on the day they were shown. The first half of the gate—retaining at least 95% of known positives—is observable; the claimed rescued-positive rate is not. This is selection bias, not something another week of shadow data fixes.
**Required amendment:** replace the impossible half of the Phase A gate with an observable measure. Options include:
- weekly operator adjudication of a fixed sample of union-only candidates;
- a small, explicitly bounded interleaving bucket that exposes union-only candidates;
- retrospective labels from an independent source, if one genuinely exists.
Then move measured user upvote yield for rescued candidates to Phase B, after those candidates can actually be exposed. Record impressions/exposure origin so this cohort can be evaluated.
### H2. Replay requires historical profile selection, but the plan stores only profile metadata
Evidence:
- §6.1 requires profile-version selection bounded by `as_of`.
- §6.2 says replay disables prose profile only “if no profile version was effective at `as_of`.”
- §7.4 stores only `profile_version` and `profile_hash` in the run manifest.
- The current implementation overwrites `taste_profile`, `taste_profile_learned`, and `profile_version` singleton keys in `kv` (`src/curate/profile/mod.rs:197-240`).
- Migration `0002` proposes no profile-history table and the manifest does not store profile text.
After the next weekly rebuild, the profile text that was effective for an earlier date is gone. The evaluator cannot select it by `as_of`, and a hash cannot reconstruct it. Silently disabling the profile would also make replay depend on whether an old version happened to survive in `kv`.
**Required amendment:** add immutable profile history, for example:
```sql
CREATE TABLE taste_profile_versions (
version INTEGER PRIMARY KEY,
built_at TEXT NOT NULL,
profile_hash TEXT NOT NULL,
profile_text TEXT NOT NULL
);
CREATE INDEX idx_taste_profiles_built_at ON taste_profile_versions(built_at);
```
Write a new row transactionally whenever the singleton/current pointer changes, select the latest `built_at <= as_of`, and migrate the currently stored profile as the first historical row. If retaining profile text is unwanted, explicitly state that replay always disables the prose profile; do not promise version selection.
### H3. Cached facets create an incumbency-only admission signal
Evidence: §16.4 gives `facet_preference` 0.14 preliminary weight while acknowledging it is absent for new articles and present only for articles previously sent through Stage A. The plan says presence-aware renormalization “handles” the asymmetry.
Presence-aware blending correctly handles outages and genuinely unavailable signals, but it does not make informative missingness fair. Previously admitted recurring articles get an extra positive or negative feature that brand-new articles cannot receive before the same admission cut. Since the 26-hour ingest window overlaps days, this makes prior Stage A admission part of the next days ranking and can create self-reinforcing survival. It also makes the admission formula depend on cache history rather than solely on the candidate and declared `as_of` evidence.
**Required amendment:** remove `facet_preference` from the preliminary/admission blend in V1. Use it only in post-Stage-A utility, where all successfully scored candidates have equal opportunity to obtain facets. Promote it into admission only if a later dedicated or deterministic pre-admission facet path provides comparable coverage across the eligible set. Presence-aware normalization should remain for genuine provider/cache failures.
### H4. “Atomic” provider budgeting is underspecified across concurrent processes
Evidence: §24 requires reserve-then-spend accounting to be atomic under concurrency, but §7.6 only persists completed usage on `runs` and preloads same-date spend. An in-process meter can coordinate `buffer_unordered` tasks, but two overlapping `generate` processes can both preload the same balance, reserve locally, publish concurrently, and exceed both provider ceilings.
The same overlap can race issue publication and whole-table feed-prior rebuilds. A systemd timer lowers the probability but does not prevent an operator-triggered rerun from overlapping the scheduled process.
**Required amendment:** choose and specify one model:
- simplest: a database-backed generation lease/mutex, with stale-lease recovery, that permits only one mutating generation process at a time;
- more flexible: a provider reservation ledger updated in an immediate SQLite transaction, plus explicit publication serialization.
Add a two-process/concurrent-connection test. If operational policy guarantees serialization instead, enforce it in the binary rather than relying on convention.
## Medium
### M1. The exploration ramp formula does not reach full strength at `evidence_full`
§17 defines:
```text
exploration_reserve =
round(exploration_max *
clamp((W - exploration_floor) / exploration_full, 0, 1))
```
and says `exploration_full = evidence_full = 20`, with `exploration_floor = 15`. At `W = 20`, the reserve is only `8 * 5/20 = 2`; it reaches the configured maximum at `W = 35`. That conflicts with the names and with the evidence ladders “full at 20” semantics.
**Recommendation:** either use `(W - exploration_floor) / (evidence_full - exploration_floor)`, or introduce a separate explicit `exploration_ramp_width`/full threshold and document that full exploration begins at 35. Add boundary tests at below-floor, floor, full, and above-full values.
### M2. The diversification algorithm is not single-linkage clustering
§20 calls the method “single-linkage cluster caps,” but its algorithm assigns a candidate to the first existing cluster containing a similar member and never merges two existing clusters bridged by a later candidate.
For A similar to C, B similar to C, and A not similar to B, processing A then B then C leaves two clusters; true single linkage produces one connected component. The current method is deterministic greedy threshold assignment, but its cluster caps and explanations can differ materially from the stated design.
**Recommendation:** either:
- implement actual connected components/union-find over the threshold graph (cheap at 120 candidates), accepting single-linkage chaining; or
- deliberately keep the greedy algorithm, rename it, specify cluster-order semantics, and test bridge cases.
Complete-linkage or leader clustering is also worth considering if chaining entire news cycles into one cluster is undesirable.
### M3. Run-manifest creation conflicts with its required fields
§30 tells `pipeline.rs` to create the manifest “immediately after the run row,” but `run_manifests.rating_evidence_weight` is `NOT NULL` and is only computed when preference state is loaded later in the §5 pipeline. Profile selection and feature availability are also not necessarily final at run creation.
This invites either fake defaults in supposedly authoritative manifests or incremental mutation of a snapshot described as the record needed to interpret a run.
**Recommendation:** distinguish an initial run configuration from a finalized ranking manifest. Either load the bounded profile/rating evidence before inserting the manifest, or allow a clearly defined `running` manifest to be finalized transactionally before candidate rows become evaluable. Evaluation must require finalized manifest state in addition to terminal run status.
### M4. The Stage A example uses an invalid facet enum value
§15.2 defines the scored `format` vocabulary as:
`reported_news | analysis_essay | how_to_technical | first_hand_account | announcement_roundup`.
But §18.1s canonical response example uses `"postmortem_case_study"`. Under the required tolerant parser, that value is dropped to `None`, so an implementation copied from the example loses precisely the first-hand postmortem signal used throughout the plans motivating examples.
**Recommendation:** make the prompt example and all fixtures use the exact enum vocabulary, or add `postmortem_case_study` to the schema and update the claimed cardinality. Add a test that every enum token embedded in prompt examples is accepted by the parser.
## Low
### L1. `article_facets.article_id` lacks the foreign key used by the other feature tables
The proposed `article_facets` schema declares `article_id INTEGER NOT NULL` without `REFERENCES articles(id) ON DELETE CASCADE`. `article_embeddings` has that relationship. Add it so article deletion or future archival cannot leave orphaned facet rows.
### L2. Run mode has two writable sources of truth
§7.4 stores `run_manifests.mode`, while §7.6 also adds `runs.mode`. Without a constraint or one-way derivation they can disagree, undermining the evaluators mode filtering. Prefer one authoritative column and expose the other through a join; if both are kept, write them in one transaction and test consistency.
### L3. The lifecycle wording should distinguish “budget-degraded” from “truncated and unusable”
The plan says a Stage A budget trip should exclude the run from metrics, while the current application deliberately marks guardrail trips `degraded` and can still publish a valid fallback issue. Some metrics (provider completion and Stage A accuracy) should exclude such a run, while admission, fallback behavior, issue size, and user ratings remain meaningful.
Use per-stage completeness fields/counts from the report/manifest rather than excluding an otherwise valid run wholesale.
## Nits
- §16.1 says thin hygiene rows make “acceptance criterion 10” testable; in v2 this is acceptance criterion 12.
- The plan alternates between “completed run” as an English phrase and the nonexistent literal status `complete`; use typed status names consistently.
- `source TEXT -- 'stage_a' | 'dedicated'` in `article_facets` should have a `CHECK` constraint if the value is used for cache or provenance decisions.
- `run_manifests.shadow`, `dry_run`, and `mode` should receive the same SQLite `CHECK` treatment already required for candidate flags.
## Alternatives
### Alternative A: Keep facets strictly post-admission in V1
Remove facet preference from preliminary ranking, extract/reuse facets during Stage A, and apply facet preference only to utility. This produces uniform feature opportunity, simplifies Phase A, and preserves facets for explanation, profile rebuilding, and later learning. Prefer this until evaluation justifies a uniform pre-admission facet path.
### Alternative B: Controlled interleaving for counterfactual recall evidence
Reserve one or two issue slots—not merely shortlist slots—for candidates admitted only by the new union, subject to the existing quality floor. Mark their exposure origin and compare their explicit rating rate with baseline picks. Prefer this when the operator accepts a small visible experiment and wants genuine user labels. If not, use blinded operator adjudication during Phase A and postpone user-yield claims to Phase B.
### Alternative C: Central provider-policy wrapper
Represent external-processing permission as a typed policy on each article and require every Voyage/DeepSeek orchestration function to accept only an `ExternallyProcessableArticle` wrapper produced by one central filter. Prefer this over scattered feed checks: it makes accidental Stage B/editorial leakage harder to compile and easier to test.
### Alternative D: Serialize generation rather than building a distributed budget ledger
For this single-reader, single-host service, take a SQLite-backed generation lease before starting a mutating run and release it on terminal completion, with timeout/stale-owner recovery. This is simpler than cross-process provider reservations and also prevents publication and feed-prior races. Prefer the ledger only if overlapping generation is a real requirement.
## Open Questions
1. Which current statuses count as evaluable: `ok` only, `ok + degraded`, and/or `dry_run` for shadow diagnostics?
2. Does `no_external_content_feeds` prohibit body text only, or titles, feed names, facets, and rating-history metadata as well?
3. How will Phase A obtain labels for union-only candidates that the authoritative selector never exposes?
4. Is historical prose-profile fidelity required for replay? If yes, immutable profile text must be stored; if no, replay should always disable that input and say so.
5. Can two `generate` processes overlap in supported operation? If not, should the application reject the second invocation immediately or wait on a lease?
6. Is true single linkage intended despite its chaining behavior, or is the current greedy first-matching-cluster algorithm the desired product rule?
@@ -0,0 +1,83 @@
# Plan review: personalized ranking and facets, revision 8
## Verdict
Revision 8 is substantially stronger and most earlier architectural defects are now resolved, but it is not quite implementation-ready. Two remaining issues affect hard guarantees rather than tuning: the durable privacy authority is described inconsistently enough that an implementation can still fail open, and the provider reservation formula is not yet a proven upper bound for batched requests with provider-added tokens. Resolve those before implementation; the remaining medium/low items can be folded into the same amendment pass.
## Critical
### C1. The durable privacy authority is not wired into one atomic, fail-closed contract
The new `article_feed_observations` table is the right data model, but four normative parts of the plan disagree about how it becomes authoritative:
- Section 25.1 says `bootstrap_observation_history()` seeds `article_feed_observations`, while §7.4b defines that bootstrap as seeding only `rating_events` and `publication_events` before writing its durable marker. If implemented from §7.4b, the marker can permanently certify an incomplete privacy bootstrap.
- Section 25.1 says `ProviderPolicy::load` builds the protected set and the wrapper consults it, but §30 specifies `externally_processable(&Article, &CurationConfig)`. That signature has neither the loaded policy nor the durable observation set and invites reimplementation of the v7 current-source check.
- “Article persistence upserts one row per observed source” does not require those writes to be in the same transaction as `articles`/`sources_json`. If the article upsert commits and an observation insert fails, the newly persisted protected article is indistinguishable from a public one to the next policy load.
- The general fallback rule says new external stages are non-fatal, but no rule says what happens when `ProviderPolicy::load` or an observation write fails. Treating an error as an empty protected set would disclose content under exactly the failure mode the type is supposed to prevent.
This is a confidentiality boundary, so ambiguity is itself a blocker. Make one normative contract:
1. Put `article_feed_observations` in the migration/observation-layer work item, and have `bootstrap_observation_history()` seed all three observation authorities plus its marker in one transaction.
2. Persist an article and all feed observations from that ingest in one transaction. A failure rolls back both.
3. Make the only constructor `externally_processable(&ProviderPolicy, &Article)` (or a method on `ProviderPolicy`); it must not accept configuration alone.
4. Define failure as closed: if the policy cannot load or privacy provenance cannot be committed, make no Voyage or DeepSeek calls. The issue may continue through the local-only path, but an empty/default policy must never be substituted.
5. Fix §33 sequencing. Step 0 is called independent, yet durable classification requires the table currently assigned nowhere explicitly and the bootstrap currently placed later. Either move the privacy table/bootstrap into step 0 or split “introduce the wrapper” from “activate provider calls” so no intermediate commit claims the guarantee without its authority.
Add fault-injection tests for failure between article and observation writes, a bootstrap with existing protected `sources_json`, a pre-existing bootstrap marker, and a failed policy query. Each must result in zero external dispatches.
## High
### H1. `bytes + 256` is not yet a strict upper bound on provider-billed input tokens
Section 7.6 correctly rejects `len/4`, but the replacement only bounds caller-visible payload bytes. The plan itself states in §11.1 that Voyage prepends a retrieval instruction server-side for `input_type = "query"`. That text is not in `payload_utf8_bytes`; moreover, a request may contain up to 1,000 inputs, so provider-added framing or instructions can scale per input rather than once per request. DeepSeek chat framing similarly scales with message structure. A fixed `per_request_overhead_tokens = 256` is therefore an assumption, not a demonstrated bound.
The acceptance test is also internally contradictory: it requires settlement never to turn an admitted under-ceiling total into an over-ceiling one, while the next bullet says usage above the reservation merely trips the meter. Tripping after settlement detects that the guarantee failed; it does not preserve the pre-dispatch ceiling.
Define a provider-specific bound over the actual request shape, for example:
```text
payload/body byte bound
+ request framing bound
+ per-message bound * message_count
+ per-input bound * input_count
+ maximum output tokens at the undiscounted price
```
The constants must be justified by provider limits or conservatively replaced with a documented maximum-context reservation. If no stable bound exists, weaken the product contract to “conservative guardrail” instead of “strict ceiling”; do not claim both. Add tiny-input/maximum-batch and many-message tests, with the ledger one reservation below the ceiling, so hidden overhead—not only adversarial article text—is exercised. Also add every bound constant, including `per_request_overhead_tokens`, to §38 and the manifest; the appendix currently claims every number is there, but this one is absent.
## Medium
### M1. Existing cached features conflict with the protected-article semantics
Section 25.1 says a protected article has “no embedding, no facets” and that its rating cannot raise `W_embedding` or `W_facet`. Durable classification can, however, discover protection after an embedding/facet was already cached: the operator may add a feed ID later, or the migration may recover a currently visible protected source after an earlier public run processed the article. Nothing currently says whether those cached rows are deleted, ignored, or remain locally usable.
Choose and specify one behavior. The simplest contract matching the current prose is to filter protected article IDs out of embedding/facet loads and all evidence-weight calculations, without requiring destructive deletion. Add a test that caches both features first, then marks the article protected, and verifies zero provider calls plus no contribution to either learned signal.
### M2. Conservative reservations can accumulate beyond the in-flight set
Section 7.6 says the roughly 4× reservation inflation applies only to at most four in-flight attempts and “never to the days accumulated total.” That is false for the deliberately conservative crash/retry behavior: `failed_estimated` rows and process-death `reserved` rows retain their estimates for the rest of the billing day. Several failures can therefore consume substantially more apparent budget than the concurrency limit suggests.
The safe accounting rule should stay, but correct the capacity claim and make the operational consequence visible. Report standing estimated reservations separately, including stale `reserved` rows, and state that repeated ambiguous failures may intentionally halt the provider for the day. Do not add an automatic release timeout unless provider billing semantics can prove the request was not charged.
## Low
### L1. One normative ledger test still uses the superseded class name
Section 31.11b says a run mixing “production and shadow” calls should stop shadow while production continues. The actual closed vocabulary is now `publication | shadow | maintenance`. Rename the test wording so an implementation does not recreate or alias a fourth class. Historical resolution-table uses can remain when explicitly marked superseded.
## Nits
- The plan header and the R8 resolution table are dated 2026-08-19, while this revision is being reviewed on 2026-08-20. Updating the revision date would make the review chain easier to audit.
- Section 29s example log still prints a single `W=0.0`; use the four evidence weights already required elsewhere so the canonical example does not teach an obsolete field.
## Alternatives
For privacy, the strongest alternative is to make externally processable status a persisted monotone article flag maintained transactionally during ingest. The separate observation table is preferable because it preserves auditability and supports deliberate unprotection by configuration, but only if all provider access goes through a successfully loaded policy and failures close the gate.
For budgets, reserving the providers maximum accepted context per request is coarser but easier to prove than maintaining tokenizer/framing constants. It may reduce concurrency near a small ceiling, but it is a sound fallback if provider-specific hidden overhead cannot be bounded from a stable contract.
## Open Questions
1. On a policy-load or privacy-observation write failure, should generation abort entirely, or continue as a local-only degraded issue? The plan should choose; either is safe, while continuing external calls is not.
2. Are cached embeddings/facets for a newly protected article allowed for local ranking, or must protection also remove their influence? The current prose chooses the latter implicitly, but the storage rules do not enforce it.
3. Can each providers billed-token definition and hidden framing overhead be bounded from a stable API contract? If not, is a conservative guardrail acceptable in place of the stated strict daily ceiling?