Discovery measurement: independent server and warehouse re-review
Finalized 2026-09-19 in codex/discovery-measurement, after Herschel declared the importer frozen. This follows the original server review. Scope: controller fixes 1/3/4, durable known-story history, canonical page/receipt/outcome transactions, erasure, and the ready warehouse import/export/prune/migrations and source world-history/dimension export. No app, store/admission, or offline trainer re-review. Source was read-only; only review documentation was written. The controller's final integration suite is pending and remains outside this review's completed verification.
Verdict: the scoped code re-review passes after the fixes below; collection and new-model promotion must remain disabled pending the operational gates. The original history-removal, retry-group, stale-target and clock-skew erasure reproductions are fixed. The additional first-persistence group/newness regression and archive-wide import exclusion set are now addressed and independently checked. No further actionable defect was found in the ready edits reviewed here. This is not full-branch approval, an independent review of my own findings 2/5 implementations, or approval to enable the pipeline.
Open code findings in the independently reviewed scope: none. The activation prerequisites below are unverified operational conditions and documented limits, not unresolved C1/W1 findings. Own findings 2/5 remain explicitly outside the independent verdict.
Original findings
| Finding | Re-review result |
|---|---|
| 1. Deleted history / guest null becomes new | Original reproductions ADDRESSED. Triggers retain first-known world/group evidence after Library/session removal; pre-capture accounts with absent history remain unknown; guest null is not promoted. The added indexed world-key/current-sibling lookup also passes the save-standalone/remove/regroup regression. The separate first-persistence temporal-group regression below is now addressed too. |
| 2. Missing numeric ranking and delivery inputs | Implemented by this reviewer under subsequent authorization, not independently re-reviewed. The snapshot contract records 136 numeric ranking keys, 19 delivery keys, delivery metadata and rollup freshness/unknown semantics. Own five-suite verification: 89/89, scoped TypeScript diagnostics zero. |
| 3. Retry signs a fresh group | ADDRESSED for the original defect. First persistence takes deliveryMetadata.languageGroupId, preserving explicit null; retries sign canonical stored origin identity. Observation/outcome validation compares that immutable identity. A regrouped replay returns the original token. |
| 4. Outcome uses stale route lookup | ADDRESSED. Preparation rereads the target with FOR SHARE inside the mutation transaction and rejects draft/unlinked targets. The mutation, outcome and trigger-written first-known ledger roll back together. |
| 5. Catalog labeled popular | Implemented by this reviewer, not independently re-reviewed. Actual catalog-query provenance is now catalog, with a full-schema regression in the 89-test result above. |
C1. P2 — First canonical persistence can label an immutable known group as new — ADDRESSED
Current fix: immutable exposure CTE and ledger lookup, receipt identities passed to knownness, and unknown changed-group censoring.
Reproduced with real recordDiscoveryServe in isolated full-schema PGlite: an account with complete capture saves sibling A in G, then removes it, leaving durable group:G evidence. The offered B's delivery snapshot records G. Before its first canonical persistence, B is moved to H. Persistence correctly retains/signs G, but knownDiscoveryWorlds queries B's current H and writes wasKnownAtServe=false. An ensuing confirmed outcome treats the original known G opportunity as new. This is the interval between cursor/delivery commitment and the first database write, including a retry after a failed first persistence; canonical-row replay alone does not cover it.
Fix verified: the query now receives immutable receipt identities and uses exposure.language_group_id for group evidence. Positive knownness survives target regrouping; absent evidence with a changed target group yields null. The new regression and measurement/attribution suites pass 14/14 (discovery-frozen-group-rereview-tests.log). This resolves the exact first-persistence reproduction. It is not full historical reconstruction of other siblings' membership: the stable-group check compares the delivered world's current group only. The controller has explicitly retained that temporal-history limitation; strict historical family proof needs reconciled source history, not this stability flag alone.
W1. P1 — Global archive exclusion makes event import stop as retained history grows — ADDRESSED
The initially reviewed importer used id NOT IN (SELECT id FROM discovery_events FINAL) in every raw 250-row page. That independent subquery has no ID or time restriction. The outer LIMIT 250 bounds output, not archive-set construction. The 64 MiB set / 10-million-read-row / 256 MiB query limits eventually fail every repeated import against the same growing 180-day archive. The set can hit its cap before the row cap; an exact threshold depends on engine allocation and was not measured. If events cannot archive, exact-ID pruning also cannot remove their operational source rows.
This is a SQL-shape finding, not a claimed ClickHouse EXPLAIN or live performance result. Failing within a resource budget does not demonstrate sustained ingestion capacity.
Fix independently verified: raw keyset selection reads at most 250 IDs per page, including expired/deleted rows so they cannot prevent cursor progress. Archive reconciliation restricts membership to that page's eligible IDs. Checkpointing follows archive acknowledgment, uses monotonically increasing stored versions and resets after EOF. Failed/lost archive or checkpoint acknowledgment repeats safely or resumes acknowledged work; a late ID behind the cursor is revisited on a later sweep. Receipt time is eligibility, not a permanent high-water exclusion. Independent import/export tests pass 30/30, including those failure modes and the targeted capacity test.
The million-row virtual-capacity test drives the actual importer over synthetic bounded pages, including already archived IDs. With the current 2,500-row run budget and simulated one-minute cadence, it needs 401 invocations including EOF: completion at minute 400; the newly inserted behind-cursor ID is reached at minute 401. completedSweepDurationMs remains 400 minutes after wrap, even though current sweep age resets to zero. This is a deterministic scheduling/algorithm test, not a ClickHouse throughput benchmark.
Remaining activation gates — separate from code findings
| Gate | Evidence required before activation |
|---|---|
| Archive lateness and capacity | Controller-declared maximum lateness, measured retained raw volume/cadence/CDC delay, and a passing full-sweep capacity check. The million-row simulation does not establish an acceptable production latency budget. |
| Actual ClickHouse execution | Disposable-engine migration, FINAL/dedup/TTL and erasure checks; EXPLAIN and representative load evidence for selective raw/archive ID queries. No live ClickHouse execution occurred in this review. |
| CDC preparation and reconciliation | Explicit migration/mapping readiness for the nine interval columns, exact event/erasure reconciliation, and recorded coverage gaps. Publication tests and heartbeats do not prove source completeness. |
| Erasure-safe consumption | Current erasure imports, consumer access through filtered views, and invalidation of previously exported/derived artifacts after later erasures. Logical filtering does not immediately remove physical archive bytes. |
| Training/promotion validity | Independently established event/interval coverage and any required temporal family evidence; unknown history and observation-time/commit-time gaps remain explicit. Export success is not a complete-through watermark. |
The controller's final integration/build/release checks are a separate pending release-verification step. This report does not record them as passed. None of these gates requires stopping catch-up imports merely because their data is stale; collection and new-model promotion stay disabled until the applicable gates are satisfied.
Hard activation gate: the controller must declare the maximum acceptable end-to-end lateness, measure expected retained raw volume and actual cadence/CDC lag, and show they fit that budget before enabling collection or promoting new models. Include all raw FINAL rows, including archived, expired and deleted records. Evaluate completed sweep duration, current sweep age and age since completion; also reject stale/missing observations and account for external CDC delay, worker/query delay and the 30-second receipt guard. Neither sampled observedReceiptLagMs, a recent checkpoint, nor a recent heartbeat proves capture freshness. Failed latency gates must keep data consumption/promotion blocked while catch-up imports continue. No numeric production latency budget or successful real-engine capacity result was supplied in this review.
The final warehouse runbook gate now documents these conditions. This is an operational approval gate, not a runtime assertion enforced by the export CLI; the CLI remains an explicitly incomplete observed-data export. The importer was declared frozen after these code/test changes and the runbook update.
The new SQL removes the unbounded archive membership set and is compatible with the archive's ORDER BY id; it does not prove that ClickHouse FINAL, raw-table ordering, part fragmentation and optimizer pushdown keep physical reads within the caps. Inspect EXPLAIN and run a representative disposable-engine capacity test before activation. Raising caps alone is not the fix.
The separate pending-erasure reconciliation still joins the full permanent marker table. Its cutoff comparison is consistent with monotonic source cutoffs and the destination's cutoff-based replacement version. It remains a deletion-cardinality scale ceiling, rather than another event-volume exclusion set; no claim of page-bounded work is made for it.
Independently verified behavior and operational limits
- Clock-skew erasure addressed: the real client parser/ingester accepts an authenticated click at
now + 4,500ms; permanent user erasure plus user deletion now removes it. Source filtering permanently rejects erased user associations; source deletion removes guest references when occurrence or receipt is at/before cutoff. The warehouse view applies the matching five-reference predicate. Actual later guest visits survive only when both clocks are later. Permanent user semantics are derived from the actor prefix; no third tombstone field is required by this implementation. - Canonical transactions: empty/unavailable pages persist a canonical page record without a receipt. Invalid numeric snapshots roll back the page and opportunities. Internal serving transactions and mutation transactions passed separate full-schema probes. Account deletion invokes erasure before deleting the account even when measurement collection is off. Actor/user-row lock order and both serialized schedules are tested; PGlite does not establish real multi-backend PostgreSQL concurrency performance.
- Exports: the event/interval export preserves canonical payloads and timestamps, requires actual session/receipt fields and strict
consumer_eligible=true, excludes erased users, and reports overlapping exclusion counters without promoting them to completeness. Pending markers and before/after marker-generation changes abort output; exclusive files, size/row budgets and partial cleanup are tested. The observed CDC snapshot remains explicitly inconsistent/incomplete. Unseen WAL erasures and later deletions require reconciliation/revocation of prior artifacts; exported intervals are live retained input, not a 180-day interval archive. - Pruning: candidate and archive intersection uses at most 1,000 exact IDs per batch, defaults to dry-run, rechecks age/type on deletion and preserves identity links/markers. Warehouse failures cannot authorize source deletion. Receipts older than the 179-day reconciliation window are deliberately retained, so an operational backlog is not automatically a bounded 30-day source store.
- Migrations: only explicit flags apply ClickHouse DDL; migration 008 is in the build copy list. The source publication change is additive and repeatable, recognizes the exact six/eight/nine interval-column lists, preserves unrelated members, and rejects broad/different lists. Actual ClickPipe destination mapping of nullable
session_id,received_at,consumer_eligibleremains a manual activation prerequisite, not something the publication test proves. - World history/dimensions: capture and bounded baseline preserve nulls, do not backdate initial knowledge, serialize through world-row locks and roll back with mutations. Deletion observations close intervals before being excluded. Dimension export uses a dedicated repeatable-read/read-only connection, exact microseconds and a C-collation composite keyset. Capture timestamps are observation times inside transactions, not commit times; unknown pre-capture and late-commit boundary gaps remain explicitly unproven. No independent defect beyond these documented limits was found in the reviewed history/export slice.
Verification
- Independently executed controller-owned receipt/measurement/migration/erasure/attribution suites: 28/28 at the earlier re-review point, including the corrected 413 body-limit test.
- The new standalone-to-group history regression was separately rerun with its measurement suite: 8/8.
- Before the cyclic importer replacement, warehouse config/import/export/prune/world-history/dimension-export plus source erasure/serialization suites: 64/64, log
discovery-warehouse-rereview-tests.log. - After the cyclic importer replacement: independent import/export re-run 30/30, including the million-row scheduling/capacity regression, log
discovery-cyclic-import-rereview-tests.log. These overlap the earlier suites and must not be summed as unique tests. - Additional credential-stripped, dotenv-disabled, in-memory full-schema probes verified page rollback, original-null group replay, stale target rejection, mutation/outcome/ledger rollback, and the fixed accepted-clock-skew erasure. C1 was reproduced there before the controller fix; its subsequent regression and focused suites pass 14/14.
- Separately reported by the owners at finalization: Herschel's frozen warehouse suite 48/48 plus scoped TypeScript pass; controller's frozen-group knownness suite 9/9. These are owner-reported results, not additional independent executions or unique-test totals. The controller is running the final integration suite; its result is not yet included.
- No ClickHouse runtime or local Docker was available. Local predicate shims and transport fakes do not validate ClickHouse FINAL/TTL engines, query plans, replica visibility, optimizer pushdown or production-sized capacity. Disposable-engine verification remains required before activation. No production DDL, browser, global build/OSS job, source edits, commits, pushes or spawned agents were used for this re-review.
