fix(registry): normalize model IDs with underscores to dashes #467
Conversation
Adds underscore-to-dash normalization for model ID lookups across: - ClientSupportsModel - GetModelCount - GetModelProviders - GetModelInfo Also adds wildcard/prefix matching for amp model mappings (e.g., 'claude-haiku-*' matches 'claude-haiku-4-5-20251001'). Fixes: Claude Code sending model names like 'claude-sonnet-4_5-20250929' which don't match registered models 'claude-sonnet-4-5-20250929'.
Summary of ChangesHello @heyhuynhgiabuu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves issues related to inconsistent model ID formats by standardizing model name lookups. It ensures that model IDs containing underscores are correctly recognized by normalizing them to use dashes, and enhances model mapping capabilities by introducing wildcard matching. These changes improve the robustness of model identification and mapping, preventing "unknown provider" errors and supporting more flexible model configurations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces two main changes: normalizing model IDs by converting underscores to dashes for lookups, and adding wildcard/prefix matching for AMP model mappings. While the intent is good, I've found a few issues in the implementation. The wildcard matching logic can be non-deterministic, and the model ID normalization is incomplete in several functions, leading to bugs where models might not be found. I've provided suggestions to fix these issues to make the logic more robust and deterministic.
| trimmedID := strings.TrimSpace(id) | ||
| if strings.EqualFold(trimmedID, modelID) || strings.EqualFold(trimmedID, normalizedModelID) { | ||
| return true | ||
| } |
There was a problem hiding this comment.
The current logic for checking model support is not robust. It fails if a model is registered with underscores (e.g., claude_3_opus) and a request is made with dashes (claude-3-opus). To fix this, you should normalize both the registered model ID and the requested model ID to a canonical form (e.g., using dashes) before comparing them.
trimmedID := strings.TrimSpace(id)
normalizedRegisteredID := strings.ReplaceAll(trimmedID, "_", "-")
if strings.EqualFold(normalizedRegisteredID, normalizedModelID) {
return true
}| // Normalize model ID: replace underscores with dashes for consistent lookup | ||
| normalizedID := strings.ReplaceAll(modelID, "_", "-") | ||
| registration, exists := r.models[normalizedID] | ||
| if !exists { | ||
| registration, exists = r.models[modelID] | ||
| } |
There was a problem hiding this comment.
The lookup logic here is flawed. It does not correctly handle cases where a model is registered with underscores (e.g., claude_1) and a request is made with dashes (e.g., claude-1). In this scenario, both lookups will fail.
A more robust solution would be to normalize model IDs to a canonical form (e.g., with dashes) upon registration in RegisterClient.
As a less invasive fix, you can adjust the lookup logic to check for all variants. The same logic should be applied consistently to GetModelProviders and GetModelInfo as well, as they suffer from the same issue.
// Normalize model ID: replace underscores with dashes for consistent lookup
normalizedID := strings.ReplaceAll(modelID, "_", "-")
registration, exists := r.models[normalizedID]
if !exists {
if modelID != normalizedID {
registration, exists = r.models[modelID]
}
if !exists {
withUnderscores := strings.ReplaceAll(modelID, "-", "_")
if withUnderscores != modelID && withUnderscores != normalizedID {
registration, exists = r.models[withUnderscores]
}
}
}| if !exists { | ||
| for pattern, target := range m.mappings { | ||
| if strings.HasSuffix(pattern, "*") { | ||
| prefix := strings.TrimSuffix(pattern, "*") | ||
| if strings.HasPrefix(normalizedRequest, prefix) { | ||
| targetModel = target | ||
| exists = true | ||
| log.Debugf("amp model mapping: wildcard match %s -> %s (pattern: %s)", normalizedRequest, target, pattern) | ||
| break | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation for wildcard matching iterates over a map and breaks on the first match. Since map iteration order in Go is not guaranteed, this can lead to non-deterministic behavior if multiple wildcard patterns could match a given model ID. For example, with patterns claude-* and claude-haiku-*, a request for claude-haiku-20240101 could be mapped to either target depending on the iteration order. To ensure deterministic behavior, you should find the longest matching prefix.
if !exists {
var bestMatchPrefix string
var bestMatchTarget string
var bestMatchPattern string
for pattern, target := range m.mappings {
if strings.HasSuffix(pattern, "*") {
prefix := strings.TrimSuffix(pattern, "*")
if strings.HasPrefix(normalizedRequest, prefix) {
// Find the longest matching prefix to ensure deterministic matching.
if len(prefix) > len(bestMatchPrefix) {
bestMatchPrefix = prefix
bestMatchTarget = target
bestMatchPattern = pattern
}
}
}
}
if bestMatchTarget != "" {
targetModel = bestMatchTarget
exists = true
log.Debugf("amp model mapping: wildcard match %s -> %s (pattern: %s)", normalizedRequest, targetModel, bestMatchPattern)
}
}There was a problem hiding this comment.
Pull request overview
This PR addresses model ID normalization issues where Claude Code sends model names with underscores (e.g., claude-sonnet-4_5-20250929) but the registry expects dashes (claude-sonnet-4-5-20250929). It also adds wildcard pattern matching for flexible Amp model routing configurations.
- Adds underscore-to-dash normalization in model registry lookup functions
- Implements wildcard suffix matching for Amp model mappings (e.g.,
claude-haiku-*matches versioned variants) - Applies consistent normalization across both registry lookups and model mapping configurations
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
internal/registry/model_registry.go |
Adds underscore-to-dash normalization to ClientSupportsModel, GetModelCount, GetModelProviders, and GetModelInfo functions for consistent model ID lookup |
internal/api/modules/amp/model_mapping.go |
Implements underscore-to-dash normalization and wildcard suffix pattern matching in model mapping lookups and configuration updates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if !exists { | ||
| for pattern, target := range m.mappings { | ||
| if strings.HasSuffix(pattern, "*") { | ||
| prefix := strings.TrimSuffix(pattern, "*") | ||
| if strings.HasPrefix(normalizedRequest, prefix) { | ||
| targetModel = target | ||
| exists = true | ||
| log.Debugf("amp model mapping: wildcard match %s -> %s (pattern: %s)", normalizedRequest, target, pattern) | ||
| break | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
[nitpick] The wildcard matching iterates through all mappings for every request that doesn't have a direct match. If there are many mappings, this could impact performance.
Consider optimizing by:
- Separating direct mappings from wildcard patterns during
UpdateMappings - Only iterating through wildcard patterns when needed
- Caching successful wildcard matches
| normalizedFrom = strings.ReplaceAll(normalizedFrom, "_", "-") | ||
| m.mappings[normalizedFrom] = to | ||
|
|
||
| log.Debugf("amp model mapping registered: %s -> %s", from, to) |
There was a problem hiding this comment.
[nitpick] The debug log shows the original from value before normalization, which could be confusing when debugging. The actual stored key is normalizedFrom (with underscores replaced by dashes and lowercased).
Consider updating the log to show the normalized key:
log.Debugf("amp model mapping registered: %s -> %s", normalizedFrom, to)| log.Debugf("amp model mapping registered: %s -> %s", from, to) | |
| log.Debugf("amp model mapping registered: %s -> %s", normalizedFrom, to) |
| if !exists { | ||
| for pattern, target := range m.mappings { | ||
| if strings.HasSuffix(pattern, "*") { | ||
| prefix := strings.TrimSuffix(pattern, "*") | ||
| if strings.HasPrefix(normalizedRequest, prefix) { | ||
| targetModel = target | ||
| exists = true | ||
| log.Debugf("amp model mapping: wildcard match %s -> %s (pattern: %s)", normalizedRequest, target, pattern) | ||
| break | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Map iteration order in Go is non-deterministic. If multiple wildcard patterns could match the same request (e.g., both "claude-" and "claude-haiku-" matching "claude-haiku-4-5"), the selected mapping will be unpredictable.
Consider sorting patterns by specificity (longest prefix first) or documenting this limitation to prevent configuration issues.
| // If no direct match, try prefix/wildcard matching | ||
| // This allows mappings like "claude-haiku-*" to match "claude-haiku-4-5-20251001" | ||
| if !exists { | ||
| for pattern, target := range m.mappings { | ||
| if strings.HasSuffix(pattern, "*") { | ||
| prefix := strings.TrimSuffix(pattern, "*") | ||
| if strings.HasPrefix(normalizedRequest, prefix) { | ||
| targetModel = target | ||
| exists = true | ||
| log.Debugf("amp model mapping: wildcard match %s -> %s (pattern: %s)", normalizedRequest, target, pattern) | ||
| break | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The new wildcard matching feature lacks test coverage. Consider adding tests for:
- Wildcard suffix matching (e.g., "claude-haiku-*" matches "claude-haiku-4-5-20251001")
- Precedence between direct matches and wildcard matches
- Multiple wildcard patterns
- Edge cases like "" alone or patterns without the "" suffix
| // Replace underscores with dashes for consistent lookup (e.g., claude-sonnet-4_5 -> claude-sonnet-4-5) | ||
| normalizedRequest := strings.ToLower(strings.TrimSpace(requestedModel)) | ||
| normalizedRequest = strings.ReplaceAll(normalizedRequest, "_", "-") | ||
|
|
There was a problem hiding this comment.
The underscore-to-dash normalization in model mapping lacks test coverage. Consider adding tests for:
- Mapping keys with underscores (e.g., "claude_sonnet_4_5" should match "claude-sonnet-4-5")
- Request models with underscores matching normalized mapping keys
- Combination of underscore normalization with wildcard matching
- Add normalizeModelID() helper to convert underscores to dashes - Apply normalization consistently in RegisterClient and lookups - Sort wildcard patterns by length (longest first) for deterministic matching - Add 'sort' import to model_mapping.go for stable pattern ordering
1 participant
Summary
Problem
Claude Code sends model names with underscores (e.g.,
claude-sonnet-4_5-20250929) but registered models use dashes (claude-sonnet-4-5-20250929), causing "unknown provider for model" errors.Also, users may configure mappings like
claude-3.5-haikubut requests come for versioned names likeclaude-haiku-4-5-20251001.Solution
Model Registry: Normalize underscores to dashes in lookup functions:
ClientSupportsModelGetModelCountGetModelProvidersGetModelInfoModel Mapping:
claude-haiku-*matchesclaude-haiku-4-5-20251001)Files Changed
internal/registry/model_registry.gointernal/api/modules/amp/model_mapping.go