Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
8dbe7df
test(eval): add 002-dead-config-field functional test case
guyoron1 Aug 2, 2026
ae74c63
test(eval): address PR review feedback and fix fixture bugs
guyoron1 Aug 4, 2026
407b98d
test(eval): address review feedback — budget baseline, tests, code_ex…
guyoron1 Aug 9, 2026
69dfc0e
test(eval): remove field-enumeration spoiler from runner doc comment
guyoron1 Aug 12, 2026
f3b7592
test(eval): cover runner retry/timeout, ground budget, note 002 in ev…
guyoron1 Aug 13, 2026
ea54615
test(eval): correct budget/timeout comments, drop stray docs change
guyoron1 Aug 13, 2026
2705386
test(eval): add removed_symbols judge to verify fix content in PR diff
guyoron1 Aug 16, 2026
14cd11d
test(eval): scrub captured PR diffs before artifact upload
guyoron1 Aug 16, 2026
14bb716
test(eval): correlate removed_symbols with per-file deletion sites
guyoron1 Aug 18, 2026
f5dea5c
Merge branch 'main' into eval/code-dead-config-field
guyoron1 Aug 18, 2026
0b42b80
test(eval): scope removed_symbols survivors and attribute whole-file …
guyoron1 Aug 19, 2026
9160864
test(eval): count deletion sites per file and honour hunk ranges in r…
guyoron1 Aug 22, 2026
35f1b62
test(eval): exclude comment lines from removed_symbols deletion counts
guyoron1 Aug 25, 2026
0a39bd7
test(eval): gate removed_symbols cases on go build/test and pin annot…
guyoron1 Aug 26, 2026
6d658f2
test(eval): contain the build/test gate and harden its capture plumbing
guyoron1 Aug 28, 2026
4bcc8e2
fix(eval): reject whole-file deletion of a declared removed_symbols file
guyoron1 Sep 5, 2026
37bbe07
fix(eval): state run_go_checks containment honestly and bound its kills
guyoron1 Sep 5, 2026
f16bb61
docs(eval): re-derive the timeout budget for three code cases
guyoron1 Sep 5, 2026
c2f55e9
fix(eval): keep fixture_checks from raising on a malformed annotation
guyoron1 Sep 5, 2026
09eee85
fix(eval): fail closed on a malformed removed_symbols annotation
guyoron1 Sep 5, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -63,5 +63,7 @@ script-test:
$(call run-timed,python3 scripts/process-fix-result-test.py)
$(call run-timed,bash eval/scripts/scrub-eval-results-test.sh)
$(call run-timed,bash .github/scripts/check-rollup-result-test.sh)
$(call run-timed,bash eval/scripts/capture-fixture-test.sh)
$(call run-timed,python3 eval/scripts/removed-symbols-judge-test.py)

test: script-test
58 changes: 58 additions & 0 deletions eval/code/cases/003-dead-config-field/annotations.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
state: open

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

MEDIUM — Case ordinal 002 collides with open PR #682, which adds a second 002- case and edits the same eval.yaml block

Verified live: open PR #682 ("feat(#677): add needs_input pushback for the code agent", branch feat/677-code-needs-input, state OPEN) adds eval/code/cases/002-push-back-on-nonsense/annotations.yaml, input.yaml and a repo symlink, and also modifies eval/code/eval.yaml. Because the two case directories have different names, git merges both cleanly and main ends up with two distinct 002- cases; meanwhile the overlapping eval/code/eval.yaml edit (both PRs rewrite the top-level description block that documents removed_symbols / case behaviour) will conflict for whichever PR lands second. Not raised on any existing thread.

Suggestion: Renumber this case to 003-dead-config-field, or coordinate with #682 on who keeps 002, and rebase onto whichever lands first so the eval.yaml description edits are reconciled deliberately rather than through conflict resolution.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Renumbered to 003-dead-config-field in 0b42b80 (dir, the two comment mentions in eval.yaml, PR title/body). Numbering gaps already exist in this tree (eval/fix/cases has 001 and 003), and capture-fixture.sh/tests key on the annotations, not the directory name, so nothing else moves. The eval.yaml description-block overlap with #682 is real but small and outside the judge body — happy to rebase onto whichever lands first; the branch is currently a clean merge against main (git merge-tree reports no conflicts at 816b89b).


expected_files:
- config/config.go
- config/fields.go
- config/config_test.go

labels:
forbidden: []

