fix: Stamp the selected ModelId on LlmRequest much earlier - #393
Conversation
|
@CodeRabbit review |
✅ Action performedReview finished.
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
WalkthroughRouting now stamps the selected model into requests. LLM clients, tracing, error handling, tests, and documentation use the request model as the routed target. ChangesRequest-Model Routing
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to The change is merge-ready after normal checks and review; no actionable merge-blocking risk remains. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
Previously `libsy` would give `CallModel` a request with the wrong model ID, and a `Decision` with the correct (chosen by routing algo) model ID. `libsy-llm-client` would stamp the later onto the former before making the actual call. Now `Driver::call_model` stamps the correct model ID onto `LlmRequest`, so everything after that can use `LlmRequest` directly. This was confusing me for a while. Seems easier to understand like this. This also means `CallModel` can read the `selected_model_id()` off the request now, which gives us options for changing `Decision` later. Assisted-by: Codex:GPT 5.6 Sol high Signed-off-by: Graham King <grahamk@nvidia.com>
aa06e52 to
f5dfba6
Compare
The `Request` has the correct model id, so use that. Signed-off-by: Graham King <grahamk@nvidia.com>
ayushag-nv
left a comment
There was a problem hiding this comment.
Nice Catch. Looks good.
Previously
libsywould giveCallModela request with the incoming modelID (e.g.
switchyard/random), and aDecisionwith the correct id (e.g.openai/gpt-oss-120b)as chosen by the routing algorithm.
libsy-llm-clientwould stamp the later ontothe former before making the actual call.
Now
Driver::call_modelstamps the correct model ID ontoLlmRequest,so everything after that can use
LlmRequestdirectly.This was confusing me for a while. Seems easier to understand like this.
This also means
CallModelcan read theselected_model_id()off therequest now, which gives us options for changing
Decisionlater.The
RoutedLlmClient::callmethod took aDecisionpurely to extract it'smodel_id and stamp it on the request, so we also simplify that to
call(Request).Claude's review:
Assisted-by: Codex:GPT 5.6 Sol high
Signed-off-by: Graham King grahamk@nvidia.com
Summary by CodeRabbit
Improvements
Documentation