Discovery measurement: focused server review
Follow-up: independent controller/warehouse re-review and current finding status. This document retains the original evidence and subsequent implementation updates for findings 2/5; its original verdict/test failures below describe the initial review point.
Reviewed 2026-09-19 on codex/discovery-measurement, base 49eec145a, against the current measurement contract and implementation plan. Read AGENTS.md. Scope: controller-owned receipts, canonical observations/outcomes, serving snapshots, mutation integrations and schema. No app, Redis admission/store, warehouse or offline-worker review; no reopening of foundation PR172.
Verdict: changes required before enabling measurement or treating these events as trustworthy new-discovery/training data. Two P1 data-validity blockers and three P2 attribution defects are below. This is not a finding against deploying the completed foundation with measurement disabled. Source files were not edited; only this report was written. The controller was editing concurrently, so the verification results describe the executions below, not a frozen branch or full release approval.
1. P1 — Deleted history becomes a false “new story” label
Evidence: knownDiscoveryWorlds, lines 14–22, outcome newness, lines 29–33, and Library removal, lines 342–354.
The historical query only sees surviving Library and session rows. created_at <= servedAt cannot recover deleted rows. Library removal deletes both sources, including translated siblings. Moreover, wasKnownAtServe: null on a guest opportunity passes the !== true test and becomes a definite positive when the later query returns no rows.
Reproduced in isolated full-schema PGlite: insert a Library entry and non-ephemeral session dated one day before exposure; verify the story is known; issue a guest receipt; remove both historical rows; identify the same guest browser as that account and prepare a save/play outcome. The actual implementation returns isNewStory: true, although the story was saved and played before exposure. Removing a story before a later signed-in exposure has the same loss-of-history problem.
Action: base historical newness on durable, time-stamped first-save/consumer-play evidence, including group identity, or emit unknown/censored newness when historical coverage is insufficient. Do not promote guest null plus absence from mutable tables to proven novelty. Test removal/re-add and guest-to-user outcomes after history removal. This is required by the contract's “saved/played before the opportunity” definition, not a new label requirement.
2. P1 — Snapshots omit inputs that actually determine the serving order
Implementation update: addressed by this reviewer's subsequent authorized fix, not independently re-reviewed. Ranking-stage instrumentation now captures retention inputs, actual scoring components, diversity/quota/semantic/exploration decisions and pre/post positions; selection adds separate immutable delivery costs and metadata. The engagement loader exposes the source rollup timestamp with explicit unknown freshness semantics. See final snapshot contract and verification. The five focused suites pass 89/89, including real-feed page/retry mutation tests; scoped TypeScript diagnostics are zero. The original evidence below is retained as the regression rationale.
Evidence: snapshot construction, lines 45–60, cold-start retention scoring, lines 3294–3304, and delivery diversity cost, lines 74–91.
The snapshot contains the model vector and per-candidate base-score breakdown. Cold-start ordering additionally uses totalPlayMinutes30d and returners30d; neither is present in those collected features. rankPosition is captured after rankRecommendedWorlds has already applied its slate/diversity policy. The subsequent cursor selection uses current hydrated creator/topic metadata and recent-page history without recording those decision inputs or costs.
Reproduced using the real ranking and feature functions: two otherwise identical candidate/context fixtures differ only in the two retention statistics. All numeric features collected by the production snapshot logic compare equal per candidate, while the actual ranked orders are [a,b] and [b,a]. Recording the resulting rank does not recover the missing input that caused it.
Action: instrument the decision stages themselves. Preserve the retention inputs/derived retention score, relevant statistic snapshot time, and the derived diversity/exposure inputs and pre/post positions actually used. Keep queued values immutable and record delivery-stage decisions separately. Add a parity test that changes only a deciding retention/diversity input and verifies the corresponding snapshot changes. Until then these are partial model/base-score snapshots, not the exact serving-policy snapshots promised by this milestone.
3. P2 — Retrying an opportunity can sign a different language group
Evidence: receipt entries and insert conflict behavior, lines 36–57, observation-origin validation, lines 71–74, outcome group validation, lines 23–28, and replay hydration, lines 98–108.
The committed page retains IDs/time/features but no immutable group identity. recordDiscoveryServe signs the currently hydrated group on every retry, while onConflictDoNothing preserves the original served row. Ingestion checks actor/world/position but not the group; outcomes validate a selected sibling against the new receipt instead of the stored origin group. Group edits are real supported operations, including linking and unlinking translations.
Reproduced in PGlite: serve a world with a null group, link it and a sibling into a group, then replay the same page/request/position. The token changes. An accepted click records the new group while its served opportunity still records null. A save attribution for the newly linked sibling is also accepted against that original null-group opportunity.
Action: retain the original opportunity group and sign retries from the persisted canonical opportunity identity. Validate or derive every accepted observation/outcome's immutable identity against that origin. If regrouping makes a translated target incompatible with the original opportunity, omit attribution or issue an explicitly new opportunity; do not silently reuse its ID. Add link/unlink-between-retries and translated-outcome tests.
4. P2 — Successful-outcome eligibility relies on a stale pre-transaction world
Evidence: eligibility checks, lines 17–28, Library lookup and transaction, lines 261–275, and session lookup/outcome transaction.
Both routes read the target before the mutation transaction. prepareDiscoveryOutcome checks that supplied object's publication/group fields but never reads or locks the target row. Its advisory lock only coordinates this user's attribution operations; publication/group edits do not acquire it.
Concrete interleaving, reproduced at the outcome boundary: read a published target; change its database status to draft; invoke the transactional preparation with the route's previously read target. Preparation still accepts it. A sibling unlinked after the route lookup is similarly evaluated with obsolete membership. The same transaction can then persist a successful mutation and an authoritative attribution whose eligibility check was no longer true. This finding concerns the new attribution guarantee; it does not claim the receipt itself grants access or that the pre-existing route race originated in this change.
Action: obtain authoritative target eligibility/group data inside the mutation transaction and serialize the relevant world edits through a suitable row lock/conditional write. Verify against the canonical opportunity. Add a deterministic interleaving test for unpublish/unlink between the initial lookup and mutation; an incompatible target must not receive a confirmed discovery attribution.
5. P2 — Catalog replenishment is logged as popular retrieval
Implementation update: addressed by this reviewer's subsequent authorized fix, not independently re-reviewed. Keyset catalog retrieval is now explicitly catalog; its numeric provenance flags and candidate-source list survive ranking. The full-schema regression exercises the actual catalog query and confirms source_catalog=1, source_popular_recent=0. Included in the 89/89 focused result above.
Evidence: snapshot source capture, replenishment integration, and catalog normalization.
Every candidate from the ID-keyset catalog scan is normalized with candidateSource: ["popular_recent"]. Consequently the snapshot's ?? ["catalog"] fallback never applies to this path. Once the initial candidate queue needs replenishment, long-tail stories fetched solely by the catalog scan are recorded as popular-retrieval successes. This systematically misattributes source coverage and downstream outcomes; it is not an occasional missing label.
Action: mark the actual catalog retrieval route before snapshotting, distinct from genuine popular-source membership. Verify with a story absent from the initial source union that is reached only through catalog replenishment. The previously harmless normalization default becomes a measurement defect when persisted as source provenance here.
Verification and qualifications
- Ran
node packages/server/scripts/test-local.mjs src/lib/discovery-receipt.test.ts src/lib/discovery-measurement.test.ts src/lib/discovery-session.test.ts src/routes/discovery-attribution.test.ts: 18 passed, 1 failed. The 14 receipt/measurement/session tests passed; the route suite passed 4/5. The failing route case was the 70,000-character body-budget test: 400 returned, 413 expected, at discovery-attribution.test.ts:74. - Re-ran only the attribution route suite after observing a controller edit to the error handling: the same body-budget assertion still failed, while the other four cases passed. Treat this as an outstanding integration-test failure, not evidence that the body-size resource bound can be bypassed. Controller verification must resolve it before claiming a green focused suite.
- Ran separate credential-stripped, dotenv-disabled, in-memory full-schema reproductions for findings 1, 3 and 4, plus a pure ranking/feature comparison for finding 2. All four assertions reproduced. No fixture/test files were added. Finding 5 follows directly from the catalog-source assignment and its snapshot consumer.
- Positive findings: signed receipts enforce actor ownership and page membership; authenticated receipts cannot be reused by another account; guest observations preserve their original actor; client intent cannot masquerade as confirmed success. Canonical and compatibility observations are transactionally written and deduplicated. Save/session outcomes share the actual mutation transaction, duplicate Library inserts do not create another save outcome, and creator/ephemeral sessions are excluded by the reviewed code. Feature values already captured in a queued snapshot survive retries. These strengths do not resolve the historical/group/feature issues above.
- No full build, OSS export, production access, browser, commit/push or subagent use. Full release verification remains controller-owned. This review establishes neither complete system coverage nor causal recommendation uplift.
