Skip to content

Commit 0ba7782

Browse files
Make the benchmark observable, survivable, and stop it shrinking its own denominator
Three problems, all found by watching a run instead of waiting for one. It printed nothing until the end. A 120-review run sat at zero bytes for ninety minutes with no way to distinguish progress from a hang, and killing it — which is what happened — threw away 83 completed reviews.每 run now appends to a TSV as it finishes and progress goes to stderr, so the suite is observable while it runs and its partial results survive being interrupted. One slow arm stalled everything. gstack's /review reads a much larger tree and was averaging minutes per run against a 300-second ceiling, which turned the suite into hours. The ceiling is 150 seconds now and overridable. A review that does not finish is recorded as a miss, which is the honest treatment. And the denominator moved. When scoring failed, it returned planted:0, so a review that errored or timed out quietly removed its own planted defect from the total rather than counting as a miss — inflating recall for whichever arm failed more. That is why one arm reported 34 planted where the others reported 35, a difference that looked like rounding and was four swallowed failures. The denominator now comes from ground truth and never from the scoring path. That is the sixth way this benchmark was wrong, and like the previous five it would have flattered whichever arm I happened to be measuring.
1 parent 8a45118 commit 0ba7782

1 file changed

Lines changed: 35 additions & 4 deletions

File tree

