Reject a Case-Only Registry Name Mismatch Too - #919
Conversation
Real fix, a natural extension of the padded-name near-miss check (8de62f9): a registry entry named "fixture" when configure.sh resolves "Fixture" (the GitHub-canonical casing) would silently read as "no description declared" rather than flagging the likely typo - GitHub itself treats repo names case-insensitively, so a same-name- different-case entry is a data-entry mistake, not a different repo. Folded into the same near-miss check (normalize with .strip().casefold() on both sides) rather than a second parallel check. New test case. Declined (reasoning posted to PR #914): CodeRabbit's "preserve interpreter probe failures" - the identical finding already declined on PR #918 citing this same file's own gh_ok() precedent for discarding a capability probe's output; and "reject malformed repository entries" (a null entry in repos) - that belongs to spec/validate.py's whole-registry structural check, and an unrelated malformed entry degrading an unrelated repo's lookup to a safe "nothing declared" (not a wrong value) has a materially different, much wider blast radius than the specific near-miss cases already fixed.
|
Warning Review limit reached
Next review available in: 3 minutes Limit details: You’ve used all 10 included reviews currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesRepository name resolution
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change rejects case-only registry name mismatches while preserving the existing near-miss behavior, with automated checks passing; no actionable merge-blocking risk remains beyond normal review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoFail loud on case-only registry repo name near-misses
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
There was a problem hiding this comment.
🟢 Approval recommended
Only a minor docstring grammar nit was found; core behavior and test coverage look sound.
Pull request overview
Extends registry-description resolution to fail loud when a requested repo name differs from a registry entry only by letter case (in addition to existing whitespace near-miss handling), preventing silent “absent” behavior for GitHub-case-insensitive repo names.
Changes:
- Normalize candidate comparisons using
strip().casefold()to detect case-only near misses as ResolveError conditions. - Add a unit test covering case-only registry name mismatches.
File summaries
| File | Description |
|---|---|
| spec/resolve_description.py | Extends near-miss detection to include case-only mismatches and updates the corresponding error message/docstring text. |
| scripts/tests/test_resolve_description.py | Adds a regression test ensuring a case-only mismatch raises ResolveError. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab) |
…d 2) Reintroduced the exact typo already fixed once in an earlier commit when I rewrote this docstring to add the case-insensitivity note.
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, consistent with existing validation intent, and includes a direct regression test for the new mismatch class.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
Fixes a real gap from PR #914's fresh review round after #918 merged, plus declines two twins of findings already resolved on #918.
spec/resolve_description.py: extended the padded-name near-miss check (from Delegate configure.sh's Description Check to Python Too #918) to also catch a case-only mismatch (e.g., resolvingFixtureagainst a registry entry namedfixture) - GitHub treats repo names case-insensitively, so this is a data-entry typo, not a different repo. Same mechanism, now normalizing with.strip().casefold()on both sides. New test case.Declined (reasoning posted to #914):
gh_ok()precedent for discarding a capability probe's output.nullinrepos) - that'sspec/validate.py's whole-registry structural check; an unrelated malformed entry degrading to "nothing declared" for a different, valid repo's lookup is a safe outcome with a much wider blast radius than the specific near-miss cases already fixed.786 automated tests, prose lint, ruff, pyright, and mypy all pass.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests