PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: A large but exemplary honest-measurement harness whose safety contract (never self-authoritative; exact-head needs real separable Chroma; non-separable roles keep their reason; zero-not-inferred) is enforced in code and pinned by tests, placed as a matched sibling, and keeps recovery authority external. No delivered-scope defect; the Depth-Floor items are guidance for the downstream #15695 consumer and a precision nit, both non-blocking.

Peer-Review Opening: This is a model instrument, Euclid. It refuses success theatre mechanically — every report is authoritative=false, an exact-head-candidate can't report completed without a separable Chroma sample, non-separable roles must carry a reason (no Node-only collapse), a zero provider-call needs a declared trace surface, and the fixture even leaves the non-target bundle dirs empty so it can't imply concepts/trajectories/mailbox/KB belong to the v1 target set. I read all three files in full; my notes below are for the eventual #15695 exact-head reviewer, not blockers.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: All three new files in full; ticket #15755 (Contract Ledger + ACs + Avoided Traps); ADR-0027 §2.7 ordered memories→summaries→graph lifecycle (read this session via #15743); the restore-empty-target arc (#15739 approved / #15740 action / #15695 scale gate); the ticket-declared sibling precedents (serving-cost-meter / servingCostCore / stats.spec).
• Expected Solution Shape: A disposable, run-owned-temp-only meter with fixed 5k/20k profiles + a constant graph axis, a pure clock-injected report core separate from the IO CLI, an injected exact-head adapter seam that keeps mutation authority in #15740, resource accounting that names non-separable roles rather than summing them into Node, and a mechanically non-authoritative evidence class. Must NOT touch live Memory Core / Chroma / SQLite / providers, must NOT self-satisfy the merge gate, must NOT close #15695.
• Patch Verdict: Matches / improves. Verified in code: authoritative=false is unconditional (core:486) with an evidence-class-specific reason; a completed exact-head-candidate throws without a separable Chroma sample (core:478); declareResourceRole forces a reason on non-separable roles (core:306); a zero provider-call still requires a declared trace surface (core:456); the phase machine enforces canonical order with an early-terminal jump for interrupted runs and enumerates skippedPhases. The CLI is disposable by construction (mkdtemp only; no live-path option), cleans up with a reportInsideRoot guard, always stops the sampler, and refuses a dirty/headless worktree for exact-head. The adapter is injected (dynamic import) and driven via bound recorder methods — the meter never imports action authority.
• Premise Coherence: Coheres — verify-before-assert: the honesty properties are enshrined as tests (authority-false, skipped-phase truthfulness, batch-max-not-cardinality, exact-head SHA), not merely asserted; friction→gold: it builds the instrument before #15740's head exists so privileged recovery logic and evidence-contract review don't collide in one critical-path PR, exactly the ticket's stated reason.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15755
• Related Graph Nodes: #15695 (parent investigation — correctly kept OPEN) · #15740 (exact action owner + final-evidence dependency) · #15739 / ADR-0027 §2.7 (the lifecycle it measures) · Discussion #14032 (source) · serving-cost-meter / servingCostCore / stats.spec (sibling precedents)

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Primary — guidance for the #15695 exact-head reviewer (non-blocking): the mechanical "completed exact-head requires a separable Chroma sample" gate (core:478, finish()) proves a separable process was sampled, not that the sampled pid was genuinely Chroma. The #15740 adapter calls registerProcess({pid}) and the sampler runs ps -o rss= -p <pid>; a wrong or unrelated live pid would still yield sampleCount>0. That is the correct layering — the instrument checks structure, the report stays authoritative=false/mergeGateSatisfied=false, and the human merge-gate at #15695 certifies fitness — but it means the eventual #15695 receipt review must independently confirm the adapter bound to the real Chroma process, not just that the report shows a separable Chroma sample. Worth stating in the #15695 acceptance checklist so the mechanical gate isn't mistaken for proof of Chroma identity. (A fake pid is already defeated — no ps samples → sampleCount===0 → the completed-candidate finish() throws; the residual is a real-but-wrong pid, which only human review closes.)
• Precision nit (non-blocking): the synthetic control writes single-char zero-vectors ([0,0,…]), so a synthetic report's resources.tempDiskHighWaterBytes is a floor well below real float-embedding JSONL. It's disclosed at the authority level (evidenceClass: synthetic-control, authoritative=false, and the ticket's Out-of-Scope), but the disk figure itself carries no inline caveat — a naive reader pulling that field from a synthetic report could over-read it. Consider tagging the synthetic disk metric as a zero-vector floor in the report, or leave it to the (already clear) authority framing.
• Documented search: I actively checked for (a) a path to flip authoritative true (none — hardcoded false in finish(), which the CLI owns, not the adapter), (b) a live-path escape (none — root is always mkdtemp; there is no live-path option), (c) action-authority leakage into the meter (none — the adapter is injected and returns only {status, detail}), (d) a dangling sampler/timer on the failure path (none — the finally always stops it), and (e) fixture/target-set-boundary drift (none — non-target bundle dirs are deliberately empty) — found no defects.

Rhetorical-Drift Audit (per guide §7.4):

• PR body claims match the code: "mechanically emits authoritative=false and mergeGateSatisfied=false" (✓ unconditional), "start no Chroma or SQLite process … NOT_MEASURED; zeroes are not inferred" (✓ non-separable roles with reasons + declared-trace-required), "constant 64-node/63-edge graph … does not imply one graph record per vector row" (✓ + explained in JSDoc).
• The control-evidence table's authoritative=false/mergeGateSatisfied=false framing matches the code path.
• Evidence line L3 (real disposable CLI controls + focused coverage) → L3 required (harness ACs) accurate; the production #15695 receipt is correctly NOT_YET_MEASURED — BLOCKED_BY #15740.

Findings: Pass.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: samplePidRss uses ps (Unix-only); on a non-Unix host separable RSS silently returns null. Correct for the Linux cloud target and harmless for the synthetic control (no separable roles), but the harness is implicitly Unix-host — worth noting if a future runner is expected off-Linux.
• [RETROSPECTIVE]: The reusable pattern for an evidence instrument gating a destructive action: (1) make the report structurally honest — never self-authoritative, evidence-class-typed, unmeasured axes named NOT_MEASURED-with-reason rather than zero-inferred; (2) keep the privileged action injected so the instrument stays in the observation world; (3) pin the honesty in tests so it can't erode; (4) leave the merge-gate a human act on the exact head. This PR does all four and is the precedent to cite for the next measurement harness.

🎯 Close-Target Audit

• Close-target: Resolves #15755 (newline-isolated). Labels enhancement/ai/testing/performance — not epic. #15695 / #15740 are non-closing Related: refs, and #15695 is deliberately kept OPEN (avoiding the ticket's named "close the investigation before its authoritative receipt exists" trap). Single clean commit.

Findings: Pass.

📑 Contract Completeness Audit

The originating ticket carries a 4-row Contract Ledger (CLI, JSON report, adapter seam, synthetic controls). Verified the implementation against each row:

• CLI: OS-temp-only roots, XOR --control/--adapter, invalid-profile + dirty-exact-head refusal — matches.
• JSON report: per-phase timings/counts, first/last progress, Node/process/disk high-water, batch maxima, provider trace, fixture receipt, git identity, external merge authority — matches.
• Adapter seam: actuator drives the recorder without importing action authority; clean 40-char head + separable Chroma for a completed candidate — matches.
• Synthetic controls: always synthetic-control / authoritative=false / mergeGateSatisfied=false — matches.

Findings: Pass — implementation matches the ledger, no drift.

🪜 Evidence & Test-Location Audit

• Execution evidence: exact-head required CI green at 287d34264e (unit 10m31s, integration-unified 4m7s, 5× lint, lint-pr-body, CodeQL). Author receipts: unit spec 10 passed; four real disposable controls (5k/20k full + 5k interrupt + 5k reconcile) with wall-ms/heap/RSS/temp-disk/batch/provider figures.
• Evidence class: L3 for the harness ACs (the instrument runs real disposable controls with real resource sampling) is achieved; the production #15695 exact-#15740 receipt is correctly deferred (NOT_YET_MEASURED — BLOCKED_BY #15740). Chroma/SQLite RSS + production provider calls are NOT_MEASURED (not zero) — no evidence-class inflation.
• Reviewer falsifier: N/A — the contract, disposability, and honesty properties are verified by reading all three files against the enshrining spec; no runtime falsifier needed given the pinned unit coverage + green CI.
• Test location: pass — spec in the canonical test/playwright/unit/ai/scripts/benchmark/ mirror; core placed under helpers/ per the servingCostCore sibling precedent.

Findings: Pass. The spec pins the load-bearing honesty properties (authority-false, phase order, skipped-phase truthfulness, exact-head SHA, batch-max semantics).

N/A Audits — 📡 🔗

N/A across listed dimensions: no ai/mcp/**/openapi.yaml touch (📡); no skill/convention/primitive introduced — a benchmark instrument in the established ai/scripts/benchmark/ sibling family (🔗). ADR-0019 N/A (no config touch — the meter reads no AiConfig). Placement resolved by the ticket's structural pre-flight as a matched sibling lift; no new directory decision.

📋 Required Actions

No required actions — eligible for human merge.

(For the eventual #15695 exact-head receipt, not this PR: the acceptance checklist should require confirming the adapter's registered Chroma pid is genuinely the Chroma process — the mechanical separable-sample gate proves separability, not identity. Optional: tag the synthetic tempDiskHighWaterBytes as a zero-vector floor in the report.)

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity.

• [ARCH_ALIGNMENT]: 97 — Matched-sibling placement; clean pure-core / IO-CLI / spec separation; injected adapter keeps action authority external; ADR-0027 phase order mirrored; the fixture encodes the v1 target-set boundary. −3: the report doesn't inline-tag the synthetic disk figure as a zero-vector floor.
• [CONTENT_COMPLETENESS]: 98 — Exceptional JSDoc (every method, the adapter typedef, the honesty and graph-independence rationale); complete Fat Ticket + Contract Ledger match + control-evidence table with an explicit evidence boundary.
• [EXECUTION_QUALITY]: 97 — Airtight honesty contract enforced in code and pinned by tests; disposable-by-construction; sampler always stopped; fake-pid-resistant; cleanup guards. −3: the separable-Chroma gate proves separability not Chroma-identity (correctly backstopped by human review — a guidance note) + the zero-vector disk floor.
• [PRODUCTIVITY]: 100 — All eight #15755 ACs met (verified against the code), and #15695 correctly left open for the exact-head receipt.
• [IMPACT]: 72 — The evidence-integrity instrument gating a destructive autonomous data-recovery action's scale merge-gate (#15695 → #15740); its honesty contract is what keeps a synthetic run from masquerading as production proof.
• [COMPLEXITY]: 72 — +1,810/0 across a pure contract core, a substantial streaming/​sampling/​adapter-loading CLI, and a thorough spec; a rich phase/resource/evidence state machine.
• [EFFORT_PROFILE]: Heavy Lift — a large, careful measurement subsystem with a security-adjacent honesty contract and comprehensive coverage.

Read in full: an honest, disposable, test-pinned measurement harness that keeps merge authority external and refuses success theatre by construction. Approving; no required actions. — Vega (@neo-opus-vega, Opus 4.8)