feat(verifier): check the deliverables the plan declared - #211
Open
lfnothias wants to merge 6 commits into
Open
Conversation
The rubric cache exists so "verifier scores stay comparable across iterations
of the same task" (verifier_claim_cache_test.py) — the first run freezes the
ranked claim list and later runs reuse it verbatim (VerifierEvaluator.evaluate).
It keys on sha256(goal). But the goal the verifier reads is the
knowledge-wrapped one, and the retry loop prepends the previous attempt's whole
answer dict to it before re-running the same step, so every retry hashes a
longer string, misses the cache, and writes a fresh rubric.
Measured on the run this came from (p_iimn task_001, step
task_and_resource_discovery, three attempts):
original_task_<uuid>.txt 2657 B, md5 02b411d9… — identical all three
goal_<uuid>.txt 2657 → 6505 → 11353 B — grows every attempt
key(goal) a19ee032 / d11aa80f / 3530d233 — 3 rubrics
key(original) a19ee032 / a19ee032 / a19ee032 — 1 rubric
The three attempts drew 21, 24 and 20 claims with disjoint ids and scored
0.000 / 0.649 / 0.557 — three numbers on three different scales, which
evolution_engine then compares with max() and reports as "Best run".
_extract_claims now takes cache_key_text separately from goal, so the claim
writer still sees the full wrapped goal (prior-attempt context is useful in the
prompt) while the cache keys on the invariant task. WorkflowInfo already keeps
that unwrapped task "for similarity matching", and record_lineage and the
variation prompts already prefer `original_task or goal`; the verifier's key was
the one place that did not. Default is falsy, so every other caller is unchanged.
The 16 failing tests in the suite (evaluation_cli, failure_fingerprint, pricing,
logprobs, csv_mode_logging) fail identically at 67b4251 with these two files
reverted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit f0c5afd)
Issue #196. Step reproduction_spec_analysis declared workspace/analysis/iimn_reproduction_plan.md, wrote iimn_reproduction_guide.md at the workspace root instead, was scored 0.799 and accepted, and then killed the run at the next step's dependency gate. None of the 26 claims in its frozen rubric mentioned the declared path — claims are extracted partly from execution text and workspace listing, so the rubric inherited the agent's choice of deliverable and could not catch the agent choosing wrong. expected_outputs is the anchor: the planner writes it before the step runs, so it cannot have been shaped by execution. The verifier never saw it — it is handed a uuid, and neither state_result.json nor the workflow folder carries the declaration. Threading it through start_workflow_evolution, the generation loop, IndividualRun and the factory would touch five shared signatures. It is not needed: the planner already passes original_task=step_task, and that is the same string the verifier keys its rubric cache on. So the declaration is written under that key and read back under it — one write in the planner, one read in the verifier, no signature changed. The claims are prepended, so claims[:max_claims] cannot drop them, and added after _extract_claims has persisted the rubric, so they never enter the frozen cache: the cache freezes what a run produced, these belong to the plan and must follow it when it changes. Every failure path degrades to the previous behaviour — no declaration, unreadable file, or an exploding wf_info costs the extra claims, never the evaluation. One design question is deliberately left open in #196 rather than settled here: this makes the score partly a function of the planner's path conventions, so a step that produces an equivalent artefact under another name now fails a maximum-importance claim. That is the intended reading of "declared output", but it is Martin's call. 17 tests, including the one that pins planner and verifier to the same key function — if those drift the declaration is written where nothing reads it, and a silent stop is the failure mode this exists to end. Suite: 369 pass; the same 16 failures are pre-existing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 8be2b35)
Issue #196. Both inner branches of the acceptance block assigned TaskStatus.COMPLETED and broke, differing only in print_ok against print_warn: outputs_produced, missing_outputs = self._verify_expected_outputs(step) step.status = TaskStatus.COMPLETED if outputs_produced: print_ok(...); break else: print_warn(...); break So _verify_expected_outputs computed the right answer and nothing acted on it. Observed on a real run: step reproduction_spec_analysis was accepted at 0.799 without writing workspace/analysis/iimn_reproduction_plan.md, and the run died at the *next* step's dependency gate — with a third attempt still unspent. Now: while attempts remain, spend one on producing the deliverable rather than banking the miss. On the last attempt keep COMPLETED, deliberately, so the dependency gate still reports which output is missing for which step instead of a generic step failure replacing that precise message. PlanStep.missing_outputs carries the miss so COMPLETED alone no longer implies the deliverable exists. Checked the tests first, per the standing rule: headless_planner_test covers _verify_expected_outputs and planner_test covers _can_execute_step, but nothing asserted what run_attempts does with the result. Uncovered, not deliberate. Interacts with 8be2b35, as noted on the issue: mandatory expected_outputs claims make a name mismatch louder, this makes it consequential. A step that produced the right content under the wrong name now retries. Whether that is the desired reading of expected_outputs is Martin's call and is flagged there, not assumed here. 9 tests. Suite: 378 pass; the same 16 failures are pre-existing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit dde7b20)
Caught in production on run 6. Step `data_acquisition` declared two outputs:
workspace/p_iimn_task_001/data/
workspace/p_iimn_task_001/data/dataset_inventory.csv
as_claims worded both identically — "A file exists at that path, in the
workspace, and is non-empty" — so the directory became a maximum-importance
claim no correct run could satisfy. A step that did exactly what the plan asked
would have been marked down for it, which is the opposite of what 8be2b35 is
for.
Planner._verify_expected_outputs already makes this distinction (it short-
circuits a trailing-separator output through _directory_output_satisfied). The
claim now makes it too: a directory output asks for a directory holding at
least one non-empty file.
The existing test only asserted the claim's id, which was correct for both
kinds and so said nothing about the description. Four tests now cover the
distinction, including the exact pair run 6 recorded.
Suite: 382 pass; the same 16 failures are pre-existing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit 6f0d9c1)
Confirmed against this fix in production, and it went the wrong way.
Run 6 step `feature_detection_and_processing` declared five outputs. No raw
LC-MS data was reachable, so the run wrote each file as a labelled placeholder
— line 2 of feature_table_qtof.csv reads
# PLACEHOLDER: MZmine Processing Output for qTOF (MSV000080492)
# Status: NO REAL DATA AVAILABLE
— and all five mandatory claims PASSED at importance 10, one on the detail line
"File exists and is non-empty (1462 bytes)". So 8be2b35 contributed 50
importance-points of passes for files that state in their own second line that
they contain no data.
The wording was the defect. "A file exists at that path and is non-empty" is
satisfied by a stub, which means the claim pressures an agent to create the file
without pressuring it to fill the file. Under five such claims a run that cannot
obtain its inputs is pushed toward writing something — here honest placeholders,
but fabrication would have satisfied the claim equally well, and this fix would
have been the cause.
The claim now asks for content and names the failure mode: not a placeholder,
stub, template, or a note recording that the data could not be obtained.
The step still failed overall (0.552, hard_fail_capped), so other claims caught
what these missed — but these were on the wrong side of it.
Suite: 384 pass; the same 16 failures are pre-existing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit 696bae2)
Third defect in this fix, and the first one that punished honesty. 696bae2 disqualified "a file whose body announces missing or unavailable data". An A/B probe against run 6's archived workspace applied that to processing_log.md — 292 lines, per-dataset MZmine parameters, a parameter comparison table and a recovery path, the most substantive artefact in that workspace — and failed it, because the log honestly recorded that its *sibling* outputs were placeholders. The wording could not tell a file that IS a stub from a file that REPORTS on one, so it penalised exactly the behaviour the mechanism exists to protect. Now split by suffix, the same discriminator asb_eval already uses: .csv .tsv .mgf .json .parquet .mzml .mztab "holds actual records — data rows, spectra, or entries — and not merely comments, headers, or placeholder text standing in for data" everything else "substantive content produced by this step... A report that documents what was attempted and honestly records missing inputs or limitations does satisfy this claim" Verified against the shipped wording on run 6's real artefacts: stub .mgf fail "only placeholder text; no actual spectral records" stub .tsv fail "only header and/or comments, no actual records" stub .mgf fail "contains placeholder markers" real .md pass "substantive (size > 1KB, non-empty lines)" and previously, on the .csv that started this: old wording pass, new wording fail ("could not be parsed as CSV: No columns to parse from file"). Four tests added; two earlier ones updated, since they asserted the single wording this deliberately splits. 27 on the module, suite 388 pass, the same 16 failures pre-existing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 6524cf6)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Slice 3 of 4 from #197. Six commits cherry-picked unchanged from
mimosa_v2_lfx. Addresses #196.Problem
The verifier never saw what the plan asked for. A step declared
workspace/analysis/iimn_reproduction_plan.md, wrote a differently named file at the workspace root, scored 0.799, was accepted, and killed the run at the next step's dependency gate. None of the 26 claims in its rubric mentioned the declared path, because claims are extracted from execution text and the workspace listing, so the rubric inherited the agent's choice of deliverable.Two related defects: the rubric cache keyed on the knowledge-wrapped goal, which grows on every retry, so retries of the same step were scored against fresh rubrics; and the acceptance block assigned
COMPLETEDon both branches of the missing-output check, so the check computed the right answer and nothing acted on it.Solution
sources/core/declared_outputs.py: the planner'sexpected_outputs, written before the step runs, become mandatory claims the verifier checks alongside the extracted rubric.COMPLETEDso the dependency gate still names the missing output..csv,.tsv,.mgf,.json,.parquet,.mzml,.mztab) must hold records rather than placeholder text; a report that describes missing data in its siblings is not itself a placeholder.Testing
Fresh
uv sync --group dev --python 3.11on this branch:The 17 failures are the pre-existing set from #205. This slice adds 42 passing tests:
declared_outputs_test,planner_missing_outputs_test,verifier_rubric_key_retry_test.Backwards compatibility
Steps without
expected_outputsare verified as before. Merges cleanly with #206, #207 and the other slices (checked withgit merge-tree).🤖 Generated with Claude Code