Semantic routing policy and Intelligent Model Routing #1236
Merged
Semantic routing policy and Intelligent Model Routing #1236
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughTwo new custom policy components (IntelligentModelRouting and SemanticRouting) are introduced to configure semantic and model-based routing rules across production and sandbox environments. The General policy form is updated to render and validate these components. Localization keys are added to support the new UI elements and validation messaging. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Form as General.tsx<br/>(Policy Form)
participant Policy as SemanticRouting /<br/>IntelligentModelRouting
participant API as Backend APIs
participant State as Form State
User->>Form: Select routing policy type
Form->>Policy: Render component with props
Note over Policy: Initialize from manualPolicyConfig
Policy->>API: Fetch model list & endpoints
API-->>Policy: Return models and endpoints
Policy->>State: Parse & hydrate policy config
User->>Policy: Configure routing rules, models, paths
Policy->>Policy: Validate form (contentPath, rules, models)
Policy->>State: Update isManualFormValid
User->>Form: Click Save
Form->>Form: Check isManualFormValid
alt Form Valid
Form->>Policy: Serialize config to string
Policy-->>Form: Return serialized config
Form->>State: Call setManualPolicyConfig
Form->>API: Submit policy with config
API-->>Form: Success
else Form Invalid
Form->>Form: Show validation errors
Form->>User: Display error messages
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Contributor
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
portals/publisher/src/main/webapp/site/public/locales/en.json (1)
1389-1442: Tighten consistency for new routing policy strings (casing + pluralization).A few values look inconsistent (e.g., “Add Rule” vs “Add rule”, “Routing Rules” vs “Routing rule”). If these are user-facing labels/buttons, aligning casing/pluralization will make the two new policy UIs feel cohesive.
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx (1)
136-148: ResetisManual/providerNotConfiguredwhenpolicyObjchanges (current code can get “stuck”).Right now you only ever call
setManual(true); if the same drawer instance is reused for a different policy,isManualmay remain true and/orproviderNotConfiguredmay remain true, breaking subsequent policy editing.Proposed fix
useEffect(() => { - if ( - (policyObj && policyObj.name === 'modelRoundRobin') || - (policyObj && policyObj.name === 'modelWeightedRoundRobin') || - (policyObj && policyObj.name === 'modelFailover') || - (policyObj && policyObj.name === 'semanticRouting') || - (policyObj && policyObj.name === 'intelligentModelRouting') - ) { - setManual(true); - } + const manualPolicies = new Set([ + 'modelRoundRobin', + 'modelWeightedRoundRobin', + 'modelFailover', + 'semanticRouting', + 'intelligentModelRouting', + ]); + setManual(manualPolicies.has(policyObj?.name ?? '')); + // Avoid “sticky” disable if user switches away from routing policies. + setProviderNotConfigured(false); }, [policyObj]);
🤖 Fix all issues with AI agents
In
@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx:
- Around line 492-505: The save-button disabled logic ignores the global saving
state for manual policies and allows empty manual configs; update the disabled
expression so saving always disables the button (include saving independent of
isManual) and add a validation check that manualPolicyConfig is non-empty when
isManual (use the same manualPolicyConfig value passed to
SemanticRouting/IntelligentModelRouting, e.g.,
getValue(policySpec.policyAttributes[0]) or the manualPolicyConfig state), while
preserving the existing providerNotConfigured and the non-manual validation
branches; adjust the disabled prop wherever used (refer to
providerNotConfigured, isManual, saving, manualPolicyConfig) including the other
occurrence mentioned.
- Around line 258-261: The code assumes policySpec.policyAttributes[0] exists
which can throw if the array is empty or reordered; before assigning
updateCandidates[policySpec.policyAttributes[0].name] = manualPolicyConfig,
guard access by verifying policySpec.policyAttributes is defined and non-empty
and ideally locate the correct attribute by name (e.g., const attr =
policySpec.policyAttributes.find(a => a.name === expectedName) or fall back to
the first element), then use attr.name when setting updateCandidates; ensure
this check is applied in the block that handles policyObj values
(modelRoundRobin, modelWeightedRoundRobin, modelFailover, semanticRouting,
intelligentModelRouting) so you don’t dereference an undefined element.
In
@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx:
- Around line 207-219: fetchModelList currently calls
JSON.parse(apiFromContext.subtypeConfiguration.configuration) without validation
which will throw if subtypeConfiguration or configuration is missing or invalid;
wrap the parse in a try/catch and/or use a safe check (e.g., optional chaining
to verify apiFromContext?.subtypeConfiguration?.configuration exists) before
calling JSON.parse, validate that the parsed object contains llmProviderId, and
only then call API.getLLMProviderModelList; on parse or validation failure, log
the error via console.error or process logger and return early, and keep the
existing .catch for the API call to handle network/response errors before
calling setModelList.
- Around line 269-279: The current useEffect builds configForBackend and then
corrupts valid JSON by doing JSON.stringify(...) followed by
jsonString.replace(/"/g, "'"); remove that replace and preserve the valid JSON
(i.e., call setManualPolicyConfig(JSON.stringify(configForBackend))); if the
backend truly requires single quotes instead of double quotes, instead encode
the JSON safely (e.g., Base64 encode the JSON string or use encodeURIComponent)
and have the backend decode it; update the useEffect around configForBackend,
JSON.stringify, and setManualPolicyConfig accordingly (or coordinate backend
decoding) rather than blindly replacing quotes.
- Around line 760-768: The production routing rules are being keyed by array
index in the map over config.production.routingrules (and rendered via
renderRuleCard), which breaks reconciliation when items are deleted; update the
RoutingRuleConfig model to include a unique id field (e.g., id: string), ensure
any code that creates/adds rules (the add rule handler) generates a stable
unique id for each new rule, then change the map to use rule.id as the React key
and pass that id as the key prop to the Paper returned by renderRuleCard; also
update handleProductionRuleDelete to locate and remove rules by id rather than
by index to avoid index-shift bugs.
- Around line 221-224: The useEffect that calls fetchModelList and
fetchEndpoints needs cleanup and correct dependencies: add apiFromContext.id and
apiFromContext.subtypeConfiguration to the effect dependency array and ensure
both async functions accept an AbortSignal (or check a mounted flag) so
in-flight requests are aborted on unmount; inside fetchModelList and
fetchEndpoints pass the AbortSignal to fetch/axios calls, catch abort errors,
and guard any setState calls (e.g., setModelList, setEndpoints) so they only run
when the request wasn’t aborted.
- Around line 653-673: The delete button Grid item in
IntelligentModelRouting.tsx is placed after the Grid container is closed so it
isn't a direct child of the Grid container; move the conditional block that
renders the IconButton (the Grid with data-testid='rule-delete' and its
IconButton/DeleteIcon children) so it is inside the surrounding Grid container
(or wrap it with a Grid container) that contains the other Grid items, ensuring
all Grid items are direct children of the same Grid container for correct
layout.
In
@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx:
- Around line 208-220: fetchModelList currently calls
JSON.parse(apiFromContext.subtypeConfiguration.configuration) without guarding
for missing or invalid JSON which can throw and crash the UI; update
fetchModelList to first verify apiFromContext.subtypeConfiguration.configuration
exists, wrap the JSON.parse in a try/catch (or use a safe parse helper) to
handle malformed JSON, only call API.getLLMProviderModelList when a valid
llmProviderId is extracted, and on error setModelList to an empty array or keep
prior state and log the parse/API error so the policy drawer won't crash
(references: fetchModelList, apiFromContext.subtypeConfiguration.configuration,
API.getLLMProviderModelList, setModelList).
- Around line 227-300: The current parsing/serialization in the useEffect
(reading manualPolicyConfig) and the serialization block that builds
configForBackend uses blind string replacements of quotes which corrupts
contentpath and utterances; remove the manual .replace(/\'/g, "'").replace(/'/g,
'"') and the JSON.stringify(...).replace(/"/g, "'") steps and instead keep
manualPolicyConfig as valid JSON (double-quoted) end-to-end or use a proper
serializer/escaper; update the parsing useEffect to
JSON.parse(manualPolicyConfig) (or JSON5.parse if you adopt JSON5) and update
the writer useEffect to setManualPolicyConfig(JSON.stringify(configForBackend))
(or JSON5.stringify) so contentpath and user strings are preserved, and
similarly remove quote normalization from normalizeContentPath and
handleContentPathUpdate; reference functions/vars: manualPolicyConfig,
setManualPolicyConfig, useEffect blocks that parse/serialize configForBackend,
normalizeContentPath, and handleContentPathUpdate.
🧹 Nitpick comments (3)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx (2)
764-789: Switch + Accordion onChange likely double-toggles; stop propagation or pick one control path.Clicking the Switch often triggers Accordion expansion as well; you’re handling both paths, which can cause redundant state updates (and double-resets when turning off).
Also applies to: 849-874
480-483: Consider localizing remaining hard-coded UI strings (loading + helper/placeholder text).You added locale keys for much of the UI, but a few strings are still hard-coded (e.g., loading message, helperText, placeholders). Aligning these with
en.jsonwill keep i18n coverage complete.Also applies to: 501-515, 683-684
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx (1)
200-204: Silent error handling provides no feedback to users.API errors are only logged to console. If endpoint fetching fails, users see an empty list with no explanation or retry option.
♻️ Consider adding error state
+const [fetchError, setFetchError] = useState<string | null>(null); const fetchEndpoints = () => { setLoading(true); + setFetchError(null); const endpointsPromise = API.getApiEndpoints(apiFromContext.id); endpointsPromise .then((response) => { // ... existing logic }).catch((error) => { console.error(error); + setFetchError('Failed to load endpoints. Please try again.'); }).finally(() => { setLoading(false); }); }Then display the error in the UI with a retry button.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
portals/publisher/src/main/webapp/site/public/locales/en.jsonportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsxportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsxportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx (1)
portals/admin/src/main/webapp/source/src/app/components/Base/Navigator.jsx (1)
policyObj(132-132)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx (1)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/Types.d.ts (1)
ModelVendor(35-38)
🔇 Additional comments (2)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx (2)
51-82: LGTM!The interface definitions are well-structured and provide clear type safety for the routing configuration. The separation of
EnvironmentConfig,RoutingRuleConfig,DefaultModelConfig, andContentPathConfigpromotes good modularity.
678-686: Good user feedback for missing LLM provider configuration.The warning alert with documentation link provides helpful guidance when the LLM provider is not configured. The use of
rel='noopener noreferrer'on the external link is a good security practice.
...c/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx
Outdated
Show resolved
Hide resolved
...c/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx
Outdated
Show resolved
Hide resolved
...p/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx
Show resolved
Hide resolved
...p/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx
Show resolved
Hide resolved
...p/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx
Outdated
Show resolved
Hide resolved
...p/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx
Show resolved
Hide resolved
...p/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx
Outdated
Show resolved
Hide resolved
...in/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx
Show resolved
Hide resolved
...in/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx
Show resolved
Hide resolved
...in/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx
Outdated
Show resolved
Hide resolved
|
pamaljayasinghe
commented
Jan 16, 2026
...p/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx
Outdated
Show resolved
Hide resolved
pamaljayasinghe
requested review from
Krishanx92,
lasanthaS,
piyumaldk and
tharikaGitHub
as code owners
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@portals/publisher/src/main/webapp/site/public/locales/en.json`:
- Around line 1428-1429: The string value for the localization key
"Apis.Details.Policies.CustomPolicies.SemanticRouting.add.route" currently reads
"Add rule"; update it to "Add Rule" to match the casing used by the
IntelligentModelRouting UI and ensure consistent routing UI text across the app.
♻️ Duplicate comments (4)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx (2)
231-241: Quote replacement still corrupts strings containing quotes.The previous review flagged this pattern as corrupting user data (e.g., model names, contexts containing quotes). The comment was marked as addressed, but the code still uses
.replace(/"/g, "'")on line 239.If string values contain double quotes, this corrupts the JSON structure. If the backend truly requires single-quoted strings, consider:
- Coordinating with backend to accept standard JSON
- Using Base64 encoding for the payload
- At minimum, only escaping the outer structure while preserving inner string values
188-192: Parsing also uses quote replacement that can fail on edge cases.Line 191 uses
.replace(/'/g, '"')to convert single quotes back to double quotes before parsing. If user-entered values (rule names, contexts) contain apostrophes, the round-trip through.replace(/"/g, "'")(serialization) and.replace(/'/g, '"')(parsing) will corrupt them.Example: A context like
"User's input"becomes'User's input'on serialization, then fails to parse or becomes malformed on the next load.portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx (2)
357-367: Content path normalization corrupts valid JSONPath expressions.The
normalizeContentPathfunction converts single quotes to double quotes, but JSONPath expressions commonly use single quotes for string literals (as shown in the placeholder at line 473:$.messages[?(@.role=='user')].content).Converting
'user'to"user"may cause the JSONPath to fail at runtime depending on the parser implementation.
239-262: Same quote replacement issue as IntelligentModelRouting.The serialization at line 260 uses
.replace(/"/g, "'")which corrupts any user-entered strings containing double quotes (utterances, etc.).
🧹 Nitpick comments (4)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx (2)
243-259: Consider using stable IDs for rules.The ID generation on line 245 uses
Date.now()combined with a random suffix, which provides good uniqueness. However,Math.random().toString(36).substr(2, 9)uses the deprecatedsubstrmethod.♻️ Minor improvement: use substring instead of substr
const handleAddProductionRule = () => { const newRule: RoutingRuleConfig = { - id: `rule-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + id: `rule-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`, name: '', context: '', model: '', endpointId: '', };
283-299: Samesubstrdeprecation applies to sandbox rule ID generation.♻️ Apply the same fix here
const handleAddSandboxRule = () => { const newRule: RoutingRuleConfig = { - id: `rule-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + id: `rule-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`, name: '', context: '', model: '', endpointId: '', };portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx (2)
264-280: Samesubstrdeprecation in route ID generation.♻️ Use substring instead of substr
const handleAddProductionRoute = () => { const newRoute: RoutingConfig = { - id: `route-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + id: `route-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`, model: '', endpointId: '', utterances: [], scorethreshold: '0.8', };
304-320: Samesubstrdeprecation for sandbox route ID.♻️ Use substring instead of substr
const handleAddSandboxRoute = () => { const newRoute: RoutingConfig = { - id: `route-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + id: `route-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`, model: '', endpointId: '', utterances: [], scorethreshold: '0.8', };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
portals/publisher/src/main/webapp/site/public/locales/en.jsonportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsxportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsxportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: pamaljayasinghe
Repo: wso2/apim-apps PR: 1236
File: portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx:257-260
Timestamp: 2026-01-16T10:16:10.352Z
Learning: In `portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx`, the built-in WSO2 LLM routing policies (modelRoundRobin, modelWeightedRoundRobin, modelFailover, semanticRouting, intelligentModelRouting) have guaranteed policy specs that always include at least one attribute in policySpec.policyAttributes, so accessing policySpec.policyAttributes[0] is safe without defensive guards for these specific policies.
Learnt from: pamaljayasinghe
Repo: wso2/apim-apps PR: 1236
File: portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx:221-224
Timestamp: 2026-01-16T07:33:18.136Z
Learning: In LLM policy components (ModelRoundRobin, ModelFailover, ModelWeightedRoundRobin, IntelligentModelRouting, SemanticRouting) in the APIM publisher portal, `useEffect(() => { fetchModelList(); fetchEndpoints(); }, []);` uses an empty dependency array for intentional mount-once fetching behavior. This is an established pattern across all policy components to avoid unnecessary re-fetches.
Learnt from: pamaljayasinghe
Repo: wso2/apim-apps PR: 1236
File: portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx:207-219
Timestamp: 2026-01-16T07:31:21.015Z
Learning: In LLM policy components (ModelRoundRobin, ModelFailover, ModelWeightedRoundRobin, IntelligentModelRouting, SemanticRouting) in the APIM publisher portal, JSON.parse(apiFromContext.subtypeConfiguration.configuration) is used without try-catch because parent components enforce that these components only render when the API has a valid LLM subtype configuration.
📚 Learning: 2026-01-16T10:16:10.352Z
Learnt from: pamaljayasinghe
Repo: wso2/apim-apps PR: 1236
File: portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx:257-260
Timestamp: 2026-01-16T10:16:10.352Z
Learning: In `portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx`, the built-in WSO2 LLM routing policies (modelRoundRobin, modelWeightedRoundRobin, modelFailover, semanticRouting, intelligentModelRouting) have guaranteed policy specs that always include at least one attribute in policySpec.policyAttributes, so accessing policySpec.policyAttributes[0] is safe without defensive guards for these specific policies.
Applied to files:
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsxportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsxportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsxportals/publisher/src/main/webapp/site/public/locales/en.json
📚 Learning: 2026-01-16T07:33:18.136Z
Learnt from: pamaljayasinghe
Repo: wso2/apim-apps PR: 1236
File: portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx:221-224
Timestamp: 2026-01-16T07:33:18.136Z
Learning: In LLM policy components (ModelRoundRobin, ModelFailover, ModelWeightedRoundRobin, IntelligentModelRouting, SemanticRouting) in the APIM publisher portal, `useEffect(() => { fetchModelList(); fetchEndpoints(); }, []);` uses an empty dependency array for intentional mount-once fetching behavior. This is an established pattern across all policy components to avoid unnecessary re-fetches.
Applied to files:
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx
📚 Learning: 2026-01-16T07:31:21.015Z
Learnt from: pamaljayasinghe
Repo: wso2/apim-apps PR: 1236
File: portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx:207-219
Timestamp: 2026-01-16T07:31:21.015Z
Learning: In LLM policy components (ModelRoundRobin, ModelFailover, ModelWeightedRoundRobin, IntelligentModelRouting, SemanticRouting) in the APIM publisher portal, JSON.parse(apiFromContext.subtypeConfiguration.configuration) is used without try-catch because parent components enforce that these components only render when the API has a valid LLM subtype configuration.
Applied to files:
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsxportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsxportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx
📚 Learning: 2026-01-16T07:33:10.845Z
Learnt from: pamaljayasinghe
Repo: wso2/apim-apps PR: 1236
File: portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx:221-224
Timestamp: 2026-01-16T07:33:10.845Z
Learning: In policy components (ModelRoundRobin, ModelFailover, ModelWeightedRoundRobin, IntelligentModelRouting, SemanticRouting) within the APIM publisher portal, use useEffect(() => { fetchModelList(); fetchEndpoints(); }, []) to perform mount-once data fetching. This pattern should be applied across all TSX policy components to avoid unnecessary re-fetches. If a dependency-based re-fetch is needed, specify exact dependencies in the array.
Applied to files:
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsxportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx
🔇 Additional comments (11)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx (3)
100-122: Component structure and state initialization looks good.The state setup with proper TypeScript interfaces, reasonable defaults, and clear separation of concerns (production vs sandbox, endpoints, model list, loading state, toggle flags) is well-organized.
495-644: Rule card rendering is well-structured.The
renderRuleCardfunction properly encapsulates the UI for each routing rule with:
- Model and endpoint selection from fetched data
- Proper form controls with labels and IDs
- Delete button correctly placed inside the Grid container (addressed from past review)
- Clean separation of change handlers per environment
646-828: Main render structure with accordions is well-implemented.The Production and Sandbox accordions with:
- Toggle switches for enabling/disabling
- Proper alerts for missing models/endpoints
- Dynamic rule rendering with unique keys (
rule.id || fallback)- Content path configuration section
The UI structure is clear and follows MUI patterns correctly.
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx (4)
573-583: Score threshold validation is well-implemented.The validation correctly:
- Allows empty string (for clearing)
- Validates numeric range 0-1
- Prevents invalid input from being stored
657-702: Utterance ChipInput implementation looks good.The utterance management with:
- Add handler that trims and validates non-empty values
- Delete handler using index lookup
- Proper ChipInput component integration
- Helpful placeholder and helper text
101-122: Component structure mirrors IntelligentModelRouting consistently.The state setup follows the same established pattern as other LLM policy components, maintaining consistency across the codebase.
729-911: Main render structure is consistent with IntelligentModelRouting.The accordion-based UI for Production/Sandbox environments with proper:
- Toggle switches and state management
- Alert messages for missing models/endpoints
- Dynamic route rendering with unique keys
- Content path configuration
The overall structure maintains consistency with the sibling component.
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx (4)
48-49: New routing policy components imported cleanly.
137-145: Manual-policy detection now covers the new routing types.
257-259: Manual policy save mapping extended appropriately for new policies.
491-502: Semantic & Intelligent routing editors are wired into manual mode as expected.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx (3)
172-176:validateFormmissing fromuseEffectdependency array.
validateFormis re-created on every render and is not in the dep array. Since all values it closes over (config,productionEnabled,sandboxEnabled) are already listed, there is no stale-closure risk in practice, butreact-hooks/exhaustive-depswill flag it. The cleanest fix is to inline the validation logic or wrapvalidateForminuseCallbackwith the same deps.♻️ Option: inline or add to deps
useEffect(() => { if (setIsFormValid) { setIsFormValid(validateForm()); } - }, [config, productionEnabled, sandboxEnabled, setIsFormValid]); + }, [config, productionEnabled, sandboxEnabled, setIsFormValid, validateForm]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx` around lines 172 - 176, The useEffect calls validateForm but does not include validateForm in its dependency array, causing a linter warning; fix by stabilizing validateForm or inlining the logic: either wrap the validateForm function in useCallback with dependencies [config, productionEnabled, sandboxEnabled] (so the function identity is stable) and then include validateForm in the useEffect deps, or remove the helper and inline the validation expression directly inside the useEffect that calls setIsFormValid (keeping deps [config, productionEnabled, sandboxEnabled, setIsFormValid]); reference validateForm, useEffect, setIsFormValid, config, productionEnabled, sandboxEnabled when applying the change.
243-316: BidirectionaluseEffectdependency creates unnecessary extra render cycles.Effect A (line 243, deps
[manualPolicyConfig]) callssetConfig; Effect B (line 293, deps[config, ...]) callssetManualPolicyConfig. Every user action triggers a 2-cycle ping-pong: the user's action updatesconfig→ Effect B writes a new string → the parent re-renders → Effect A parses it back (same values, new object reference) → Effect B fires again with the same string → React bails out via string equality.A
useRefguard prevents the redundant reverse-parse:♻️ Proposed refactor
+ const isParsing = useRef(false); useEffect(() => { if (manualPolicyConfig !== '') { try { + isParsing.current = true; const parsedConfig = JSON.parse(manualPolicyConfig); // ... setConfig call ... } catch (error) { ... } } }, [manualPolicyConfig]); useEffect(() => { + if (isParsing.current) { + isParsing.current = false; + return; + } const configForBackend: any = { ... }; setManualPolicyConfig(JSON.stringify(configForBackend)); }, [config, setManualPolicyConfig]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx` around lines 243 - 316, The two useEffect hooks (one reading manualPolicyConfig to call setConfig, the other serializing config to call setManualPolicyConfig) cause a ping-pong render; add a useRef guard to skip the reverse-parse: create a ref like skipParseRef, in the serialization effect (the one that builds configForBackend and calls setManualPolicyConfig) set skipParseRef.current = true immediately before calling setManualPolicyConfig, and in the parse effect (the one that JSON.parse manualPolicyConfig and calls setConfig) check if skipParseRef.current is true — if so, set it back to false and return early; keep references to manualPolicyConfig, setConfig, config, setManualPolicyConfig and the two useEffect blocks when applying this change.
320-320:substris deprecated; replace withslice.
String.prototype.substr()is deprecated and is defined only in Annex B as normative-optional for non-browser runtimes;String.prototype.substring()orslice()should be used instead.♻️ Proposed fix
- id: `route-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + id: `route-${Date.now()}-${Math.random().toString(36).slice(2, 11)}`,The same pattern appears at line 360 in
handleAddSandboxRoute.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx` at line 320, Replace uses of the deprecated String.prototype.substr in the id-generation expressions inside SemanticRouting (both in the handler that creates routes and in handleAddSandboxRoute): change the substr(2, 9) call on Math.random().toString(36) to use slice instead by computing slice(start, start+length) (i.e., start=2 and length=9 → slice(2, 11)), updating both occurrences so the generated id remains unchanged but avoids the deprecated API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx`:
- Around line 527-531: The TextField for contentpath in the SemanticRouting
component is marked required but the payload only includes contentpath when
non-empty (see config.contentpath check), so remove the misleading required
attribute on that TextField (or make it conditional) to avoid browser
validation; locate the TextField usage in SemanticRouting.tsx (the element
rendering the "contentpath" input) and delete the required prop (or set
required={!!someCondition} if you intend it to be required only in certain
modes).
- Around line 293-316: The effect building configForBackend should strip the
UI-only id from each route before sending to backend: when constructing
production and sandbox routes (config.production.routes and
config.sandbox.routes) map over each route and create a new object omitting the
id property (e.g., using object rest to exclude id or explicitly pick allowed
fields) so configForBackend.production.routes and
configForBackend.sandbox.routes contain route objects without the
client-generated id; keep the rest of the logic (contentpath handling,
JSON.stringify and quote-replacement) and then call setManualPolicyConfig with
the formatted string.
---
Duplicate comments:
In `@portals/publisher/src/main/webapp/site/public/locales/en.json`:
- Around line 1427-1429: The localization key
Apis.Details.Policies.CustomPolicies.SemanticRouting.add.route currently uses
"Add rule" and should be normalized to title case to match other routing UIs;
update the value for
Apis.Details.Policies.CustomPolicies.SemanticRouting.add.route from "Add rule"
to "Add Rule" in en.json so it matches IntelligentModelRouting and other
entries.
In
`@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx`:
- Around line 825-832: The component SemanticRouting shows the "No models
available" alert while fetchModelList is in-flight because fetchModelList never
toggles a loading flag; add a boolean state (e.g., modelsLoading) in the
component, set modelsLoading = true before calling fetchModelList and set it
false in finally (or both success/error branches), and update the render checks
that currently use modelList.length === 0 (the alert locations in the component)
to only show the warning when !modelsLoading && modelList.length === 0;
reference fetchModelList, modelList, and the SemanticRouting render where the
Alert is returned and apply the same guard to both alert occurrences.
- Line 246: Stop the quote-replacement round-trip: remove the manual replacement
of single quotes to double quotes in the JSON.parse call for manualPolicyConfig
(the line using JSON.parse(manualPolicyConfig.replace(/'/g, '"'))) and instead
parse manualPolicyConfig as standard JSON; likewise remove any serialization
code that converts double quotes to single quotes when writing
manualPolicyConfig (the serialize block mentioned around lines 314–315) so the
config remains valid JSON end-to-end. Also update normalizeContentPath to avoid
converting single quotes inside JSONPath expressions (do not replace "'" → '"'
within JSONPath handling), and ensure any error handling around JSON.parse fails
gracefully without resetting the entire form.
- Around line 126-169: The validateForm function currently omits checking
route.scorethreshold so a route with an empty threshold can pass; update
validateForm (inside the production loop over config.production.routes and the
sandbox loop over config.sandbox.routes) to also require a non-empty
scorethreshold (same validation logic as used by handleThresholdChange where ''
is considered invalid) for each route before returning true so both production
and sandbox routes fail validation when scorethreshold is empty.
---
Nitpick comments:
In
`@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx`:
- Around line 172-176: The useEffect calls validateForm but does not include
validateForm in its dependency array, causing a linter warning; fix by
stabilizing validateForm or inlining the logic: either wrap the validateForm
function in useCallback with dependencies [config, productionEnabled,
sandboxEnabled] (so the function identity is stable) and then include
validateForm in the useEffect deps, or remove the helper and inline the
validation expression directly inside the useEffect that calls setIsFormValid
(keeping deps [config, productionEnabled, sandboxEnabled, setIsFormValid]);
reference validateForm, useEffect, setIsFormValid, config, productionEnabled,
sandboxEnabled when applying the change.
- Around line 243-316: The two useEffect hooks (one reading manualPolicyConfig
to call setConfig, the other serializing config to call setManualPolicyConfig)
cause a ping-pong render; add a useRef guard to skip the reverse-parse: create a
ref like skipParseRef, in the serialization effect (the one that builds
configForBackend and calls setManualPolicyConfig) set skipParseRef.current =
true immediately before calling setManualPolicyConfig, and in the parse effect
(the one that JSON.parse manualPolicyConfig and calls setConfig) check if
skipParseRef.current is true — if so, set it back to false and return early;
keep references to manualPolicyConfig, setConfig, config, setManualPolicyConfig
and the two useEffect blocks when applying this change.
- Line 320: Replace uses of the deprecated String.prototype.substr in the
id-generation expressions inside SemanticRouting (both in the handler that
creates routes and in handleAddSandboxRoute): change the substr(2, 9) call on
Math.random().toString(36) to use slice instead by computing slice(start,
start+length) (i.e., start=2 and length=9 → slice(2, 11)), updating both
occurrences so the generated id remains unchanged but avoids the deprecated API.
...in/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx
Outdated
Show resolved
Hide resolved
...in/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx
Show resolved
Hide resolved
pamaljayasinghe
force-pushed
the
semantic_routing
branch
from
670824e to
656da92
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx (1)
308-311:⚠️ Potential issue | 🟠 MajorStrip UI-only route
idbefore serializing config for backend.Line 310 serializes
configdirectly, so client-generatedidis sent inproduction.routesandsandbox.routes. This should be removed from backend payloads.💡 Proposed fix
useEffect(() => { isInternalUpdate.current = true; - setManualPolicyConfig(JSON.stringify(config).replace(/'/g, '\\u0027').replace(/"/g, "'")); + const configForBackend = { + ...config, + production: { + ...config.production, + routes: config.production.routes.map(({ id, ...rest }) => rest), + }, + sandbox: { + ...config.sandbox, + routes: config.sandbox.routes.map(({ id, ...rest }) => rest), + }, + }; + setManualPolicyConfig(JSON.stringify(configForBackend).replace(/'/g, '\\u0027').replace(/"/g, "'")); }, [config, setManualPolicyConfig]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx` around lines 308 - 311, The current useEffect in SemanticRouting uses JSON.stringify(config) which includes client-only route.id values; before calling setManualPolicyConfig, create a sanitized copy of config (e.g., clone config and for each route in production?.routes and sandbox?.routes remove the id property) so the UI-only id is stripped, then JSON.stringify the sanitizedConfig and perform the same character replacements and call setManualPolicyConfig; keep the existing isInternalUpdate.current assignment and dependency list (config, setManualPolicyConfig) and reference the same symbols (useEffect, isInternalUpdate, setManualPolicyConfig, config, production.routes, sandbox.routes) when implementing the sanitation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx`:
- Around line 139-153: The effect in General's useEffect only sets manual mode
true for certain policyObj names but never resets it; update the effect that
watches policyObj to explicitly clear manual state and validation when the
incoming policyObj is not one of the manual types: inside the same useEffect
(referencing policyObj, setManual, and setIsManualFormValid) add an else branch
that calls setManual(false) and resets validation (e.g.,
setIsManualFormValid(true) or appropriate default) so stale
isManual/isManualFormValid values are cleared whenever policyObj changes.
In
`@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx`:
- Around line 153-170: The validation currently only checks falsy values for
fields like rule.name and rule.context so whitespace-only strings pass; update
the checks in the loops over config.production.routingrules and
config.sandbox.routingrules to verify trimmed content (e.g., ensure rule.name
and rule.context exist and rule.name.trim().length > 0 and
rule.context.trim().length > 0) and likewise validate sandbox.defaultModel
fields if they can be whitespace; change the if conditions that use !rule.name /
!rule.context to trimmed-empty checks to prevent saving whitespace-only values.
- Around line 305-308: In useEffect where isInternalUpdate.current is set and
setManualPolicyConfig is called, strip UI-only RoutingRuleConfig.id values from
the config before serializing: create a copy of config (referenced as config)
and for each routing rule remove the id property, then JSON.stringify that
cleaned payload (instead of the raw config) and pass it to
setManualPolicyConfig; keep the existing escape replacements and preserve
isInternalUpdate.current behavior.
In
`@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx`:
- Around line 148-176: The current validateForm logic (used when
productionEnabled/sandboxEnabled checks run) misses validating each route's
score threshold; update the validation in the production and sandbox blocks (the
loops over config.production.routes and config.sandbox.routes inside
validateForm) to require route.scoreThreshold to be non-empty and within the
allowed bounds (e.g., between the configured min and max or a sensible range
such as 0 and 1); if route.scoreThreshold is missing or out of range, return
false so the form cannot be saved.
---
Duplicate comments:
In
`@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx`:
- Around line 308-311: The current useEffect in SemanticRouting uses
JSON.stringify(config) which includes client-only route.id values; before
calling setManualPolicyConfig, create a sanitized copy of config (e.g., clone
config and for each route in production?.routes and sandbox?.routes remove the
id property) so the UI-only id is stripped, then JSON.stringify the
sanitizedConfig and perform the same character replacements and call
setManualPolicyConfig; keep the existing isInternalUpdate.current assignment and
dependency list (config, setManualPolicyConfig) and reference the same symbols
(useEffect, isInternalUpdate, setManualPolicyConfig, config, production.routes,
sandbox.routes) when implementing the sanitation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f25f46a-5d2a-4cb4-989e-d5af8ace473e
📒 Files selected for processing (13)
.github/workflows/ui-test.ymlpom.xmlportals/admin/pom.xmlportals/admin/src/main/webapp/site/public/locales/en.jsonportals/admin/src/main/webapp/source/src/app/components/KeyManagers/AddEditKeyManager.jsxportals/devportal/pom.xmlportals/devportal/src/main/webapp/site/public/locales/en.jsonportals/pom.xmlportals/publisher/pom.xmlportals/publisher/src/main/webapp/site/public/locales/en.jsonportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsxportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsxportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx
💤 Files with no reviewable changes (2)
- portals/admin/src/main/webapp/source/src/app/components/KeyManagers/AddEditKeyManager.jsx
- portals/admin/src/main/webapp/site/public/locales/en.json
✅ Files skipped from review due to trivial changes (1)
- portals/publisher/pom.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- portals/publisher/src/main/webapp/site/public/locales/en.json
...c/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx
Show resolved
Hide resolved
...p/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx
Show resolved
Hide resolved
...p/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx
Outdated
Show resolved
Hide resolved
...in/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx
Show resolved
Hide resolved
pamaljayasinghe
force-pushed
the
semantic_routing
branch
from
656da92 to
9377340
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (3)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx (2)
285-287:⚠️ Potential issue | 🟠 MajorUI-only
idfield is serialized to backend.The
configobject includesRoutingRuleConfig.idfields (used for React keys) which are UI-only. These should be stripped before serialization to avoid sending unnecessary data to the backend.💡 Proposed fix
useEffect(() => { - setManualPolicyConfig(JSON.stringify(config).replace(/"/g, '"')); + const configForBackend = { + ...config, + production: { + ...config.production, + routingrules: config.production.routingrules.map(({ id, ...rest }) => rest), + }, + sandbox: { + ...config.sandbox, + routingrules: config.sandbox.routingrules.map(({ id, ...rest }) => rest), + }, + }; + setManualPolicyConfig(JSON.stringify(configForBackend).replace(/"/g, '"')); }, [config]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx` around lines 285 - 287, The current useEffect serializes the whole config (in useEffect -> setManualPolicyConfig) including UI-only RoutingRuleConfig.id fields used as React keys; remove/strip any id properties from each routing rule before calling setManualPolicyConfig so the serialized JSON sent to backend omits those UI-only ids (operate on the config copy or map config.routingRules to omit 'id' prior to JSON.stringify).
142-159:⚠️ Potential issue | 🟡 MinorWhitespace-only
name/contextvalues pass validation.Lines 143 and 157 use falsy checks (
!rule.name,!rule.context) which allow whitespace-only strings like" "to pass validation. Consider using.trim()for consistency with the contentPath validation at line 131.💡 Proposed fix
for (const rule of config.production.routingrules) { - if (!rule.model || !rule.endpointId || !rule.name || !rule.context) { + if (!rule.model || !rule.endpointId || !rule.name.trim() || !rule.context.trim()) { return false; } } // ... similar change for sandbox at line 157 for (const rule of config.sandbox.routingrules) { - if (!rule.model || !rule.endpointId || !rule.name || !rule.context) { + if (!rule.model || !rule.endpointId || !rule.name.trim() || !rule.context.trim()) { return false; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx` around lines 142 - 159, The validation currently uses falsy checks for rule.name and rule.context in the production and sandbox loops (inside the handling of config.production.routingrules and config.sandbox.routingrules), which allows whitespace-only strings to pass; update those checks to use .trim() (e.g., validate rule.name.trim() and rule.context.trim()) so that empty or whitespace-only values are rejected; apply the same trimmed validation logic where other string fields are validated (consistent with contentPath at line 131) for both the production and sandbox branches and return false when trimmed values are empty.portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx (1)
140-155:⚠️ Potential issue | 🟠 MajorManual-mode flags not reset when
policyObjchanges to non-manual policy.The effect only enables manual mode when
policyObjmatches certain names but never resetsisManual,isManualFormValid, orshowValidationErrorswhen switching to a non-manual policy. This could leave stale state if the component receives a differentpolicyObj.💡 Proposed fix
useEffect(() => { + const manualPolicies = [ + 'modelRoundRobin', + 'modelWeightedRoundRobin', + 'modelFailover', + 'ContentBasedModelRouter', + 'semanticRouting', + 'intelligentModelRouting', + ]; + const policyName = policyObj?.name ?? ''; + const isManualPolicy = manualPolicies.includes(policyName); + + setManual(isManualPolicy); + setShowValidationErrors(false); + - if ( - (policyObj && policyObj.name === 'modelRoundRobin') || - ... - ) { - setManual(true); - if (policyObj.name === 'semanticRouting' || policyObj.name === 'intelligentModelRouting') { - setIsManualFormValid(false); - } - } + if (isManualPolicy) { + // Initialize form as invalid for policies that require validation + if (policyName === 'semanticRouting' || policyName === 'intelligentModelRouting') { + setIsManualFormValid(false); + } else { + setIsManualFormValid(true); + } + } else { + setIsManualFormValid(true); + } }, [policyObj]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx` around lines 140 - 155, The useEffect watching policyObj only sets manual-mode flags when policyObj matches manual policy names but never resets them when switching to a non-manual policy; update the effect that references policyObj to explicitly set setManual(false), setIsManualFormValid(true) (or appropriate default), and setShowValidationErrors(false) in the else branch so manual-related state is cleared when policyObj.name is not one of 'modelRoundRobin','modelWeightedRoundRobin','modelFailover','ContentBasedModelRouter','semanticRouting','intelligentModelRouting'; ensure you adjust the conditional around setIsManualFormValid to only set false for semanticRouting/intelligentModelRouting and otherwise restore the default in the else path.
🧹 Nitpick comments (1)
portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx (1)
453-459: Consider simplifying accordion toggle handler.Creating a synthetic event object to call the toggle handler works but is indirect. A small refactor could improve clarity.
♻️ Suggested simplification
const handleAccordionChange = (env: 'production' | 'sandbox') => (event: React.SyntheticEvent, expanded: boolean) => { if (env === 'production') { - handleProductionToggle({ target: { checked: expanded } } as React.ChangeEvent<HTMLInputElement>); + setProductionEnabled(expanded); + if (!expanded) { + setConfig(prev => ({ + ...prev, + production: { defaultModel: { ...EMPTY_MODEL }, routingrules: [] }, + })); + } } else { - handleSandboxToggle({ target: { checked: expanded } } as React.ChangeEvent<HTMLInputElement>); + setSandboxEnabled(expanded); + if (!expanded) { + setConfig(prev => ({ + ...prev, + sandbox: { defaultModel: { ...EMPTY_MODEL }, routingrules: [] }, + })); + } } };Alternatively, extract the toggle logic into a shared helper to avoid duplication.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx` around lines 453 - 459, The current handleAccordionChange builds synthetic React events to call handleProductionToggle/handleSandboxToggle, which is indirect; refactor by extracting the toggle behavior into a shared helper (e.g., toggleEnvironment) that accepts (env: 'production'|'sandbox', expanded: boolean) and invokes the appropriate handler logic directly, or change handleProductionToggle/handleSandboxToggle to accept a boolean and call them directly from handleAccordionChange, replacing the synthetic event creation while preserving existing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsx`:
- Around line 140-155: The useEffect watching policyObj only sets manual-mode
flags when policyObj matches manual policy names but never resets them when
switching to a non-manual policy; update the effect that references policyObj to
explicitly set setManual(false), setIsManualFormValid(true) (or appropriate
default), and setShowValidationErrors(false) in the else branch so
manual-related state is cleared when policyObj.name is not one of
'modelRoundRobin','modelWeightedRoundRobin','modelFailover','ContentBasedModelRouter','semanticRouting','intelligentModelRouting';
ensure you adjust the conditional around setIsManualFormValid to only set false
for semanticRouting/intelligentModelRouting and otherwise restore the default in
the else path.
In
`@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx`:
- Around line 285-287: The current useEffect serializes the whole config (in
useEffect -> setManualPolicyConfig) including UI-only RoutingRuleConfig.id
fields used as React keys; remove/strip any id properties from each routing rule
before calling setManualPolicyConfig so the serialized JSON sent to backend
omits those UI-only ids (operate on the config copy or map config.routingRules
to omit 'id' prior to JSON.stringify).
- Around line 142-159: The validation currently uses falsy checks for rule.name
and rule.context in the production and sandbox loops (inside the handling of
config.production.routingrules and config.sandbox.routingrules), which allows
whitespace-only strings to pass; update those checks to use .trim() (e.g.,
validate rule.name.trim() and rule.context.trim()) so that empty or
whitespace-only values are rejected; apply the same trimmed validation logic
where other string fields are validated (consistent with contentPath at line
131) for both the production and sandbox branches and return false when trimmed
values are empty.
---
Nitpick comments:
In
`@portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx`:
- Around line 453-459: The current handleAccordionChange builds synthetic React
events to call handleProductionToggle/handleSandboxToggle, which is indirect;
refactor by extracting the toggle behavior into a shared helper (e.g.,
toggleEnvironment) that accepts (env: 'production'|'sandbox', expanded: boolean)
and invokes the appropriate handler logic directly, or change
handleProductionToggle/handleSandboxToggle to accept a boolean and call them
directly from handleAccordionChange, replacing the synthetic event creation
while preserving existing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d1140cf4-faec-406b-8617-6452a2e1529f
📒 Files selected for processing (4)
portals/publisher/src/main/webapp/site/public/locales/en.jsonportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/AttachedPolicyForm/General.tsxportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsxportals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- portals/publisher/src/main/webapp/source/src/app/components/Apis/Details/Policies/CustomPolicies/SemanticRouting.tsx
- portals/publisher/src/main/webapp/site/public/locales/en.json
...p/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx
Show resolved
Hide resolved
...p/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx
Show resolved
Hide resolved
...p/source/src/app/components/Apis/Details/Policies/CustomPolicies/IntelligentModelRouting.tsx
Show resolved
Hide resolved
Member
|
Let's also try to refactor the 2 files |
tharikaGitHub
previously approved these changes
Mar 10, 2026
tharikaGitHub
previously approved these changes
Mar 13, 2026
pamaljayasinghe
force-pushed
the
semantic_routing
branch
from
70fa4d3 to
432a67f
Compare
|
PasanT9
approved these changes
Mar 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description
Purpose
Add frontend support for Semantic Routing and Intelligent Model Routing policies in the API Publisher portal.
Semantic Routing Policy
Intelligent Model Routing Policy
Changes
SemanticRouting.tsxandIntelligentModelRouting.tsxcustom policy formsGeneral.tsxto integrate new policy forms and validationen.jsonwith new i18n strings