fix: preserve slot allowlist fail-closed behavior - #2797
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Workflow source neededPR #2797 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughChangesLLM registry hardening
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression in the shared LLM registry/slot resolution logic by restoring “fail-closed” behavior when a bundled slot allowlist exists but yields no usable slots, while also avoiding mutable defaults on frozen registry entries. It also synchronizes the consumer template copy and updates tests to lock in the intended behavior.
Changes:
- Stop populating
ModelRegistryEntry.qualitywith an empty dict during registry load; leave it unset (None) to avoid mutable state on frozen entries. - Restrict
LANGCHAIN_MODELemergency bootstrap to only when no bundled slot allowlist file is present (not merely when resolved slots are empty). - Update consumer template and adjust/add regression tests to assert the new fail-closed and
quality is Noneexpectations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tools/llm_registry.py | Removes quality={} initialization and tightens LANGCHAIN_MODEL bootstrap gating based on bundled slot allowlist presence. |
| tests/tools/test_llm_registry_selection.py | Updates/extends regression coverage for quality is None and for failing closed despite LANGCHAIN_MODEL when a bundled slot config exists but is unusable. |
| tests/tools/test_langchain_client.py | Aligns existing tests with the new contract that loaded registry entries leave quality unset (None). |
| templates/consumer-repo/tools/llm_registry.py | Mirrors the llm_registry.py behavior changes in the consumer template to prevent drift. |
Automated Status SummaryHead SHA: 4d98aa5
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
|
Fixes the shared consumer-sync regression in
tools/llm_registry.py:\n\n- leave compatibilityqualityunset rather than adding a mutable dictionary to frozen entries\n- allowLANGCHAIN_MODELemergency bootstrap only when the bundled slot allowlist is absent\n- sync the consumer template and add regression coverage\n\nValidation:pytest -q tests/tools/test_langchain_client.py tests/tools/test_llm_registry_selection.py(82 passed);python scripts/validate_template_sync.py.Summary by CodeRabbit