Repository navigation
fix(score): stage agent-judge outputs by copying, not via memory - #260
fullsend-ai-coder[bot] wants to merge 5 commits into
Conversation
score.py read every file under each declared output dir into
record["files"] as a str, and _stage_agent_workspace wrote those
strings back out for agent judges. A full checkout staged as an
output (odh-dashboard: ~11k files) therefore passed through Python
memory on every scoring pass, and binary files were dropped as
{"_binary": True} placeholders, so the judge never saw them.
_stage_agent_workspace now copies the declared output dirs and
_modified/ straight from the case dir with the same dereferencing,
confined copier agent.context already uses. Binary files arrive
intact, an in-dir symlink becomes a regular file, and a link out of
its output dir is skipped with a warning. The agent.inputs filter,
the reserved output/ and .context/ names, and the case-dir
containment check are unchanged. Unreadable content fails the judge,
as it does for agent.context.
load_case_record gains load_files. score_cases passes False only
when every judge is an agent judge whose prompt does not render
{{ outputs }} and no if: condition names outputs. Any other judge
kind (LLM, check, code, builtin) still gets record["files"] as before.
Note: tests/test_cli_runner.py::test_timeout fails in the sandbox
with and without this change (process kill not permitted).
`make` is blocked in the sandbox, so skillsaw was run directly
(0 errors).
Closes #258
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 2:30 PM UTC · Completed 2:45 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $3.48 |
|
<!-- fullsend:risk-assessment --> DetailsProtected path modifications and historical regression churn in score.py are offset by bot authorship, substantial test coverage additions, and close alignment with active linked issue #258. Previous run<!-- fullsend:risk-assessment --> DetailsProtected path modifications and historical regression churn in score.py are offset by bot authorship, substantial test additions, and close alignment with active linked issue #258. Previous run (2)<!-- fullsend:risk-assessment --> DetailsProtected path modifications and historical regression churn in score.py are offset by bot authorship, substantial test coverage additions, and close alignment with active linked issue #258. Previous run (3)<!-- fullsend:risk-assessment --> DetailsProtected path modifications and historical regression churn in score.py are offset by bot authorship, comprehensive test additions, and close alignment with linked issue #258. Previous run (4)<!-- fullsend:risk-assessment --> DetailsModerate risk change modifying existing staging behavior in protected path score.py without a feature flag, balanced by clear alignment with issue #258, bot authorship, and comprehensive test coverage. |
ReviewFindingsMedium
Low
Previous run<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJjb21tZW50IiwiZmluZGluZ3MiOlt7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJlZGdlLWNhc2UiLCJmaWxlIjoic2tpbGxzL2V2YWwtcnVuL3NjcmlwdHMvc2NvcmUucHkiLCJpZCI6ImZfMTNmNWY4MmU5NWM0Yzg3YSIsImxpbmUiOjM0NDksImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoiaW5mbyIsImNhdGVnb3J5IjoiZmFpbC1vcGVuIiwiZmlsZSI6InNraWxscy9ldmFsLXJ1bi9zY3JpcHRzL3Njb3JlLnB5IiwiaWQiOiJmXzFkNjFjOTNhNGMxNDY5YzQiLCJsaW5lIjoyODEsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJpbmplY3Rpb24tdnVsbiIsImZpbGUiOiJza2lsbHMvZXZhbC1ydW4vc2NyaXB0cy9zY29yZS5weSIsImlkIjoiZl83MjZmNjg4Y2M4OTE1MzBmIiwibGluZSI6MzM5MSwiYWN0aW9uYWJsZSI6ZmFsc2V9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImluY29tcGxldGUtZG9jIiwiZmlsZSI6IndlYnNpdGUvY29uY2VwdHMvanVkZ2VzLm1kIiwiaWQiOiJmXzQ5ODA2ZGIwNzFjZTdiMjgiLCJsaW5lIjoxNDcsImFjdGlvbmFibGUiOmZhbHNlfV0sImRpc3Bvc2l0aW9ucyI6W3siaWQiOiJmXzEzZjVmODJlOTVjNGM4N2EiLCJzdGF0dXMiOiJvcGVuIn0seyJpZCI6ImZfMWQ2MWM5M2E0YzE0NjljNCIsInN0YXR1cyI6InJlY2xhc3NpZmllZCJ9LHsiaWQiOiJmXzcyNmY2ODhjYzg5MTUzMGYiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifSx7ImlkIjoiZl80OTgwNmRiMDcxY2U3YjI4Iiwic3RhdHVzIjoicmVzb2x2ZWRfYnlfY2hhbmdlIn1dfQ== --> ReviewFindingsMedium
Low
Earlier findings
Previous run (2)<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJjb21tZW50IiwiZmluZGluZ3MiOlt7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJlZGdlLWNhc2UiLCJmaWxlIjoic2tpbGxzL2V2YWwtcnVuL3NjcmlwdHMvc2NvcmUucHkiLCJpZCI6ImZfMTNmNWY4MmU5NWM0Yzg3YSIsImxpbmUiOjMzNTUsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoiaW5mbyIsImNhdGVnb3J5IjoiZmFpbC1vcGVuIiwiZmlsZSI6InNraWxscy9ldmFsLXJ1bi9zY3JpcHRzL3Njb3JlLnB5IiwiaWQiOiJmXzFkNjFjOTNhNGMxNDY5YzQiLCJsaW5lIjozMzIwLCJhY3Rpb25hYmxlIjpmYWxzZX0seyJzZXZlcml0eSI6ImxvdyIsImNhdGVnb3J5IjoiaW5qZWN0aW9uLXZ1bG4iLCJmaWxlIjoic2tpbGxzL2V2YWwtcnVuL3NjcmlwdHMvc2NvcmUucHkiLCJpZCI6ImZfNzI2ZjY4OGNjODkxNTMwZiIsImxpbmUiOjMzOTEsImFjdGlvbmFibGUiOmZhbHNlfSx7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJpbmNvbXBsZXRlLWRvYyIsImZpbGUiOiJ3ZWJzaXRlL2NvbmNlcHRzL2p1ZGdlcy5tZCIsImlkIjoiZl80OTgwNmRiMDcxY2U3YjI4IiwibGluZSI6MTQ3LCJhY3Rpb25hYmxlIjpmYWxzZX1dLCJkaXNwb3NpdGlvbnMiOlt7ImlkIjoiZl83MjZmNjg4Y2M4OTE1MzBmIiwic3RhdHVzIjoicmVzb2x2ZWRfYnlfY2hhbmdlIn0seyJpZCI6ImZfMWQ2MWM5M2E0YzE0NjljNCIsInN0YXR1cyI6InJlY2xhc3NpZmllZCJ9LHsiaWQiOiJmXzQ5ODA2ZGIwNzFjZTdiMjgiLCJzdGF0dXMiOiJyZXNvbHZlZF9ieV9jaGFuZ2UifV19 --> ReviewFindingsMedium
Low
Earlier findings
Previous run (3)<!-- fullsend:review-findings-v2:eyJ2ZXJzaW9uIjoyLCJhY3Rpb24iOiJjb21tZW50IiwiZmluZGluZ3MiOlt7InNldmVyaXR5IjoibG93IiwiY2F0ZWdvcnkiOiJpbmplY3Rpb24tdnVsbiIsImZpbGUiOiJza2lsbHMvZXZhbC1ydW4vc2NyaXB0cy9zY29yZS5weSIsImlkIjoiZl83MjZmNjg4Y2M4OTE1MzBmIiwibGluZSI6MzM5MSwiYWN0aW9uYWJsZSI6ZmFsc2V9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImZhaWwtb3BlbiIsImZpbGUiOiJza2lsbHMvZXZhbC1ydW4vc2NyaXB0cy9zY29yZS5weSIsImlkIjoiZl8xZDYxYzkzYTRjMTQ2OWM0IiwibGluZSI6MzMyMCwiYWN0aW9uYWJsZSI6ZmFsc2V9LHsic2V2ZXJpdHkiOiJsb3ciLCJjYXRlZ29yeSI6ImluY29tcGxldGUtZG9jIiwiZmlsZSI6IndlYnNpdGUvY29uY2VwdHMvanVkZ2VzLm1kIiwiaWQiOiJmXzQ5ODA2ZGIwNzFjZTdiMjgiLCJsaW5lIjoxNDcsImFjdGlvbmFibGUiOmZhbHNlfV19 --> ReviewFindingsMedium
Low
Previous run (4)ReviewFindingsMedium
Low
Next steps:
|
|
🤖 Finished Fix · ✅ Success · Started 2:47 PM UTC · Completed 2:50 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $0.46 |
- Drop declared output paths that have a declared ancestor, regardless of declaration order, so a child is never copied twice (EACCES on read-only files). - Detect `outputs` in judge templates with Jinja's parser instead of a regex; unparseable templates load the files. - Bound staging with cumulative entry/byte/time limits to stop acyclic symlink fan-out. - Fix README agent.inputs default wording. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
🔧 Fix agent — iteration 1 (bot-triggered)Addressed all review findings: order-independent overlap handling in output staging, parser-based detection of outputs in judge templates, cumulative staging limits, and README wording. The protected-path finding needs human approval and was not edited. pytest/jinja2 are not installed in the sandbox, so the test suite could not be run; the staging logic was checked with a standalone script and files compile, but the jinja-based detection and new tests are unexecuted here. Fixed (4):
Disagreed (1):
Tests: failed CI inspections (3):
Conflict update: skipped; strategy Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 2:52 PM UTC · Completed 3:10 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $3.32 |
Superseded by updated review
|
/fs-fix Please address two of the latest review findings:
|
|
🤖 Finished Fix · ✅ Success · Started 7:17 PM UTC · Completed 7:20 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $0.32 |
…judge file loading (#260) - _format_staging_problems now escapes control characters in paths and reasons (repr-style) and neutralizes a leading "::", so a hostile symlink name cannot forge log lines or workflow commands. Regression tests cover a dangling symlink whose name contains a newline. - website/concepts/judges.md: note that when every judge is an agent judge that doesn't render outputs, file contents are not loaded and those judges get the declared output dirs and _modified/ copied in. The scandir fail-open finding is intentionally left as is (predates this PR; the PR makes it smaller). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
🔧 Fix agent — iteration 2 (human-triggered)Escaped control characters in staging problem reports (with regression tests) and documented agent-judge file loading. The scandir fail-open finding was intentionally left unchanged, as instructed. Fixed (2):
Disagreed (1):
Tests: passed Conflict update: skipped; strategy
|
|
🤖 Finished Review · ✅ Success · Started 7:22 PM UTC · Completed 7:34 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $2.47 |
|
/fs-fix Please do the following:
|
|
🤖 Finished Fix · ✅ Success · Started 5:30 PM UTC · Completed 5:36 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $0.33 |
Resolve the data-pipeline.md conflict in the agent-judge bullet: keep this PR's copy-based output staging wording for step (1) and main's (#254) wording for step (2) (default allowed/denied tools, permission mode). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
🔧 Fix agent — iteration 3 (human-triggered)Merged origin/main (#254) into the branch and resolved the single docs conflict in data-pipeline.md as instructed. Full test suite after the merge: 2322 passed, 27 failed, 22 skipped. The agent-judge and scoring tests all pass (295 passed). The other failures come from packages missing in my local test environment (anthropic, openai, mlflow, kubernetes) plus the known sandbox-only test_timeout failure. The edge-case finding f_13f5f82e95c4c87a was left unchanged as instructed. Fixed (1):
Disagreed (1):
Tests: passed CI inspections (3):
Conflict update: merged; strategy
|
|
🤖 Finished Review · ✅ Success · Started 5:38 PM UTC · Completed 5:53 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $2.77 |
|
/fs-fix The merge in 5d38b61 reverted #254's wording in skills/eval-run/references/data-pipeline.md. Please fix it mechanically rather than editing by hand:
|
|
🤖 Finished Fix · ✅ Success · Started 8:55 PM UTC · Completed 8:57 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $0.21 |
🔧 Fix agent — iteration 4 (human-triggered)Restored #254's wording in data-pipeline.md mechanically (main's file plus this PR's step (1) only). git diff origin/main for the file changes only the one bullet; the required strings are present. Committed ea10e7a. The project's Python tests were not run, because pytest is not installed in the sandbox and the change is docs-only. Fixed (1):
Tests: passed CI inspections (1):
Conflict update: skipped; strategy
|
|
🤖 Finished Review · ✅ Success · Started 9:00 PM UTC · Completed 9:10 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $2.54 |
| if over: | ||
| errors.append((str(child), over)) | ||
| break | ||
| shutil.copy2(child, target) |
There was a problem hiding this comment.
[low] edge-case
The staging deadline is checked only before each entry or file copy; a slow shutil.copy2 (especially the last file) can exceed the 600s allowance with no later check, so staging reports success and the judge starts anyway.
Suggested fix: Check the budget after each copy, or run staging under an independently enforced timeout.
Summary
Agent judges now get their workspace by copying a case's declared output dirs (plus
_modified/) straight from the case directory. Before, every file was read intorecord["files"]as a string and then written back out. When every configured judge is an agent judge that doesn't render{{ outputs }}, scoring now skips reading the output trees into memory.What changed (
skills/eval-run/scripts/score.py)_stage_agent_workspace/ new_stage_case_outputs: copies each declaredoutputs[].pathdir and_modified/with the existing_copy_tree_dereferenced(the copieragent.contextalready uses).{"_binary": True}placeholders.agent.contextfollows.agent.inputsfilter (./empty = all), the reservedoutput/and.context/names, and the case-dir containment check._modified/still leaves out.work,subagentsandhooks.load_case_record(..., load_files=True): whenload_filesis False, it skips the output-tree reads, the<output>_contentconvenience keys andmodified_files._judges_read_files:score_casespassesload_files=Falseonly when every judge is an agent judge, none of their prompts rendersoutputsinside a{{ }}/{% %}block, and noif:condition namesoutputs. Every other judge kind (LLM, pairwise, inline check, code/module, builtin) still getsrecord["files"]. Anything it can't classify falls back to loading._copy_tree_dereferencedtakes an optional top-levelskip,_report_staging_problemstakes a label, and the out-of-root reason now reads "resolves outside the staged entry".No
eval.yamlchanges are needed.Docs
skills/eval-run/references/data-pipeline.mdandwebsite/reference/config/judges.mdnow describe copy-based staging, symlink handling and the containment rules.Testing
tests/test_agent_judge.py:agent.inputsfiltering;_modified/staged without harness dirs;read_textover the output tree (checked with a spy);outputs["files"];{{ outputs }}still gets the files;python3 -m pytest tests/: 2347 passed. The one failure,tests/test_cli_runner.py::test_timeout, also fails on unmodifiedmainin this sandbox (killing processes isn't permitted there).skillsaw: 0 errors.make lintcouldn't run becausemakeis blocked in the sandbox, so skillsaw was invoked directly. pyflakes reports nothing new.Notes
score.py, so expect merge conflicts.🤖 Generated with Claude Code
Closes #258
Post-script verification
agent/258-copy-agent-judge-inputs)1a980e707909a23f8c143ce333d97f5203db34ab..HEAD)