Fix model collision by filtering and validating kimchi-dev provider models#121
Fix model collision by filtering and validating kimchi-dev provider models#121
Conversation
Kimchi Code Review
Summary📊 Review Score: 92/100 (overall code quality — 0 lowest, 100 highest) 🧪 Tests: yes — Comprehensive test coverage added for the new model filtering logic and No significant issues found. LGTM! 🎉 What to expectKimchi will analyze the changes in this pull request and post:
The review typically completes within a few minutes. This comment will be updated once the review is ready. Interact with Kimchi
ConfigurationReviews are configured by your organization admin. Powered by Kimchi — AI-powered code review by CAST AI |
![kimchi-review[bot]](https://avatars.githubusercontent.com/in/3223788?s=60&v=4){kind=small}
There was a problem hiding this comment.
📊 Review Score: 88/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 2/5 (1 = trivial, 5 = very complex)
🧪 Tests: yes — Comprehensive tests added for model filtering logic (filtering by provider, empty results, all-kimchi scenarios) and validation behavior (accepting kimchi-dev models, rejecting others with specific error messages). However, tests duplicate implementation logic rather than importing the actual validation function.
📝 Found 2 issue(s). See inline comments for details.
|
@kimchi review |
|
🔄 Starting review on |
There was a problem hiding this comment.
📊 Review Score: 92/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 2/5 (1 = trivial, 5 = very complex)
🧪 Tests: yes — Comprehensive test coverage added for the new model filtering logic and validateKimchiModel function. Tests cover edge cases including empty results, non-kimchi providers (anthropic, amazon-bedrock), and validation error messages.
No significant issues found. LGTM! 🎉
1 participant
Problem
When selecting models like , pi-mono's built-in model registry caused collisions due to duplicate model IDs under different providers:
This led to:
Solution
Two-layer defense:
Layer 1: UI Filtering (ctrl+p cycling)
Only show models from provider in the model cycle list.
Layer 2: Runtime Validation
Added to throw a clear error if any non-kimchi model is selected, catching edge cases like saved sessions or CLI args.
Changes
Tests
All 724 tests pass, including 6 new tests for the fix.
Kimchi Summary
What changed
Filter the model registry to only display models from the
kimchi-devprovider when cycling through available models with Ctrl+P. This prevents duplicate entries and API conflicts when model IDs collide with pi-mono's built-in providers.Why
Model IDs such as
claude-opus-4-7exist under multiple providers (anthropic, amazon-bedrock) in pi-mono's registry, causing confusion and potential API routing conflicts. This ensures only Kimchi-served models are selectable.Key changes
KIMCHI_PROVIDERconstant ("kimchi-dev") insrc/models.tsto centralize the provider identifiervalidateKimchiModel()function insrc/extensions/ui.tsthat throws an error when attempting to use non-kimchi-dev modelssrc/extensions/ui.tsto filterctx.modelRegistry.getAvailable()byKIMCHI_PROVIDERsrc/extensions/ui.test.tscovering provider filtering and validation behaviorImpact
Users will now only see Kimchi-hosted models in the model switcher. Models from anthropic, amazon-bedrock, and other pi-mono built-in providers are excluded to avoid ID collisions.