Skip to content

fix(core): preserve slash model ids for explicit providers#176

Merged
SantiagoDePolonia merged 1 commit intomainfrom
fix/aliases-conflicts-error
Mar 25, 2026
Merged

fix(core): preserve slash model ids for explicit providers#176
SantiagoDePolonia merged 1 commit intomainfrom
fix/aliases-conflicts-error

Conversation

@SantiagoDePolonia
Copy link
Copy Markdown
Contributor

@SantiagoDePolonia SantiagoDePolonia commented Mar 25, 2026 •
edited by coderabbitai bot
Loading

Summary

  • keep the explicit provider authoritative when parsing model selectors
  • preserve slash-containing raw model IDs like openai/gpt-oss-120b for explicit provider routing and alias resolution
  • update routing, validation, and alias tests to cover the normalized selector behavior

Testing

  • go test ./internal/aliases ./internal/core ./internal/providers ./internal/server

Summary by CodeRabbit

  • Bug Fixes

    • Fixed handling of slash-qualified model identifiers when explicit providers are specified. Models with slashes (e.g., openai/gpt-oss-120b) can now be used with explicit provider settings without generating conflicts.

  • Tests

    • Expanded test coverage for model resolution with slash-qualified identifiers and explicit provider configurations.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 25, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9db0f6a3-e1cd-4c55-bd02-ffae474d5c56

📥 Commits

Reviewing files that changed from the base of the PR and between a617ba1 and 6dd2b22.

📒 Files selected for processing (6)
  • internal/aliases/service_test.go
  • internal/core/model_selector.go
  • internal/core/model_selector_test.go
  • internal/core/request_model_resolution_test.go
  • internal/providers/router_test.go
  • internal/server/model_validation_test.go
📝 Walkthrough

Walkthrough

The changes modify provider resolution logic to treat explicitly specified providers as authoritative when parsing model selectors. A helper function was added to conditionally strip matching provider prefixes from models, enabling slash-containing model identifiers to coexist with different explicit providers. Test cases across the codebase were updated to reflect the new behavior, removing conflict-error assertions and replacing them with pass-through scenarios.

Changes

Cohort / File(s) Summary
Core Model Resolution Logic
internal/core/model_selector.go, internal/core/model_selector_test.go, internal/core/request_model_resolution_test.go		 Added splitQualifiedModel helper to parse prefix/rest forms. Updated ParseModelSelector to treat explicit providers as authoritative: matching provider prefixes are stripped once from models, while non-matching prefixes are preserved. Removed provider-conflict validation error scenarios and added test cases verifying matching and non-matching provider behaviors.
Service Layer
internal/aliases/service_test.go		 Added TestServiceResolveAliasWithExplicitProviderAndSlashModel to verify alias resolution succeeds with explicit provider (groq) and slash-qualified model (openai/gpt-oss-120b).
Router Layer
internal/providers/router_test.go		 Replaced TestRouterChatCompletion_ProviderConflict with TestRouterChatCompletion_ExplicitProviderKeepsSlashModelRaw. New test asserts routing succeeds with explicit provider and slash model, verifying response originates from correct provider and upstream request preserves raw model ID.
Validation Layer
internal/server/model_validation_test.go		 Updated test to expect 200 OK response instead of 400 conflict error when provider and slash-qualified model pair matches in the registry.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A provider's word is now quite clear,
No more conflicts need appear,
With slashes wild and names so bright,
Explicit choice makes routing right! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: preserving slash model IDs when explicit providers are specified, which is the core objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/aliases-conflicts-error

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@SantiagoDePolonia SantiagoDePolonia merged commit 0219925 into main Mar 25, 2026
16 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Mar 25, 2026
Merged
@SantiagoDePolonia SantiagoDePolonia deleted the fix/aliases-conflicts-error branch April 4, 2026 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@SantiagoDePolonia