fix(#2054): synthesize review body when findings contradict summary#67
fix(#2054): synthesize review body when findings contradict summary#67guyoron1 wants to merge 10 commits into
Conversation
…t summary When the review agent produces a result where the action is request-changes with critical/high findings but the body omits those findings (e.g. says "No findings"), the sticky comment misleads reviewers into thinking the review is clean. The previous approach (PR fullsend-ai#2055, closed) used regex replacement to patch "No findings" text in-place. This was fragile: the regex could match inside longer phrases, ReplaceAllString could duplicate content, and inserting bullet lists mid-sentence produced malformed markdown. This fix takes a different approach. Instead of string surgery, ensureBodyFindingsConsistency checks whether the body references any critical/high finding categories (case-insensitive substring match on hyphenated tokens like "logic-error", "auth-bypass"). If none are referenced, the entire body is replaced with one synthesized from the structured findings array using the standard review format from the pr-review skill. The pr-review skill is also updated with an explicit instruction that when action is request-changes or reject, the body MUST list the findings — fixing the issue closer to the source while the CLI provides a safety net. Note: pre-commit could not run in the sandbox due to shellcheck network restrictions (infrastructure issue, not code issue). Closes fullsend-ai#2054
|
/fs-qf |
|
🤖 Finished Review · ✅ Success · Started 12:18 PM UTC · Completed 12:30 PM UTC |
ReviewReason: stale-head The review agent reviewed commit Previous runReviewReason: stale-head The review agent reviewed commit Previous run (2)ReviewReason: stale-head The review agent reviewed commit Previous run (3)ReviewReason: stale-head The review agent reviewed commit |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 12:32 PM UTC · Completed 12:43 PM UTC |
|
/fs-review |
Resolved all 8 review findings (3 major, 5 minor) in 1 iteration: - Remove internal function names from Scope and Testing Goals - Replace code-level Evidence with requirement-level traceability - Add partial category coverage P0 scenario - Simplify Testing Tools, add Head SHA risk, fix priorities - Update Owning SIG to match component/harness label Add QualityFlow output for fullsend-aiGH-2054 [skip ci]
|
🤖 Finished Review · ✅ Success · Started 12:46 PM UTC · Completed 12:57 PM UTC |
- STD YAML: 16 scenarios (11 unit tests, 5 functional) - Go test stubs: 3 files with PSE comments (Phase 1 design) - Coverage: all 16 STP scenarios mapped to test stubs
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 12:59 PM UTC · Completed 1:12 PM UTC |
…p ci] - Standardize tier values to "Tier 1" (was "Unit Tests"/"Functional") - Add patterns field to all 16 scenarios - Fix metadata count fields (tier_1_count/tier_2_count) - Remove related_prs from document_metadata - Replace PR fullsend-ai#2189 references with fullsend-aiGH-2054 ticket reference - Update review: APPROVED_WITH_FINDINGS → APPROVED (score 71.85 → 90.95) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/fs-review |
Replaces intermediate pipeline artifacts with organized test files. Total: 3 test files → qf-tests/fullsend-aiGH-2054/ Jira: fullsend-aiGH-2054 [skip ci]
QualityFlow Pipeline Summary
Test Output
Issue: GH-2054 Generated by QualityFlow |
1 participant
Mirror of upstream fullsend-ai#2189
When review findings contradict the summary verdict, the review body is now re-synthesized to match the findings, preventing confusing reviews where the summary says "looks good" but findings list critical issues.