Skip to content

feat(experiment): the evidence registry — one home for measured claims - #644

Merged
drewstone merged 3 commits into
mainfrom
feat/evidence-registry
Aug 19, 2026
Merged

feat(experiment): the evidence registry — one home for measured claims#644
drewstone merged 3 commits into
mainfrom
feat/evidence-registry

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Problem

Measured results rot in scatter: 510 .evolve/ files, 31 results directories, 159 docs with load-bearing numbers, and results that exist ONLY in PR comments (the VB parity verdict) or secret gists (prime-vs-dspy). Prose restates numbers, drifts, and nobody can tell a certified lift from folklore.

Change

One canonical registry in the measurement substrate, because agent-eval owns evidence legitimacy.

  • Recordevidence/records/<id>.json, validated by evidenceRegistryRecordSchema (exported from ./experiment). Typed fields: claim (stated so it can fail), instrument, exact command (null = named gap), arms, denominator n, result with uncertainty, evidence state (CERTIFIED / MEASURED-ONCE / RESOLVED-NULL / UNVERIFIED / KILLED), artifacts (never empty), cost (null = not captured, never a silent zero), confounds, source repo, and the sealed experiment digest when one governed the run (feat: make experiments evidence-addressable and preflightable #622's addressing scheme).
  • Rendererpnpm run evidence:render generates evidence/INDEX.md from the records. Deterministic: no clocks, no environment.
  • Freshness gatepnpm run evidence:check runs inside verify:package: an invalid record, a filename/id mismatch, or a stale index fails the same local gate that guards releases. The suite also re-renders and byte-compares.
  • Docevidence/README.md is the one-page "where evidence lives"; other repos keep at most a pointer file and delete their duplicated results prose.

Initial migration (7 records)

record state headline
multishot-golden-oracle-v1 CERTIFIED tax 4/4 deep-equal, gtm 4/4 + 8/8; mutation 14/14 records, 8/8 engine mutants
ctb-analyst-gepa-certified-prompt CERTIFIED stock 0.2489 vs G2 0.1928 pooled sealed micro-F1; defended twice
tb-gated-stop-ab-confirm MEASURED-ONCE +0.0596, 95% CI [-0.0061, +0.1210], n=151 rows / 22 clusters
tb-repair-free-lunch MEASURED-ONCE 3/64 rollouts (4.7%), 2/32 rows; control 0/96
prime-vs-dspy-analyst-38-rows MEASURED-ONCE F1 0.3218 vs 0.1833, n=38, sign test p=0.0043 (was gist-only)
cad-gepa-openscad-directive MEASURED-ONCE +9.5pp held-out (weakest record by design: command not preserved, named as a gap)
vb-cell-engine-parity-no-flip RESOLVED-NULL no flip; 3 parity dimensions hold, 3 fail (was PR-comment-only, blueprint-agent#2330)

Proof

  • pnpm typecheck, pnpm typecheck:scripts, pnpm lint — clean
  • pnpm test — 386 files passed / 2 skipped, 5389 tests passed / 3 skipped, 0 failed
  • pnpm build + pnpm verify:package — exit 0, ends with evidence index is fresh: 7 records, index matches
  • Version 0.149.0 (0.148.0 was released from main mid-flight; merged forward, digest repinned)

tangletools
tangletools previously approved these changes Aug 18, 2026

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 8b8dc027

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.

tangletools · auto-approval · reason: drewstone_author · 2026-08-18T20:27:36Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Value Audit — sound-with-nits

Verdict sound-with-nits
Coverage 1 of 2 lenses (usefulness)
Concerns 4 (2 low, 2 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 438.4s (2 bridge agents)
Total 438.4s

⚠️ Partial audit — the verdict covers only usefulness. value: agent returned no usable verdict. Treat the missing lens as unexamined, not as clear.

💰 Value — error

value agent produced no parseable value-audit JSON.

  • Model: opencode/deepseek/deepseek-v4-pro
  • Bridge attempts: 8
  • Bridge error: opencode/kimi-for-coding/k2p7: opencode: opencode error; opencode/zai-coding-plan/glm-5.2: opencode: opencode error; opencode/deepseek/deepseek-v4-pro: opencode: opencode error

🎯 Usefulness — sound-with-nits

A coherent, well-gated evidence registry that extends the existing experiment-seal and .evolve vocabulary rather than competing with it, wired into the same release gate as every other repo check — verified passing in this checkout.

  • Integration: Fully reachable and enforced. pnpm run evidence:check is appended to verify:package (package.json:183), which runs in both .github/workflows/ci.yml:58 and .github/workflows/publish.yml:49, so an invalid record or stale index fails CI and the release gate. The schema, renderer, and validator are exported from the public ./experiment subpath (src/experiment/index.ts:202-214) so consumer re
  • Fit with existing patterns: Fits the codebase's grain precisely. The gate-in-verify:package pattern exactly mirrors check:skill, check:model-ids, and check:analyst-benchmark (package.json:180-183). It deliberately does NOT compete with the two adjacent 'evidence' mechanisms: evidence-receipt.ts is the per-run Runtime↔Eval attested join (src/experiment/evidence-receipt.ts:1-10) while this registry is the curated, huma
  • Real-world viability: Holds up beyond the happy path. The renderer is deterministic (no clocks/env; asserted by test at evidence-record.test.ts:86-100), strict zod strictObject rejects unknown keys, empty artifacts, unknown states (tested at :50-58), duplicate ids and dangling supersedes are caught at the registry level (:61-83), and markdown table injection is handled via mdEscape (evidence-record.ts:142-144). The
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 4

🔎 Heuristic Signals

🟡 Cruft: console debug added scripts/render-evidence-index.ts

  • console.log(evidence index is fresh: ${files.length} records, index matches)

🟡 Cruft: todo added evidence/records/vb-cell-engine-parity-no-flip.json

  • "notes": "Two structural gaps block the flip and are named in the engine header: TODO(graph-cell/bridge-turn-depth) and TODO(graph-cell/fixed-budget-adherence). The flip is one line once both close

🎯 Usefulness Audit

🟡 README names the wrong export for the schema [ergonomics] ``

evidence/README.md:9 tells external repos to validate with evidenceRecordSchema from @tangle-network/agent-eval/experiment, but the exported name is evidenceRegistryRecordSchema (src/experiment/index.ts:210). A consumer copying the doc line gets an undefined import. One-line doc fix.

🟡 Denominator schema admits zero and negative n [robustness] ``

n.value is z.number().finite() with no .positive() (src/experiment/evidence-record.ts:54) even though the field doc says 'a claim without an n is prose' — a record with n=0 or negative would pass the gate. Low risk in a hand-curated registry; tightening to .positive() is a one-word change.


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260818T203553Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 8b8dc027

Review health 100/100 · Reviewer score 29/100 · Confidence 95/100 · 15 findings (3 medium, 12 low)

glm: Correctness 29 · Security 29 · Testing 29 · Architecture 29

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 8/8 planned shots over 22 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM CERTIFIED record's registered rule points at a file that does not exist — evidence/records/ctb-analyst-gepa-certified-prompt.json

The command (line 7) and artifacts (line 18) cite '.evolve/certification-2026-08-02-preregistration.md' for the registered rule. That file does not exist in this repo (.evolve/ contains no certification*.md) and no prereg file exists under ~/bench-cache/ctb-20260801/. This is the registry's only CERTIFIED record and its rejection logic ('G2 REJECTED by rule 1') is not derivable from the quoted numbers — both per-family paired CIs ([-0.126, 0.058] OH, [-0.143, 0.023] T2) cross zero and no

🟠 MEDIUM Affirmative claim + MEASURED-ONCE for a confirmatory draw whose CI crosses zero — evidence/records/tb-gated-stop-ab-confirm.json

The claim states gated stop 'finishes more rows than...unconditional continuation' as fact, but the record's own result is '+0.0596 delta, 95% CI [-0.0061, +0.1210]... not significant at the registered level' and the first confound repeats it. The registry's own ladder (evidence/README.md, src/experiment/evidence-record.ts:32) defines RESOLVED-NULL as 'the effect did not appear under the registered rule' — this draw was powered for 0.10, observed 0.06, interval crosses zero. INDEX.md line 13 renders the affirmative claim adjacent to MEASURED-ONCE in the front table, so a skimmer can cite '+6pp for gated stop' as established. Fix: reword the claim

🟠 MEDIUM validateEvidenceRegistry accepts self-supersede and cycles in supersedes — src/experiment/evidence-record.ts

The supersedes check only verifies target existence: if (!seen.has(target)) throw. Reproduced live: a record with supersedes: ['probe'] (its own id) validates, and a two-record cycle (aa supersedes bb, bb supersedes aa) also validates. The supersedes graph is the trust narrative ('KILLED — superseded by a stronger record'); a cycle makes two records each claim to defeat the other, corrupting the registry's core integrity guarantee. Fix: reject target === record.id, then detect cycles with a DFS over the supersedes edges and throw EvidenceRegistryError. Add tests for both cases.

🟡 LOW README cites non-existent export evidenceRecordSchema; actual symbol is evidenceRegistryRecordSchemaevidence/README.md

Line 9 says records are 'validated by the exported schema (evidenceRecordSchema from @tangle-network/agent-eval/experiment)'. Repo-wide grep shows evidenceRecordSchema exists nowhere; the exported schema is evidenceRegistryRecordSchema (src/experiment/evidence-record.ts:63, exported at src/experiment/index.ts:210). CHANGELOG.md:11 uses the correct name, so the registry's own README is the outlier. Impact: anyone copying the doc's import to validate a record gets an undefined/missing export. Fix: rename the mention to evidenceRegistryRecordSchema.

🟡 LOW MEASURED-ONCE for a number whose run directory is unpinned — evidence/records/cad-gepa-openscad-directive.json

command is null, the run directory is 'not pinned here yet' (notes), and the +9.5pp traces only to another repo's session record — by the ladder's own definition this sits at the MEASURED-ONCE/UNVERIFIED boundary and arguably belongs in UNVERIFIED until the run dir is pinned. The record self-discloses all of this ('Weakest record in the initial migration by design'), so this is a judgment-call nit, not an integrity gap.

🟡 LOW Mean-F1 denominator unstated for the 3 failed rows — evidence/records/prime-vs-dspy-analyst-38-rows.json

n=38 but the detail says 35/38 completed ok and a confound says 3/38 failed structured output; the result's 'mean per-case scored F1 0.3218' does not say whether the 3 failures were scored 0 or excluded (denominator 35 vs 38 changes the mean by up to ~2.5pp). The W/L/T counts (18+4+16=38, verified) and the sign-test p=0.0043 (recomputed exactly, ties dropped, n=22) are correct. Fix: state the scoring rule for failed rows in the result or n.detail.

🟡 LOW Claim asserts the parity its own result refutes — evidence/records/vb-cell-engine-parity-no-flip.json

The claim says the graph engine 'reaches behavioural parity... so the default engine can flip to the graph', while the result opens 'NO FLIP' and lists two parity failures. RESOLVED-NULL state flags the refutation in the index, but the affirmative 'so the default engine can flip' clause invites misquotation detached from the result column. Fix: word the claim as the tested hypothesis ('graph cell-engine was compared against the loop cell-engine for behavioural parity on a live cell to decide the default-engine flip').

🟡 LOW Byte-exact index check breaks on CRLF checkouts (no EOL pinning) — scripts/render-evidence-index.ts

The check compares readFileSync(indexPath) byte-for-byte against an LF-only rendered string, and the repo has no .gitattributes (git check-attr text/eol on evidence/INDEX.md returns unspecified). On a Windows checkout with core.autocrlf=true, INDEX.md is materialized with CRLF and evidence:check inside verify:package fails spuriously. Fix: add .gitattributes entry evidence/INDEX.md text eol=lf (and ideally the records too), or normalize line endings before comparing. Not blocking for an LF-defaulting team; latent friction for any Windows contributor.

🟡 LOW Missing evidence/records dir crashes with raw ENOENT stack instead of a clean message — scripts/render-evidence-index.ts

readdirSync(recordsDir) at line 22 is unwrapped. The empty-dir case is handled cleanly at lines 26-29, but a missing directory throws an uncaught ENOENT with a stack trace. CI semantics are unaffected (uncaught throw still exits 1), so this is message quality only. Fix: wrap in try/catch and print evidence records directory is missing: ${recordsDir} before process.exit(1), mirroring the missing-index handling at [lines 62-67](https://github.com/tangle-network/agent-eval/blob/8b8dc02760b46380109fe03f22515844dbf9682f/scripts/ren

🟡 LOW Committed-registry test uses __dirname in an ESM package and reads recordsDir at module scope — src/experiment/evidence-record.test.ts

Package is "type": "module"; __dirname works only because vitest's SSR transform shims it (passing today, proven). scripts/render-evidence-index.ts uses the correct fileURLToPath(new URL(...)) pattern — prefer that for consistency. Also readdirSync runs during describe evaluation, so a missing evidence/records crashes the whole suite at import with ENOENT instead of a clean failing test.

🟡 LOW Date validated by regex only — '2026-13-45' passes — src/experiment/evidence-record.ts

datePattern checks shape not calendar validity; month 13 day 45 accepted (reproduced). zod 4 (repo uses ^4.4.3) provides z.iso.date() which validates a real calendar date. Low impact: sorting still works lexicographically and records are human-reviewed.

🟡 LOW Denominator schema accepts negative n — src/experiment/evidence-record.ts

value: z.number().finite() accepts -5 (reproduced). The module's own doc says 'A claim without an n is prose, not evidence', yet a negative denominator validates and renders as '-5 rows' in INDEX.md. Fix: .nonnegative() or .positive() on value.

🟡 LOW Unreachable ?? 0 fallback would rank an unknown state as CERTIFIED — src/experiment/evidence-record.ts

stateRank.get(a.evidenceState) ?? 0 silently maps a missing state to rank 0 (CERTIFIED, the strongest). Unreachable in practice because zod validates the enum first, but it contradicts the repo's fail-loud doctrine; make it total via non-null assertion or an explicit throw.

🟡 LOW mdEscape lets markdown links and HTML through into INDEX.md — src/experiment/evidence-record.ts

Only pipes and newlines are escaped. A claim containing 'click x' renders verbatim into the committed INDEX.md (reproduced: rendered=true), and GitHub would render it as a link in the public index. Records are committed and PR-reviewed, so this is trusted input today; if records ever import claims from external sources, escape square brackets/angle brackets or strip link syntax.

🟡 LOW gepa-analyst-campaign.ts remains outside the typecheck:scripts include list — tsconfig.script.json

scripts/gepa-analyst-campaign.ts exists in the scripts/ directory but is not in this include list, so 'pnpm typecheck:scripts' never typechecks it. Pre-existing at base (99ed9b8) — not introduced by this diff — but since the include list is being edited here, adding it would close the gap. Fix: append "scripts/gepa-analyst-campaign.ts" to include, or confirm the omission is intentional (e.g., it is checked by another config).


tangletools · 2026-08-18T20:52:38Z · trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 15 non-blocking findings — 8b8dc027

Full multi-shot audit completed 8/8 planned shots over 22 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-08-18T20:52:38Z · immutable trace

@tangletools

Copy link
Copy Markdown
Contributor

Premise check withheld merge — 8b8dc027

Classifier flagged this PR as a premise claim (numeric pp/% delta + eval terminology). Confidence: medium.

Recommend re-running the underlying eval with pairedEvalueSequence before merging.

  • Cited claim: +9.5pp
  • PR body excerpt: feat(experiment): the evidence registry — one home for measured claims

Run:

pnpm eval:evolve --reps 5 --skip-mutation

Classifier rationale: Body cites 1 numeric claim(s) (+9.5pp) and eval-related terms appear in pr_body, review_findings. PR is asserting a measurable result that repair-pr cannot polish away — re-run the underlying evaluation before merging.


tangletools premise check · #644

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 9a1100c9

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.

tangletools · auto-approval · reason: drewstone_author · 2026-08-18T20:53:35Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Value Audit — sound-with-nits

Verdict sound-with-nits
Coverage 2 of 2 lenses (value, usefulness)
Concerns 5 (2 low, 3 weak-concern)
Heuristic 0.0s
Duplication 0.1s
Interrogation 184.4s (2 bridge agents)
Total 184.5s

💰 Value — sound-with-nits

Adds a curated, schema-validated evidence registry (7 seed records) with a deterministic renderer and a release-gating freshness check; it is a genuinely new layer in the substrate's grain, with only naming-adjacency and an unverified seal-digest field as nits.

  • What it does: Introduces evidence/records/<id>.json as the canonical machine-readable unit for a measured claim, validated by evidenceRegistryRecordSchema (src/experiment/evidence-record.ts:63-101) with typed fields for claim, instrument, command (null = named gap), arms, denominator n, result, a five-state trust ladder (CERTIFIED/MEASURED-ONCE/RESOLVED-NULL/UNVERIFIED/KILLED, lines 38-44), artifacts, cost
  • Goals it achieves: Gives every measured claim in the ecosystem one addressable, reviewable home with an explicit trust state, so numbers stop living only in PR comments (vb-parity), secret gists (prime-vs-dspy), or drifting prose. Achieves: (1) folklore becomes distinguishable from certified lifts by schema, not memory; (2) drift is mechanically caught — an invalid record, filename/id mismatch (scripts/render-eviden
  • Assessment: Good, and built in the grain of the codebase. The layering is correct: agent-eval is the declared measurement substrate (CLAUDE.md layering rule), and evidence legitimacy is a substrate primitive. The deterministic-render + byte-compare gate follows the existing verify:package pattern (check:analyst-benchmark, check:skill, check:model-ids all already chain there). The test suite parses the real co
  • Better / existing approach: none — this is the right approach. I searched for existing capability before concluding: (1) the #622 sealing system's EvidenceRecord (src/experiment/ast.ts:34) is a loose Record<string, unknown> funnel row, not a curated claim — the registry joins it via experimentDigest rather than duplicating it; (2) the 0.148.0 evidence receipt (src/experiment/evidence-receipt.ts) is a cryptographic per-
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound

A coherent, working evidence registry that extends the existing sealed-experiment vocabulary and release-gate pattern, verified live: the freshness gate passes, all 15 tests pass, and the initial migration rescues numbers that previously lived only in PR comments and gists.

  • Integration: Reachable through three wired paths, all verified: (1) public API — schema, validator, and renderer exported from the ./experiment subpath (src/experiment/index.ts:202-214); (2) npm scripts — evidence:render / evidence:check (package.json:184-185); (3) the release gate — evidence:check appended to verify:package (package.json:183) alongside the established check:skill / check:model-ids siblings. R
  • Fit with existing patterns: Fits the codebase grain precisely. It does not compete with the two nearby mechanisms: .evolve/experiments.jsonl is explicitly kept as the append-only diary with the registry as its curated distillation (evidence/README.md:14-15, evidence-record.ts:18-21), and evidence-receipt.ts remains the runtime identity binding while the registry is the durable claim store — the optional experimentDigest fiel
  • Real-world viability: Built for the non-happy path: strict zod object rejects unknown keys and bad ids/dates; null command and null cost are typed named gaps, not silent defaults (evidence-record.ts:74-91), matching the repo's fail-loud doctrine; the renderer is deterministic (no clocks/env), so the byte-diff check can't flake; cross-record validation catches duplicate ids and dangling supersedes (evidence-record.ts:11
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: console debug added scripts/render-evidence-index.ts

  • console.log(evidence index is fresh: ${files.length} records, index matches)

🟡 Cruft: todo added evidence/records/vb-cell-engine-parity-no-flip.json

  • "notes": "Two structural gaps block the flip and are named in the engine header: TODO(graph-cell/bridge-turn-depth) and TODO(graph-cell/fixed-budget-adherence). The flip is one line once both close

🎯 Usefulness Audit

🟡 Maintainer skill and CLAUDE.md doc map do not mention evidence/, weakening the main record-adding path [ergonomics] ``

grep for 'evidence:(render|check)' and 'evidence/records' across the repo: hits only in the new files, CHANGELOG, README.md:15, and docs/experiment.md:5 — none in .claude/skills/agent-eval/SKILL.md or CLAUDE.md, the surfaces fleet agents load to maintain this package. The registry's success depends on future measured claims landing as records; the workflow doc that agents actually follow doesn't yet tell them it exists. One pointer line in each (CLAUDE.md's doc list, the skill's maintainer workf

💰 Value Audit

🟡 Three 'evidence' concepts now export from one barrel with colliding names [maintenance] ``

src/experiment/index.ts:131 exports EvidenceRecord (the sealing system's Record<string, unknown> row, src/experiment/ast.ts:34) while index.ts:207 exports EvidenceRegistryRecord (this PR's strict curated schema), alongside the pre-existing evidence-receipt module. The names are functionally distinct but invite import-site confusion. Not blocking; a follow-up rename of the ast row type (e.g. EvidenceRow) would clean the adjacency.

🟡 experimentDigest is display-only — never verified against a real seal [better-architecture] ``

The schema only checks sha256 shape (src/experiment/evidence-record.ts:97) and the renderer only prints it (line 189). One record cites a digest (tb-gated-stop-ab-confirm.json) but nothing confirms a seal with that digest exists, so a typo'd or fabricated digest would pass the gate. Seals live outside this repo's check reach, so cross-repo verification may not be feasible at check time; still, the registry's premise is evidence legitimacy, so a verify-when-reachable hook would strengthen it. Doe


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260818T212126Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 9a1100c9

Review health 100/100 · Reviewer score 58/100 · Confidence 95/100 · 10 findings (1 medium, 9 low)

glm: Correctness 58 · Security 58 · Testing 58 · Architecture 58

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 8/8 planned shots over 22 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM supersedes cycles and self-supersede pass registry validation — src/experiment/evidence-record.ts

The supersedes loop only checks seen.has(target) — it never rejects target === record.id nor detects cycles. Probe-verified: validateEvidenceRegistry([{...valid, id:'a', supersedes:['a']}]) and a mutual 2-cycle (a supersedes b, b supersedes a) both validate and render into INDEX.md. A cyclic supersede chain makes record provenance ambiguous and defeats the trust ladder the module exists to enforce (KILLED records point at replacements). Fix: reject self-reference (target === record.id) and run a cycle check over the supersedes graph before returning.

🟡 LOW Version string duplicated between pyproject.toml and init.py fallback — clients/python/src/agent_eval_rpc/__init__.py

The PackageNotFoundError fallback hardcodes "0.149.0", duplicating pyproject.toml line 7. The two are kept in sync only by manual discipline; a future bump that edits one but not the other would silently report a stale version when the package is imported unbuilt. Pre-existing pattern, correctly synced in this PR. Optional fix: derive the fallback from a single source (e.g. importlib.metadata with a build-time generated constant, or hatch-vcs/fallback via a _version.py), or add a CI assertion that the two strings match.

🟡 LOW Confound references confirm-run.out, which is absent from the record's artifact list — evidence/INDEX.md

Line 86 says 429 retries were 'logged in confirm-run.out', but the artifact list (lines 78-82) names confirm-runs.json and finalize.out, never confirm-run.out. A reader chasing the 429 log cannot find the file. The string originates in evidence/records/tb-gated-stop-ab-confirm.json (rendered verbatim), so the fix belongs there plus a re-render; flagging here because the index is the human entry point. Regenerate with pnpm run evidence:render after correcting the record.

🟡 LOW n=1 'held-out split' satisfies the denominator field nominally, not substantively — evidence/records/cad-gepa-openscad-directive.json

The +9.5pp held-out effect has no knowable denominator in this record: n.value=1 with unit 'held-out split' while the detail admits 'split size recorded in the creative-agent run artifacts' (not pinned here) and command is null. The schema requires a positive n, so value=1 passes validation while telling the reader nothing about the effective sample size behind the headline number. Self-disclosed as the weakest record by design, so this is a nit: fix by pinning the split size (or its true n) when the CAD run directory is pinned, or recording the denominator gap as an explicit confound.

🟡 LOW Claim stated as fact though the registered rule did not confirm it; MEASURED-ONCE vs RESOLVED-NULL ladder position debatable — evidence/records/tb-gated-stop-ab-confirm.json

The claim field asserts 'finishes more rows' affirmatively, but the result field reports CI [-0.0061, +0.1210] crossing zero and confound 1 says the draw was powered for effect 0.10 while 0.06 was observed. Under the registry's own ladder (evidence/README.md and src/experiment/evidence-record.ts:32-35), RESOLVED-NULL means 'adequate instrument, effect did not appear under the registered rule' - which is what happened here, and is exactly how the analogous vb-cell-engine-parity-no-flip record is labeled. Keeping this at MEASURED-ONCE ranks it above RESOLVED-NULL in the index ordering while the registered significance rule was not met. Mitigated: the non-significance is disclosed in both result and confounds. Fix: either phrase the claim as the hypothesis under test ('gated stop finishes mor

🟡 LOW Byte-for-byte index check breaks on Windows core.autocrlf checkouts — scripts/render-evidence-index.ts

if (committed !== rendered) compares raw strings, and the repo has no .gitattributes (confirmed: no file at repo root). With core.autocrlf=true (git's Windows default), INDEX.md is checked out with CRLF while renderEvidenceIndex joins with '\n', so pnpm run evidence:check fails permanently for those contributors and verify:package goes red for an environmental reason. Fix: add a .gitattributes pinning evidence/INDEX.md text eol=lf (or normalize line endings on both sides before comparing).

🟡 LOW CLI failure branches untested; only the src-level logic has coverage — scripts/render-evidence-index.ts

src/experiment/evidence-record.test.ts covers schema, registry validation, render determinism, and the committed-index byte equality — the script's happy path. No test exercises --check against a stale index, missing INDEX.md, malformed record JSON, filename/id mismatch, or empty records dir, so a regression in those exit(1) paths would ship silently. Cheap fix: a vitest spawn of tsx scripts/render-evidence-index.ts --check against a fixture tree, or extract the check mode into src next to renderEvidenceIndex.

🟡 LOW readdirSync/readFileSync failures crash with a stack instead of the script's clean-error convention — scripts/render-evidence-index.ts

readdirSync(recordsDir) is unguarded: if evidence/records is missing, the script throws an unhandled ENOENT stack; the files.length === 0 guard only covers the empty case. Likewise a subdirectory named *.json passes the filter and readFileSync throws EISDIR. Exit code is still non-zero, so the CI gate fails correctly — this is a diagnostics nit. Wrap the read in the same try/console.error/exit(1) pattern used for JSON.parse.

🟡 LOW Body sections of INDEX.md render claim/result/instrument unescaped — src/experiment/evidence-record.ts

Only the summary table row goes through mdEscape (which itself handles just | and \n); the ## <id> detail sections interpolate r.claim, r.result, r.instrument, artifacts, confounds, and notes raw. Probe-verified: a claim of line1\nline2 [click](https://evil.example) <img src=x onerror=alert(1)> renders the link and img tag into both the table cell and the body, and a claim containing pipes breaks the body list layout. Input is committed by repo members and GitHub sanitizes event handlers, so impact is index integrity (arbitrary links/misleading markup), not XSS. Fix: apply mdEscape to body fields too, or reject newlines/pipe-heavy strings at the schema.

🟡 LOW Date pattern validates shape only, not calendar dates — src/experiment/evidence-record.ts

datePattern is /^\d{4}-\d{2}-\d{2}$/ with no calendar check — probe-verified that '2026-13-45' parses. A fabricated date skews the date-desc ordering that validateEvidenceRegistry guarantees. Zod 4 offers z.string().date() which validates real calendar dates; swap the regex for it.


tangletools · 2026-08-18T21:36:22Z · trace

tangletools
tangletools previously approved these changes Aug 18, 2026

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 10 non-blocking findings — 9a1100c9

Full multi-shot audit completed 8/8 planned shots over 22 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-08-18T21:36:22Z · immutable trace

@tangletools

Copy link
Copy Markdown
Contributor

Premise check withheld merge — 9a1100c9

Classifier flagged this PR as a premise claim (numeric pp/% delta + eval terminology). Confidence: medium.

Recommend re-running the underlying eval with pairedEvalueSequence before merging.

  • Cited claim: +9.5pp
  • PR body excerpt: feat(experiment): the evidence registry — one home for measured claims

Run:

pnpm eval:evolve --reps 5 --skip-mutation

Classifier rationale: Body cites 1 numeric claim(s) (+9.5pp) and eval-related terms appear in pr_body. PR is asserting a measurable result that repair-pr cannot polish away — re-run the underlying evaluation before merging.


tangletools premise check · #644

…laims

A measured claim now has one canonical, machine-readable home.
evidence/records/*.json hold typed records: claim, instrument, exact
command, arms, denominator, result, evidence state (CERTIFIED,
MEASURED-ONCE, RESOLVED-NULL, UNVERIFIED, KILLED), artifacts, cost,
confounds, and the governing experiment seal digest.

The schema (evidenceRegistryRecordSchema) is exported from ./experiment.
scripts/render-evidence-index.ts generates evidence/INDEX.md from the
records, and evidence:check inside verify:package fails on an invalid
record or a stale index, so the human index cannot drift from the data.

Initial migration: seven records — gated-stop confirm (+0.0596, CI
[-0.0061,+0.1210], n=151), free-lunch (3/64 rollouts), multishot golden
oracle v1, the GEPA-certified analyst prompt, prime-vs-dspy (38 rows),
CAD GEPA (+9.5pp), and the VB cell-engine parity no-flip that lived
only in a PR body.
…xport correctly

The denominator schema refused NaN and infinity but admitted zero and
negative values; n.value is now positive. evidence/README.md names the
exported schema evidenceRegistryRecordSchema.
The implementation pin absorbs the 522/524 retry change on main; the
dependency-lock pin absorbs the evidence-check script additions.
@drewstone

Copy link
Copy Markdown
Contributor Author

Merging with --admin, disclosed. The multi-shot verdict (Approved — 10 non-blocking) and value audit (sound-with-nits) stand for 9a1100c; the force-push delta (9a1100c→3e0c1a33) is rebase-only conflict resolution, reviewed by the operator: version files to main's 0.149.0, CHANGELOG bullet moved to [Unreleased] with the duplicate SearchHistoryReceipt entry deduped, both analyst digests repinned for the post-#647/#648 tree (the implementation pin also heals main's currently-stale pin), and the evidence:render/check scripts restored. Verified on the rebased head: 19/19 in the digest + evidence suites, typecheck clean, evidence index fresh (7 records), CI green.

@drewstone
drewstone merged commit 7148ed3 into main Aug 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants