Skip to content

Discovery offline measurement review — 2026-09-19

Independent review in C:\Users\jefra\.codex\worktrees\yumina-discovery, branch codex/discovery-measurement. Scope: Python dataset/evaluation logic and its interface with canonical server snapshots, mutation outcomes, play intervals and exports. This is not a review of this worker's own client implementation or dismissal routes. The supplied client-event contract is an input boundary only. No Python or production source was edited by the reviewer.

Post-correction verdict: no remaining P1/P2 code-contract defect was reproduced in the reviewed v3 boundary. All three baseline findings below are resolved, as are the full-story D7 and nullable-history integration gaps. Independent verification passed 86 Python tests, 9 strict warehouse-export tests and 4 source-history export tests. This is a bounded implementation review, not proof of complete production observations, causal recommendation quality or a model-promotion decision. Source-completeness and experiment gates remain below.

Reviewed correctionIndependently verified result
Confirmed dismiss/UndoOriginal mutation-bound hide and Undo survive as negative state; matching Undo clears hidden state without adding success labels. Conflicting event IDs mark stateKnown=false / finalHidden=null.
Frozen delivery snapshotsDeclared delivery features and original creator/topic/group metadata survive; rank-time features remain separate.
Actor denominatorsTen successful visits by A plus one empty visit by B yield visit rate 10/11, equal-actor mean 0.5, and mean distinct successful stories 5.
Full story D7A later verified new Library chat counts after the original new-start anchor. False/null consumer eligibility does not supply strict play time.
Historical group identityNull groups are accepted as observed ungrouped periods; membership beginning after exposure does not backfill attribution.

Resolved findings from the reviewed baseline

P2 — Canonical dismissal and Undo are discarded

At the reviewed baseline, trainer/discovery_data.py:27 admits only save_confirmed and session_started as outcomes. EVENT_TYPES at line 28 consequently rejects the server's dismiss_confirmed and dismiss_undone, which packages/server/src/lib/discovery-outcomes.ts:49 already supports. A valid served opportunity followed by a matching confirmed dismissal and Undo produces unknown_event_type: 2; neither action appears in its observations or labels.

This is a concrete schema integration gap, independent of whether enough source history has been collected. It loses explicit negative feedback and cannot distinguish an active dismissal from regret/Undo. Do not substitute an unclicked impression for a dismissal. Negative feedback also must not require new-story eligibility: the server intentionally accepts known, creator-owned and unpublished targets in negative mode.

Requested correction: validate original opportunity/actor/world context, preserve confirmed mutation identity, match Undo to the same dismissalId, deduplicate, and report active versus undone feedback without converting client intent to confirmation. The server owns the mutation proof. Poincare confirmed this was not previously in-progress work and the controller subsequently assigned it.

Resolved: v3 negative attribution now performs those checks and exports separate negative state. An additional independently reproduced canonical-ID conflict was corrected and rechecked: quarantining contradictory hides no longer certifies an unhidden state. Knownness does not gate negative feedback.

P2 — Dataset output loses delivery-stage policy inputs

The server now freezes deliveryFeatures and deliveryMetadata in packages/server/src/lib/discovery-pagination.ts:108. These record cursor diversity costs, exclusion counts, original/delivered positions, and creator/topic/group metadata. The canonical serve persists the snapshot. However, the reviewed make_samples allowlist at trainer/discovery_data.py:655 copies only features, source, rank, variant and knownness.

Independent input with deliveryFeatures={creator_repeats:2,cost:43,post_diversity_position:0} and corresponding creator/topic/group metadata is accepted, but both fields disappear from the artifact. The source export retains enough information; the builder removes it. This prevents the promised full-policy audit and creator/topic analyses even after source coverage is complete.

Requested correction: validate and preserve these immutable delivery fields separately from the original rankedAt feature vector, with explicit schema/version semantics. Do not reconstruct them from current worlds, recompute them offline, or silently mix delivery-time decisions into a rank-time model-input schema. Correction assigned to Poincare.

Resolved: v3 validates a separately declared delivery_feature_schemas mapping and immutable metadata, then retains both fields in its candidate output. Independent probes confirmed exact value preservation and separation from the original features.

P2 — Contracted actor aggregation is missing

