test(recursion): memory-bounded execute/prove smoke tests; wire CI#750
Conversation
Stream the execute-only path via Executor::resume() and read the committed marker straight off Memory (Executor::finish), dropping the run()+Traces build that OOM'd. Prove the outer recursion guest memory-bounded via prove_and_verify_continuation (2^20-cycle epochs) instead of a monolithic ~125 GB prove. Un-ignore the three execute-only tests (now sub-minute); the full-prove tests stay #[ignore]d (>60s) and run in the comprehensive CI job. CI: build the recursion guest ELFs (cache + conditional setup-rust + make compile-recursion-elfs) in both the per-PR and comprehensive prover jobs, and add test(test_recursion_prove) to the comprehensive filter.
|
/ai-review |
Codex Code ReviewFindings:
I did not find safety/security issues in the changed Rust code. Static review only; no builds/tests run per instructions. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Review: test(recursion) — memory-bounded execute/prove smoke tests; wire CI Verified the core behavioral changes against their dependencies. The PR is sound; one CI caching gap worth addressing. Correctness (OK):
Findings:
CI conditional logic (OK): Nothing else flagged. |
AI ReviewPR #750 · 2 changed files Findings
Status column reflects the verdict from the verifier: deepseek-verifier (openrouter/deepseek/deepseek-v4-pro). AI-001: seed-elf-cache does not seed recursion ELFs, so every merge-queue entry recompiles them
Claim Adding a recursion ELF cache to test-prover/test-prover-comprehensive is undermined because the seed-elf-cache job (the only job that writes caches on main) doesn't seed recursion ELFs. Per the comment block above the seed job, caches are branch-scoped and merge_group branches only read from main, so the new recursion cache will always miss on merge runs and compile-recursion-elfs will run every time — the very problem seed-elf-cache was added to solve. Evidence The seed-elf-cache job (lines 426-466) only adds ASM and Rust cache/compile steps; it has no 'cache-recursion-elfs' or 'make compile-recursion-elfs' steps, unlike the test-prover/test-prover-comprehensive jobs which the PR newly adds them to. The job's own comment says "without this job no ELF cache is ever saved on main and every merge-queue entry recompiles" — that is now literally true for the recursion ELFs. Suggested fix Add the same recursion cache/compile (and conditional Setup Rust) steps to the seed-elf-cache job that the test-prover job now uses, with the same cache key and path, so the recursion artifacts are persisted on main and available to merge-queue runs. AI-002: test_recursion_execute_* now run on every test-prover shard and may time out
Claim The PR removes #[ignore] from test_recursion_execute_empty / test_recursion_execute_1query / test_recursion_execute. These tests previously documented tens of GB and were excluded from CI; the streaming resume() path avoids the trace OOM but the executor still runs the full STARK verifier in-VM (the verifier's cycles are unchanged), so wall time stays high. With the same #[ignore] still present on test_recursion_prove_* but not on the execute_* tests, only ONE of the two tiers is in CI now — likely an unstated intent — and the new CI test load is now on every shard. Evidence Diff removes Suggested fix Either keep the execute-only tests #[ignore]d until they are confirmed fast in CI (matching the still-ignored prove tests), or add an explicit cycle timeout / step bound to resume()/execute_outer_and_commit so the test cannot hang a shard for hours. If intentional, also keep the Makefile/SPEC docs in sync — see next finding. AI-003: Makefile comment claims recursion tests are #[ignore]d but execute_* tests no longer are
Claim The Makefile comment block at line 144-147 states "the recursion smoke tests are #[ignore]d (not run by Evidence Grep in prover/src/tests/recursion_smoke_test.rs shows #[ignore] only on test_recursion_prove_empty, test_recursion_prove_1query, test_recursion_prove (and the separate roundtrip test), while the three test_recursion_execute_* functions have no #[ignore]. The Makefile comment from the diff base still says all of them are #[ignore]d. Suggested fix Update the Makefile comment to reflect the split: the execute-only tests now run in CI shards, the prove-tier tests still only run via Reviewer Lanes
Verification Lanes
Native Codex and Claude reviews run separately and post their own comments. They are not included in this structured provenance report. Raw lane outputs, candidates, final issues, and model metrics are uploaded as workflow artifacts. |
… epochs The prove tier's continuation bundle retains every epoch's STARK proof in RAM, so it OOMs CI runners. Narrow the CI selector to test_recursion_execute (streaming, bounded) and leave the prove tier manual-only with a 2^16 epoch to cap the per-epoch trace+LDE spike.
3 participants
Stream the execute-only path via Executor::resume() and read the committed marker straight off Memory (Executor::finish), dropping the run()+Traces build that OOM'd. Prove the outer recursion guest memory-bounded via prove_and_verify_continuation (2^20-cycle epochs) instead of a monolithic ~125 GB prove.
CI: build the recursion guest ELFs (cache + conditional setup-rust + make compile-recursion-elfs) in the comprehensive prover jobs, and add test(test_recursion) to the comprehensive filter.