-
Notifications
You must be signed in to change notification settings - Fork 64
feat(agents): add action-hints footer to review, triage, and fix comments #2775
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
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 |
|---|---|---|
|
|
@@ -313,3 +313,11 @@ Information is sufficient for a developer to investigate and fix. | |
| - Do not present unverified assumptions with certainty. Convey uncertainty when appropriate. | ||
| - Write in second person ("you") addressing the reporter. Do not use first person ("I") — the comment is from the triage system, not an individual. | ||
| - If you include `label_actions`, the pipeline appends your label reason to the comment automatically — do not include label justifications in the `comment` field yourself. | ||
| - **Action hints footer (sufficient action only):** When `action` is `sufficient`, append the following footer to the end of the `comment` field. Omit it for all other actions (`insufficient`, `duplicate`, `prerequisites`, `question`). | ||
|
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] design-direction Conditional footer logic for triage is instruction-driven, not enforced by schema or post-script, unlike the fix agent footer which is programmatic. |
||
|
|
||
| ```markdown | ||
| --- | ||
| **Next steps:** | ||
| - `/fs-code` — agent creates a PR to implement this issue | ||
| - `/fs-code <your instruction>` — agent implements with your specific guidance | ||
| ``` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -165,6 +165,19 @@ def test_fix_without_path(self): | |
| self.assertIn("**typo in docs**:", body) | ||
| self.assertNotIn("(`", body) | ||
|
|
||
| def test_action_hints_footer_present(self): | ||
| data = { | ||
| "summary": "Fixed.", | ||
| "tests_passed": True, | ||
| "actions": [ | ||
| {"type": "fix", "finding": "bug", "description": "Fixed"}, | ||
| ], | ||
| } | ||
| body = build_summary_body(data) | ||
| self.assertIn("**Next steps:**", body) | ||
| self.assertIn("/fs-review", body) | ||
| self.assertIn("/fs-fix", body) | ||
|
|
||
|
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] test-inadequate The assertion assertIn('/fs-fix', body) passes via substring match against '/fs-fix ' in the production code. The fix agent footer omits a bare '/fs-fix' option, unlike the PR review footer in SKILL.md which lists both. The substring assertion masks this inconsistency. Suggested fix: Either add bare '/fs-fix' to the fix agent footer (consistent with the review footer), or make the assertion precise: assertIn('/fs-fix ', body). |
||
|
|
||
| post_summary = mod.post_summary | ||
| MAX_COMMENT_LENGTH = mod.MAX_COMMENT_LENGTH | ||
|
|
||
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.
[low] pattern-inconsistency
Triage footer placement guidance is adequate but less detailed than SKILL.md's placement instructions.