The baseline evaluator's successfulVisitRate is correctly calculated as successful visits divided by eligible visits (trainer/discovery_evaluation.py:154). It is not an actor-weighted result. The baseline visit artifact also drops actor/user identity (trainer/discovery_data.py:731), including genuinely empty visits, which have no candidate row from which to recover it.

Independent probe: actor A has 10 successful visits out of 10; actor B has zero out of one. The report returns 0.9090909090909091, whereas the mean of the two actor visit-success fractions is 0.5. No actor aggregates are emitted. This is a missing contracted metric and artifact field, not an arithmetic error in the honestly named visit metric. The measurement contract requests actor-level outcomes and distinct qualified new stories per actor; heavy visitors must not silently stand in for more randomized actors.

Requested correction: preserve canonical actor identity on every visit, including page-only empties; specify handling of mixed/conflicting identities and guest links; emit clearly named actor aggregates and distinct-story counts while retaining visit diagnostics. Actor aggregation does not by itself establish randomized assignment, causal uplift or uncertainty. Correction assigned to Poincare.

Resolved: visits retain original actor/user identity and consistency status. The evaluator emits eligibleActors, actorMeanVisitSuccessRate and actorMeanDistinctSuccessfulStories; mixed-identity visits are counted as excluded from actor diagnostics. The unequal-visit-count reproduction now reports both the correct visit and equal-actor values. Guest identities are not silently merged or rewritten.

Resolved retention semantics

The initial strict implementation only admitted session_started within 24 hours of exposure (trainer/discovery_data.py:476) and only attached strict intervals to those exact sessions (attribute_intervals, line 515). A qualified initial play followed by a verified interval in a newly created D7 chat therefore produced qualifiedNewPlay=1, activeSecondsFollowup=300, returnUtcDays=0, d7Continuation=0, and unattributed_interval=1. Continuing the original chat worked; returning to the same story through a new Library chat did not.

This narrowed the intended story-return metric to continuation of initially attributed chats. Extending receipt validity or merely relaxing the outcome timestamp check would not provide trustworthy linkage: server receipts expire after 24 hours. The controller instead specified later unique user/story attribution after the original confirmed new-play anchor, requiring a real verified sessionId, consumerEligible=true, original receivedAt, bounded windows, and ambiguity rejection. The server writes occurrence-time consumer eligibility, Herschel's strict exporter emits it, and Poincare implemented the matching logic. The superseded proposal to only annotate the narrower label is not the final agreed behavior.

Resolved: v3 interval attribution distinguishes confirmed-session attribution from later unique anchored story attribution. The original five-minute initial-play plus new-chat D7 reproduction now returns qualifiedNewPlay=1, d7Continuation=1, returnUtcDays=1. Repeating it with consumerEligible=false or null yields no strict D7 contribution. The change does not relax receipt validity or manufacture later session-start outcomes. Attribution mode counts, ambiguous joins and provenance rejections remain explicit.

Temporal and denominator checks that passed in the baseline

  • Features are copied from declared serving snapshots, require finite numeric values and an exact configured schema, and reject rankedAt/featuresAsOf from the future. No current-stat reconstruction occurs in the builder.
  • Full eight-day followup plus lateness is required even for early positives. Split gaps cover maturation; visits crossing boundaries and connected actor/story episodes across splits are excluded, including exposure chains in embargo gaps. Identity links can make exclusion stricter without rewriting guest knownness.
  • Newness requires explicit wasKnownAtServe=false and authoritative confirmation. Known/unknown stories remain represented but do not become eligible new-discovery negatives. Play/save intent and preview observations do not establish mutation success.
  • Interval IDs deduplicate; intervals are clipped and unioned before active seconds or UTC dates are counted. Midnight endpoints do not manufacture another active day. Strict receipt deadlines and source receipt aliases are respected.
  • Genuine empty page-only visits enter the visit denominator without synthetic candidates. Unavailable or incomplete page-backed visits remain visible but are excluded from that primary denominator. Positive-only NDCG is separately named; eligible all-zero slates are retained.
  • NDCG deliberately remains a diagnostic over observed eligible candidates, including candidates from incomplete visits. A reproduced two-card page with only one observed successful candidate returns diagnostic NDCG 1 while excluding its incomplete visit from the visit rate. This limitation is explicitly documented; it is not a hidden complete-page evaluation claim.
  • Test evaluation requires explicit opt-in; the builder does not fit, select or publish a model. The evaluator reports no causal uplift.

