Skip to content

feat(evals): agent evals harness for before-and-after comparisons - #1274

Open
andychoquette wants to merge 17 commits into
aws-deadline:mainlinefrom
andychoquette:feature/agent-evals
Open

feat(evals): agent evals harness for before-and-after comparisons#1274
andychoquette wants to merge 17 commits into
aws-deadline:mainlinefrom
andychoquette:feature/agent-evals

Conversation

@andychoquette

@andychoquette andychoquette commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Add evals/, a dev tool that measures how well an AI agent achieves goals with Deadline Cloud tooling and docs — and closes the loop: it can generate an improvement to what the agent relied on, then A/B-prove that the change actually helps before anyone reviews a patch.

Module map

  • agent_evals/harness.py — runs one isolated, headless agent (claude -p) per case in a throwaway sandbox, restricted to the case's tools, capturing tool/turn/cost telemetry from stream-json. Wraps launch in a timeout + error guard so one bad run can't hang or abort the batch.
  • agent_evals/judge.py — grades the final answer against a plain-language rubric (LLM-as-judge, tool-less single call), so a new eval domain needs a rubric string, not new code. Coerces the verdict safely so a stringified "false" can't become a silent pass.
  • agent_evals/subject.py — the material under test is any git checkout + pathspec. repo_subject() A/Bs this checkout (CLI src/ or docs); corpus_subject() seeds fetched material into a throwaway repo. Git failures raise, a dirty checkout refuses to run, and destructive resets are scoped to the owned paths so operator work is never discarded.
  • agent_evals/runner.py — JSON eval files, k runs per variant. --revised-ref runs each case paired (baseline vs revised); proposal.patch is emitted only when the change improved at least one eval and regressed none. Also gates real_aws cases (below).
  • agent_evals/reviser.py — closes the loop: an agent edits the subject from a struggling run's transcript onto a scratch ref, which feeds back into --revised-ref.
  • examples/ + README.md — starter evals for the deadline CLI, AWS CLI usage, a docs page, and a real-AWS job submission; the operator guide.

Isolation rule: the tested agent always runs as a separate claude -p subprocess; the orchestrating session never performs the goal itself, so the telemetry measures the agent under test.

Fixes: N/A (new dev tooling, no linked issue)

What was the problem/requirement? (What/Why)

