docs: restore the rationale docstring on the fleet issue-format validator - #2962
Conversation
…ator #2960 improved this validator's logic and, in doing so, replaced a 22-line docstring with a single summary line. The logic changes were all good and are untouched here. What was lost is the reasoning, and losing it is a real cost for a file synced verbatim into 13 consumer repos: the next person to read it cannot tell WHY the contract is what it is, and a well-meaning simplification can silently reintroduce a bug the wording existed to prevent. Restored, with one addition that #2960 itself earned. `_headings()` skipping fenced code blocks is load-bearing, not cosmetic: without it a body whose only "Tasks" and "Acceptance Criteria" lines sit inside a ```bash fence validates as CONFORMING. That is a false negative in the guard, and it was live in Fine-Art-Archive until the sync landed there today -- a probe body with zero real sections exited 0 against the old validator and 1 against this one. Since good issues quote commands and expected output in fences constantly, that is the common case rather than an edge one, so the docstring now says so and points at tests/scripts/test_issue_format.py, which already covers it via test_fenced_headings_do_not_satisfy_required_sections. Also brings the root .github/scripts copy back into byte-for-byte lockstep with the consumer template; it had drifted to the pre-#2960 text. No behaviour change: verified by comparing the AST of both files before and after with docstrings stripped (identical), and by re-running the fenced-heading probe (still correctly non-conforming). tests/scripts/test_issue_format.py: 10 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Workflow source neededPR #2962 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
📝 WalkthroughWalkthroughThe PR expands the module docstrings for the issue-format validator and its consumer-repository template. It documents the contract, required sections, validation workflows, fenced-code handling, and runtime constraints. Executable behavior remains unchanged. ChangesIssue format documentation
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 455ae241f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * `agents-issue-format-guard.yml` validates every issue on open/edit and, on | ||
| failure, applies `agents:format` — the label the existing Agents Issue | ||
| Optimizer already listens for — so a bad issue is ROUTED to the machinery | ||
| that repairs it rather than merely flagged; |
There was a problem hiding this comment.
This description is false for several events handled by the named workflow: .github/workflows/agents-issue-format-guard.yml:42-45,54 skips validation for durable/wontfix, bot-authored, and explicitly non-coding issues, while its route step at line 85 suppresses agents:format and optimizer dispatch for issues carrying agents:auto-pilot-pause or needs-human. Because this docstring is intended to preserve the workflow rationale across consumers, saying that every issue is validated and every failure is routed obscures the hold/exemption contract added by the preceding change; qualify this as non-exempt, unheld issues.
Useful? React with 👍 / 👎.
![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 @.github/scripts/issue_format.py:
- Around line 7-19: Update the module docstring wording at
`.github/scripts/issue_format.py` lines 7-19 to distinguish issue validation
from downstream pipeline routing, so it no longer claims unlabeled issues are
unvalidated; apply the identical wording at
`templates/consumer-repo/.github/scripts/issue_format.py` lines 7-19 to keep
both synchronized.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 9b59ae5e-9c3a-443c-883d-c6e4d131a090
📒 Files selected for processing (2)
.github/scripts/issue_format.pytemplates/consumer-repo/.github/scripts/issue_format.py
| Why it exists: every automated lane reaches an issue through a LABEL. An issue | ||
| filed with no label and no Tasks/Acceptance block is invisible to the entire | ||
| pipeline — nothing validates it, nothing optimises it, nothing claims it. Local | ||
| automation that files *findings* rather than *work orders* therefore produces | ||
| issues no agent can ever pick up: good evidence, permanently unactionable. | ||
| (Observed in Fine-Art-Archive #406-409: four well-evidenced audit findings, zero | ||
| labels, no Tasks section between them.) | ||
|
|
||
| Used at both ends: | ||
| * `agents-issue-format-guard.yml` validates every issue on open/edit and, on | ||
| failure, applies `agents:format` — the label the existing Agents Issue | ||
| Optimizer already listens for — so a bad issue is ROUTED to the machinery | ||
| that repairs it rather than merely flagged; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the validation and routing claims consistent in both module docstrings.
The docstring says at Line 16 that agents-issue-format-guard.yml validates every issue, but at Line 8 it says that nothing validates an unlabeled issue. Clarify that the missing behavior is downstream routing, or narrow the scope of “pipeline.”
.github/scripts/issue_format.py#L7-L19: update the authoritative wording.templates/consumer-repo/.github/scripts/issue_format.py#L7-L19: apply the identical wording to preserve synchronization.
📍 Affects 2 files
.github/scripts/issue_format.py#L7-L19(this comment)templates/consumer-repo/.github/scripts/issue_format.py#L7-L19
🤖 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 @.github/scripts/issue_format.py around lines 7 - 19, Update the module
docstring wording at `.github/scripts/issue_format.py` lines 7-19 to distinguish
issue validation from downstream pipeline routing, so it no longer claims
unlabeled issues are unvalidated; apply the identical wording at
`templates/consumer-repo/.github/scripts/issue_format.py` lines 7-19 to keep
both synchronized.
There was a problem hiding this comment.
Pull request overview
Restores the previously removed, long-form module docstring for the fleet issue-format validator, preserving the rationale and operational context for a script that is synced verbatim into consumer repositories.
Changes:
- Replaced the one-line module docstring with a detailed rationale covering why the validator exists, how it’s used, and why fenced-code heading skipping is required.
- Re-established docstring parity between the Workflows source copy and the consumer template copy.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| templates/consumer-repo/.github/scripts/issue_format.py | Restores the detailed module docstring explaining validator rationale and load-bearing behaviors. |
| .github/scripts/issue_format.py | Mirrors the same restored module docstring to keep source and template copies aligned. |
| Rules mirror docs/AGENT_ISSUE_FORMAT.md rather than inventing a parallel | ||
| standard: Tasks and Acceptance Criteria are REQUIRED; Why / Scope / | ||
| Implementation Notes / Non-Goals are recommended; and at least one acceptance | ||
| criterion must name a real test, runnable command, or observable verification | ||
| gate. |
| Rules mirror docs/AGENT_ISSUE_FORMAT.md rather than inventing a parallel | ||
| standard: Tasks and Acceptance Criteria are REQUIRED; Why / Scope / | ||
| Implementation Notes / Non-Goals are recommended; and at least one acceptance | ||
| criterion must name a real test, runnable command, or observable verification | ||
| gate. |
Automated Status SummaryHead SHA: f5e6b71
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
…hanges #2962 restored a 22-line rationale docstring to the fleet issue-format validator; this branch had replaced it with a one-line summary and added a shebang. Conflict resolved by keeping both: #2963's shebang and logic, and the docstring. The docstring's description of the contract needed one correction to stay truthful against this branch's behaviour. `Report.ok` now requires `not missing_recommended`, so a missing "recommended" section fails the run and causes the guard to apply `agents:format` -- while the message still calls it an advisory. The docstring now records that tension rather than repeating the old required/recommended split as if it still held. Both copies remain byte-for-byte in lockstep. tests/scripts/test_issue_format.py: 13 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#2960 improved this validator's logic and, in doing so, replaced a 22-line docstring with a single summary line. The logic changes were all good and are untouched here — this is a docstring-only change.
Losing the reasoning is a real cost for a file synced verbatim into 13 consumer repos: the next reader can't tell why the contract is what it is, and a well-meaning simplification can silently reintroduce a bug the wording existed to prevent.
One addition that #2960 itself earned
_headings()skipping fenced code blocks is load-bearing, not cosmetic. Without it, a body whose onlyTasksandAcceptance Criterialines sit inside a ```bash fence validates as conforming — a false negative that lets an unactionable issue straight through the guard.That was live in Fine-Art-Archive until the sync landed there today. Demonstrated with a probe body containing zero real sections:
Good issues quote commands and expected output in fences constantly, so this is the common case, not an edge one. The docstring now says so and points at
tests/scripts/test_issue_format.py, which already covers it viatest_fenced_headings_do_not_satisfy_required_sections.Also
Brings the root
.github/scriptscopy back into byte-for-byte lockstep with the consumer template — it had drifted to the pre-#2960 text.Verification
tests/scripts/test_issue_format.py: 10 passed.🤖 Generated with Claude Code
Summary by CodeRabbit