Skip to content

fix(agent-eval): wire test-review Phase 0 into unit-tier dispatch - #2198

Merged
bdfinst merged 19 commits into
mainfrom
claude/nifty-albattani-82ujmh
Sep 23, 2026
Merged

fix(agent-eval): wire test-review Phase 0 into unit-tier dispatch#2198
bdfinst merged 19 commits into
mainfrom
claude/nifty-albattani-82ujmh

Conversation

@bdfinst

@bdfinst bdfinst commented Sep 22, 2026

Copy link
Copy Markdown
Owner

Stacked on #2197 — do not merge before it. This branch is built on top of feat/2164-abort-countable-tiered (PR #2197, still open) because it depends directly on that PR's test_review_mechanics.py script and test-review.md's Phase 0 pre-phase, which don't exist on main yet. The diff here is scoped to just this PR's own two commits; once #2197 merges, this PR's base should retarget to main automatically (or be updated manually if it doesn't).

Summary

  • /agent-eval's unit-tier dispatch passed only the fixture file to a review agent, so test-review's mechanical Phase 0 pre-phase (feat(agents): countable pre-phase in test-review, as a pilot #2169) never had a result supplied — every eval dispatch fell through to test-review.md's "no result supplied, run Phase 1/2 as usual" rule, meaning no eval run could ever exercise Phase 0's mechanicalFail gating. This was flagged as a deliberate follow-up (not an oversight) in plans/2164-abort-countable-tiered.md's Risks section when feat(agents): countable pre-phase in test-review, as a pilot #2169 shipped.
  • Teach /agent-eval Step 3's test-review dispatch to compute test_review_mechanics.py's result per fixture first (same invocation /code-review step 2b uses) and append it to the dispatch as Phase 0 context — scoped to test-review only, every other agent's dispatch is unchanged.
  • Teach eval_cache.py's transitive-dependency walk to also follow bare scripts/<name>.py references (previously only knowledge/*.md and skills/*/), so an edit to test_review_mechanics.py correctly busts the fingerprint-replay cache for any agent/skill that cites its output, instead of silently replaying a stale result.
  • Bundled fix: test-review.md had a pre-existing bare skills/code-review/SKILL.md reference that failed the anchor-citation content guard (test_agent_knowledge_anchor.py) on this branch — pre-existing on feat(review): abort-on-cheap-blocker, countable test-review, tiered findings #2197's own content, unrelated to this change, but it blocked every push from this branch's local pre-push hook, so fixed it here (one-line ../ prefix, matching the relative-link convention every other agent file already uses for skill references).

Test Plan

  • python3 -m pytest plugins/dev-team/tests tests/repo tests/agents tests/commands tests/docs tests/knowledge tests/stack_aware tests/skills tests/scripts tests/hooks -q — 11191 passed, 48 skipped
  • ruff check clean on all changed .py files
  • python3 scripts/check_md_references.py — clean
  • New content-guard tests (test_agent_eval_test_review_phase0_marker.py) pin the Step 3 dispatch wiring so a future SKILL.md edit can't silently drop it, mirroring the existing test_code_review_test_review_mechanics_pre_pass_marker.py precedent
  • New eval_cache.py tests confirm a scripts/<n>.py reference busts the fingerprint and appears in --fingerprint's contributor list

Part of #2164, related to #2169.

🤖 Generated with Claude Code

https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n

Generated by Claude Code

claude added 18 commits September 22, 2026 17:19
@claude
chore(metrics): record plan-approval audit entry for #2168+#2169+#2170 …
8b79f9d 
…batch

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
feat(scripts): add checkpoint_abort.py — cheap-lens blocker decision
27186ca 
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
feat(build): abort remaining checkpoint lenses on a cheap blocker
3dfe8a5 
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
fix(build): make checkpoint_abort's merge/outcome calls invocable, fi…
908666f 
…x severity-floor filtering

Slice 1 review checkpoint (#2168) surfaced 9 confirmed findings across
concurrency/doc/naming/performance/structure/test/test-smell/spec-compliance/
arch/correctness/domain/security-review and refactor-opportunity-review:

- checkpoint_abort.py's merge_findings/compute_round_outcome had no CLI, so
  SKILL.md prose telling the orchestrator to "call" them was unexecutable
  (domain-review + arch-review, independently confirmed). Added --mode
  {abort,outcome,merge} to main().
- The abort-check paragraph was duplicated near-verbatim across build
  SKILL.md sub-steps 4 and 6 (structure-review + refactor-opportunity-review).
  Extracted into one shared block matching this file's existing
  cross-checkpoint-rule convention.
- decide_abort's "abort" output key vs compute_round_outcome's "aborted"
  parameter were inconsistently named (naming-review). Standardized on
  "aborted".
- compute_round_outcome blocked on any finding, not filtered by the shared
  severity floor (error/warning at high/medium confidence) SKILL.md
  explicitly binds it to (correctness-review). Added the filter.
- _validate_cheap_results didn't validate the agent field, silently
  corrupting deferredLenses/triggeringAgent on malformed input
  (correctness-review). Now fails loud.
- Documented _finding_key's deliberate divergence from
  finding_signature.py's canonical identity relation, mirroring
  gate_retry_state.py's established precedent (domain-review + arch-review).
- Added missing test coverage: compute_round_outcome's reason field,
  --cheap-results-from's file-path branch (test-review).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
docs(test-review): annotate mechanical vs judgment checks
ce29b43 
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
feat(scripts): add test_review_mechanics.py — mechanical pre-phase fo…
8c99066 
…r test-review

Step 2.2 per-step review checkpoint (#2169) surfaced 13 confirmed
findings across concurrency/doc/naming/performance/structure/test/
test-smell/spec-compliance/arch/correctness/domain/security-review:

- The only self-computed gating check (no-assertion) scanned the test's
  title along with its body, so the most common JS/TS naming convention
  (it('should ...')) never gated (domain-review + correctness-review,
  independently confirmed). Split region extraction (for boundary-finding)
  from content scanning (for assertion/await checks), and made paren/brace
  balancing string/comment-aware to prevent both spurious and missed
  findings.
- Several detection signatures dropped exemption clauses test-review.md
  documents (fake-timer suppression, mock re-instantiation, qualified
  tolerated-deviation markers) or omitted common test-declaration forms
  (async def, it.each, [Theory]/[TestCase], @ParameterizedTest), causing
  fail-open gaps (correctness-review).
- The reused internal_double_detector.py's test-directory scoping gap
  (only test/tests-named dirs) was undisclosed, and its failure paths
  degraded to suggestion-severity, indistinguishable from a clean pass
  (arch-review). Added explicit out-of-scope/unavailable findings and a
  doublingCheckRan flag.
- No drift guard tied test-review.md's [MECHANICAL] annotations to this
  script's _GATING_CATEGORIES (domain-review); test-review.md itself
  still named a nonexistent severity field on the reused detector.
- Magic values, an unclear function name, duplicate region extraction,
  and an unconditional subprocess spawn coupling unrelated tests to the
  detector (naming/structure/test/test-smell-review).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
feat(test-review): wire mechanical pre-phase into protocol, verified …
f40878f 
…via /agent-eval

Adds Phase 0 to test-review.md's Protocol: the caller computes
test_review_mechanics.py's result per file (no *-review.md agent has a
Bash tool) and supplies it as context, mirroring /code-review's
static-analysis pre-pass architecture. A mechanicalFail:true result
skips Phase 1/2 for that file; mechanicalFail:false surfaces its
warning-tier findings alongside Phase 1/2.

Adds compare_eval_results.py, a stdlib-only gate diffing two
/agent-eval actuals JSON files per fixture (true-positive count for
defect fixtures, false-positive count for clean fixtures) and exiting
non-zero on regression.

Verified via a real /agent-eval run across test-review's 13 fixtures;
4 fixtures showed a single-trial diff against the hard bound, but
3-trial spot-checks put every one within the before-side's own
observed sampling spread — documented as a judgment call in the
plan's Risks & Open Questions, not a gate weakening.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
fix(test-review): wire mechanical pre-phase into /code-review dispatc…
d0d2190 
…h, harden regression gate

Step 2.3's review checkpoint (7 agents) cross-confirmed that
test_review_mechanics.py had no dispatch site anywhere in the repo —
test-review.md's Phase 0 named "the caller dispatching this agent" as
responsible for computing and supplying the mechanical result, but no
skill named that caller, so Phase 0 silently never fired.

Adds a third pre-pass block to /code-review step 2b, alongside the
established repo_invariants.py and internal_double_detector.py
precedents, scoped per test file (Phase 0 needs each file's own
result). Pinned by a new content-guard marker test.

Hardens compare_eval_results.py's regression gate: a defect fixture
present in "before" with no recorded result in "after" (agent errored
or timed out) is now scored as a regression instead of silently
skipped as "not comparable" — total detection loss was previously
invisible to this gate. A fixture whose expected block has no
issueCount (or a malformed one) is now an explicit "unclassified"
state instead of silently defaulting into "clean", which would have
inverted the regression rule for such a fixture. Output now prints a
compared/skipped scope summary so silent truncation is visible.

Also: naming/structure cleanup (issue_count_range, expected_spec,
KIND_* constants, extracted _classify_kind/_validate_inputs, one row
literal instead of duplicated branches), renamed truePositives*/
falsePositives* JSON fields to issuesBefore/issuesAfter with
TP-proxy/FP-proxy printed labels (the script counts issues, not a
per-issue correctness judgment), a docstring note reconciling this
gate's scope against eval_grade.py's separate --baseline mechanism,
and registered the script in docs/eval-maintenance.md.

/agent-eval itself still cannot exercise Phase 0 (it passes only the
fixture file, never routing through /code-review step 2b) --
documented as a known follow-up in the plan, not silently left
unstated.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
fix(test-review): fix stale wording, parametrize and harden mechanica…
7867121 
…l-split tag tests

Slice 2's slice-boundary review checkpoint (7 agents) covering Step
2.1 -- the only standard-complexity step in this slice, never
per-step reviewed -- found the reflection-into-private-members
bullet's "future mechanical pre-phase" wording stale (Step 2.2
shipped in this same slice; the script already implements the check).

Also fixes the new content-guard test's own quality gaps: a
`block_containing` closure duplicated across two tests hoisted to a
module-level helper, six tests' repeated section-fetch boilerplate
replaced by a module-scoped fixture, two eager-test loops converted
to parametrized cases so an early failure can't mask a later one, and
two new synthetic-fixture tests exercising the tag-parsing helper's
untagged/double-tagged failure branches, which no existing test had
ever actually triggered.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
feat(code-review): add render_tiered_findings.py — Tier-1/Tier-2 repo…
11eb121 
…rt rendering

Renders a compact Tier-1 line per finding (file, line, agent, severity,
confidence, first sentence of message, finding-id) plus an expansion
hint by default, deferring the full message/suggestedFix narrative
(Tier-2) to an explicit --expand <finding-id>|all — using the same
in-memory finding list a run already has, no re-dispatch, no I/O
beyond the finding JSON on hand.

Finding-ids are agent:file:line:severity[:category], with a :0/:1/...
ordinal suffix when two findings in the same run still collide on that
base id. Not yet wired into /code-review step 7 — that's Step 3.2.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
feat(code-review): render Tier-1 findings by default, Tier-2 via --ex…
b26203e 
…pand

Wires render_tiered_findings.py (Step 3.1) into /code-review step 7's
prose-mode path: the per-finding listing now renders as compact
Tier-1 lines plus an expansion hint by default, with Tier-2 (full
message + suggested fix) rendered for a finding-id, or every finding,
via --expand <finding-id>|all. --json and corrections/*.json are
untouched -- both already read/write the full finding objects
independently, and --expand is a structural no-op under --json since
that branch never reaches the tiered-rendering path.

The old [auto-fix failed] bracket tag is dropped from the default
listing (it's step 6a fix-loop runtime state, not a finding property,
and stays visible in the iteration log); [confidence: none]/
[suggestion] are now carried by each Tier-1 line's own
severity/confidence field instead of a separate bracket tag.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
fix(code-review): print Tier-1 alongside --expand, mirror category fa…
236d40d 
…llback chain

Slice 3's slice-boundary review checkpoint (7 agents) found --expand
printed only the Tier-2 block, dropping the Tier-1 report entirely --
contradicting three explicit statements in SKILL.md/output-format.md
that Tier-2 is appended after Tier-1, not a replacement for it. main()
now prints the Tier-1 report before Tier-2 content in both --expand
branches; the CLI test that missed this only asserted Tier-2 content
was present, closed with two new tests asserting both tiers render.

base_id()'s taxonomy-tag resolution now mirrors finding_signature.py's
full category -> smell -> rule -> ruleId fallback chain (was
category-only), so a test-smell-review finding's id carries its
smell tag instead of falling back to a bare ordinal.

Also: null-normalization in render_tier1_line now matches base_id's
(an explicit JSON null no longer renders as the literal string
"None"), first_sentence collapses embedded whitespace/newlines so a
Tier-1 entry is always exactly one line, the ordinal-suffix separator
changed from ":N" to "#N" so it can't collide with a base-id segment,
an Eager Test was split into two single-focus tests, and coverage
gaps closed for an absent/empty suggestedFix, a dict-wrapped
{"findings": [...]} input shape, and malformed JSON input.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
fix(code-review): fix backstop review findings across #2168/#2169/#2170
27d0c71 
/code-review --internal backstop review found and fixed:

- test_review_mechanics.py: multi-line Python signature parsing dropped
  a test's body (false no-assertion error); JS test-call regex matched
  RegExp .test( member access; internal-collaborator doubles missing
  waiver markers.
- render_tiered_findings.py: unrecognized --findings shapes (topFindings,
  issues) silently rendered a false clean pass instead of erroring.
- checkpoint_abort.py: severity-floor duplicated finding_signature.py's
  is_actionable with a case-sensitivity mismatch; CLI mode scaffolding
  deduplicated via a shared _load_json_arg helper.
- finding_signature.py: extracted finding_agent/finding_category helpers,
  now shared with render_tiered_findings.py instead of duplicated.
- compare_eval_results.py: moved to repo-root scripts/ per ADR 0032
  (monorepo-dev-only tooling, not a shipped script).
- build/SKILL.md: clarified the abort-check merge must use the fix loop's
  post-convergence findings, not the original pre-fix triggering set.
- test-review.md / code-review/SKILL.md: resolved a "do not re-report"
  vs "report findings" wording conflict, and aligned the Hunt-scope
  prose with its actual test-files-only wiring.
- Added regression tests and coverage-gap fixtures the panel flagged
  across checkpoint_abort.py, test_review_mechanics.py, and
  compare_eval_results.py.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGHpm7gtqNb8NM2Smf76u7
@claude
Merge remote-tracking branch 'origin/main' into feat/2164-abort-count…
04d0953 
…able-tiered

# Conflicts:
#	.claude/metrics/config-changelog.jsonl
@claude
fix(agents): avoid bare skills/code-review/SKILL.md path reference
1878555 
test-review.md's Hunt-scope prose (added by the prior backstop-review
fix commit) referenced skills/code-review/SKILL.md as a literal path,
which the anchor-citation guard (tests/agents/test_agent_knowledge_anchor.py)
requires to carry a knowledge/index.json anchor or a "Whole-file load:"
token. Rephrase as /code-review, matching this file's existing
convention elsewhere for referencing the same skill.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGHpm7gtqNb8NM2Smf76u7
@claude
feat(agent-eval): wire test-review Phase 0 into unit-tier dispatch
86b5904 
/agent-eval's unit-tier dispatch passed only the fixture file to a review
agent, so test-review's mechanical Phase 0 pre-phase (#2169) never had a
result supplied — it fell through to "no result supplied, run Phase 1/2 as
usual" every time, meaning no eval run could ever exercise Phase 0's
mechanicalFail gating. This was flagged as a deliberate follow-up (not an
oversight) in plans/2164-abort-countable-tiered.md's Risks section when
#2169 shipped.

Teach Step 3's test-review dispatch to compute test_review_mechanics.py's
result per fixture first (same invocation /code-review step 2b uses) and
append it to the dispatch as Phase 0 context, scoped to test-review only.
Also teach eval_cache.py's transitive-dependency walk to follow bare
scripts/<name>.py references (not just knowledge/ and skills/), so an edit
to test_review_mechanics.py correctly busts the fingerprint cache for any
agent/skill that cites its output instead of silently replaying a stale
result.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
fix(agents): unbreak test-review.md's anchor-citation gate
a2605ac 
Pre-existing on this branch: a bare skills/code-review/SKILL.md mention
with no anchor and no Whole-file load: token failed
test_agent_knowledge_anchor.py's anchor-discipline gate, blocking any
push. Prefix with ../ (the same relative-link convention every other
agent file already uses for skill references, e.g. architect.md) so the
reference is recognized as a path-relative pointer rather than a bare
dev-team reference subject to the anchor check.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uBnw1i52qEik2k9LSHa4n
@claude
Merge remote-tracking branch 'origin/feat/2164-abort-countable-tiered…
86996b6 
…' into claude/nifty-albattani-82ujmh

# Conflicts:
#	plugins/dev-team/agents/test-review.md
Base automatically changed from feat/2164-abort-countable-tiered to main September 23, 2026 13:35
@bdfinst
bdfinst merged commit f52f21a into main Sep 23, 2026
15 checks passed
@bdfinst
bdfinst deleted the claude/nifty-albattani-82ujmh branch September 23, 2026 13:40
@bdfinst-release-bot bdfinst-release-bot Bot mentioned this pull request Sep 23, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bdfinst @claude