pr_runtime pipeline v0.3: SWE-bench-style PR mining with sandbox verification - #4
Merged
Conversation
…ification) End-to-end implementation of the second pipeline. Generation auto-triggers bootstrap when the pipeline declares `requires_bootstrap=True`; for each candidate PR the validation harness runs the test suite twice inside the bootstrap container (once with test_patch alone, once with patch + test_patch) to derive the FAIL_TO_PASS and PASS_TO_PASS oracle sets. Phase 0 — Bootstrap orchestration - New `Pipeline.requires_bootstrap: ClassVar[bool]` (default False) - Pipeline.__init__ gains optional `bootstrap` arg (BootstrapResult | None) - cmd_generate inspects requires_bootstrap and calls ensure_bootstrap() inside a BootstrapView before constructing the pipeline; cache makes repeat runs free - New --language / --base-image / --max-spend-usd / --force-bootstrap flags on `generate` mirror the `bootstrap` verb Phase 1 — PRRuntimePipeline + mining - New `src/repo2rlenv/pipelines/pr_runtime.py` (registered as "pr_runtime") - Diff-split: `split_patch_and_test_patch()` mirrors SWE-bench's keyword- on-path heuristic (test/tests/e2e/testing); renames into tests/ count - Lite-style filters: `lite_filter=True` requires single source file, ≥40-word problem, no images/external-links/commit-SHAs in body - `build_eval_script()` builds tests/test.sh per SWE-bench's make_eval_script_list_common: reset → apply test_patch → run with START/END markers → reset - PRRuntimeOptions extends with validation + lite-filter fields Phase 2 — Validation harness + log parser + grading - New `pipelines/pr_runtime_validate.py:validate_pr()` runs the two-stage test execution inside a shared DockerSandbox (git reset --hard between stages keeps PRs isolated) - New top-level `log_parsers/` package; `pytest_parser.parse_pytest()` recognises PASSED/FAILED/SKIPPED/ERROR lines; handles parametrized names + `SKIPPED [N] ...` count prefixes - reward.py gains `ExecutionReport` + `grade_test_execution()` — computes f2p_rate/p2p_rate/resolution_status (FULL/PARTIAL/NO), matching SWE-bench's grading.py semantics Phase 3 — Harbor emitter - HarborTask gains optional `environment_dockerfile` + `test_script` - write_harbor_task emits `environment/Dockerfile` and `tests/test.sh` (mode 0o755) when set; reward_kinds auto-upgrades to ["test_execution", "diff_similarity"] for sandbox-required tasks Phase 4 — LiteLLM + budget - Audited: all LLM calls go through `llm.complete()` → litellm - `--max-spend-usd` now available on both `bootstrap` and `generate`; budget is enforced inside the agent loop (already in v0.2) Tests: 97/97 passing (was 71). New coverage: - test_pipeline_pr_runtime.py — diff split, eval script, contract - test_log_parsers.py — pytest parser edge cases - test_grading.py — F2P/P2P math + resolution status - test_emitter.py — environment/Dockerfile + tests/test.sh emission - test_pipeline_contract.py — every pipeline declares requires_bootstrap Reference implementations studied (not vendored): SWE-bench's collect/utils.py:extract_patches, harness/test_spec/utils.py:make_eval_script_list_common, harness/grading.py:get_resolution_status. Independent Apache-2.0 impl, acknowledgement blocks in pr_runtime.py + log_parsers/__init__.py.
Two bugs surfaced by the first real run on pallets/click: 1. DockerSandbox.start() always tried `docker pull` before run, which fails for local-only bootstrap images (the ones we commit but never push). Fix: check `docker image inspect` first; only pull if not already cached locally. 2. Bootstrap typically records `pytest --collect-only` as test_cmds (the smoke gate's fast/tolerant verification command). That command only enumerates tests and never emits PASSED/FAILED lines, so the pr_runtime log parser sees nothing and F2P/P2P always come back empty. Fix: add normalize_test_cmds_for_runtime() that strips --collect-only / --co and adds -v when the command is pytest. Applied at both the validation call site and the eval-script construction site so the Harbor task.sh that ships in each dataset uses the actual test runner. Verified on pallets/click: bootstrap cache hit, 10 of 20 PRs emitted with skip_validation=true; with validation enabled, 4 PRs ran the full two-stage harness (no F2P found because recent click PRs are docs-only, which is the correct null result, not a bug). 101/101 tests pass.
…ub artifact only
Collaborator
Author
|
@codex please review this implementation end to end |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04b3f7989b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Addresses both P1 findings on PR #4 plus the structural-quality recommendations from the data-quality audit on the first emission run. Codex P1 fixes - pytest parser: `pytest -v` emits `tests/foo.py::test_x PASSED` (status at end), not `PASSED tests/foo.py::test_x` (status at start). The old parser only matched the start-of-line form, so pre/post_status came back empty and F2P was always 0. Now matches BOTH formats; falls back to summary-line semantics when both appear for the same test. - validate_pr: bootstrap clones at --depth 1, so historical PR base commits aren't in the container's object database. `git reset --hard <sha>` was silently failing. New `_fetch_base_commit()` does `git fetch --depth 1 origin <sha>` (falls back to --unshallow if the server refuses by-sha fetch). Quality filters - Path-component classifier (was substring match): files under docs/ are NEVER classified as test files, even when path contains "test"/"testing". Kills the `docs/testing.md` and `src/click/testing.py` false positives that polluted v0.3's first emission. - Skip CI-only PRs: source patch 100% under `.github/` → skip with reason `ci_only_patch`. Gated on options.skip_ci_only (default True). - Require new test functions in test_patch: typo/comment-only test_patches can't produce a FAIL_TO_PASS oracle. `_count_new_test_funcs()` matches `+def test_*`, `+class .*Test*`, `+func Test*` (Go), `+it(/test(/describe(` (JS). Gated on options.require_new_test_funcs (default True). Performance - `targeted_test_cmds_for_pr()`: limits the pytest invocation to the files touched by the PR's test_patch, instead of running the whole suite. SWE-bench-Live does this and it cuts validation walltime ~10-50x. Only applies to pytest invocations; other runners (go test, npm test) pass through unchanged. Verified on pallets/click (limit=20): before: 10 emitted, 5 training-worthy (50%) after: 5 emitted, 5 training-worthy (100%) skip_reasons: {'no_test_patch': 13, 'no_new_test_funcs': 2} Tests: 113/113 passing (was 101). New coverage: - parser handles verbose progress format + ignores random lines - path classifier rejects docs/, examples/, src/.../testing.py - structural filters: CI-only skip, new-test-func count (Py/Go/JS) - targeted_test_cmds_for_pr behavior
Collaborator
Author
|
@codex please review this in detail |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7781966149
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…sitioning
Two real correctness bugs that would make emitted tasks unusable by Harbor:
1. build_eval_script swallowed the test exit code. The final
`git checkout ... || true` always succeeds, so the script's exit was
the cleanup's exit (0) even if pytest failed. Harbor's verifier reads
the exit code to decide pass/fail; with this bug, every model patch
looks like a success. Fix: capture `TEST_EXIT_CODE=$?` immediately
after the test block, then `exit $TEST_EXIT_CODE` after cleanup.
2. Emitted Dockerfile didn't reposition the working tree to the PR's
base_commit. The bootstrap image has the repo at bootstrap-time HEAD;
each PR has its own base_commit (usually different). Model patches
are authored against base_commit's line context, so applying them
against HEAD's lines causes hunk failures even for correct patches.
Fix: new `build_environment_dockerfile()` adds a `git fetch <base_sha>`
(no-op if already in shallow clone, falls back to --unshallow) and
`git reset --hard <base_sha>` at image build time.
Third codex finding ("Fetch PR base commits before resetting" on
pr_runtime_validate.py:69) is a false positive — `_fetch_base_commit()`
already runs in `validate_pr()` orchestration BEFORE any stage script.
Codex appears to read the stage-script helper in isolation.
Tests: 115/115 passing (+2: exit-code preservation, dockerfile reset to
base_commit). Smoke-regenerated 8 click PRs and confirmed both fixes
appear in the emitted Dockerfile + test.sh.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/repo2rlenv/pipelines/pr_runtime.py— the first sandbox-verified pipeline. For each merged PR we split the unified diff intopatch(source) andtest_patch(tests) by SWE-bench's keyword-on-path heuristic, then optionally run the test suite twice inside the bootstrap container to derive theFAIL_TO_PASSandPASS_TO_PASSoracle sets.cmd_generate: pipelines declarerequires_bootstrap: ClassVar[bool]; cmd_generate callsensure_bootstrap()with the live UI before constructing the pipeline. Cache hit ⇒ instant. The newPipeline.__init__signature takes an optionalbootstrap: BootstrapResult | Nonearg.log_parsers/top-level package withparse_pytest()(PASSED/FAILED/SKIPPED/ERROR detection frompytest -voutput).reward.pywithExecutionReport+grade_test_execution()— implements SWE-bench's FULL / PARTIAL / NO resolution semantics.HarborTask+write_harbor_taskwith optionalenvironment/Dockerfile(FROM <bootstrap_image>) andtests/test.sh(mode 0o755). Lite tasks (pr_diff) skip both; sandbox tasks (pr_runtime) emit them and Harbor reads them as the verifier.--max-spend-usd/--language/--base-image/--force-bootstraptogenerate(mirrors thebootstrapverb so spend caps and overrides work uniformly).Closes #3.
Live verification
Bootstrap cache hit + 20-PR scan on
pallets/click:Each task contains
task.toml+instruction.md+solution/patch.diff+environment/Dockerfile+tests/test.sh. The test.sh is the SWE-bench-style eval script: reset test files → applytest_patch(heredoc +git apply --reject) → run tests withSTART_TEST_OUTPUT/END_TEST_OUTPUTmarkers → reset.Bugs that surfaced during the first run and got fixed before merge:
DockerSandbox.start()always trieddocker pulleven for local-only bootstrap images. Fix: checkdocker image inspectfirst; only pull if not already cached locally.pytest --collect-onlyastest_cmds(the smoke gate's fast/tolerant verifier). That never emits PASSED/FAILED lines, so F2P/P2P came back empty. Fix:normalize_test_cmds_for_runtime()strips--collect-onlyand adds-vfor pytest.Architecture
Reference work studied (not vendored)
swebench/collect/utils.py:extract_patchessplit_patch_and_test_patch()swebench/collect/build_dataset.py:is_valid_pull_pre_filter()inPRRuntimePipelineswebench/harness/test_spec/utils.py:make_eval_script_list_commonbuild_eval_script()swebench/harness/log_parsers/python.py:parse_log_pytestlog_parsers/pytest_parser.py:parse_pytestswebench/harness/grading.py:get_resolution_statusreward.py:ExecutionReport.resolution_statusIndependent Apache-2.0 implementations with acknowledgment blocks (matches the posture established by
bootstrap/__init__.pyandreward.py).Test plan
uv run pytest -q— 101/101 passing (was 71; +30 new tests)tests/test_pipeline_pr_runtime.py— diff split, eval script, normalize_test_cmds, contracttests/test_log_parsers.py— pytest parser edge cases (parametrized names, SKIPPED [N] prefix, last-write-wins)tests/test_grading.py— F2P/P2P arithmetic + resolution status edge casestests/test_emitter.py— environment/Dockerfile + tests/test.sh emission + executable bittests/test_pipeline_contract.py— every registered pipeline declaresrequires_bootstrapOut of scope (v0.4 follow-ups)
pr_streampipeline (continuous mining, wrapspr_runtimewith a scheduler)File summary