fix: restore legacy registry sync compatibility - #2807
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. |
|
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 (3)
📝 WalkthroughWalkthroughThe changes make empty model-registry configuration values use bundled defaults, restrict selection evidence IDs to valid strings, adjust legacy pin freshness findings, and document the required capability evidence identifier format. ChangesModel registry updates
Estimated code review effort: 3 (Moderate) | ~20 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 |
There was a problem hiding this comment.
Pull request overview
Restores compatibility for legacy/consumer registry sync behavior by adjusting how empty registry env overrides are interpreted, relaxing freshness-gate enforcement for unprofiled legacy pins, and tightening evidence-id parsing while clarifying the runner output contract.
Changes:
- Treat empty/whitespace
LANGCHAIN_MODEL_REGISTRY_CONFIGas “use bundled default registry” while keeping empty slot config fail-closed. - Make unprofiled (legacy) explicit pins advisory again (while still requiring a default selection to exist).
- Normalize
evidence_idsby ignoring non-string entries; clarifycapability-idformat in the contract doc.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/llm_registry.py | Adds empty_uses_default handling for registry path; ignores non-string evidence_ids. |
| tools/check_model_registry_freshness.py | Treats unprofiled legacy pins as advisory (after verifying a default selection exists). |
| tests/tools/test_llm_registry_selection.py | Updates expectations for empty registry env behavior; adds evidence-id non-string test. |
| tests/test_check_model_registry_freshness.py | Updates legacy-pin tests to reflect advisory handling. |
| templates/consumer-repo/tools/llm_registry.py | Mirrors tools/llm_registry.py changes into consumer template. |
| docs/contracts/agent-runner-output.md | Clarifies capability-id must be capability:<lowercase-kebab-id>. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06e8f79778
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Automated Status SummaryHead SHA: 90948f1
Coverage Overview
Failure triageDetected failure types: pytest.
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: 2
🤖 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 `@templates/consumer-repo/tools/llm_registry.py`:
- Around line 186-188: The canonical evidence-ID filtering in
templates/consumer-repo/tools/llm_registry.py:186-188 must remain unchanged; add
coverage for the freshness-validation path. Keep the mirrored filtering aligned
in tools/llm_registry.py:186-188, and update
tools/check_model_registry_freshness.py to normalize evidence IDs by retaining
only non-empty strings after trimming, using the same contract as both loaders.
In `@tests/test_check_model_registry_freshness.py`:
- Around line 169-186: Strengthen both test_unknown_legacy_pin_is_advisory and
test_blocked_legacy_pin_is_advisory to verify advisory legacy pins produce no
selection_override finding. Add selection_override to the excluded finding
kinds, or assert that the fixture produces no findings while preserving the
existing assertions.
🪄 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: 48bafe58-18f0-40e1-b1c8-39ba83e46cfc
📒 Files selected for processing (6)
docs/contracts/agent-runner-output.mdtemplates/consumer-repo/tools/llm_registry.pytests/test_check_model_registry_freshness.pytests/tools/test_llm_registry_selection.pytools/check_model_registry_freshness.pytools/llm_registry.py
|
Addressed the active review findings in a654a4e: synchronized the consumer capability-ID contract, normalized freshness-gate evidence IDs to the runtime string-only contract, and strengthened advisory legacy-pin coverage. Validation: |
GitHub Actions
Inactive
Summary
Validation
Summary by CodeRabbit
capability:<lowercase-kebab-id>.