diff --git a/history/infrastructure/verify-artifact-integrity/2026-08-16T042348Z-claude-code-0e825f.yaml b/history/infrastructure/verify-artifact-integrity/2026-08-16T042348Z-claude-code-0e825f.yaml new file mode 100644 index 00000000..acad69e4 --- /dev/null +++ b/history/infrastructure/verify-artifact-integrity/2026-08-16T042348Z-claude-code-0e825f.yaml @@ -0,0 +1,55 @@ +history_version: 1 +target: + kind: infrastructure + path: scripts/run_trait_graph_audit.py + slug: verify-artifact-integrity +session: + id: 2026-08-16T042348Z-claude-code-0e825f + timestamp: '2026-08-16T04:23:48Z' + actors: + - type: ai_agent + name: claude-code +links: + issues: + - https://github.com/CultureBotAI/TraitMech/issues/244 +events: +- type: EDIT + outcome: changed + sections: + - research + summary: Make --verify check emptiness and walk disk-to-manifest, not just existence + details: '244 named three gaps in --verify beyond the manifest-to-disk existence check. One is now moot: + it asked that the citation sidecar be asserted alongside each artifact, and 388 deleted all 353 sidecars + as a broken regex over report prose, exactly as 249 predicted when it said to decide that issue first. + The other two are implemented. EXISTENCE IS NOT NON-EMPTINESS: .exists() passes for a zero-byte file, + so a truncated write, a process killed mid-flush, or a provider returning an empty body all read as + success. Floor set from the corpus rather than guessed - the smallest real report is 20785 bytes (ecology/biosafety_level_4), + so 1 KiB leaves a 20x margin and cannot fail on real data. DISK TO MANIFEST: a report with no ok row + was invisible, while still suppressing the call that would have produced it, because resume keys on + the artifact existing. Blocking from the start with one named exception, cellulolysis-deep-research-codex.md, + using the same escape idiom as audit_biolink_curies.py''s ALLOWED_UNBACKED so adding one is a reviewed + change; it is harmless today only because its -codex suffix is not the -falcon name resume looks for, + which is luck rather than design. Scoped to .md deliberately: a -meta.yaml written by --dry-run also + lives under research/traits and represents no research at all (status dry-run, cost None, task id + None, see 246), so counting it would let a plan nobody paid for satisfy an existence check. FOUND + WHILE PROBING: the counters were per ROW, and the manifest is append-only with 342 of 353 artifacts + carrying two ok rows apiece - the original sweep and the re-run after its output was lost - so 700 + rows describe 353 files and a single truncated report was reported as two. Both counters now dedupe + per artifact, which is how the invariants are phrased and how the adjacent CURIE scan already reported. + Extracted ok_outputs, missing_artifacts, undersized_artifacts and orphan_reports as pure functions + so the gates are testable, in the same style as scan_malformed_curies. Eight new tests, each making + a gate FIRE, on the argument the existing test file already states: a gate whose corpus is clean is + the kind that can be silently broken, and tests are what distinguish found nothing from cannot find + anything. Verified by probe before the tests existed: an injected orphan and a truncated artifact + were both caught, and the tree was restored clean. 548 tests pass, qc green. ADDENDUM after the dispatched + review (issue 397). The disk-to-manifest gate as first written globbed every .md under research/traits + and would have failed on documented workflows: just research-trait --provider openai writes slug-deep-research-openai.md + and just research-trait-edison writes slug-edison-job.md, neither of which resume looks for. The PR''s + own exception list contained the refutation - it justified excluding the codex file on the grounds + that -codex is not the name resume looks for, which generalises. Narrowed the glob to the resume namespace, + slug-deep-research-provider.md for the default provider, so the gate matches its invariant exactly. + The exception list is now gone entirely and cellulolysis-deep-research-codex.md needs no naming, which + is also a better outcome for 245. Re-probed after narrowing: an injected falcon-named orphan fires, + an openai-named one does not. Also took three smaller review notes - the justfile recipe comment described + only the pre-244 checks, the missing-artifact line still said rows after being deduped to artifacts, + and two of the four report blocks lacked the trailing and-N-more line the other two carry.' diff --git a/justfile b/justfile index 65a4a10c..2eea94d3 100644 --- a/justfile +++ b/justfile @@ -906,7 +906,10 @@ audit-derived-reports: echo "=== derived reports: all current ===" # Integrity gate for the tracked sweep artifacts: every manifest `ok` row's -# report is on disk, and no artifact carries a malformed CURIE. Deliberately +# report is on disk AND above a 1 KiB floor (existence is not non-emptiness), no +# report in the resume namespace lacks an `ok` row (the disk-to-manifest +# direction, which a missing-artifact check cannot see), and no artifact carries +# a malformed CURIE (#244). Deliberately # credential-free and network-free (see run_trait_graph_audit.py --verify), so it # runs on a fresh clone and in CI — the two places where a lost artifact is # actually noticed. Plain `uv run`, no `--extra dev`: --verify makes no calls and diff --git a/scripts/run_trait_graph_audit.py b/scripts/run_trait_graph_audit.py index 5be9c266..e63b27fd 100644 --- a/scripts/run_trait_graph_audit.py +++ b/scripts/run_trait_graph_audit.py @@ -104,6 +104,97 @@ def output_path(category: str, slug: str, provider: str = DEFAULT_PROVIDER) -> P ) +# A report below this is a truncated write, a killed process mid-flush, or a +# provider returning an empty body — all of which satisfy `.exists()` (#244). +# The floor is set from the corpus rather than guessed: the smallest real report +# is 20,785 bytes (ecology/biosafety_level_4), so 1 KiB leaves a 20x margin and +# cannot fail on real data while still catching an artifact with nothing in it. +MIN_ARTIFACT_BYTES = 1024 + +# A report on disk with no `ok` manifest row SUPPRESSES A CALL THAT WAS NEVER +# PAID FOR OR RECORDED — but only if resume would have looked for that name. +# Resume keys on `output_path()`, i.e. `{slug}-deep-research-{provider}.md` for +# the DEFAULT provider, so the gate is scoped to exactly that namespace (#244). +# +# The wide version of this check (every `*.md` under research/traits) was the +# review's finding on #396, and it was wrong for the reason the exception list +# itself gave: `cellulolysis-deep-research-codex.md` is harmless BECAUSE `-codex` +# is not the name resume looks for. That generalises. `just research-trait +# --provider openai` (README) writes `{slug}-deep-research-openai.md` and +# `just research-trait-edison` writes `{slug}-edison-{job}.md`; neither can +# suppress anything, and blocking on them would have turned `just qc` red on the +# first documented non-falcon run, remediable only by adding a filename to a +# constant — which is precisely how the `-codex` file came to need one. +# +# Scoping to the resume namespace means there is no exception list at all. + + +def ok_outputs(manifest: Path) -> dict[str, str]: + """``{output path: first run_id}`` over the manifest's ``ok`` rows. + + Deduplicated per artifact on purpose. The manifest is append-only and 342 of + the 353 artifacts carry two ``ok`` rows apiece -- the original sweep and the + re-run after its output was lost -- so 700 rows describe 353 files. Every + count derived from it is about artifacts, which is how the invariants are + phrased and how the CURIE scan below already reports. + """ + out: dict[str, str] = {} + with manifest.open() as fh: + for row in csv.DictReader(fh, delimiter="\t"): + if row.get("status") != "ok": + continue + rel = (row.get("output") or "").strip() + if rel: + out.setdefault(rel, row.get("run_id", "?")) + return out + + +def missing_artifacts(recorded: dict[str, str], repo_root: Path) -> list[tuple[str, str]]: + """``ok`` rows whose artifact is gone -- a call paid for and unresumable.""" + return [(run_id, rel) for rel, run_id in sorted(recorded.items()) + if not (repo_root / rel).exists()] + + +def undersized_artifacts(recorded: dict[str, str], repo_root: Path, + floor: int = MIN_ARTIFACT_BYTES) -> list[tuple[str, str, int]]: + """``ok`` artifacts that exist but are below ``floor`` bytes (#244). + + ``.exists()`` passes for a zero-byte file, so a truncated write, a process + killed mid-flush, or a provider returning an empty body all read as success. + """ + out = [] + for rel, run_id in sorted(recorded.items()): + path = repo_root / rel + if path.exists() and path.stat().st_size < floor: + out.append((run_id, rel, path.stat().st_size)) + return out + + +def orphan_reports(research_dir: Path, repo_root: Path, recorded: dict[str, str], + provider: str = DEFAULT_PROVIDER) -> list[str]: + """Reports in the RESUME NAMESPACE with no ``ok`` row (#244). + + Such a file suppresses a call that was never paid for or recorded, because + the `pending` filter skips a target whose ``output_path()`` exists. + + Scoped to ``*-deep-research-{provider}.md`` rather than every ``*.md``, + because that is the only name resume consults -- see the note above the + MIN_ARTIFACT_BYTES/namespace commentary. A report from another provider is untidy and + may well be a defect (#245), but it cannot cause the harm this gate exists + to prevent, and failing on it would block documented workflows. + + ``.md`` only: a ``-meta.yaml`` written by ``--dry-run`` also lives here and + represents NO research (``status: dry-run``, ``cost: None``, ``task_id: + None`` -- #246), so counting it would let a plan nobody paid for satisfy an + existence check. + """ + pattern = f"*-deep-research-{resolve_provider(provider)}.md" + return sorted( + rel for rel in (str(p.relative_to(repo_root)) for p in research_dir.rglob(pattern)) + if rel not in recorded + ) + + def scan_malformed_curies(paths: list[Path]) -> list[tuple[Path, int, str, str]]: """Return (path, line_no, pattern_name, matched_text) for every bad CURIE.""" bad: list[tuple[Path, int, str, str]] = [] @@ -176,21 +267,31 @@ def main() -> int: # 342 rows were in that state before research/ was tracked, and four # more were created by deleting reports the running sweep had already # passed. Relying on someone remembering is how the first 342 were lost. - missing = [] - with MANIFEST.open() as fh: - for row in csv.DictReader(fh, delimiter="\t"): - if row.get("status") != "ok": - continue - out = (row.get("output") or "").strip() - if out and not (REPO_ROOT / out).exists(): - missing.append((row.get("run_id", "?"), out)) - print(f"manifest ok rows with a missing artifact: {len(missing)}", - file=sys.stderr) + recorded = ok_outputs(MANIFEST) + + missing = missing_artifacts(recorded, REPO_ROOT) + print(f"ok artifacts missing from disk: {len(missing)}", file=sys.stderr) for run_id, out in missing[:20]: print(f" {run_id} {out}", file=sys.stderr) if len(missing) > 20: print(f" ... and {len(missing) - 20} more", file=sys.stderr) + undersized = undersized_artifacts(recorded, REPO_ROOT) + print(f"ok artifacts below {MIN_ARTIFACT_BYTES} bytes: {len(undersized)}", + file=sys.stderr) + for run_id, out, size in undersized[:20]: + print(f" {run_id} {out} ({size} bytes)", file=sys.stderr) + if len(undersized) > 20: + print(f" ... and {len(undersized) - 20} more", file=sys.stderr) + + orphans = orphan_reports(RESEARCH_DIR, REPO_ROOT, recorded, provider) + print(f"resume-namespace reports with no ok manifest row: {len(orphans)}", + file=sys.stderr) + for rel in orphans[:20]: + print(f" {rel}", file=sys.stderr) + if len(orphans) > 20: + print(f" ... and {len(orphans) - 20} more", file=sys.stderr) + # Scanned over every .md under research/. This used to mean reports AND # their citation sidecars, justified by the sidecar echoing the rendered # prompt — but #249 dropped the sidecars as a broken duplicate, and that @@ -214,7 +315,7 @@ def main() -> int: if len(bad_curies) > 20: print(f" ... and {len(bad_curies) - 20} more", file=sys.stderr) - return 1 if (missing or bad_curies) else 0 + return 1 if (missing or undersized or orphans or bad_curies) else 0 # One id for every row this invocation writes. The manifest is append-only # and a trait can legitimately appear more than once — a failure and its diff --git a/tests/test_run_trait_graph_audit.py b/tests/test_run_trait_graph_audit.py index d3a6abd5..4498b481 100644 --- a/tests/test_run_trait_graph_audit.py +++ b/tests/test_run_trait_graph_audit.py @@ -3,7 +3,8 @@ The scan exists because the sweep produced double-prefixed CURIEs twice, and the second time the manual grep for them raced a report still being generated. It is wired into `--verify` and therefore into `just qc`, where it currently reports -zero hits across all 707 tracked artifacts. +zero hits across all 354 tracked artifacts (707 before #388 dropped the +citation sidecars). A gate whose corpus is already clean is exactly the kind that can be silently broken — a mistyped pattern would keep reporting zero forever. These tests are @@ -20,7 +21,14 @@ REPO_ROOT = Path(__file__).resolve().parent.parent sys.path.insert(0, str(REPO_ROOT / "scripts")) -from run_trait_graph_audit import scan_malformed_curies # noqa: E402 +from run_trait_graph_audit import ( # noqa: E402 + MIN_ARTIFACT_BYTES, + missing_artifacts, + ok_outputs, + orphan_reports, + scan_malformed_curies, + undersized_artifacts, +) def _scan(tmp_path: Path, text: str): @@ -97,3 +105,88 @@ def test_the_tracked_corpus_is_clean(): assert artifacts, "no research artifacts found — research/ should be tracked" hits = scan_malformed_curies(artifacts) assert hits == [], f"malformed CURIEs in tracked artifacts: {hits[:5]}" + + +# ------------------------------------------------ artifact integrity (#244) +# +# Same argument as the CURIE scan above: these gates report zero on a clean +# corpus, so without tests "found nothing" and "cannot find anything" look +# identical. Each one below is made to FIRE. + + +def _manifest(tmp_path: Path, rows: list[tuple[str, str, str]]) -> Path: + path = tmp_path / "manifest.tsv" + lines = ["run_id\tcategory\tslug\tstatus\toutput"] + for run_id, status, out in rows: + lines.append(f"{run_id}\tcat\tslug\t{status}\t{out}") + path.write_text("\n".join(lines) + "\n") + return path + + +def test_ok_outputs_keeps_one_entry_per_artifact(tmp_path): + m = _manifest(tmp_path, [ + ("r1", "ok", "research/a.md"), + ("r2", "ok", "research/a.md"), # the re-run + ("r3", "error", "research/b.md"), # not ok + ]) + recorded = ok_outputs(m) + assert recorded == {"research/a.md": "r1"} + + +def test_missing_artifact_is_reported_once_not_per_row(tmp_path): + m = _manifest(tmp_path, [("r1", "ok", "gone.md"), ("r2", "ok", "gone.md")]) + assert missing_artifacts(ok_outputs(m), tmp_path) == [("r1", "gone.md")] + + +def test_zero_byte_artifact_is_caught_though_it_exists(tmp_path): + """`.exists()` passes for a truncated write; that is the whole point.""" + (tmp_path / "empty.md").write_text("") + m = _manifest(tmp_path, [("r1", "ok", "empty.md")]) + found = undersized_artifacts(ok_outputs(m), tmp_path) + assert found == [("r1", "empty.md", 0)] + + +def test_a_real_sized_artifact_passes(tmp_path): + (tmp_path / "full.md").write_text("x" * (MIN_ARTIFACT_BYTES + 1)) + m = _manifest(tmp_path, [("r1", "ok", "full.md")]) + assert undersized_artifacts(ok_outputs(m), tmp_path) == [] + + +def test_floor_is_far_below_the_smallest_real_report(): + """Set from the corpus, not guessed: the smallest real report is 20,785 + bytes, so the floor must leave room rather than track it.""" + assert MIN_ARTIFACT_BYTES < 20_785 / 10 + + +def test_report_with_no_ok_row_is_an_orphan(tmp_path): + research = tmp_path / "research" / "traits" / "ecology" + research.mkdir(parents=True) + (research / "stray-deep-research-falcon.md").write_text("x") + recorded = {"research/traits/ecology/known-deep-research-falcon.md": "r1"} + found = orphan_reports(tmp_path / "research" / "traits", tmp_path, recorded) + assert found == ["research/traits/ecology/stray-deep-research-falcon.md"] + + +def test_other_providers_are_outside_the_resume_namespace(tmp_path): + """The gate must match its own invariant. Resume keys on + `{slug}-deep-research-{default provider}.md`, so a report from another + provider cannot suppress a call — and blocking on one would turn qc red on + the documented `--provider openai` and `research-trait-edison` paths, fixable + only by adding a filename to a constant (#396 review).""" + research = tmp_path / "research" / "traits" / "metabolism" + research.mkdir(parents=True) + for name in ("cellulolysis-deep-research-codex.md", + "cellulolysis-deep-research-openai.md", + "cellulolysis-edison-literature.md"): + (research / name).write_text("x") + assert orphan_reports(tmp_path / "research" / "traits", tmp_path, {}) == [] + + +def test_dry_run_meta_yaml_is_not_counted_as_a_report(tmp_path): + """A --dry-run writes -meta.yaml with status: dry-run, cost: None and + task_id: None (#246). Counting it would let a plan nobody paid for satisfy + an existence check.""" + research = tmp_path / "research" / "traits" / "environment" + research.mkdir(parents=True) + (research / "psychrotolerant-edison-literature-meta.yaml").write_text("status: dry-run\n") + assert orphan_reports(tmp_path / "research" / "traits", tmp_path, {}) == []