the-daily-epub/docs/reviews/2026-08-17-personalized-ranking-and-facets-review-2026-08-19-3.md

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.