Independent discovery client review — 2026-09-19

Reviewed the current uncommitted client slice on codex/discovery-measurement in C:/Users/jefra/.codex/worktrees/yumina-discovery. Client implementation and tests were read-only. No browser, external account, production access, commit or deployment. The separately requested urgent erasure correction is covered in 2026-09-19-discovery-erasure.md.

Re-review verdict: all three client items are addressed in Dewey's follow-up. The two original independent reproductions now pass, as do all 39 client tests. The guest greeting-preview conversion now consumes and attaches its exact-target handoff. No client fixes were applied by this reviewer. The original evidence below is retained as regression context; it does not describe current open items.

Final review frozen: re-read Dewey's final beacon, observer and guest-start implementations and regression tests after his completion handoff. All three items remain closed. The assigned store/admission/resume and ranking/delivery snapshot review is complete, with no remaining concrete blocker found in that scope. Independent evidence is 39 passing client tests, 116 passing server tests and one unavailable-local-Redis skip; app and server typechecks both pass. Only this review document was edited during the final read-only review.

Original findings — now addressed

P2 — Retry only occurrences whose receiving path is idempotent (confidence: 10/10)

packages/app/src/lib/feed-beacon.ts:96–97 retains every retryable event:

const retained = events.filter(item => {
  if (retry && item.attempts < MAX_ATTEMPTS && Date.now() - item.createdAt <= MAX_AGE_MS) return true;

That includes tokenless legacy impressions/clicks/play from other surfaces and measurement-disabled feeds. The server legacy parser discards eventId and occurredAt, attaches the request's current userId, and performs an ordinary db.insert(feedEvents).values(rows) at packages/server/src/routes/feed-events.ts:124. The feed_events table (db/schema.ts:3029) has only a generated primary key; there is no occurrence-ID uniqueness constraint.

Reproduction used the real beacon module, fake timers, and the real legacy parser extracted in memory: queue one tokenless popular click; the first fake transport records the parsed row then throws to model a response lost after commit; change the request account from Alice to Bob; advance the four-second retry. Both request bodies are identical, but the receiving path produces two rows, one for Alice and one for Bob. Without an account change it still duplicates the row. Local stats report one queued, one sent, one retried. This adds duplicate feedback and can reassign the same occurrence after a cookie/account transition.

Bounded fix: retain retries for receipt-bearing canonical events only until the legacy receiver can deduplicate and preserve occurrence identity. A mixed batch must not requeue its tokenless members merely because its canonical members are retryable. Regression coverage should model a commit followed by lost response, including a changed account; a 503 returned before insertion does not exercise it.

P2 — A batched visibility dip does not reset the continuous dwell timer (confidence: 10/10)

packages/app/src/features/hub/hub-card.tsx:143–146 considers only the final entry:

const entry = entries[entries.length - 1];
// ...
const nextShare = entry.isIntersecting && entry.intersectionRatio >= IMPRESSION_VIEW_SHARE;
if (nextShare === visibleShare) return;

If the card is already above 50%, a delivery containing a below-threshold entry followed by an above-threshold entry leaves visibleShare unchanged. The pending timer survives, even though the required 500ms of uninterrupted exposure ended.

Reproduction mounted the actual HubCard in jsdom with a controlled observer and fake timers: deliver 0.8; advance 450ms; deliver [0.2, 0.8]; advance 50ms. It emits an impression after only 50ms of the new visible interval. This is relevant to rapid scroll/resize deliveries on mobile. The existing test covers a batch ending below threshold, but not a dip-and-return batch.

Bounded fix: process transitions in delivery order, resetting on every dip and restarting for the final visible interval. Add the above regression. No threshold, new event type, or UI behavior change is needed.

Original controller integration gap — now addressed

At the initial review, the guest greeting-preview login path did not consume the new handoff. The modal stores it at packages/app/src/features/hub/world-preview-modal.tsx:486, and discovery-attribution.ts:62 exports consumeDiscoveryHandoff, but the automatic session hook still posts body: JSON.stringify({ worldId }) at packages/app/src/features/chat/use-guest-session-start.ts:27. The real greeting preview calls that hook when login completes (features/chat/world-preview.tsx:162). There is no production consumer of consumeDiscoveryHandoff in this snapshot.

Thus guest Discover → optional translated greeting preview → login created a session without its original discovery attribution. This is the existing handoff listed in Dewey's client-attribution document, not a newly discovered defect in the helper or a request to expand its scope. The controller should consume once for the exact target, retain the original-world snapshot across retries and StrictMode, and attach it to the existing POST. Keep this item open until that integration is verified; the helper/storage tests alone do not complete it. That integration has now been independently inspected and its regression passed; see the re-review below.

Other reviewed behavior

• Canonical retries preserve the serialized UUID, occurrence time, original page, slot and receipt. The server derives the actor from that receipt; a receipt for user A fails verification under user B instead of falling back to legacy data. Guest receipts require the signed guest cookie, including after login. Both feed fetches and event/mutation requests include credentials.
• Canonical ingestion and mutation preparation validate stored origins and tombstones. Expired, wrong-actor and missing-origin receipts omit attribution; a legitimate mutation may still succeed. Client freezing alone is not treated as a trust boundary. PostHog mirrors the occurrence ID and excludes the token.
• Append/dedup retains each card's original page token and absolute position. Preview and picker snapshot the original world; translating changes the mutation target, not the attribution world. Existing session selection and game session reuse produce no new-session POST. Greeting rendering adds no confirmed start event; the automatic guest conversion gap is described above.
• Foreground preview dwell excludes hidden time and survives StrictMode effect replay without duplicate closes. Normal hidden/visible impression transitions reset the timer. Hard shutdown/final-close loss is already documented as a best-effort transport limitation; no delivery guarantee was inferred.
• Dismiss and delayed Undo keep the same world and frozen receipt after another preview opens. Failed dismissal offers no Undo; failed Undo restores hidden state. A signed receipt authorizes attribution only: it does not bind the mutation to a historical account. Undo still uses current cookies, as before this slice. No new account-switch mutation guarantee is claimed here.

Verification

All 36 client slice/lifecycle/mobile tests passed, plus the existing guest-start test (37 total). App typecheck passed. The two independent defect reproductions ran entirely in memory using the real transformed modules; they did not modify source or test files. The retry reproduction models server commit/response loss at the transport boundary and uses the actual parser; it is not a browser/network or database race test.

pnpm --filter @yumina/app exec tsx --tsconfig tsconfig.app.json --test --test-timeout=120000 src/lib/feed-beacon.test.ts src/lib/feed-beacon-visibility.test.ts src/lib/discovery-attribution.test.ts src/lib/analytics.test.ts src/features/hub/hub-card.test.ts src/features/hub/world-preview-store.test.ts src/features/hub/world-preview-modal.test.ts src/features/hub/rec-tab.test.ts src/hooks/use-play-with-language.test.ts
pnpm --filter @yumina/app exec tsx --tsconfig tsconfig.app.json --test --test-timeout=120000 src/features/hub/preview-lifecycle.test.ts src/features/hub/world-preview-mobile-actions.test.ts src/features/chat/use-guest-session-start.test.ts
pnpm --filter @yumina/app typecheck

The independent erasure verification also passed 24 helper/serialization and 25 measurement/HTTP/dismissal/retention tests. Server typecheck reported unrelated TS18048 errors in discovery-store-admission.test.ts:50–52; it is not recorded as passing. jsdom covers controlled visibility and callbacks, not real mobile layout, browser cookie policy or unload delivery. Those were not browser-tested.

Independent client re-review

Reviewed the completed code changes in feed-beacon.ts, hub-card.tsx and use-guest-session-start.ts, plus their new tests. All three requested changes are implemented without changing the receipt or mutation contract:

1. Pending events retain a canonical/legacy distinction at occurrence. A failed mixed batch requeues only receipt-bearing members; tokenless members retire. The original independent lost-ack/account-switch probe now produces exactly one Alice row and no Bob retry, with the buffer empty. The new mixed-batch test also verifies the canonical member keeps its original occurrence.
2. The observer processes each delivered transition in order. The original [0.2, 0.8] probe after 450ms now emits nothing after another 50ms or 499ms; it emits once only at 500ms after the dip. Existing background/reset and opportunity-dedup behavior remains covered.
3. The guest-start effect consumes a handoff only once login is ready and for the exact target world. A ref retains the original exposed world/token across failed creation retries and StrictMode replay. An unrelated world cannot inherit that ref. A pending start cannot be duplicated by the Retry action; failure to navigate after successful creation retries the same resolved session ID instead of issuing another POST. Existing stale-route cancellation remains in place. The automatic greeting-preview hook's actual POST now includes discoveryAttribution alongside the selected target worldId.

The full client command set above, now including the guest-start suite, passes 39/39, and the app typecheck passes again. Both independent original-defect probes were rerun in memory and pass. The new handoff test uses the actual storage helper, hook, React StrictMode and controlled asynchronous requests. No browser was used.

Independent store, resume and snapshot review

At the controller's follow-up request, inspected Copernicus's final discovery-store.ts, discovery-admission.ts, discovery-session.ts, the final testing harness/fixture and related tests; independently inspected Epicurus's ranking and delivery instrumentation in recommendations.ts, engagement.ts, discovery-feed.ts and discovery-pagination.ts. Source remained read-only. No additional actionable defect was found in these reviewed changes.

• Recovery occurs only after actor/global admission rejection, not arbitrary Redis errors. The bounded lookup requires exact actor, scope, page size, unexpired matching indexes, a body and nonterminal metadata. Normal read, ownership/expiry validation and CAS remain authoritative after lookup. Recovery does not reserve capacity, extend TTL or evict another stream. Concurrent selections of the same revision reuse the committed page; cursor replay still has the existing three-page window.
• The Lua admission path uses Redis time and absolute expiry. Existing updates remain writable at capacity; ownership/scope/limit/expiry cannot change. The normal comparison uses metadata rather than decoding the old payload. Missing indexed accounting fails explicitly. Legacy adoption, mixed-writer drain, standalone/noeviction requirements and whole-payload serialization costs are documented limitations, not hidden capacity guarantees.
• Ranking capture preserves returned order and values with capture disabled or enabled; model prediction occurs once. Retention inputs, actual component scores, cold constraints/relaxation, listwise costs, exploration, source flags and stage positions are captured where computed. Catalog replenishment carries catalog provenance. Source rollup time remains distinct from rank time, with explicit unknown markers for old/missing statistics.
• Delivery records actual selection costs and hydrated creator/topic/group data once, preserving explicit null groups. Queued ranking inputs survive later profile/stat changes; committed delivery data survives replay. Current eligibility is still checked at delivery. The fixture and harness use the final 136 ranking / 19 delivery / 3 metadata shape. Their payload and memory observations are not concurrent production capacity evidence.

Independent isolated verification: 116 passed, zero failed, one skipped:

pnpm --filter @yumina/server test src/lib/discovery-store-admission.test.ts src/lib/discovery-store-redis.test.ts src/lib/discovery-session.test.ts src/lib/discovery-pagination.test.ts src/lib/discovery-feed.test.ts src/lib/recommendations.test.ts src/lib/engagement.test.ts
pnpm --filter @yumina/server test src/lib/discovery-store-scale.test.ts src/routes/discovery-feed.test.ts

The first run passed 111 and skipped its disposable local Redis test because redis-server was unavailable; the second passed five. This reviewer did not run the remote Redis harness. Copernicus's updated scale report attributes its final 8/8 real testing Redis result to the controller; it is separate evidence, not a claim of an independently observed remote run here. An interim server typecheck reported concurrent warehouse test errors in discovery-import.test.ts for pending completedSweepDurationMs / sweepCompletionAgeMs fields. The final pnpm --filter @yumina/server typecheck now passes with exit 0; neither those diagnostics nor the earlier store meta diagnostics remain. No activation or production sizing approval is implied by this scoped review.