-
Notifications
You must be signed in to change notification settings - Fork 7
feat: configuration hygiene (secrets migration, ignore list, validation config) #67
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 |
|---|---|---|
|
|
@@ -259,6 +259,100 @@ def get_pr_title_prefix(): | |
| return f"{prefix} " if prefix else "" | ||
|
|
||
|
|
||
| _VALIDATION_DEFAULTS = { | ||
| "removal-threshold": 0.20, | ||
| "min-lines": 30, | ||
| "llm-verification": True, | ||
| } | ||
|
|
||
|
|
||
| def get_validation_config(repo_config=None): | ||
| """Extract and validate the validation settings from repo config. | ||
|
|
||
| Returns a dict with keys: removal_threshold, min_lines, llm_verification. | ||
| """ | ||
| defaults = _VALIDATION_DEFAULTS | ||
| if repo_config is None: | ||
| repo_config = load_repo_config() | ||
|
|
||
| v = repo_config.get("validation", {}) | ||
| if not isinstance(v, dict): | ||
| print("Warning: validation config is not an object, using defaults") | ||
| v = {} | ||
|
|
||
| threshold = v.get("removal-threshold", defaults["removal-threshold"]) | ||
| if not isinstance(threshold, (int, float)) or not (0.0 <= threshold <= 1.0): | ||
| print( | ||
| f"Warning: invalid removal-threshold {threshold!r}, " | ||
| f"using default {defaults['removal-threshold']}" | ||
| ) | ||
| threshold = defaults["removal-threshold"] | ||
|
|
||
| min_lines = v.get("min-lines", defaults["min-lines"]) | ||
| if not isinstance(min_lines, int) or min_lines < 0: | ||
| print(f"Warning: invalid min-lines {min_lines!r}, using default {defaults['min-lines']}") | ||
| min_lines = defaults["min-lines"] | ||
|
|
||
| llm_verify = v.get("llm-verification", defaults["llm-verification"]) | ||
| if not isinstance(llm_verify, bool): | ||
| print( | ||
| f"Warning: invalid llm-verification {llm_verify!r}, " | ||
| f"using default {defaults['llm-verification']}" | ||
| ) | ||
| llm_verify = defaults["llm-verification"] | ||
|
|
||
| return { | ||
| "removal_threshold": float(threshold), | ||
| "min_lines": int(min_lines), | ||
| "llm_verification": llm_verify, | ||
| } | ||
|
|
||
|
|
||
| # ============================================================================= | ||
| # IGNORE LIST | ||
| # ============================================================================= | ||
|
|
||
| _IGNORE_FILE = ".code-to-docs/ignore" | ||
|
|
||
|
|
||
| def load_ignore_patterns(): | ||
| """Load gitignore-style exclusion patterns from the base branch. | ||
|
|
||
| Returns a list of pattern strings. Empty list if the file is absent. | ||
| """ | ||
| base_branch = os.environ.get("DOCS_BASE_BRANCH") or "main" | ||
| try: | ||
| result = run_command_safe( | ||
| ["git", "show", f"origin/{base_branch}:{_IGNORE_FILE}"], | ||
| check=False, | ||
| ) | ||
| if result.returncode != 0 or not result.stdout.strip(): | ||
| return [] | ||
| lines = result.stdout.strip().splitlines() | ||
| patterns = [ln.strip() for ln in lines if ln.strip() and not ln.strip().startswith("#")] | ||
| if patterns: | ||
| print(f"Loaded {len(patterns)} ignore pattern(s) from {base_branch}:{_IGNORE_FILE}") | ||
| return patterns | ||
| except Exception as e: | ||
| print(f"Warning: Could not load ignore patterns: {sanitize_output(str(e))}") | ||
| return [] | ||
|
|
||
|
|
||
| def is_path_ignored(path, patterns): | ||
| """Check whether a file path matches any gitignore-style pattern.""" | ||
| if not patterns: | ||
| return False | ||
| from fnmatch import fnmatch | ||
|
|
||
| path_str = str(path) | ||
| for pattern in patterns: | ||
|
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] 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). |
||
| if fnmatch(path_str, pattern) or fnmatch(path_str, f"**/{pattern}"): | ||
| return True | ||
| if "/" in pattern and fnmatch(path_str, pattern): | ||
| return True | ||
| return False | ||
|
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] 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. |
||
|
|
||
|
|
||
| def check_context_error(e): | ||
| """ | ||
| If e is a context-window error, print actionable guidance. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -157,6 +157,7 @@ def generate_updates_parallel( | |
| file_instructions=None, | ||
| style_guidelines="", | ||
| pr_description="", | ||
| validation_config=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] 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. |
||
| ): | ||
| """ | ||
| Generate documentation updates in parallel. | ||
|
|
@@ -238,6 +239,7 @@ def ask_ai_for_updated_content( | |
| current_content, | ||
| user_instructions="", | ||
| file_instructions=None, | ||
| validation_config=None, | ||
| style_guidelines="", | ||
| pr_description="", | ||
| ): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| """Tests for .code-to-docs/ignore exclusion list.""" | ||
|
|
||
| from unittest.mock import MagicMock, patch | ||
|
|
||
| from config import is_path_ignored, load_ignore_patterns | ||
|
|
||
|
|
||
| class TestIsPathIgnored: | ||
| def test_no_patterns_returns_false(self): | ||
| assert is_path_ignored("docs/guide.md", []) is False | ||
|
|
||
| def test_exact_match(self): | ||
| assert is_path_ignored("README.md", ["README.md"]) is True | ||
|
|
||
| def test_glob_match(self): | ||
| assert is_path_ignored("docs/api-ref.md", ["docs/api-*.md"]) is True | ||
|
|
||
| def test_no_match(self): | ||
| assert is_path_ignored("docs/guide.md", ["docs/api-*.md"]) is False | ||
|
|
||
| def test_directory_glob(self): | ||
| assert is_path_ignored("generated/openapi/ref.md", ["generated/*"]) is True | ||
|
|
||
| def test_bare_filename_matches_anywhere(self): | ||
| assert is_path_ignored("deep/nested/CHANGELOG.md", ["CHANGELOG.md"]) is True | ||
|
|
||
| def test_multiple_patterns(self): | ||
| patterns = ["CHANGELOG.md", "generated/*", "*.bak"] | ||
| assert is_path_ignored("docs/old.bak", patterns) is True | ||
| assert is_path_ignored("docs/guide.md", patterns) is False | ||
|
|
||
|
|
||
| class TestLoadIgnorePatterns: | ||
| def test_loads_patterns_from_branch(self): | ||
| result = MagicMock(returncode=0, stdout="# comment\ngenerated/*\nREADME.md\n\n") | ||
| with patch("config.run_command_safe", return_value=result): | ||
| patterns = load_ignore_patterns() | ||
| assert patterns == ["generated/*", "README.md"] | ||
|
|
||
| def test_returns_empty_when_file_missing(self): | ||
| result = MagicMock(returncode=1, stdout="") | ||
| with patch("config.run_command_safe", return_value=result): | ||
| patterns = load_ignore_patterns() | ||
| assert patterns == [] | ||
|
|
||
| def test_returns_empty_on_error(self): | ||
| with patch("config.run_command_safe", side_effect=RuntimeError("git failed")): | ||
| patterns = load_ignore_patterns() | ||
| assert patterns == [] | ||
|
|
||
| def test_skips_comments_and_blanks(self): | ||
| result = MagicMock(returncode=0, stdout="# skip this\n\n \nkeep-this.md\n") | ||
| with patch("config.run_command_safe", return_value=result): | ||
| patterns = load_ignore_patterns() | ||
| assert patterns == ["keep-this.md"] |
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] 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.