Conversation
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
There was a problem hiding this comment.
🟡 Changes recommended
The Qwen model import and configuration contract break the existing Qwen test harness.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds configurable, trajectory-stable session routing to SWE-bench requests, with updated service behavior, tests, and documentation.
Changes:
- Propagates routing configuration through accuracy runs and service requests.
- Adds stable per-trajectory routing IDs for supported models.
- Updates capabilities, tests, and operational documentation.
File summaries
| File | Reviewed change |
|---|---|
tests/unit/evaluation/test_swe_bench_scorer.py |
Tests routing headers in request payloads. |
tests/unit/evaluation/swebench_service/test_server.py |
Tests advertised routing capabilities. |
tests/unit/evaluation/swebench_service/test_runner.py |
Tests routing-aware model configuration. |
tests/unit/commands/test_score_accuracy.py |
Tests routing configuration propagation. |
src/inference_endpoint/evaluation/swebench_service/swebench_service/schemas.py |
Adds routing-header request configuration. |
src/inference_endpoint/evaluation/swebench_service/swebench_service/runner.py |
Configures routing-aware model classes. |
src/inference_endpoint/evaluation/swebench_service/swebench_service/routing_model.py |
Implements stable per-model routing IDs. |
src/inference_endpoint/evaluation/swebench_service/swebench_service/qwen_tools_model.py |
Applies routing support to Qwen models; existing Qwen test stubs require updates for the new config contract. |
src/inference_endpoint/evaluation/swebench_service/swebench_service/__init__.py |
Advertises routing capability. |
src/inference_endpoint/evaluation/swebench_service/README.md |
Documents service routing behavior. |
src/inference_endpoint/evaluation/swe_bench_scorer.py |
Sends routing headers to the service. |
src/inference_endpoint/commands/benchmark/accuracy.py |
Inherits routing settings from performance configuration. |
examples/10_Agentic_Inference/README.md |
Documents routing configuration and defaults. |
examples/10_Agentic_Inference/accuracy/RUNBOOK.md |
Documents operational routing behavior. |
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
Compatibility handling is needed for existing v1 services before approval.
Review details
Suppressed comments (2)
src/inference_endpoint/evaluation/swe_bench_scorer.py:68
- This adds a required capability while the service API still advertises
v1. Any existing v1 service will now fail preflight here, and itsextra="forbid"RunRequestwould reject the new field even if preflight were bypassed. Because routing headers are an optional feature, either bump/version this request contract or negotiate the capability and omit the field/fall back when the service does not advertise it.
"swebench.routing_headers",
src/inference_endpoint/evaluation/swebench_service/swebench_service/qwen_tools_model.py:28
- Could this add a Qwen-specific test that constructs
QwenToolsModelwith non-emptyrouting_headersand asserts thelitellm.completioncall receives the resultingextra_headers? The existing Qwen tests instantiate the model without routing headers, while the routing-model tests only inspectSessionRoutingLitellmModel.config; becauseQwenToolsModeloverrides_query, a regression in this subclass could silently drop the headers while all current tests still pass.
class QwenToolsModel(SessionRoutingLitellmModel):
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #503 +/- ##
=======================================
Coverage ? 80.86%
=======================================
Files ? 154
Lines ? 21091
Branches ? 0
=======================================
Hits ? 17056
Misses ? 4035
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟢 Approval recommended
The routing-header feature is consistently wired through the scorer, service schema/config patching, and model wrapper, with unit tests and documentation updates validating the intended behavior.
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
feat(swebench): add trajectory-stable session routing
What does this PR do?
Adds trajectory-stable session routing headers to SWE-bench requests, matching agentic performance routing so consistent-hash routers keep each trajectory on one backend. The header names remain configurable and default to
X-Session-ID.Type of change
Related issues
N/A
Testing
194 focused tests passed. A live accuracy-only run sent 200 routed requests across 100 trajectory IDs with zero backend-routing mismatches.
Checklist