feat(bench): run SWE-Gym through Harbor - #1239
Merged
Merged
Conversation
Contributor
|
This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review. |
📝 WalkthroughWalkthroughThe benchmark runner now uses pinned Harbor for Terminal-Bench and SWE-Gym Lite. It shares command generation, synchronization, artifacts, and metrics. Linear decoding now forwards accepted proposal tokens to pending input without duplicate suffixes. ChangesHarbor evaluation integration
Linear decode pending-token handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EvalCLI
participant HarborAdapter
participant HarborRepository
participant HarborJobs
EvalCLI->>HarborRepository: sync pinned Harbor revision
EvalCLI->>HarborAdapter: build evaluation command
HarborAdapter->>HarborJobs: launch Terminal-Bench or SWE-Gym
HarborJobs-->>EvalCLI: write job and trial artifacts
EvalCLI->>HarborJobs: parse rewards and calculate metrics
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/skippy-server/src/frontend/local_generation/linear_decode.rs (1)
298-330: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExercise the receipt-to-query path in this test.
This test calls
append_pending_linear_proposal_tokensandquery_linear_proposalseparately. It does not execute the call at lines 209-212 throughtry_execute_linear_proposal.A regression that removes the receipt handoff, or clears pending tokens before the next query, would still pass. Drive a ready proposal and its receipt through
try_execute_linear_proposal, then assert that the nextPendingTokenIngress::proposecall receives[41, 42]exactly once.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/skippy-server/src/frontend/local_generation/linear_decode.rs` around lines 298 - 330, The test accepted_proposal_tokens_are_pending_on_the_next_query currently bypasses try_execute_linear_proposal and therefore does not verify receipt handoff. Update it to drive a ready proposal and its receipt through try_execute_linear_proposal, then assert the subsequent PendingTokenIngress::propose call receives [41, 42] exactly once, preserving the existing no-proposal and pending-token expectations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/skippy-bench/src/evals/adapters/harbor.rs`:
- Around line 56-65: Remove the redundant format! wrapper inside the task_path
construction in the task_id map closure, returning the existing shell_quote
result directly while preserving the current task_root path selection and
fallback behavior.
In `@crates/skippy-bench/src/evals/run.rs`:
- Line 195: Remove the obsolete Terminal-Bench parsing path throughout the
evaluation code: delete terminal_bench_metrics, terminal_bench_output_path, and
the legacy parser test, then remove any now-unused imports or helpers. Update
stale documentation that describes Terminal-Bench token metrics or the
raw/terminal-bench format, while preserving the Harbor-based dispatch for
EvalId::TerminalBench.
---
Nitpick comments:
In `@crates/skippy-server/src/frontend/local_generation/linear_decode.rs`:
- Around line 298-330: The test
accepted_proposal_tokens_are_pending_on_the_next_query currently bypasses
try_execute_linear_proposal and therefore does not verify receipt handoff.
Update it to drive a ready proposal and its receipt through
try_execute_linear_proposal, then assert the subsequent
PendingTokenIngress::propose call receives [41, 42] exactly once, preserving the
existing no-proposal and pending-token expectations.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c91e9e97-9bf9-43d7-b365-49f5861ce6d7
📒 Files selected for processing (9)
crates/skippy-bench/README.mdcrates/skippy-bench/src/cli.rscrates/skippy-bench/src/evals.rscrates/skippy-bench/src/evals/adapters/harbor.rscrates/skippy-bench/src/evals/adapters/mod.rscrates/skippy-bench/src/evals/registry.rscrates/skippy-bench/src/evals/run.rscrates/skippy-bench/src/evals/sync.rscrates/skippy-server/src/frontend/local_generation/linear_decode.rs
i386
marked this pull request as ready for review
August 11, 2026 20:41
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/skippy-bench/src/cli.rs (1)
150-170: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winWire
cacheline_stateinto the Cacheline evaluation or remove the option. The parsed--cacheline-statevalue is never read, so a supplied path has no effect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/skippy-bench/src/cli.rs` around lines 150 - 170, Wire the parsed cacheline_state option into the Cacheline evaluation flow so the supplied PathBuf controls the smoke state directory and its assertion files. Locate the evaluation entry point and pass this value through its configuration or function parameters; if no Cacheline evaluation consumes it, remove the cacheline_state field and CLI argument instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/skippy-bench/src/cli.rs`:
- Around line 150-170: Wire the parsed cacheline_state option into the Cacheline
evaluation flow so the supplied PathBuf controls the smoke state directory and
its assertion files. Locate the evaluation entry point and pass this value
through its configuration or function parameters; if no Cacheline evaluation
consumes it, remove the cacheline_state field and CLI argument instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 72c255b0-7742-4d99-a9c5-42e72569f107
📒 Files selected for processing (8)
crates/skippy-bench/README.mdcrates/skippy-bench/src/cli.rscrates/skippy-bench/src/evals.rscrates/skippy-bench/src/evals/adapters/harbor.rscrates/skippy-bench/src/evals/adapters/terminal_bench.rscrates/skippy-bench/src/evals/run.rscrates/skippy-server/src/frontend/linear_proposal/execution.rscrates/skippy-server/src/frontend/local_generation/linear_decode.rs
💤 Files with no reviewable changes (1)
- crates/skippy-bench/src/evals/adapters/terminal_bench.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/skippy-bench/src/evals/run.rs
- crates/skippy-bench/src/evals/adapters/harbor.rs
- crates/skippy-bench/README.md
- crates/skippy-bench/src/evals.rs
ndizazzo
self-requested a review
August 11, 2026 21:38
ndizazzo
approved these changes
Aug 11, 2026
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
2 participants
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
Root cause
Mesh advanced the authoritative decode position after an accepted linear proposal but did not retain those committed proposal tokens as the next plugin-host pending suffix. The host tracker then rejected later proposal queries with a pending-token count mismatch.
Validation
cargo test -p skippy-bench: 78 passedcargo test -p skippy-server --lib: 394 passedcargo test -p mesh-native-serving-plugin-host: 31 passedDo not merge this PR independently of the private-fork landing sequence.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes