feat: context file assessor improvements (ADR A.1, A.9, A.4)#477
Conversation
Add context file length validation, agent access documentation detection, and skill depth tiering to improve how AgentReady evaluates context file quality per the accepted ADR alignment with wg-agentic-sdlc best practices. CLAUDEmdAssessor: two-phase scoring (70 presence + 30 length) with thresholds at 150/300 lines, agent access section as substantiating evidence. PatternReferencesAssessor: tiered skill scoring (1-2: 30pts, 3+: 60pts) with context file length correlation warning. Closes #459 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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 (5)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughImplements ADR A.1/A.9/A.4: renames CLAUDEmdAssessor → AgentInstructionsAssessor, refactors assess() into presence + length-tier scoring, adds non-blocking agent-access evidence, tiers PatternReferencesAssessor skill credit (1–2 partial, 3+ full), and updates docs, tests, weights, fixers, and CLI wiring. ChangesADR A.1, A.9, A.4: Context file assessor improvements
Sequence Diagram: assess flowsequenceDiagram
participant CLI as CLI/Caller
participant Assessor as AgentInstructionsAssessor
participant Resolver as DocumentationResolver
participant AccessChk as _check_agent_access
CLI->>Assessor: assess(repository)
Assessor->>Resolver: resolve CLAUDE.md/.claude/AGENTS.md (follow symlinks/@ refs)
Resolver-->>Assessor: content or missing
Assessor->>Assessor: Phase1 presence gate (>=50 bytes)
Assessor->>Assessor: Phase2 length-tier scoring (<=150/<=300/>300)
Assessor->>AccessChk: detect agent access evidence
AccessChk-->>Assessor: evidence (non-blocking)
Assessor-->>CLI: Finding(score,evidence)
Possibly related PRs
Suggested labels
🚥 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
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 |
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 `@src/agentready/assessors/documentation.py`:
- Around line 78-83: When resolving and opening symlinked files (e.g.,
claude_md_path -> resolved_path), restrict reads to inside the repository root:
after computing resolved_path = claude_md_path.resolve(strict=True), verify the
target is under repository.path (use repository.path.resolve() and either
Path.is_relative_to(resolved_path) or compare pathlib.Path(commonpath(...)) )
and skip or raise if it is outside; apply the same guard around the other
file-open block that reads the alternate path (the block at lines 91-97) so
neither symlink target nor resolved files outside repository.path are read.
In `@src/agentready/assessors/patterns.py`:
- Around line 106-122: The loop that checks CLAUDE.md and AGENTS.md stops after
the first existing file because of the unconditional break; update the logic in
the block that handles the no-skills length check (variables: has_skills,
ctx_name, ctx_path, ctx_lines, evidence) to remove the break so both files are
evaluated (so a short CLAUDE.md does not suppress a long AGENTS.md); ensure you
still catch OSError/UnicodeDecodeError as before and only skip that file on
exception, allowing the loop to continue to the next ctx_name.
🪄 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: 0e869854-bb8c-4e33-9d76-e7e82766f27b
📒 Files selected for processing (6)
CLAUDE.mddocs/attributes.mdsrc/agentready/assessors/documentation.pysrc/agentready/assessors/patterns.pytests/unit/test_assessors_documentation.pytests/unit/test_assessors_patterns.py
The assessor now covers both CLAUDE.md and AGENTS.md equally, so the CLAUDE-specific name was misleading. Rename the class, display name, and attribute ID across all source, tests, configs, and docs. - Class: CLAUDEmdAssessor -> AgentInstructionsAssessor - Display name: "CLAUDE.md Configuration Files" -> "Agent Instruction Files" - Attribute ID: claude_md_file -> agent_instructions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/agentready/cli/align.py (1)
149-156:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate the Claude CLI tip to missing-file failures only.
Line 152 now shows the Claude CLI /
ANTHROPIC_API_KEYtip for anyagent_instructionsfailure. With the new length-based scoring, this attribute can fail even when instruction files exist, so this message can point users to the wrong remediation.Suggested fix
- if "agent_instructions" in failing_ids: + has_instruction_file = any( + (repo_path / rel_path).exists() + for rel_path in ("CLAUDE.md", "AGENTS.md", ".claude/CLAUDE.md") + ) + if "agent_instructions" in failing_ids and not has_instruction_file: click.echo( "\n💡 Tip: Install the Claude CLI and set ANTHROPIC_API_KEY to " "enable automatic CLAUDE.md generation." )🤖 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/cli/align.py` around lines 149 - 156, The tip is currently shown whenever "agent_instructions" is in failing_ids; change this so you only show the Claude CLI / ANTHROPIC_API_KEY tip when the agent_instructions failure is due to a missing instruction file. Replace the simple membership check with a targeted check over assessment.findings (e.g. find any f where f.attribute.id == "agent_instructions" and f.status == "fail" and the failure indicates a missing file — check f.message, f.reason or f.metadata for keywords like "missing" / "not found" or a missing_files entry); only call click.echo(...) when that missing-file condition is true. Ensure you still reference the same symbols: failing_ids, assessment.findings, and the click.echo block for the tip.docs/api-reference.md (1)
111-117:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the example description to match current assessor behavior.
The example now uses
agent_instructions, but the description still says “CLAUDE.md file at repository root,” which no longer reflects AGENTS.md and.claude/CLAUDE.mdsupport.Suggested doc tweak
- description="CLAUDE.md file at repository root", + description="Agent instruction file (CLAUDE.md, AGENTS.md, or .claude/CLAUDE.md)",🤖 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 `@docs/api-reference.md` around lines 111 - 117, The entry with id="agent_instructions" has an outdated description mentioning only “CLAUDE.md file at repository root”; update the description to reflect current assessor behavior by listing supported instruction files (e.g., AGENTS.md and .claude/CLAUDE.md as well as CLAUDE.md) and adjust the rationale or description text in the same block (id="agent_instructions", name="Agent Instruction Files") so it accurately documents all supported locations and filenames.
♻️ Duplicate comments (1)
src/agentready/assessors/documentation.py (1)
77-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlock out-of-repo symlink targets before any file read.
CLAUDE.md/ referenced files are opened after resolution without enforcing repository boundaries. A crafted symlink can make the assessor read host files outsiderepository.path, affecting score/evidence and expanding filesystem read scope.Suggested hardening pattern
- with open(resolved_path, "r", encoding="utf-8") as f: + repo_root = repository.path.resolve() + if not resolved_path.is_relative_to(repo_root): + raise FileNotFoundError("Resolved path outside repository") + with open(resolved_path, "r", encoding="utf-8") as f: file_content = f.read()- def _read_referenced_file(self, file_path: Path) -> tuple[str | None, int]: + def _read_referenced_file( + self, file_path: Path, repository_root: Path + ) -> tuple[str | None, int]: """Read a referenced file and return its content and size. Returns (content, size) tuple, or (None, 0) if file doesn't exist or can't be read. """ try: - with open(file_path, "r", encoding="utf-8") as f: - content = f.read() + resolved = file_path.resolve(strict=True) + if not resolved.is_relative_to(repository_root.resolve()): + return None, 0 + with open(resolved, "r", encoding="utf-8") as f: + content = f.read() return content, len(content) except (FileNotFoundError, OSError, UnicodeDecodeError): return None, 0Also applies to: 242-252
🤖 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 77 - 83, The code opens claude_md_path after resolving symlinks without ensuring the resolved target lives inside the repository; change the flow in the block that uses claude_md_path (and the similar block at the other occurrence) to first detect symlinks via claude_md_path.is_symlink(), then resolve the target (claude_md_path.resolve(strict=True)) and immediately validate that resolved_path is inside repository.path (use resolved_path.is_relative_to(repository.path) or compare resolved_path.resolve().parents against repository.path.resolve()); if the resolved target is outside the repo, log/ignore/raise and do not open the file. Ensure the same check is applied before any open(...) call in both locations.
🤖 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.
Outside diff comments:
In `@docs/api-reference.md`:
- Around line 111-117: The entry with id="agent_instructions" has an outdated
description mentioning only “CLAUDE.md file at repository root”; update the
description to reflect current assessor behavior by listing supported
instruction files (e.g., AGENTS.md and .claude/CLAUDE.md as well as CLAUDE.md)
and adjust the rationale or description text in the same block
(id="agent_instructions", name="Agent Instruction Files") so it accurately
documents all supported locations and filenames.
In `@src/agentready/cli/align.py`:
- Around line 149-156: The tip is currently shown whenever "agent_instructions"
is in failing_ids; change this so you only show the Claude CLI /
ANTHROPIC_API_KEY tip when the agent_instructions failure is due to a missing
instruction file. Replace the simple membership check with a targeted check over
assessment.findings (e.g. find any f where f.attribute.id ==
"agent_instructions" and f.status == "fail" and the failure indicates a missing
file — check f.message, f.reason or f.metadata for keywords like "missing" /
"not found" or a missing_files entry); only call click.echo(...) when that
missing-file condition is true. Ensure you still reference the same symbols:
failing_ids, assessment.findings, and the click.echo block for the tip.
---
Duplicate comments:
In `@src/agentready/assessors/documentation.py`:
- Around line 77-83: The code opens claude_md_path after resolving symlinks
without ensuring the resolved target lives inside the repository; change the
flow in the block that uses claude_md_path (and the similar block at the other
occurrence) to first detect symlinks via claude_md_path.is_symlink(), then
resolve the target (claude_md_path.resolve(strict=True)) and immediately
validate that resolved_path is inside repository.path (use
resolved_path.is_relative_to(repository.path) or compare
resolved_path.resolve().parents against repository.path.resolve()); if the
resolved target is outside the repo, log/ignore/raise and do not open the file.
Ensure the same check is applied before any open(...) call in both locations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 5276dfa4-1be0-4196-a2f1-48f9d87dbce9
📒 Files selected for processing (30)
.agentready-config.example.yamlAGENTS.mdCLAUDE.mdREADME.mddocs/api-reference.mddocs/attributes.mddocs/developer-guide.mddocs/user-guide.mdexperiments/configs/claude-md.yamlexperiments/configs/tier1.yamlscripts/generate_slides.pysrc/agentready/assessors/__init__.pysrc/agentready/assessors/base.pysrc/agentready/assessors/documentation.pysrc/agentready/cli/align.pysrc/agentready/data/.agentready-config.example.yamlsrc/agentready/data/default-weights.yamlsrc/agentready/fixers/base.pysrc/agentready/fixers/documentation.pysrc/agentready/github/review_formatter.pysrc/agentready/models/attribute.pysrc/agentready/models/eval_harness.pysrc/agentready/services/llm_cache.pytests/e2e/test_critical_paths.pytests/integration/test_scan_workflow.pytests/unit/cli/test_main.pytests/unit/test_assessors_documentation.pytests/unit/test_batch_assessment.pytests/unit/test_cli_align.pytests/unit/test_fixers.py
jwm4
left a comment
There was a problem hiding this comment.
Review from Bill Murdock (with assistance from Claude Code)
Code Review
The core logic is correct and well-tested. A few things to address:
Should fix before merge:
-
Drop AGENTS.md from this PR. It's a near-copy of CLAUDE.md (only differences: stale self-score "80.0/100 Gold" vs the correct "74.5/100 Silver", and a minor line reference). Adding AGENTS.md for cross-tool compatibility is a good idea, but it's outside the scope of the assessor logic changes and should be its own PR where it can get a proper review pass.
-
docs/attributes.md heading (line 53): Section heading still reads "### 1. CLAUDE.md Configuration File" but the ID and display name were renamed to
agent_instructions/ "Agent Instruction Files".
Optional (quick fixes):
- Unconditional
breakin patterns.py length check (line 122): The context-file-length correlation loop breaks after the first existing file. A short CLAUDE.md suppresses checking a potentially long AGENTS.md. CodeRabbit flagged this too. Removing thebreakis a one-line fix.
Fine as follow-ups:
- Symlink traversal outside repo boundary (pre-existing, low risk)
base_scorevariable naming could be clearer (nit)
Real-repo validation
Ran the PR branch's assessor against four repositories to validate scoring behavior:
| Repo | agent_instructions | Score | Key evidence |
|---|---|---|---|
| wasp-lang/open-saas | FAIL | 0 | No CLAUDE.md or AGENTS.md |
| cloudflare/chanfana | PASS | 100 | AGENTS.md only, 137 lines (full length credit) |
| atopile/atopile | PASS | 100 | CLAUDE.md 76 lines + AGENTS.md symlink + 21 skills |
| ambient-code/agentready | PASS | 100 | CLAUDE.md 136 lines + AGENTS.md, 74.5 overall (Silver) |
Observations:
- Rename from
claude_md_filetoagent_instructionsworks cleanly across all repos - Length validation correctly awards full credit for files <=150 lines (unit tests cover the 151-300 and >300 tiers)
- AGENTS.md-only path scores correctly (chanfana: 100, no CLAUDE.md present)
- Skill tiering works: atopile's 21 SKILL.md files earn full 60-point credit in
pattern_references(score 80 = 60 skills + 20 examples) - No false positives on agent access detection
- Self-score confirmed at 74.5 Silver, matching PR description
- Drop AGENTS.md from PR (should be its own PR) - Fix docs/attributes.md heading to match renamed attribute - Remove unconditional break in patterns.py length correlation loop so both CLAUDE.md and AGENTS.md are checked - Gate align CLI tip to missing-file failures only (not length failures) - Fix docs/api-reference.md example description for renamed attribute Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jwm4
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround. Three of the four fixes look good:
- docs/attributes.md heading: fixed
- patterns.py
breakremoved: fixed - align.py tip gated to missing-file only: nice bonus, good catch
- docs/api-reference.md description updated: good catch
One issue: AGENTS.md is still in the diff. The commit message says "Drop AGENTS.md from PR" but the file is still present on the branch (4895 bytes, doesn't exist on main). Looks like the git rm didn't happen.
Review from Bill Murdock with assistance from Claude Code.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Actionable comments posted: 0 |
|
🎉 This PR is included in version 2.43.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
claude_md_file->agent_instructions) since the assessor now covers both CLAUDE.md and AGENTS.md equallyImplements Proposals A.1, A.9, and A.4 from the accepted ADR. Second of six implementation PRs for the ADR (first was #464).
Self-score change: 80.0 -> 74.5 Silver (unchanged from #464's rebalancing; length validation has no impact since this repo's CLAUDE.md is 136 lines).
Breaking change: attribute ID renamed from
claude_md_filetoagent_instructions. Any custom weight configs referencing the old ID will need updating.Related issues
Test plan
black . && isort . && ruff check .passespytest tests/unit/passes (1089 passed, 17 skipped)agentready assess .runs successfully (74.5/100 Silver)grep -rnconfirms zero stale references toCLAUDEmdAssessororclaude_md_filein source, tests, and configsCloses #459
Posted by Bill Murdock with assistance from Claude Code.
Summary by CodeRabbit
New Features
Documentation
Tests