fix: catalog sync, validation, and models list#124
Conversation
Four related fixes for the catalog subsystem: 1. validateCatalog now skips models with unknown providers instead of failing fatally (upstream catalog has dirty data). 2. ListProviderModels uses the pre-built ProviderModels index instead of iterating Models with a key prefix, which doesn't work with the current catalog format where model keys use native provider names. 3. catalog sync now migrates the downloaded JSON to SQLite so that models list and other SQLite-backed commands can find it. 4. models list without --provider now shows all catalog providers that have models, instead of only showing configured routing providers (which have no models of their own). 5. Missing catalog is now a printed message instead of a fatal error.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| fmt.Println("\nPress Ctrl+C to stop.") | ||
| } | ||
| // Open GUI (macOS: native webview, Linux/Windows: print URL) | ||
| if err := openGUI(guiURL, cancel); err != nil { |
There was a problem hiding this comment.
🔥 The Roast: You changed the call to openGUI(guiURL, cancel) with two arguments, but all three platform-specific implementations (start_gui_darwin.go:12, start_gui_darwin_nocgo.go:9, start_gui_other.go:9) still have the signature func openGUI(guiURL string) error. This PR won't compile on any platform — not even the macOS one with the fancy native webview. That's like sending your kids to school without pants and calling it a fashion statement.
🩹 The Fix: Revert the call back to openGUI(guiURL) on this line, since none of the openGUI implementations were updated to accept cancel. Or, if you actually meant to plumb cancel through, update all three start_gui_* files to match.
| if err := openGUI(guiURL, cancel); err != nil { | |
| if err := openGUI(guiURL); err != nil { |
📏 Severity: critical
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review Roast 🔥Verdict: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: The catalog migration logic actually solves a real pain point — syncing JSON but not migrating to SQLite was a user-facing disaster that this PR finally fixes. 💀 Worst part: 📊 Overall: Like a chef who perfectly seasons the steak but sets the kitchen on fire. The catalog fixes are solid; the broken Files Reviewed (5 files)
Fix these issues in Kilo Cloud Reviewed by step-3.7-flash · Input: 157.8K · Output: 15.4K · Cached: 1.6M Review guidance: REVIEW.md from base branch |
1 participant
What
Four fixes for the catalog and
models listcommand:catalog syncnow migrates JSON to SQLite — previously it only downloaded the JSON file, somodels list(which reads from SQLite) would always show "catalog not found".validateCatalog()handles orphan models gracefully — the upstream catalog may contain models for providers not yet in the catalog. Instead of failing the entire sync, orphan models are now skipped with a warning. Currently 7 models are affected:deepreinforce/ornith-1.0-31bdeepreinforce/ornith-1.0-35bdeepreinforce/ornith-1.0-397bdeepreinforce/ornith-1.0-9bmeituan/longcat-2.0microsoft/mai-code-1-flashtencent/hy3-previewThese reference providers (
deepreinforce,meituan,microsoft,tencent) not yet present in the upstreamprovidersmap.ListProviderModels()uses the pre-built index — the new catalog format uses aProviderModelsmap built during load. Previously it fell through to astrings.HasPrefixscan that did not match the new key format, returning 0 models.models listwithout--providershows catalog providers — instead of returning routing intermediaries with no models, it now lists all catalog providers that have models.Testing
catalog syncthenmodels listshows all providers