Add Pre-Commit Hook to Validate Sub-CLAUDE.md Completeness#1978
Conversation
Introduces scripts/check_sub_claude_md.py as a validate-only pre-commit hook that checks every sub-CLAUDE.md file table mentions all .py files in its directory. This catches coverage gaps at commit time rather than CI time. - scripts/check_sub_claude_md.py: new hook script with SRC_EXPECTED (27 dirs) and TESTS_EXPECTED (19 dirs) lists mirroring the test suite's lists - .pre-commit-config.yaml: new check-sub-claude-md hook in local repo hooks - tests/docs/test_check_sub_claude_md_script.py: 9 unit/integration tests for check_coverage() and main(), plus list-sync guards against the test files - tests/infra/test_ci_dev_config.py: structural test verifying the hook is present in pre-commit config with pass_filenames: false - tests/docs/CLAUDE.md: added test_check_sub_claude_md_script.py entry Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: approved_with_comments
| SRC_ROOT = PROJECT_ROOT / "src" / "autoskillit" | ||
| TESTS_ROOT = PROJECT_ROOT / "tests" | ||
|
|
||
| SRC_EXPECTED = [ |
There was a problem hiding this comment.
[warning] arch: SRC_EXPECTED and TESTS_EXPECTED are static hardcoded lists. When a new package sub-directory with a CLAUDE.md is added, the script silently ignores it. The hook should dynamically discover all CLAUDE.md files under SRC_ROOT and TESTS_ROOT rather than relying on an opt-in list that must be manually maintained.
There was a problem hiding this comment.
Valid observation — flagged for design decision. Dynamic discovery vs. hardcoded opt-in list is an architectural choice requiring human decision: dynamic discovery would make the lists in TestExpectedListsSync sync tests obsolete and change the hook's scope significantly.
| spec = importlib.util.spec_from_file_location(test_module_name, test_file) | ||
| assert spec is not None and spec.loader is not None | ||
| test_mod = importlib.util.module_from_spec(spec) | ||
| spec.loader.exec_module(test_mod) |
There was a problem hiding this comment.
[warning] bugs: spec.loader.exec_module(test_mod) is called without guarding that spec.loader implements exec_module. The assertion on L110 only checks spec.loader is not None. A TypeError or AttributeError would surface as an opaque test failure rather than a clear message.
There was a problem hiding this comment.
Valid observation — flagged for design decision. spec_from_file_location for a .py file always returns SourceFileLoader which implements exec_module — near-zero risk in practice. Adding a hasattr guard would improve the error message but requires a design decision on failure behavior.
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit review: warning-only findings detected. See inline comments — no blocking changes required.
…ile coverage
- Missing CLAUDE.md paths now append a failure message instead of silently
continuing, preserving the coverage guarantee for directories that lost
their CLAUDE.md file.
- Regular .py file check changed from bare substring match to backtick-form
check (f"\`{name}\`") — consistent with the existing __init__.py check and
the actual table format used in all sub-CLAUDE.md files.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…edium - test_check_coverage_skips_nonexistent_claude_md renamed and updated to expect a failure message for missing CLAUDE.md (mirrors script fix). - Module pytestmark changed from small to medium: test_main_returns_zero_on_live_repo performs real filesystem I/O against the live repo tree, exceeding the small definition (no persistent I/O). - Fix pre-existing E501 line-length violations in docstrings and string literals. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Trecek
deleted the
add-pre-commit-hook-to-validate-sub-claude-md-completeness/1975
branch
## Summary Add a validate-only pre-commit hook (`scripts/check_sub_claude_md.py`) that checks every sub-CLAUDE.md file table mentions all `.py` files in its directory. This catches the gap at commit time (during `pre-commit run --all-files`) instead of at CI time, preventing the systematic 5+ CI round-trip failures observed since PR #1820. The script replicates the coverage logic from `test_sub_claude_md_covers_all_py_files` and `test_tests_sub_claude_md_covers_all_py_files`, using the same `EXPECTED_SUB_CLAUDE_MDS` lists. A new `.pre-commit-config.yaml` stanza triggers it on `.py` file changes under `src/autoskillit/` and `tests/`. ## Requirements - New script: `scripts/check_sub_claude_md.py` (validate-only, exits 1 with structured message on mismatch) - New stanza in `.pre-commit-config.yaml` triggered on `files: ^(tests/|src/autoskillit/).*\.py$` - Must check both `tests/<subdir>/CLAUDE.md` and `src/autoskillit/<subdir>/CLAUDE.md` file tables - Must use the same `EXPECTED_SUB_CLAUDE_MDS` lists as the test files (or derive from disk) - No auto-fix — the agent must manually add the row with a meaningful Purpose description - Pattern: `pass_filenames: false` (like existing `doc-counts` hook) ## Changed Files ### New (★): ★ scripts/check_sub_claude_md.py ★ tests/docs/test_check_sub_claude_md_script.py ### Modified (●): ● .pre-commit-config.yaml ● tests/docs/CLAUDE.md ● tests/infra/test_ci_dev_config.py Closes #1975 ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/impl-20260505-211945-173795/.autoskillit/temp/make-plan/add_pre_commit_hook_sub_claude_md_plan_2026-05-05_213000.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code) via AutoSkillit <!-- autoskillit:pipeline-signature steps=prepare_pr,run_arch_lenses,compose_pr,annotate_pr_diff,review_pr --> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 participant
Summary
Add a validate-only pre-commit hook (
scripts/check_sub_claude_md.py) that checks every sub-CLAUDE.md file table mentions all.pyfiles in its directory. This catches the gap at commit time (duringpre-commit run --all-files) instead of at CI time, preventing the systematic 5+ CI round-trip failures observed since PR #1820. The script replicates the coverage logic fromtest_sub_claude_md_covers_all_py_filesandtest_tests_sub_claude_md_covers_all_py_files, using the sameEXPECTED_SUB_CLAUDE_MDSlists. A new.pre-commit-config.yamlstanza triggers it on.pyfile changes undersrc/autoskillit/andtests/.Requirements
scripts/check_sub_claude_md.py(validate-only, exits 1 with structured message on mismatch).pre-commit-config.yamltriggered onfiles: ^(tests/|src/autoskillit/).*\.py$tests/<subdir>/CLAUDE.mdandsrc/autoskillit/<subdir>/CLAUDE.mdfile tablesEXPECTED_SUB_CLAUDE_MDSlists as the test files (or derive from disk)pass_filenames: false(like existingdoc-countshook)Changed Files
New (★):
★ scripts/check_sub_claude_md.py
★ tests/docs/test_check_sub_claude_md_script.py
Modified (●):
● .pre-commit-config.yaml
● tests/docs/CLAUDE.md
● tests/infra/test_ci_dev_config.py
Closes #1975
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/impl-20260505-211945-173795/.autoskillit/temp/make-plan/add_pre_commit_hook_sub_claude_md_plan_2026-05-05_213000.md🤖 Generated with Claude Code via AutoSkillit
Token Usage Summary
Token Efficiency