smenatic router: budgeting+telemetry management - #5686
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change records routing-classification embedding usage, attaches it to responses, calculates routing embedding costs, settles eligible warmup attribution, and exposes request and warmup Prometheus metrics. ChangesRouting embedding attribution
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🔵 Low · up to Routing embedding telemetry and budget attribution can be understated when fallback attempts or validation-error paths consume additional tokens, and non-fatal attribution failures should use warning-level logging. The PR is mergeable with explicit owner follow-up on these bounded accounting and logging issues. Sequence Diagram(s)sequenceDiagram
participant Request
participant GovernancePlugin
participant ModelCatalog
participant PrometheusPlugin
Request->>GovernancePlugin: classify with routing embedding
GovernancePlugin->>Request: stamp RoutingDebug
Request->>ModelCatalog: calculate response cost
ModelCatalog-->>Request: return composed cost
GovernancePlugin->>PrometheusPlugin: record request or warmup metrics
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
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 `@framework/modelcatalog/datasheet/cost.go`:
- Around line 52-74: Update RoutingEmbeddingCost to use an embedding-specific
copy of lookupScopes whose Provider is always routingDebug.ProviderUsed before
calling resolvePricing, while preserving any other scope fields supplied by
callers. Ensure provider-scoped pricing overrides for the routing/cache
embedding provider and model are matched even when request-scoped scopes are
passed.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4d0a7c23-0c0e-420c-99fc-4eb115bafac0
📒 Files selected for processing (10)
core/schemas/bifrost.goframework/modelcatalog/datasheet/cost.goframework/modelcatalog/datasheet/cost_test.goframework/modelcatalog/pricing.goplugins/governance/embedding.goplugins/governance/embedding_test.goplugins/governance/main.goplugins/telemetry/main.goplugins/telemetry/main_test.gotransports/bifrost-http/server/server.go
5b1211b to
e5636a9
Compare
e5636a9 to
7d1b1d3
Compare
fad421f to
3747cee
Compare
3747cee to
e755887
Compare
7d1b1d3 to
3767315
Compare
3767315 to
28e1a04
Compare
5260ba4 to
d3dcfa1
Compare
03ac36e to
f7a2a26
Compare
d3dcfa1 to
04efeae
Compare
04efeae to
ee0a136
Compare
f7a2a26 to
3b2841d
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/telemetry/main.go (1)
726-730: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
bifrost.Ptrfor unmodified values.Replace address-of expressions with
bifrost.Ptr(...)for consistency with repository pointer construction.
plugins/telemetry/main.go#L726-L730: usebifrost.Ptr(provider),bifrost.Ptr(model), andbifrost.Ptr(inputTokens).plugins/telemetry/main_test.go#L218-L221: usebifrost.Ptr(provider),bifrost.Ptr(model), andbifrost.Ptr(inputTokens).Based on learnings, “prefer using bifrost.Ptr() to create pointers instead of the address operator (&) even when & would be valid syntactically.”
🤖 Prompt for 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. In `@plugins/telemetry/main.go` around lines 726 - 730, Replace direct address-of pointer construction with bifrost.Ptr for provider, model, and inputTokens in plugins/telemetry/main.go lines 726-730 and plugins/telemetry/main_test.go lines 218-221, preserving the existing values and behavior.Source: Learnings
🤖 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.
Nitpick comments:
In `@plugins/telemetry/main.go`:
- Around line 726-730: Replace direct address-of pointer construction with
bifrost.Ptr for provider, model, and inputTokens in plugins/telemetry/main.go
lines 726-730 and plugins/telemetry/main_test.go lines 218-221, preserving the
existing values and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b13e297-3579-4808-86c8-121dc8cfc97b
📒 Files selected for processing (10)
core/schemas/bifrost.goframework/modelcatalog/datasheet/cost.goframework/modelcatalog/datasheet/cost_test.goframework/modelcatalog/pricing.goplugins/governance/embedding.goplugins/governance/embedding_test.goplugins/governance/main.goplugins/telemetry/main.goplugins/telemetry/main_test.gotransports/bifrost-http/server/server.go
🚧 Files skipped from review as they are similar to previous changes (8)
- plugins/governance/main.go
- framework/modelcatalog/pricing.go
- framework/modelcatalog/datasheet/cost.go
- framework/modelcatalog/datasheet/cost_test.go
- core/schemas/bifrost.go
- plugins/governance/embedding.go
- transports/bifrost-http/server/server.go
- plugins/governance/embedding_test.go
ee0a136 to
b4d4b30
Compare
3b2841d to
879943d
Compare
879943d to
ac97779
Compare
b4d4b30 to
2e66e15
Compare
ac97779 to
abec8d9
Compare
2e66e15 to
695c16f
Compare
695c16f to
4beb42a
Compare
1c09e09 to
bd540fe
Compare
4beb42a to
1936bfc
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |

