claudiomaradonna/agent-foundry
claudiomaradonna/agent-foundry
Template
1
0
Fork 0
Code Issues 5 Pull requests Activity

perf: cut reviewer effort and unused tools #138

Merged
claudiomaradonna merged 3 commits from feat/cut-review-cost into main 2026-10-18 09:05:23 +02:00
Conversation 0 Commits 3 Files changed 14 +305 -18
motoko commented 2026-10-18 09:02:14 +02:00
Collaborator
Copy link

Why

The context breakdown of the last eval run found two costs the policy gets little back for:

  • Reviewer thinking. Output tokens are about 44% of a run's cost, and subagents write 77% of them. Each change-reviewer and security-reviewer run, at effort: xhigh, wrote 10k to 19k tokens, almost all thinking, to produce a report of about 1k.
  • Unused tool definitions. Every main-session call carries 4.9k tokens of definitions for Workflow, ScheduleWakeup, ReportFindings, and ListAgents, which no skill uses.

No eval showed whether a reviewer still catches defects at lower effort, so that change couldn't be judged before.

What changed

  • A new review-pr eval case. The agent reviews a contributor branch that adds --min N, with a boundary bug and an awk code injection seeded in it. The case passes only when the change-reviewer's own finding items name the bug and the security-reviewer's name the injection. run.sh gains a case branch/, committed on contrib.
  • Claude reviewers at effort: high. Over three attempts at each effort, both reviewers named their defect every time at high and at xhigh. Reviewer output fell from about 16.7k to 5.6k tokens, and the case cost fell from $1.61 to $0.98. The Codex and OpenCode adapters keep xhigh, since nothing measures them.
  • The four tools denied in .claude/settings.json. That also turns off Claude Code's own /loop, Workflow runs and /code-review findings, and a user's own settings can't allow them again. The README says so, and a repo that needs one deletes its line. A new UPGRADING.md migration tells an upgrading user before the merge and keeps out the denial of each tool they want to keep.

How to verify

  • jq .permissions.deny .claude/settings.json lists the four tools.
  • evals/run.sh --policy HEAD --case review-pr --repeat 3 passes. As the README now says, read the reviewers' reports before letting a pass decide effort.

Run on the tip, all passing:

  • bash tests/verify-agent-config.sh (256), bash tests/check-work-folder.sh (46);
  • .agents/hooks/shellcheck.sh repo, prettier --check '**/*.md', scripts/verify-agent-config.sh, git diff --check main.

Evals, tip against main: 7 of 7 on both, $7.16 against $10.30 (single attempts). The first turn of a session is 4.8k tokens smaller.

Considerations

  • Easy defects. The seeded defects may be easy enough to catch at any effort. The evidence holds for these two, not for subtler bugs.
  • Word-matching grader. The grader matches the words of findings, and that can't fully tell "found it" from "mentioned it". Four review rounds tightened it. You chose to keep it failing safe and to read the reports before an effort or model decision. I read all 16 reports behind the effort change, and every catch is real.
  • Settings file removed on purpose. A repo that deleted .claude/settings.json would still get the migration's notice once. I set that aside as rare and harmless.
  • Your call: the reviewer reports in the work folder are kept as given this time.
## Why The context breakdown of the last eval run found two costs the policy gets little back for: - **Reviewer thinking.** Output tokens are about 44% of a run's cost, and subagents write 77% of them. Each `change-reviewer` and `security-reviewer` run, at `effort: xhigh`, wrote 10k to 19k tokens, almost all thinking, to produce a report of about 1k. - **Unused tool definitions.** Every main-session call carries 4.9k tokens of definitions for `Workflow`, `ScheduleWakeup`, `ReportFindings`, and `ListAgents`, which no skill uses. No eval showed whether a reviewer still catches defects at lower effort, so that change couldn't be judged before. ## What changed - **A new `review-pr` eval case.** The agent reviews a contributor branch that adds `--min N`, with a boundary bug and an `awk` code injection seeded in it. The case passes only when the change-reviewer's own finding items name the bug and the security-reviewer's name the injection. `run.sh` gains a case `branch/`, committed on `contrib`. - **Claude reviewers at `effort: high`.** Over three attempts at each effort, both reviewers named their defect every time at `high` and at `xhigh`. Reviewer output fell from about 16.7k to 5.6k tokens, and the case cost fell from $1.61 to $0.98. The Codex and OpenCode adapters keep `xhigh`, since nothing measures them. - **The four tools denied** in `.claude/settings.json`. That also turns off Claude Code's own `/loop`, `Workflow` runs and `/code-review` findings, and a user's own settings can't allow them again. The README says so, and a repo that needs one deletes its line. A new `UPGRADING.md` migration tells an upgrading user before the merge and keeps out the denial of each tool they want to keep. ## How to verify - `jq .permissions.deny .claude/settings.json` lists the four tools. - `evals/run.sh --policy HEAD --case review-pr --repeat 3` passes. As the README now says, read the reviewers' reports before letting a pass decide effort. Run on the tip, all passing: - `bash tests/verify-agent-config.sh` (256), `bash tests/check-work-folder.sh` (46); - `.agents/hooks/shellcheck.sh repo`, `prettier --check '**/*.md'`, `scripts/verify-agent-config.sh`, `git diff --check main`. Evals, tip against main: 7 of 7 on both, $7.16 against $10.30 (single attempts). The first turn of a session is 4.8k tokens smaller. ## Considerations - **Easy defects.** The seeded defects may be easy enough to catch at any effort. The evidence holds for these two, not for subtler bugs. - **Word-matching grader.** The grader matches the words of findings, and that can't fully tell "found it" from "mentioned it". Four review rounds tightened it. You chose to keep it failing safe and to read the reports before an effort or model decision. I read all 16 reports behind the effort change, and every catch is real. - **Settings file removed on purpose.** A repo that deleted `.claude/settings.json` would still get the migration's notice once. I set that aside as rare and harmless. - **Your call:** the reviewer reports in the work folder are kept as given this time. 
Adds the review-pr case: a contributor branch adding --min with a
boundary bug and an awk code injection, passing only when the
change-reviewer and security-reviewer reports name them. run.sh commits
a case's branch/ on contrib, and grade-lib.sh reads what each
subagent reported from its transcript. It lets a reviewer's effort be
judged by its catch rate.
Workflow, ScheduleWakeup, ReportFindings, and ListAgents add about
4.9k tokens of definitions to every main-session call: a one-word
prompt starts at 19,847 tokens, and at 14,947 with the four denied. The
README says what the denial turns off and how a repository gets a tool
back, and an UPGRADING.md migration tells an upgrading user before the
merge and keeps out the denial of each tool they keep.
On the review-pr eval case, three attempts at each effort, each
reviewer named its seeded defect every time at high as at xhigh, while
the reviewers wrote about 5.6k output tokens a run against 16.7k, and
the case cost 0.98 USD against 1.61. The Codex and OpenCode adapters
keep their effort, since the evals run Claude Code only.
claudiomaradonna deleted branch feat/cut-review-cost 2026-10-18 09:05:26 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
claudiomaradonna/agent-foundry!138
No description provided.