feat(routing): add routing profiles dashboard editor - #1108
Merged
Conversation
Contributor
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change adds an editable Routing Profile Editor, shared draft and DTO utilities, management API create/update/delete operations, validation and persistence tests, localization, and user documentation. ChangesRouting Profile Editor
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant RoutingProfiles
participant RoutingProfileRoutes
participant Configuration
participant CodexCatalog
User->>RoutingProfiles: Edit and save routing profile
RoutingProfiles->>RoutingProfileRoutes: PUT serialized profile
RoutingProfileRoutes->>Configuration: Persist normalized profile
RoutingProfileRoutes->>CodexCatalog: Reconcile and refresh catalog
RoutingProfileRoutes-->>RoutingProfiles: Return saved profile
RoutingProfiles-->>User: Show updated profile and status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
…rving wrapper The PUT and DELETE routing-profile handlers assigned the guarded saver to a local named \saveConfig\, which trips the config-save-boundary guard that forbids bare saveConfig calls from live-config management writers. Alias it to \save\ like the other management routes so test 4/4 goes green.
…ent specs
- Replace the inline min/max/step ternary chain in the numeric requirement
inputs with a per-key spec table (single source for constraints).
- Merge the status/statusOk state pair into one {message, ok} | null state,
collapsing the notify helper and the repeated reset pairs.
Behavior-preserving: same input attributes emitted, same notice behavior
(errors persist, success auto-clears after 5s).
The numeric requirement spec object has a union shape ({min, step} for
minContextWindow vs {min, max, step} for minQuotaHeadroom), so indexing
[NUMERIC_REQUIREMENT_SPEC[key].max] fails the GUI typecheck. Add
max: undefined to the minContextWindow entry so both members share the
same shape; behavior is unchanged (max={undefined} renders no max
attribute).
Collaborator
Author
[GD] Verdict: gatedTLDR
Full verdictSemantic propagation
Linked: none. UsefulnessDelivers the claimed value: previously Bugs / correctness
Security
Spec / standards
Reviews
Base / CI
Simplification
GateDraft — Bottom lineThe routing profiles dashboard editor is a useful, well-tested feature. The PR-introduced |
Wibias
marked this pull request as ready for review
August 6, 2026 09:42
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@gui/src/pages/RoutingProfiles.tsx`:
- Around line 403-405: The create-profile button in RoutingProfiles should use a
dedicated routing.createProfile i18n key instead of routing.detail; add the key
to every locale under gui/src/i18n with the appropriate translated label. Update
docs-site/src/content/docs/guides/routing-profile-editor.md at line 11 and the
ja, ko, ru, and zh-cn localized pages to document the new label.
In `@gui/src/routing-profile-editor-data.ts`:
- Around line 167-205: Update routingProfilePutBody to accept or derive the
required write mode and include mode ("create" or "update") alongside id and
profile in its returned body. Update all callers and routing-profile-editor-data
tests to provide the mode and assert the complete serialized request contract.
In `@gui/tests/routing-profiles.test.tsx`:
- Around line 145-149: Update requirementSelect to first locate the fieldset
whose legend matches the rendered translation for t("routing.require"), then
search only that fieldset’s labels for the requested key. Preserve the existing
null behavior when the requirements fieldset or matching select is absent.
In `@tests/routing-profile-management-editor.test.ts`:
- Around line 148-171: Add a focused test beside “PUT create refuses to
overwrite an existing profile” that sends a PUT request for existing profile
“fast” with mode “update” and replacement candidates. Assert the response
succeeds, its payload contains the normalized updated profile,
config.routingProfiles.fast is replaced accordingly, and the dependency counters
confirm exactly one save and one catalog refresh.
🪄 Autofix
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 Plus
Run ID: d7ed67d8-617b-4d0d-ad78-46c4de820ae9
📒 Files selected for processing (8)
docs-site/src/content/docs/guides/routing-profile-editor.mdgui/src/pages/RoutingProfiles.tsxgui/src/routing-profile-editor-data.tsgui/tests/routing-profiles.test.tsxsrc/server/management/routing-profile-routes.tstests/routing-intelligence-ui.test.tstests/routing-profile-editor-data.test.tstests/routing-profile-management-editor.test.ts
Ingwannu
previously requested changes
Aug 6, 2026
Ingwannu
left a comment
Ingwannu
left a comment
Owner
There was a problem hiding this comment.
The runtime/API feature is useful and the core CI is green, but the current head is not ready for approval yet.
I checked the latest f67753e0 increment after the self-verdict. The live saveProfile caller does append mode, so the current GUI request is not missing it at runtime. However, the newest CodeRabbit review arrived after that verdict and has four still-actionable items that should be resolved on this head:
- The create action is rendered with
routing.detail; use a dedicated localized create-profile label in every locale and keep the guide aligned. - Make
routingProfilePutBodyown the full server request contract, includingmode, instead of relying on a caller-side spread that its unit tests cannot verify. - Scope the requirement-select test helper to the requirements fieldset so duplicate labels elsewhere cannot produce a false pass.
- Add the missing successful
mode: "update"management regression proving replacement plus exactly one save and one catalog refresh.
These are small but relevant: the first is visible behavior, the second prevents the editor helper and server contract from drifting, and the fourth covers the only mutation mode not presently proven end to end. The repeated react-doctor failure also remains red; even if it is diff-derivation infrastructure rather than code, required checks must be green or formally waived by the owner after inspecting the job.
Please address or rebut each current finding with exact-head evidence, rerun React Doctor, and then request owner review for the GUI/text surface. I am not merging GUI/text work autonomously.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f67753e047
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Three findings from react-doctor 0.9.3: - saveProfile/removeProfile read the fetch Response body before checking response.ok, so an HTTP error payload was parsed as success. Route both through readJsonIfOk (which checks res.ok before consuming) and read the structured error body explicitly on the non-OK branch. - the candidate card used an index-derived React key. Draft candidates now carry a stable client-side key (newDraftCandidate) that is stripped by routingProfilePutBody and never reaches the server; add/update keep it. react-doctor --scope changed --base upstream/dev: No issues found. Verified: root+GUI typecheck, GUI lint, 610 GUI tests, 128 focused server tests.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
gui/src/pages/RoutingProfiles.tsx (2)
387-390: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMove these user-visible strings into locale resources.
Both sites render untranslated text in the Routing dashboard.
gui/src/pages/RoutingProfiles.tsx#L387-L390: Replace thedry-run ${response.status}fallback with a localized status-aware message.gui/src/pages/RoutingProfiles.tsx#L633-L648: Map eachUnknownEvidenceModevalue to a localized option label while preserving the enum value.As per path instructions, user-visible strings go through the i18n locale files rather than hardcoded text.
🤖 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 `@gui/src/pages/RoutingProfiles.tsx` around lines 387 - 390, The dry-run error fallback in RoutingProfiles.tsx:387-390 must use a localized status-aware message instead of the hardcoded `dry-run ${response.status}` string; update the relevant i18n locale resources and consume them through the page’s existing translation mechanism. At RoutingProfiles.tsx:633-648, map every UnknownEvidenceMode value to a localized option label while preserving the original enum value for selection and submission.Source: Path instructions
81-93: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate all nested profile fields before accepting a profile.
parseProfilesaccepts a profile withoutrequire,optimize,limits, orunknownEvidence.loadthen passes it toroutingProfileDraftFromDtoat Line 220. That conversion dereferences nested fields such asprofile.require.minContextWindow, so a malformed management response fails the load path.Validate the complete DTO shape before returning it from
parseProfiles. Reject invalid profiles or show a controlled load error. Add a regression test for a response that omits a required nested object.🤖 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 `@gui/src/pages/RoutingProfiles.tsx` around lines 81 - 93, Update parseProfiles to validate every required nested RoutingProfileDto field, including require, optimize, limits, and unknownEvidence, before accepting a profile; reject profiles with missing or malformed nested objects so routingProfileDraftFromDto cannot dereference invalid data. Add a regression test covering a management response that omits a required nested object and verify loading fails in the existing controlled manner.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@gui/src/pages/RoutingProfiles.tsx`:
- Around line 387-390: The dry-run error fallback in RoutingProfiles.tsx:387-390
must use a localized status-aware message instead of the hardcoded `dry-run
${response.status}` string; update the relevant i18n locale resources and
consume them through the page’s existing translation mechanism. At
RoutingProfiles.tsx:633-648, map every UnknownEvidenceMode value to a localized
option label while preserving the original enum value for selection and
submission.
- Around line 81-93: Update parseProfiles to validate every required nested
RoutingProfileDto field, including require, optimize, limits, and
unknownEvidence, before accepting a profile; reject profiles with missing or
malformed nested objects so routingProfileDraftFromDto cannot dereference
invalid data. Add a regression test covering a management response that omits a
required nested object and verify loading fails in the existing controlled
manner.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 65c854bd-e39a-4e3c-b684-20084df9f215
📒 Files selected for processing (3)
gui/src/pages/RoutingProfiles.tsxgui/src/routing-profile-editor-data.tstests/routing-profile-editor-data.test.ts
CodeRabbit, Codex connector, and owner review items on #1108: - routing.createProfile key replaces the reused routing.detail on the create button; guide synced; localized in all six locales. - routingProfilePutBody now owns the serialized request contract: mode is part of the returned body (not a caller-side spread), and updates carry expectedRevision for optimistic concurrency. - requirementSelect test helper scopes to the Hard requirements fieldset so health/cost/quota keys in other fieldsets cannot false-match. - management regression test covers the mode:update success path (one save, one catalog refresh) plus a 409 stale-revision conflict. - dry-run error fallback and UnknownEvidenceMode option labels are localized. - parseProfiles validates the complete nested DTO shape so a malformed management response cannot crash the load path; GUI regression test added. - PUT update migrates config references on alias change (disabledModels, subagentModels, injectionModel, shadowCallIntercept, claudeCode) and syncs Claude agent defs, mirroring the /api/combos pattern. - delete confirmation and candidate remove aria-label are localized. react-doctor 0.9.3 --scope changed --base upstream/dev: No issues found. Verified: root+GUI typecheck, GUI lint+i18n lint, 611 GUI tests, 137 focused server tests.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/server/management/routing-profile-routes.ts`:
- Around line 159-213: Update migrateProfileModelReferences to migrate
config.subagentModelFallback entries using the same deduplication logic as
subagentModels, and migrate both keys and values of claudeCode.modelMap so old
aliases remain matched for rerouting. Preserve shouldSyncClaudeAgentDefs
behavior for migrated agent references, and add coverage in
routing-profile-management-editor tests.
🪄 Autofix
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 Plus
Run ID: 869d71c9-3452-4d5e-9f2b-731ebcaba8f3
📒 Files selected for processing (13)
docs-site/src/content/docs/guides/routing-profile-editor.mdgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/pages/RoutingProfiles.tsxgui/src/routing-profile-editor-data.tsgui/tests/routing-profiles.test.tsxsrc/server/management/routing-profile-routes.tstests/routing-profile-editor-data.test.tstests/routing-profile-management-editor.test.ts
…s change CodeRabbit follow-up on cf3d4b5: migrateProfileModelReferences rewrote subagentModels and claudeCode.modelMap values but missed the sibling config.subagentModelFallback chain and modelMap keys, which are the inbound ids matched for reroute in src/claude/inbound.ts. Both now follow an alias rename, with the migration regression test extended to cover them.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/server/management/routing-profile-routes.ts`:
- Around line 211-216: Update the modelMap migration in the surrounding
routing-profile handling to detect duplicate keys produced by
migrateAgentReference before assigning the migrated map. Reject conflicting
mappings (or apply a clearly defined merge policy) instead of allowing
Object.fromEntries to overwrite entries, and add a regression test covering both
colliding source keys with different targets.
🪄 Autofix
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 Plus
Run ID: b52977d1-6213-4141-8992-0ff3b6fd74f0
📒 Files selected for processing (2)
src/server/management/routing-profile-routes.tstests/routing-profile-management-editor.test.ts
CodeRabbit follow-up on bcec06d: migrating modelMap keys via Object.fromEntries silently drops a mapping when the map already contains the new alias as a key with a different target. Detect that collision before any mutation and reject the update with 409 alias_reference_conflict so no mapping is lost; regression test covers both keys with different targets.
4 tasks
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
2 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.
Summary
PUTandDELETEmanagement endpoints for routing profiles while preserving the existing list and dry-run APIsVerification
RoutingProfiles.tsxand the editor data module: passedrouting-profile-routes.ts: passeddev(ef131787) before opening this PRChecklist
Summary by CodeRabbit
New Features
Documentation