feat: complexity router : adds complexity_tier CEL routing#3711
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a complexity analyzer that extracts lexical signals from requests and prior user turns, computes a blended complexity score and tier, extracts inputs from multiple API payload shapes, and wires a lazily-computed ChangesRequest Complexity Scoring and Governance Routing
Sequence DiagramsequenceDiagram
participant HTTP as HTTPTransportPreHook
participant Apply as ApplyRoutingRules
participant EvalRules as EvaluateRoutingRules
participant CELRef as CELRefsChecker
participant Analyzer as ComplexityAnalyzer
participant CELExec as evaluateCELExpression
HTTP->>Apply: incoming request
Apply->>Apply: build RoutingContext with lazy computeComplexity
Apply->>EvalRules: start rule evaluation
loop per routing rule
EvalRules->>CELRef: does rule reference complexity_tier?
alt references
EvalRules->>Analyzer: computeComplexity() (lazy, cached)
Analyzer->>Analyzer: extract ComplexityInput and analyze
Analyzer->>EvalRules: ComplexityResult (tier, score, word count)
EvalRules->>CELExec: evaluate rule with complexity_tier value
else not referenced
EvalRules->>CELExec: evaluate rule with complexity_tier unknown (partial)
end
CELExec->>EvalRules: boolean match result
end
EvalRules->>Apply: matched routing decision
Apply->>HTTP: rewritten model/provider
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
Confidence Score: 5/5Safe to merge — the lazy-computation, partial-evaluation, and fail-closed unknown handling are all correct; no existing routing paths are affected when rules do not reference complexity_tier. The core routing logic is unchanged for non-complexity rules. Complexity computation runs at most once per request, the CEL partial-evaluation path is correctly gated at program-creation time in the store, and the test suite covers the boundary conditions (negative predicates, unavailable complexity, chain steps, mixed-modality opt-out) thoroughly. routingcelrefs.go (unbounded package-level cache) and complexityextract.go (whole-request opt-out on any prior non-text user turn) have minor design trade-offs worth revisiting before scaling to high rule-churn deployments. Important Files Changed
Reviews (15): Last reviewed commit: "feat: complexity router : adds complexit..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
plugins/governance/routing.go (1)
191-202: ⚡ Quick winDefer complexity analysis until after CEL compilation succeeds.
computeComplexity()runs beforeGetRoutingProgram, so a malformed rule that mentionscomplexity_tierstill pays the request-time extraction/scoring cost even though the rule is skipped immediately afterward.♻️ Suggested change
re.logger.Debug("[RoutingEngine] Evaluating rule: name=%s, expression=%s", rule.Name, rule.CelExpression) referencesComplexity := celExpressionReferencesIdentifier(rule.CelExpression, "complexity_tier") - // Lazy complexity: compute only when a rule references complexity and it hasn't been computed yet - if complexityResult == nil && computeComplexity != nil && referencesComplexity { - complexityResult = computeComplexity() - computeComplexity = nil // compute at most once - if complexityResult != nil { - variables["complexity_tier"] = complexityResult.Tier - } - } - program, err := re.store.GetRoutingProgram(ctx, rule) if err != nil { re.logger.Warn("[RoutingEngine] Failed to compile rule %s: %v", rule.Name, err) ctx.AppendRoutingEngineLog(schemas.RoutingEngineRoutingRule, schemas.LogLevelError, fmt.Sprintf("Rule '%s' skipped: compile error: %v", rule.Name, err)) continue } + + // Lazy complexity: compute only when a valid rule references complexity + // and it hasn't been computed yet. + if complexityResult == nil && computeComplexity != nil && referencesComplexity { + complexityResult = computeComplexity() + computeComplexity = nil // compute at most once + if complexityResult != nil { + variables["complexity_tier"] = complexityResult.Tier + } + }🤖 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/governance/routing.go` around lines 191 - 202, The code eagerly calls computeComplexity() before attempting CEL compilation, causing wasted work for malformed rules; change the flow so referencesComplexity is still detected but delay calling computeComplexity() (and setting complexityResult/variables["complexity_tier"]) until after re.store.GetRoutingProgram(ctx, rule) returns successfully (no err). Keep the existing guard (computeComplexity != nil and complexityResult == nil) and the once-only behavior (set computeComplexity = nil after running) and use the same symbols (referencesComplexity, computeComplexity, complexityResult, re.store.GetRoutingProgram, rule.CelExpression) so complexity extraction/scoring only occurs for rules that compile.plugins/governance/main.go (1)
1003-1028: 💤 Low valueInconsistent nil-guard on
p.logger.Line 1009 guards
p.logger.Debugwithif p.logger != nil, but line 1024 callsp.logger.Debugunconditionally. For consistency (and defensive safety if logger could ever be nil), add the same guard on line 1024.♻️ Suggested fix
return result } - p.logger.Debug("[Governance] Complexity analysis skipped: unsupported request type") + if p.logger != nil { + p.logger.Debug("[Governance] Complexity analysis skipped: unsupported request type") + } ctx.AppendRoutingEngineLog(schemas.RoutingEngineRoutingRule, schemas.LogLevelInfo, "Complexity analysis skipped: no supported text-bearing input detected") return nil🤖 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/governance/main.go` around lines 1003 - 1028, The code in the computeComplexity closure uses p.logger.Debug guarded once but later calls p.logger.Debug at the end unguarded; update the closure used by computeComplexity to check p.logger != nil before calling p.logger.Debug on the "Complexity analysis skipped" path (the same nil-guard used earlier), so wrap that second p.logger.Debug call in an if p.logger != nil { ... } block to avoid potential nil dereference in the computeComplexity function/method.
🤖 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 `@plugins/governance/httptransportprehook_test.go`:
- Around line 480-486: The test's loop over bfCtx.GetRoutingEngineLogs()
currently looks for the substring "Complexity analysis" which misses success
logs like "Complexity: tier=..."; update the assertion to search for the broader
substring "Complexity" when comparing entry.Message for entries with Engine ==
schemas.RoutingEngineRoutingRule so any complexity-related log (both skip and
success) fails the test, e.g., change the strings.Contains check to use
"Complexity" and keep the same failure message and routing engine filter.
---
Nitpick comments:
In `@plugins/governance/main.go`:
- Around line 1003-1028: The code in the computeComplexity closure uses
p.logger.Debug guarded once but later calls p.logger.Debug at the end unguarded;
update the closure used by computeComplexity to check p.logger != nil before
calling p.logger.Debug on the "Complexity analysis skipped" path (the same
nil-guard used earlier), so wrap that second p.logger.Debug call in an if
p.logger != nil { ... } block to avoid potential nil dereference in the
computeComplexity function/method.
In `@plugins/governance/routing.go`:
- Around line 191-202: The code eagerly calls computeComplexity() before
attempting CEL compilation, causing wasted work for malformed rules; change the
flow so referencesComplexity is still detected but delay calling
computeComplexity() (and setting complexityResult/variables["complexity_tier"])
until after re.store.GetRoutingProgram(ctx, rule) returns successfully (no err).
Keep the existing guard (computeComplexity != nil and complexityResult == nil)
and the once-only behavior (set computeComplexity = nil after running) and use
the same symbols (referencesComplexity, computeComplexity, complexityResult,
re.store.GetRoutingProgram, rule.CelExpression) so complexity extraction/scoring
only occurs for rules that compile.
🪄 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: 851f02ff-74bb-40bc-8c32-a4bbad63851f
📒 Files selected for processing (14)
plugins/governance/complexity/analyzer.goplugins/governance/complexity/analyzer_test.goplugins/governance/complexity/config.goplugins/governance/complexity/keywords.goplugins/governance/complexity/matcher.goplugins/governance/complexity/utils.goplugins/governance/complexity_extract.goplugins/governance/complexity_extract_test.goplugins/governance/httptransportprehook_test.goplugins/governance/main.goplugins/governance/routing.goplugins/governance/routing_cel_refs.goplugins/governance/routing_complexity_test.goplugins/governance/store.go
89613dd to
7a674e4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/governance/httptransportprehook_test.go (1)
389-390: ⚡ Quick winUse
bifrost.Ptr(...)for routing target pointers in these tests.Please replace
&provider/&modelwithbifrost.Ptr(provider)/bifrost.Ptr(model)to stay consistent with repository pointer-construction conventions.♻️ Proposed fix
- {Provider: &provider, Model: &model, Weight: 1.0}, + {Provider: bifrost.Ptr(provider), Model: bifrost.Ptr(model), Weight: 1.0}, ... - {Provider: &provider, Model: &model, Weight: 1.0}, + {Provider: bifrost.Ptr(provider), Model: bifrost.Ptr(model), Weight: 1.0},Based on learnings: "prefer using bifrost.Ptr() to create pointers instead of the address operator (&) ... including test utilities."
Also applies to: 449-450
🤖 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/governance/httptransportprehook_test.go` around lines 389 - 390, Replace uses of the address operator for routing target pointers with the repository helper: change occurrences of &provider and &model to bifrost.Ptr(provider) and bifrost.Ptr(model) in the test so routing entries use bifrost.Ptr(...) consistently (look for the table entries constructing routing targets where Provider and Model fields are set). Ensure you update all occurrences mentioned (including the ones around lines indicated) so tests use bifrost.Ptr for pointer construction.
🤖 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 `@plugins/governance/complexity_extract.go`:
- Around line 177-193: The branch handling case []interface{} incorrectly
concatenates multiple prompt entries into one synthetic prompt (variable p) and
sets ComplexityInput.LastUserText, which merges separate prompts; instead, treat
batched prompts as unsupported for single-text complexity extraction by
returning complexity.ComplexityInput{} and false (i.e., do not build
LastUserText) so callers can handle batch prompts explicitly or fail closed;
update the case []interface{} branch to avoid joining items (remove the
strings.Builder logic) and simply return an empty ComplexityInput and false when
p is a []interface{}.
---
Nitpick comments:
In `@plugins/governance/httptransportprehook_test.go`:
- Around line 389-390: Replace uses of the address operator for routing target
pointers with the repository helper: change occurrences of &provider and &model
to bifrost.Ptr(provider) and bifrost.Ptr(model) in the test so routing entries
use bifrost.Ptr(...) consistently (look for the table entries constructing
routing targets where Provider and Model fields are set). Ensure you update all
occurrences mentioned (including the ones around lines indicated) so tests use
bifrost.Ptr for pointer construction.
🪄 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: eadf37b9-8a48-4090-9437-ac4a49de104c
📒 Files selected for processing (14)
plugins/governance/complexity/analyzer.goplugins/governance/complexity/analyzer_test.goplugins/governance/complexity/config.goplugins/governance/complexity/keywords.goplugins/governance/complexity/matcher.goplugins/governance/complexity/utils.goplugins/governance/complexity_extract.goplugins/governance/complexity_extract_test.goplugins/governance/httptransportprehook_test.goplugins/governance/main.goplugins/governance/routing.goplugins/governance/routing_cel_refs.goplugins/governance/routing_complexity_test.goplugins/governance/store.go
7a674e4 to
97c557c
Compare
97c557c to
299b806
Compare
299b806 to
898afcc
Compare
898afcc to
cbe2f5a
Compare
cbe2f5a to
7b91de3
Compare
live model cache store and port keyconfig regression tests for alias/model isolation
#4034
3dfab94 to
f9750b3
Compare
7a1013e to
c6758e2
Compare
f9750b3 to
9917bc3
Compare
c6758e2 to
f4f8a14
Compare
9917bc3 to
c10fe4d
Compare
|
|
✅ Action performedComments resolved. Approval is disabled; enable |
7f30ad6 to
45d048c
Compare
|
Tip For best results, initiate chat on the files or code changes.
To fix it, update the single line in reviews:
request_changes_workflow: true # change from false → trueOnce that change is merged into your default branch ( |
45d048c to
2abea67
Compare
Merge activity
|
) ## Summary Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [ ] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes maximhq#123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## Summary Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [ ] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes #123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

Summary
Briefly explain the purpose of this PR and the problem it solves.
Changes
Type of change
Affected areas
How to test
Describe the steps to validate this change. Include commands and expected outcomes.
If adding new configs or environment variables, document them here.
Screenshots/Recordings
If UI changes, add before/after screenshots or short clips.
Breaking changes
If yes, describe impact and migration instructions.
Related issues
Link related issues and discussions. Example: Closes #123
Security considerations
Note any security implications (auth, secrets, PII, sandboxing, etc.).
Checklist
docs/contributing/README.mdand followed the guidelines