check_doc_gate.py: a broken/unparseable config exits 1 identically to a real violation - #2306
Conversation
An unparseable or missing doc-gate config previously raised an unhandled traceback that exited 1 -- identical to a real documentation-drift violation, so a typo in docs/doc-gate.toml was indistinguishable from a missing changelog. Catch tomllib.TOMLDecodeError and OSError in main(), print a clear error to stderr, and exit 2 instead.
📝 WalkthroughWalkthroughThe documentation gate now uses named exit codes for success, rule violations, and configuration errors. Configuration-loading failures are reported to stderr and return the configuration-error code. Tests cover malformed and missing configuration files and preserve the violation code for rule failures. ChangesDocumentation gate exit status handling
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoDoc-gate: exit 2 on config errors (distinct from violations)
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
|
nemotron-super review VERDICT: Pass Automated first-pass review by the nemotron-super lane. The lead still reviews before merge. |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Reviewed by step-3.7-flash · Input: 62K · Output: 6.4K · Cached: 244.9K |
Code Review by Qodo
1. Schema errors still crash
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| try: | ||
| config = load_config(args.config) | ||
| except (tomllib.TOMLDecodeError, OSError) as e: | ||
| print(f"doc-gate: config error: {args.config}: {e}", file=sys.stderr) |
There was a problem hiding this comment.
1. Schema errors still crash 🐞 Bug ☼ Reliability
main() only maps TOML parse/read failures to EXIT_CONFIG_ERROR; a syntactically valid but structurally invalid config (wrong types for keys like gate/rules) can still raise later and likely exit 1, making some broken configs indistinguishable from real violations again.
Agent Prompt
## Issue description
`scripts/check_doc_gate.py` now returns `EXIT_CONFIG_ERROR` for `TOMLDecodeError` and `OSError` during `load_config()`, but it still assumes the loaded TOML has the expected structure/types. A syntactically valid TOML with wrong types (e.g., `gate = "x"`) can crash later (e.g., `str` has no `.get()`), which typically exits with status 1 and undermines the “config errors are distinct from violations” contract.
## Issue Context
The PR’s goal is to ensure config problems do not look like doc-gate violations. This currently holds for unreadable/unparseable files, but not for schema/type errors.
## Fix Focus Areas
- scripts/check_doc_gate.py[255-258]
- scripts/check_doc_gate.py[270-307]
## Suggested implementation sketch
- Add a small `validate_config(config: dict) -> None` that checks:
- `config` is a `dict`
- `gate` is absent or a `dict`
- `rules` is absent or a `list` of `dict`
- any other sections used in `main()` (`invariants.referenced_paths_scan`, etc.) have expected types
- Call `validate_config()` immediately after `load_config()`.
- Catch `ValueError` (raised by validation) in the same `except` block and return `EXIT_CONFIG_ERROR`.
- Add a unit test with a syntactically valid TOML that violates schema (e.g., `gate = "x"`) asserting `EXIT_CONFIG_ERROR` and a clear stderr message.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check_doc_gate.py`:
- Around line 283-287: Update the exception tuple in the config-loading handler
around load_config to include UnicodeDecodeError, ensuring invalid UTF-8 returns
EXIT_CONFIG_ERROR with the existing diagnostic. Add a regression test that
writes invalid bytes such as b"\xff" and verifies the configuration-error
outcome.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f8a92547-54d4-468c-af3e-a9e398ea0df7
📒 Files selected for processing (2)
scripts/check_doc_gate.pytests/test_doc_gate.py
| try: | ||
| config = load_config(args.config) | ||
| except (tomllib.TOMLDecodeError, OSError) as e: | ||
| print(f"doc-gate: config error: {args.config}: {e}", file=sys.stderr) | ||
| return EXIT_CONFIG_ERROR |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import io
import tomllib
try:
tomllib.load(io.BytesIO(b"\xff"))
except UnicodeDecodeError:
print("confirmed")
else:
raise SystemExit("expected UnicodeDecodeError")
PYRepository: jaylfc/taOS
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import io, tomllib
for value in (b"\xff", b"\x80", b"\x80\x80\x80"):
try:
tomllib.load(io.BytesIO(value))
except Exception as exc:
print(type(exc).__name__, bool(isinstance(exc, tomllib.TOMLDecodeError)), bool(isinstance(exc, UnicodeDecodeError)))
else:
print("parsed OK")
PY
printf '\n--- scripts/check_doc_gate.py context ---\n'
sed -n '250,300p' scripts/check_doc_gate.py
printf '\n--- config error tests/usages ---\n'
rg -n "EXIT_CONFIG_ERROR|load_config|TOMLDecodeError|UnicodeDecodeError|bad\.write_bytes|config error" scripts -SRepository: jaylfc/taOS
Length of output: 2804
Handle invalid UTF-8 as a configuration error.
tomllib.load() decodes the file before TOML parsing, so invalid UTF-8 raises UnicodeDecodeError, not tomllib.TOMLDecodeError. Catch that exception here so a corrupt configuration returns EXIT_CONFIG_ERROR instead of escaping the config-error handler. Add a regression test that writes invalid UTF-8 bytes, such as bad.write_bytes(b"\xff").
Proposed fix
- except (tomllib.TOMLDecodeError, OSError) as e:
+ except (tomllib.TOMLDecodeError, UnicodeDecodeError, OSError) as e:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| config = load_config(args.config) | |
| except (tomllib.TOMLDecodeError, OSError) as e: | |
| print(f"doc-gate: config error: {args.config}: {e}", file=sys.stderr) | |
| return EXIT_CONFIG_ERROR | |
| try: | |
| config = load_config(args.config) | |
| except (tomllib.TOMLDecodeError, UnicodeDecodeError, OSError) as e: | |
| print(f"doc-gate: config error: {args.config}: {e}", file=sys.stderr) | |
| return EXIT_CONFIG_ERROR |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/check_doc_gate.py` around lines 283 - 287, Update the exception tuple
in the config-loading handler around load_config to include UnicodeDecodeError,
ensuring invalid UTF-8 returns EXIT_CONFIG_ERROR with the existing diagnostic.
Add a regression test that writes invalid bytes such as b"\xff" and verifies the
configuration-error outcome.
|
Reviewed against The feature is real, verified red-then-green. On this branch: On dev, both config cases produce a raw BLOCKING —
|
|
Correcting my own verdict above. I closed that review with "once the code moves off 2 and the 'different codes' test lands, this is good to merge." That was premature. Qodo and CodeRabbit each filed a finding I had missed, both are correct, and I verified both on this branch rather than taking their word. Between them they mean the config-error path still has two holes of exactly the class this card exists to close. 1. A structurally invalid config still exits 1 (qodo). Valid TOML, wrong shape:
2. Invalid UTF-8 in the config file still exits 1 (CodeRabbit). And the except clause cannot catch it:
Revised asks, replacing the single one in my earlier comment:
Point 3 is the one worth doing properly rather than minimally. "The config file loaded" and "the config file is usable" are different claims, and this PR currently only checks the first — which is the same absent-versus-negative distinction that keeps producing these bugs. Credit to both bots; my review would have let this through with two of the three holes still open. Noting for the record that CodeRabbit's finding here came with an executed script demonstrating the decode failure, not just an assertion, which is why it was quick to confirm. |
jaylfc
added
the
lead-blocked
|
Closing as superseded by #2312 (its fix-forward), which merged with the corrected config-error exit paths plus the review fixes, STEP-0 verified (no commits lost). The doc-gate work continues on dev. |
…figs Pr jaylfc#2306 left two config-error paths that still exited 1 instead of the config-error code: - Structurally invalid config (valid TOML, wrong shape, e.g. rules is a string) passed load_config and died later in the rule loop with an AttributeError. Add _validate_config to check the shape after parsing. - Non-UTF-8 config bytes raised UnicodeDecodeError inside tomllib.load (which decodes before parsing), uncaught by the TOMLDecodeError-only handler. Catch that alongside the other config errors. Also move EXIT_CONFIG_ERROR from 2 to 3: exit code 2 is argparse's own usage-error exit, so a typo'd flag was indistinguishable from a broken config. Now 1 (violation), 2 (argparse), and 3 (config error) are all mutually distinct. Supersedes exec/tsk-thh54d (pr jaylfc#2306).
1 participant
CARD TITLE (intent, not commit subject): check_doc_gate.py: a broken/unparseable config exits 1 identically to a real violation
Autonomous build of board card tsk-thh54d.
An unparseable or missing doc-gate config previously raised an unhandled
traceback that exited 1 -- identical to a real documentation-drift violation,
so a typo in docs/doc-gate.toml was indistinguishable from a missing
changelog. Catch tomllib.TOMLDecodeError and OSError in main(), print a
clear error to stderr, and exit 2 instead.
Files:
scripts/check_doc_gate.py | 17 ++++++++++++++---
tests/test_doc_gate.py | 31 +++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 3 deletions(-)
Summary by CodeRabbit
Bug Fixes
Tests