-
Notifications
You must be signed in to change notification settings - Fork 7
feat: scheduled full-repo documentation drift audit #72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| """Scheduled full-repo documentation drift audit. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] module-docstring-format Module docstring does not follow the codebase pattern of structured 'This module handles:' inventory used by peer modules. |
||
|
|
||
| Walks the docs tree using folder indexes, asks the LLM to assess each | ||
| doc's staleness, and reports findings grouped by severity. | ||
| """ | ||
|
|
||
| import json | ||
| import os | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] import-organization Imports lack section comments (# Import configuration, etc.) used in peer modules like discovery.py and generation.py. |
||
| from config import get_client, get_max_context_chars, get_model_name, truncate_content | ||
| from doc_index import get_doc_folders, get_docs_in_folder, get_docs_root, load_manifest | ||
| from security_utils import run_command_safe, sanitize_output | ||
|
|
||
|
|
||
| def run_audit(max_files=20): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] input-validation max_files parameter has no bounds validation. Zero or negative values silently skip all files; very large values could cause excessive LLM API calls. Suggested fix: Clamp max_files to a reasonable range (e.g. 1-500). |
||
| """Audit documentation for drift against the current codebase. | ||
|
|
||
| Returns a list of {file, severity, reason} dicts. | ||
| """ | ||
| docs_root = get_docs_root().resolve() | ||
| findings = [] | ||
| files_checked = 0 | ||
| files_skipped = 0 | ||
|
|
||
| folders = get_doc_folders(docs_root) | ||
| manifest = load_manifest(docs_root) | ||
|
|
||
| for folder in folders: | ||
| docs = get_docs_in_folder(folder, docs_root) | ||
| for doc in docs: | ||
| if files_checked >= max_files: | ||
| files_skipped += 1 | ||
| continue | ||
|
|
||
| rel_path = str(doc.relative_to(docs_root)) | ||
| try: | ||
| content = doc.read_text(encoding="utf-8") | ||
| except Exception: | ||
| continue | ||
|
|
||
| folder_info = manifest.get("folders", {}).get(folder, {}) | ||
| index_summary = folder_info.get("index", "") | ||
|
|
||
| severity, reason = _assess_doc(rel_path, content, index_summary) | ||
| if severity != "fresh": | ||
| findings.append({"file": rel_path, "severity": severity, "reason": reason}) | ||
| files_checked += 1 | ||
|
|
||
| if files_skipped > 0: | ||
| print( | ||
| f"Audit budget reached: checked {files_checked} files, " | ||
| f"skipped {files_skipped}. Increase audit-budget to cover more." | ||
| ) | ||
|
|
||
| return findings | ||
|
|
||
|
|
||
| def _assess_doc(file_path, content, index_summary): | ||
| """Ask the LLM to assess a single doc's staleness. | ||
|
|
||
| Returns (severity, reason) where severity is "fresh", "stale", or "very-stale". | ||
| """ | ||
| prompt = f"""Assess whether this documentation file is likely up to date with the current codebase. | ||
|
|
||
| File: {file_path} | ||
|
|
||
| Folder context: {index_summary[:500] if index_summary else "No index available"} | ||
|
|
||
| Documentation content (first 2000 chars): | ||
| {content[:2000]} | ||
|
|
||
| Rate the doc as one of: | ||
| - FRESH: content appears current, no obvious staleness | ||
| - STALE: some sections may be outdated (e.g. refers to old defaults, missing recent features) | ||
| - VERY-STALE: significant portions are likely incorrect or missing | ||
|
|
||
| Respond with exactly one line: SEVERITY: brief reason | ||
| Example: STALE: refers to v1 API but v2 has been released""" | ||
|
|
||
| budget = get_max_context_chars() | ||
| prompt = truncate_content(prompt, budget, label=f"audit prompt for {file_path}") | ||
|
|
||
| try: | ||
| client = get_client() | ||
| response = client.chat.completions.create( | ||
| model=get_model_name(), | ||
| messages=[{"role": "user", "content": prompt}], | ||
| ) | ||
| text = (response.choices[0].message.content or "").strip() | ||
|
|
||
| for severity in ("VERY-STALE", "STALE", "FRESH"): | ||
| if severity in text.upper(): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] logic-error Severity parsing uses substring matching (severity in text.upper()), causing false positives. An LLM response like 'FRESH: docs are not stale' matches STALE before FRESH due to iteration order (VERY-STALE, STALE, FRESH), misclassifying fresh documents as stale. Suggested fix: Use text.upper().startswith(severity) or a regex to match only the leading severity token. |
||
| reason = text.split(":", 1)[1].strip() if ":" in text else text | ||
| return severity.lower(), reason | ||
|
|
||
| return "stale", text[:200] | ||
| except Exception as e: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] error-handling-pattern The _assess_doc() except block does not call check_context_error(e). Every other LLM API call handler in the codebase (discovery.py, doc_index.py, generation.py, jira_integration.py) calls this function to surface actionable context-window errors. Suggested fix: Import check_context_error from config and call it before the print statement in the except block. |
||
| print(f"Warning: Could not assess {file_path}: {sanitize_output(str(e))}") | ||
| return "fresh", "" | ||
|
|
||
|
|
||
| def format_audit_report(findings): | ||
| """Format findings into a Markdown report grouped by severity.""" | ||
| if not findings: | ||
| return "No documentation drift detected." | ||
|
|
||
| lines = ["# Documentation Drift Audit", ""] | ||
|
|
||
| for severity in ("very-stale", "stale"): | ||
| group = [f for f in findings if f["severity"] == severity] | ||
| if not group: | ||
| continue | ||
| label = "Very Stale" if severity == "very-stale" else "Stale" | ||
| lines.append(f"## {label} ({len(group)} file{'s' if len(group) != 1 else ''})") | ||
| lines.append("") | ||
| for f in group: | ||
| lines.append(f"- **{f['file']}**: {f['reason']}") | ||
| lines.append("") | ||
|
|
||
| return "\n".join(lines) | ||
|
|
||
|
|
||
| def post_audit_issue(findings, repo=None): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] api-contract post_audit_issue() accepts a repo parameter but never passes --repo to gh CLI commands. After setup_docs_environment() changes the working directory to a cloned docs repo, gh infers the repo from the current directory's git remote, creating issues in the wrong repository in separate-docs-repo mode. Suggested fix: Pass --repo repo to each gh command (gh label create, gh issue list, gh issue edit, gh issue create). |
||
| """Create or update a single GitHub Issue with audit findings.""" | ||
| gh_token = os.environ.get("GH_TOKEN") | ||
| if not gh_token: | ||
| print("Warning: GH_TOKEN not set, cannot post audit issue") | ||
| return | ||
|
|
||
| if repo is None: | ||
| repo = os.environ.get("GITHUB_REPOSITORY", "") | ||
| if not repo: | ||
| print("Warning: GITHUB_REPOSITORY not set, cannot post audit issue") | ||
| return | ||
|
|
||
| label = "docs-drift-audit" | ||
| title = "Documentation Drift Audit Report" | ||
| body = format_audit_report(findings) | ||
|
|
||
| run_command_safe( | ||
| [ | ||
| "gh", | ||
| "label", | ||
| "create", | ||
| label, | ||
| "--description", | ||
| "Automated docs drift audit", | ||
| "--color", | ||
| "FBCA04", | ||
| "--force", | ||
| ], | ||
| check=False, | ||
| env={**os.environ, "GH_TOKEN": gh_token}, | ||
| ) | ||
|
|
||
| existing = run_command_safe( | ||
| [ | ||
| "gh", | ||
| "issue", | ||
| "list", | ||
| "--label", | ||
| label, | ||
| "--state", | ||
| "open", | ||
| "--json", | ||
| "number", | ||
| "--limit", | ||
| "1", | ||
| ], | ||
| check=False, | ||
| env={**os.environ, "GH_TOKEN": gh_token}, | ||
| ) | ||
|
|
||
| try: | ||
| issues = json.loads(existing.stdout.strip()) if existing.returncode == 0 else [] | ||
| except (json.JSONDecodeError, ValueError): | ||
| issues = [] | ||
|
|
||
| if issues: | ||
| issue_num = issues[0]["number"] | ||
| run_command_safe( | ||
| ["gh", "issue", "edit", str(issue_num), "--body", body], | ||
| check=False, | ||
| env={**os.environ, "GH_TOKEN": gh_token}, | ||
| ) | ||
| print(f"Updated existing audit issue #{issue_num}") | ||
| else: | ||
| result = run_command_safe( | ||
| ["gh", "issue", "create", "--title", title, "--body", body, "--label", label], | ||
| check=False, | ||
| env={**os.environ, "GH_TOKEN": gh_token}, | ||
| ) | ||
| if result.returncode == 0: | ||
| print(f"Created audit issue: {result.stdout.strip()}") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -258,6 +258,23 @@ def main(): | |
| print(f"Index build complete: {result['status']}") | ||
| return | ||
|
|
||
| # Handle audit mode (scheduled full-repo drift check) | ||
| mode = os.environ.get("MODE", "comment") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] input-validation MODE env var read without validation against an allowlist. Unrecognized values silently fall through to comment mode. Suggested fix: Validate MODE is one of the expected values and warn on unrecognized input. |
||
| if mode == "audit": | ||
| from audit import format_audit_report, post_audit_issue, run_audit | ||
|
|
||
| print("Mode: audit") | ||
| if not setup_docs_environment(): | ||
| print("Failed to set up docs environment") | ||
| return | ||
| budget = int(os.environ.get("AUDIT_BUDGET", "20")) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] error-handling-gap AUDIT_BUDGET env var parsed with int() without error handling. A non-numeric value causes an unhandled ValueError, inconsistent with how MAX_CONTEXT_CHARS is handled in config.py. Suggested fix: Wrap in try/except ValueError with fallback to 20. |
||
| findings = run_audit(max_files=budget) | ||
| report = format_audit_report(findings) | ||
| print(report) | ||
| if findings and not args.dry_run: | ||
| post_audit_issue(findings) | ||
| return | ||
|
|
||
| # Detect which command was used | ||
| comment_body = os.environ.get("COMMENT_BODY", "") | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| """Tests for audit.py -- scheduled drift audit.""" | ||
|
|
||
| from audit import format_audit_report | ||
|
|
||
|
|
||
| class TestFormatAuditReport: | ||
| def test_no_findings(self): | ||
| assert "No documentation drift" in format_audit_report([]) | ||
|
|
||
| def test_groups_by_severity(self): | ||
| findings = [ | ||
| {"file": "a.md", "severity": "stale", "reason": "outdated"}, | ||
| {"file": "b.md", "severity": "very-stale", "reason": "missing feature"}, | ||
| {"file": "c.md", "severity": "stale", "reason": "old defaults"}, | ||
| ] | ||
| report = format_audit_report(findings) | ||
| assert "## Very Stale (1 file)" in report | ||
| assert "## Stale (2 files)" in report | ||
| assert "b.md" in report | ||
| assert "a.md" in report | ||
|
|
||
| def test_skips_empty_groups(self): | ||
| findings = [{"file": "a.md", "severity": "stale", "reason": "outdated"}] | ||
| report = format_audit_report(findings) | ||
| assert "Very Stale" not in report | ||
| assert "Stale" in report |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[medium] scope-coherence
The mode input description advertises three values (comment, detect-only, audit) but detect-only is completely unimplemented. A user setting mode: detect-only gets silent fallthrough to comment behavior, which may fail in a scheduled workflow context.
Suggested fix: Remove detect-only from the description until implemented, or add validation for unrecognized mode values.