Merge pull request #40 from microsoft/jesperschulz-bc-knowledge-gap-analysis

Add error-handling knowledge domain and review leaf skill

.github/scripts/validate_frontmatter.py

@@ -59,7 +59,7 @@ MAX_KNOWLEDGE_LINES = 100
 
 KEBAB_CASE = re.compile(r"^[a-z0-9]+(-[a-z0-9]+)*$")
 ISO_ALPHA2 = re.compile(r"^[a-z]{2}$")
-RANGE_SHORTHAND = re.compile(r"^(\d+)\.\.(\d+)$")
+RANGE_SHORTHAND = re.compile(r"^(\d+)\.\.(\d+)?$")
 FENCED_CODE_BLOCK = re.compile(r"^```", re.MULTILINE)
 HEADING_H2 = re.compile(r"^##\s+(.+?)\s*$", re.MULTILINE)
 
@@ -149,6 +149,8 @@ def expand_bc_version(value: Any) -> tuple[list[int] | str | None, str | None]:
     """Return (expanded, error-message). One of the two is None.
 
     For the universal sentinel ["all"], `expanded` is the string "all".
+    For an open-ended range like ["26.."], `expanded` is the normalized
+    string "26.." (it cannot be enumerated; consumers match target >= 26).
     Otherwise it is the expanded list of version integers.
     """
     if not isinstance(value, list) or not value:
@@ -163,15 +165,19 @@ def expand_bc_version(value: Any) -> tuple[list[int] | str | None, str | None]:
         if any(v <= 0 for v in value):
             return None, "integers must be positive"
         return sorted(set(value)), None
-    # Case 2: single-element range-shorthand like "[26..28]"
+    # Case 2: single-element range shorthand — closed "[26..28]" or open-ended "[26..]"
     if len(value) == 1 and isinstance(value[0], str):
         m = RANGE_SHORTHAND.match(value[0].strip())
         if m:
-            start, end = int(m.group(1)), int(m.group(2))
+            start = int(m.group(1))
+            if m.group(2) is None:
+                # Open-ended: "start.." applies from start onwards, no upper bound.
+                return f"{start}..", None
+            end = int(m.group(2))
             if start > end:
                 return None, f"range '{value[0]}' is not ascending"
             return list(range(start, end + 1)), None
-    return None, "must be [all], a list of integers, or a single-element range shorthand like [26..28]"
+    return None, "must be [all], a list of integers, or a range shorthand like [26..28] or [26..]"
 
 
 def headings_in_order(body: str) -> list[tuple[str, int]]:

README.md

@@ -58,7 +58,7 @@ Skills define how agents consume knowledge. They come in three flavors:
 
   READ and DO are read on demand — typically when the first dispatched action skill runs. They are not prerequisites for invoking Entry. WRITE is only used when scaffolding new content.
 
-- **Action skills** — concrete skills that follow the Action Skill template to do real work (review code, audit telemetry, etc.). Action skills live inside the layers that own them (`/microsoft/skills/`, `/community/skills/`, `/custom/skills/`). An action skill is either a **leaf** that evaluates knowledge files directly, or a **super-skill** that composes other action skills (declared via `sub-skills` in frontmatter). The canonical reference is [`microsoft/skills/review/al-code-review.md`](microsoft/skills/review/al-code-review.md) (super-skill), which composes six leaf skills under [`microsoft/skills/review/`](microsoft/skills/review/) — one per knowledge domain (performance, security, privacy, upgrade, style, UI).
+- **Action skills** — concrete skills that follow the Action Skill template to do real work (review code, audit telemetry, etc.). Action skills live inside the layers that own them (`/microsoft/skills/`, `/community/skills/`, `/custom/skills/`). An action skill is either a **leaf** that evaluates knowledge files directly, or a **super-skill** that composes other action skills (declared via `sub-skills` in frontmatter). The canonical reference is [`microsoft/skills/review/al-code-review.md`](microsoft/skills/review/al-code-review.md) (super-skill), which composes seven leaf skills under [`microsoft/skills/review/`](microsoft/skills/review/) — one per knowledge domain (performance, security, privacy, upgrade, style, UI, error handling).
 
 ### Agent bootstrapping
 
