mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 09:26:52 +01:00
Scope suggested-code and location to exactly the changed lines
Addresses reviewer-reported misplaced suggestions from the eval: insert-only-property emitting the whole field, single-statement rewrites anchored on the procedure name, and reductive multi-line collapses. The skill now emits a location range that matches precisely the rewritten lines.
This commit is contained in:
parent
8132d8a87d
commit
a39756c0a3
1 changed files with 6 additions and 0 deletions
|
|
@ -214,6 +214,12 @@ A review super-skill MUST preserve `domain` verbatim when rolling a leaf finding
|
|||
|
||||
**`findings[].suggested-code`** — optional in the schema but **expected for mechanical findings**. It is a concrete code-replacement payload for the lines indicated by `location`. When present, the string MUST be a literal replacement for the source lines covered by `location.line` (or `location.range` if set) — i.e., what the file would contain after the fix, with no surrounding diff markers, fences, or commentary. Consumers MAY render it as a one-click suggestion in the delivery surface (for example, a GitHub ```` ```suggestion ```` block).
|
||||
|
||||
**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.
|
||||
- **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`.
|
||||
|
||||
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`.
|
||||
|
||||
Omit `suggested-code` only when the appropriate fix depends on context the skill cannot determine, when multiple defensible replacements exist, or when the fix spans non-contiguous code. If a finding is mechanical-looking but `suggested-code` is omitted, set `findings[].suggested-code-omission-reason` to a short explanation (for example, `requires choosing a real event id` or `fix spans multiple non-contiguous locations`). The `suggested-code` payload supplements `message`; it does not replace the explanation in `message`.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue