Skip to content

refactor: Extracts routing utilites to a separate package#2544

Merged
akshaydeo merged 6 commits intov1.5.0from
04-07-refactor_extracts_routing_utilites_to_a_separate_package
Apr 16, 2026
Merged

refactor: Extracts routing utilites to a separate package#2544
akshaydeo merged 6 commits intov1.5.0from
04-07-refactor_extracts_routing_utilites_to_a_separate_package

Conversation

@roroghost17
Copy link
Copy Markdown
Contributor

@roroghost17 roroghost17 commented Apr 7, 2026

Summary

Extracted routing-related CEL (Common Expression Language) functionality into a dedicated framework package to improve code organization and reusability. This refactoring moves CEL environment creation, expression validation, and map key normalization functions from the governance plugin to a shared routing framework.

Changes

  • Created new framework/routing/routing.go package with exported functions for CEL operations
  • Moved createCELEnvironment, validateCELExpression, and normalizeMapKeysInCEL functions from governance plugin to routing framework
  • Updated governance plugin to import and use the new routing framework functions
  • Modified test files to reference the new exported functions from the routing package
  • Removed duplicate regex patterns and function definitions from governance plugin

Type of change

  • Refactor
  • Bug fix
  • Feature
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Plugins
  • Transports (HTTP)
  • Providers/Integrations
  • UI (Next.js)
  • Docs

How to test

Validate that the refactoring maintains existing functionality by running the test suite:

# Core/Transports
go version
go test ./...

# Specifically test the affected areas
go test ./framework/routing/...
go test ./plugins/governance/...

Screenshots/Recordings

N/A - This is a code refactoring with no UI changes.

Breaking changes

  • Yes
  • No

This is an internal refactoring that maintains the same public API and functionality.

Related issues

N/A

Security considerations

No security implications - this is a code organization refactoring that maintains the same CEL expression validation and normalization logic.

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 7, 2026

📝 Walkthrough

Summary by CodeRabbit

  • Refactor

    • Centralized CEL expression normalization and validation moved into a shared framework layer for consistent handling across routing and governance, removing duplicated local helpers.
  • Behavior

    • Normalization now canonicalizes header and parameter keys for case-insensitive matching.
    • Validation accepts empty expressions and the literals "true"/"false" and requires at least one operator/token fragment.
  • Tests

    • Tests updated to use the centralized utilities.

Walkthrough

Adds package framework/routing with two exported functions: NormalizeMapKeysInCEL (lowercases quoted headers/params keys in CEL expressions) and ValidateCELExpression (lightweight operator/keyword presence check). Governance plugin helpers were removed and call sites updated to use the new routing utilities.

Changes

