fix: ignore tier slots with malformed registry - #2724
Conversation
stranske
added
automation
📝 WalkthroughWalkthroughAdds ChangesRegistry format validation and slot fallback
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Workflow source neededPR #2724 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
1 similar comment
Workflow source neededPR #2724 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
Automated Status SummaryHead SHA: 8705942
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tools/llm_registry.py`:
- Around line 245-253: The early-fallback path in llm_registry is doing a
redundant second read/parse via _model_registry_format_valid() after
load_model_registry() has already loaded the same registry. Update the logic
around load_model_registry() and _model_registry_format_valid() so the fallback
decision reuses a single parse and gets validity/parse status from
load_model_registry() instead of re-reading the file. Keep the check in the same
branch that builds slot_entries and returns fallback_slots, but base it on the
already-loaded registry data and a shared validity signal.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: 3e00d214-508c-434e-bb5b-90fbfb6fa48e
📒 Files selected for processing (3)
templates/consumer-repo/tools/llm_registry.pytests/tools/test_langchain_client.pytools/llm_registry.py
| registry = load_model_registry() | ||
| slot_entries = _slot_entries(payload, path) | ||
| if not _model_registry_format_valid() and any( | ||
| str(entry.get("provider", "")).strip() | ||
| and not str(entry.get("model", "")).strip() | ||
| and str(entry.get("quality_tier") or entry.get("tier") or "").strip() | ||
| for entry in slot_entries | ||
| ): | ||
| return fallback_slots |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
Redundant registry read/parse on this path.
load_model_registry() (Line 245) already reads and parses the same file that _model_registry_format_valid() re-reads at Line 247, so the registry file is read and JSON-parsed twice per call. Beyond the extra I/O, the two have subtly different validity definitions (the validator only checks the top-level models is a list, while load_model_registry() also drops malformed entries), which can drift over time. Consider having load_model_registry() surface a validity/parse-status signal so the early-fallback decision reuses a single parse.
🤖 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 `@tools/llm_registry.py` around lines 245 - 253, The early-fallback path in
llm_registry is doing a redundant second read/parse via
_model_registry_format_valid() after load_model_registry() has already loaded
the same registry. Update the logic around load_model_registry() and
_model_registry_format_valid() so the fallback decision reuses a single parse
and gets validity/parse status from load_model_registry() instead of re-reading
the file. Keep the check in the same branch that builds slot_entries and returns
fallback_slots, but base it on the already-loaded registry data and a shared
validity signal.
2 participants
Summary
Validation
Unblocks sync/dependency cleanup for stranske/Inv-Man-Intake#731, where malformed registry fallback currently resolves only the first provider slot instead of the complete default slot list.
Summary by CodeRabbit