fix(bench-verify): Measure each recursion ref's guest cycles with a CLI built from that ref

[nicole-graus]

Motivation

/bench-verify's recursion cycle bench builds a single measuring CLI from the workflow checkout, which, in the issue_comment flow, is the default branch (main), and runs it against both the baseline and PR guest ELFs. That works only while every guest uses syscalls main already knows. The moment a PR's guest emits a new syscall the main executor aborts with ExecutionError(UnknownSyscall(...)) and the cycle bench fails, even though the PR is correct. PR #818 (wiring the recursion verifier's Fp3 mul-add through the FEXT accelerator) is the first to hit this: its guest emits FEXT_LOAD (u64::MAX - 19 = 18446744073709551596), which main doesn't recognize. It will recur for any future PR that adds an accelerator/syscall.

Description

Build the measuring CLI per ref instead of once: the baseline guest is measured by a cli built from the baseline ref and the PR guest by a cli built from the PR ref, each inside the worktree the script already creates for that ref's guest build. Each VM therefore understands exactly its own guest's syscall, robust for PRs that add or remove syscalls in either direction. This mirrors the existing per-side build in scripts/bench_verify.sh. The result-cache key drops its counter-SHA component (the ref SHA now identifies the counter) and the CLI build shares HOST_TARGET_DIR with the blob-dump build so deps stay warm. Only scripts/bench_recursion_cycles.sh changes behavior; the workflow gets a comment update.

[nicole-graus]

/bench-verify

[github-actions]

⏳ Benchmark started on the bench server. The recursion-guest cycle comparison adds guest builds on top of the verifier bench, longer on a cold runner. The bench server is occupied until it finishes.

[nicole-graus]

/ai-review

[github-actions]

Codex Code Review

• Low — scripts/bench_recursion_cycles.sh:290: The capability check searches only bin/cli/src/main.rs for a literal output string. A PR that moves cycle reporting into another module or changes formatting while retaining the feature will be rejected before measurement. Check CLI behavior/output after building, or search the whole CLI source tree.

[claude]

Review — no significant issues

Reviewed the per-ref measuring-CLI change (scripts/bench_recursion_cycles.sh) and the workflow comment update. This is a well-reasoned, carefully-documented fix and I found no correctness, safety, or performance problems.

Verified:

