fix(proxy): estimate ensemble panel + judge sub-call usage on missing-usage backends#806
Conversation
Follow-up to #794 (AISIX-Cloud#1074). The ensemble sub-call usage events (one per panel member + the judge) bypassed the token-estimation fallback: against a no-usage backend they recorded silent zeros. #794 wired only the streaming judge (SSE Drop guard); the non-streaming panel + judge and the streaming-path panel members were still unflagged. Carry the per-member answer text (PanelOutcome.est_output_text) and the judge's synthesis request (EnsembleOutcome.judge_req) out to the emit sites, and route every sub-call emit through one shared helper (estimate_subcall_ tokens) so the family can't drift: prompt from the sub-call's own request, completion from its answer text, #794 or-semantics (a non-zero upstream value always wins), flagged usage_estimated. Telemetry-only — the client-facing response/stream is never rewritten. Fixes #796
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesThe ensemble proxy now centralizes fallback token estimation for missing or partial upstream usage data. Non-streaming and streaming panel/judge telemetry records include estimated counters and Ensemble usage estimation
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant EnsembleProxy
participant PanelBackends
participant JudgeBackend
participant SLSTelemetry
Client->>EnsembleProxy: ensemble chat completion
EnsembleProxy->>PanelBackends: panel sub-calls
PanelBackends-->>EnsembleProxy: answers with missing usage
EnsembleProxy->>JudgeBackend: judge sub-call
JudgeBackend-->>EnsembleProxy: synthesized answer
EnsembleProxy->>SLSTelemetry: estimated sub-call usage with usage_estimated
EnsembleProxy-->>Client: ensemble response or stream
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/aisix-proxy/src/chat.rs`:
- Around line 2608-2612: Update all estimate_subcall_tokens call sites to pass
each sub-call’s resolved upstream model rather than ensemble display names such
as member.model, member.attempt_model, or outcome.judge_model. Thread the
resolved model through the streaming panel telemetry path as well, preserving
the existing tokenizer and usage-estimation behavior.
🪄 Autofix (Beta)
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
Run ID: 67f543aa-ba46-468a-994b-a96356dc1619
📒 Files selected for processing (3)
crates/aisix-proxy/src/chat.rscrates/aisix-proxy/src/ensemble.rstests/e2e/src/cases/ensemble-usage-estimation-796-e2e.test.ts
…stream model Per review: estimate_subcall_tokens keyed the tokenizer off the ensemble display name (member.model / attempt_model / judge_model). A custom alias that doesn't match a known tokenizer prefix falls back to cl100k even when the real upstream is o200k (gpt-4o), skewing the estimate against #1074's <5% target. Resolve the upstream model (Model.upstream_model()) in resolve_sub and thread it through every sub-call estimate site, including the streaming panel telemetry (PanelTelem.est_model) — matching the direct and streaming-judge paths, which already key off upstream_model.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Follow-up to #794 (AISIX-Cloud#1074). An ensemble request fans out to one usage event per sub-call — each panel member plus the judge — but none of those events went through the #1074 token-estimation fallback. Against a backend that reports no
usage(a relay/transit upstream), each sub-call recorded silent zeros. #794 wired only the streaming judge (via the SSE Drop guard); the non-streaming panel + judge, and the panel members on the streaming path, were still unflagged.What was missing
emit_panel_member)emit_subcalls)on_complete)The panel members' answer texts and the judge's synthesis request lived only inside
ensemble.rs, so the emit sites had nothing to estimate from — the refactor #794 explicitly deferred here.Change
PanelOutcomecarriesest_output_text(the member's answer text) andEnsembleOutcomecarriesjudge_req(the synthesis request), so every emit site has the request+output it needs.estimate_subcall_tokens(req, model, usage, output_text)applies the feat(usage): estimate tokens locally when the upstream reports no usage #794 per-field or-semantics — a non-zero upstream value always wins; estimation fills only zeros (prompt from the sub-call's own request, completion from its answer text) — and returns theusage_estimatedflag. Routing all four emit sites through it keeps the family from drifting again.This closes the whole bug class, not just the non-streaming path named in the issue: the streaming-path panel members shared the identical gap and are fixed in the same PR.
Estimation feeds telemetry only (usage events → exporters / TPM / metrics). The client-facing response and SSE stream are never rewritten with synthesised usage. No CP or schema change — the sub-call events already carry
usage_estimatedon the existing/dp/telemetrywire.LiteLLM has no ensemble (panel/judge) construct emitting per-sub-call usage, so there is no baseline to compare against here; the underlying token-counting semantics are unchanged from #794 (already LiteLLM-aligned).
Tests
ensemble-usage-estimation-796-e2e.test.ts): a real ensemble (2 panel members + judge) over usage-less mock upstreams, non-streaming and streaming. Asserts, per-record, that every sub-call (both panel members + judge) ships its ownusage_estimated-flagged event. Verified failing before the fix (non-streaming: nothing flagged; streaming: only the judge flagged) and passing after.estimate_subcall_tokensor-semantics — fully-missing usage fills+flags, reported usage is preserved unflagged, and a reported prompt + missing completion fills only the completion.Fixes #796
Summary by CodeRabbit
Bug Fixes
Tests