feat(retro): add flapping detection to retro analysis skill (#5512)#5513
Open
Benkapner wants to merge 2 commits into
Open
feat(retro): add flapping detection to retro analysis skill (#5512)#5513Benkapner wants to merge 2 commits into
Benkapner wants to merge 2 commits into
Conversation
…-ai#5512) Add a "Flapping detection" section to the retro-analysis skill that teaches the retro agent how to identify fix-break oscillation patterns: file-level oscillation (same files changed and reverted across runs), test result flipping, and excessive review-fix cycles. Includes concrete signals to check, bash commands for diff comparison, guidance on when to flag vs when NOT to flag (single rework is normal), and how to structure proposals when flapping is detected. Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
1 similar comment
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
PR Summary by QodoAdd flapping detection guidance to retro-analysis skill
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Site previewPreview: https://4aebfafa-site.fullsend-ai.workers.dev Commit: |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+115
to
+125
| ```bash | ||
| # Compare diffs between consecutive code/fix runs | ||
| gh api "repos/$REPO_FULL_NAME/pulls/$PR_NUMBER/files" \ | ||
| --jq '.[].filename' | sort > /tmp/current_files.txt | ||
|
|
||
| # Get previous run's changed files from the commit before | ||
| git diff --name-only HEAD~1 HEAD | sort > /tmp/prev_files.txt | ||
|
|
||
| # Files changed in both runs (potential oscillation) | ||
| comm -12 /tmp/prev_files.txt /tmp/current_files.txt | ||
| ``` |
There was a problem hiding this comment.
1. Run diff comparison wrong 🐞 Bug ≡ Correctness
The flapping-detection snippet compares the full PR file list against git diff HEAD~1 HEAD, which is not a comparison between consecutive code/fix runs and only yields filename overlap (not reversed changes). This can cause the retro agent to miss real oscillations or generate false flapping candidates that aren’t tied to specific run-to-run transitions.
Agent Prompt
### Issue description
The current flapping-detection bash example doesn’t compare *consecutive code/fix runs* (it compares PR-wide files to the last commit), and it doesn’t provide a way to verify that changes were actually reversed.
### Issue Context
The section describes detecting oscillation between *runs* (run N vs run N+1), but the snippet uses `HEAD~1..HEAD` and PR-wide file listing, which is not run-scoped.
### Fix Focus Areas
- internal/scaffold/fullsend-repo/skills/retro-analysis/SKILL.md[115-125]
### Suggested direction
- Change the snippet to:
- Use run-specific commit SHAs (e.g., from each run’s produced commit / PR head at the time) and compare consecutive SHAs via `gh api repos/$REPO_FULL_NAME/compare/<shaN>...<shaNplus1>` (or `git diff <shaN>..<shaNplus1>` if available locally).
- Add an explicit step that checks *reversal* for candidate files (e.g., show `git diff <shaN>..<shaNplus1> -- <file>` and `git diff <shaNplus1>..<shaNplus2> -- <file>` to demonstrate add/remove reversal).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Author
There was a problem hiding this comment.
fixed. now compares consecutive commit SHAs instead of using HEAD~1.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Address review feedback: - Compare consecutive commit SHAs instead of HEAD~1 - Add --paginate to gh api calls for PR files Signed-off-by: Benjamin Kapner <bkapner@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds flapping detection guidance to the
retro-analysisskill. The retro agent now knows how to identify fix-break oscillation patterns during its post-workflow analysis.What it adds to the skill
Why this matters
Flapping wastes agent cycles and often indicates a deeper problem (conflicting instructions, flaky tests, or an approach the agent cannot converge on). Today, the retro agent has no specific guidance to detect this pattern, even though the flapping-convergence problem doc identifies it as a key concern.
Related Issue
Closes #5512
Testing
Checklist