@@ -72,7 +72,7 @@ Every knowledge file is a markdown file with mandatory YAML frontmatter. Files t
 
 ```yaml
 ---
-bc-version: [all]                       # or [26..28] for version-gated guidance
+bc-version: [all]                       # or [26..28], or [26..] for "26 and later"
 domain: performance                     # security | performance | ux | telemetry | ...
 keywords: [query, filtering, partial]   # free-text tags for retrieval
 technologies: [al]                      # al | javascript | powershell | ...

microsoft/knowledge/error-handling/collect-validation-errors-with-errorbehavior.bad.al

@@ -0,0 +1,21 @@
+codeunit 50187 "Collect Errors Bad Sample"
+{
+    procedure ValidateAllItems()
+    var
+        Item: Record Item;
+        ErrorText: Text;
+    begin
+        // Hand-rolled accumulation: reimplements the platform feature, loses each
+        // error's ErrorInfo structure, and skips telemetry classification.
+        if Item.FindSet() then
+            repeat
+                if Item.Description = '' then
+                    ErrorText += StrSubstNo('Item %1 has no description.\', Item."No.");
+                if Item."Unit Cost" <= 0 then
+                    ErrorText += StrSubstNo('Item %1 must have a positive unit cost.\', Item."No.");
+            until Item.Next() = 0;
+
+        if ErrorText <> '' then
+            Error(ErrorText);
+    end;
+}

microsoft/knowledge/error-handling/collect-validation-errors-with-errorbehavior.good.al

@@ -0,0 +1,37 @@
+codeunit 50185 "Collect Errors Good Sample"
+{
+    [ErrorBehavior(ErrorBehavior::Collect)]
+    procedure ValidateAllItems()
+    var
+        Item: Record Item;
+        CollectedErrors: List of [ErrorInfo];
+        CollectedError: ErrorInfo;
+        ErrorText: Text;
+    begin
+        if Item.FindSet() then
+            repeat
+                // Run each item in its own context so one failure does not abandon the rest.
+                Codeunit.Run(Codeunit::"Collect Errors Item Check", Item);
+            until Item.Next() = 0;
+
+        if HasCollectedErrors() then begin
+            CollectedErrors := GetCollectedErrors();
+            foreach CollectedError in CollectedErrors do
+                ErrorText += CollectedError.Message() + '\';
+            Message('The following must be fixed before posting:\%1', ErrorText);
+        end;
+    end;
+}
+
+codeunit 50186 "Collect Errors Item Check"
+{
+    TableNo = Item;
+
+    trigger OnRun()
+    begin
+        if Rec.Description = '' then
+            Error('Item %1 has no description.', Rec."No.");
+        if Rec."Unit Cost" <= 0 then
+            Error('Item %1 must have a positive unit cost.', Rec."No.");
+    end;
+}

microsoft/knowledge/error-handling/collect-validation-errors-with-errorbehavior.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: error-handling
+keywords: [collectible-errors, errorbehavior, collect, getcollectederrors, hascollectederrors, validation, batch]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Collect validation errors with ErrorBehavior::Collect and handle the collected list
+
+## Description
+
+By default a procedure stops on the first `Error`, so a user fixing ten bad rows must rerun the operation ten times. The collectible-errors feature postpones error handling to the end of the call: a procedure attributed `[ErrorBehavior(ErrorBehavior::Collect)]` keeps running as errors occur and gathers them, so all failures can be presented together. The collected errors are read with `HasCollectedErrors()` and `GetCollectedErrors()` (which returns a `List of [ErrorInfo]`); `ClearCollectedErrors()` empties the buffer. This is a platform mechanism most LLMs are unaware of — they reach for a manually concatenated `Text` buffer or a temporary error table instead.
+
+## Best Practice
+
+Mark the orchestrating procedure `[ErrorBehavior(ErrorBehavior::Collect)]` and run each item's validation so one failure doesn't abandon the rest — typically by calling the per-item routine through `Codeunit.Run`. When the run finishes, inspect `HasCollectedErrors()` and surface `GetCollectedErrors()` to the user as a single list. Always handle the collected errors yourself: the platform's own guidance is that any errors still in the collected list when the procedure ends are concatenated into one dialog, which is hard for users to read.
+
+See sample: `collect-validation-errors-with-errorbehavior.good.al`.
+
+## Anti Pattern
+
+Two shapes signal trouble. The first is hand-rolled accumulation — appending messages to a `Text` variable and showing them at the end — which reimplements the platform feature, loses each error's `ErrorInfo` structure, and skips telemetry classification. The second is applying `[ErrorBehavior(ErrorBehavior::Collect)]` but never calling `HasCollectedErrors`/`GetCollectedErrors`, so every collected error spills into the platform's concatenated end-of-procedure dialog. Detection: a `Collect` attribute with no matching `GetCollectedErrors` call, or a per-row loop that builds an error string by concatenation.
+
+See sample: `collect-validation-errors-with-errorbehavior.bad.al`.

microsoft/knowledge/error-handling/errortype-internal-vs-client-for-diagnostics.bad.al

@@ -0,0 +1,14 @@
+codeunit 50191 "Error Type Bad Sample"
+{
+    procedure ApplyLedgerBucket(BucketId: Integer)
+    begin
+        // Developer-facing detail shown straight to the user, and no structured telemetry signal.
+        if not BucketInitialized(BucketId) then
+            Error('Unexpected state: ledger bucket %1 not initialized', BucketId);
+    end;
+
+    local procedure BucketInitialized(BucketId: Integer): Boolean
+    begin
+        exit(false);
+    end;
+}

microsoft/knowledge/error-handling/errortype-internal-vs-client-for-diagnostics.good.al

@@ -0,0 +1,19 @@
+codeunit 50190 "Error Type Good Sample"
+{
+    procedure ApplyLedgerBucket(BucketId: Integer)
+    var
+        InternalErr: ErrorInfo;
+    begin
+        if not BucketInitialized(BucketId) then begin
+            InternalErr.ErrorType := ErrorType::Internal;
+            InternalErr.Message := StrSubstNo('Ledger bucket %1 was not initialized before posting.', BucketId);
+            InternalErr.DetailedMessage := 'Internal invariant violated. Inspect the call stack captured in telemetry.';
+            Error(InternalErr);
+        end;
+    end;
+
+    local procedure BucketInitialized(BucketId: Integer): Boolean
+    begin
+        exit(false);
+    end;
+}

microsoft/knowledge/error-handling/errortype-internal-vs-client-for-diagnostics.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: error-handling
+keywords: [errorinfo, errortype, internal, client, telemetry, diagnostics, generic-message]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Set ErrorInfo.ErrorType to Internal for defects you want in telemetry but not in the user's face
+
+## Description
+
+`ErrorInfo.ErrorType` controls where an error's message is shown. With `ErrorType::Client` — the behaviour of a normal `Error` — the message is both shown to the user and sent to telemetry. With `ErrorType::Internal` the user sees a generic message while the specific message you set is sent to telemetry only. The distinction matters for *unexpected* failures — a broken invariant, a failed internal assertion, a "this should never happen" branch — where the technical detail helps the partner diagnose the defect but would only confuse the end user. LLMs are unaware `ErrorType` exists, so they expose raw internal-failure text directly to users.
+
+## Best Practice
+
+Reserve `ErrorType::Internal` for errors the user cannot act on: corrupted internal state, an unreachable branch, a contract a caller violated. Set a precise, detail-rich `Message` and `DetailedMessage` for telemetry, raise it via `Error(ErrorInfo)`, and let the platform show the user a generic dialog. Keep `ErrorType::Client` (or a plain `Error`) for failures the user is expected to read and resolve — validation messages, missing setup, business-rule violations. The test is simple: if the message only makes sense to a developer, mark it `Internal`.
+
+See sample: `errortype-internal-vs-client-for-diagnostics.good.al`.
+
+## Anti Pattern
+
+Raising an internal failure with a plain `Error('Unexpected state: ledger bucket %1 not initialized', BucketId)`. The user is shown a technical message they can do nothing about, and the signal is buried in a generic error rather than carried as structured telemetry detail. Detection: an `Error` whose wording targets a developer ("unexpected", "should not happen", raw internal identifiers) raised with default `Client` visibility instead of an `ErrorInfo` marked `ErrorType::Internal`.
+
+See sample: `errortype-internal-vs-client-for-diagnostics.bad.al`.

microsoft/knowledge/error-handling/prefer-errorinfo-for-actionable-errors.bad.al

@@ -0,0 +1,21 @@
+table 50182 "Actionable Error Bad Sample"
+{
+    fields
+    {
+        field(1; "Entry No."; Integer) { }
+        field(2; "Qty. to Invoice"; Decimal)
+        {
+            trigger OnValidate()
+            begin
+                // Dead-end error: the code knows the maximum but offers the user no way to apply it.
+                if "Qty. to Invoice" > MaxQtyToInvoice() then
+                    Error('You cannot invoice more than %1 units.', MaxQtyToInvoice());
+            end;
+        }
+    }
+
+    local procedure MaxQtyToInvoice(): Decimal
+    begin
+        exit(10);
+    end;
+}

microsoft/knowledge/error-handling/prefer-errorinfo-for-actionable-errors.good.al

@@ -0,0 +1,44 @@
+table 50180 "Actionable Error Good Sample"
+{
+    fields
+    {
+        field(1; "Entry No."; Integer) { }
+        field(2; "Qty. to Invoice"; Decimal)
+        {
+            trigger OnValidate()
+            var
+                CannotInvoiceErr: ErrorInfo;
+            begin
+                if "Qty. to Invoice" > MaxQtyToInvoice() then begin
+                    CannotInvoiceErr.Title := 'Qty. to Invoice isn''t valid';
+                    CannotInvoiceErr.Message := StrSubstNo('You cannot invoice more than %1 units.', MaxQtyToInvoice());
+                    CannotInvoiceErr.DetailedMessage := 'Reduce the quantity to invoice, or apply the maximum allowed.';
+                    CannotInvoiceErr.RecordId := Rec.RecordId();
+                    CannotInvoiceErr.AddAction(
+                        StrSubstNo('Set value to %1', MaxQtyToInvoice()),
+                        Codeunit::"Actionable Error Fixit Sample",
+                        'SetQtyToMax');
+                    Error(CannotInvoiceErr);
+                end;
+            end;
+        }
+    }
+
+    local procedure MaxQtyToInvoice(): Decimal
+    begin
+        exit(10);
+    end;
+}
+
+codeunit 50181 "Actionable Error Fixit Sample"
+{
+    procedure SetQtyToMax(SourceError: ErrorInfo)
+    var
+        Line: Record "Actionable Error Good Sample";
+    begin
+        if Line.Get(SourceError.RecordId) then begin
+            Line.Validate("Qty. to Invoice", 10);
+            Line.Modify(true);
+        end;
+    end;
+}

microsoft/knowledge/error-handling/prefer-errorinfo-for-actionable-errors.md

@@ -0,0 +1,26 @@
+---
+bc-version: [23..]
+domain: error-handling
+keywords: [errorinfo, actionable-errors, fix-it, show-it, addaction, addnavigationaction, error-dialog]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Prefer ErrorInfo with recommended actions over a plain Error for recoverable failures
+
+## Description
+
+A plain `Error('text')` ends the operation with a dead-end dialog: the user reads the message but the system offers no way forward. The `ErrorInfo` data type, combined with the actionable-errors framework added in 2023 release wave 2, lets an error carry a recommended action the user can take to unblock themselves without leaving their task. Two kinds exist: a **Fix-it** action (`AddAction`), used when the code already knows the correct value and can apply it in one step, and a **Show-it** action (`AddNavigationAction` together with `PageNo`), used when the correction lives on a related record the user should be taken to. An error dialog renders at most two recommended actions. LLMs trained on older AL almost always emit a bare `Error(...)` and rarely reach for `ErrorInfo`, so this guidance is remedial.
+
+## Best Practice
+
+Build an `ErrorInfo`, set `Title`, `Message`, and `DetailedMessage`, then attach the action that matches the situation. For a Fix-it, call `AddAction(Caption, Codeunit::Handler, 'MethodName')` where the handler method (which receives the `ErrorInfo`) applies the known-good value; phrase the caption as "Set value to …". For a Show-it, set `PageNo := Page::"…"`, set `RecordId` so navigation opens the right record, and call `AddNavigationAction('Show …')`. Raise it with `Error(ErrorInfo)`. Reserve recommended actions for cases where the solution is genuinely known and the user has permission to apply it.
+
+See sample: `prefer-errorinfo-for-actionable-errors.good.al`.
+
+## Anti Pattern
+
+Surfacing a recoverable validation failure with `Error('You cannot invoice more than %1 units.', MaxQty)` and nothing else. The user is blocked with no offered remedy even though the code knows the maximum and could set it. The detection signal: an `Error` call in a validation or posting path whose message names a specific correct value or a specific related page, with no surrounding `ErrorInfo`, `AddAction`, or `AddNavigationAction`. Replace it with an `ErrorInfo` that carries the corresponding Fix-it or Show-it action.
+
+See sample: `prefer-errorinfo-for-actionable-errors.bad.al`.

microsoft/skills/review/al-code-review.md

@@ -3,7 +3,7 @@ kind: action-skill
 id: al-code-review
 version: 1
 title: AL code review
-description: Reviews AL source changes by composing the AL review leaf skills (performance, security, privacy, upgrade, style, UI).
+description: Reviews AL source changes by composing the AL review leaf skills (performance, security, privacy, upgrade, style, UI, error handling).
 inputs: [pr-diff, file-path]
 outputs: [findings-report]
 bc-version: [all]
@@ -17,6 +17,7 @@ sub-skills:
   - microsoft/skills/review/al-upgrade-review.md
   - microsoft/skills/review/al-style-review.md
   - microsoft/skills/review/al-ui-review.md
+  - microsoft/skills/review/al-error-handling-review.md
 ---
 
 # AL code review
@@ -37,6 +38,7 @@ The sub-skills invoked by this skill are those listed in frontmatter `sub-skills
 - `microsoft/skills/review/al-upgrade-review.md`
 - `microsoft/skills/review/al-style-review.md`
 - `microsoft/skills/review/al-ui-review.md`
