feat: wire family aliases - #208
Conversation
Kimi-K2 GGUF files often report general.architecture "deepseek2" or "deepseek" since they share the same underlying transformer as DeepSeek-V3, and architecture-based lookup ran ahead of name-pattern matching, so a Kimi model reporting that arch got classified as deepseek. Adding kimi to special_rules.preserve_family lets the name win instead, via the token-prefix check that already runs ahead of the arch stage - chosen over inverting arch-vs-name precedence globally.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe change adds Kimi to preserved model families. Metadata extraction now prioritises configured families, supports token-boundary matching and aliases, and retains fallback handling for known families. Tests cover Kimi, DeepSeek, aliases, and nil configuration. ChangesModel family extraction
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/adapter/unifier/metadata_extractor.go (1)
251-251: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove or replace this redundant comment.
Line 251 repeats the function name and describes behaviour. State the reason for the helper, or remove the comment.
As per coding guidelines, comment on why, not what.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/adapter/unifier/metadata_extractor.go` at line 251, Remove the redundant comment above extractFromArchitecture, or replace it with a concise explanation of why the helper is needed rather than restating what the function does.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/adapter/unifier/metadata_extractor.go`:
- Around line 223-225: Update the model-name matching flow in the metadata
extraction logic to remove the publisher prefix before calling
matchesTokenPrefix, ensuring names such as moonshotai/Kimi-K2-Instruct match
kimi before deepseek architecture handling takes effect. Add a regression test
covering this architecture collision and preserving the expected model
classification.
---
Nitpick comments:
In `@internal/adapter/unifier/metadata_extractor.go`:
- Line 251: Remove the redundant comment above extractFromArchitecture, or
replace it with a concise explanation of why the helper is needed rather than
restating what the function does.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 673895eb-5e09-4061-9121-69fecb8f46d3
📒 Files selected for processing (5)
config/models.yamlinternal/adapter/unifier/metadata_extractor.gointernal/adapter/unifier/metadata_extractor_test.gointernal/adapter/unifier/metadata_integration_test.gointernal/adapter/unifier/model_config.go
matchPreserveFamily ran its token-prefix check against the whole model name, so a publisher-prefixed name like moonshotai/Kimi-K2-Instruct (common from LM Studio and vLLM) never matched the kimi entry and the deepseek arch string won instead - the exact misclassification the kimi fix was meant to close, just surviving for prefixed names. Now strips everything up to and including the last '/' before checking, while the arch-equality check keeps using the raw arch string.
Follow-up to #206/#207. The model unification config carried two fields that were parsed but never consumed:
model_extraction.family_aliasesandspecial_rules.preserve_family. This wires both into the family-extraction pipeline, and uses the new mechanism to fix the Kimi-K2 misclassification flagged by review on #207.Changes
preserve_familyguard: runs before all extraction stages with exact-token/prefix matching (never bare substring). Listed models keep their full family name instead of being collapsed by architecture mappings or patterns:deepseek-coder-v2staysdeepseek-coder-v2rather than collapsing todeepseeknomic-bertgains a family (previously extracted as"")family_aliasesconsumption, in two places:architecture_mappings(e.g. archdeepseek2reported by some GGUFs)kimiadded topreserve_family. Kimi-K2 GGUFs report architecturedeepseek/deepseek2(shared lineage), so arch-based extraction misclassified them; they now resolve to familykimiregardless of arch metadata. Chosen over inverting name-vs-arch precedence globally, which would affect every model where the two disagree.Scope
Family feeds only the
/olla/modelsfamilyfield and its?family=filter. Unified model IDs, sticky-session keys, routing and the status/dashboard grouping (which uses the provider-reporteddetails.family) are unaffected.Notes
kimi-k2now extracts variant""(preserve short-circuits pattern matching) - the same trade-offdeepseek-coder-v2andnomic-bertmake.devstralresolves via its explicit pattern from chore: models 2026 refresh #207 (familydevstral), not thedevstral -> mistralalias; the alias remains reachable only through the arch-miss and delimiter fallbacks.Summary by CodeRabbit