feat: configuration hygiene (secrets migration, ignore list, validation config) - #67
feat: configuration hygiene (secrets migration, ignore list, validation config)#67Benkapner wants to merge 3 commits into
Conversation
DOCS_SUBFOLDER, DOCS_BASE_BRANCH, and MAX_CONTEXT_CHARS were documented as repository secrets despite not being secret. GitHub masks secret values in logs, so a misconfigured subfolder prints as *** which actively obstructs debugging. These were already first-class action.yml inputs; the fix is in the README: move them from the secrets table to the inputs table, update the workflow example to use literal values, and add a migration note.
There was no way to keep the tool away from hand-crafted landing pages, generated API reference, or vendored docs. Users need this before pointing the tool at a real docs tree. Add .code-to-docs/ignore with gitignore-style glob syntax, loaded from the base branch. Filter at file-selection time in both full-scan and index-based discovery paths, before doc content is read. Excluded files never reach an LLM prompt.
_REMOVAL_THRESHOLD is a module constant governing a heuristic whose correct value varies by repo and doc style. Add validation.removal-threshold, validation.min-lines, and validation.llm-verification to .code-to-docs/config.json. Thread as parameters into the generation functions rather than reading config inside them, keeping them testable in isolation. Invalid values fall back to defaults with a warning. The validation logic itself is in PR #53; this commit adds the config plumbing.
|
🤖 Finished Review · ✅ Success · Started 5:41 AM UTC · Completed 5:59 AM UTC Commit: |
ReviewFindingsMedium
Low
Labels: PR adds new configuration features (ignore list, validation thresholds) and modifies documentation Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| file_instructions=None, | ||
| style_guidelines="", | ||
| pr_description="", | ||
| validation_config=None, |
There was a problem hiding this comment.
[medium] dead-code
The validation_config parameter is added to generate_updates_parallel and ask_ai_for_updated_content but is never used in either function body. The inner process_file closure in generate_updates_parallel calls ask_ai_for_updated_content without forwarding validation_config, so the config passed by suggest_docs.py is silently dropped.
Suggested fix: Forward validation_config=validation_config in process_file's call to ask_ai_for_updated_content, or remove the parameter until the validation logic is implemented.
| | Key | Description | Example | | ||
| |-----|-------------|---------| | ||
| | `pr-title-prefix` | Prefix prepended to all PR titles and commit messages created by the tool | `":book:"` | | ||
| | `validation.removal-threshold` | Fraction of original lines that can be removed before flagging (0.0 to 1.0) | `0.30` | |
There was a problem hiding this comment.
[medium] documentation-code-mismatch
The README example shows removal-threshold: 0.30 and min-lines: 20, but _VALIDATION_DEFAULTS in config.py uses 0.20 and 30. The table also shows llm-verification example as false while the code default is True. Users who omit these settings will get different behavior than the examples suggest.
Suggested fix: Align the README examples with the code defaults (0.20 and 30), or update _VALIDATION_DEFAULTS to match the README.
| from fnmatch import fnmatch | ||
|
|
||
| path_str = str(path) | ||
| for pattern in patterns: |
There was a problem hiding this comment.
[low] semantic-mismatch
is_path_ignored is documented as using gitignore-style glob patterns but uses Python fnmatch, where * matches path separators (unlike gitignore). generated/* will match generated/sub/file.md with fnmatch but not with gitignore. The implementation is over-inclusive (excludes more files than expected).
| return True | ||
| if "/" in pattern and fnmatch(path_str, pattern): | ||
| return True | ||
| return False |
There was a problem hiding this comment.
[low] dead-code
The third branch if '/' in pattern and fnmatch(path_str, pattern) is unreachable. fnmatch(path_str, pattern) was already checked and returned False in the first branch, so this identical check will also be False.
fullsend-ai-review
Bot
added
feature
documentation
1 participant
Summary
Three backward-compatible configuration improvements. Behavior changes are opt-in; existing workflows work without modification.
DOCS_SUBFOLDER,DOCS_BASE_BRANCH, andMAX_CONTEXT_CHARSwere documented as secrets despite not being secret. GitHub masks secret values in logs, so misconfigured values print as***. Moved to the inputs table in the README with a migration note..code-to-docs/ignorewith gitignore-style globs. Excluded files are never selected, never read into an LLM prompt, and are omitted from index-based selection. Loaded from the base branch.validation.removal-threshold,validation.min-lines, andvalidation.llm-verificationin.code-to-docs/config.json. Threaded as parameters through the generation functions. Invalid values fall back to defaults with a warning.Test plan
uv run pytest -vpasses (433 tests)uv run ruff check src/ tests/anduv run ruff format --check src/ tests/clean