feat: support multi-slash model IDs and provider hint extraction#2758
Open
hunghoang3011 wants to merge 2 commits intocode-yeongyu:devfrom
Open
feat: support multi-slash model IDs and provider hint extraction#2758hunghoang3011 wants to merge 2 commits intocode-yeongyu:devfrom
hunghoang3011 wants to merge 2 commits intocode-yeongyu:devfrom
Conversation
- Add extractProviderHint utility to extract provider from model strings - Update fuzzyMatchModel to retry without provider filter when no matches - Fix extractModelPrefix to use lastIndexOf for multi-slash IDs - Update model resolution to use provider hints instead of naive split - Pass agent model overrides to background and sync executors - Add tests for multi-slash model ID handling and provider hint extraction
Contributor
|
All contributors have signed the CLA. Thank you! ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
3 issues found across 14 files
Confidence score: 3/5
- There is a concrete regression risk in
src/tools/call-omo-agent/tools.ts: model resolution currently skips category-level inheritance and only usesagentOverride.model, which can cause the wrong primary model to be selected at runtime. - The other findings in
src/shared/model-resolution-pipeline.tsandsrc/shared/model-availability.tsare lower severity (duplicate/shadowed initialization and unreachable fallback branch) and are more about correctness clarity and maintainability than immediate breakage. - Given one medium-severity, high-confidence user-facing behavior issue plus a couple of minor refactor artifacts, this carries some merge risk but is likely manageable with a focused fix.
- Pay close attention to
src/tools/call-omo-agent/tools.ts,src/shared/model-resolution-pipeline.ts,src/shared/model-availability.ts- ensure inheritance-based model selection is restored and clean up shadowed/dead fallback logic.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/shared/model-resolution-pipeline.ts">
<violation number="1" location="src/shared/model-resolution-pipeline.ts:49">
P2: Refactor introduced duplicate `connectedProviders` initialization; inner fallback-chain declaration shadows outer variable and redundantly re-reads cache.</violation>
</file>
<file name="src/tools/call-omo-agent/tools.ts">
<violation number="1" location="src/tools/call-omo-agent/tools.ts:51">
P2: `call-omo-agent` model resolution ignores category-level model inheritance and only reads `agentOverride.model`, so category-configured primary models are silently skipped.</violation>
</file>
<file name="src/shared/model-availability.ts">
<violation number="1" location="src/shared/model-availability.ts:68">
P3: The added `else` branch in the empty-candidates fallback is dead code and cannot be reached with current control flow.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
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
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
aws/anthropic/bedrock-claude-opus-4-6) where the first/-segment is not a real provider, causing agents tofall through to free/default models
call-omo-agent, which previously sent prompts without anymodelfieldChanges
extractProviderHintutility (src/shared/extract-provider-hint.ts): Validates the first segment of a model string against the connected-providers cachebefore using it as a provider filter. Returns
undefinedfor unrecognized providers, enabling cross-provider matching.fuzzyMatchModelcross-provider fallback (src/shared/model-availability.ts): When provider-filtered candidates = 0, retries without the provider filter insteadof returning
nullimmediately. Acts as a universal safety net for multi-slash IDs.extractModelPrefixfix (src/hooks/think-mode/switcher.ts): ChangedindexOf("/")tolastIndexOf("/")so think-mode correctly extracts the base model namefrom multi-slash IDs.
src/shared/model-resolution-pipeline.ts): Replaced naive[parts[0]]provider hint extraction at 4 sites withextractProviderHintvalidated against connected providers.
src/tools/delegate-task/model-selection.ts): SameextractProviderHintfix for the category-default and user fallback model paths.call-omo-agentmodel forwarding (src/tools/call-omo-agent/tools.ts,sync-executor.ts,background-executor.ts): AddedresolveModelForCallOmoAgentto readagent override model from config, and pass
model/variantin thepromptAsyncbody andmanager.launch()call -- matching howdelegate-taskalready handles this.Testing