Correct suggested-code scoping guidance to match one-click anchor mechanics

A lone inserted line matches no existing file line and cannot be anchored; bracket the new line with one adjacent unchanged line instead. Reductive collapses omit suggested-code and fall back to a manual snippet.
This commit is contained in:
wenjiefan 2026-07-15 18:45:09 +02:00
parent a39756c0a3
commit bff2e7398c

View file

@ -216,9 +216,9 @@ A review super-skill MUST preserve `domain` verbatim when rolling a leaf finding
**Scope `location` and `suggested-code` to exactly the lines that change — no wider, no narrower.** `location` MUST cover every original line the replacement rewrites and MUST NOT extend to surrounding lines that stay the same; the delivery surface replaces precisely the lines `location` names with `suggested-code`, so a mismatch lands the fix on the wrong lines. Three shapes recur:
- **Adding a property to an existing member** (for example a missing `DataClassification` on a field): point `location` at the single insertion line and emit only the added line as `suggested-code`. Do not restate the enclosing field, key, or procedure — that would replace the whole block instead of inserting one line.
- **Adding a property to an existing member** (for example a missing `DataClassification` on a field): emit the new line together with just one adjacent unchanged line that brackets it — the preceding property or the member's closing `}` — and set `location` to that minimal two-line span. Do not restate the entire field, key, or procedure; reproducing the whole block makes the one-click suggestion replace everything instead of inserting the single line.
- **Rewriting one statement** (for example `exit (X)``exit(X)`): point `location.line` at that statement's line, not at the enclosing procedure signature or `begin`/`end`.
- **Collapsing several source lines into fewer** (for example replacing a one-line `Count() > 0` guard and its follow-up existence check with a single `not IsEmpty()` line): set `location.range` to span every original line being removed so the one-click replacement consumes them together, and emit the shorter result as `suggested-code`.
- **Collapsing several source lines into fewer** (for example replacing a one-line `Count() > 0` guard and a follow-up existence check on the next line with a single `not IsEmpty()` line): the one-click mechanism replaces only the lines it can match, so a shorter `suggested-code` cannot express the removal of the extra original line and would land on just one of them. Omit `suggested-code`, set `suggested-code-omission-reason` (for example `fix removes lines; manual edit needed`), and describe the collapsed result in `message`.
Emit `suggested-code` whenever the fix is small, local, and mechanical: deleting unreachable code; replacing one expression (`Count() > 0``not IsEmpty()`); moving a local `Label` to object scope; adding a missing property such as `ToolTip`, `OptionCaption`, or `DataClassification`; replacing a string-concatenated `Error` with a Label-backed call; changing a permission token; or adding a missing `else`/guard branch whose replacement is unambiguous from the surrounding diff. When a `.good.al` companion exists and the diff context matches the `.bad.al` shape, prefer adapting the `.good.al` replacement into `suggested-code`.