Skip to content

complexity router : add complexity analyzer config DB and API changes - #3712

Merged
akshaydeo merged 1 commit into
devfrom
05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes
Jun 10, 2026
Merged

complexity router : add complexity analyzer config DB and API changes#3712
akshaydeo merged 1 commit into
devfrom
05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes

Conversation

@Madhuvod

@Madhuvod Madhuvod commented May 24, 2026

Copy link
Copy Markdown
Contributor

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.

# 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 by CodeRabbit

Release Notes

  • New Features
    • Added HTTP endpoints for managing complexity analyzer configuration (GET, PUT, POST /reset)
    • Complexity analyzer settings now support customizable tier boundaries and keyword lists
    • Configuration persists across service restarts with runtime reload capability
    • Configuration validation enforces integrity of tier boundaries and keyword definitions

@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds persisted, normalized ComplexityAnalyzerConfig (tier boundaries + keywords), schema definitions, RDB persistence and ConfigStore APIs, plugin defaults and matcher parameterization, atomic runtime reload with a ReloadComplexityAnalyzerConfig API, HTTP GET/PUT/POST endpoints, and tests syncing file, store, and runtime state.

Changes

Complexity Analyzer Runtime Configuration

Layer / File(s) Summary
Config store types and persistence
framework/configstore/complexityconfig.go, framework/configstore/clientconfig.go, framework/configstore/rdb.go, framework/configstore/store.go, framework/configstore/tables/config.go, framework/configstore/rdb_test.go
Defines ComplexityAnalyzerConfig, ComplexityTierBoundaries, normalization/validation, DecodeComplexityAnalyzerConfig, adds GovernanceConfig.ComplexityAnalyzerConfig, extends ConfigStore and RDBConfigStore with Get/UpdateComplexityAnalyzerConfig, and adds RDB tests for round-trip, governance inclusion, and invalid-tier validation.
Plugin config abstractions and matcher parameterization
plugins/governance/complexity/config.go, plugins/governance/complexity/matcher.go, plugins/governance/complexity/analyzer.go, plugins/governance/complexity/analyzer_test.go
Exports plugin-side aliases and KeywordConfig, provides Default* constructors and ValidateAndNormalize, parameterizes newCompiledKeywordMatcher(KeywordConfig), and makes ComplexityAnalyzer carry per-instance tier boundaries; tests verify tier and keyword overrides affect classification.
Atomic reload and runtime updates
plugins/governance/main.go
Replaces analyzer storage with atomic.Pointer, initializes from validated/normalized config (store or file) with default fallback, adds ReloadComplexityAnalyzerConfig to atomically replace the analyzer, and updates evaluation sites to use atomic.Load().
HTTP API endpoints and handler tests
transports/bifrost-http/handlers/governance.go, transports/bifrost-http/handlers/governance_test.go
Adds GET, PUT, and POST /reset endpoints for complexity-analyzer-config with strict JSON decoding, uses complexity.ValidateAndNormalize, persists via ConfigStore, triggers in-memory reloads, and includes comprehensive handler tests for defaults, persistence+reload, invalid payloads, and reset.
Config sync, schema, server wrapper, and tests
transports/bifrost-http/lib/config.go, transports/bifrost-http/lib/config_test.go, transports/bifrost-http/server/server.go, transports/config.schema.json
Validates/normalizes file-provided complexity config during merge/create, persists normalized config into store, extends test MockConfigStore, updates schema $defs and mappings, and adds server wrapper ReloadComplexityAnalyzerConfig.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • danpiths
  • akshaydeo
  • roroghost17

Poem

🐰 I hopped through code and schema bright,
New tiers and keywords set just right,
Atomics swap without a fight,
Endpoints hum by day and night,
Now analyzers bloom in light.

🚥 Pre-merge checks | ✅ 1 | ❌ 4

