Skip to content

Feat/review suggestion blocks#52

Merged
factory-nizar merged 11 commits intodevfrom
feat/review-suggestion-blocks
Mar 27, 2026
Merged

Feat/review suggestion blocks#52
factory-nizar merged 11 commits intodevfrom
feat/review-suggestion-blocks

Conversation

@factory-nizar
Copy link
Copy Markdown
Contributor

@factory-nizar factory-nizar commented Mar 3, 2026

Add review-bot instructions to include suggestion blocks when the fix is high-confidence, using RIGHT-side anchors and a 250-line cap.

There's a bit of repetition in the prompts but feel free to correct me if anything is not relevant:

candidate: generates comment bodies, so would naturally want to add suggestions there as well
Validator: enforcement, reject if anchor changes
Review prompt: needed in the case a single pass is enabled

examples

or in this pr we are in currently

I did focus on anchoring on the right, since applying suggestions on deleted lines is not currently supported
image

Followup: adding guidance about including suggestions or not based on a boolean in this pr is not super important if we are moving the prompts to skills, but might be a nice in between till I get everything working.

Closes FAC-16670

@factory-nizar factory-nizar force-pushed the feat/review-suggestion-blocks branch 2 times, most recently from 7f1cb65 to 987bf4d Compare March 3, 2026 23:48
@factory-nizar factory-nizar marked this pull request as ready for review March 4, 2026 00:15
@factory-droid
Copy link
Copy Markdown
Contributor

factory-droid bot commented Mar 4, 2026

Droid finished @factory-nizar's task —— View job


Droid reviewed PR #52 — 1 finding approved (P1: blanket anchor-content restriction suppresses replacement suggestions), 1 rejected (speculative snippet-cap conflict). View review

@factory-nizar
Copy link
Copy Markdown
Contributor Author

btw @jonathan-factory, @lukealvoeiro was suggesting 250 lines might be too big. I guess most suggestions even if they had egregious white space issues should not be more than sub 100?

Copy link
Copy Markdown
Contributor

@factory-droid factory-droid bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One approved finding: the blanket "do not change the anchor line content" instruction at line 352 contradicts the <replacement code> template and the suggestion blocks Rules section (which correctly scopes the unchanged-anchor restriction to insert-only suggestions). This would suppress the most common type of suggestion—single-line replacements. The suggested fix properly narrows the restriction to insert-only cases.

@jonathan-factory
Copy link
Copy Markdown

btw @jonathan-factory, @lukealvoeiro was suggesting 250 lines might be too big. I guess most suggestions even if they had egregious white space issues should not be more than sub 100?

Yeah 250 was more of a relaxed upper bound to use as a failsafe against overly large comments, no concerns reducing to something like 100

Copy link
Copy Markdown

@jonathan-factory jonathan-factory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG after re-running evals

factory-nizar and others added 8 commits March 23, 2026 10:33
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

fix(review): escape suggestion blocks in prompt

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

fix(review): enforce suggestion-only replacements

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

update
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
@factory-nizar factory-nizar force-pushed the feat/review-suggestion-blocks branch from 916f6dc to 498436d Compare March 23, 2026 17:35
factory-nizar and others added 3 commits March 23, 2026 12:15
…ions parameter

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
@factory-nizar
Copy link
Copy Markdown
Contributor Author

Benchmark: v3 baseline vs suggestion blocks + dedup (50 PRs, 5 repos)

Run Precision Recall F1 TP FP FN
v3 baseline 63.2% 57.6% 60.3% 91 53 67
+suggestions+dedup 55.8% 60.8% 58.2% 96 76 62

+5 TPs, +23 FPs. Recall up 3.2pp, precision down 7.4pp, F1 down 2.1pp.

### Deduplication (STRICT)

Before approving a candidate, check for duplicates:
1. **Among candidates**: If two or more candidates describe the same underlying bug (same root cause, even if anchored to different lines or worded differently), approve only the ONE with the best anchor and clearest explanation. Reject the rest with reason "duplicate of candidate N".
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also include this dedupe block in the core review skill?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup! I can make sure it's there when it's moved to mono-repo ( if it's not there)

@factory-nizar factory-nizar merged commit db4bdca into dev Mar 27, 2026
5 checks passed
@factory-nizar factory-nizar deleted the feat/review-suggestion-blocks branch March 27, 2026 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants