semantic router #2: semantic routing config - #5655
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 (9)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds semantic complexity configuration with timeout handling, validation, schema support, hashing, persistence, merge behavior, and startup checks for external vector stores. ChangesSemantic complexity configuration
Estimated code review effort: 4 (Complex) | ~60 minutes Mergeability Score: ⚪ Minimal · up to The change adds semantic routing configuration with focused persistence, validation, schema, and timeout handling tests; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant LoadConfig
participant GovernanceConfig
participant validateComplexitySemanticVectorStore
participant VectorStore
LoadConfig->>GovernanceConfig: load governance configuration
GovernanceConfig->>validateComplexitySemanticVectorStore: validate semantic vector-store mode
validateComplexitySemanticVectorStore->>VectorStore: check external vector-store initialization
VectorStore-->>validateComplexitySemanticVectorStore: return configured or missing state
validateComplexitySemanticVectorStore-->>LoadConfig: return validation result
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 `@transports/config.schema.json`:
- Around line 3518-3544: The provider enum in the complexity classification
configuration is narrower than the providers accepted by
ComplexitySemanticConfig.Validate and custom provider names. Remove the
hardcoded enum from the provider property while retaining type string and
minLength 1, so all non-empty runtime-supported and custom provider values pass
schema validation.
🪄 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: 2aea0e26-0767-46b6-8056-7b7db176f435
📒 Files selected for processing (8)
framework/configstore/clientconfig.goframework/configstore/complexityconfig.goframework/configstore/complexityconfig_test.goframework/configstore/rdb.goplugins/governance/complexity/config.gotransports/bifrost-http/lib/config.gotransports/config.schema.jsontransports/schema_test/config_schema_test.go
65d0768 to
767dcc5
Compare
25e7b2a to
0298243
Compare
767dcc5 to
00825bc
Compare
00825bc to
aa0217c
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
framework/configstore/rdb.go (1)
5360-5398: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake the read-modify-write for
ConfigHashes/EmbeddingFingerprintatomic.
UpdateComplexityAnalyzerConfigreads the existing row, merges missing fields, then writes it back — a separate DB round-trip withoutclause.Locking. Concurrent writers such as an API payload/runtime request and file merge can each read stale data and overwrite the other’s field; use a row lock around the read and update, ensuring callers consistently run in a shared transaction.🤖 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 `@framework/configstore/rdb.go` around lines 5360 - 5398, Make UpdateComplexityAnalyzerConfig perform the read-modify-write for ConfigHashes and EmbeddingFingerprint within a shared transaction: when either field is missing, begin or reuse a transaction, lock the existing configuration row with the appropriate clause.Locking, merge the missing values, and execute the update through that same transaction. Preserve validation and encoding behavior, and ensure the transaction commits on success or rolls back on failure.
🤖 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.
Outside diff comments:
In `@framework/configstore/rdb.go`:
- Around line 5360-5398: Make UpdateComplexityAnalyzerConfig perform the
read-modify-write for ConfigHashes and EmbeddingFingerprint within a shared
transaction: when either field is missing, begin or reuse a transaction, lock
the existing configuration row with the appropriate clause.Locking, merge the
missing values, and execute the update through that same transaction. Preserve
validation and encoding behavior, and ensure the transaction commits on success
or rolls back on failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f820afb1-d22f-406f-8d93-8b628d393300
📒 Files selected for processing (8)
framework/configstore/clientconfig.goframework/configstore/complexityconfig.goframework/configstore/complexityconfig_test.goframework/configstore/rdb.goplugins/governance/complexity/config.gotransports/bifrost-http/lib/config.gotransports/config.schema.jsontransports/schema_test/config_schema_test.go
aa0217c to
d2438a7
Compare
0298243 to
12fd6e7
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
framework/configstore/complexityconfig.go (1)
441-448: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
normalized()calls on already-normalized values.
mergeComplexitySemanticConfigreceivesnormalizedBase.Semantic/normalizedFile.Semantic(both produced byNormalized()), and line 491 re-normalizesnormalizedFile.Semanticagain. Harmless becausenormalized()is idempotent, but it does an extra allocation and slightly obscures the invariant. Consider dropping the inner calls, or keeping them and documenting that they exist purely for defensive copying.Also applies to: 486-494
🤖 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 `@framework/configstore/complexityconfig.go` around lines 441 - 448, The merge path redundantly normalizes semantic configurations that are already normalized by Normalized(). Update mergeComplexitySemanticConfig and its callers around normalizedBase.Semantic and normalizedFile.Semantic to reuse the normalized values directly, preserving the nil-file behavior and avoiding extra allocations.framework/configstore/rdb.go (1)
3065-3092: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated
relationclosure across the two preload helpers.
preloadCustomerRelationsnow redefines the same prefix closure thatpreloadCustomerRelationsWithoutVirtualKeysalready has, only to build one relation name. Inlining it (prefix + "VirtualKeys"guarded on empty prefix, or a small package-level helper) would remove the copy.🤖 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 `@framework/configstore/rdb.go` around lines 3065 - 3092, Remove the duplicated relation closure from preloadCustomerRelations and construct the VirtualKeys preload path using the existing prefix behavior directly or a shared package-level helper. Keep preloadCustomerRelationsWithoutVirtualKeys unchanged and preserve correct relation names for both empty and non-empty prefixes.
🤖 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 `@framework/configstore/complexityconfig.go`:
- Around line 441-448: The merge path redundantly normalizes semantic
configurations that are already normalized by Normalized(). Update
mergeComplexitySemanticConfig and its callers around normalizedBase.Semantic and
normalizedFile.Semantic to reuse the normalized values directly, preserving the
nil-file behavior and avoiding extra allocations.
In `@framework/configstore/rdb.go`:
- Around line 3065-3092: Remove the duplicated relation closure from
preloadCustomerRelations and construct the VirtualKeys preload path using the
existing prefix behavior directly or a shared package-level helper. Keep
preloadCustomerRelationsWithoutVirtualKeys unchanged and preserve correct
relation names for both empty and non-empty prefixes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e37c887c-6d59-4ec7-bd29-f2aefd50e739
📒 Files selected for processing (8)
framework/configstore/clientconfig.goframework/configstore/complexityconfig.goframework/configstore/complexityconfig_test.goframework/configstore/rdb.goplugins/governance/complexity/config.gotransports/bifrost-http/lib/config.gotransports/config.schema.jsontransports/schema_test/config_schema_test.go
904c40b to
efdd349
Compare
d9e67d3 to
27ada5c
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. |
27ada5c to
e27aca7
Compare
efdd349 to
4274871
Compare
e27aca7 to
e812e4c
Compare
4274871 to
b79354e
Compare
e812e4c to
94b8ad8
Compare
b79354e to
2d0a4a5
Compare
94b8ad8 to
86d4d58
Compare
2d0a4a5 to
8703860
Compare
86d4d58 to
9f4aace
Compare
8703860 to
5fe6384
Compare
9f4aace to
ee0b438
Compare
5fe6384 to
88178b5
Compare
ee0b438 to
39989f8
Compare
88178b5 to
21dad38
Compare
39989f8 to
621d619
Compare
21dad38 to
03b94f2
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 a
semanticconfiguration block to the complexity analyzer that enables embedding-based tier classification. When present, the classifier generates embeddings for incoming requests and compares them against per-tier exemplar utterances (derived from the shared keyword lists) to determine complexity, falling back to lexical classification (or no classification) when the embedding call is unavailable or exceeds its timeout.Changes
ComplexitySemanticConfigwith fields for provider, embedding model, vector dimension, timeout, fallback behavior, budget tracking, and vector store selection.Timeoutsupports both duration strings ("100ms") and millisecond numbers in JSON, with customMarshalJSON/UnmarshalJSONto ensure round-trip fidelity.SemanticSettingstoComplexityAnalyzerConfigHashes. Scalar settings and the shared keyword lists are hashed independently so edits to one do not register as changes to the other.GenerateComplexityAnalyzerConfigHashesnow populatesSemanticSettingswhen a semantic section is present.MergeComplexityAnalyzerConfigandMergeComplexityAnalyzerConfigByHasheshandle the semantic section: a missing file section leaves DB state untouched, and hash changes replace the semantic block wholesale.EmbeddingFingerprinttoComplexityAnalyzerConfig(persisted as_embedding_fingerprint) so warmup can detect when stored exemplar embeddings need to be recomputed.UpdateComplexityAnalyzerConfigpreserves the storedEmbeddingFingerprintwhen the caller does not supply one, preventing UI writes from wiping it.validateComplexitySemanticVectorStore) that fails fast whenvector_storeis set to"external"but no vector store is configured.SemanticConfigtype alias from the complexity plugin package.config.schema.jsonwith thecomplexity_semantic_configdefinition, including enum constraints onfallbackandvector_store, and a dual-type (string/number) schema fortimeout.ComplexityAnalyzerConfigHashes.Empty()andEqual()now use struct comparison instead of field-by-field checks.Type of change
Affected areas
How to test
go test ./framework/configstore/... ./transports/schema_test/... ./plugins/governance/complexity/...Key scenarios covered by the new tests:
TestComplexitySemanticConfigTimeoutDecoding— verifies duration strings, millisecond numbers, absent/null values, and rejection of negative or unparseable values.TestComplexitySemanticConfigTimeoutMarshalRoundTrip— confirms JSON encode → decode preserves the timeout.TestComplexitySemanticConfigNormalizedDefaults— checks that defaults (100 ms timeout,lexicalfallback,embeddedvector store) are applied.TestComplexitySemanticConfigValidation— exercises all invalid-field paths.TestGenerateComplexityAnalyzerConfigHashesSemantic— asserts that keyword edits do not move the semantic settings hash and vice versa.TestMergeComplexityAnalyzerConfigByHashesSemantic— covers first-time addition, unchanged-hash preservation, hash-triggered replacement, and file-without-semantic preservation.TestRDBConfigStore_ComplexityAnalyzerConfigSemanticPersistence— integration test confirming round-trip persistence and fingerprint preservation across UI-style writes.TestSchemaComplexitySemanticConfig— JSON Schema validation for valid and invalid semantic blocks.New config fields (
governance.complexity_analyzer_config.semantic):providerembedding_modeldimensiontimeout"100ms"fallback"lexical"|"none""lexical"count_toward_budgetsfalsevector_store"auto"|"embedded"|"external""embedded"Breaking changes
Security considerations
Embedding calls are made against the configured provider using existing credential plumbing. The
count_toward_budgetsflag is record-only and never enforced, so there is no risk of classification embeddings triggering budget blocks.Checklist
docs/contributing/README.mdand followed the guidelines