feat(new-api): add endpoint debug selection#1836
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughReplaces ownedBy-based NewAPI endpoint selection with metadata-driven ModelType inference. Shared helpers, provider enrichment, and the settings dialog now derive selectable endpoint types from model metadata and explicit type precedence, with updated tests covering the new normalization and loading behavior. ChangesNew-API model type and selectable endpoint type resolution
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
![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: 3
🤖 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/renderer/src/components/settings/ModelConfigDialog.vue`:
- Around line 1561-1568: The watcher in ModelConfigDialog.vue only reacts to
config.value.endpointType, config.value.type, and
showEndpointTypeSelector.value, so it can miss provider metadata updates that
change availableEndpointTypes. Update that watcher’s source to also observe the
data that drives available endpoint types (or the resolved provider metadata) so
syncNewApiDerivedFields() reruns when the valid endpoint set changes. Keep the
existing calls to syncNewApiDerivedFields() and syncCapabilityProviderId() in
place, and ensure any invalid config.endpointType is revalidated/reset after
metadata arrives.
- Around line 1237-1344: The load guard in loadConfig is being cleared by
whichever async request finishes last, so overlapping calls from the immediate
watcher and onMounted can let an older load reset isLoadingModelConfig while a
newer one is still updating config. Update loadConfig in ModelConfigDialog.vue
to use a request token/counter (or otherwise ignore stale completions) so only
the latest invocation can clear the loading state and trigger follow-on watcher
behavior, or remove the duplicate initial call so initialization cannot overlap.
In `@src/shared/model.ts`:
- Around line 159-181: Update the model inference logic in model type detection
so known media model IDs are recognized before the chat fallback: in the
function that normalizes model IDs and returns ModelType (the block with
normalizeModelId, isPureNewApiMediaEndpointRoute, and the chat default), add
explicit checks for dalle-*, gpt-image-*, and sora-* and map them to
ImageGeneration or VideoGeneration as appropriate. This should happen before the
existing return-to-chat path so sparse NewAPI /models metadata still selects the
media debug endpoint instead of being treated as chat.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 473a4c1c-3962-4dc7-9be1-ac706775b528
📒 Files selected for processing (8)
src/main/presenter/configPresenter/providerModelHelper.tssrc/main/presenter/llmProviderPresenter/providers/aiSdkProvider.tssrc/renderer/src/components/settings/ModelConfigDialog.vuesrc/shared/model.tstest/main/presenter/configPresenter/providerModelHelper.test.tstest/main/presenter/llmProviderPresenter/newApiProvider.test.tstest/main/shared/model.test.tstest/renderer/components/ModelConfigDialog.test.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/renderer/components/ModelConfigDialog.test.ts (1)
1028-1055: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid pinning this test to mount-time call count.
Line 1054 makes the test depend on the current watcher/load scheduling by requiring exactly two
getModelConfigcalls right aftersetup(). That is brittle for harmless refactors that still preserve the real contract here: stale loads must not clearisLoadingModelConfig, and the latest load must win. Prefer explicitly triggering the second load in the test instead of relying on mount internals.🤖 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 `@test/renderer/components/ModelConfigDialog.test.ts` around lines 1028 - 1055, The test is too tightly coupled to mount-time scheduling by asserting an exact `getModelConfig` call count immediately after `setup()`, which makes it brittle. Update `ModelConfigDialog.test.ts` so the overlapping-load scenario is driven explicitly through the component behavior or store action rather than relying on the initial watcher/mount internals. Keep the focus on the real contract in `isLoadingModelConfig`: stale requests must not clear the loading guard, and the latest load should still be active until it resolves. Reference the existing `getModelConfig`, `setup`, and `isLoadingModelConfig` paths when adjusting the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/renderer/components/ModelConfigDialog.test.ts`:
- Around line 1028-1055: The test is too tightly coupled to mount-time
scheduling by asserting an exact `getModelConfig` call count immediately after
`setup()`, which makes it brittle. Update `ModelConfigDialog.test.ts` so the
overlapping-load scenario is driven explicitly through the component behavior or
store action rather than relying on the initial watcher/mount internals. Keep
the focus on the real contract in `isLoadingModelConfig`: stale requests must
not clear the loading guard, and the latest load should still be active until it
resolves. Reference the existing `getModelConfig`, `setup`, and
`isLoadingModelConfig` paths when adjusting the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 05dfffb3-5dd8-4a34-9ac9-ba602d42614c
📒 Files selected for processing (6)
src/renderer/src/components/settings/ModelConfigDialog.vuesrc/shared/model.tstest/main/presenter/configPresenter/providerModelHelper.test.tstest/main/presenter/llmProviderPresenter/newApiProvider.test.tstest/main/shared/model.test.tstest/renderer/components/ModelConfigDialog.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- test/main/presenter/configPresenter/providerModelHelper.test.ts
- src/shared/model.ts
- test/main/presenter/llmProviderPresenter/newApiProvider.test.ts
- src/renderer/src/components/settings/ModelConfigDialog.vue
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
1 participant
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.
add new-api provider endpoint debug selection
Summary by CodeRabbit
New Features
Bug Fixes
Tests