Fix: inference-parser misses /chat/completions from /v1-less proxies (opencode, litellm) - #823
Conversation
OpenAI-compatible proxies (e.g. litellm used by opencode) post to /chat/completions without the /v1 prefix. The OnRequest dispatch switch only matched /v1/chat/completions and /v1/completions, so requests fell through to the default arm and pctx.Extensions.Inference was never populated — resulting in missing model, token counts, and method fields in abctl session views. Extend the switch to also match /chat/completions and /completions. No changes to OnResponse or OnResponseFrame — the default branch there already routes to the OpenAI parsers. Add two tests: TestInferenceParser_VlessPath_ChatCompletions and TestInferenceParser_VlessPath_Completions. Fixes #822 Signed-off-by: steinder <steinder@us.ibm.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe inference parser now handles OpenAI-compatible ChangesOpenAI endpoint support
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change enables metadata extraction for two additional compatible completion paths using the existing parsing behavior, with no change to authorization, routing, or request handling. No actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue
✨ 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 |
mrsabath
left a comment
There was a problem hiding this comment.
Summary
Correct, minimal, well-scoped fix. Extends the OnRequest dispatch to match the /v1-less /chat/completions and /completions variants that litellm/opencode emit, restoring model and token telemetry in abctl.
What I verified against the source (PR head 4bcb677):
endpointPath()strips the query string viastrings.Cut(path, "?"), so the new exact-match cases stay robust for/chat/completions?foo=bar.- Response side needs no change:
OnResponsegates only onanthropicMessagesPathand defaults everything else to the OpenAI parsers, but it only runs whenextwas populated on the request. So the request-switch fix is what unblocks the response telemetry too, end-to-end. - Legacy
/completions(prompt-based) routed toparseOpenAIRequeststill captures model + sampling params +IsActioncorrectly, which is exactly the telemetry intended. - Tests assert the real regression (
Extensions.Inference != nil, correctModel,IsAction) using the samepipeline.Context{Path:...}shape the code reads.
My only note is the symmetric /messages gap for Anthropic-compatible proxies (inline), which is a non-blocking follow-up.
Areas reviewed: Go (inference-parser plugin + tests), Tests, commit/PR conventions.
Commits: 1, signed-off, conventional prefix, under 72 chars. CI: all green (Go CI x3, CodeQL go/python, Bandit, Trivy, pre-commit, CodeRabbit).
Verdict: APPROVE — no must-fix issues; the single comment is a suggestion for a follow-up.
| var ext *pipeline.InferenceExtension | ||
| switch endpointPath(pctx) { | ||
| case "/v1/chat/completions", "/v1/completions": | ||
| case "/v1/chat/completions", "/v1/completions", "/chat/completions", "/completions": |
There was a problem hiding this comment.
suggestion (non-blocking): same-class gap on the Anthropic side. anthropicMessagesPath is /v1/messages, so a /v1-stripping proxy (this PR's exact scenario) posting Anthropic-style traffic to /messages would still fall through to the default arm and record no inference telemetry. Out of scope for #822, but worth a follow-up (or a matching /messages case here) so the asymmetry is a deliberate choice rather than an oversight.
Why
OpenAI-compatible proxies such as litellm (used by opencode) post to
/chat/completionswithout the/v1prefix. TheOnRequestdispatch switch ininference-parseronly matched/v1/chat/completionsand/v1/completions, so these requests fell through to thedefault: return Continuearm —pctx.Extensions.Inferencewas never populated andabctlshowed—for model, method, and token counts on every opencode request.Observed before the fix:
What changed
authbridge/authlib/plugins/inferenceparser/plugin.goOnRequestswitch case to also match/chat/completionsand/completions.authbridge/authlib/plugins/inferenceparser/plugin_test.goTestInferenceParser_VlessPath_ChatCompletionsandTestInferenceParser_VlessPath_Completions.No changes to
OnResponseorOnResponseFrame— thedefaultbranch there already routes to the OpenAI parsers, which is correct.Testing
Verified end-to-end: opencode requests through
authbridge-proxy --demonow show model, token counts, and method inabctl.Related issue(s)
Fixes #822
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com
Summary by CodeRabbit
/chat/completionsand/completionsendpoints without the/v1prefix.