❌ Failed checks (3 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is completely empty—it only contains the unfilled template with no actual summary, changes, type, affected areas, testing details, or other required information. Complete the PR description by filling in all template sections: provide a summary of changes, explain design decisions, select change type(s), mark affected areas, include testing steps, declare breaking changes, link related issues, and note security considerations.
Linked Issues check ⚠️ Warning The linked issue #123 is about Files API support (uploading files for fine-tuning, RAG, etc.), which is entirely unrelated to the changeset that adds complexity analyzer configuration for routing rules. Remove the incorrect linked issue #123 and link the actual issues related to complexity analyzer configuration (if any exist) to clarify the PR's true objectives and scope.
Docstring Coverage ⚠️ Warning Docstring coverage is 37.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Out of Scope Changes check ❓ Inconclusive The changeset implements complexity analyzer configuration persistence and API endpoints, which align with the PR title's stated purpose. However, the linked issue describes an unrelated Files API feature, making it unclear what the actual intended scope should be. Clarify the PR's scope by correcting the linked issue(s) to reflect the actual complexity analyzer configuration work, or confirm whether Files API support should also be included.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'complexity router : add complexity analyzer config DB and API changes' directly matches the main changeset, which adds database configuration support and API endpoints for the complexity analyzer.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/configstore/rdb.go`:
- Around line 4208-4214: In GetGovernanceConfig (the switch handling
tables.ConfigComplexityAnalyzerConfigKey) skip DecodeComplexityAnalyzerConfig
when entry.Value is empty/zero-length and treat it as unset: check if
entry.Value == "" (or len(entry.Value) == 0) before calling
DecodeComplexityAnalyzerConfig, and if empty simply continue without logging a
warning; otherwise call DecodeComplexityAnalyzerConfig and keep the existing
error handling that logs via s.logger.Warn. Ensure you reference
tables.ConfigComplexityAnalyzerConfigKey and DecodeComplexityAnalyzerConfig in
the change.
🪄 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: f5124ad4-95ed-4c76-b054-c7e960035454

📥 Commits

Reviewing files that changed from the base of the PR and between 89613dd and 0dbc48b.

📒 Files selected for processing (17)
  • framework/configstore/clientconfig.go
  • framework/configstore/complexityconfig.go
  • framework/configstore/rdb.go
  • framework/configstore/rdb_test.go
  • framework/configstore/store.go
  • framework/configstore/tables/config.go
  • plugins/governance/complexity/analyzer.go
  • plugins/governance/complexity/analyzer_test.go
  • plugins/governance/complexity/config.go
  • plugins/governance/complexity/matcher.go
  • plugins/governance/main.go
  • transports/bifrost-http/handlers/governance.go
  • transports/bifrost-http/handlers/governance_test.go
  • transports/bifrost-http/lib/config.go
  • transports/bifrost-http/lib/config_test.go
  • transports/bifrost-http/server/server.go
  • transports/config.schema.json

Comment thread framework/configstore/rdb.go
@greptile-apps

greptile-apps Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Safe to merge; the concurrency model, validation pipeline, and HTTP handler flow are all correct.

The atomic swap of the analyzer pointer is correct, validation runs before every DB write, the HTTP handler chain (validate → persist → reload) is properly ordered, and the config file merge path is consistent with existing governance patterns. Test coverage is thorough across the DB, handler, and analyzer layers.

plugins/governance/complexity/config.go — the mapping of user-provided reasoning_keywords onto StrongReasoningKeywords only, leaving WeakReasoningKeywords as hidden defaults, is worth a follow-up documentation or API clarification.

Important Files Changed

Filename Overview
framework/configstore/complexityconfig.go New file: normalize/validate/decode helpers for ComplexityAnalyzerConfig; clean validation and normalization pipeline
framework/configstore/rdb.go Adds GetComplexityAnalyzerConfig and UpdateComplexityAnalyzerConfig to the RDB store; reads back the config in GetGovernanceConfig; correctly handles empty/invalid stored values
plugins/governance/complexity/config.go Introduces type aliases to configstore types, DefaultAnalyzerConfig, ValidateAndNormalize, and mergeEditableKeywordsOntoDefaults; weak reasoning keywords are not user-configurable (P2)
plugins/governance/main.go Changes complexityAnalyzer field to atomic.Pointer for safe concurrent swap; adds ReloadComplexityAnalyzerConfig and storeComplexityAnalyzerConfig helpers with fallback to defaults
transports/bifrost-http/handlers/governance.go Adds GET/PUT/POST reset endpoints for complexity analyzer config; correctly chains DB write + live reload; partial-success behavior on reload failure already noted in prior review thread
transports/bifrost-http/lib/config.go mergeGovernanceConfig now syncs file-sourced complexity config to DB with a change-detected update; createGovernanceConfigInStore seeds initial config inside the governance transaction
transports/bifrost-http/server/server.go Adds ReloadComplexityAnalyzerConfig bridge on BifrostHTTPServer that dispatches to the governance plugin via type assertion
transports/config.schema.json Adds complexity_tier_boundaries, complexity_analyzer_keywords, and complexity_analyzer_config schema definitions; referenced correctly from governance object

Reviews (17): Last reviewed commit: "complexity router : add complexity analy..." | Re-trigger Greptile

Comment thread plugins/governance/main.go
Comment thread transports/bifrost-http/handlers/governance.go
@Madhuvod
Madhuvod force-pushed the 05-24-feat_complexity_router_adds_complexity_tier_cel_routing branch from 89613dd to 7a674e4 Compare May 24, 2026 16:36
@Madhuvod
Madhuvod force-pushed the 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes branch from 0dbc48b to 88e2f0e Compare May 24, 2026 16:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
plugins/governance/main.go (1)

1050-1077: ⚡ Quick win

Memoize lazy complexity evaluation per request.

computeComplexity can be invoked multiple times during a single routing evaluation, which repeats analysis and emits duplicate logs. Cache the first result in the closure.

♻️ Proposed change
 	// Set up lazy complexity computation; only runs if a rule actually references "complexity_tier".
 	var computeComplexity func() *complexity.ComplexityResult
 	if analyzer := p.complexityAnalyzer.Load(); analyzer != nil {
+		var cached *complexity.ComplexityResult
+		computed := false
 		computeComplexity = func() *complexity.ComplexityResult {
+			if computed {
+				return cached
+			}
+			computed = true
 			if input, ok := buildComplexityInput(ctx, body); ok {
 				result := analyzer.Analyze(input)
 				if p.logger != nil {
 					p.logger.Debug(
 						"[Governance] Complexity analysis details: tier=%s score=%.2f words=%d",
 						result.Tier,
 						result.Score,
 						result.WordCount,
 					)
 				}
 				ctx.AppendRoutingEngineLog(
 					schemas.RoutingEngineRoutingRule,
 					schemas.LogLevelInfo,
 					fmt.Sprintf("Complexity: tier=%s score=%.2f words=%d", result.Tier, result.Score, result.WordCount),
 				)
-				return result
+				cached = result
+				return cached
 			}
 			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
+			cached = nil
+			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 1050 - 1077, The computeComplexity
closure currently re-runs buildComplexityInput and analyzer.Analyze on every
call, causing duplicate analysis and logs; change the closure (created when
p.complexityAnalyzer.Load() != nil) to memoize the outcome by introducing a
closure-scoped variable (e.g., cached *complexity.ComplexityResult and a bool
like computed) so the first invocation runs buildComplexityInput ->
analyzer.Analyze, emits the p.logger.Debug and ctx.AppendRoutingEngineLog
entries, stores the result in cached and sets computed=true, and subsequent
calls simply return cached (or nil) without repeating analysis or logging; keep
existing logic paths (unsupported input logs once) and use the same identifying
functions: computeComplexity, p.complexityAnalyzer.Load(), buildComplexityInput,
analyzer.Analyze, p.logger.Debug, and ctx.AppendRoutingEngineLog.
🤖 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/configstore/complexityconfig.go`:
- Around line 109-127: The normalizeComplexityKeywordList function currently
dedupes and preserves input order causing non-deterministic output; after
building the deduped slice `out` in normalizeComplexityKeywordList, sort it
deterministically (e.g., call sort.Strings(out)) before returning to ensure
canonical ordering; also add the "sort" import if it's not already present so
the function compiles.

In `@transports/bifrost-http/lib/config.go`:
- Around line 2687-2697: The write to ComplexityAnalyzerConfig inside the
ExecuteTransaction closure is not using the transaction (it calls
config.ConfigStore.UpdateComplexityAnalyzerConfig(ctx, normalized)), so make the
update transactional by adding a tx-aware API and using the transaction variable
from the closure: either change UpdateComplexityAnalyzerConfig to accept a tx
(e.g., UpdateComplexityAnalyzerConfig(ctx, tx, cfg)) or add
UpdateComplexityAnalyzerConfigTx(ctx, tx, cfg), then update the rdb
implementation to pass the tx into s.UpdateConfig (use the provided tx instead
of s.DB()) and call the tx-aware method from the ExecuteTransaction closure so
the complexity config persistence is included in the surrounding transaction.

---

Nitpick comments:
In `@plugins/governance/main.go`:
- Around line 1050-1077: The computeComplexity closure currently re-runs
buildComplexityInput and analyzer.Analyze on every call, causing duplicate
analysis and logs; change the closure (created when p.complexityAnalyzer.Load()
!= nil) to memoize the outcome by introducing a closure-scoped variable (e.g.,
cached *complexity.ComplexityResult and a bool like computed) so the first
invocation runs buildComplexityInput -> analyzer.Analyze, emits the
p.logger.Debug and ctx.AppendRoutingEngineLog entries, stores the result in
cached and sets computed=true, and subsequent calls simply return cached (or
nil) without repeating analysis or logging; keep existing logic paths
(unsupported input logs once) and use the same identifying functions:
computeComplexity, p.complexityAnalyzer.Load(), buildComplexityInput,
analyzer.Analyze, p.logger.Debug, and ctx.AppendRoutingEngineLog.
🪄 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: 26e4243f-3cc5-4310-868a-fcb4c49008b2

📥 Commits

Reviewing files that changed from the base of the PR and between 0dbc48b and 88e2f0e.

📒 Files selected for processing (17)
  • framework/configstore/clientconfig.go
  • framework/configstore/complexityconfig.go
  • framework/configstore/rdb.go
  • framework/configstore/rdb_test.go
  • framework/configstore/store.go
  • framework/configstore/tables/config.go
  • plugins/governance/complexity/analyzer.go
  • plugins/governance/complexity/analyzer_test.go
  • plugins/governance/complexity/config.go
  • plugins/governance/complexity/matcher.go
  • plugins/governance/main.go
  • transports/bifrost-http/handlers/governance.go
  • transports/bifrost-http/handlers/governance_test.go
  • transports/bifrost-http/lib/config.go
  • transports/bifrost-http/lib/config_test.go
  • transports/bifrost-http/server/server.go
  • transports/config.schema.json
✅ Files skipped from review due to trivial changes (1)
  • framework/configstore/tables/config.go

Comment thread framework/configstore/complexityconfig.go
Comment thread transports/bifrost-http/lib/config.go
@Madhuvod
Madhuvod force-pushed the 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes branch from 88e2f0e to 34d7901 Compare May 25, 2026 08:38
@Madhuvod
Madhuvod force-pushed the 05-24-feat_complexity_router_adds_complexity_tier_cel_routing branch from 7a674e4 to 97c557c Compare May 25, 2026 08:38
@coderabbitai
coderabbitai Bot requested a review from akshaydeo May 25, 2026 08:39
@Madhuvod
Madhuvod force-pushed the 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes branch from 34d7901 to 92c82f1 Compare May 25, 2026 08:40
@Madhuvod
Madhuvod force-pushed the 05-24-feat_complexity_router_adds_complexity_tier_cel_routing branch from 97c557c to 299b806 Compare May 25, 2026 08:40

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
plugins/governance/main.go (1)

1051-1077: ⚡ Quick win

Memoize per-request complexity computation inside computeComplexity.

Line 1051 currently recalculates/analyzes on every invocation. If multiple rules/functions touch complexity fields in one evaluation, this repeats scans and duplicate logs unnecessarily.

♻️ Proposed change
 	var computeComplexity func() *complexity.ComplexityResult
 	if analyzer := p.complexityAnalyzer.Load(); analyzer != nil {
+		var (
+			computed bool
+			cached   *complexity.ComplexityResult
+		)
 		computeComplexity = func() *complexity.ComplexityResult {
+			if computed {
+				return cached
+			}
+			computed = true
+
 			if input, ok := buildComplexityInput(ctx, body); ok {
-				result := analyzer.Analyze(input)
+				cached = analyzer.Analyze(input)
 				if p.logger != nil {
 					p.logger.Debug(
 						"[Governance] Complexity analysis details: tier=%s score=%.2f words=%d",
-						result.Tier,
-						result.Score,
-						result.WordCount,
+						cached.Tier,
+						cached.Score,
+						cached.WordCount,
 					)
 				}
 				ctx.AppendRoutingEngineLog(
 					schemas.RoutingEngineRoutingRule,
 					schemas.LogLevelInfo,
-					fmt.Sprintf("Complexity: tier=%s score=%.2f words=%d", result.Tier, result.Score, result.WordCount),
+					fmt.Sprintf("Complexity: tier=%s score=%.2f words=%d", cached.Tier, cached.Score, cached.WordCount),
 				)
-				return result
+				return cached
 			}
 			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
 		}
 	}

Also applies to: 1088-1088

🤖 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 1051 - 1077, The computeComplexity
closure currently runs analyzer.Analyze and emits logs on every call; change it
to memoize the result per request by introducing a local cached variable (e.g.,
cached *complexity.ComplexityResult and a flag like computed bool) inside the
scope where computeComplexity is defined and have the closure return the cached
value if computed is true. Ensure buildComplexityInput, analyzer.Analyze,
p.logger.Debug and ctx.AppendRoutingEngineLog are only invoked when computing
the result the first time (store the result in cached and set computed), and
subsequent calls simply return cached without re-running analysis or re-logging.
transports/bifrost-http/lib/config_test.go (1)

1364-1364: ⚡ Quick win

Avoid pointer-identity assertion in this test.

require.Same(...) makes the test fragile by enforcing aliasing, not behavior. A valid implementation that deep-copies/normalizes can fail this test even when semantics are correct. Prefer value-based assertions on normalized content instead.

Suggested change
-	require.Same(t, store.complexityConfig, config.GovernanceConfig.ComplexityAnalyzerConfig)
+	require.NotNil(t, config.GovernanceConfig.ComplexityAnalyzerConfig)
+	require.Equal(t, store.complexityConfig.TierBoundaries, config.GovernanceConfig.ComplexityAnalyzerConfig.TierBoundaries)
+	require.Equal(t, store.complexityConfig.Keywords, config.GovernanceConfig.ComplexityAnalyzerConfig.Keywords)
🤖 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 `@transports/bifrost-http/lib/config_test.go` at line 1364, The test currently
uses pointer-identity assertion require.Same(t, store.complexityConfig,
config.GovernanceConfig.ComplexityAnalyzerConfig) which is fragile; replace it
with a value-based assertion that verifies the semantics rather than aliasing.
Change the assertion to compare the contents of store.complexityConfig and
config.GovernanceConfig.ComplexityAnalyzerConfig (for example using
require.Equal / require.EqualValues or a deep-compare/cmp.Diff on those structs)
or compare specific normalized fields if normalization is required before
comparing; ensure you remove the require.Same usage and assert equality of
values instead.
🤖 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/governance/main.go`:
- Around line 1051-1077: The computeComplexity closure currently runs
analyzer.Analyze and emits logs on every call; change it to memoize the result
per request by introducing a local cached variable (e.g., cached
*complexity.ComplexityResult and a flag like computed bool) inside the scope
where computeComplexity is defined and have the closure return the cached value
if computed is true. Ensure buildComplexityInput, analyzer.Analyze,
p.logger.Debug and ctx.AppendRoutingEngineLog are only invoked when computing
the result the first time (store the result in cached and set computed), and
subsequent calls simply return cached without re-running analysis or re-logging.

In `@transports/bifrost-http/lib/config_test.go`:
- Line 1364: The test currently uses pointer-identity assertion require.Same(t,
store.complexityConfig, config.GovernanceConfig.ComplexityAnalyzerConfig) which
is fragile; replace it with a value-based assertion that verifies the semantics
rather than aliasing. Change the assertion to compare the contents of
store.complexityConfig and config.GovernanceConfig.ComplexityAnalyzerConfig (for
example using require.Equal / require.EqualValues or a deep-compare/cmp.Diff on
those structs) or compare specific normalized fields if normalization is
required before comparing; ensure you remove the require.Same usage and assert
equality of values instead.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 38781847-5c69-482c-9bb8-84c25dd9c05f

📥 Commits

Reviewing files that changed from the base of the PR and between 88e2f0e and 34d7901.

📒 Files selected for processing (17)
  • framework/configstore/clientconfig.go
  • framework/configstore/complexityconfig.go
  • framework/configstore/rdb.go
  • framework/configstore/rdb_test.go
  • framework/configstore/store.go
  • framework/configstore/tables/config.go
  • plugins/governance/complexity/analyzer.go
  • plugins/governance/complexity/analyzer_test.go
  • plugins/governance/complexity/config.go
  • plugins/governance/complexity/matcher.go
  • plugins/governance/main.go
  • transports/bifrost-http/handlers/governance.go
  • transports/bifrost-http/handlers/governance_test.go
  • transports/bifrost-http/lib/config.go
  • transports/bifrost-http/lib/config_test.go
  • transports/bifrost-http/server/server.go
  • transports/config.schema.json
✅ Files skipped from review due to trivial changes (1)
  • framework/configstore/tables/config.go

@Madhuvod
Madhuvod force-pushed the 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes branch 2 times, most recently from b2756f5 to d7bff03 Compare May 27, 2026 08:28
@Madhuvod
Madhuvod force-pushed the 05-24-feat_complexity_router_adds_complexity_tier_cel_routing branch from 299b806 to 898afcc Compare May 27, 2026 08:28

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
framework/configstore/complexityconfig.go (1)

110-130: 💤 Low value

Consider returning nil for all-empty results to match the empty-input pattern.

The function returns nil for an empty input (line 112), but returns an empty slice []string{} when all values are blank or duplicates after normalization. For consistency and to avoid allocating an empty slice, return nil when out is empty.

♻️ Proposed consistency fix
 	seen := make(map[string]struct{}, len(values))
 	out := make([]string, 0, len(values))
 	for _, value := range values {
 		normalized := strings.ToLower(strings.TrimSpace(value))
 		if normalized == "" {
 			continue
 		}
 		if _, ok := seen[normalized]; ok {
 			continue
 		}
 		seen[normalized] = struct{}{}
 		out = append(out, normalized)
 	}
+	if len(out) == 0 {
+		return nil
+	}
 	sort.Strings(out)
 	return out
 }
🤖 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 110 - 130,
normalizeComplexityKeywordList currently returns nil for empty input but returns
a non-nil empty slice when all entries normalize away; update the function so
that after building out it returns nil if len(out) == 0 (i.e., replace the final
return with a conditional that returns nil when out is empty) to keep the
empty-input/empty-result behavior consistent and avoid allocating an empty
slice.
🤖 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 110-130: normalizeComplexityKeywordList currently returns nil for
empty input but returns a non-nil empty slice when all entries normalize away;
update the function so that after building out it returns nil if len(out) == 0
(i.e., replace the final return with a conditional that returns nil when out is
empty) to keep the empty-input/empty-result behavior consistent and avoid
allocating an empty slice.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 052848a3-082d-4d19-8e00-4b50e830bbb4

📥 Commits

Reviewing files that changed from the base of the PR and between 34d7901 and d7bff03.

📒 Files selected for processing (17)
  • framework/configstore/clientconfig.go
  • framework/configstore/complexityconfig.go
  • framework/configstore/rdb.go
  • framework/configstore/rdb_test.go
  • framework/configstore/store.go
  • framework/configstore/tables/config.go
  • plugins/governance/complexity/analyzer.go
  • plugins/governance/complexity/analyzer_test.go
  • plugins/governance/complexity/config.go
  • plugins/governance/complexity/matcher.go
  • plugins/governance/main.go
  • transports/bifrost-http/handlers/governance.go
  • transports/bifrost-http/handlers/governance_test.go
  • transports/bifrost-http/lib/config.go
  • transports/bifrost-http/lib/config_test.go
  • transports/bifrost-http/server/server.go
  • transports/config.schema.json
💤 Files with no reviewable changes (6)
  • transports/config.schema.json
  • transports/bifrost-http/server/server.go
  • transports/bifrost-http/handlers/governance.go
  • transports/bifrost-http/handlers/governance_test.go
  • transports/bifrost-http/lib/config.go
  • transports/bifrost-http/lib/config_test.go
✅ Files skipped from review due to trivial changes (1)
  • framework/configstore/tables/config.go
🚧 Files skipped from review as they are similar to previous changes (9)
  • framework/configstore/store.go
  • framework/configstore/clientconfig.go
  • plugins/governance/complexity/matcher.go
  • plugins/governance/complexity/analyzer.go
  • plugins/governance/complexity/analyzer_test.go
  • plugins/governance/complexity/config.go
  • framework/configstore/rdb.go
  • plugins/governance/main.go
  • framework/configstore/rdb_test.go

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 27, 2026
@Madhuvod
Madhuvod force-pushed the 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes branch from d7bff03 to 5f16493 Compare May 27, 2026 12:04
@Madhuvod
Madhuvod force-pushed the 05-24-feat_complexity_router_adds_complexity_tier_cel_routing branch from 898afcc to cbe2f5a Compare May 27, 2026 12:04
@coderabbitai
coderabbitai Bot requested a review from danpiths May 27, 2026 12:09
@coderabbitai

coderabbitai Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

@Madhuvod Sure, I'll re-review the PR right away!

✅ Actions performed

Full review triggered.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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/configstore/rdb_test.go`:
- Around line 116-125: Extend
TestRDBConfigStore_UpdateComplexityAnalyzerConfigRejectsInvalidConfig into a
table-driven test that exercises additional invalid cases for
UpdateComplexityAnalyzerConfig: create variants of
testComplexityAnalyzerConfig() where TierBoundaries contain values ≤0 or ≥1
(e.g., -0.1, 0, 1.0), where ordering is wrong (e.g., ComplexReasoning <
MediumComplex), and where keyword arrays required by the schema are empty; for
each variant call store.UpdateComplexityAnalyzerConfig(ctx, invalid) and assert
require.Error(t, err). Reference the existing test name
TestRDBConfigStore_UpdateComplexityAnalyzerConfigRejectsInvalidConfig, the
helper testComplexityAnalyzerConfig(), and the UpdateComplexityAnalyzerConfig
method and TierBoundaries fields to locate and implement these cases.

In `@plugins/governance/complexity/analyzer.go`:
- Around line 5-10: The comment on ComplexityAnalyzer is incorrect: it no longer
is stateless because it stores immutable configuration in tierBoundaries and
matcher; update the struct comment for ComplexityAnalyzer to say it holds
immutable configuration (tierBoundaries and matcher) and remains safe for
concurrent use rather than calling it "stateless". Mention the specific fields
(tierBoundaries, matcher) and that they are immutable after construction and
concurrency-safe.

In `@transports/bifrost-http/handlers/governance.go`:
- Around line 528-533: When persisting new runtime configs you must make the
write rollback-aware: before calling UpdateComplexityAnalyzerConfig take and
keep the current config (via the store or an existing getter), then call
UpdateComplexityAnalyzerConfig(ctx, normalized); if
reloadComplexityAnalyzerConfig(ctx, normalized) fails, perform a compensating
rollback by calling UpdateComplexityAnalyzerConfig(ctx, previous) (and log both
the reload and rollback outcomes via process logger/SendError), return 500 only
after attempting rollback; apply the same pattern to the other handlers
referenced (the methods around lines 547-552 and 559-564) that persist then
reload so persistent store and in-memory state never diverge.

In `@transports/bifrost-http/lib/config_test.go`:
- Line 389: Tests store complexity config in two separate fields
(complexityConfig and governanceConfig.ComplexityAnalyzerConfig) causing drift;
update the mock store so there is a single source of truth: pick one canonical
field (e.g., governanceConfig.ComplexityAnalyzerConfig) and make all
setters/getters and seed logic (including any functions like
SetComplexityAnalyzerConfig, GetComplexityAnalyzerConfig, and the test seeding
paths around complexityConfig) read from and write to that canonical field only,
and remove or forward the duplicate complexityConfig field so reads behave the
same as the real store.
- Line 1427: The test currently asserts pointer identity with require.Same on
store.complexityConfig vs config.GovernanceConfig.ComplexityAnalyzerConfig;
change this to a value equality assertion (e.g., require.Equal or
require.EqualValues) so the test verifies the configs' contents rather than
pointer identity, avoiding fragile failures when a defensive copy is returned.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: 14a5b74f-0a9d-492c-93c9-40f02d170342

📥 Commits

Reviewing files that changed from the base of the PR and between 62a6254 and deb50b7.

📒 Files selected for processing (17)
  • framework/configstore/clientconfig.go
  • framework/configstore/complexityconfig.go
  • framework/configstore/rdb.go
  • framework/configstore/rdb_test.go
  • framework/configstore/store.go
  • framework/configstore/tables/config.go
  • plugins/governance/complexity/analyzer.go
  • plugins/governance/complexity/analyzer_test.go
  • plugins/governance/complexity/config.go
  • plugins/governance/complexity/matcher.go
  • plugins/governance/main.go
  • transports/bifrost-http/handlers/governance.go
  • transports/bifrost-http/handlers/governance_test.go
  • transports/bifrost-http/lib/config.go
  • transports/bifrost-http/lib/config_test.go
  • transports/bifrost-http/server/server.go
  • transports/config.schema.json

Comment thread framework/configstore/rdb_test.go
Comment thread plugins/governance/complexity/analyzer.go
Comment thread transports/bifrost-http/handlers/governance.go
Comment thread transports/bifrost-http/lib/config_test.go Outdated
Comment thread transports/bifrost-http/lib/config_test.go Outdated
@Madhuvod
Madhuvod force-pushed the 05-24-feat_complexity_router_adds_complexity_tier_cel_routing branch from 62a6254 to b0f60d8 Compare June 1, 2026 10:06
@Madhuvod
Madhuvod force-pushed the 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes branch from deb50b7 to 00d7bee Compare June 1, 2026 10:06
@Madhuvod
Madhuvod force-pushed the 05-24-feat_complexity_router_adds_complexity_tier_cel_routing branch from b0f60d8 to 3dfab94 Compare June 1, 2026 11:54
@Madhuvod
Madhuvod force-pushed the 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes branch from 00d7bee to 7e06261 Compare June 1, 2026 11:54
@Madhuvod
Madhuvod force-pushed the 05-24-feat_complexity_router_adds_complexity_tier_cel_routing branch from 3dfab94 to f9750b3 Compare June 8, 2026 12:21
@Madhuvod
Madhuvod force-pushed the 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes branch from 7e06261 to aa8c316 Compare June 8, 2026 12:21
@akshaydeo
akshaydeo force-pushed the 05-24-feat_complexity_router_adds_complexity_tier_cel_routing branch 2 times, most recently from 9917bc3 to c10fe4d Compare June 9, 2026 05:38
@akshaydeo
akshaydeo force-pushed the 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes branch from aa8c316 to 8377b5a Compare June 9, 2026 05:38
@Madhuvod
Madhuvod force-pushed the 05-24-feat_complexity_router_adds_complexity_tier_cel_routing branch from c10fe4d to 7f30ad6 Compare June 9, 2026 09:09
@Madhuvod
Madhuvod force-pushed the 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes branch from 8377b5a to 87b2afc Compare June 9, 2026 09:09
@Madhuvod
Madhuvod force-pushed the 05-24-feat_complexity_router_adds_complexity_tier_cel_routing branch from 7f30ad6 to 45d048c Compare June 9, 2026 11:36
@Madhuvod
Madhuvod force-pushed the 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes branch 2 times, most recently from feb5264 to 4d0a27b Compare June 10, 2026 12:56
@Madhuvod
Madhuvod force-pushed the 05-24-feat_complexity_router_adds_complexity_tier_cel_routing branch from 45d048c to 2abea67 Compare June 10, 2026 12:56

akshaydeo commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • Jun 10, 1:30 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jun 10, 1:33 PM UTC: Graphite rebased this pull request as part of a merge.
  • Jun 10, 1:34 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo changed the base branch from 05-24-feat_complexity_router_adds_complexity_tier_cel_routing to graphite-base/3712 June 10, 2026 13:31
@akshaydeo
akshaydeo changed the base branch from graphite-base/3712 to dev June 10, 2026 13:31
@akshaydeo
akshaydeo dismissed coderabbitai[bot]’s stale review June 10, 2026 13:31

The base branch was changed.

@akshaydeo
akshaydeo force-pushed the 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes branch from 4d0a27b to d7fc48a Compare June 10, 2026 13:32
@akshaydeo
akshaydeo merged commit 321c618 into dev Jun 10, 2026
13 checks passed
@akshaydeo
akshaydeo deleted the 05-24-complexity_router_add_complexity_analyzer_config_db_and_api_changes branch June 10, 2026 13:34
akshaydeo pushed a commit that referenced this pull request Jun 12, 2026
…#3712)

## 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




<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

## Release Notes

* **New Features**
  * Added HTTP endpoints for managing complexity analyzer configuration (`GET`, `PUT`, `POST /reset`)
  * Complexity analyzer settings now support customizable tier boundaries and keyword lists
  * Configuration persists across service restarts with runtime reload capability
  * Configuration validation enforces integrity of tier boundaries and keyword definitions

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Files API Support

2 participants