Skip to content

fix(model): split provider/model at first separator (slash or colon)#980

Open
mauroociappinaph wants to merge 1 commit into
Gentleman-Programming:mainfrom
mauroociappinaph:fix/model-provider-model-split
Open

fix(model): split provider/model at first separator (slash or colon)#980
mauroociappinaph wants to merge 1 commit into
Gentleman-Programming:mainfrom
mauroociappinaph:fix/model-provider-model-split

Conversation

@mauroociappinaph

@mauroociappinaph mauroociappinaph commented Jun 28, 2026

Copy link
Copy Markdown

Closes #802

Summary

Introduces model.SplitProviderModel helper that splits at the FIRST occurrence of '/' or ':', fixing OpenRouter free model specs like openrouter/qwen/qwen3.6-plus:free which contain both separators.

Applied to the 3 parse sites:

  • internal/cli/sync.go: parseModelSpec
  • internal/components/sdd/profiles.go: extractModelFromAgent
  • internal/components/sdd/read_assignments.go: ReadCurrentModelAssignments

Changes

File Change
internal/model/split.go New SplitProviderModel helper (first-separator split)
internal/model/split_test.go 11 test cases covering colon, slash, OpenRouter free/paid, edge cases
internal/cli/sync.go Use new helper in parseModelSpec
internal/components/sdd/profiles.go Use new helper in extractModelFromAgent
internal/components/sdd/read_assignments.go Use new helper in ReadCurrentModelAssignments
internal/components/sdd/profiles_test.go TestExtractModelFromAgent regression tests
internal/components/sdd/read_assignments_test.go TestReadCurrentModelAssignmentsOpenRouterFreeSuffix regression test

Test Plan

  • go test ./internal/model/... — 11 SplitProviderModel tests pass
  • go test ./internal/cli/... — all sync tests pass
  • go test ./internal/components/sdd/... — all profiles/read_assignments tests pass
  • New regression tests: openrouter/qwen/qwen3.6-plus:free correctly splits to provider=openrouter, model=qwen/qwen3.6-plus:free

Summary by CodeRabbit

  • Bug Fixes
    • Improved model specification parsing across the app, including support for both provider/model and provider:model formats.
    • Fixed handling of model names that contain multiple separators, preventing incorrect parsing of certain provider strings.
    • Invalid or incomplete model entries are now safely ignored or reported instead of being misread.
  • Tests
    • Added regression coverage for standard and edge-case model parsing scenarios.

Introduces model.SplitProviderModel helper that splits at the FIRST
occurrence of '/' or ':', fixing OpenRouter free model specs like
'openrouter/qwen/qwen3.6-plus:free' which contain both separators.

Applied to the 3 parse sites:
- internal/cli/sync.go: parseModelSpec
- internal/components/sdd/profiles.go: extractModelFromAgent
- internal/components/sdd/read_assignments.go: ReadCurrentModelAssignments

Added regression tests for OpenRouter free models (Gentleman-Programming#260/Gentleman-Programming#802).

Closes Gentleman-Programming#802
@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0a78f835-dedc-4010-aaf6-5a3ee8cef3df

📥 Commits

Reviewing files that changed from the base of the PR and between 8df34cd and 0f098e0.

📒 Files selected for processing (7)
  • internal/cli/sync.go
  • internal/components/sdd/profiles.go
  • internal/components/sdd/profiles_test.go
  • internal/components/sdd/read_assignments.go
  • internal/components/sdd/read_assignments_test.go
  • internal/model/split.go
  • internal/model/split_test.go

📝 Walkthrough

Walkthrough

Introduces internal/model/split.go with a shared SplitProviderModel helper that splits on the first / or : separator. Three call sites that previously duplicated manual separator-scan logic (parseModelSpec, extractModelFromAgent, inline parser in ReadCurrentModelAssignments) are each replaced with a call to this helper. Regression tests are added for OpenRouter free model strings.

SplitProviderModel helper and call-site consolidation

Layer / File(s) Summary
SplitProviderModel helper and unit tests
internal/model/split.go, internal/model/split_test.go
New SplitProviderModel splits on the first / or :, returns (providerID, modelID, ok), with table-driven tests covering edge cases and OpenRouter free model regression.
Replace manual parsing in three call sites
internal/cli/sync.go, internal/components/sdd/profiles.go, internal/components/sdd/read_assignments.go
parseModelSpec, extractModelFromAgent, and the inline parser in ReadCurrentModelAssignments each replaced with SplitProviderModel; strings import removed from read_assignments.go.
Regression tests for updated call sites
internal/components/sdd/profiles_test.go, internal/components/sdd/read_assignments_test.go
TestExtractModelFromAgent and TestReadCurrentModelAssignmentsOpenRouterFreeSuffix verify first-separator splitting for OpenRouter free model strings at both call sites.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: splitting provider/model specs at the first separator.
Linked Issues check ✅ Passed The PR matches #802 by fixing read-path parsing in all three specified locations and adding regression coverage.
Out of Scope Changes check ✅ Passed The changes stay focused on the parsing bug and related tests, with no obvious unrelated scope added.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(model): provider/model specs split at first colon even when a slash comes first

1 participant