# Consumed by eval.yaml's removed_symbols judge: for each symbol, each file
# maps to the minimum number of deletion lines that must name the symbol,
# and the symbol must not survive in any added or context line of the diff.
# Both the Go identifier and its YAML key must vanish from all three sites.
#
# Counts rather than a bare file list on purpose: one deletion line per
# file is satisfied by the struct field alone, while the `VerboseLogging:
# false,` in Defaults() sits 11 lines away — outside git's 3-line context,
# so it is neither deleted nor visible as a survivor, and the tree no
# longer compiles. The counts are the lines that name the symbol today
# (`grep -n` in eval/code/repos/taskrunner/config/), excluding the doc
# comment above the field, which a fix may legitimately leave or drop.
removed_symbols:
VerboseLogging:
config/config.go: 2 # struct field, Defaults()
config/fields.go: 1 # c.VerboseLogging = v
config/config_test.go: 6 # TestDefaults, TestLoad, TestLoadPartial (2 each)
verbose_logging:
config/config.go: 1 # yaml tag on the struct field
config/fields.go: 1 # case "verbose_logging"
config/config_test.go: 1 # raw literal in TestLoad

# Code agent budgets (sandbox work + retries). Unmeasured ceiling, and to be
# precise about what "unmeasured" means here: the eval harness has never
# executed this case at all. Every Functional Tests run on the branch that
# adds it was gated off by check-e2e-authorization.sh (external contributor,
# no ok-to-test label), so nothing below is derived from a run of this
# fixture — the numbers are a ceiling, not an observation. Re-derive from
# the first real run, which needs a maintainer to apply ok-to-test.
# Values mirror 001-fix-add's ceiling (60 turns / $4.00) and eval.yaml's
# max_budget_usd. For reference, 001 (a trivial fixture) observed 12 turns /
# $2.12 (CI run 29424512121) and 35 turns / $0.98 (CI run 30166455238); this
# cross-file task is harder, so its ceiling is set no lower.
max_turns: 60
max_cost_usd: 4.00

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

MEDIUM — Budget copied from 001-fix-add with no observed baseline, and case shipped without an end-to-end harness run

max_turns: 60 / max_cost_usd: 4.00 are byte-identical to 001-fix-add's values, but 001-fix-add/annotations.yaml documents its numbers with actual observed CI baselines ("12 turns / $2.12 (CI run 29424512121), then 35 turns / $0.98 (CI run 30166455238)") and explains the chosen headroom multiplier. This annotations.yaml has no such comment at all. This PR's own test plan leaves "End-to-end fullsend eval code run (requires CI)" unchecked — the case has only been validated by go test ./... and eval/lint-cases.sh code, never by actually driving an agent through the real fullsend eval code harness — yet the PR justifies its design partly by citing an unrelated prior benchmark (fullsend#5808, N=13 pairs on a structurally different, single-file fixture) as if it generalizes to this new cross-file case. Given the PR itself describes 002 as meaningfully harder than 001 (multi-file tracing across 3 files vs. a one-line arithmetic fix), reusing 001's untouched budget without any baseline or real run is an unverified guess presented as final configuration.

Suggestion: Run the case at least once through the real fullsend eval code harness (or CI) before merging, and add an annotations.yaml comment documenting the observed turns/cost baseline and chosen headroom multiplier, matching the convention in 001-fix-add/annotations.yaml. If CI access is genuinely blocking pre-merge, say so explicitly and treat the unchecked test-plan item as a required follow-up rather than optional.


# Human reference only; not consumed by judges (unlike triage/review quality).
code_expectations: |
The repo has a dead config field: Config.VerboseLogging is declared, defaulted,
parsed, and tested, but never read by any consumer (runner/runner.go doesn't
use it). A successful run creates a PR that removes VerboseLogging from: the
struct definition (config.go), the Defaults() return value (config.go), the
SetField() switch case (fields.go), and all test assertions (config_test.go).
Tests must still pass after removal. This case tests cross-file dead-code
removal — tracing symbol references across multiple files to determine what
to change, not just fixing a single line. The raw YAML literal in TestLoad
(verbose_logging: true) must also be removed — SetField rejects unknown keys,
so leaving it breaks Load().
25 changes: 25 additions & 0 deletions eval/code/cases/003-dead-config-field/input.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
forge: github
fixture:
type: issue
title: "config: VerboseLogging field appears unused"
body: |
## Bug Report

**What happened:**
We have a `VerboseLogging` field in the config that I can't find being used anywhere
in the actual runner logic. I added it a while back thinking we'd need it for debug
output, but I'm not sure we ever wired it up to anything.

**Expected behavior:**
If it's truly unused, we should remove it to keep the config clean. But I want to
make sure I'm not missing something — maybe there's some code path that reads it
that I didn't spot?

**Additional context:**
- The field exists in `config/config.go`
- YAML key is `verbose_logging`
- I see tests for it, but those might just be testing the config parsing itself
- Haven't found where the runner actually checks this value, but I might be wrong

Can someone verify whether this is actually used? If not, please clean it up.
Make sure tests still pass after any changes.
1 change: 1 addition & 0 deletions eval/code/cases/003-dead-config-field/repo
Loading
Loading