tests/evals/run-pathways.sh

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,11 @@ run_arm() { # <arm> <dir> -> findings TAB pathway_entered
132132
# nothing needs to be appended here.
133133
*) prompt="/review" ;;
134134
esac
135-
raw=$( cd "$d" && timeout 300 claude -p "$prompt" --setting-sources=project \
135+
# 300s per review let one slow pathway stall the whole suite: gstack's /review reads a large
136+
# tree and its arm was averaging minutes per run, which turned a 120-review benchmark into
137+
# hours. A shorter ceiling loses the occasional slow run — recorded as a miss, which is the
138+
# honest treatment of a review that did not finish — rather than losing the whole suite.
139+
raw=$( cd "$d" && timeout "${REVIEW_TIMEOUT:-150}" claude -p "$prompt" --setting-sources=project \
136140
--output-format=stream-json --verbose < /dev/null 2>/dev/null )
137141
text=$(printf '%s' "$raw" | jq -rs '[.[]|select(.type=="assistant")|.message.content[]?|select(.type=="text")|.text]|join("")' 2>/dev/null)
138142
# Did the harness's own pathway actually engage? For `none` there is nothing to enter.
@@ -182,27 +186,48 @@ extract_findings() {
182186
# So the extractor now drops anything the review itself frames as a nit, a suggestion, a
183187
# missing test, a style or typing preference, or an informational note, and keeps only what it
184188
# presents as an actual bug. Same instruction, same model, every arm.
185-
out=$( cd "$ed" && timeout 180 claude -p "Read review.txt. It is a code review. Extract ONLY findings that the review presents as a genuine BUG, security problem, or resource-handling error in the code — something that would misbehave at runtime. EXCLUDE anything the review frames as a nit, style, naming, typing or annotation preference, a missing test, missing documentation, a suggestion, or an informational note, however it is labelled. Output ONLY a JSON array and no prose: [{\"line\": <integer>, \"category\": \"security|correctness|resource-leak\", \"summary\": \"<one sentence>\"}]. Use the line number the review gives. If it reports no genuine bug, output []." \
189+
out=$( cd "$ed" && timeout "${EXTRACT_TIMEOUT:-90}" claude -p "Read review.txt. It is a code review. Extract ONLY findings that the review presents as a genuine BUG, security problem, or resource-handling error in the code — something that would misbehave at runtime. EXCLUDE anything the review frames as a nit, style, naming, typing or annotation preference, a missing test, missing documentation, a suggestion, or an informational note, however it is labelled. Output ONLY a JSON array and no prose: [{\"line\": <integer>, \"category\": \"security|correctness|resource-leak\", \"summary\": \"<one sentence>\"}]. Use the line number the review gives. If it reports no genuine bug, output []." \
186190
--setting-sources=project --output-format=stream-json --verbose < /dev/null 2>/dev/null \
187191
| jq -rs '[.[]|select(.type=="assistant")|.message.content[]?|select(.type=="text")|.text]|join("")' 2>/dev/null )
188192
printf '%s' "$out" | grep -o '\[[^][]*\]' | tail -1
189193
}
190194

191195
score() { # <fixture> <findings>
192-
local f="$1" found="$2"
196+
local f="$1" found="$2" planted
193197
[ -n "$found" ] || found='[]'
194198
printf '%s' "$found" | jq -e . >/dev/null 2>&1 || found='[]'
199+
# The denominator comes from ground truth and never from the scoring path.
200+
#
201+
# It used to fall back to {"hits":0,"planted":0,"fp":0} whenever the jq scoring failed, so a
202+
# review that errored or timed out removed its own planted defect from the total instead of
203+
# counting as a miss. That silently inflates recall for whichever arm fails more often, and it
204+
# is why one arm reported 34 planted where the others reported 35 — a difference that read as a
205+
# rounding detail and was actually four swallowed failures.
206+
planted=$(jq --arg f "$f" '[.fixtures[]|select(.file==$f)|.planted[]?]|length' "$GT" 2>/dev/null)
207+
[ -n "$planted" ] || planted=0
195208
jq -n --argjson g "$(jq --arg f "$f" '.fixtures[]|select(.file==$f)' "$GT")" \
196209
--argjson found "$found" --argjson tol "$TOL" '
197210
($g.planted // []) as $p
198211
| { hits: ([$p[] | . as $pl | select(($found|map(select(((.line-$pl.line)|fabs)<=$tol))|length)>0)]|length),
199212
planted: ($p|length),
200213
fp: ([$found[] | . as $fd | select(($p|map(select(((($fd.line)-.line)|fabs)<=$tol))|length)==0)]|length) }' \
201-
2>/dev/null || echo '{"hits":0,"planted":0,"fp":0}'
214+
2>/dev/null || printf '{"hits":0,"planted":%s,"fp":0}' "$planted"
202215
}
203216

204217
NFIX=$(jq -r '.fixtures|length' "$GT")
205218
RESULTS=""
219+
220+
# Per-run results are appended to a log as they complete, and progress goes to stderr.
221+
#
222+
# The first version accumulated everything in a shell variable and printed one table at the very
223+
# end. A 120-review run then produced a zero-byte output file for ninety minutes with no way to
224+
# tell progress from a hang, and killing it — which is what eventually happened — threw away 83
225+
# completed reviews. Anything that takes hours has to be observable while it runs and survivable
226+
# when it does not.
227+
RUNLOG="${RUNLOG:-$ROOT/runs.tsv}"
228+
printf 'arm\tfixture\tsample\thits\tplanted\tfp\tentered\n' > "$RUNLOG"
229+
TOTAL_RUNS=$(( $(printf '%s' "$ARMS_CSV" | tr ',' ' ' | wc -w) * NFIX * SAMPLES ))
230+
DONE_RUNS=0
206231
for a in $(printf '%s' "$ARMS_CSV" | tr ',' ' '); do
207232
H=0; P=0; FP=0; ENT=0; RUNS=0
208233
for f in $(jq -r '.fixtures[].file' "$GT"); do
@@ -217,6 +242,11 @@ for a in $(printf '%s' "$ARMS_CSV" | tr ',' ' '); do
217242
FP=$((FP + $(printf '%s' "$sc" | jq -r '.fp')))
218243
[ "$ent" = yes ] && ENT=$((ENT+1))
219244
RUNS=$((RUNS+1))
245+
DONE_RUNS=$((DONE_RUNS+1))
246+
printf '%s\t%s\t%s\t%s\t%s\t%s\t%s\n' "$a" "$f" "$s" \
247+
"$(printf '%s' "$sc" | jq -r '.hits')" "$(printf '%s' "$sc" | jq -r '.planted')" \
248+
"$(printf '%s' "$sc" | jq -r '.fp')" "$ent" >> "$RUNLOG"
249+
printf '\r %s/%s %-8s %-18s sample %s ' "$DONE_RUNS" "$TOTAL_RUNS" "$a" "$f" "$s" >&2
220250
done
221251
done
222252
RESULTS="$RESULTS$a|$H|$P|$FP|$ENT|$RUNS
@@ -231,6 +261,7 @@ if [ "$JSON" = 1 ]; then
231261
exit 0
232262
fi
233263

264+
printf '\n' >&2
234265
printf 'review-pathway benchmark — %s fixtures x %s sample(s), each harness through its own /review\n\n' "$NFIX" "$SAMPLES"
235266
printf '%-8s %-8s %-9s %-17s %s\n' "arm" "found" "planted" "false positives" "pathway entered"
236267
printf '%-8s %-8s %-9s %-17s %s\n' "--------" "--------" "---------" "-----------------" "---------------"

0 commit comments

Comments
 (0)