Summary
Adds observability and cost attribution for the embedding calls made by the semantic complexity router. Previously, routing classification embeddings were invisible to telemetry and never counted toward budgets. This PR stamps a
RoutingDebugfield onto responses when a routing embed ran, wires warmup embed usage through a new observer callback, and exposes two new Prometheus counters for routing overhead.Changes
BifrostRoutingDebugschema: New struct added toBifrostResponseExtraFieldscarrying the embedding provider, model, input token count, and acount_toward_budgetsflag for the routing classification embed that ran before provider selection.CalculateCostnow composes routing embedding cost on top of the base/cache cost whenRoutingDebug.CountTowardBudgetsis set. A new exportedRoutingEmbeddingCostmethod on the datasheetStore(andCalculateRoutingEmbeddingCostonModelCatalog) prices routing overhead unconditionally so telemetry can report it regardless of the budget flag.embedComplexityTextrecords embed usage on the triggering request'sBifrostContextvia a context key.stampRoutingDebug(called fromPostLLMHookbefore cost calculation) reads that usage and writesRoutingDebugonto the response. For streams, only the final chunk is stamped, matching the semantic cache pattern.embedComplexityTextsand the single-input warmup fallback inembedComplexityTextroute usage to a newsettleWarmupEmbedUsagemethod instead of recording on a request context.settleWarmupEmbedUsagefires aWarmupEmbedUsageObservercallback (for telemetry) and, whencount_toward_budgetsis on, attributes cost to the admin-owned provider/model-level budgets viaUpdateProviderAndModelBudgetUsageInMemory.WarmupEmbedUsageObserver/WarmupEmbedUsageObserverSetter: New interface and callback type, wired by the HTTP server at bootstrap and plugin reload, forwarding warmup usage to the telemetry plugin'sObserveWarmupRoutingEmbeddingmethod.bifrost_routing_embedding_requests_totalandbifrost_routing_embedding_cost_total— labeled byprovider,model, andphase(requestfor per-request classification embeds,warmupfor boot/config-change exemplar embeds). Request-phase metrics are recorded inPostLLMHookoff theRoutingDebugstamp; warmup-phase metrics are recorded viaObserveWarmupRoutingEmbedding.ReloadPluginwire theWarmupEmbedUsageObservertoobserveWarmupRoutingEmbedding, which resolves the telemetry plugin per call so a reloaded telemetry instance is picked up without re-wiring governance.Type of change
Affected areas
How to test
go test ./core/schemas/... ./framework/modelcatalog/... ./plugins/governance/... ./plugins/telemetry/... ./transports/bifrost-http/...Key scenarios covered by the new tests:
TestCalculateCost_RoutingDebugFlagOff/TestCalculateCost_RoutingDebugFlagOn: verifies routing embed cost is added only whencount_toward_budgetsis set.TestCalculateCost_RoutingDebugComposesWithCacheDebug: verifies both debug structs can coexist on one response and their costs compose correctly.TestRoutingEmbeddingCost_Standalone/TestRoutingEmbeddingCost_MissingFields: standalone pricing helper, including nil/missing-field guards.TestEmbedComplexityTextRecordsRoutingUsage: classification embed on a request context records usage forRoutingDebugstamping.TestEmbedComplexityTextWarmupPathObservesInsteadOfRecording: warmup path fires the observer, never the request context key.TestStampRoutingDebug: stamping logic, stream final-chunk gating, and nil-result safety.TestSettleWarmupEmbedUsageAttributesProviderBudget/TestSettleWarmupEmbedUsageFlagOffLeavesBudgetsUntouched: warmup budget attribution gated on the flag.TestRoutingEmbeddingCounters/TestObserveWarmupRoutingEmbedding/TestRoutingEmbeddingCountersAbsentWithoutStamp: Prometheus counter correctness for both phases and the no-stamp case.Breaking changes
Related issues
Security considerations
No new auth surfaces. The
RoutingDebugfield is included in API responses underextra_fields(already an internal/debug envelope); no PII is introduced — only provider name, model name, and token counts are recorded.Checklist
docs/contributing/README.mdand followed the guidelines