Cohort / File(s) Summary
Framework Routing Module
framework/routing/routing.go
New file exporting NormalizeMapKeysInCEL(expr string) string and ValidateCELExpression(expr string) error. Implements regex-driven lowercasing for quoted headers/params keys and a simple validation that checks for operator/keyword fragments or accepts empty/true/false.
Governance Plugin — Call sites
plugins/governance/store.go, plugins/governance/routing_test.go
Replaced local CEL preprocessing/validation calls with routing.NormalizeMapKeysInCEL and routing.ValidateCELExpression; tests updated to import framework/routing.
Governance Plugin — Removed helpers
plugins/governance/routing.go
Removed local regex variables and helper functions that previously normalized/validated CEL expressions; runtime lowercasing of extracted headers/params remains in extractRoutingVariables.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nudged the keys to lowercase, quick and bright,
Shared helpers hopped in, keeping logic light,
Expressions checked for operators, trimmed with care,
Tests followed the trail and found them waiting there,
A rabbit's little tweak — tidy, neat, and right.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main refactoring objective: extracting routing utilities to a separate package, which is clearly reflected in the code changes moving CEL functions from governance plugin to framework/routing package.
Description check ✅ Passed The description comprehensively covers all required template sections: summary explaining the purpose, detailed changes listing what was moved, type of change (refactor) checked, affected areas marked (Core/Go and Plugins), testing instructions provided, breaking changes clearly stated as none, and security considerations documented.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 04-07-refactor_extracts_routing_utilites_to_a_separate_package

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

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@roroghost17 roroghost17 force-pushed the 04-07-refactor_extracts_routing_utilites_to_a_separate_package branch from 7e6de90 to ea62825 Compare April 7, 2026 11:43
@roroghost17 roroghost17 force-pushed the 03-28-feat_prompt-deployment-strategies-fallback-screens branch from 2d9950f to 1554b53 Compare April 7, 2026 11:43
Comment thread framework/routing/routing.go Outdated
@roroghost17 roroghost17 force-pushed the 04-07-refactor_extracts_routing_utilites_to_a_separate_package branch 2 times, most recently from 869bf55 to e65dccc Compare April 7, 2026 14:22
@roroghost17 roroghost17 force-pushed the 03-28-feat_prompt-deployment-strategies-fallback-screens branch from 1554b53 to e807cae Compare April 7, 2026 19:50
@roroghost17 roroghost17 force-pushed the 04-07-refactor_extracts_routing_utilites_to_a_separate_package branch from e65dccc to 633d32f Compare April 7, 2026 19:50
@roroghost17 roroghost17 force-pushed the 03-28-feat_prompt-deployment-strategies-fallback-screens branch from e807cae to cb703ef Compare April 9, 2026 13:20
@roroghost17 roroghost17 force-pushed the 04-07-refactor_extracts_routing_utilites_to_a_separate_package branch from 633d32f to 66cc058 Compare April 9, 2026 13:20
@roroghost17 roroghost17 force-pushed the 04-07-refactor_extracts_routing_utilites_to_a_separate_package branch from 66cc058 to f37eed7 Compare April 9, 2026 13:22
@roroghost17 roroghost17 force-pushed the 03-28-feat_prompt-deployment-strategies-fallback-screens branch 2 times, most recently from 649c739 to 6896ceb Compare April 10, 2026 07:03
@roroghost17 roroghost17 force-pushed the 04-07-refactor_extracts_routing_utilites_to_a_separate_package branch from f37eed7 to 4416453 Compare April 10, 2026 07:03
@roroghost17 roroghost17 marked this pull request as ready for review April 10, 2026 07:24
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 10, 2026

Confidence Score: 5/5

Safe to merge — all extracted functions are pure string utilities, the governance plugin compiles and behaves identically to before, and the only finding is a documentation mismatch in the PR description.

All remaining findings are P2: the implementation is functionally correct, the two extracted functions are pure string utilities with no side effects, and createCELEnvironment still resides and works correctly in the governance plugin.

No files require special attention; the discrepancy is in the PR description, not the code.

Important Files Changed

Filename Overview
framework/routing/routing.go New package exporting NormalizeMapKeysInCEL and ValidateCELExpression; correctly avoids importing cel-go, keeping the framework dependency-light.
plugins/governance/routing.go createCELEnvironment remains here as a private function; store.go still calls it locally. Functionally correct; the function was not extracted as the PR description states.
plugins/governance/routing_test.go Updated to use routing.ValidateCELExpression and routing.NormalizeMapKeysInCEL; TestCreateCELEnvironment constructs an env with different variables (rate_limit/budget maps) than the actual createCELEnvironment() function uses, but this appears to be a pre-existing discrepancy.
plugins/governance/store.go Now delegates NormalizeMapKeysInCEL and ValidateCELExpression calls to the framework/routing package; createCELEnvironment is still called locally and correctly initialises routingCELEnv at store construction time.

Reviews (10): Last reviewed commit: "refactor: Extracts routing utilites to a..." | Re-trigger Greptile

Copy link
Copy Markdown
Contributor Author

@CodeRabbit-ai full review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 12, 2026

@roroghost17 Sure! I'll redo the full review of the changes in this PR now.

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

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 the current code and only fix it if needed.