+- `microsoft/skills/review/al-error-handling-review.md`
 
 Additional leaf skills (for example, telemetry, testing) are added by updating the `sub-skills` list. The skill does not discover sub-skills implicitly.
 

microsoft/skills/review/al-error-handling-review.md

@@ -0,0 +1,136 @@
+---
+kind: action-skill
+id: al-error-handling-review
+version: 1
+title: AL error handling review
+description: Reviews AL source changes against error-handling guidance from BCQuality.
+inputs: [pr-diff, file-path]
+outputs: [findings-report]
+bc-version: [all]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# AL error handling review
+
+Reviews AL source changes against the `error-handling` knowledge domain in BCQuality and emits a findings report. This is a leaf action skill: it invokes no sub-skills. It is one of the skills composed by `al-code-review`.
+
+An orchestrator invokes this skill with either a `pr-diff` (the standard PR-review entry point) or a `file-path` (single-file review). The skill produces a single JSON document conforming to the DO output contract.
+
+## Source
+
+Read the BCQuality knowledge index once — the `knowledge-index.json` BCQuality builds at the root of the knowledge checkout (Entry's preparation step regenerates it over the live, already-filtered clone — see `skills/entry.md`). It lists every article that survived layer and allow/deny filtering and carries, per article, its `path`, `layer`, `domain`, frontmatter dimensions, `keywords`, `title`, and a one-line `description` hint — exactly the fields Relevance and Worklist consume. Take the index entries whose `domain` is `error-handling` as this skill's candidate set across every enabled layer; do not open the individual article files at this step. Open an article's full body only once it enters the Worklist below, so a review reads the index plus the handful of worklisted articles instead of every file under `*/knowledge/error-handling/**`.
+
+## Relevance
+
+Apply the frontmatter matching rules defined in READ (*Frontmatter matching semantics*) against the task context:
+
+- `bc-version` — the target BC version from the PR branch's `app.json` or the orchestrator-supplied version. If unavailable, the dimension is `unknown`.
+- `technologies` — `[al]`.
+- `countries` — the countries declared in the consuming app's `app.json`. Default to the orchestrator's configured context; if absent, `unknown`.
+- `application-area` — the union of application areas declared by the changed objects. Pass the actual set; do not substitute `[all]`. If the area cannot be determined from the changes, the dimension is `unknown`.
+
+Discard files that are not applicable. Retain conditionally applicable files (any dimension `unknown`) only when the orchestrator's configuration permits them; findings derived from those files MUST have `confidence` no higher than `medium`, AND the finding's `message` MUST name the dimension or dimensions that were unknown.
+
+## Worklist
+
+Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against:
+
+- The changed AL object names and types — especially codeunits that post or validate, tables and table extensions with `OnValidate` triggers, and any procedure that raises errors or orchestrates a batch over records.
+- The changed procedures and triggers, weighted toward `OnValidate`/`OnInsert`/`OnModify` triggers, posting and validation routines, and procedures attributed with `[ErrorBehavior(...)]`.
+- Tokens extracted from the diff that relate to error surfacing and diagnostics (`Error`, `ErrorInfo`, `Title`, `Message`, `DetailedMessage`, `AddAction`, `AddNavigationAction`, `RecordId`, `PageNo`, `ErrorBehavior`, `Collect`, `HasCollectedErrors`, `GetCollectedErrors`, `ClearCollectedErrors`, `ErrorType`, `Internal`, `Client`).
+
+A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
+
+Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`.
+
+When the post-conflict worklist is empty because no applicable error-handling knowledge exists, or because configuration suppressed every candidate, emit `outcome: "no-knowledge"`. When the worklist is empty because no applicable error-handling knowledge matched the changes, emit `outcome: "completed"` with an empty `findings` array.
+
+## Action
+
+For each worklist entry, evaluate the diff against the file's `## Best Practice` and `## Anti Pattern` sections. Emit findings as follows:
+
+- When the diff contains a clear match for an Anti Pattern, emit a finding with severity `major` or `blocker`, a message summarizing the anti-pattern, `location` pointing to the offending line or range, and a `references` entry pointing to the knowledge file. Use `blocker` only when the knowledge file states the anti-pattern violates a platform-level guarantee. When the file does not make such a claim, the ceiling is `major`.
+- When the diff contains code that contradicts a Best Practice without being a full anti-pattern, emit `minor` with the same reference shape.
+- When the skill cannot detect a violation but the file is clearly applicable to the change, emit `info` citing the file. Repository-wide observations MAY omit `location`.
+
+Set `confidence` to:
+
+- `high` when the detection is based on an unambiguous pattern match (identifier, syntax, object type).
+- `medium` when detection relies on heuristics or when any frontmatter dimension was `unknown`.
+- `low` when the finding is an advisory derived only from applicability.
+
+After evaluating each worklist entry, also consider whether the diff exhibits an error-handling defect the agent recognises from its general AL knowledge that no knowledge file in the worklist covers. Such candidates are agent findings within this skill's domain — emit them with `references: []`, an `id` slug prefixed with `agent:`, `confidence` capped at `medium`, `severity` capped at `minor` (agent findings are advisory and non-gating), and a `message` that is self-contained (describing both the issue and a concrete recommendation, since there is no knowledge-file footer for the consumer to fall back on). Hold every candidate to the precision bar in `skills/do.md` (*Agent findings*): emit only a concrete, material error-handling defect a knowledgeable BC reviewer would agree is wrong — steelman it first and drop anything stylistic, speculative, dependent on code outside the diff, or merely a valid alternative; when in doubt, omit. The scope is strictly error handling; defects outside this domain belong to other leaves and MUST NOT be emitted here. Before emitting, check the worklist for a knowledge file that matches the candidate — if one exists, upgrade the candidate to a knowledge-backed finding instead. See `skills/do.md` for the full contract.
+
+For every emitted finding, decide whether the fix is mechanical. A fix is mechanical when it is small, local, and unambiguous from the diff context (for example: replace a string-concatenated `Error` with a Label-backed call; mark an internal-only failure `ErrorType::Internal`; add a missing `DetailedMessage`). For mechanical findings, emit `findings[].suggested-code` with the literal replacement for the source lines indicated by `location`. The payload must be a verbatim replacement — no diff markers, no fences, no commentary — that the consumer can render as a one-click suggestion. When a `.good.al` companion exists and the diff context matches the `.bad.al` shape, adapt 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 you omit `suggested-code`, set `findings[].suggested-code-omission-reason` to a short explanation. See `skills/do.md` for the full contract.
+
+Outcome selection:
+
+- `completed` — the skill evaluated every worklist item; default when the skill finishes normally, including when the resulting `findings` array is empty.
+- `no-knowledge` — no applicable error-handling knowledge survived Source, Relevance, configuration filtering, and conflict resolution. `findings` is empty.
+- `not-applicable` — the task context lacks an AL dimension (no AL changes in the diff, or `technologies` filter rejected the task).
+- `partial` — a time or token budget was hit before the worklist was exhausted. `summary.coverage` reflects the evaluated subset; `outcome-reason` explains the cause.
+- `failed` — an unrecoverable error occurred. `outcome-reason` is required.
+
+## Output
+
+Output conforms to the DO output contract. A populated example:
+
+```json
+{
+  "skill": { "id": "al-error-handling-review", "version": 1 },
+  "outcome": "completed",
+  "summary": {
+    "counts": { "blocker": 0, "major": 1, "minor": 1, "info": 0 },
+    "coverage": { "worklist-size": 2, "items-evaluated": 2 }
+  },
+  "findings": [
+    {
+      "id": "microsoft/knowledge/error-handling/prefer-errorinfo-for-actionable-errors.md",
+      "severity": "major",
+      "message": "A validation error names the maximum allowed quantity but raises a plain Error with no recommended action. Use an ErrorInfo with a Fix-it AddAction so the user can apply the known value.",
+      "location": {
+        "file": "src/Sales/SalesLine.TableExt.al",
+        "line": 88,
+        "range": { "start-line": 86, "end-line": 89 }
+      },
+      "references": [
+        { "path": "microsoft/knowledge/error-handling/prefer-errorinfo-for-actionable-errors.md" }
+      ],
+      "confidence": "high"
+    },
+    {
+      "id": "microsoft/knowledge/error-handling/errortype-internal-vs-client-for-diagnostics.md",
+      "severity": "minor",
+      "message": "This 'unexpected state' failure is developer-facing but is raised with default Client visibility. Mark it ErrorType::Internal so the detail goes to telemetry and the user sees a generic message.",
+      "location": {
+        "file": "src/Ledger/PostingEngine.Codeunit.al",
+        "line": 211
+      },
+      "references": [
+        { "path": "microsoft/knowledge/error-handling/errortype-internal-vs-client-for-diagnostics.md" }
+      ],
+      "confidence": "medium"
+    }
+  ],
+  "suppressed": []
+}
+```
+
+The empty-corpus case — BCQuality's state until error-handling knowledge files land — produces:
+
+```json
+{
+  "skill": { "id": "al-error-handling-review", "version": 1 },
+  "outcome": "no-knowledge",
+  "summary": {
+    "counts": { "blocker": 0, "major": 0, "minor": 0, "info": 0 },
+    "coverage": { "worklist-size": 0, "items-evaluated": 0 }
+  },
+  "findings": [],
+  "suppressed": []
+}
+```

skills/read.md

@@ -26,7 +26,7 @@ A file that violates any of these rules is invalid and MUST be skipped by consum
 
 ```yaml
 ---
-bc-version: [all]                       # or [26, 27, 28] or the range shorthand [26..28]
+bc-version: [all]                       # or [26, 27, 28], the range [26..28], or the open-ended range [26..]
 domain: performance
 keywords: [query, filtering, partial]
 technologies: [al]
@@ -39,13 +39,14 @@ All six fields are required. Missing or empty fields invalidate the file.
 
 ### Fields
 
-**`bc-version`** — Array. The Business Central major versions this file applies to. Three forms are accepted:
+**`bc-version`** — Array. The Business Central major versions this file applies to. Four forms are accepted:
 
 - Universal sentinel: `[all]` means the guidance applies to every BC version and matches any target.
 - Explicit list: `[26, 27, 28]`.
-- Range shorthand: `[26..28]` means every integer from 26 through 28 inclusive.
+- Closed range shorthand: `[26..28]` means every integer from 26 through 28 inclusive.
+- Open-ended range shorthand: `[26..]` means version 26 and every later version, with no upper bound. Use it for guidance tied to a feature introduced in a specific version that is not expected to be removed.
 
-`[all]` is mutually exclusive with explicit versions; do not combine. Consumers MUST expand ranges to the full set before comparison.
+`[all]` is mutually exclusive with explicit versions; do not combine. Consumers MUST expand closed ranges to the full set before comparison; an open-ended range `[N..]` is not enumerable and instead matches any target version greater than or equal to `N`.
 
 **`domain`** — String. A single domain tag that places the file within a broader area of concern. Standard values include `performance`, `security`, `ux`, `telemetry`, `testing`, `api`, `pipelines`, `finance`, `supply-chain`, `manufacturing`, `jobs`. New domains may be introduced by contributors; no closed enumeration is enforced at the schema level. Consumers MUST treat unknown domains as valid.
 
@@ -94,7 +95,7 @@ Conflict detection is the consumer's responsibility; BCQuality does not enforce
 
 When a consumer filters or matches files against a task context, these rules apply:
 
-- **`bc-version`** — the file matches if its set is `[all]`, or if the target BC version is an element of the file's expanded `bc-version` set. Range shorthand (`[26..28]`) MUST be expanded before comparison.
+- **`bc-version`** — the file matches if its set is `[all]`, or if the target BC version is an element of the file's expanded `bc-version` set. Closed range shorthand (`[26..28]`) MUST be expanded before comparison; an open-ended range (`[26..]`) matches when the target BC version is greater than or equal to its lower bound.
 - **`technologies`** — non-empty intersection between the task's technologies and the file's technologies. There is no sentinel for this field.
 - **`countries`** — the file matches if its set contains `w1`, or if there is a non-empty intersection with the task's countries.
 - **`application-area`** — the file matches if its set contains `all`, or if there is a non-empty intersection with the task's application areas.

skills/write.md

@@ -43,7 +43,7 @@ Knowledge files do not contain code. Samples live as **sibling files** next to t
 
 ## Choosing frontmatter values
 
-**`bc-version`.** Default to `[all]` when the guidance is universal — a BC language pattern, a property on a long-standing platform type, a CodeCop rule, or a platform behaviour that has not changed across versions. Use an explicit list or range (`[26, 27, 28]`, `[26..28]`) only when the guidance is tied to a version-gated API, a deprecation, or platform behaviour that genuinely differs across versions. Most knowledge files should be `[all]`; reach for a range only with a concrete reason.
+**`bc-version`.** Default to `[all]` when the guidance is universal — a BC language pattern, a property on a long-standing platform type, a CodeCop rule, or a platform behaviour that has not changed across versions. Use an explicit list or range (`[26, 27, 28]`, `[26..28]`) only when the guidance is tied to a version-gated API, a deprecation, or platform behaviour that genuinely differs across versions. When guidance applies to a feature introduced in version N and not expected to be removed, prefer the open-ended range `[N..]` over a closed range so the file keeps matching future versions — reserve a closed upper bound for guidance that genuinely stops applying (for example, a behaviour removed or replaced in a later version). Most knowledge files should be `[all]`; reach for a range only with a concrete reason.
 
 **`domain`.** Pick one. If two fit, the file is probably two concerns. If no existing domain fits, introduce a new one — domains are open. Prefer existing domains when they are a reasonable fit, to keep retrieval predictable.