feat: detect-only mode for docs drift checking - #68
Conversation
The detection half of the tool is far more reliable and cheaper than generation, but currently they are bundled. Add a detect-only mode that identifies docs affected by a diff, compares against docs actually changed in the PR, and reports which files may need updates. Generates nothing, opens no PR. Runnable on pull_request events, not just issue_comment.
Add a README section with an example workflow for detect-only mode on pull_request events. Frame it as the first thing a cautious team should adopt before using the full generation pipeline.
|
🤖 Finished Review · ✅ Success · Started 5:46 AM UTC · Completed 6:03 AM UTC Commit: |
ReviewFindingsHigh
Medium
Low
Labels: PR adds a new detect-only execution mode with Python source, tests, and GitHub Action config changes. Next steps:
|
| if not setup_docs_environment(): | ||
| print("Failed to set up docs environment") | ||
| return | ||
| diff = get_diff() |
There was a problem hiding this comment.
[high] logic-error
In detect-only mode, setup_docs_environment() is called before get_diff(). setup_docs_environment() changes the working directory to the docs subfolder or a cloned docs repo. get_diff() then runs git diff in that new CWD. In separate-repo mode, this produces the docs repo's diff instead of the source code diff. In the comment-based flow, get_diff() correctly runs before setup_docs_environment().
Suggested fix: Move diff = get_diff() and the empty-diff guard before the setup_docs_environment() call in the detect-only block, matching the ordering used by the comment-based flow.
|
|
||
| def run_detect_only(diff, relevant_files, changed_docs): | ||
| """Compare affected docs against docs actually changed in the PR. | ||
|
|
There was a problem hiding this comment.
[high] logic-error
run_detect_only compares relevant_files (paths relative to docs root) against changed_docs (repo-relative paths from extract_changed_doc_paths). In same-repo mode with DOCS_SUBFOLDER, these path namespaces differ. The set subtraction will never find matches, so every affected file will be reported as untouched even when updated in the PR.
Suggested fix: Normalize paths before comparison. Strip the DOCS_SUBFOLDER prefix from changed_docs paths or prepend it to relevant_files.
| return | ||
|
|
||
| # Handle detect-only mode (runs on pull_request events, not comments) | ||
| mode = os.environ.get("MODE", "comment") |
There was a problem hiding this comment.
[medium] import-style
The detect-only mode block uses a lazy inline import. Every other module import in suggest_docs.py is at the top of the file. No other conditional/deferred import exists in the file, and detect.py has no heavy dependencies justifying deferral.
Suggested fix: Move the import to the top-level imports section alongside the other from-module-import statements.
| "Comment [review-docs] on the PR to review suggested changes, " | ||
| "or [update-docs] to generate updates directly." | ||
| ) | ||
| else: |
There was a problem hiding this comment.
[low] edge-case
When relevant_files is empty and changed_docs is also empty, run_detect_only reports 'All affected documentation files are already updated in this PR.' This is misleading -- no docs were identified as affected.
Suggested fix: Add a check: if affected_set is empty, output a distinct message like 'No documentation files were identified as affected by this change.'
| @@ -0,0 +1,51 @@ | |||
| """Detect-only mode: identify docs affected by a diff without generating anything.""" | |||
There was a problem hiding this comment.
[low] module-docstring-style
The module docstring is a single line. Most other source modules in src/ use a multi-line docstring with summary, blank line, and elaboration.
| @@ -0,0 +1,52 @@ | |||
| """Tests for detect-only mode.""" | |||
There was a problem hiding this comment.
[low] test-docstring-convention
The test docstring references the feature rather than the module. Other test files follow the convention of referencing the source module filename.
| print(f"Index build complete: {result['status']}") | ||
| return | ||
|
|
||
| # Handle detect-only mode (runs on pull_request events, not comments) |
There was a problem hiding this comment.
[low] scope-coherence
The detect-only mode block sits between the --build-index handler and COMMENT_BODY parsing. The existing comment partially explains this but could be more explicit about why the position matters.
Summary
Adds a detect-only mode that identifies docs affected by a code change without generating anything. Runnable on
pull_requestevents as a lightweight docs drift check.modeinput:comment(default, current behavior) ordetect-onlydocs-drift-severityinput:warn(exit 0) orerror(exit non-zero for use as a required status check)src/detect.pymodule with diff parsing and comparison logicThe failure message names specific files and tells the user what to do next.
Test plan
uv run pytest -vpasses (423 tests)