Inline comments:
In `@framework/routing/routing.go`:
- Around line 30-34: The regex replacements on expr using headerKeyPattern,
paramKeyPattern, headerInPattern and paramInPattern can mutate quoted CEL string
literals; change the normalization to be token-aware by first scanning expr and
skipping any text inside quoted string tokens (single and double quotes, with
escape handling) before applying ReplaceAllStringFunc, or alternatively parse
into CEL AST/tokens and apply the key-lowering only to identifier/map-key lookup
nodes; ensure headerKeyPattern, paramKeyPattern, headerInPattern and
paramInPattern are only applied to regions outside quoted strings so expressions
like model == "headers['X-Tier']" are left unchanged.
🪄 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: ff6ebcbb-8e0f-4675-9cf7-d83bb1c198a2

📥 Commits

Reviewing files that changed from the base of the PR and between 6896ceb and 4416453.

📒 Files selected for processing (4)
  • framework/routing/routing.go
  • plugins/governance/routing.go
  • plugins/governance/routing_test.go
  • plugins/governance/store.go
💤 Files with no reviewable changes (1)
  • plugins/governance/routing.go

Comment thread framework/routing/routing.go
@roroghost17 roroghost17 force-pushed the 04-07-refactor_extracts_routing_utilites_to_a_separate_package branch 2 times, most recently from fbf80a2 to c8e97d7 Compare April 13, 2026 13:46
@roroghost17 roroghost17 force-pushed the 03-28-feat_prompt-deployment-strategies-fallback-screens branch from 246c43c to 2b88043 Compare April 13, 2026 13:46
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
framework/routing/routing.go (1)

10-20: ⚠️ Potential issue | 🟡 Minor

Add identifier boundaries to map-name regexes.

These patterns still match headers/params as substrings of longer identifiers (e.g., xheaders["K"], "k" in headersMap), which can rewrite unrelated symbols.

Suggested patch
-var headerKeyPattern = regexp.MustCompile(`headers\[["']([^"']+)["']\]`)
+var headerKeyPattern = regexp.MustCompile(`\bheaders\[["']([^"']+)["']\]`)

-var headerInPattern = regexp.MustCompile(`["']([^"']+)["']\s+in\s+headers`)
+var headerInPattern = regexp.MustCompile(`["']([^"']+)["']\s+in\s+\bheaders\b`)

-var paramKeyPattern = regexp.MustCompile(`params\[["']([^"']+)["']\]`)
+var paramKeyPattern = regexp.MustCompile(`\bparams\[["']([^"']+)["']\]`)

-var paramInPattern = regexp.MustCompile(`["']([^"']+)["']\s+in\s+params`)
+var paramInPattern = regexp.MustCompile(`["']([^"']+)["']\s+in\s+\bparams\b`)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@framework/routing/routing.go` around lines 10 - 20, The existing regexes
headerKeyPattern, headerInPattern, paramKeyPattern and paramInPattern
incorrectly match when "headers" or "params" appear as substrings of longer
identifiers; update each pattern to require identifier boundaries around the map
name (e.g., use word-boundary or negative lookbehind/lookahead for identifier
chars so only standalone headers or params match) while preserving the captured
key group and the rest of the pattern; apply this change to all four vars so
rewrites only target true map access/membership like headers["X"] or "X" in
params.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@framework/routing/routing.go`:
- Around line 10-20: The existing regexes headerKeyPattern, headerInPattern,
paramKeyPattern and paramInPattern incorrectly match when "headers" or "params"
appear as substrings of longer identifiers; update each pattern to require
identifier boundaries around the map name (e.g., use word-boundary or negative
lookbehind/lookahead for identifier chars so only standalone headers or params
match) while preserving the captured key group and the rest of the pattern;
apply this change to all four vars so rewrites only target true map
access/membership like headers["X"] or "X" in params.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6507a8b3-3bed-49fb-945e-e6aa4001d90b

📥 Commits

Reviewing files that changed from the base of the PR and between 0fb1cb5 and 2acff08.

