Skip to content

Commit fd9a018

Browse files
authored
Merge pull request #786 from microsasa/fix/520-responder-fix-forward
fix: add fix-forward to responder and pre-push self-review to implementer (#520)
2 parents c7125aa + 4f7d7bd commit fd9a018

2 files changed

Lines changed: 20 additions & 4 deletions

File tree

.github/workflows/issue-implementer.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,15 @@ Read `.github/copilot-instructions.md` and follow all referenced guidelines for
4949

5050
Read all files in the repository. Read issue #${{ github.event.inputs.issue_number }} to understand what needs to be fixed. Implement the fix following the spec in the issue, including any testing requirements.
5151

52-
Before committing, run `make fix` to auto-fix lint and format issues, then run `make check` to verify all checks pass (lint, type check, security, tests):
52+
**Self-review before pushing**: After implementing, audit your own changes for common reviewer findings:
53+
- **Docstrings**: Does every changed/new function have an accurate docstring? Do existing docstrings still match the new behavior?
54+
- **Test assertions**: Does each test actually verify what its name/docstring claims? Are assertions specific (exact match, not substring)?
55+
- **Dead references**: If you renamed or removed something, grep for stale references in docs, comments, imports, and tests
56+
- **Parallel code paths**: If you fixed a bug or added a guard, check sibling functions/paths for the same gap
57+
- **Naming consistency**: Do test names, variable names, and comments all match the current behavior?
58+
Fix any issues you find.
59+
60+
Then run `make fix` to auto-fix lint and format issues, then run `make check` to verify all checks pass (lint, type check, security, tests):
5361

5462
```
5563
make fix && make check

.github/workflows/review-responder.md

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,18 @@ This workflow addresses unresolved review comments on a pull request.
7070
c. Make the requested fix in the code
7171
d. Reply to the comment thread using `reply_to_pull_request_review_comment` with the comment's `databaseId` as the `comment_id`
7272

73-
5. After addressing all comments, run `make fix` to auto-fix lint and format issues, then run `make check` to verify all checks pass: `make fix && make check`
73+
5. **Fix-forward scan**: After addressing all comments, review what you just fixed and identify the *class* of each issue (e.g., "stale docstring", "missing assertion", "TOCTOU race", "dead code"). For each class, scan ALL files changed in this PR for other instances of the same problem. Common patterns to check:
74+
- If you fixed a stale/inaccurate docstring → audit every docstring in the changed functions
75+
- If you fixed a weak test assertion → check sibling tests for similar assertion gaps
76+
- If you fixed a bug in one code path → check parallel code paths for the same bug
77+
- If you removed dead code → grep for similar dead references elsewhere
78+
- If you fixed a naming mismatch → check all related names/comments for consistency
79+
Fix any additional instances you find. This prevents the reviewer from flagging the same class of issue in the next round.
7480

75-
6. If CI checks fail, fix the issues and re-run until they pass. Do not push broken code.
81+
6. Run `make fix` to auto-fix lint and format issues, then run `make check` to verify all checks pass: `make fix && make check`
7682

77-
7. Push all changes in a single commit with message "fix: address review comments".
83+
7. If CI checks fail, fix the issues and re-run until they pass. Do not push broken code.
84+
85+
8. Push all changes in a single commit with message "fix: address review comments".
7886

7987
If a review comment requests a change that would be architecturally significant or you're unsure about, reply to the thread explaining your concern rather than making the change blindly.

0 commit comments

Comments
 (0)