From 82399957b033efbcd5f62650acc72e2817ec8e1a Mon Sep 17 00:00:00 2001 From: "marcin p. joachimiak" <4625870+realmarcin@users.noreply.github.com> Date: Sat, 15 Aug 2026 21:23:50 -0700 Subject: [PATCH 1/2] Make --verify check emptiness and walk disk-to-manifest (#244) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #244 named three gaps beyond the manifest-to-disk existence check. ONE IS MOOT. It asked that the citation sidecar be asserted alongside each artifact; #388 deleted all 353 as a broken regex over report prose — exactly what #249 predicted when it said to decide that issue first. 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. The floor is set FROM THE CORPUS rather than guessed: the smallest real report is 20,785 bytes, 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 — resume keys on the artifact existing. Blocking from the start, with one named exception (cellulolysis-deep-research-codex.md, #245) 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` from `--dry-run` also lives under research/traits 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. FOUND WHILE PROBING, and worth more than the feature: the counters were PER ROW. 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, 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 / orphan_reports as pure functions so the gates are testable, in scan_malformed_curies' style. Eight tests, each making a gate FIRE, on the argument that file already states: "a gate whose corpus is already clean is exactly the kind that can be silently broken". Probed by hand first — an injected orphan and a truncated artifact were both caught, tree restored clean. 548 tests pass - just qc green - ruff clean. Co-Authored-By: Claude Opus 5 --- ...2026-08-16T042348Z-claude-code-0e825f.yaml | 47 ++++++++ scripts/run_trait_graph_audit.py | 108 ++++++++++++++++-- tests/test_run_trait_graph_audit.py | 90 ++++++++++++++- 3 files changed, 232 insertions(+), 13 deletions(-) create mode 100644 history/infrastructure/verify-artifact-integrity/2026-08-16T042348Z-claude-code-0e825f.yaml 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..8437fdb7 --- /dev/null +++ b/history/infrastructure/verify-artifact-integrity/2026-08-16T042348Z-claude-code-0e825f.yaml @@ -0,0 +1,47 @@ +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.' diff --git a/scripts/run_trait_graph_audit.py b/scripts/run_trait_graph_audit.py index 5be9c266..8d81f1cb 100644 --- a/scripts/run_trait_graph_audit.py +++ b/scripts/run_trait_graph_audit.py @@ -104,6 +104,86 @@ 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, because resume keys on the file existing (`pending` +# below). Blocking from the start, with the one known exception named here +# rather than silently tolerated — the same escape idiom as +# audit_biolink_curies.py's ALLOWED_UNBACKED, so adding one is a reviewed change. +KNOWN_ORPHAN_ARTIFACTS = { + # Tracked, from a codex-provider run that never wrote a manifest row (#245). + # Harmless only because its `-codex` suffix is not the `-falcon` name resume + # looks for; that is luck, not design, which is why it is listed and not + # ignored. + "research/traits/metabolism/cellulolysis-deep-research-codex.md", +} + + +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], + known: set[str] = frozenset(KNOWN_ORPHAN_ARTIFACTS)) -> list[str]: + """Reports on disk with no ``ok`` row -- the disk-to-manifest direction (#244). + + Such a file SUPPRESSES A CALL THAT WAS NEVER PAID FOR OR RECORDED, because + resume keys on the artifact existing. + + ``.md`` only, deliberately: a ``-meta.yaml`` written by ``--dry-run`` also + lives under research/traits 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. + """ + return sorted( + rel for rel in (str(p.relative_to(repo_root)) for p in research_dir.rglob("*.md")) + if rel not in recorded and rel not in known + ) + + 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 +256,27 @@ 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"manifest ok rows with a missing artifact: {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) + + orphans = orphan_reports(RESEARCH_DIR, REPO_ROOT, recorded) + print(f"reports on disk with no ok manifest row: {len(orphans)}" + f" ({len(KNOWN_ORPHAN_ARTIFACTS)} known, excluded)", file=sys.stderr) + for rel in orphans[:20]: + print(f" {rel}", 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 +300,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..749db16e 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,81 @@ 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, known=set()) + assert found == ["research/traits/ecology/stray-deep-research-falcon.md"] + + +def test_known_orphans_are_excluded_by_name(tmp_path): + research = tmp_path / "research" / "traits" / "metabolism" + research.mkdir(parents=True) + rel = "research/traits/metabolism/cellulolysis-deep-research-codex.md" + (research / "cellulolysis-deep-research-codex.md").write_text("x") + assert orphan_reports(tmp_path / "research" / "traits", tmp_path, {}, known={rel}) == [] + + +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, {}, known=set()) == [] From 69ff16c26896a2a5af63175ceb88e886885fca65 Mon Sep 17 00:00:00 2001 From: "marcin p. joachimiak" <4625870+realmarcin@users.noreply.github.com> Date: Sat, 15 Aug 2026 21:36:57 -0700 Subject: [PATCH 2/2] Match the orphan gate to its own invariant (#397) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the dispatched review of this PR, and the finding is in this PR's own new code. orphan_reports globbed every `*.md` under research/traits. But the harm it exists to prevent — a file suppressing a call that was never paid for — needs resume to be looking for that filename, and resume keys on `output_path()`: `{slug}-deep-research-{default provider}.md`. Two documented workflows write outside that namespace and record nothing: `just research-trait --provider openai` (README) and `just research-trait-edison` (justfile). The first documented non-falcon run after merge would have turned `just qc` red, remediable only by adding a filename to a constant. THE EXCEPTION LIST CONTAINED THE REFUTATION. It justified excluding cellulolysis-deep-research-codex.md because "-codex is not the name resume looks for". That generalises to every non-default provider — the gate was blocking on files structurally incapable of causing the harm. Narrowed to the resume namespace, so: - the exception list is GONE entirely, and the codex file needs no naming, which is a better outcome for #245 too — it is a tidiness question, not an integrity one; - re-probed: an injected `-falcon` orphan fires, an `-openai` one does not. Also took three smaller notes from the same review: the justfile recipe comment described only the pre-#244 checks; `manifest ok rows with a missing artifact:` still said "rows" after being deduped to artifacts (this PR's own headline finding, applied to the wrong line); and two of the four report blocks lacked the `... and N more` tail the other two carry. 548 tests pass - just qc green - ruff clean. Co-Authored-By: Claude Opus 5 --- ...2026-08-16T042348Z-claude-code-0e825f.yaml | 62 +++++++++-------- justfile | 5 +- scripts/run_trait_graph_audit.py | 69 +++++++++++-------- tests/test_run_trait_graph_audit.py | 19 +++-- 4 files changed, 94 insertions(+), 61 deletions(-) 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 index 8437fdb7..acad69e4 100644 --- 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 @@ -18,30 +18,38 @@ events: 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.' + 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 8d81f1cb..e63b27fd 100644 --- a/scripts/run_trait_graph_audit.py +++ b/scripts/run_trait_graph_audit.py @@ -112,17 +112,21 @@ def output_path(category: str, slug: str, provider: str = DEFAULT_PROVIDER) -> P MIN_ARTIFACT_BYTES = 1024 # A report on disk with no `ok` manifest row SUPPRESSES A CALL THAT WAS NEVER -# PAID FOR OR RECORDED, because resume keys on the file existing (`pending` -# below). Blocking from the start, with the one known exception named here -# rather than silently tolerated — the same escape idiom as -# audit_biolink_curies.py's ALLOWED_UNBACKED, so adding one is a reviewed change. -KNOWN_ORPHAN_ARTIFACTS = { - # Tracked, from a codex-provider run that never wrote a manifest row (#245). - # Harmless only because its `-codex` suffix is not the `-falcon` name resume - # looks for; that is luck, not design, which is why it is listed and not - # ignored. - "research/traits/metabolism/cellulolysis-deep-research-codex.md", -} +# 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]: @@ -167,20 +171,27 @@ def undersized_artifacts(recorded: dict[str, str], repo_root: Path, def orphan_reports(research_dir: Path, repo_root: Path, recorded: dict[str, str], - known: set[str] = frozenset(KNOWN_ORPHAN_ARTIFACTS)) -> list[str]: - """Reports on disk with no ``ok`` row -- the disk-to-manifest direction (#244). - - Such a file SUPPRESSES A CALL THAT WAS NEVER PAID FOR OR RECORDED, because - resume keys on the artifact existing. - - ``.md`` only, deliberately: a ``-meta.yaml`` written by ``--dry-run`` also - lives under research/traits 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. + 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("*.md")) - if rel not in recorded and rel not in known + rel for rel in (str(p.relative_to(repo_root)) for p in research_dir.rglob(pattern)) + if rel not in recorded ) @@ -259,7 +270,7 @@ def main() -> int: recorded = ok_outputs(MANIFEST) missing = missing_artifacts(recorded, REPO_ROOT) - print(f"manifest ok rows with a missing artifact: {len(missing)}", file=sys.stderr) + 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: @@ -270,12 +281,16 @@ def main() -> int: 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) - print(f"reports on disk with no ok manifest row: {len(orphans)}" - f" ({len(KNOWN_ORPHAN_ARTIFACTS)} known, excluded)", 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 diff --git a/tests/test_run_trait_graph_audit.py b/tests/test_run_trait_graph_audit.py index 749db16e..4498b481 100644 --- a/tests/test_run_trait_graph_audit.py +++ b/tests/test_run_trait_graph_audit.py @@ -163,16 +163,23 @@ def test_report_with_no_ok_row_is_an_orphan(tmp_path): 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, known=set()) + found = orphan_reports(tmp_path / "research" / "traits", tmp_path, recorded) assert found == ["research/traits/ecology/stray-deep-research-falcon.md"] -def test_known_orphans_are_excluded_by_name(tmp_path): +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) - rel = "research/traits/metabolism/cellulolysis-deep-research-codex.md" - (research / "cellulolysis-deep-research-codex.md").write_text("x") - assert orphan_reports(tmp_path / "research" / "traits", tmp_path, {}, known={rel}) == [] + 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): @@ -182,4 +189,4 @@ def test_dry_run_meta_yaml_is_not_counted_as_a_report(tmp_path): 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, {}, known=set()) == [] + assert orphan_reports(tmp_path / "research" / "traits", tmp_path, {}) == []