Repository navigation
fix: price routed turns using the serving model - #4579
Conversation
Signed-off-by: Arnaud Héritier <aheritier@users.noreply.github.com>
Signed-off-by: Arnaud Héritier <aheritier@users.noreply.github.com>
Signed-off-by: Arnaud Héritier <aheritier@users.noreply.github.com>
Signed-off-by: Arnaud Héritier <aheritier@users.noreply.github.com>
Signed-off-by: Arnaud Héritier <aheritier@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The implementation consistently propagates serving identity and includes comprehensive regression coverage for pricing, fallback, concurrency, telemetry, and UI behavior.
0 open findings
What changed in this PR
Adds request-bound serving-model metadata so routed turns are priced and attributed to the provider that successfully served them.
Changes:
- Propagates serving identity and configuration through routed and instrumented streams.
- Uses serving-model pricing, endpoint, cache rates, and service tier throughout runtime accounting.
- Adds routing, fallback, concurrency, telemetry, and TUI regression coverage.
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
aheritier
added
area/models
This comment was marked as resolved.
This comment was marked as resolved.
Checked the current PR commit list and each commit's GitHub verification record. All five commits already report
The referenced |
aheritier
removed
the
status/needs-signed-commits
aheritier
left a comment
aheritier
left a comment
There was a problem hiding this comment.
No blocking findings. The request-bound serving metadata improves on mutable last-selection state, and the pricing changes preserve explicit/free overrides, cache rates, endpoint policy, and fallback cooldown behavior. Linked issue #4576 is addressed with regression coverage for serving-model pricing, routing, and fallback. No demonstrated security or material performance regression was found.
One nonblocking attribution inconsistency is noted inline. The required CI gate and reported build, lint, test, and CodeQL checks passed; the race-test job was skipped. No local tests or benchmarks were run. GitHub reports mergeable_state=blocked, so this is not a merge-ready declaration.
Signed-off-by: Arnaud Héritier <aheritier@users.noreply.github.com>
Fixes #4576.
Summary
cost:overrides, cache-read/write rates and actual response service tier, including same-ID routing and resolved custom endpoints.Validation
task build,task test,task lint,task test-race— pass.go test -count=1 ./pkg/model/provider/ ./pkg/model/provider/rulebased/ ./pkg/runtime/ -run 'TestServingModel|TestLastSelectedModelID_Concurrent|TestRouted|TestFallbackPricing|TestRunStreamServiceTierCost'— pass.go test -race -count=2 ./pkg/model/provider/... ./pkg/runtime/ ./pkg/leantui/ -run 'TestServingModel|TestLastSelectedModelID_Concurrent|TestRouted|TestInstrument'— pass.go test -race -shuffle=on ./pkg/runtime/ ./pkg/model/provider/... ./pkg/leantui/— pass.New tests use local fake providers and a fixed catalogue. They check attribution and matching costs across hooks, session messages, usage, budgets and telemetry, plus free/unpriced models, endpoint policy, no-match routing and stale selections after successful outer fallback. The pricing regressions fail against the unmodified base; the no-match and outer-fallback controls pass there.
No live/paid inference or SDK changes. This addresses accounting, not a demonstrated concurrent accounting defect. The pre-existing no-match provider construction behavior is not changed. Serving metadata is available at stream creation and stable after close; fallback identity notifications now arrive at stream open.