From 076e41efa358c3151748ad431cf0b75d86756274 Mon Sep 17 00:00:00 2001 From: Anthony Johnson II Date: Wed, 17 Jun 2026 15:46:57 -0700 Subject: [PATCH] refactor(hooks): centralize thresholds and drop dead code - Add MIN_DOCUMENT_WORDS, MAX_PARAGRAPH_WORDS, DEFAULT_PUBLISH_THRESHOLD and get_publish_threshold() (reads grades.B.min) to hook_utils.py. - doc_post_review.py: derive the score-only publish threshold from quality-gates.json instead of a hardcoded 80. - doc_post_write.py: use the shared paragraph threshold and remove the dead seen_lines set (line indices are already unique per pass). - doc_pre_write.py: drop the unreachable non-fenced Protocol ellipsis handler (valid examples belong in fenced blocks) and hoist content.lower() out of the forbidden-pattern loop. - smoke.sh: add a regression asserting post-review honors the configured Grade B threshold. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/hooks/doc_post_review.py | 10 +++---- .claude/hooks/doc_post_write.py | 7 ++--- .claude/hooks/doc_pre_write.py | 48 ++++---------------------------- .claude/hooks/hook_utils.py | 21 +++++++++++++- tests/smoke.sh | 39 +++++++++++++++++++++++++- 5 files changed, 69 insertions(+), 56 deletions(-) diff --git a/.claude/hooks/doc_post_review.py b/.claude/hooks/doc_post_review.py index 4065b2f..8a536d8 100644 --- a/.claude/hooks/doc_post_review.py +++ b/.claude/hooks/doc_post_review.py @@ -21,10 +21,7 @@ sys.path.insert(0, str(Path(__file__).parent)) -from hook_utils import get_tool_input # noqa: E402 - - -PUBLISH_THRESHOLD = 80 +from hook_utils import get_project_dir, get_publish_threshold, get_tool_input # noqa: E402 def get_tool_result() -> str: @@ -81,17 +78,18 @@ def extract_bool(name: str): results["passed"] = extract_bool("passed") results["ready_for_publish"] = extract_bool("ready_for_publish") + publish_threshold = get_publish_threshold(get_project_dir()) if results["passed"] is None: results["passed"] = results["grade"] in ["A", "B"] or ( - results["score"] is not None and results["score"] >= PUBLISH_THRESHOLD + results["score"] is not None and results["score"] >= publish_threshold ) if results["ready_for_publish"] is None: results["ready_for_publish"] = results["grade"] in ["A", "B"] or ( results["grade"] is None and results["score"] is not None - and results["score"] >= PUBLISH_THRESHOLD + and results["score"] >= publish_threshold ) # Try to extract document path diff --git a/.claude/hooks/doc_post_write.py b/.claude/hooks/doc_post_write.py index b740335..0280bf5 100644 --- a/.claude/hooks/doc_post_write.py +++ b/.claude/hooks/doc_post_write.py @@ -21,6 +21,7 @@ sys.path.insert(0, str(Path(__file__).parent)) from hook_utils import ( # noqa: E402 + MAX_PARAGRAPH_WORDS, get_project_dir, get_tool_input, get_tool_result, @@ -74,12 +75,8 @@ def check_terminology(content: str, rules: dict) -> list: for correct_term, forbidden_variants in terminology.items(): for variant in forbidden_variants: pattern = re.compile(r'\b' + re.escape(variant) + r'\b', re.IGNORECASE) - seen_lines = set() for i, line in enumerate(lines, 1): if pattern.search(line): - if i in seen_lines: - continue - seen_lines.add(i) snippet = line.strip() if len(snippet) > 80: snippet = snippet[:77] + "..." @@ -148,7 +145,7 @@ def check_document_consistency(file_path: str, content: str) -> dict: paragraphs = content.split('\n\n') for i, para in enumerate(paragraphs, 1): word_count = len(para.split()) - if word_count > 200 and not para.strip().startswith('```'): + if word_count > MAX_PARAGRAPH_WORDS and not para.strip().startswith('```'): suggestions.append( f"Paragraph {i} has {word_count} words - consider breaking up for readability" ) diff --git a/.claude/hooks/doc_pre_write.py b/.claude/hooks/doc_pre_write.py index 2457cbb..a59c421 100644 --- a/.claude/hooks/doc_pre_write.py +++ b/.claude/hooks/doc_pre_write.py @@ -22,6 +22,7 @@ sys.path.insert(0, str(Path(__file__).parent)) from hook_utils import ( # noqa: E402 + MIN_DOCUMENT_WORDS, format_blocking_feedback, format_warning_feedback, get_project_dir, @@ -40,10 +41,10 @@ def check_forbidden_patterns(content: str, rules: dict) -> list: issues = [] forbidden = rules.get("forbidden_patterns", {}).get("patterns", []) word_boundary_patterns = {"foo", "bar", "baz"} + content_lower = content.lower() for pattern in forbidden: pattern_lower = pattern.lower() - content_lower = content.lower() if pattern_lower in word_boundary_patterns: regex = r'\b' + re.escape(pattern_lower) + r'\b' @@ -95,49 +96,12 @@ def _is_inside_spans(pos: int, spans: list) -> bool: return False -def is_valid_protocol_ellipsis(content: str, ellipsis_pos: int) -> bool: - """Check if an ellipsis at the given position is valid Protocol/ABC syntax. - - Valid PEP 544 ellipsis must: - 1. Appear as a standalone statement (just '...' on its own line) - 2. Be preceded by 'def' within the recent lookback window - 3. Be inside a class that inherits from Protocol or ABC - """ - lines = content[:ellipsis_pos].split('\n') - if not lines: - return False - - current_line_start = content.rfind('\n', 0, ellipsis_pos) + 1 - current_line_end = content.find('\n', ellipsis_pos) - if current_line_end == -1: - current_line_end = len(content) - current_line = content[current_line_start:current_line_end].strip() - - if current_line != '...': - return False - - lookback_lines = lines[-10:] if len(lines) >= 10 else lines - lookback_text = '\n'.join(lookback_lines) - - has_def = bool(re.search(r'\bdef\s+\w+', lookback_text)) - if not has_def: - return False - - has_protocol_context = bool(re.search( - r'class\s+\w+\s*\([^)]*\b(Protocol|ABC)\b[^)]*\)', - lookback_text - )) - - return has_protocol_context - - def check_ellipsis_patterns(content: str) -> list: """Check for ellipsis patterns that indicate incomplete content. Ellipsis is allowed inside fenced code blocks (comments, spread operators, - range syntax, Protocol bodies). In prose, ellipsis is also allowed when - immediately preceded by a sentence-ending word (e.g., "...wait, what?") - but flagged when it appears to indicate omitted content. + range syntax, Protocol bodies). Non-fenced ellipsis is flagged because it + may indicate omitted content. """ issues = [] code_spans = _fenced_code_spans(content) @@ -145,8 +109,6 @@ def check_ellipsis_patterns(content: str) -> list: pos = match.start() if _is_inside_spans(pos, code_spans): continue - if is_valid_protocol_ellipsis(content, pos): - continue line_num = content[:pos].count('\n') + 1 issues.append(f"Line {line_num}: Ellipsis '...' detected - may indicate incomplete content") return issues @@ -192,7 +154,7 @@ def validate_documentation_write(file_path: str, content: str) -> dict: warnings.extend(check_code_blocks(content)) word_count = len(content.split()) - if word_count < 50: + if word_count < MIN_DOCUMENT_WORDS: warnings.append(f"Document has only {word_count} words - seems incomplete") return { diff --git a/.claude/hooks/hook_utils.py b/.claude/hooks/hook_utils.py index 517e2d4..766cb7e 100644 --- a/.claude/hooks/hook_utils.py +++ b/.claude/hooks/hook_utils.py @@ -17,6 +17,9 @@ VALID_DOC_PATHS = ["/spec_driven_docs/", "/app_docs/"] EXCLUDED_DOC_PATHS = ["/.claude/docs/", "/specs/docs/"] +MIN_DOCUMENT_WORDS = 50 +MAX_PARAGRAPH_WORDS = 200 +DEFAULT_PUBLISH_THRESHOLD = 80 def get_tool_input() -> dict: @@ -67,6 +70,22 @@ def load_quality_profiles(project_dir: Path) -> dict: return {} +def get_publish_threshold(project_dir: Path) -> int: + """Load the Grade B minimum score from quality-gates.json.""" + gates_path = project_dir / ".claude" / "docs" / "config" / "quality-gates.json" + if gates_path.exists(): + try: + with open(gates_path) as f: + data = json.load(f) + return int(data.get("grades", {}).get("B", {}).get( + "min", + DEFAULT_PUBLISH_THRESHOLD, + )) + except (json.JSONDecodeError, IOError, TypeError, ValueError): + pass + return DEFAULT_PUBLISH_THRESHOLD + + def is_documentation_file(file_path: str) -> bool: """Return True if file_path is a markdown file under a tracked docs directory.""" if not file_path or not file_path.endswith(".md"): @@ -81,7 +100,7 @@ def is_documentation_file(file_path: str) -> bool: _FIX_HINTS = [ ("Forbidden pattern", "Remove or replace this placeholder before saving."), ("Placeholder content", "Replace with finalized content; placeholders block writes."), - ("Ellipsis", "Replace with real content, or verify this is a Protocol/ABC method body."), + ("Ellipsis", "Replace with real content, or move valid code examples into a fenced block."), ("Code block missing language hint", "Add a language hint, e.g. ```python or ```bash."), ("Terminology", "Use the project's preferred term per consistency-rules.json."), ("Broken link", "Fix the path or remove the link."), diff --git a/tests/smoke.sh b/tests/smoke.sh index d593ed8..cc5fce0 100755 --- a/tests/smoke.sh +++ b/tests/smoke.sh @@ -557,9 +557,10 @@ fi run_post_review_hook() { local command_str="$1" local result_text="$2" + local project_dir="${3:-$FRAMEWORK_ROOT}" CLAUDE_TOOL_INPUT=$(python3 -c "import json,sys; print(json.dumps({'command': sys.argv[1]}))" "$command_str") \ CLAUDE_TOOL_RESULT="$result_text" \ - CLAUDE_PROJECT_DIR="$FRAMEWORK_ROOT" \ + CLAUDE_PROJECT_DIR="$project_dir" \ python3 .claude/hooks/doc_post_review.py } @@ -691,6 +692,42 @@ else fail "post-review-hook: derives readiness from JSON score" "$output" fi +# Test 8e: post-review score-only readiness uses quality-gates.json Grade B threshold. +tmp_project=$(mktemp -d) +mkdir -p "$tmp_project/.claude/docs/config" +python3 - "$FRAMEWORK_ROOT/.claude/docs/config/quality-gates.json" \ + "$tmp_project/.claude/docs/config/quality-gates.json" <<'PY' +import json +import sys + +source, target = sys.argv[1], sys.argv[2] +with open(source) as f: + data = json.load(f) +data["grades"]["B"]["min"] = 86 +with open(target, "w") as f: + json.dump(data, f) +PY +output=$(run_post_review_hook \ + "/doc-review spec_driven_docs/rough_draft/api/users.md" \ + '{"score": 85}' \ + "$tmp_project") +rm -rf "$tmp_project" +if [ -z "$output" ]; then + pass "post-review-hook: uses configured publish threshold" +elif echo "$output" | python3 -c " +import json, sys +try: + d = json.load(sys.stdin) + fb = d.get('feedback', '').lower() + sys.exit(0 if 'promote' not in fb and 'pending_approval' not in fb else 1) +except Exception: + sys.exit(1) +" 2>/dev/null; then + pass "post-review-hook: uses configured publish threshold" +else + fail "post-review-hook: uses configured publish threshold" "$output" +fi + # Test 9: post-review hook is silent for non-review commands output=$(run_post_review_hook \ "/doc-write specs/docs/api-spec.md" \