• No dangling references. Removing the step-1 MEASURE_CLI/HEAD_SHA block is complete — every runtime use is rewritten to the per-ref $measure_cli, the result-cache key drops _m${HEAD_SHA}, and all echo/error/parse messages are updated. No stray HEAD_SHA/MEASURE_CLI remains.
• Path assumptions hold. The new pre-dump guard greps $wt/bin/cli/src/main.rs for Keccak calls: — that path and string exist on main (feat(cli): report keccak/ecsm accelerator call counts under execute --cycles #807). The cli package's default bin name is cli, so cp "$CARGO_TARGET_DIR/release/cli" and the $wt/target/release/cli fallback are both correct.
• Atomicity is sound. The cp → $measure_cli.tmp → mv -f sequence prevents a killed run from leaving a truncated-but-executable binary that the [ -x ] reuse check would trust, matching the comment and the existing result-file pattern. The .tmp lives in $WORK, so the mv is intra-filesystem/atomic even when HOST_TARGET_DIR is on another FS.
• Caching/pruning consistent. measure_cli_<sha8> is keyed on SHA only, so it's built once per ref and reused across all four presets (presets run sequentially in run_recursion_bench.sh) — no intra-job race on the shared .tmp. Prune globs now cover measure_cli_<sha8>* and build_cli_<sha8>.log, and the one-time sweep removes the retired fixed-name artifacts that escaped per-SHA globs.

One thing worth stating explicitly (not a defect — I believe it's the correct tradeoff): each ref now counts cycles with its own VM rather than a single shared counter, so a delta reflects "total verifier work for each side's own proof." That's exactly what the header comment already frames, and it's the only scheme that survives a PR introducing/removing a syscall — the shared-main counter would otherwise abort with UnknownSyscall. Good call.

[github-actions]

Verifier benchmark — 05e2be02d6 vs main (20 pairs)

=== Verify ABBA result ===

Metric	main	PR	Δ
Verify time	3.483s	3.478s	-0.13% ⚪
Proof size	204.30 MiB	204.30 MiB	+0.00% ⚪

  pairs: 20   mean A (PR): 3.478s   mean B (main): 3.483s
  [parametric] paired-t   mean -0.13%   sd 0.40%   se 0.09%
               95% CI: [-0.31%, +0.06%]   (t df=19 = 2.093)
  [robust]     median -0.09%   Wilcoxon W+=57 W-=133  p(exact)=0.1311  (z=-1.51)

  run-to-run jitter:    A CV 0.28%   B CV 0.28%        (lower = steadier)
  within-session drift: -0.24% over the run, 1st->2nd half -0.11%

⚪ INCONCLUSIVE — effect not separable from 0 at n=20 (point estimate ~-0.09%). Add pairs to resolve.

_{Drift-free interleaved A/B/B/A measurement. - = PR faster. Trust the verdict when paired-t and Wilcoxon agree.}

---

Recursion guest cycles (main vs PR)

=== Recursion-guest cycle comparison — single query (blowup=2, 1 query) — deterministic to ~±100k cycles ===
REF_B (baseline) origin/main 528a841 guest=recursion-min.elf
REF_A (PR) 05e2be0 05e2be0 guest=recursion-min.elf

Metric	REF_B (baseline)	REF_A (PR)	Δ (A-B)
Guest cycles	41.7M	41.7M	-0.0M (-0.06%)
Keccak calls	3025	3025	0

note: cycles reproduce to ~±100k (build codegen + proof nondeterminism); treat sub-100k deltas as noise, not signal.

raw (exact integer counts)

ref_b_sha=528a8411bb641a1290c0794cdd79c76eba76c739 ref_b_elf=recursion-min.elf ref_b_cycles=41749851 ref_b_keccak=3025 ref_b_execute_wall_s=2
ref_a_sha=05e2be02d63242cd38cbe56fd3f4d7a8e21cb50b ref_a_elf=recursion-min.elf ref_a_cycles=41726358 ref_a_keccak=3025 ref_a_execute_wall_s=1
delta_cycles=-23493 delta_keccak=0

(blowup2 regime unavailable for these refs — see the workflow log.)

(blowup4 regime unavailable for these refs — see the workflow log.)

=== Recursion-guest cycle comparison — 128-bit (blowup=4, 110 queries) — real ethrex block, 4 transfers — deterministic to ~±100k cycles ===
REF_B (baseline) origin/main 528a841 guest=recursion-cont-blowup4.elf
REF_A (PR) 05e2be0 05e2be0 guest=recursion-cont-blowup4.elf

Metric	REF_B (baseline)	REF_A (PR)	Δ (A-B)
Guest cycles	1470.1M	1470.2M	+0.1M (+0.01%)
Keccak calls	1755694	1755646	-48

note: cycles reproduce to ~±100k (build codegen + proof nondeterminism); treat sub-100k deltas as noise, not signal.

raw (exact integer counts)

ref_b_sha=528a8411bb641a1290c0794cdd79c76eba76c739 ref_b_elf=recursion-cont-blowup4.elf ref_b_cycles=1470095496 ref_b_keccak=1755694 ref_b_execute_wall_s=22
ref_a_sha=05e2be02d63242cd38cbe56fd3f4d7a8e21cb50b ref_a_elf=recursion-cont-blowup4.elf ref_a_cycles=1470209700 ref_a_keccak=1755646 ref_a_execute_wall_s=23
delta_cycles=114204 delta_keccak=-48

[github-actions]

AI Review

PR #856 · 2 changed files

Findings

Status	Sev	Location	Finding	Found by
confirmed	low	scripts/bench_recursion_cycles.sh:290	Keccak-counter preflight check greps a brittle source string	kimi openrouter/moonshotai/kimi-k2.7-code

Status column reflects the verdict from the verifier: deepseek-verifier (openrouter/deepseek/deepseek-v4-pro).

AI-004: Keccak-counter preflight check greps a brittle source string

• Status: confirmed
• Severity: low
• Location: scripts/bench_recursion_cycles.sh:290
• Found by: kimi:openrouter/moonshotai/kimi-k2.7-code
• Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
• Rejected by: -

Claim

The up-front compatibility check for the execute --cycles keccak/ecsm counters greps for the literal string Keccak calls: in bin/cli/src/main.rs. This is brittle: if the output string moves to a different module, is refactored into a constant, or the source file is renamed/moved, the heuristic fails even though the feature exists (or vice-versa if a comment contains the string).

Evidence

Line 290: if ! grep -q "Keccak calls:" "$wt/bin/cli/src/main.rs" 2>/dev/null; then. The parse in step 2d also expects this exact string, so the heuristic is internally consistent but tied to one source file's text rather than to the built artifact's actual behavior.

Suggested fix

Replace the source grep with a probe of the built CLI: either run cli execute --help and check for the --cycles flag, or build the CLI earlier and do a dry-run against a tiny fixture to verify the output contains the expected Cycles: and Keccak calls: lines.

Reviewer Lanes

Lane	Model	Prompt	Status	Findings
glm	openrouter/z-ai/glm-5.2	general	success	0
kimi	openrouter/moonshotai/kimi-k2.7-code	general	success	2
minimax	minimax/MiniMax-M3	general	error: opencode failed (provider/auth/runtime error) and no findings were submitted	0
moonmath	zro/minimax-m3	general	success	3
nemotron	openrouter/nvidia/nemotron-3-ultra-550b-a55b	general	success	2

Verification Lanes

Lane	Model	Status	Confirmed	Rejected	Uncertain
deepseek-verifier	openrouter/deepseek/deepseek-v4-pro	success	1	5	0

Native Codex and Claude reviews run separately and post their own comments. They are not included in this structured provenance report.

Discarded candidates (5) — rejected by the verifier

• Prune function references undefined variable sha8 (scripts/bench_recursion_cycles.sh:126, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — Line 126 of the actual file reads "$WORK"/build_cli_"${s8}".log — it uses ${s8}, not ${sha8}. The variable s8 is defined locally at line 120 (s8="$(basename "$wt")"; s8="${s8#wt_}"). The claim that ${sha8} is an undefined variable is based on a misreading of the code; no unbound-variable error would occur.
• Per-ref measuring CLI loses counter-comparability caveat (scripts/bench_recursion_cycles.sh:17, found by kimi:openrouter/moonshotai/kimi-k2.7-code) — The per-ref CLI is a deliberate, well-documented design change explained across multiple comment blocks (lines 17-35, 153-158, 282-289, 350-360). The rationale is explicit: a shared main-built CLI aborts with UnknownSyscall(...) when a PR introduces a new syscall. The existing header comment (lines 10-15) already warns that proof-structure changes can conflate with guest-code deltas. This is an intentional engineering trade-off, not a defect.
• Result cache key change leaves orphaned old-format files on long-lived runners (scripts/bench_recursion_cycles.sh:205, found by moonmath:zro/minimax-m3) — The prune function at line 123 uses glob result_${s8}_*.txt which matches BOTH old-format (_m<head_sha> suffix) and new-format files. While old-format caches for SHAs whose worktrees aren't yet pruned would remain until the worktree ages out, these files are trivially small (~4 lines each) and will eventually be cleaned. The comment at line 203 says 'naturally ignores' in the sense of cache-read semantics, not cleanup. This is too speculative to be a meaningful issue.
• Keccak guard fires after the (also expensive) guest build, contradicting the "up front" intent (scripts/bench_recursion_cycles.sh:290, found by moonmath:zro/minimax-m3, nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The keccak guard at line 290 fires after the guest build (step 2a, lines 250-258) but before the blob dump (step 2c, line 295+). The comment at line 287 says 'Refuse up front (before the expensive blob dump)' — it specifically claims to be before the blob dump, not before the guest build. The guard is placed consistently with all other 'ref predates X' guards (lines 301-308) which also run after the guest build. The claim that it 'contradicts the up front intent' is incorrect.
• Per-ref CLI binary cache relies only on [ -x ] check, no content validation (scripts/bench_recursion_cycles.sh:362, found by moonmath:zro/minimax-m3) — The [ -x ] check at line 362, combined with the atomic tmp+mv pattern (lines 371/378 cp to .tmp, line 380 mv), is the standard caching idiom for binaries in shell scripts. The claim acknowledges this handles the partial-copy case, then speculates about 'post-mv corruption' from disk errors or manual intervention. That level of filesystem integrity is outside the scope of a bash benchmark script; if disk corruption occurs, many other artifacts would also be compromised. The code's comment at lines 356-359 explicitly explains the atomicity protection, which is sufficient for the real failure modes.

Raw lane outputs, candidates, final issues, and model metrics are uploaded as workflow artifacts.