📒 Files selected for processing (4)
  • framework/routing/routing.go
  • plugins/governance/routing.go
  • plugins/governance/routing_test.go
  • plugins/governance/store.go
💤 Files with no reviewable changes (1)
  • plugins/governance/routing.go
✅ Files skipped from review due to trivial changes (2)
  • plugins/governance/routing_test.go
  • plugins/governance/store.go

coderabbitai[bot]
coderabbitai Bot previously approved these changes Apr 13, 2026
Comment thread plugins/governance/store.go
@roroghost17 roroghost17 force-pushed the 03-28-feat_prompt-deployment-strategies-fallback-screens branch from 1a73c1b to 9f01c3f Compare April 14, 2026 10:17
@roroghost17 roroghost17 force-pushed the 04-07-refactor_extracts_routing_utilites_to_a_separate_package branch from 2acff08 to 3562875 Compare April 14, 2026 10:17
@roroghost17 roroghost17 force-pushed the 04-07-refactor_extracts_routing_utilites_to_a_separate_package branch from 3562875 to d54ccca Compare April 15, 2026 20:19
@roroghost17 roroghost17 force-pushed the 03-28-feat_prompt-deployment-strategies-fallback-screens branch 2 times, most recently from 2c77290 to 684cf74 Compare April 16, 2026 08:05
@roroghost17 roroghost17 force-pushed the 04-07-refactor_extracts_routing_utilites_to_a_separate_package branch from d54ccca to dc92eb1 Compare April 16, 2026 08:05
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
framework/routing/routing.go (1)

10-20: ⚠️ Potential issue | 🟡 Minor

Add identifier boundaries to headers/params regex targets.

These patterns can still match substrings inside larger identifiers and rewrite unintended text. Constrain headers and params to identifier boundaries.

Suggested patch
-var headerKeyPattern = regexp.MustCompile(`headers\[["']([^"']+)["']\]`)
+var headerKeyPattern = regexp.MustCompile(`\bheaders\[["']([^"']+)["']\]`)

-var headerInPattern = regexp.MustCompile(`["']([^"']+)["']\s+in\s+headers`)
+var headerInPattern = regexp.MustCompile(`["']([^"']+)["']\s+in\s+\bheaders\b`)

-var paramKeyPattern = regexp.MustCompile(`params\[["']([^"']+)["']\]`)
+var paramKeyPattern = regexp.MustCompile(`\bparams\[["']([^"']+)["']\]`)