AI agents are now real traffic against this package: since the CLI began tagging agent-invoked calls (#1210), agent-driven CLI events grew from ~500 (June) to 5,000+ (July, partial) across claude-code, kiro, codex, and cursor. We regularly change CLI help text, repo docs, and tutorials with agents in mind, but had no way to know whether a change actually helps an agent complete a task — review opinion was the only signal.

What was the solution? (How)

A small (~700 line, stdlib-only) eval loop under evals/, deliberately not part of the shipped package. An isolated agent attempts a goal; an LLM judge grades it against a rubric; in A/B mode the same eval runs against two git refs of the subject and reports the paired delta; a reviser agent can generate the candidate ref from a struggling transcript. A real_aws mode has the agent submit a real job and confirm it reaches SUCCEEDED — opt-in (--allow-real-aws), with the farm/queue supplied via DEADLINE_EVAL_FARM_ID/DEADLINE_EVAL_QUEUE_ID/DEADLINE_EVAL_REGION env vars so the eval file names no account, and skipped (not failed) when not opted in or unconfigured.

What is the impact of this change?

None on the shipped package: evals/ is a top-level dev directory (like test/), excluded from the wheel, with zero new dependencies. Anyone with a repo checkout, pip install -e ., and Claude Code on PATH can run it. Run artifacts land in evals/output/ (gitignored).

How was this change tested?

See DEVELOPMENT.md for information on running tests.

  • Have you run the unit tests? — 38 unit tests under evals/tests/ (no live model calls): git subject mechanics on real temp repos (seeding, scoped diffs, scratch refs, missing-ref failures, dirty/untracked-tree refusal, pathspec-scoped reset), judge verdict parsing and safe bool coercion, runner aggregation, proposal gating, sandbox-path safety, harness launch errors, and real_aws env-var/skip gating. hatch run lint clean.
  • Have you run the integration tests? — Live end-to-end: the deadline_cli examples at k=3 (9/9 judged PASS against the real CLI help), a docs-page eval built from a real AWS blog tutorial (incl. a reviser pass that produced an +85/−13 improvement diff), and the real_aws_submit eval, which submitted a real job to a sandbox farm, waited via deadline job wait, and reached SUCCEEDED.

Was this change documented?

  • Are relevant docstrings in the code base updated? — every module and non-trivial helper carries a purpose docstring.
  • Has the README.md been updated? — evals/README.md covers setup, the eval-file schema, all run flags, the real-AWS mode, and the close-the-loop recipe.

Does this PR introduce new dependencies?

  • This PR adds one or more new dependency Python packages. I acknowledge I have reviewed the considerations for adding dependencies in DEVELOPMENT.md.
  • This PR does not add any new dependencies.

Is this a breaking change?

No. Nothing under src/ changes; no public contract is touched.

Does this change impact security?

The tool runs local subprocesses (git, claude) with the operator's own credentials, only against checkouts/paths the operator configures. Destructive git operations are scoped to the eval's pathspec and gated behind a clean-tree check; sandbox file writes reject ../absolute keys. real_aws cases submit real jobs only when the operator opts in with --allow-real-aws and supplies a sandbox farm/queue via env vars — no account is named in the repo. No new attack surface in the shipped package (excluded from the wheel).


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Add evals/, a dev tool that measures how well an AI agent achieves goals
with Deadline Cloud tooling and docs, and A/B-proves that a change to the
CLI source, the repo docs, or any fetched reference material (AWS docs
page, blog post) actually helps.

- agent_evals/harness.py: run an isolated headless agent per case in a
  throwaway sandbox, capturing tool/turn/cost telemetry from stream-json.
- agent_evals/judge.py: grade the final answer against a plain-language
  rubric (LLM-as-judge, tool-less single call) so new eval domains need a
  rubric string, not new code.
- agent_evals/subject.py: any git checkout as the material under test;
  repo_subject() A/Bs this checkout (src/ or docs), corpus_subject() seeds
  fetched markdown into a throwaway repo so it can be diffed and revised.
- agent_evals/runner.py: JSON eval files, k runs per variant, paired
  baseline-vs-revised summary, and a proposal.patch emitted only when the
  revision measurably improved an eval.
- agent_evals/reviser.py: close the loop -- an agent edits the subject from
  a struggling run's transcript onto a scratch ref for re-testing.
- examples/ + README: starter evals for the deadline CLI, AWS CLI, and a
  docs page; stdlib-only, no new dependencies.

Isolation rule: the tested agent always runs as a separate claude -p
subprocess; the orchestrating session never performs the goal itself, so
the telemetry measures the agent under test.

Signed-off-by: Andy Choquette <78888816+andychoquette@users.noreply.github.com>
@github-actions github-actions Bot added the waiting-on-maintainers Waiting on the maintainers to review. label Jul 16, 2026
Comment thread evals/agent_evals/judge.py Fixed
Comment thread evals/agent_evals/subject.py
Comment thread evals/agent_evals/subject.py
…kout

Address review feedback on the Subject git mechanics:

- Subject._git_checked raises SubjectError on non-zero git exit; checkout,
  diff_refs, and commit_scratch now use it. Previously a checkout of a
  nonexistent --revised-ref failed silently, leaving the revised variant
  running baseline source and reporting a plausible-looking no-delta.
- Subject.assert_clean refuses to operate when the checkout has uncommitted
  or untracked changes under the owned pathspec, so reset_clean/git clean -fd
  can never discard operator work. The runner validates cleanliness and
  resolves both refs up front, before any costly agent runs.
- Explain the intentional JSONDecodeError fall-through in judge.py (CodeQL
  empty-except finding).

Adds tests for missing-ref failures and the dirty/untracked/outside-pathspec
assert_clean cases.

Signed-off-by: Andy Choquette <78888816+andychoquette@users.noreply.github.com>
@andychoquette
andychoquette marked this pull request as ready for review July 22, 2026 18:02
@andychoquette
andychoquette requested a review from a team as a code owner July 22, 2026 18:02
Comment thread evals/agent_evals/judge.py Outdated
apcho-amazon and others added 2 commits July 28, 2026 08:51
…e-pass

bool("false") evaluates to True in Python, so a model emitting a
stringified verdict would be silently recorded as a PASS. Add
_coerce_passed that handles known string forms (true/false/pass/fail/
yes/no/0/1) and raises JudgeError on anything ambiguous, so a
malformed verdict surfaces as a failure to grade rather than a bogus
positive.

Adds tests for string-false, string-true, and ambiguous-value cases.

Signed-off-by: Andy Choquette <apcho@amazon.com>
Comment thread evals/agent_evals/runner.py Outdated
With multiple cases, a change that improved one eval but regressed
another would still write proposal.patch (the whole base..revised diff)
while also exiting 4 for the regression -- presenting a known-breaking
change as PR-ready. Gate the proposal on 'improved at least one AND
regressed none', extracted to _should_emit_proposal with tests for the
mixed-verdict, no-improvement, and empty-diff cases.

Signed-off-by: Andy Choquette <apcho@amazon.com>
Comment thread evals/agent_evals/runner.py
The sandboxed agent runs in a throwaway tempdir and never sees the repo
checkout, so swapping git refs for a read-material subject (docs/prose)
had zero effect -- baseline and revised ran identical inputs and always
reported no_change. Add --seed-subject, which copies the subject's owned
files (at the currently checked-out ref) into each run's sandbox under
subject/, so a ref swap actually changes what the agent reads. The
CLI-source case is unaffected (pip install -e routes the ref swap through
the installed deadline). --seed-subject requires --revised-ref; the docs
row in the README now shows the flag. Adds _subject_files with tests for
ref-tracking and pathspec scoping.

Signed-off-by: Andy Choquette <apcho@amazon.com>
Adds a deadline_cli eval that checks an agent describes Open Job
Description as a general-purpose compute job template, not something
limited to rendering/visual compute. Passed 3/3 at k=3.

Signed-off-by: Andy Choquette <apcho@amazon.com>
Comment thread evals/agent_evals/subject.py Outdated
reset_clean ran an unscoped 'git reset --hard HEAD', but assert_clean --
the guard meant to protect operator work -- is scoped to diff_pathspec.
So an operator A/Bing 'src' while carrying uncommitted edits to docs/
passed the clean check, then had those edits silently wiped by the first
checkout -> reset_clean. Replace the whole-tree reset with a scoped
'git checkout HEAD -- <pathspec>' (reverts staged + worktree edits) so
reset_clean destroys only what it owns, matching the guard. Adds tests
for out-of-pathspec preservation and staged-edit revert.

Signed-off-by: Andy Choquette <78888816+andychoquette@users.noreply.github.com>
Comment thread evals/agent_evals/harness.py Outdated
Comment thread evals/agent_evals/runner.py Outdated
Two review findings:

- harness.run_agent had no timeout and let a missing binary raise a raw
  FileNotFoundError that aborts the batch. Add a wall-clock timeout and
  wrap launch in try/except, raising HarnessError (matching judge_answer
  and reviser.revise). _run_case catches it, fails just that run, and
  records a harness_error result rather than aborting or scoring a pass.
- Material/subject files were written with a flat write_text, so a key
  containing a path separator raised FileNotFoundError. Add _safe_write
  which creates parent dirs and rejects absolute paths and '..' segments
  so a key can never escape the sandbox; both materials and subject
  seeding use it.

Adds tests for nested/unsafe sandbox paths and the missing-binary launch
error.

Signed-off-by: Andy Choquette <78888816+andychoquette@users.noreply.github.com>
Comment thread evals/agent_evals/judge.py Outdated
try:
# stdin=DEVNULL: with -p the CLI still waits on stdin and can exit non-zero
# on a closed pipe.
proc = subprocess.run(cmd, capture_output=True, text=True, stdin=subprocess.DEVNULL)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The judge subprocess.run has no timeout, unlike run_agent in harness.py (which uses DEFAULT_TIMEOUT_S precisely because "a hung network/auth interaction has no turn cost, so ... one stuck run could block the whole batch indefinitely"). --max-turns 5 does not bound wall-clock time. Since judge_answer is called for every run in the batch, a single hung judge call will hang the entire eval run, defeating the harness timeout. Consider adding a timeout here and catching subprocess.TimeoutExpired as a JudgeError.

capture_output=True,
text=True,
stdin=subprocess.DEVNULL,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like the judge, this reviser subprocess.run has no timeout. The reviser drives a full agent (Read/Edit/Write/Grep/Glob) with no --max-turns bound at all, so a hung or runaway session can block indefinitely with no ceiling. harness.run_agent deliberately guards against exactly this. Consider passing a timeout (and a --max-turns) and translating subprocess.TimeoutExpired into a ReviseError.

Adds a real_aws eval mode that has the agent submit a real job bundle to
a real farm and confirm it reaches SUCCEEDED -- a true end-to-end check.
Because these submit real, billable jobs:

- Cases with "env": "real_aws" are SKIPPED (not failed) unless the
  operator passes --allow-real-aws, so a plain `run` stays green in CI.
- Farm/queue/region come from DEADLINE_EVAL_FARM_ID / DEADLINE_EVAL_QUEUE_ID
  / DEADLINE_EVAL_REGION -- never hardcoded, so the eval file names no
  account. prompt/rubric {placeholders} are filled from them.
- The runner prechecks that the env vars are set and `deadline auth` is
  live, skipping with a clear reason otherwise rather than submitting to
  the wrong place.

examples/real_aws_submit.json carries a minimal inline OpenJD echo bundle.
Verified end-to-end against a sandbox farm: agent submitted, waited via
`deadline job wait`, job reached SUCCEEDED. Adds tests for the env-var
config and skip-reason gating; README documents the mode.

Signed-off-by: Andy Choquette <78888816+andychoquette@users.noreply.github.com>
The README's eval-schema block predated the real_aws mode: add the env
field and the {farm_id}/{queue_id}/{region} placeholder note, a field
table for the optional keys, and a flags table covering --model,
--base-ref, --allow-real-aws, and the others that weren't shown.

Signed-off-by: Andy Choquette <78888816+andychoquette@users.noreply.github.com>
Comment thread evals/agent_evals/runner.py Outdated
print(f"\nsummary -> {out_root / 'summary.json'}")

if subj and args.revised_ref:
diff = summaries[0].get("source_diff", "") if summaries else ""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads the diff from summaries[0], but the first case may be a skipped real_aws case, whose summary is {"case_id", "skipped"} with no source_diff key. In that situation diff becomes "", so _should_emit_proposal returns False (empty diff.strip()) and the PR-ready patch is silently never written — even if a later case genuinely improved. Since source_diff is identical across all A/B cases, pick it from any summary that has one, e.g. next((s["source_diff"] for s in summaries if s.get("source_diff")), "").

Print an [env] provenance line at the start of every run: the deadline
path, version, and whether it's an editable (source-checkout) install.
Guards against silently evaluating a stale or fork-shadowed CLI -- the
'editable install shadows your real deadline' footgun -- especially for
real_aws runs where the wrong install produces a plausible but misleading
result.

Signed-off-by: Andy Choquette <78888816+andychoquette@users.noreply.github.com>
Comment thread evals/agent_evals/runner.py Fixed
Comment thread evals/examples/real_aws_submit.json Outdated
{
"id": "submit_job_end_to_end",
"env": "real_aws",
"prompt": "There is a Deadline Cloud job bundle in the ./bundle/ directory (an Open Job Description template that runs a short echo task). Using the `deadline` CLI, submit this bundle to farm {farm_id} and queue {queue_id}, wait for the job to finish, and report its final lifecycle/task-run status. Use `deadline bundle submit ./bundle --farm-id {farm_id} --queue-id {queue_id} --yes` and then wait for completion (for example with `deadline job wait` or by polling `deadline job get`). State the final status clearly in your answer.",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prompt tells the agent the bundle is in ./bundle/ and instructs deadline bundle submit ./bundle --farm-id ..., but _run_case writes every materials key under workdir/materials/, so this bundle actually lands at materials/bundle/template.yaml, not ./bundle/. The literal deadline bundle submit ./bundle command in the prompt will fail with a missing-path error, and the auto-appended hint ("Reference material is available ... materials/bundle/template.yaml") directly contradicts the ./bundle/ path in the prose. Since the rubric requires a successful submission, this real_aws case will fail spuriously. Either change the prompt to reference materials/bundle (and submit that path) or place the bundle at the location the prompt names.

try:
# A console-script shebang names the interpreter; ask pip where the package
# lives and whether it's an editable (source-checkout) install.
interp = Path(path).read_text().splitlines()[0].lstrip("#!").strip()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path(path).read_text() reads the deadline launcher as UTF-8 text to parse its shebang. When deadline is a native binary rather than a Python console-script text stub — the normal case on Windows (deadline.exe), and possible with zipapp/PyInstaller-style launchers — decoding raises UnicodeDecodeError, which is a subclass of ValueError, not OSError. It is therefore not caught by the except (OSError, subprocess.TimeoutExpired, IndexError) handler, so _deadline_provenance() propagates and aborts the entire run at startup, before any eval executes. Add ValueError (or UnicodeDecodeError) to the caught exceptions.

…re paths

The harness timeout only covers the agent subprocess. The judge runs once per
run in the batch and the reviser drives an unbounded edit session, so a hang in
either defeated it — both now take a timeout (and the reviser a --max-turns).

Three cases where a failure produced a plausible-looking wrong answer instead of
an error:
- proposal diff came from summaries[0], which is a skipped real_aws case with no
  source_diff — suppressing a patch a later case earned.
- _deadline_provenance() read the launcher as UTF-8; a native deadline.exe raises
  UnicodeDecodeError (ValueError, not OSError), aborting the run before any eval.
- real_aws_submit prompt named ./bundle/, but materials land under materials/.

Signed-off-by: Andy Choquette <78888816+andychoquette@users.noreply.github.com>
base_ref = None
if args.revised_ref:
subj = subject_mod.repo_subject(args.pathspec)
base = subj._git("rev-parse", "--abbrev-ref", "HEAD").stdout.strip()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git rev-parse --abbrev-ref HEAD prints the literal string HEAD when the checkout is on a detached HEAD (common: after git checkout <sha>, or in CI that checks out a commit rather than a branch). base_ref then becomes the string "HEAD", which passes the rev-parse --verify fail-fast check below, so nothing errors.

From then on base_ref is not a fixed commit but "wherever HEAD currently points", which silently corrupts the A/B:

  • Case 1: checkout("HEAD") is a no-op so baseline runs at the original commit; then checkout(revised_ref) moves HEAD to the revised commit.
  • Case 2..N: checkout("HEAD") now resolves to the revised commit, so every subsequent case runs its baseline variant against revised source and reports a bogus no_change.
  • The finally: subj.checkout(base_ref) also leaves the tree on the revised ref rather than restoring the operator’s original commit.

Suggest resolving the baseline to a concrete commit up front, e.g. subj._git_checked("rev-parse", "HEAD") (always a sha), or explicitly rejecting a detached HEAD when --base-ref was not supplied.

"--permission-mode",
"bypassPermissions",
"--max-turns",
str(max_turns),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security note on the trust boundary here: the module docstring calls this an "ISOLATED subprocess ... in a sandbox directory", but --permission-mode bypassPermissions only sets the cwd — it does not sandbox the process. The agent can run any Bash command against the operator’s real machine, credentials, and repo checkout; the temp workdir is a convention, not a boundary.

That matters because the prompt text is not always operator-authored. subject.corpus_subject() is documented as seeding "any fetched material (an AWS docs page, a blog post, a web-search result)", and runner._run_case concatenates case materials / seeded subject/ files into the sandbox for the agent to read. Untrusted fetched prose reaching a bypassPermissions agent with Bash is a prompt-injection path to arbitrary command execution with the operator’s AWS credentials — and real_aws mode is explicitly pointed at live, billable resources.

Worth either (a) tightening the wording so operators understand there is no sandbox and eval files/materials are trusted input, or (b) dropping to a real boundary (a container, or --permission-mode acceptEdits plus a Bash allowlist) for any case whose material was fetched rather than authored.

if subj and args.revised_ref:
summary["source_diff"] = subj.diff_refs(base_ref, args.revised_ref)
b, c = variants["baseline"]["aggregate"], variants["revised"]["aggregate"]
improved = c["pass_rate"] > b["pass_rate"] or (

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The median_turns tie-break lets a change be reported as improved — and therefore emitted as a PR-ready proposal.patch — without ever making an eval pass.

Concretely, if both variants fail every run (pass_rate == 0.0 on each), the first clause is False but the second is 0.0 == 0.0 and c["median_turns"] < b["median_turns"], which is True whenever the revised agent merely gave up faster. _should_emit_proposal then sees "improved" in verdicts and writes the patch, and the [A/B] line prints pass 0% -> 0% ... improved. A revision that fixed nothing gets surfaced as ready to ship.

This is amplified by --k defaulting to 1: with a single run per variant, median_turns is one sample of a stochastic agent, so ordinary run-to-run variance is enough to trip the tie-break. _should_emit_proposal’s docstring says "measurably improved", but at k=1 nothing here is a measurement.

Two suggestions:

  • Gate the tie-break on a non-zero baseline, e.g. require b["pass_rate"] > 0 (or c["pass_rate"] > 0) before turn count can decide improved.
  • Require k > 1 (or a minimum turn-count delta) before a turns-only win counts as improved for proposal purposes.

for variant, ref in refs.items():
subject_files = None
if subj and ref:
subj.checkout(ref)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subject.checkout() is pathspec-scoped only for the discard step (reset_clean); the actual git checkout -q <ref> switches the entire working tree. That creates two problems the pathspec-scoped assert_clean() above does not cover:

  1. The runner swaps out its own source. With the default --pathspec src, checkout(revised_ref) still replaces everything, including evals/. If the revised ref does not contain this eval harness (e.g. a fix branch cut from main before this PR landed), evals/agent_evals/*.py and evals/examples/*.json vanish mid-run. Already-imported modules keep working, but anything read lazily after that point (a second eval file, _subject_files on a docs pathspec) reads from a tree that no longer matches what the operator invoked.

  2. Uncommitted work outside the pathspec aborts the run mid-batch. assert_clean() deliberately only inspects -- self.diff_pathspec, so edits elsewhere pass the fail-fast gate — then git checkout <ref> refuses to overwrite them and _git_checked raises SubjectError after the costly agent runs have already been paid for. Worse, the finally: subj.checkout(base_ref) at the end will raise for the same reason, and that exception replaces the original one, so the operator sees a confusing restore failure and is left on the revised ref.

Consider using git checkout <ref> -- <pathspec> (scoped, matching reset_clean and assert_clean) instead of a whole-tree branch switch, and wrapping the finally restore in a try/except so a restore failure cannot mask the real error.

and rubric for real_aws cases; None for offline/mock cases.
"""
fmt = dict(aws_ctx or {})
prompt = case["prompt"].format(**fmt) if fmt else case["prompt"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

str.format is applied to the entire prompt/rubric for real_aws cases, so every brace in the case text becomes format syntax — not just the three intended placeholders:

  • A prompt containing an unrelated brace (a JSON snippet, a shell brace expansion, or an OpenJD template reference — examples/real_aws_submit.json already uses doubled-brace OpenJD syntax in its materials) raises KeyError/ValueError out of _run_case. Neither is caught anywhere, so it aborts the entire batch mid-run instead of failing just that case.
  • Doubled braces are silently unescaped to single braces, quietly corrupting the prompt the agent actually sees.
  • An authoring typo (farm-id instead of farm_id) surfaces as a bare KeyError rather than a readable error naming the case.

Since only three names are ever substituted, a targeted str.replace per placeholder avoids all of it and leaves every other brace untouched. Alternatively, wrap the two .format calls and re-raise as a clear "case X has an invalid placeholder" error so one bad case cannot take the whole batch down.

"""
prompt = _REVISE_PROMPT.format(goal=goal, transcript=transcript_text(run_dir / "events.jsonl"))

subj.checkout(base_ref) # edit from a clean baseline

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revise() calls subj.checkout(base_ref) without first calling subj.assert_clean() — and checkout()reset_clean() runs git checkout HEAD -- <pathspec> plus git clean -fd <pathspec>, which silently discards the operator’s uncommitted tracked edits and deletes untracked files under the pathspec.

Subject.assert_clean exists precisely to prevent this; its docstring states the contract: "Callers invoke this once before the first destructive operation; a dirty tree is the operator’s to stash or commit, not ours to delete." runner._cmd_run honors that. revise() does not, even though the README documents it as a first-class entry point invoked directly against subject.repo_subject() — the user’s real deadline-cloud checkout, defaulting to pathspec src:

subj = subject.repo_subject()                       # or corpus_subject(markdown)
ref = reviser.revise(subj, run_dir, goal="...", base_ref="mainline")

Anyone following that snippet with work-in-progress under src/ loses it with no prompt and no error. Adding subj.assert_clean() immediately before this checkout would close the gap.

Two related notes on the same call path:

  • commit_scratch leaves the repo checked out on the scratch eval-revise-* branch, and revise() has no finally restore, so the caller is silently left on a different branch than they started on (and on the error paths above, on base_ref rather than their original branch).
  • The reviser agent runs --permission-mode bypassPermissions with Write/Edit and cwd=subj.root — the operator’s actual repo, not a sandbox. The prompt asks for minimal additive edits and no git, but nothing enforces the pathspec scope; the later add <pathspec> limits only what gets committed, not what gets written.

{prompt}

=== THE AGENT'S FINAL ANSWER ===
{answer}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The graded agent’s final_text is interpolated here with no delimiter or escaping, and the surrounding scaffolding is plain prose the answer can imitate. Because the agent under test produces {answer}, it can emit text that reads as fresh judge instructions — a line resembling one of the === ... === headers, or simply an instruction to respond with a passing verdict. _extract_verdict scans first { to last }, so a compliant-looking injected object is accepted verbatim.

This is more than theoretical for this harness: reviser.revise() closes the loop by having an agent edit the subject so the next agent scores better, and _should_emit_proposal converts a score improvement into a PR-ready proposal.patch. An edit that nudges the agent toward answer phrasing the judge rubber-stamps is a cheaper win than one that genuinely improves the material — reward hacking whose output is a patch aimed at this repo.

Cheap mitigations that preserve the "rubric is just a string" property:

  • Wrap the answer in a per-call random delimiter and tell the judge everything inside it is untrusted data (the agent cannot guess the nonce).
  • State explicitly that text inside the answer block is never an instruction, and that attempting to direct the grading is itself a fail.
  • Bound the answer length so a very long answer cannot crowd out the real instructions.

import pytest

sys.path.insert(0, str(Path(__file__).resolve().parents[1]))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three test modules (~440 lines) are never collected by this repo’s pytest configuration: pyproject.toml pins testpaths = ["test/unit", "test/cli_e2e"], and evals/tests is under neither. So the whole suite is dead in CI — the assert_clean / _safe_write traversal / verdict-coercion guards it encodes will not catch a regression, and the harness can silently rot.

They also won’t pass under the repo’s default addopts as-is: --cov=src/deadline with --cov-report means a run that exercises only evals/ reports 0% coverage, and each module relies on its own sys.path.insert(...) rather than the configured pythonpath = ["test"].

Either add evals to testpaths (or a dedicated pytest section / hatch script for it, e.g. pytest evals/tests --no-cov) and wire it into CI, or state in evals/README.md that the tests are run manually — right now a reader reasonably assumes CI covers them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-on-maintainers Waiting on the maintainers to review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants