fix: case-insensitive file detection for README, Makefile, and ADR directories#415
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCase-insensitive discovery was added for README and ADR locations; README failure messages reference the discovered filename. ADR search prefers ChangesDocumentation discovery and ADR detection
Setup automation file recognition
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📉 Test Coverage Report
Coverage calculated from unit tests only |
![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 `@src/agentready/assessors/documentation.py`:
- Around line 403-412: The README selection is nondeterministic because
repository.path.iterdir() isn't ordered; update the readme_path lookup to
iterate over a deterministic ordering (e.g., sorted(repository.path.iterdir(),
key=lambda f: f.name.lower())) so that readme_names matching (the readme_names
set and readme_path resolution) picks a predictable file; mirror the approach
used by ArchitectureDecisionsAssessor by sorting before filtering for is_file()
and name.lower() in readme_names.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 919c6d9e-b6df-4f90-baf4-d50cd956b843
📒 Files selected for processing (2)
src/agentready/assessors/documentation.pysrc/agentready/assessors/structure.py
jeremyeder
force-pushed
the
fix/case-sensitive-lookups-and-gnumakefile
branch
from
5c03fd6 to
77efebe
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/agentready/assessors/documentation.py (1)
405-412:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake README variant selection deterministic.
When multiple README variants exist, plain
iterdir()order is filesystem-dependent, so selected file can vary run-to-run. Sort candidates beforenext(...).🤖 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 `@src/agentready/assessors/documentation.py` around lines 405 - 412, The README selection using next(...) over repository.path.iterdir() is non-deterministic; instead collect matching candidates (filtering with readme_names), sort them (e.g., by name or another consistent key), and then pick the first element to assign readme_path. Update the logic around the readme_path assignment that currently uses repository.path.iterdir() so it builds a sorted list of files before selecting the candidate.
🤖 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 `@src/agentready/assessors/documentation.py`:
- Around line 405-412: The README discovery can raise OSError from
repository.path.iterdir() and must be handled instead of crashing assess(); wrap
the iteration that assigns readme_path (the next(...) over
repository.path.iterdir()) in a try/except OSError block inside assess(), and on
exception return a Finding.error(...) (using the Finding class already used in
assess) that includes a clear message and the exception details; ensure you
still return None or proceed normally when no error occurs so subsequent logic
using readme_path behaves as before.
- Around line 573-589: The iterdir calls over docs_dir and repository.path can
raise OSError and must be guarded; wrap each sorted(...iterdir()) usage (the
loop that searches for adr_dir using docs_dir.iterdir() and the fallback loop
using repository.path.iterdir()) in try/except OSError blocks (or extract a
small helper like safe_iterdir) and on exception do not raise—record/return an
appropriate skipped/error Finding and leave adr_dir unset so the assessor
continues gracefully; reference the variables docs_dir, repository.path,
adr_dir, and adr_target_names when applying the fix.
---
Duplicate comments:
In `@src/agentready/assessors/documentation.py`:
- Around line 405-412: The README selection using next(...) over
repository.path.iterdir() is non-deterministic; instead collect matching
candidates (filtering with readme_names), sort them (e.g., by name or another
consistent key), and then pick the first element to assign readme_path. Update
the logic around the readme_path assignment that currently uses
repository.path.iterdir() so it builds a sorted list of files before selecting
the candidate.
🪄 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: Enterprise
Run ID: 89da69e2-9a8e-47ca-a068-292525d5276a
📒 Files selected for processing (2)
src/agentready/assessors/documentation.pysrc/agentready/assessors/structure.py
| for candidate in sorted(docs_dir.iterdir()): | ||
| if candidate.is_dir() and candidate.name.lower() in adr_target_names: | ||
| adr_dir = candidate | ||
| break | ||
|
|
||
| # Fall back to repo root and hidden .adr | ||
| if not adr_dir: | ||
| if (repository.path / ".adr").is_dir(): | ||
| adr_dir = repository.path / ".adr" | ||
| else: | ||
| for candidate in sorted(repository.path.iterdir()): | ||
| if ( | ||
| candidate.is_dir() | ||
| and candidate.name.lower() in adr_target_names | ||
| ): | ||
| adr_dir = candidate | ||
| break |
There was a problem hiding this comment.
Guard ADR directory scans against OSError.
Both sorted(docs_dir.iterdir()) and sorted(repository.path.iterdir()) may raise OSError; this would crash the assessor before it can emit a fail/error finding.
Suggested fix
- if docs_dir.is_dir():
- for candidate in sorted(docs_dir.iterdir()):
- if candidate.is_dir() and candidate.name.lower() in adr_target_names:
- adr_dir = candidate
- break
+ if docs_dir.is_dir():
+ try:
+ for candidate in sorted(docs_dir.iterdir()):
+ if candidate.is_dir() and candidate.name.lower() in adr_target_names:
+ adr_dir = candidate
+ break
+ except OSError as e:
+ return Finding.error(
+ self.attribute, reason=f"Could not scan docs/ for ADR directories: {e}"
+ )
@@
- for candidate in sorted(repository.path.iterdir()):
- if (
- candidate.is_dir()
- and candidate.name.lower() in adr_target_names
- ):
- adr_dir = candidate
- break
+ try:
+ for candidate in sorted(repository.path.iterdir()):
+ if (
+ candidate.is_dir()
+ and candidate.name.lower() in adr_target_names
+ ):
+ adr_dir = candidate
+ break
+ except OSError as e:
+ return Finding.error(
+ self.attribute,
+ reason=f"Could not scan repository root for ADR directories: {e}",
+ )As per coding guidelines "Check for proper error handling (return skipped/error Finding, don't crash)."
🤖 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 `@src/agentready/assessors/documentation.py` around lines 573 - 589, The
iterdir calls over docs_dir and repository.path can raise OSError and must be
guarded; wrap each sorted(...iterdir()) usage (the loop that searches for
adr_dir using docs_dir.iterdir() and the fallback loop using
repository.path.iterdir()) in try/except OSError blocks (or extract a small
helper like safe_iterdir) and on exception do not raise—record/return an
appropriate skipped/error Finding and leave adr_dir unset so the assessor
continues gracefully; reference the variables docs_dir, repository.path,
adr_dir, and adr_target_names when applying the fix.
77efebe to
8440e83
Compare
jwm4
force-pushed
the
fix/case-sensitive-lookups-and-gnumakefile
branch
from
8440e83 to
0461eb2
Compare
- READMEAssessor: find README.md/rst/txt case-insensitively via iterdir() instead of hardcoded path (closes #387) - OneCommandSetupAssessor: recognize GNUmakefile and makefile in addition to Makefile (closes #380) - ArchitectureDecisionsAssessor: scan docs/ and repo root for ADR directories case-insensitively (closes #379) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jwm4
force-pushed
the
fix/case-sensitive-lookups-and-gnumakefile
branch
from
0461eb2 to
960e005
Compare
- README scan: sort candidates before next() so selection is deterministic when multiple variants exist (e.g. readme.md and README.rst both present) - README scan: wrap iterdir() in try/except OSError, return Finding.error instead of crashing - ADR scan: wrap both docs/ and root iterdir() calls in try/except OSError, fall through gracefully rather than propagating Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jwm4
force-pushed
the
fix/case-sensitive-lookups-and-gnumakefile
branch
from
91aa5a4 to
fd2be32
Compare
Real-World ValidationTested this PR against 7 repositories to verify the three fixes. Here are the results. README case-insensitive detection (#387)renovatebot/renovate ( GNUmakefile detection (#380)performancecopilot/pcp ( ADR directory scanning, including
|
| Repo | Directory matched | Result | Verdict |
|---|---|---|---|
| performancecopilot/pcp | docs/specs |
Pass (score 40+) | False positive - contains formal API/interface specs, not decision records |
| devcontainers/spec | docs/specs |
Pass (score 82) | Arguable - 18 formal spec .md files, low template compliance |
| liatrio-labs/spec-driven-workflow | docs/specs |
Fail (score 40) | False negative - genuine ADR-style content exists, but in numbered subdirectories (01-spec-.../) that the assessor does not recurse into |
| github/spec-kit | (none matched) | Fail (score 0) | True negative - no ADR directory present, correct result |
| bdfinst/agentic-dev-team | docs/specs |
Pass (score 83) | True positive - 5 design/decision records for agentic components (e.g. beads-workflow-plugin.md, subagent-report-write-contract.md) |
Assessment of specs as a directory name: there is genuine signal here. bdfinst/agentic-dev-team is using docs/specs exactly as intended - to give agents background on why the system is the way it is. The pcp case is a real false positive, but it reflects a fundamental ambiguity in the name specs rather than a flaw in the detection logic. Other names on the list (e.g. decisions, architecture-decisions) carry the same risk in different repos. On balance, keeping specs in the list seems reasonable.
Separate issue - subdirectory depth: the liatrio-labs false negative is not specific to specs. The assessor only counts .md files directly inside the matched directory, so any repo that organizes its ADRs into numbered subdirectories will score zero regardless of which directory name is used. Filed as #423 for follow-up.
Summary
All three fixes work as intended on real repos. No regressions observed. The specs addition produces mixed results, but the true positive case (bdfinst) justifies keeping it. The subdirectory scanning limitation is a pre-existing gap tracked in #423.
Reviewed with Claude Code under the supervision of Bill Murdock.
|
🎉 This PR is included in version 2.36.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
readme.md,README.MD,README.rst, etc. via case-insensitiveiterdir()scan instead of hardcodedREADME.mdpathGNUmakefileandmakefilealongsideMakefilein one-command setup detectiondocs/and repo root for ADR directories case-insensitively, coveringADRs/,Adr/,Adrs/,Decisions/, etc.Note: issue #373 (actionlint silent failure) was already resolved in #409.
Test plan
renovatebot/renovate(lowercasereadme.md) — should pass README checkperformancecopilot/pcp(GNUmakefile) — should pass one-command setupdocs/ADRs/— should pass architecture decisions checkCloses #387, #380, #379
🤖 Generated with Claude Code under the supervision of Bill Murdock
Summary by CodeRabbit