Track research/, and launch the sweep through just (#183) - #240
Conversation
research/ was ignored as "large, regenerable". The second half turned out to be the expensive half: 342 of 353 reports were lost because nothing outside one machine held them, and the manifest recorded 353 successes against 11 surviving files. Regenerating costs a paid Edison call per trait at roughly 7.5 minutes each, so these are provenance rather than build output. Now tracked, starting with the 12 reports on disk. The sweep also had no just recipe, which is a credential trap rather than an omission. scripts/research_trait.py has no load_dotenv and says so in its own comment: a run launched outside `just` sees no EDISON_API_KEY, so every call in the batch fails instantly. `set dotenv-load := true` is what injects the per-repo .env, so trait-graph-sweep now exists and the sweep is launched through it. Canary before fan-out, per the batch rule, and verified on side effects rather than the exit code: one real unit (ecology/animal_pathogen) produced a 50,846-byte report plus a 5,339-byte citations sidecar, `cached: false` so it was genuinely billed, 57 DOI/PMID references, a manifest row appended, and the count moved 11 -> 12. 452s wall clock. The recipe comment carries the paid warning and the canary instruction, since the next person to run it will not have this context. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @realmarcin's task in 4m 29s —— View job ReviewThe 🟡 The manifest is committed with 342 of 354
|
![culturebot-reviewer[bot]](https://avatars.githubusercontent.com/u/233210009?s=60&v=4){kind=small}
There was a problem hiding this comment.
Tracking research/ is the right call. Three things to settle before the batch lands.
The manifest is committed with 342 of 354 ok rows pointing at files that aren't in the tree. The premise here is that research/ is provenance and the manifest indexes it; as committed the index disagrees with the tree by 97%, and a clone can't distinguish 'billed once, output lost' from 'billed, output here'.
The running sweep will append a second ok row for each of the 341. scripts/run_trait_graph_audit.py:118 appends with no dedup, so the finished manifest carries ~700 rows with duplicate (category, slug) keys. That breaks the exact set-difference analysis NEXT_TASKS.md:483-485 already performs on this file — fail − ok stops separating a retry from a re-bill, which is the question this PR exists to make answerable. It's a one-line decision now and an untangling job after the batch finishes.
NEXT_TASKS.md:489 is now false — it asserts research/ is in .gitignore at line 42, which this PR deletes.
Also minor: two tracked *-edison-literature-meta.yaml sidecars are status: dry-run stubs with no result and no spend, which argues against the 'provenance, not build output' rationale; the recipe comment's 'every call in the sweep fails instantly' is contradicted by the preflight at run_trait_graph_audit.py:98-99, which exits 2 with a message before spawning anything; and every report's front matter bakes in an absolute /Users/marcin/... template path.
No data/traits/** change, so no history/ record expected. The reports themselves hedge their groundings correctly (label-only nodes, explicit verify-before-import, per-edge taxon specificity) — full CURIE resolution belongs to the PR that lifts them into the KB.
Details in the comment above.
Tracking research/ put Edison's reports in the link checker's scope, and CI failed on `artifact:artifact-02` — an Edison-internal reference scheme being read as a relative path. Two findings, neither a real broken link. SKIP_LINK_PREFIXES was an allowlist of http/https/mailto/tel that had to grow every time a new scheme appeared, which is a gate that breaks on content rather than on defects. Replaced with an RFC 3986 scheme match; `#` stays a prefix since a same-document anchor has no scheme. Measured before changing it, across every tracked .md: the scheme rule skips exactly those two links and nothing else that was previously checked. Also pre-checked the batch output rather than waiting to be surprised by it — all 29 research reports on disk, tracked and untracked, produce zero link findings, so committing the remaining ~341 will not trip this gate. Two tests: schemes skipped (artifact:, doi:, https:, mailto:, #anchor), and a broken relative path still reported alongside them so the rule cannot swallow ordinary findings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @realmarcin's task in 3m 13s —— View job ReviewThe new commit is good. The three things the last review asked for are all still open, and one of them stopped being a prediction. 🟡 The duplicate-key problem is now in the committed file, not just ahead of itLast review flagged this as something to decide before the batch landed. The batch has now written into the tracked manifest:
Three rows is a one-command untangle today. Seven hundred is not. 🟡 342
|
There was a problem hiding this comment.
Re-review after b52d858. The link-check commit is good; the three items from the previous review are all still open, and one has materialized.
The duplicate-key problem is now in the committed manifest. The batch appended rows 363-365 (ecology/animal_pathogen, biosafety_level, biosafety_level_1) that duplicate existing ok rows at lines 2, 11, 13. ecology/biosafety_level now carries fail:1, ok, ok — the set-difference analysis at NEXT_TASKS.md:483-485 can no longer separate a July retry-success from an August re-bill. Three rows is a one-command untangle; 700 is not.
342 ok rows still point at files not in the tree (356 ok rows, 15 tracked reports), and NEXT_TASKS.md:489 still asserts research/ is gitignored — the line this PR deletes.
On the new commit: the RFC 3986 scheme match is the right shape and the second test keeps it from swallowing real findings. One unmentioned widening — the pattern also matches a bare filename before a colon, so [x](NEXT_TASKS.md:489) is now skipped where it was previously BROKEN_LINK. Nothing tracked has that shape today. I could not execute the test suite (sandbox declined).
Groundings in both new reports spot-check clean against data/traits/ecology/ and their claimed labels, all hedged as candidates. No data/traits/** change, so no history/ record expected.
Details in the comment.
NEXT_TASKS §8 still said research/ is gitignored and nothing under it is tracked, with a heading asserting the output is GONE. Both were made false by this PR. Rewritten to record what happened, what the recipe is for, and what the canary proved. The manifest inconsistency the review raised is stated rather than patched: 366 rows, 9 duplicate (category, slug) keys from retries, and 337 `ok` rows whose file is not yet in the tree. That count falls to 0 as the sweep completes, and faking agreement now would corrupt the one thing the manifest is actually good for — the record of what was billed. It lands consistent with the tree in the completion commit. Also noted there: cost per call is captured nowhere. duration_seconds is recorded but no USD figure, so this run can report time and not spend. Batch status at time of writing: 4 done, 0 failures. The 8 fail:1 rows in the manifest are all from the 2026-07-20 run and were retried successfully then. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @realmarcin's task in 3m 59s —— View job Review
✅ Settled
The manifest/tree disagreement is now recorded rather than only discoverable by
Stating it and landing the reconciliation in the completion commit is a fine answer to that item — better than a synthetic status value. Dropping it. 🟡 The duplicate
|
There was a problem hiding this comment.
Re-review after ca93318. §8 is properly fixed, and the manifest/tree disagreement is now stated in the repo with figures I verified independently (366 rows, 9 duplicate keys, 337 ok rows whose file is absent — all three exact). Dropping that item; landing the reconciliation in the completion commit is a better answer than a synthetic status value.
The duplicate-key item is the only one left, and it grew. scripts/run_trait_graph_audit.py:118 still appends with no dedup, and five (category, slug) keys now carry two ok rows — biofilm_formation and biosafety_level_5 joined the three from last review. The commit message declines to patch because faking agreement would corrupt the record of what was billed. Agreed on the premise, but a run-id/timestamp column is not faking agreement — it is what makes that record readable. ecology/biofilm_formation has fail:1, ok, ok and no field separating the July retry from the August re-bill. Recoverable today by git archaeology (July ends at line 362), which bounds the severity, but 341 more rows are landing into it now.
Minor: the recipe's canary instruction (justfile:307) says check the artifact is 'on disk and non-empty' — a cache hit satisfies that, exactly like the exit code it replaces; the PR body's own cached: false is the right check and belongs in the durable copy. And 'every call in the sweep fails instantly' is now in both justfile:302 and NEXT_TASKS §8, still contradicted by the preflight at run_trait_graph_audit.py:98-100.
Groundings in both new reports check out against data/traits/ecology/ (METPO:1001106, METPO:1001101, traitmech:000053, METPO:1000059) and their claimed labels (GO:0042710, GO:0071973, CHEBI:49537/15996/16480/15379, NCBITaxon:287/1423). CHEBI:58187 and CHEBI:62202 unverified — no offline ontology. All hedged as candidates; cached: false on both sidecars. No data/traits/** change, so no history/ record expected. Could not execute pytest (sandbox declined).
Details in the comment.
The review turned my own rationale back on me correctly. I argued against touching the manifest because "the record of what was billed is the one thing worth keeping accurate" — and a run id is precisely what makes that record readable, not a patch over it. `biofilm_formation` carried three indistinguishable rows: a July failure, its July retry, and the August re-bill. "What was billed, when" was the one question the spend record could not answer about itself. manifest gains a leading run_id column, one value per invocation, printed at start so a log can be tied to its rows. Existing rows backfilled: the July run is the contiguous head of 361, everything after is 2026-08-04. That boundary is unambiguous rather than inferred — the file was 362 lines before today. 2026-07-20T000000Z ecology biofilm_formation fail:1 2026-07-20T000000Z ecology biofilm_formation ok 2026-08-04T000000Z ecology biofilm_formation ok The batch was STOPPED to do this rather than patched underneath itself: it opens the manifest in append mode, so rewriting the file while it ran would have raced against the very record being repaired. Restarting cost nothing in re-billing — resume is file-existence based — beyond the four calls in flight at the time. Relaunched and verified the new schema end to end: run_id 2026-08-04T061322Z appears on rows written after the restart, distinct from the backfilled 2026-08-04T000000Z, so the column separates runs rather than just existing. Status: 23 of 353 reports on disk, ok=4 fail=0 since the restart, 330 pending. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @realmarcin's task in 4m 55s —— View job Review
✅ The
|
| check | result |
|---|---|
rows per run_id |
361 July / 8 backfilled Aug / 4 post-restart 2026-08-04T061322Z |
duplicate (run_id, category, slug) |
8, all July, all fail:1 + ok |
duplicate (category, slug) across runs |
13, every one now separated by run_id |
That second row is the one that matters: within a single run a repeated key is always a failure and its retry, never two oks. So NEXT_TASKS.md:483-485's set difference is well-defined again once scoped to a run — ecology/biofilm_formation reads 2026-07-20 fail:1 / 2026-07-20 ok / 2026-08-04 ok, and the retry is distinguishable from the re-bill without git blame. Stopping the batch rather than rewriting an append-mode file underneath itself was the right call. Dropping this item.
🟡 Three of the seven new reports emit a double-prefixed CURIE, in the section a curator lifts from
research/traits/ecology/free_living-deep-research-falcon.md:187
- **free-living** — `METPO:traitmech:000048`
research/traits/ecology/endosymbiosis-deep-research-falcon.md:202
- Endosymbiosis — `METPO:traitmech:000045`
research/traits/ecology/commensalism-deep-research-falcon.md:205
- **Commensalism** — `METPO:traitmech:000042` as supplied.
All three are under ## 2. Candidate graph nodes. METPO:traitmech:000048 resolves to nothing; the trait's actual identifier is traitmech:000048 (data/traits/ecology/free_living.yaml:1), which is correct everywhere else in the same reports.
The cause is in the repo, not the model: templates/trait_causal_graph_research.md:5 is
- **METPO identifier:** {trait_identifier}
and {trait_identifier} is a traitmech: CURIE for 120 traits under data/traits/. biofilm_formation's earlier report shows the near-miss — it renders **METPO identifier:** traitmech:000053 intact — so the collapse into a prefixed string is intermittent, which is worse than systematic: it means spot-checking one report won't tell you whether the next 330 are clean. commensalism's "as supplied" is the model reporting that it believed it was handed that string.
Renaming that field to something that isn't a prefix — **Trait identifier:** — costs nothing and can't affect the already-billed reports. ~330 calls are still to come. Fix this →
🔵 The backfilled run_ids are synthetic below day precision, and nothing in the file says so
2026-07-20T000000Z and 2026-08-04T000000Z are invented midnights sitting in the same column as 2026-08-04T061322Z, which is a real invocation stamp. The dates are true and the reasoning is in the commit message, but the commit message isn't the durable copy — a reader a year out sees a timestamp-shaped value and reads it as a start time. Same shape as the argument this commit accepted about the manifest itself. A NEXT_TASKS §8 line, or T000000Z called out as the backfill marker, would settle it.
🔵 An existing 4-column manifest would silently gain 5-column rows
scripts/run_trait_graph_audit.py:127 writes the header only when the file is absent, so the schema change is invisible to any manifest that already exists — a stale local copy, or a checkout that predates this commit, gets 5-field rows appended under a 4-field header with no error. A clone is fine because the tracked file is already migrated, which bounds it; a one-line check that the first line matches the expected header would close it.
🔵 §8's figures moved with the sweep, and don't mention the new column
NEXT_TASKS.md:506-508 says 366 rows / 9 duplicate keys / 337 absent ok files. Now 373 / 13 / 330. Drift is inherent to a "currently" statement mid-sweep and I'm not asking for a number chase — but §8 is where the manifest is described, and it doesn't mention that the schema gained a run_id, which isn't transitional.
🔵 Carried, unchanged
justfile:302— "every call in the sweep fails instantly" is still contradicted by the preflight atscripts/run_trait_graph_audit.py:99-101, which returns 2 with a message before spawning anything. Now inNEXT_TASKS.md:498too.justfile:307— "check the artifact is on disk and non-empty" has the same blind spot as the exit code it replaces; a cache hit satisfies both. The reports carrycached:in their front matter and the PR body already names it as the check that matters.template_file: /Users/marcin/Documents/VIMSS/...in 12 of the 24 tracked reports.- Two
status: dry-runsidecars still tracked.
Groundings in the seven new reports
Checked against the repo: METPO:1001103 / 1001104 / 1001105 and the shared parent METPO:1001101 match data/traits/ecology/biosafety_level_2.yaml:1,10, _3.yaml:1,11, _4.yaml:1,10. traitmech:000042 / 000045 / 000048 / 000052 and parents 000040 / 000047 / 000049 match their trait files — apart from the three prefixed renderings above.
The OBO CURIEs (GO:0043656, GO:0009399, GO:0006298, GO:0031929, GO:0030254, CHEBI:422, CHEBI:13705, CHEBI:506227, CHEBI:17968, CHEBI:30089) are unverified — no offline ontology here, and none of them appear elsewhere in data/ to cross-check against. GO:0043656 carries its own "verify ontology version before committing," and everything is marked candidate. cached: false on all seven, so the spend is real. No data/traits/** change, so no history/ record expected.
Could not run pytest — the sandbox declined the command, as in the previous three reviews.
Requesting changes on the template field only. It's the last point at which it costs a rename rather than a pass over 350 files.
· branch feat/track-research-artifacts
There was a problem hiding this comment.
Re-review after e93800c. The run_id column closes the manifest item — I verified the July/August boundary independently (origin/main's manifest is 362 lines = header + 361 rows, so the contiguous head of 361 is July, not inferred), and the column does the work rather than just existing: the 8 within-run duplicate keys are all July fail:1 + ok retry pairs, so no run contains two indistinguishable ok rows and the set-difference analysis at NEXT_TASKS.md:483-485 is well-defined again per run. Stopping the batch rather than rewriting an append-mode file underneath itself was right. Dropping that item.
One new 🟡, in the seven reports this commit adds. Three of them emit an unresolvable double-prefixed CURIE in the 'Candidate graph nodes' section — the section a curator lifts from: METPO:traitmech:000048 (free_living-deep-research-falcon.md:187), METPO:traitmech:000045 (endosymbiosis:202), METPO:traitmech:000042 (commensalism:205, annotated 'as supplied'). The correct identifiers are traitmech:000048/000045/000042 and are right everywhere else in the same files. The cause is in the repo: templates/trait_causal_graph_research.md:5 is 'METPO identifier: {trait_identifier}', and 120 traits under data/traits/ carry traitmech: identifiers rather than METPO ones. biofilm_formation's earlier report renders the same field intact, so the collapse is intermittent — which means spot-checking one report tells you nothing about the next 330. Renaming the field to 'Trait identifier' costs nothing, cannot affect already-billed reports, and is the last cheap moment before the rest of the sweep lands.
Minor: the backfilled run_ids (2026-07-20T000000Z, 2026-08-04T000000Z) are invented midnights sitting in the same column as the real 2026-08-04T061322Z, with the reasoning only in the commit message rather than the repo; run_trait_graph_audit.py:127 writes the header only when the file is absent, so a pre-existing 4-column manifest would silently gain 5-field rows; NEXT_TASKS §8's figures moved with the sweep (366/9/337 -> 373/13/330) and don't mention the new column. Carried unchanged: justfile:302 'every call fails instantly' still contradicted by the preflight at run_trait_graph_audit.py:99-101 (now copied into NEXT_TASKS.md:498), justfile:307's 'on disk and non-empty' canary has the same blind spot as the exit code it replaces (cached: is the field that doesn't), absolute /Users/marcin template_file in 12 of 24 tracked reports, two dry-run sidecars.
Groundings: METPO:1001103/1001104/1001105/1001101 match data/traits/ecology/biosafety_level_2-4.yaml exactly; traitmech:000042/000045/000048/000052 and parents 000040/000047/000049 match their trait files. The OBO CURIEs are unverified — no offline ontology and none appear elsewhere in data/ to cross-check. All hedged as candidates, cached: false on all seven. No data/traits/** change, so no history/ record expected. Could not execute pytest (sandbox declined).
Details in the comment.
The review caught it in the generated reports, which is the right place to have looked — the section a curator lifts from. Four of the new reports carry `METPO:traitmech:000042` and friends, and one says "as supplied", which is the tell: the model is echoing the prompt, not inventing it. The template labelled the field "**METPO identifier:**" while the value for a synthetic trait is already a `traitmech:` CURIE, so the model concatenated label and value. 120 of the 353 traits with graphs are synthetic, so this would have reached roughly a third of the corpus. Relabelled to name what the value is and say not to prefix it. The template is passed per spawned call rather than read once, so this takes effect for every subsequent trait without restarting the sweep — verified by reading how research_trait.py hands --template to the child. Deleted the four affected reports and their citation sidecars so the resumable sweep regenerates them against the corrected template. They cost four calls to redo, against a third of the corpus carrying a malformed identifier in the section curators copy from. The re-run will be distinguishable from the original by run_id, which is what that column is for. No double-prefixed CURIE remains in any report on disk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @realmarcin's task in 4m 3s —— View job Review
✅ The double-prefixed CURIE is fixed, and the "no restart needed" claim holdsChecked rather than taken:
Relabelling to 🟡 The four deleted reports will not come back from the sweep that's runningThe commit deletes them "so the resumable sweep regenerates them." That sweep can't. pending = [ ... for cat, slug, label in targets
if not output_path(cat, slug, provider).exists() ]
So the state on the branch is four That is the same shape as the 337 July rows this PR exists to explain — a clone cannot tell "billed, deliberately discarded, pending regeneration" from "billed, lost." The fix is one more 🔵 §8's manifest figures have drifted again
🔵 Carried, unchanged
The two new reports — clean
No Could not run Requesting changes on the four missing reports only. Everything else in this commit is right, and the manifest is now good enough that this one is a record-keeping fix rather than an untangle. |
There was a problem hiding this comment.
Re-review after 5909c76. The template rename closes the double-prefixed CURIE item, and I verified all three of its claims: no double-prefixed CURIE remains anywhere under research/; git grep "METPO identifier" outside research/ is empty so the rename breaks no consumer; and research_trait.py:182-183 passes --template as a path to each spawned child, so the corrected template does reach subsequent calls without a restart. Fixing the cause rather than the four reports was right.
One new IMPORTANT, from that same commit. The four deleted reports will not come back from the sweep that is running. run_trait_graph_audit.py:106-110 computes pending once at startup from files present at that moment, and futures at :176 are submitted from that fixed list. Run 2026-08-04T061322Z started 06:13:22Z and produced these four itself; the deletions landed at 06:34Z, and the commit message says explicitly the sweep was not restarted. So the branch now carries four ok rows with an output path and no file (commensalism:369, endosymbiosis:371, free_living:374, mutualism:377) - the same shape as the 337 July rows this PR exists to explain, and a clone cannot tell 'billed, deliberately discarded, pending regeneration' from 'billed, lost'. mutualism is the sharper case: it has never been in the tree at any commit, so its ok row appears in this diff as a new success for a file the diff never adds, and it is not named among the four in the commit message. The remedy is cheap - one more just trait-graph-sweep after the batch drains picks up exactly these four - but nothing in the repo records that it is owed.
Minor: NEXT_TASKS §8's manifest figures drifted again (366/9/337, now 376 rows) and still do not mention the run_id column; the four deletions and the schema note land in the same edit. Carried unchanged: justfile:302 'every call fails instantly' still contradicted by the preflight at run_trait_graph_audit.py:99-101 (also NEXT_TASKS.md:498); justfile:307's 'on disk and non-empty' canary has the same blind spot as the exit code it replaces, where cached: does not; absolute /Users/marcin template_file in 11 tracked reports including both new ones; two dry-run sidecars.
The two new reports are clean. traitmech:000047 / METPO:1000059 and traitmech:000049 / traitmech:000047 match data/traits/ecology/habitat_association.yaml and host_associated.yaml exactly, definitions included. host_associated's edge table is the best-hedged in the PR: evidence class, DOI, verbatim-marked snippet and curation status per row, taxon-specific edges labelled as such, every edge subject/object resolving to a declared node, and an explicit list of what must not be grounded. The OBO CURIEs are unverified - no offline ontology and none appear elsewhere in data/ to cross-check. cached: false on both, so the spend is real. No data/traits/** change, so no history/ record expected. Could not execute pytest (sandbox declined).
Details in the comment.
The review is right that the four deleted reports will not come back from the sweep that is already running — it computed its pending list before they were removed. My commit message said "the resumable sweep regenerates them", which is true of a future run and not the current one. The fix is not to remember. `just trait-graph-sweep --verify` walks the manifest, reports every `ok` row whose artifact is absent, and exits 1. Free, no calls. It reports 331 today, including all four deletions, and the count falls to 0 as the sweep completes. That is the durable form of a problem this PR has now hit five times in review: an artifact was billed for, is gone, and the manifest says otherwise. 342 rows were in that state before research/ was tracked, and relying on someone noticing is exactly how the first 342 were lost. Section 8 rewritten around the check rather than around a snapshot of numbers that drifts every time the sweep writes a row — which is the 🔵 the review raised twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @realmarcin's task in 2m 48s —— View job Review
🟡
|
There was a problem hiding this comment.
Re-review after 8e1414e. --verify is the right answer to the last item — a check beats a note, and rewriting §8 around the check rather than around numbers that drift every time the sweep writes a row settles the 🔵 I raised twice. One problem, in the same place the last five reviews have been.
--verify is gated behind the credential it exists to not need. run_trait_graph_audit.py:102-104 returns 2 when EDISON_API_KEY/FUTUREHOUSE_API_KEY is unset and --verify does not set args.dry_run, so the verify block at :129 is never reached. In a checkout without an Edison key, just trait-graph-sweep --verify prints a credential error and exits 2 without reading a manifest row. NEXT_TASKS.md:508 sells it as 'free, no calls' and the commit message as the durable replacement for relying on someone noticing; both are true of the block and false of the code in front of it. Its audience is exactly the people who do not hold the key — a fresh clone, just qc, CI. As it stands the only machine that can run it is the one machine that already had all 353 reports, whose sole custody is the thing this PR exists to fix. Two smaller consequences of the same placement: exit 2 (no credential) and exit 1 (artifacts missing) are both nonzero, so a truthiness check reads 'no API key' as '331 artifacts missing'; and target_traits() at :106 YAML-parses every file under data/traits/ plus a run_id printed at :126-127 for a run that will never write a row. Moving the block above :102 fixes all three.
Minor, same block: MANIFEST.open() at :137 is unguarded while the write path four lines later handles a missing file, so an absent manifest gives a traceback where every other path gives a message; for run_id, out in missing[:20] at :146 shadows the run_id from :126 (no effect today, the branch returns); justfile:299-310 does not list --verify among its usage lines, and the recipe comment is the durable copy of how to run this. Carried unchanged: justfile:302 and NEXT_TASKS.md:498 'every call fails instantly' is still contradicted by that same preflight — sharper now, since the preflight it understates is also what breaks --verify; justfile:307's 'on disk and non-empty' canary has the same blind spot as the exit code it replaces, where cached: does not; absolute /Users/marcin template_file in the tracked reports; two dry-run sidecars.
Verified against the tree: 368 ok rows against 23 tracked reports, and all four deleted reports carry ok rows with an output path and no file (July lines 16/18/19/24, August 369/371/374/377) — exactly the set --verify is built to surface, mutualism included. The mechanism is right; it just cannot fire without a credential. No new reports in this commit, so no groundings to check, and no data/traits/** change, so no history/ record expected. Could not execute pytest or the script — the sandbox declined every Python invocation, as in the previous five reviews; the ordering finding is from reading :102-104 against :129.
Details in the comment.
The key check ran at line 102, the verify branch at 129, so an integrity check that makes no calls was refused without EDISON_API_KEY. That made it unrunnable in the two places most likely to notice an `ok` row with no artifact: a fresh clone and CI. Exempted alongside --dry-run. Verified with EDISON_API_KEY, FUTUREHOUSE_API_KEY and EDISON_PLATFORM_API_KEY all unset: reports 329 and exits 1, no credential error. Also moved the run_id computation below the branch — verify was printing a run id for a run that was not happening. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @realmarcin's task in 2m 54s —— View job Review
✅
|
| claim | result |
|---|---|
--verify exempted from the preflight |
✓ — scripts/run_trait_graph_audit.py:106-107 is now if not (args.dry_run or args.verify) and not (...), so the return 2 at :109 is unreachable for a verify run |
| the exit-2/exit-1 conflation is gone | ✓ — a verify run can no longer reach :109, so a nonzero exit from --verify means "artifacts missing" and nothing else |
run_id no longer computed for a run that won't happen |
✓ — moved from :122 to :154, below the return at :146 |
the run_id shadow at the print loop |
✓ — resolved by that move rather than by a rename; :142's loop variable is now the only run_id a verify run binds |
| no import-time cost | ✓ — research_trait.py has no heavyweight or credentialed module-level import, so --verify really is inert |
The one thing from that list that wasn't done is the smallest: target_traits() at :111 still globs and YAML-parses all of data/traits/ before the verify branch, so a verify run pays for a scan it discards and would fail on an unrelated malformed trait file. It doesn't affect the outcome and I'm not asking for it.
Verified against the tree
The manifest is 376 rows / 368 ok / 353 distinct artifact paths, against 23 tracked reports. All four deliberately-deleted reports still carry ok rows with an output path and no file — commensalism 16/369, endosymbiosis 19/371, free_living 18/374, mutualism 24/377 — which is exactly what --verify is built to surface, and §8 now names them as a distinct case owed a second pass. That closes the item from the 5909c761 review: it's a check plus a written record, not a memo.
No new reports in this commit, so no groundings to check. No data/traits/** change, so no history/ record expected.
🔵 Small, all optional
- The reported number is rows, not artifacts.
missingaccumulates one entry perokrow, and 15(category, slug)keys have twookrows, so a path duplicated across the July and August runs is counted twice — today that's 330 distinct missing artifacts reported as ~334. The script's own label says "ok rows", so it isn't lying;NEXT_TASKS.md:503's "reports exactly that set" is the sentence that reads as artifacts. The terminal condition is unaffected — it still reaches 0 exactly when the last file lands. - Nothing invokes
--verifyyet.justfile:644'sqcdoesn't include it, which is right while it exits 1, but the audience §8 names (fresh clone, CI) only gets the check once something calls it. That wiring is the natural companion to the completion commit and isn't recorded anywhere as owed. justfile:299-310still doesn't list--verifyamong its usage lines, and the error text at:108still says "set it or use--dry-run". The recipe comment is the durable copy of how to run this — a free check that a fresh clone can't discover from the place it will look is half-landed.MANIFEST.open()at:133is still unguarded where the write path at:158handles a missing file. Bounded — the manifest is tracked — so a traceback needs someone to have deleted it.- Carried, unchanged:
justfile:302/NEXT_TASKS.md:492"every call in the sweep fails instantly" is still contradicted by the preflight at:106-109;justfile:307's "on disk and non-empty" canary has the same blind spot as the exit code it replaces, wherecached:doesn't; absolute/Users/marcin/...template_filein the tracked reports; twostatus: dry-runsidecars tracked.
Could not execute pytest, just qc, or the script — the sandbox declined every Python invocation, as in all six previous reviews. The --verify findings above are from reading :106-146, and the manifest figures from cut/grep over the TSV, not from running the tool. The commit message's "reports 329 and exits 1 with all three keys unset" is consistent with the code but I did not reproduce it.
Approving. The item I raised last round is closed at its cause, and what's left is documentation and one wiring step that belongs to the commit that lands the completed sweep.
· branch feat/track-research-artifacts
There was a problem hiding this comment.
Re-review after 2c7692f. The ordering fix is complete and correct: --verify is exempted from the credential preflight alongside --dry-run (run_trait_graph_audit.py:106-107), so the return 2 at :109 is unreachable for a verify run and the exit-2/exit-1 conflation is gone; run_id moved from :122 to :154, below the branch's return, which also resolves the shadowed loop variable at :142 without a rename. research_trait.py has no credentialed or heavyweight module-level import, so --verify really is inert. The only piece not moved is target_traits() at :111, which still YAML-parses all of data/traits/ before the branch — discarded work, no effect on the outcome, not asking for it.
Verified against the tree: 376 rows / 368 ok / 353 distinct artifact paths against 23 tracked reports, and all four deliberately-deleted reports carry ok rows with an output path and no file (commensalism 16/369, endosymbiosis 19/371, free_living 18/374, mutualism 24/377). That is exactly the set --verify surfaces, and NEXT_TASKS section 8 now names them as a distinct case owed a second pass — a check plus a written record, which closes the item from the 5909c76 review.
Minor, all optional: the reported number counts ok rows rather than artifacts, and 15 (category, slug) keys have two ok rows, so a path duplicated across the July and August runs is counted twice — 330 distinct missing artifacts reported as ~334. The script's own label says 'ok rows' so it is not lying; section 8's 'reports exactly that set' is the sentence that reads as artifacts, and the terminal condition is unaffected. Nothing invokes --verify yet (justfile:644's qc does not, correctly while it exits 1), so the audience section 8 names only gets the check once something calls it — natural companion to the completion commit. justfile:299-310 still does not list --verify and the error text at :108 still says 'use --dry-run'. MANIFEST.open() at :133 remains unguarded where the write path at :158 handles a missing file. Carried unchanged: 'every call in the sweep fails instantly' at justfile:302 still contradicted by that same preflight; justfile:307's 'on disk and non-empty' canary has the same blind spot as the exit code it replaces, where cached: does not; absolute /Users/marcin template_file in the tracked reports; two dry-run sidecars.
No new reports in this commit, so no groundings to check, and no data/traits/** change, so no history/ record expected. Could not execute pytest, just qc, or the script — the sandbox declined every Python invocation, as in all six previous reviews; the findings are from reading :106-146 and from cut/grep over the TSV.
Details in the comment.
…#233) (#253) * Render the research block by looking for the file the pipeline writes (#233) render_trait_pages.py looked for `research/traits/<cat>/<slug>.md`. The pipeline — and the deep-research-trait skill — write `<slug>-deep-research-<provider>.md`; the suffix is load-bearing, since sweep resume detection is file-existence based on that exact name. So `research_md` was always empty, and the <pre class="research-md"> block in trait.html and its CSS rule were both dead code. 353 reports, not one rendered. Now globs what the pipeline writes and ranks by provider. Ranking, not sorting: `cellulolysis` is the one trait with two providers, and alphabetical order picks `-codex` — the single artifact in the tree with no manifest row and no citations sidecar (#245). Unknown providers still fall back to name order, so an unrecognised provider renders reproducibly rather than by directory order (#228). Previews rather than embeds. A full embed takes pages/ from 16 MB to 31 MB — mutualism.html 36 KB → 80 KB — to store a second copy of text that #240/#241 already track, and makes every future sweep a 353-file diff against the staleness gate. The card is a scrolling 480px <pre>, so nobody was reading a 30 KB report on the page anyway. 60 lines plus a link costs 3 MB. Those 60 lines have to be the answer. The provider echoes the whole rendered prompt twice before responding, so a head-of-file preview shows YAML front matter and the prompt and zero findings. research_answer() trims front matter and everything through the prompt's last line — a marker present exactly twice in all 353 reports — and falls back gracefully for a layout that lacks it. The staleness gate's research guard is inverted rather than deleted. It rejected any research block because research/ was gitignored, and named the three ways out; #240/#241 took "track the inputs", so the collision is gone. What remains is its precondition, so the guard now fires when a block is rendered and research/traits is NOT tracked — the same divergence, reported as a cause instead of an unexplained 353-file STALE. Both branches canaried. The block is labelled for what it is: unreviewed provider output whose suggested CURIEs are measurably unreliable (#243), styled as a warning rather than as more grey metadata. 353 blocks now render. 269 tests pass, `just qc` exits 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Make the caveat read as a warning, and the guard catch the reachable case Four review findings. The caveat was tinted with --accent-soft, which on the same page is already the background of table headers, hover rows, .nn-list chips and .research-card itself — so the thing that had to read as a warning read as decoration, and it shipped no dark-mode rule. It now reuses .graph-warning: the established warning idiom on this template, with a border, a strong lead-in, and overrides in both theme branches. That mattered more than styling usually does, since the warning is the whole mitigation for rendering a corpus with ~80-100 wrong CURIEs (#243). (#256) The line count said "the first 60 of 255 lines" and linked a 417-line file. The count described the trimmed answer, the link the whole report. Now says which it counts and what else the file holds. (#254) research_answer() anchored on the LAST prompt marker. Identical today — the marker appears exactly twice in all 353 reports — but the failure modes are asymmetric: an answer quoting the instruction line, which these reports plausibly do, would be cut mid-answer and silently lose findings, whereas overshooting only leaves boilerplate in the preview. Now anchors on the second, with a test for the quoted-marker case and one asserting the twice-exactly shape the trim depends on, so a layout change fails loudly. (#255) The pages guard tested whether research/traits was tracked at all, which 353 committed reports made unreachable. The reachable divergence is narrower: a curator generates a report, renders, commits pages/ but not the report — qc passes locally, CI renders no block, STALE with the guard silent. It now reports that case separately, names the offending files, and prints the git add that fixes it. Canaried: firing on an uncommitted report names it exactly. 271 tests pass, `just qc` exits 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Narrow the guard to the files the renderer can actually select (#258) The #257 fix over-corrected. With 353 blocks always rendering, the outer condition is always true, so the check had become "any untracked file under research/traits fails qc" — and it said, of each one, that a research block was rendered from it. Not true of a `.citations.md` sidecar, which research_report() explicitly excludes. Now checks only `*-deep-research-*.md` minus sidecars: exactly the candidate set research_report() selects from. The message drops from "CI cannot reproduce it" to "CI may not reproduce this render", since a candidate only changes the output if it wins the provider ranking. Still strict about in-progress sweeps, deliberately — 342 reports were lost to the opposite instinct. The defect was the claim, not the strictness. Canaried in three states: untracked sidecar passes, untracked report fails naming the file, clean tree passes. 271 tests pass, `just qc` exits 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Exclude both citation-sidecar conventions, not just the pipeline's (#259) The tree spells this two ways. The deep-research-client pipeline writes `<report>.md.citations.md`; _edison_capture bundles write `<stem>-citations.md`. The renderer excluded only the dot form. They cannot collide today — research_trait_edison.py builds an `-edison-` stem, so nothing that helper writes lands in the `-deep-research-` namespace. The reason to fix it anyway is the failure mode if that ever changes: a hyphen-form sidecar matches the glob, survives a dot-only exclusion, and for an UNRECOGNISED provider sorts ahead of its own report, because '-' is 0x2D and '.' is 0x2E. The page renders the bibliography as the report, silently, and only for a new provider — precisely when nobody is looking for it. A recognised provider is safe, since RESEARCH_PROVIDERS ranking dominates name order. Confirmed rather than reasoned about: with the old exclusion the candidate set is ['x-deep-research-zeta-citations.md', 'x-deep-research-zeta.md'] and min() picks the citations file. Both conventions are now pinned by a parametrised test that fails against the old code. The justfile's untracked-report guard had the same shape and gets the same fix. 273 tests pass, `just qc` exits 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
deep-research-client copies its --template argument verbatim into every report's `template_file:` front matter, and research_trait.py passed an absolute path. 342 tracked reports therefore recorded one machine's home directory - harmless while research/ was gitignored, wrong for every reader once #240 made these artifacts shared. research_trait_edison.py already did this correctly via _safe_rel, which is why 11 reports were already right. Writer: build_command renders the template relative to the repo root and the child runs with cwd=REPO_ROOT so it resolves. A template outside the repo keeps its resolved absolute path. Backfill: the 342 reports are rewritten rather than left to correct themselves on regeneration, which costs paid API calls and could be deferred indefinitely. One deterministic substitution per file, asserted to occur exactly once. Gate: pr-sanity gains ABSOLUTE_REPO_PATH, keyed on the repo root computed at runtime rather than a /Users/ pattern - so it cannot rot into a check for one person's username, and does not fire on sibling-checkout paths, which have no repo-relative form and are filed separately as #310. Review caught that the cwd pin made one case actively worse: a RELATIVE --template outside the repo took the fallback branch unresolved and would re-anchor to the repo root, reading the wrong file. The output paths had the same hazard with a relative --research-dir. Both are now resolved against the caller's cwd, with a test each driven through monkeypatch.chdir. Canaried: reintroducing an absolute path into one report moved pr-sanity from 0 findings to 1 naming that file and line. Closes #248. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Groundwork for the #183 backfill, and the reason the spend has to happen twice.
research/is now trackedIt was ignored as "large, regenerable". The second half turned out to be the expensive half: 342 of 353 reports were lost because nothing outside one machine held them — the manifest recorded 353 successes against 11 surviving files. Regenerating costs a paid Edison call per trait at ~7.5 min each, so these are provenance, not build output.
~19 MB of markdown when the sweep completes; 824 KB tracked in this PR (the 12 reports currently on disk).
The sweep had no
justrecipe, which is a credential trapscripts/research_trait.pyhas noload_dotenvand says so in its own comment: a run launched outsidejustsees noEDISON_API_KEY, so every call in the batch fails instantly.set dotenv-load := trueis what injects the per-repo.env.That is precisely the "canary you run by hand proves nothing about the batch" failure, so the sweep is now launched through
just trait-graph-sweep, and the recipe comment carries the paid warning and the canary instruction for whoever runs it next.Canary, verified on side effects rather than the exit code
One real unit, same launcher as the batch:
cached: falseis the line that matters — an exit code of 0 would have looked identical for a cache hit.The batch is running
341 remaining at
--workers 4, launched through the new recipe. Sequential would be ~43 h; at 4 workers ~11 h.Cost per call is not captured anywhere in the sidecar —
duration_secondsis recorded but no USD figure — so I can report time but not spend. Worth adding if this is repeated.just qcexits 0.🤖 Generated with Claude Code