Repository navigation
feat(swe-together): add interactive coding benchmark - #4943
Conversation
svcnvidia-nemo-ci
added
area:environment
0cb0781 to
85145f0
Compare
|
/ok to test 886abac |
svcnvidia-nemo-ci
added
the
needs-review
svcnvidia-nemo-ci
left a comment
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
HOLD — 2 BLOCKERs, 4 RISKs, and 1 NOTE.
The findings cover a dependency-policy bypass, incorrect Intent Coverage scores, runtime recovery and concurrent cleanup failures, replacement of a running OpenCode executable, and gaps in smoke/simulator evidence.
Reviewed all 67 changed files and traced the affected sandbox lifecycle, scoring, agent-session, and collection contracts. This was a static, read-only review; tests and real-model rollouts were not executed. Suggested patches require author verification.
Note: formal approval is not available for this request, so this review is posted as a comment only.
Review generated by codex. This was a full review. Request a full review with /review reset=true.
| def fork_turn(self) -> "UserAgent": | ||
| """Stage a consultation independently; the caller commits it only before its deadline.""" | ||
| proposed = copy(self) | ||
| proposed._messages = deepcopy(self._messages) |
There was a problem hiding this comment.
NOTE — Discarded simulator turns still mutate accepted action counters
Failure: fork_turn() shallow-copies _counts. After proposed = sim.fork_turn(); await proposed.process(...), discarding proposed leaves sim.get_stats()["action_breakdown"] incremented, while its total_calls, message counts, and history remain unchanged. This occurs when a completed consultation is rejected by the post-call deadline check, producing contradictory accepted-state diagnostics.
Correction: Add this in fork_turn() after the shallow copy:
proposed._counts = self._counts.copy()Keep _llm shared intentionally so discarded/cancelled calls retain transport diagnostics, while every mutable accepted-turn counter remains isolated until commit.
Preservation / proposed checks, not run: Complete a no-op and a message consultation on separate forks, discard them, and assert the original stats/history are unchanged. Commit a completed fork and assert its scalar counters and action breakdown advance together. Retain the accepted-feedback/no-op-stop behavior and verify the shared model-call diagnostics still include discarded consultations.
|
/review |
|
/ok to test c6fefce |
svcnvidia-nemo-ci
left a comment
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
HOLD — 4 RISKs and 1 NOTE remain from review 5625453045.
Both earlier BLOCKERs are fixed in the current source: PyPI artifact parsing now recognizes the reported hyphenated distribution, with package-boundary and proxy-forwarding regression cases. Empty-intent scoring now delegates to the deterministic scorer, with zero-message and accepted-feedback assertions.
Accounted for all 67 changed files as required by the pinned skill. Discovery remained follow-up-scoped: verified the seven prior counterexamples against the current head, compared the fixes with the previous-review tree, and traced directly affected lifecycle, scoring, proxy, and evidence contracts. No additional escalation-eligible regression was validated.
Static, read-only review only; tests and model rollouts were not executed. Suggested code and proposed acceptance checks require author verification.
Note: formal approval is not available for this request, so this review is posted as a comment only.
Review generated by codex. This was a findings-only review. Request a full review with /review reset=true.
| def fork_turn(self) -> "UserAgent": | ||
| """Stage a consultation independently; the caller commits it only before its deadline.""" | ||
| proposed = copy(self) | ||
| proposed._messages = deepcopy(self._messages) |
There was a problem hiding this comment.
NOTE — Discarded simulator turns still mutate accepted action counters
Failure: Prior finding 4357350680 remains unresolved. fork_turn() shallow-copies _counts. With a fresh simulator, complete a no-op consultation on proposed = sim.fork_turn() and discard it: the original reports action_breakdown={"no-op": 1} while total_calls=0, its message counters and history remain unchanged. This reaches retained diagnostics when the post-call deadline check rejects a completed consultation. Accepted-state diagnostics should exclude that turn.
Correction: Add immediately after proposed = copy(self) in fork_turn():
proposed._counts = self._counts.copy()Keep _llm shared intentionally: discarded/cancelled consultations must retain transport diagnostics, while mutable accepted-turn counters stay isolated until commit.
Preservation / acceptance — proposed, not run: Complete separate no-op and message consultations on forks, discard them, and assert the original's complete stats/history are unchanged. Commit a completed fork and assert its scalar counters and action breakdown advance together exactly once. Use a fake shared model client that records calls and assert discarded consultations still appear there. Retain accepted-feedback cursor advancement and four-no-op stopping.
|
/review |
|
/ok to test 87c4a4f |
svcnvidia-nemo-ci
left a comment
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
SHIP WITH CARE — the four prior RISKs are fixed; one nonblocking diagnostic NOTE remains.
Verified fresh-helper/provider recovery and serialized bundle release in runtime preparation, including the recovery, cancellation, concurrency, and deletion-retry assertions. Offline seeding now verifies and atomically publishes a separate executable inode, with running-process and invalid-seed preservation cases. Smoke qualification now requires identity evidence on every activation, with matching, missing, invalid, and switched identity cases.
Accounted for all 67 changed files. Discovery remained follow-up-scoped: compared the fixes with the previous-review tree, verified all five latest prior findings, and traced the directly affected lifecycle and evidence contracts. No additional escalation-eligible regression was validated.
Static, read-only review only; tests and model rollouts were not executed. Suggested code and proposed checks require author verification.
Note: formal approval is not available for this request, so this review is posted as a comment only.
Review generated by codex. This was a findings-only review. Request a full review with /review reset=true.
| def fork_turn(self) -> "UserAgent": | ||
| """Stage a consultation independently; the caller commits it only before its deadline.""" | ||
| proposed = copy(self) | ||
| proposed._messages = deepcopy(self._messages) |
There was a problem hiding this comment.
NOTE — Discarded simulator turns still mutate accepted action counters
Failure: Review 5628643360, comment 4359843888 remains unresolved. With a fresh sim whose fake model returns a no-op, run:
proposed = sim.fork_turn()
await proposed.process("task", "activity", "report", None, 1, True)
# Discard proposed.The original reports action_breakdown={"no-op": 1}, but total_calls=0, wait_count=0, and empty history. copy(self) shares _counts, which process mutates. The post-call deadline branch can discard this completed fork, leaving contradictory accepted-state diagnostics in the persisted simulator state. This affects diagnostics, not the accepted-message scoring input.
Correction: In UserAgent.fork_turn(), immediately after proposed = copy(self), insert:
proposed._counts = self._counts.copy()Every mutable accepted-turn counter must remain isolated until the caller commits the fork. Keep _llm shared intentionally; do not deep-copy the complete simulator, because discarded/cancelled requests must remain visible in transport diagnostics.
Preservation / acceptance — proposed, not run: Complete and discard separate no-op and redirect("Fix tests") forks; the original's full stats and history must remain unchanged, while the shared fake model still records both requests. On a fresh simulator, commit a completed no-op fork by assigning it as the current simulator: expect total_calls=1, wait_count=1, message_count=0, and action_breakdown={"no-op": 1}. Commit a message fork separately: expect one call, one message, and one matching action count. Retain the existing accepted-feedback cursor and four-no-op stopping assertions. Author verification is required before adopting the suggested insertion.
Signed-off-by: Frederic Lafrance <flafrance@nvidia.com>
Signed-off-by: Frederic Lafrance <flafrance@nvidia.com>
Signed-off-by: Frederic Lafrance <flafrance@nvidia.com>
Preserve the collector task identity on SWE-Together requests and forward a compatible copy of the candidate trajectory. Restore the shared collector and Terminus identity convention. Signed-off-by: Frederic Lafrance <flafrance@nvidia.com>
…dings Signed-off-by: Frederic Lafrance <flafrance@nvidia.com>
Signed-off-by: Frederic Lafrance <flafrance@nvidia.com>
Signed-off-by: Frederic Lafrance <flafrance@nvidia.com>
87c4a4f to
0f042bd
Compare
|
/ok to test 0f042bd |
Signed-off-by: Frederic Lafrance <flafrance@nvidia.com>
|
/review |
|
/ok to test 049ebb3 |
svcnvidia-nemo-ci
left a comment
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
SHIP WITH CARE — the prior nonblocking diagnostic NOTE remains; no eligible blocking findings were validated.
Current source retains the previously verified fixes for runtime recovery and serialized release, atomic offline executable publication, and per-activation conversation identity. Inspected their regression assertions and the checkpoint test synchronization change.
Accounted for all 68 changed files. Discovery remained follow-up-scoped: verified the latest prior finding and claimed fixes, then traced directly affected callers and tests. The previously reviewed tree was unavailable, so comparison against review 5629272482 was limited to supplied historical evidence; no new regression was attributed without introduction evidence.
Static, read-only review only. Tests and model rollouts were not executed; the suggested insertion and proposed checks require author verification.
Note: formal approval is not available for this request, so this review is posted as a comment only.
Review generated by codex. This was a findings-only review. Request a full review with /review reset=true.
| def fork_turn(self) -> "UserAgent": | ||
| """Stage a consultation independently; the caller commits it only before its deadline.""" | ||
| proposed = copy(self) | ||
| proposed._messages = deepcopy(self._messages) |
There was a problem hiding this comment.
NOTE — Discarded simulator turns still mutate accepted action counters
Failure: Latest review 5629272482, comment 4360323096, remains unresolved. Start with a fresh UserAgent whose fake model's call() returns an object with content="no-op", tool_calls=[]:
proposed = sim.fork_turn()
await proposed.process("task", "activity", "report", None, 1, True)
# Discard proposed.The original now reports action_breakdown={"no-op": 1} despite total_calls=0, wait_count=0, and empty history. The shallow copy still shares _counts, which process mutates. The post-call deadline branch can discard a completed fork, leaving contradictory persisted accepted-state diagnostics. This affects diagnostics, not accepted-message scoring input.
Correction: In UserAgent.fork_turn(), immediately after proposed = copy(self), insert:
proposed._counts = self._counts.copy()Invariant: discarding a consultation must leave accepted history and counters unchanged; committing it must advance the scalar counters and action breakdown together. Existing history copies remain necessary. Keep _llm shared intentionally—do not deep-copy the whole simulator—so transport diagnostics still retain discarded or cancelled requests.
Preservation / acceptance — proposed, not run: Complete and discard separate no-op and structured redirect("Fix tests") forks. The original's complete stats and history must remain unchanged, while the shared fake model records both requests. On a fresh simulator, commit a completed no-op fork: expect total_calls=1, wait_count=1, message_count=0, and action_breakdown={"no-op": 1}. Separately commit a message fork: expect one call, one message, zero waits, and one matching redirect count. Retain the existing accepted-feedback and four-no-op stopping assertions: the accepted message remains Fix tests, the original-message cursor advances once, and the fourth consecutive no-op stops continuation. Verify the suggested insertion before adopting it.
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/39975677939 |
Add SWE-Together, a benchmark of 109 repository tasks reconstructed from interactive user–agent coding sessions. The environment replays user feedback across candidate activations and reports rubric-based correctness, User Correction, and Intent Coverage. Benchmark reference: Wu et al., SWE-Together: Evaluating Coding Agents in Interactive User Sessions.
To run it, follow the SWE-Together recipe: prepare the pinned task assets and runtimes, configure the model endpoints, OpenSandbox provider and dependency proxy, then use
gym env startandgym eval runwithbenchmarks/swe_together/opencode.yamlandbenchmarks/swe_together/opensandbox.yaml.Design
Qualification and boundaries
Real-model qualification exercised candidate continuation, simulation and grading. The final trajectory-identity adaptation was replayed against two recorded model episodes containing 65 native turns and 77 captured model calls, preserving native evidence, grades and rewards. Existing OpenCode conformance probes still reproduce base-branch failures around sandbox identity evidence and the model-error scenario.
Upstream score parity remains unqualified. The recipe documents differences that affect comparisons, including web-tool restrictions, runtime setup and the normalized simulator activity view. Grading transfers the declared filesystem roots rather than live processes or system-wide package changes, and crash recovery restarts the episode. No separate issue is needed for this self-contained benchmark addition.