-var paramInPattern = regexp.MustCompile(`["']([^"']+)["']\s+in\s+params`)
+var paramInPattern = regexp.MustCompile(`["']([^"']+)["']\s+in\s+\bparams\b`)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@framework/routing/routing.go` around lines 10 - 20, The regexes
headerKeyPattern, headerInPattern, paramKeyPattern, and paramInPattern are
currently matching "headers" and "params" inside larger identifiers; update each
pattern to require identifier boundaries around the target words (e.g., use
word-boundaries like \b around headers and params or equivalent
negative-lookbehind/lookahead) so that matches only occur on the standalone
identifiers (for example change occurrences of `headers\[` to `\bheaders\[` and
`in\s+headers` to `\bin\s+headers\b`, and similarly for `params`) while
preserving the existing capture groups for the key names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@framework/routing/routing.go`:
- Around line 10-20: The regexes headerKeyPattern, headerInPattern,
paramKeyPattern, and paramInPattern are currently matching "headers" and
"params" inside larger identifiers; update each pattern to require identifier
boundaries around the target words (e.g., use word-boundaries like \b around
headers and params or equivalent negative-lookbehind/lookahead) so that matches
only occur on the standalone identifiers (for example change occurrences of
`headers\[` to `\bheaders\[` and `in\s+headers` to `\bin\s+headers\b`, and
similarly for `params`) while preserving the existing capture groups for the key
names.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f44bba35-6e89-4aea-bc6e-707f3b104157

📥 Commits

Reviewing files that changed from the base of the PR and between d54ccca and dc92eb1.

📒 Files selected for processing (4)
  • framework/routing/routing.go
  • plugins/governance/routing.go
  • plugins/governance/routing_test.go
  • plugins/governance/store.go
💤 Files with no reviewable changes (1)
  • plugins/governance/routing.go
✅ Files skipped from review due to trivial changes (1)
  • plugins/governance/store.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • plugins/governance/routing_test.go

@roroghost17 roroghost17 force-pushed the 04-07-refactor_extracts_routing_utilites_to_a_separate_package branch from dc92eb1 to 6328594 Compare April 16, 2026 14:50
@roroghost17 roroghost17 force-pushed the 03-28-feat_prompt-deployment-strategies-fallback-screens branch from 684cf74 to 5465c4e Compare April 16, 2026 14:50
@roroghost17 roroghost17 force-pushed the 04-07-refactor_extracts_routing_utilites_to_a_separate_package branch from 6328594 to a9ceab4 Compare April 16, 2026 15:52
@roroghost17 roroghost17 force-pushed the 03-28-feat_prompt-deployment-strategies-fallback-screens branch from 5465c4e to 3df2fd7 Compare April 16, 2026 15:52
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
plugins/governance/store.go (1)

3449-3459: ⚠️ Potential issue | 🟡 Minor

Trim before empty-defaulting to keep validation and compile paths consistent.

routing.ValidateCELExpression trims internally, but Line [3450]-Line [3452] only treats exact "" as empty. Trimming once up front keeps whitespace-only expressions aligned with the same default "true" path.

Suggested patch
-	expr := rule.CelExpression
+	expr := strings.TrimSpace(rule.CelExpression)
 	if expr == "" {
 		expr = "true"
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@plugins/governance/store.go` around lines 3449 - 3459, Trim
rule.CelExpression into expr before checking for empty and before further
normalization/validation so whitespace-only expressions get the same default
"true" path; i.e., call strings.TrimSpace (or equivalent) on expr prior to the
empty check, then pass the trimmed expr into routing.NormalizeMapKeysInCEL and
routing.ValidateCELExpression to keep compile/validation behavior consistent for
rule.CelExpression.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@plugins/governance/store.go`:
- Around line 3449-3459: Trim rule.CelExpression into expr before checking for
empty and before further normalization/validation so whitespace-only expressions
get the same default "true" path; i.e., call strings.TrimSpace (or equivalent)
on expr prior to the empty check, then pass the trimmed expr into
routing.NormalizeMapKeysInCEL and routing.ValidateCELExpression to keep
compile/validation behavior consistent for rule.CelExpression.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 814b03e3-d7af-4b70-9b4a-7845b4b3c3a5

📥 Commits

Reviewing files that changed from the base of the PR and between 6328594 and a9ceab4.

📒 Files selected for processing (4)
  • framework/routing/routing.go
  • plugins/governance/routing.go
  • plugins/governance/routing_test.go
  • plugins/governance/store.go
💤 Files with no reviewable changes (1)
  • plugins/governance/routing.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • plugins/governance/routing_test.go

Copy link
Copy Markdown
Contributor

akshaydeo commented Apr 16, 2026

Merge activity

  • Apr 16, 4:34 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Apr 16, 4:39 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo akshaydeo changed the base branch from 03-28-feat_prompt-deployment-strategies-fallback-screens to graphite-base/2544 April 16, 2026 16:38
@akshaydeo akshaydeo changed the base branch from graphite-base/2544 to v1.5.0 April 16, 2026 16:38
@akshaydeo akshaydeo dismissed stale reviews from Pratham-Mishra04 and coderabbitai[bot] April 16, 2026 16:38

The base branch was changed.

@akshaydeo akshaydeo merged commit 5baca4b into v1.5.0 Apr 16, 2026
5 of 13 checks passed
@akshaydeo akshaydeo deleted the 04-07-refactor_extracts_routing_utilites_to_a_separate_package branch April 16, 2026 16:39
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.

5 participants