Source completeness and operational gates — separate from code defects

BuildConfig.as_of is a caller-attested complete-through watermark for both event and interval sources. The warehouse manifest deliberately reports sourceComplete=false, observationComplete=false and snapshotConsistent=false. A successful bounded export, a heartbeat, local wall-clock time or passing tests cannot discharge that gate. Do not use missing outcomes as mature negatives until retention/CDC/observation coverage has been established across exposures, followup, embargo gaps and relevant identity/episode history.

Translated attribution requires exposure-valid world/group history. The source history migration records observed membership, not reconstructed pre-baseline membership or a guaranteed WAL commit clock. The bounded source-history exporter now supplies a read-only repeatable-read snapshot and preserves microseconds and nullable groups. Its nullable languageGroupId means an observed ungrouped interval, distinct from missing history; the coordinated builder correction now accepts it. Both the export regressions and an independent null-to-group boundary probe pass. No group may be inferred from today's world rows. The exporter still reports historyComplete=false; observed history is not proof of missing historical capture. Erasure changes also require invalidating older local datasets and derived artifacts; the builder cannot discover them from previously exported files alone.

Candidate replay, retrieval recall, propensity-weighted policy comparison, randomization checks, uncertainty and promotion are not implemented by this descriptive evaluator. These are declared measurement/experiment requirements, not evidence that the current descriptive arithmetic is incorrect. Knownness/measurement exclusions must stay visible so changing source coverage is not misread as recommendation improvement.

Verification evidence and review boundary

  • Independently ran python -B -m unittest trainer.test_discovery_data -q: 69 passed before the assigned corrections.
  • Independently reran the corrected v3 suite: 86 passed in 29.933 seconds. Repeated the original integration probes plus negative-ID quarantine and future group membership checks; all passed. After Poincare's final bounded-scan index/query optimization, inspected the algebra-equivalent time predicate and reran all 86 tests, passing in 32.431 seconds against the final hashes below.
  • Ran additional temporary-directory probes using the builder API for canonical dismiss/Undo rejection, delivery field loss, a new-chat D7 return, partial-page evaluation and unequal actor visit counts. Results are recorded above. No probe modified repository Python files, fitted a model or contacted a service.
  • Independently ran node packages/server/scripts/test-local.mjs src/analytics/discovery-export.test.ts after the strict consumer flag change: 9 passed. This checks the mocked warehouse export contract, not live ClickHouse execution or source completeness.
  • Independently ran node packages/server/scripts/test-local.mjs src/analytics/discovery-dimensions-export.test.ts: 4 passed, including the actual history view in isolated PGlite, read-only snapshot boundaries, null groups and microsecond preservation. No real database connection was used.
  • At the controller's explicit request, corrected only the pre-existing reviewer's dismissal fixture by seeding its actual wrong-user actor account (someone-else) after erasure validation began rejecting nonexistent users. node packages/server/scripts/test-local.mjs src/routes/feed-dismiss-attribution.test.ts: 7 passed. This fixture maintenance is separate from independent Python review and is not presented as a review of the reviewer's own implementation.
  • Baseline SHA-256: discovery_data.py = cb84089c4190766777837ddf30941a354b592d3db150ca80f943837b6d2ceb76; discovery_evaluation.py = 074437d0372262fc3f5b0002ea3c24e96805680b564247bbf591615453883bbc; test_discovery_data.py = 18bda72091d39ec92b3973b92ca2f3831ae40fdbeb3aafcabdb1cd213605fea9.

Final reviewed SHA-256 values:

  • trainer/discovery_data.py: b1d85ca208778901156672b594ea1f3225e60499df7b9eef49d5c31a490c6122
  • trainer/discovery_evaluation.py: 95906393377218efa310060b81210dd822d827582a9f3e0c95d7838663e92699
  • trainer/test_discovery_data.py: fc634f994efdc97ca49faf6435b64314d5826a28bb50d1b0bf6f71bc57c7d49b

No production access, browser, deployment, staging, commit or push by this worker. Python changes were authored by Poincare, not this reviewer. A subsequent controller-requested client correction was performed separately and is documented in 2026-09-19-discovery-client-attribution.md; it is not presented as independent review evidence here. Shared worktree changes by other workers continue, so this verdict is tied to the hashes above and does not replace full-worktree/OSS release verification.