Add Model Router (meta-profiles) settings tab#1395
Conversation
Adds a /settings/meta-llm page for managing meta-profiles — declarative model-routing configs consumed by the agent's classify_and_switch_llm tool. A meta-profile names a classifier_model, a default_model and a list of task classes, each referencing a saved LLM profile. - MetaProfilesService over the new /api/meta-profiles endpoints (driven by the SDK's public HttpClient, mirroring ProfilesClient) - useMetaProfiles query + save/delete/activate mutations - List/create/edit view with profile-name dropdowns sourced from saved LLM profiles, plus a delete confirmation modal - Nav entry, route, and i18n strings (all languages filled per repo policy) - Local-backend only (cloud shows an explanatory message) Requires the meta-profile CRUD API from software-agent-sdk #3744. Co-authored-by: openhands <openhands@all-hands.dev>
Match the LLM profiles page: replace the inline Set active / Edit / Delete buttons with an EllipsisButton trigger + portaled actions menu (MetaProfileActionsMenu), reusing the shared settings-list row styling (MetaProfileRow). Set active is disabled for the already-active profile. Co-authored-by: openhands <openhands@all-hands.dev>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Frontend review fixes for the Model Router settings tab: 1. Create flow could silently overwrite an existing meta-profile (the backend save is create-or-overwrite). MetaProfileEditor now takes `existingNames` and, in create mode, rejects a name that already exists: Save is disabled and an explicit "name already exists" message is shown. Edit mode is unaffected (the current name stays valid). 2. Unsupported-backend boundary is now explicit. Older agent servers (pre software-agent-sdk #3744) lack /api/meta-profiles and return 404; the view detects that (HttpError.status === 404) and shows a clear "backend doesn't support model routing yet" message instead of a generic error, and hides the Add affordance. Non-404 errors still show the generic error with Add available. 3. Deleting the active meta-profile now invalidates the settings caches (SettingsService.invalidateCache + SETTINGS_QUERY_KEYS.personal), mirroring activation — active_meta_profile controls whether classify_and_switch_llm attaches to new conversations, so stale settings could otherwise linger until reload. Tests: - editor: duplicate-name rejection (create) + unique-name acceptance + edit-mode name allowed. - view: 404 -> explicit unsupported message and no Add; 500 -> generic error with Add. - new hook test: delete invalidates meta-profile AND settings caches. i18n: add SETTINGS$META_PROFILE_NAME_TAKEN and SETTINGS$META_PROFILE_UNSUPPORTED across all locales. Co-authored-by: openhands <openhands@all-hands.dev>
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
There was a problem hiding this comment.
Code Review: Add Model Router (meta-profiles) settings tab
🟢 Good taste — Clean, well-structured PR. Good separation of concerns across API service, hooks, and UI components. TanStack Query integration is solid with proper cache invalidation. Test coverage is comprehensive for the main user-facing flows.
Summary
This PR adds a new settings tab for managing meta-profiles — declarative model-routing configurations consumed by the classify_and_switch_llm tool. The implementation includes:
- API layer:
MetaProfilesServicewrapping/api/meta-profilesendpoints - Data layer: TanStack Query hooks with proper cache management
- UI layer: Settings view, profile editor, row components, and actions menu
- Integration: Routes, navigation, and i18n translations
- Tests: Component and hook tests with good coverage
Key Strengths
- Clean API design: The service mirrors existing patterns (
ProfilesClient) and handles the backend API correctly with proper typing - Proper cache invalidation:
useActivateMetaProfileanduseDeleteMetaProfileinvalidate both meta-profiles and settings caches, which is correct since activation/deletion affects conversation behavior - Good error handling: 404 detection for unsupported backends provides a clear user message instead of a generic error
- Testable architecture: Good use of mocks and isolated component testing
- Duplicate name prevention: The
MetaProfileEditorcorrectly blocks creating a profile with an existing name (the backend uses create-or-overwrite semantics)
Minor Suggestions
| File | Line | Suggestion |
|---|---|---|
src/components/features/settings/meta-llm-profiles/meta-profile-actions-menu.tsx |
~1044 | useLayoutEffect is used for scroll/resize listeners. Consider whether useEffect would suffice here — useLayoutEffect is only needed when you need to measure DOM before paint. If position calculation doesn't affect initial paint, useEffect avoids potential hydration issues. |
__tests__/hooks/mutation/ |
— | Tests exist for useDeleteMetaProfile but not for useSaveMetaProfile or useActivateMetaProfile. Consider adding tests for these hooks to match coverage. |
Testing Verification
- Component tests cover: list view, empty state, editor open/create, activation, edit loading, unsupported backend (404), and duplicate name rejection
- Hook test covers: delete with cache invalidation
Risk Assessment
- Purely additive changes with no breaking modifications
- Backend API is optional (404 handling is present)
- No new external dependencies
- Cloud vs local backend is properly gated in the route
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
| model: string; | ||
| } | ||
|
|
||
| export interface MetaProfile { |
There was a problem hiding this comment.
Can we move these interfaces to openhands/typescript-client
There was a problem hiding this comment.
@malhotra5 is it okay if I release a tag of openhands/typescript-client, so I can merge this one?
Summary
Adds a new Model Router settings page (
/settings/meta-llm) for managing meta-profiles — declarative model-routing configurations consumed by the agent'sclassify_and_switch_llmtool. A meta-profile names aclassifier_model, adefault_model, and a list of taskclasses, each referencing a saved LLM profile.Previously meta-profiles could only be set via
PATCH /api/settings; this exposes full list/get/save/delete/activate management in the UI, mirroring the existing LLM Profiles experience.What's included
MetaProfilesService— client over the new/api/meta-profilesendpoints (built on the SDK's publicHttpClient, mirroringProfilesClient).useMetaProfiles, plususeSaveMetaProfile/useDeleteMetaProfile/useActivateMetaProfile. Activate also refreshes the settings cache, since it flipsactive_meta_profile(which controls whetherclassify_and_switch_llmis attached to new conversations)....overflow menu (Edit / Set active / Delete, matching/settings/llm), a create/edit editor (name, classifier model, default model, dynamic task-class rows with profile-name dropdowns sourced from saved LLM profiles), and a delete-confirmation modal.Testing
tsc,eslint, prettier, and the i18n duplicate-key / completeness checks all pass.Dependency
Requires the meta-profile CRUD API (
/api/meta-profiles) added in software-agent-sdk PR #3744. Until that ships, the page renders but the list query returns 404.Shared client types (review follow-up)
Per review feedback, the meta-profile interfaces and HTTP wiring should live in the shared SDK client rather than being redefined here. Those types and a dedicated
MetaProfilesClient(mirroringProfilesClient) have been added in typescript-client PR #212. Once that lands and is released, this PR will bump@openhands/typescript-clientand refactorMetaProfilesServiceto import the types + client from the package (dropping the local interfaces and the rawHttpClientusage), exactly asProfilesServicedoes today. The interim local definitions are kept only so the page compiles against the currently-published client.Review fixes (commit ade0bf9)
Addressed review feedback on the create-flow semantics and the
compatibility boundary:
The backend save is create-or-overwrite, and create mode previously
validated only name syntax.
MetaProfileEditornow takesexistingNamesand, in create mode, rejects a name that already exists:Save is disabled and an explicit "name already exists" message is shown
(
meta-profile-name-taken). Edit mode is unchanged.page. On backends that predate the
/api/meta-profilesendpoints(software-agent-sdk #3744) the list query returns 404; the view now
detects
HttpError.status === 404and renders a clear "this backenddoesn't support model routing yet" message (
meta-profile-unsupported)and hides the Add affordance. Non-404 errors still show the generic
error with Add available.
(
SettingsService.invalidateCache+SETTINGS_QUERY_KEYS.personal),mirroring activation —
active_meta_profilecontrols whetherclassify_and_switch_llmattaches to new conversations, so the priorbehavior could leave settings stale until reload.
New tests covering the previously-uncovered risks:
onSave), unique name accepted, and edit-mode name still allowed.500 → generic error with Add still present.
useDeleteMetaProfileinvalidates the meta-profile andsettings caches on success.
i18n: added
SETTINGS$META_PROFILE_NAME_TAKENandSETTINGS$META_PROFILE_UNSUPPORTEDacross all locales.This PR was created by an AI agent (OpenHands) on behalf of @juanmichelini.