Merge pull request #124 from microsoft/bcq/642303-fp-guards

Add FP guards: field relocation to tableextension + event parameter addition (bug 642303)

.claude-plugin/marketplace.json

@@ -9,7 +9,10 @@
       "name": "bcquality",
       "source": "./",
       "description": "Business Central AL quality knowledge base and review skills, packaged as an installable plugin. Ships the entire BCQuality tree (skills, knowledge, tools) so the Entry routing protocol runs against the installed clone.",
-      "version": "0.1.0"
+      "version": "0.1.0",
+      "skills": [
+        "./skills/bcquality-al-review/"
+      ]
     }
   ]
 }

.github/scripts/validate_frontmatter.py

@@ -62,6 +62,7 @@ ISO_ALPHA2 = re.compile(r"^[a-z]{2}$")
 RANGE_SHORTHAND = re.compile(r"^(\d+)\.\.(\d+)?$")
 FENCED_CODE_BLOCK = re.compile(r"^```", re.MULTILINE)
 HEADING_H2 = re.compile(r"^##\s+(.+?)\s*$", re.MULTILINE)
+SAMPLE_REFERENCE = re.compile(r"`([a-z0-9]+(?:-[a-z0-9]+)*\.(?:good|bad)\.[a-z0-9]+)`")
 
 
 # --- Diagnostics ------------------------------------------------------------
@@ -222,6 +223,13 @@ def validate_knowledge(path: Path, parsed: Parsed, report: Report) -> None:
     if "domain" in fm:
         if not isinstance(fm["domain"], str) or not fm["domain"].strip():
             report.error(path, "R04", "domain must be a non-empty string", 1)
+        elif fm["domain"] != path.parent.name:
+            report.error(
+                path,
+                "R27",
+                f"frontmatter domain '{fm['domain']}' must match directory '{path.parent.name}'",
+                1,
+            )
 
     # R05 keywords
     if "keywords" in fm:
@@ -477,7 +485,16 @@ def validate_samples_in_domain(domain_dir: Path, root: Path, report: Report) ->
     """R14: every non-.md file must match <slug>.<kind>.<ext> with <slug>.md present."""
     if not domain_dir.is_dir():
         return
-    article_slugs = {p.stem for p in domain_dir.glob("*.md")}
+    articles = {p.stem: p for p in domain_dir.glob("*.md")}
+    article_slugs = set(articles)
+    article_texts: dict[str, str] = {}
+    for slug, article in articles.items():
+        try:
+            article_texts[slug] = article.read_text(encoding="utf-8")
+        except UnicodeDecodeError:
+            # R01 reports this during the article pass.
+            continue
+
     for entry in domain_dir.iterdir():
         if not entry.is_file() or entry.suffix == ".md":
             continue
@@ -491,9 +508,24 @@ def validate_samples_in_domain(domain_dir: Path, root: Path, report: Report) ->
         kind = m.group("kind")
         if slug not in article_slugs:
             report.error(entry, "R14", f"orphan sample: no matching article '{slug}.md' in {domain_dir.relative_to(root).as_posix()}")
+        elif entry.name not in article_texts.get(slug, ""):
+            report.error(
+                entry,
+                "R28",
+                f"sample is not referenced by its article '{slug}.md'",
+            )
         if kind not in VALID_SAMPLE_KINDS:
             report.warn(entry, "R14", f"non-standard sample kind '{kind}'; standard kinds are {sorted(VALID_SAMPLE_KINDS)}")
 
+    for slug, article in articles.items():
+        for sample_name in SAMPLE_REFERENCE.findall(article_texts.get(slug, "")):
+            if not (domain_dir / sample_name).is_file():
+                report.error(
+                    article,
+                    "R28",
+                    f"referenced sample does not exist: '{sample_name}'",
+                )
+
 
 # --- Orchestration ----------------------------------------------------------
 

.github/workflows/review-fixtures.yml

@@ -0,0 +1,18 @@
+name: Validate AL review fixtures
+
+on:
+  pull_request:
+    branches: [main]
+  push:
+    branches: [main]
+
+jobs:
+  validate-review-fixtures:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out repository
+        uses: actions/checkout@v4
+
+      - name: Validate review evaluation corpus
+        shell: pwsh
+        run: ./tools/Test-ReviewFixtures.ps1 -Root . -PrepareDirectory "$env:RUNNER_TEMP/bcquality-review-fixtures"

README.md

@@ -18,6 +18,8 @@ Poor fit: "Use HTTPS instead of HTTP." "Don't hardcode secrets." "Keep transacti
 
 The practical consequence: when a code-review agent flags something it shouldn't have, or misses something it should have caught, the remedy is a new knowledge file. When it already behaves correctly on a topic, no file is needed.
 
+A file that *prevents* a false positive — documenting why a pattern is legitimate so the agent stops flagging it — is as valid as one that catches a defect: negative clarifications are first-class knowledge files. What never belongs is a BC fact hard-coded into a skill. Skills are finders and appliers; knowledge files are what the agent knows. See [`skills/do.md`](skills/do.md) and [`skills/write.md`](skills/write.md).
+
 ## What's in this repo
 
 BCQuality contains **knowledge** and **skills**. It does not contain agents. Agents that consume BCQuality ship with [AL-Go](https://github.com/microsoft/AL-Go) and other orchestrators.
@@ -88,18 +90,9 @@ Code examples belong in separate files, not in the knowledge file itself. Knowle
 
 ## Scope
 
-BCQuality covers Business Central broadly — the application domains it supports, the technologies used to extend it, and the practices that keep implementations healthy. The scope includes:
+The current curated corpus is focused on **technical AL code review**: AppSource and compatibility, data modeling, error handling, events, interfaces, performance, privacy, Query objects, security, style, telemetry, testing, UI, upgrade, and web services. These are the domains backed by knowledge files and registered review leaves today.
 
-- **Business Central domains** — Finance, Supply Chain Management, Manufacturing, Jobs, Warehousing, Service, and the many other functional areas BC covers. Domain knowledge helps agents understand the business context they are working in.
-- AL language patterns and anti-patterns
-- PowerShell scripting for BC
-- Pipelines (AL-Go, GitHub Actions)
-- Business Central APIs
-- Power Platform integration
-- Telemetry and KQL
-- AppSource lifecycle
-
-A BC developer's actual job spans all of this, and BCQuality reflects that.
+Business Central functional domains (Finance, Supply Chain Management, Manufacturing, Jobs, Warehousing, Service), PowerShell, pipelines, and Power Platform remain valid future repository scope, but they are **not current coverage claims** until corresponding knowledge and action skills exist. Consumers should derive supported review scope from the live knowledge index and dispatched skills, not from roadmap breadth.
 
 ## How agents consume BCQuality
 
@@ -122,6 +115,7 @@ For the end-to-end flow — from orchestrator trigger through to how output reac
 
 ```
 ├── /skills/              # Global: entry-point skill + meta-skill contracts (READ, DO, WRITE)
+├── /evaluation/          # Neutral good/bad review fixtures and scoring contract
 ├── /.github/             # Actions and workflows
 ├── /microsoft/           # Microsoft-endorsed layer
 │   ├── /knowledge/       # Knowledge files by domain
@@ -155,9 +149,12 @@ Contributions are welcome. Before submitting a PR:
 1. Read the knowledge file format above — frontmatter and sections are validated by CI.
 2. Keep files atomic: one concern per file, under 100 lines.
 3. Target your contribution to the right layer — most community contributions go in `/community/knowledge/`.
+4. Adding a BC fact — or stopping the agent from flagging a false positive — is a knowledge file, not a skill edit. If a PR changes *what* a review skill flags, the change almost certainly belongs in a knowledge file. See [`skills/write.md`](skills/write.md).
 
 CI runs validation on every PR. If your knowledge file has schema violations, missing sections, code blocks, or exceeds 100 lines, the check will fail with a clear error message.
 
+Companion samples must be referenced by filename from their article, and every referenced sample must exist. The review evaluation corpus under [`evaluation/`](evaluation/) adds one positive and one clean control for every registered AL review leaf; see [`evaluation/README.md`](evaluation/README.md) for credential-free validation and optional fast-model scoring.
+
 ## License
 
 [MIT](LICENSE)

agent-consumption.md

@@ -21,7 +21,7 @@ flowchart LR
     E -->|3 dispatch record| A
     A -->|4 invoke dispatched skill| S[Action skill<br/>e.g. al-code-review]
     S -->|5 execute| P[Source → Relevance<br/>→ Worklist → Action<br/>reading READ · DO on demand]
-    P -->|6 emit| R[Findings · References<br/>· Confidence]
+    P -->|6 emit| R[Findings · Domain labels<br/>· References · Confidence]
     R -->|7 integrate| O
 ```
 
@@ -65,6 +65,7 @@ The output contract is defined in the DO meta-skill so that every action skill
 
 - **Outcome** — `completed`, `not-applicable`, `no-knowledge`, `partial`, or `failed`. An orchestrator can distinguish a clean run from a no-op from a failure without guessing.
 - **Findings** — what the skill observed (severity, message, optional location).
+- **Domain** — the producer-owned, human-readable display label on each review finding.
 - **References** — structured objects (`path` plus optional commit `sha`) pointing to the knowledge files that informed each finding.
 - **Confidence** — per-finding evidence strength.
 - **Suppressed** — knowledge files that were discarded by layer precedence or configuration, so reviewers can see what was overridden.
@@ -78,12 +79,12 @@ The orchestrator turns findings into PR comments, build gates, or IDE diagnostic
 
 BCQuality is an **additive** knowledge layer. The agent surfaces two kinds of findings, both shaped to the same DO output contract:
 
-- **Knowledge-backed findings** carry one or more entries in `references[]` pointing at BCQuality knowledge files. Their `id` is the primary file's repo-relative path. These are produced by leaf sub-skills and rolled up by super-skills.
-- **Agent findings** are surfaced by a super-skill from its own self-review pass when no BCQuality knowledge file backs the concern. They are tagged with `from-sub-skill: "agent"`, carry an empty `references: []`, use a slug `id` prefixed `agent:`, and have `confidence` capped at `medium`. Their `message` is self-contained because there is no knowledge-file footer to fall back on.
+- **Knowledge-backed findings** carry one or more entries in `references[]` pointing at BCQuality knowledge files. Their `id` is the primary file's repo-relative path. Leaf sub-skills set `domain` to their human-readable display label, and super-skills preserve it verbatim during rollup.
+- **Agent findings** carry an empty `references: []`, use a slug `id` prefixed `agent:`, and have `confidence` capped at `medium`. A leaf can emit one strictly within its own domain and uses that leaf's display label. A super-skill can emit a cross-cutting agent finding with `from-sub-skill: "agent"` and `domain: "Agent"`. Their `message` is self-contained because there is no knowledge-file footer to fall back on.
 
-Before a super-skill emits an agent finding, it validates the candidate against the BCQuality knowledge already loaded for the task: a matching file upgrades the candidate to a knowledge-backed finding (and merges or deduplicates against the relevant sub-skill output); a contradicting file suppresses the candidate. Only candidates with no BCQuality coverage become agent findings.
+Before a skill emits an agent finding, it validates the candidate against the BCQuality knowledge already loaded for the task: a matching file upgrades the candidate to a knowledge-backed finding (and merges or deduplicates against relevant existing output); a contradicting file suppresses the candidate. Only candidates with no BCQuality coverage become agent findings.
 
-Orchestrators MAY render the two kinds differently — for example, by labelling agent findings or routing them to a separate review domain — and MAY apply independent severity floors. The `from-sub-skill: "agent"` marker is the contract.
+Orchestrators MUST tolerate an absent `domain` in reports from older producers. When it is present, treat it as display text rather than an identifier: preserve the full string and its case, whitespace, punctuation, and non-ASCII characters, escaping only for the target rendering format. Do not tokenize it on spaces or use a lowercased or slugified form as the sole metadata or deduplication key, because distinct labels can collapse to the same slug. Retain the exact string, use a lossless encoding, or use a collision-resistant digest instead. Orchestrators MAY render knowledge-backed and agent findings differently and MAY apply independent severity floors; `references: []` and the `agent:` id prefix distinguish agent findings, while `from-sub-skill: "agent"` identifies those emitted by the super-skill itself.
 
 ## Why this architecture
 

community/knowledge/error-handling/table-event-subscriber-rolls-back-whole-batch.bad.al

@@ -1,13 +0,0 @@
-codeunit 50124 "Sales Line Guard Bad Sample"
-{
-    // A throw here executes synchronously inside the transaction of the write
-    // that fired the event. With no per-record savepoint, it rolls back ALL
-    // uncommitted work since the last COMMIT — the entire batch, not just this
-    // line. One bad row discards every row imported before it.
-    [EventSubscriber(ObjectType::Table, Database::"Sales Line", 'OnAfterInsertEvent', '', false, false)]
-    local procedure OnAfterInsertSalesLine(var Rec: Record "Sales Line")
-    begin
-        if Rec.Quantity <= 0 then
-            Rec.FieldError(Quantity, 'must be greater than zero');
-    end;
-}

community/knowledge/error-handling/table-event-subscriber-rolls-back-whole-batch.good.al

@@ -1,33 +0,0 @@
-codeunit 50124 "Batch Import Good Sample"
-{
-    procedure ImportAll(var StagingLine: Record "Sales Line")
-    var
-        FailedCount: Integer;
-    begin
-        if StagingLine.FindSet() then
-            repeat
-                // Isolate each record behind a Codeunit.Run boundary: a failure
-                // inside the run rolls back only that record's work, and the
-                // batch continues instead of discarding everything.
-                if not Codeunit.Run(Codeunit::"Batch Import One Line", StagingLine) then
-                    FailedCount += 1;
-            until StagingLine.Next() = 0;
-
-        if FailedCount > 0 then
-            Message('%1 line(s) were skipped; the rest were imported.', FailedCount);
-    end;
-}
-
-codeunit 50125 "Batch Import One Line"
-{
-    TableNo = "Sales Line";
-
-    trigger OnRun()
-    begin
-        // Validation lives here. If it throws, only this line rolls back,
-        // because the caller wrapped the call in Codeunit.Run.
-        Rec.TestField("No.");
-        Rec.TestField(Quantity);
-        Rec.Insert(true);
-    end;
-}

community/knowledge/error-handling/table-event-subscriber-rolls-back-whole-batch.md

@@ -1,24 +0,0 @@
----
-bc-version: [all]
-domain: error-handling
-keywords: [table-events, oninsert, onmodify, ondelete, transaction, rollback, commit, batch, subscriber]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# A throw in a table-event subscriber rolls back the whole batch
-
-> Contributions welcome — open a PR to refine or extend this article.
-
-## Description
-
-Table-trigger event subscribers (`OnAfterInsertEvent`, `OnAfterModifyEvent`, `OnAfterDeleteEvent`, and their `OnBefore` counterparts) execute synchronously inside the transaction of the write that fired them. Because AL runs on a single implicit transaction with no per-record savepoint, an error raised in such a subscriber rolls back **all work since the last `COMMIT`** — not just the record that triggered it. In a batch loop with no intermediate `COMMIT`s, a single failing record discards the entire batch. The intuition that subscriber validation fails only the current record is wrong on the BC platform.
-
-## Best Practice
-
-Decide the failure granularity deliberately. If a batch must continue past individual failures, do not throw from the table-event subscriber — collect the error (for example via `ErrorInfo`/collectible errors) and let the loop continue, or isolate each record's work behind a `Codeunit.Run` / `if Codeunit.Run() then` boundary so its failure rolls back only that record. Insert intermediate `COMMIT`s only with full awareness of the durability trade-off.
-
-## Anti Pattern
-
-Putting `Error`/`TestField`/`FieldError` validation inside a table-event subscriber and assuming it rejects just the offending record during bulk processing. The first failure unwinds every uncommitted record in the run, turning a one-row data problem into a whole-batch rollback.

community/knowledge/performance/deleteall-skips-ondelete-unless-runtrigger.bad.al

@@ -1,11 +0,0 @@
-codeunit 50130 "Purge Orders Bad Sample"
-{
-    procedure PurgeCancelledLines(var SalesLine: Record "Sales Line")
-    begin
-        // Assumes DeleteAll fires OnDelete and cascades to reservation entries
-        // and item applications. It does not: parameterless DeleteAll() is
-        // DeleteAll(false) and skips OnDelete, so the rows vanish but their
-        // dependent records are orphaned.
-        SalesLine.DeleteAll();
-    end;
-}

community/knowledge/performance/deleteall-skips-ondelete-unless-runtrigger.good.al

@@ -1,16 +0,0 @@
-codeunit 50130 "Purge Orders Good Sample"
-{
-    procedure PurgeCancelledLines(var SalesLine: Record "Sales Line")
-    begin
-        // These lines have OnDelete cleanup (reservation entries, item
-        // application). Pass true so DeleteAll runs OnDelete per record and the
-        // cleanup actually happens — the row-by-row cost is accepted on purpose.
-        SalesLine.DeleteAll(true);
-    end;
-
-    procedure PurgeStagingBuffer(var TempBuffer: Record "Name/Value Buffer" temporary)
-    begin
-        // No OnDelete logic to run: the fast, set-based form is correct here.
-        TempBuffer.DeleteAll();
-    end;
-}

community/knowledge/performance/deleteall-skips-ondelete-unless-runtrigger.md

@@ -1,24 +0,0 @@
----
-bc-version: [all]
-domain: performance
-keywords: [deleteall, ondelete, run-trigger, set-based-delete, bulk-delete, triggers, validation]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# DeleteAll skips OnDelete unless you pass RunTrigger
-
-> Contributions welcome — open a PR to refine or extend this article.
-
-## Description
-
-`Record.DeleteAll()` — equivalently `DeleteAll(false)` — translates to a single set-based SQL `DELETE` and **does not** run AL `OnDelete` triggers or field/table validations. Only database-level referential constraints still apply. To run `OnDelete` logic you must call `DeleteAll(true)`, which then deletes record-by-record and forfeits the set-based performance, making it equivalent to a `FindSet` loop calling `Delete(true)`. The common misconception, which training data reproduces, is that `DeleteAll` iterates and fires `OnDelete` per record; it does not. (Parameterless `Delete()` likewise defaults to `Delete(false)` and skips `OnDelete`.)
-
-## Best Practice
-
-Use `DeleteAll()` / `DeleteAll(false)` for bulk deletion only when no AL `OnDelete` cleanup is required — it is the fast, set-based form. When `OnDelete` logic must run (cascading deletes, ledger cleanup, integration events), pass `DeleteAll(true)` and accept the row-by-row cost, or refactor the cleanup to run explicitly before the bulk delete.
-
-## Anti Pattern
-
-Calling `DeleteAll()` and assuming dependent records, integration events, or validation side effects are handled by `OnDelete`. The deletion succeeds but the AL-side cleanup never runs, leaving orphaned data — and adding a manual `FindSet`/`Delete` loop "for safety" reintroduces the per-record cost the set-based form was chosen to avoid.

community/knowledge/performance/setloadfields-unlisted-field-triggers-jit-load.bad.al

@@ -1,26 +0,0 @@
-codeunit 50132 "LoadFields Bad Sample"
-{
-    procedure TotalReleasedAmount(): Decimal
-    var
-        SalesHeader: Record "Sales Header";
-        Total: Decimal;
-    begin
-        // "Currency Code" is not listed. The helper takes SalesHeader BY VALUE,
-        // so the copy neither shares the load set nor updates the enumerator:
-        // reading the unlisted field triggers a fresh JIT load (an extra Get)
-        // on EVERY iteration, quietly reversing the saving.
-        SalesHeader.SetLoadFields("Amount Including VAT", Status);
-        if SalesHeader.FindSet() then
-            repeat
-                if IsLocalReleased(SalesHeader) then
-                    Total += SalesHeader."Amount Including VAT";
-            until SalesHeader.Next() = 0;
-        exit(Total);
-    end;
-
-    local procedure IsLocalReleased(SalesHeader: Record "Sales Header"): Boolean
-    begin
-        exit((SalesHeader.Status = SalesHeader.Status::Released) and
-             (SalesHeader."Currency Code" = ''));
-    end;
-}

community/knowledge/performance/setloadfields-unlisted-field-triggers-jit-load.good.al

@@ -1,24 +0,0 @@
-codeunit 50132 "LoadFields Good Sample"
-{
-    procedure TotalReleasedAmount(): Decimal
-    var
-        SalesHeader: Record "Sales Header";
-        Total: Decimal;
-    begin
-        // Every field read anywhere downstream is listed — including the one
-        // the by-var helper reads — so no JIT load is ever triggered.
-        SalesHeader.SetLoadFields("Amount Including VAT", Status, "Currency Code");
-        if SalesHeader.FindSet() then
-            repeat
-                if IsLocalReleased(SalesHeader) then
-                    Total += SalesHeader."Amount Including VAT";
-            until SalesHeader.Next() = 0;
-        exit(Total);
-    end;
-
-    local procedure IsLocalReleased(var SalesHeader: Record "Sales Header"): Boolean
-    begin
-        exit((SalesHeader.Status = SalesHeader.Status::Released) and
-             (SalesHeader."Currency Code" = ''));
-    end;
-}

community/knowledge/performance/setloadfields-unlisted-field-triggers-jit-load.md

@@ -1,24 +0,0 @@
----
-bc-version: [all]
-domain: performance
-keywords: [setloadfields, partial-records, just-in-time-load, jit-load, round-trip, pass-by-value, enumerator]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# Reading an unlisted field after SetLoadFields triggers a JIT load
-
-> Contributions welcome — open a PR to refine or extend this article.
-
-## Description
-
-`SetLoadFields` loads only the named fields, but the trap is what happens when code later reads a field that was *not* listed: the platform silently issues a **just-in-time (JIT) load** — an implicit `Get` that fetches the missing field(s) in a second database round-trip. A single JIT load can erase the saving; the real danger is a JIT that repeats per record. The optimization is only a win if the listed set covers every field touched anywhere downstream, not just in the immediate code block.
-
-## Best Practice
-
-Before adding `SetLoadFields`, audit the *whole* access lifecycle of the record variable — every field read in the loop body, in called procedures, in `OnValidate`/`OnAfterGetRecord`, and in anything that receives the record — and list all of them via `SetLoadFields`/`AddLoadFields`. Be especially careful when passing a partial record **by value**: the copy does not share the load set and its enumerator is not updated, so a helper that reads an unlisted field re-triggers the JIT on *every* iteration. Pass by `var` where you can (a JIT then updates the enumerator, so later iterations don't re-load), or call `AddLoadFields` before passing by value. If you cannot enumerate the fields confidently, prefer not to call `SetLoadFields` at all. See the existing guidance on when partial records pay off (`use-setloadfields-for-partial-records`).
-
-## Anti Pattern
-
-Adding `SetLoadFields(Field1, Field2)` at the top of a loop, then reading `Field3` deeper in the body or inside a by-value helper. The code compiles and returns correct data, but pays a hidden JIT round-trip — and in the by-value case it repeats once per row, quietly reversing the gain. JIT loads also introduce `Inconsistent read` / record-modified race errors that a full non-partial load avoids. Reviewer signal: a `SetLoadFields` list that omits a field later read through that record variable, especially a record passed by value to a procedure that reads a field the caller never listed.

community/knowledge/security/classify-every-field-with-dataclassification.bad.al

@@ -1,28 +0,0 @@
-table 50100 "Customer Feedback"
-{
-    fields
-    {
-        field(1; "Feedback No."; Code[20])
-        {
-            // No DataClassification declared. Defaults to ToBeClassified.
-        }
-        field(2; "Contact Name"; Text[100])
-        {
-            DataClassification = ToBeClassified;
-        }
-        field(3; "Email"; Text[80])
-        {
-            // Personal data classified as CustomerContent understates privacy impact.
-            DataClassification = CustomerContent;
-        }
-        field(4; "Feedback Text"; Text[2048])
-        {
-            DataClassification = ToBeClassified;
-        }
-    }
-
-    keys
-    {
-        key(PK; "Feedback No.") { Clustered = true; }
-    }
-}

community/knowledge/security/classify-every-field-with-dataclassification.good.al

@@ -1,36 +0,0 @@
-table 50100 "Customer Feedback"
-{
-    fields
-    {
-        field(1; "Feedback No."; Code[20])
-        {
-            DataClassification = SystemMetadata;
-        }
-        field(2; "Contact Name"; Text[100])
-        {
-            DataClassification = EndUserIdentifiableInformation;
-        }
-        field(3; "Email"; Text[80])
-        {
-            DataClassification = EndUserIdentifiableInformation;
-        }
-        field(4; "Product Code"; Code[20])
-        {
-            DataClassification = CustomerContent;
-        }
-        field(5; "Feedback Text"; Text[2048])
-        {
-            // When uncertain between CustomerContent and EUII, prefer the stronger protection.
-            DataClassification = EndUserIdentifiableInformation;
-        }
-        field(6; "Submitted DateTime"; DateTime)
-        {
-            DataClassification = SystemMetadata;
-        }
-    }
-
-    keys
-    {
-        key(PK; "Feedback No.") { Clustered = true; }
-    }
-}

community/knowledge/security/classify-every-field-with-dataclassification.md

@@ -1,26 +0,0 @@
----
-bc-version: [all]
-domain: security
-keywords: [dataclassification, gdpr, privacy, euii, compliance]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# Classify every field with DataClassification
-
-## Description
-
-Every field on every AL table and table extension must have a resolved `DataClassification` value, either declared directly on the field or inherited from a table-level default. The value drives GDPR tooling, data-subject requests, retention policies, and audit reporting — all of which rely on the field metadata to know what data to include, anonymize, or delete. A field with no field-level property and no table-level default resolves to `ToBeClassified`, which is a compliance gap, not a neutral state.
-
-## Best Practice
-
-Choose the narrowest value that accurately describes the field's content: `EndUserIdentifiableInformation` for data that directly identifies a person, `EndUserPseudonymousIdentifiers` for indirect identifiers, `CustomerContent` for business operational data, `SystemMetadata` for system-generated housekeeping, `AccountData` for tenant/billing, `OrganizationIdentifiableInformation` for organization-level identifiers. Use a table-level default for homogeneous tables, and override individual fields whose content differs from that default. When uncertain between two values, pick the stronger protection.
-
-See sample: `classify-every-field-with-dataclassification.good.al`.
-
-## Anti Pattern
-
-Leaving `DataClassification = ToBeClassified` on a field, omitting classification when the table has no default, or relying on a table-level default that understates a field's actual content. Code in this state fails compliance audits and breaks the subject-access-request and retention tooling that depends on the property being set correctly.
-
-See sample: `classify-every-field-with-dataclassification.bad.al`.

community/knowledge/telemetry/default-telemetryscope-to-extensionpublisher.bad.al

@@ -1,17 +0,0 @@
-codeunit 50136 "Telemetry Bad Sample"
-{
-    procedure LogSyncDiagnostic(RecordsProcessed: Integer)
-    var
-        Dimensions: Dictionary of [Text, Text];
-    begin
-        Dimensions.Add('recordsProcessed', Format(RecordsProcessed));
-
-        // TelemetryScope::All pushes this internal diagnostic into every
-        // customer's Application Insights too, inflating their ingestion cost
-        // and burying their own signals in noise. ExtensionPublisher is the
-        // correct scope for publisher-only diagnostics.
-        Session.LogMessage(
-            'SYNC001', 'Nightly sync completed.', Verbosity::Normal,
-            DataClassification::SystemMetadata, TelemetryScope::All, Dimensions);
-    end;
-}

community/knowledge/telemetry/default-telemetryscope-to-extensionpublisher.good.al

@@ -1,15 +0,0 @@
-codeunit 50136 "Telemetry Good Sample"
-{
-    procedure LogSyncDiagnostic(RecordsProcessed: Integer)
-    var
-        Dimensions: Dictionary of [Text, Text];
-    begin
-        Dimensions.Add('recordsProcessed', Format(RecordsProcessed));
-
-        // A diagnostic only the publisher acts on: route it to the publisher's
-        // own Application Insights, not the customer's environment resource.
-        Session.LogMessage(
-            'SYNC001', 'Nightly sync completed.', Verbosity::Normal,
-            DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher, Dimensions);
-    end;
-}

community/knowledge/telemetry/default-telemetryscope-to-extensionpublisher.md

@@ -1,24 +0,0 @@
----
-bc-version: [all]
-domain: telemetry
-keywords: [telemetry, session-logmessage, telemetryscope, application-insights, extensionpublisher, ingestion-cost]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# Default TelemetryScope to ExtensionPublisher, not All
-
-> Contributions welcome — open a PR to refine or extend this article.
-
-## Description
-
-The `TelemetryScope` parameter of `Session.LogMessage` (and `LogError`) controls *where* a custom telemetry signal is routed, not just whether it is emitted. `TelemetryScope::ExtensionPublisher` sends the signal only to the extension publisher's own Application Insights resource. `TelemetryScope::All` sends it to **both** the publisher's resource **and** the customer's environment-level Application Insights resource. The distinction is easy to get wrong because both values compile and both "emit telemetry" — but `All` silently adds to the customer's ingestion volume and cost.
-
-## Best Practice
-
-Default to `TelemetryScope::ExtensionPublisher` for diagnostic telemetry that only the publisher acts on. Reserve `TelemetryScope::All` for signals the customer's own administrators are expected to monitor and act on (for example, a business event surfaced to their environment telemetry). Treat the choice as a deliberate routing decision per signal, not a copy-paste default.
-
-## Anti Pattern
-
-Emitting all custom telemetry with `TelemetryScope::All` "to be safe." This pushes the publisher's internal diagnostics into every customer's Application Insights, inflating their ingestion cost and burying their own signals in noise — a footgun a code reviewer can catch by flagging `All` on any signal the customer would not act on.

community/knowledge/ui/split-button-standard-groups.md

@@ -1,20 +0,0 @@
----
-bc-version: [21..]
-domain: ui
-keywords: [showas, splitbutton, promoted-actions, actionref, posting-actions, release-action, action-bar]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-# Reserve `ShowAs = SplitButton` For Standard Posting And Release Groups
-
-> Contributions welcome — open a PR to refine or extend this article.
-
-## Description
-Setting `ShowAs = SplitButton` on a `group` inside `area(Promoted)` renders a primary one-click button with a dropdown of related alternatives, where the FIRST `actionref` in the group becomes the primary (left) button. Business Central users have learned this pattern from the two standard groups it ships with — Posting (`Post`, `Post and Print`, `Post and Send`, `Preview Posting`) and Release (`Release`, `Reopen`). Inventing new split-button groups for unrelated actions, or ordering the dropdown so the most common action is not first, breaks that learned muscle memory and makes users guess what the left button will do.
-
-## Best Practice
-Use `ShowAs = SplitButton` only when all hold: the actions are genuinely variations of one operation, there is an obvious most-frequent primary, and the dropdown stays at roughly two to four items. Place that primary action as the first `actionref` so it occupies the left button; order the remaining refs by descending frequency. Outside the Posting and Release conventions, treat a new split-button group as something to justify, not a default — a plain promoted group or category is usually the safer choice and keeps the action bar predictable.
-
-## Anti Pattern
-Grouping unrelated actions under one split button to save toolbar space — for example pairing `Post` with `Delete`, or `Release` with `Print` — so the left button performs whatever happens to be listed first. The reviewer signal is a group with `ShowAs = SplitButton` whose member `actionref`s do not share a verb or workflow, a primary that is not the most common action, or a dropdown padded well beyond four items. Each makes the immediate left-click unpredictable and costs the user the very click the split button was meant to save.

community/knowledge/upgrade/no-series-bc24-migration.md

@@ -1,20 +0,0 @@
----
-bc-version: [24..]
-domain: upgrade
-keywords: [no-series, noseriesmanagement, codeunit-310, getnextno, peeknextno, testmanual, arerelated, no-series-batch, business-foundation, obsolete-codeunit]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-# Migrate No. Series Calls From NoSeriesManagement To The BC24 No. Series Module
-
-> Contributions welcome — open a PR to refine or extend this article.
-
-## Description
-In BC24 (2024 Wave 1) Microsoft moved number generation into the Business Foundation `No. Series` codeunit (310) and obsoleted the legacy `NoSeriesManagement` codeunit (396). Code that still declares `Codeunit NoSeriesManagement` or calls its methods compiles only against the temporary obsolete shim and will break once Microsoft removes it. The new API is not a drop-in rename: the facade exposes a small, specific set of real methods, parameter shapes changed, and the old single method that both previewed and consumed a number was split into two. Getting the mapping wrong silently consumes numbers when you only meant to preview, leaving gaps in the sequence.
-
-## Best Practice
-Replace the `NoSeriesManagement` variable with `Codeunit "No. Series"` and map each call deliberately using the facade's actual methods — `GetNextNo`, `PeekNextNo`, `GetLastNoUsed`, `TestManual`, `IsManual`, and `AreRelated`. Use `GetNextNo(SeriesCode, RefDate)` only when you intend to consume and advance the series for a committed document, and `PeekNextNo(SeriesCode, RefDate)` for any display, validation, or preview-posting path where you must not consume. Replace `InitSeries` with a guarded `if "No." = '' then "No." := NoSeries.GetNextNo(...)`. Map `SelectSeries` to `LookupRelatedNoSeries`, relationship checks the old code did by hand to `AreRelated`, and both `TestManual` and `ManualNoAllowed` to `TestManual` (which now raises its own error). For multi-document allocation use `Codeunit "No. Series - Batch"` and persist its state once with `SaveState` instead of committing per iteration. Treat the migration as an opportunity to add preview-posting support, since `PeekNextNo` now makes that trivial.
-
-## Anti Pattern
-Mechanically swapping the codeunit reference while keeping the old boolean call shape. The legacy `GetNextNo(Series, Date, false)` meant "peek" and `GetNextNo(Series, Date, true)` meant "consume"; the new `GetNextNo` always consumes and takes no boolean. Equally common is inventing validation helpers such as `IsValidNo`, `VerifySeriesExists`, `IsValidForDate`, or `TryGetNextNo` — these names are not on the `No. Series` or `No. Series - Batch` codeunits and will not compile, a frequent LLM hallucination for this migration. A reviewer can detect the defect by the residual third boolean argument, by any lingering `NoSeriesMgt`/`NoSeriesManagement` identifier, by a fabricated method name, or by an `OnBeforeGetNextNo`/`OnAfterGetNextNo` subscriber — those events were removed without replacement, so that logic must be rewritten as inline pre/post procedures, not re-subscribed. A subtler signal is `GetNextNo` used merely to display a preview, which silently advances the series and creates number gaps; that should be `PeekNextNo`.

evaluation/README.md

@@ -0,0 +1,56 @@
+# AL review evaluation
+
+The evaluation is convention-driven. For every `microsoft/skills/review/al-<domain>-review.md` leaf, the harness finds `microsoft/knowledge/<domain>/`, selects the first article (by filename) with both `.bad.al` and `.good.al` companions, and derives the expected positive and clean control automatically. Adding a conforming leaf requires no scoring-contract edit.
+
+`review-fixtures.json` contains only global thresholds and optional exceptional overrides. An override may select a different article or add context when the generic convention cannot express a scenario. It should remain empty in the normal case.
+
+Model-facing preparation hashes case IDs, neutralizes `Good`/`Bad` object-name tokens, and removes full-line sample comments so neither the article slug, domain, nor expected outcome reveals the answer.
+
+## Validate the corpus
+
+```powershell
+pwsh ./tools/Test-ReviewFixtures.ps1 -Root .
+```
+
+This credential-free check proves every registered leaf maps to a same-named knowledge domain with at least one complete AL sample pair and that all configured overrides are valid.
+
+## Run a fast-model evaluation
+
+1. Prepare neutral inputs:
+
+   ```powershell
+   pwsh ./tools/Test-ReviewFixtures.ps1 -Root . -PrepareDirectory ./.evaluation-run
+   ```
+
+   This is also the CI path. It derives all cases, builds the current index, requires the convention-selected article to rank naturally into the candidate cutoff, and prepares the neutral requests.
+
+2. For a fast/small model, use one fresh invocation per `request-case-*.json`. Each request embeds the exact leaf instructions, that domain's candidate index rows with authoritative paths, and one opaque case. The model opens only matching articles and copies finding IDs from `candidateArticles[].path`. Save each response with the matching `result-case-*.json` name in the same directory.
+
+   `request-<domain>.json` files provide optional two-case leaf batches; save those as `result-<domain>.json`. Directory scoring prefers `result-case-*.json` when present and otherwise falls back to `result-*.json`. `review-request.json` is an optional all-domains stress test for larger models. Neither batch form is the preferred fast-model profile.
+
+3. Save only this result shape:
+
+   ```json
+   {
+     "cases": [
+       {
+         "id": "case-a1b2c3d4",
+         "findings": [
+           { "id": "microsoft/knowledge/appsource/object-affixes-prevent-collisions.md" }
+         ]
+       }
+     ]
+   }
+   ```
+
+   Include every case. A clean control has an empty `findings` array.
+
+4. Score all per-leaf results together:
+
+   ```powershell
+   pwsh ./tools/Test-ReviewFixtures.ps1 -Root . -ResultsDirectory ./.evaluation-run
+   ```
+
+   For a single combined stress-test result, use `-ResultsPath` instead.
+
+The committed gate requires full expected recall, the exact convention-derived article ID, and no findings on clean controls.

evaluation/review-fixtures.json

@@ -0,0 +1,39 @@
+{
+  "version": 2,
+  "selection": "first-paired-al-article",
+  "minimumExpectedRecall": 1.0,
+  "minimumCleanRate": 1.0,
+  "overrides": {
+    "appsource": {
+      "context": "AppSourceCop mandatoryAffixes is configured to ABC."
+    },
+    "breaking-changes": {
+      "article": "do-not-expose-sensitive-data-through-public-api"
+    },
+    "events": {
+      "article": "initialize-ishandled-to-false-before-publishing"
+    },
+    "interfaces": {
+      "article": "set-defaultimplementation-on-enum"
+    },
+    "performance": {
+      "article": "use-isempty-for-existence-check"
+    },
+    "privacy": {
+      "article": "no-pii-in-telemetry-message-string"
+    },
+    "style": {
+      "article": "label-comment-explains-placeholders"
+    },
+    "telemetry": {
+      "article": "telemetry-event-id-stable-unique"
+    },
+    "upgrade": {
+      "article": "initvalue-does-not-update-existing-rows",
+      "context": "The extended table existed in the previous app version and already contains rows."
+    },
+    "web-services": {
+      "article": "expose-systemid-as-the-api-key"
+    }
+  }
+}

microsoft/knowledge/appsource/keep-copilot-help-url-to-two-path-levels.md

@@ -1,5 +1,5 @@
 ---
-bc-version: [24..]
+bc-version: [27..]
 domain: appsource
 keywords: [app-json, help-url, copilot, grounding, documentation, url-depth, contexturl]
 technologies: [al]
@@ -9,8 +9,6 @@ application-area: [all]
 
 # Keep the Copilot help URL to two path levels
 
-> Contributions welcome — open a PR to refine or extend this article.
-
 ## Description
 
 The `help` URL declared in `app.json` is what Copilot uses to ground answers about your app. That URL may be at most **two path levels** deep (for example `https://contoso.com/docs/myapp`). If you point it at a deeper path (three or more segments), Copilot does not use the URL as given: it truncates to the first two levels, drops any fragments and query strings, and then grounds on **all** content beneath that two-level path. The failure is silent — there is no build error — and the practical effect is worse answers, because Copilot may ingest sibling apps' documentation that lives under the same two-level parent.

microsoft/knowledge/appsource/object-affixes-prevent-collisions.md

@@ -11,18 +11,18 @@ application-area: [all]
 
 ## Description
 
-An AppSource extension must carry a reserved affix — a prefix or a suffix of at least three characters — on the names of the objects it owns **and** on any field, key, control, or action it adds to a base-application object. The affix is registered with Microsoft; when two coexisting extensions would otherwise collide, the registrant of the affix wins. Without it, two apps that both add a `Loyalty Points` field to `Customer`, or both define a `Loyalty Tier` table, cannot be installed side by side.
+An AppSource extension must prevent name collisions through its registered affix or, on BC23 and later for objects it owns, a namespace with at least two levels. The affix still applies to every field, key, control, or action added to a base-application object; see `two-level-namespace-replaces-object-affix-not-extension-member-affix.md`. Without either mechanism, two apps that both define a `Loyalty Tier` table cannot coexist, and two apps that add an unaffixed `Loyalty Points` field to `Customer` still collide regardless of their namespaces.
 
 AppSourceCop enforces this. The primary rule is AS0011 ("An affix is required"); the affixes are configured through `mandatoryAffixes` (and `mandatoryPrefix`) in `AppSourceCop.json`. Two placements matter and are easy to get half-right: an object you define carries the affix at **object-name** level, while a member you add to a **standard** object carries the affix on that **member's** name. Adding an affixed object is not enough — an unaffixed field bolted onto `Customer` still collides and still fails validation.
 
 ## Best Practice
 
-Own objects are named with the affix (e.g. a table `ABC Loyalty Tier`), and every field or action added to a standard object is individually affixed (e.g. `Loyalty Points ABC` on a `Customer` tableextension).
+Own objects use the registered affix (for example `ABC Loyalty Tier`) or, when targeting BC23 or later, a qualifying namespace. Every field or action added to a standard object remains individually affixed (for example `Loyalty Points ABC` on a `Customer` tableextension).
 
 See sample: `object-affixes-prevent-collisions.good.al`.
 
 ## Anti Pattern
 
-Unaffixed object or member names, or the common half-measure: the extension object carries the affix but a field it adds to a standard table does not. AS0011 flags the missing affix and the field can still collide with another app.
+An owned object with neither a qualifying namespace nor an affix, an unaffixed extension member, or the common half-measure where the extension object carries the affix but a field it adds to a standard table does not. AS0011 flags the missing collision protection and the field can still collide with another app.
 
 See sample: `object-affixes-prevent-collisions.bad.al`.

microsoft/knowledge/appsource/permission-sets-cover-setup-and-usage-without-super.bad.al

@@ -0,0 +1,43 @@
+table 50476 "Rental Setup Bad"
+{
+    DataClassification = CustomerContent;
+
+    fields
+    {
+        field(1; "Primary Key"; Code[10]) { }
+    }
+}
+
+page 50477 "Rental Setup Bad"
+{
+    PageType = Card;
+    SourceTable = "Rental Setup Bad";
+
+    layout
+    {
+        area(Content)
+        {
+            field("Primary Key"; Rec."Primary Key")
+            {
+                ApplicationArea = All;
+                Caption = 'Primary Key';
+                ToolTip = 'Specifies the setup record.';
+            }
+        }
+    }
+}
+
+codeunit 50478 "Rental Setup Mgt. Bad"
+{
+    procedure Initialize()
+    begin
+    end;
+}
+
+permissionset 50479 "Rental User"
+{
+    Assignable = true;
+    // The setup page opens, but saving or running setup logic requires SUPER.
+    Permissions =
+        page "Rental Setup Bad" = X;
+}

microsoft/knowledge/appsource/permission-sets-cover-setup-and-usage-without-super.good.al

@@ -0,0 +1,45 @@
+table 50472 "Rental Setup"
+{
+    DataClassification = CustomerContent;
+
+    fields
+    {
+        field(1; "Primary Key"; Code[10]) { }
+    }
+}
+
+page 50473 "Rental Setup"
+{
+    PageType = Card;
+    SourceTable = "Rental Setup";
+
+    layout
+    {
+        area(Content)
+        {
+            field("Primary Key"; Rec."Primary Key")
+            {
+                ApplicationArea = All;
+                Caption = 'Primary Key';
+                ToolTip = 'Specifies the setup record.';
+            }
+        }
+    }
+}
+
+codeunit 50474 "Rental Setup Mgt."
+{
+    procedure Initialize()
+    begin
+    end;
+}
+
+permissionset 50475 "Rental Manager"
+{
+    Assignable = true;
+    Permissions =
+        tabledata "Rental Setup" = RIMD,
+        table "Rental Setup" = X,
+        page "Rental Setup" = X,
+        codeunit "Rental Setup Mgt." = X;
+}

microsoft/knowledge/appsource/permission-sets-cover-setup-and-usage-without-super.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: appsource
+keywords: [permission-set, super, appsource, setup, usage, tabledata, execute, submission]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# AppSource permission sets must cover setup and usage without SUPER
+
+## Description
+
+An AppSource app must provide permission sets that let assigned users complete the app's setup and normal usage without `SUPER`. The requirement is about complete effective grants, not about naming the permission set after the app. A package can compile and install with missing tabledata or execute permissions, then fail only when Marketplace validation or a real non-SUPER user reaches the omitted path.
+
+## Best Practice
+
+Trace every setup page, normal page, report, codeunit, and tabledata operation exposed by the app and cover it through assignable role permission sets composed from focused non-assignable sets. Validate setup and representative workflows as a user assigned only those app roles. Grant the minimum required operations; completeness is not a reason to use wildcards.
+
+See sample: `permission-sets-cover-setup-and-usage-without-super.good.al`.
+
+## Anti Pattern
+
+Shipping no permission set, omitting a tabledata or execute grant used by the app's own UI, or instructing users and validators to assign `SUPER` when setup fails. Do not flag a permission-set name that differs from the app name; no such naming requirement exists.
+
+See sample: `permission-sets-cover-setup-and-usage-without-super.bad.al`.

microsoft/knowledge/appsource/two-level-namespace-replaces-object-affix-not-extension-member-affix.bad.al

@@ -0,0 +1,22 @@
+namespace Contoso;
+
+table 50462 "Rental Agreement"
+{
+    DataClassification = CustomerContent;
+
+    fields
+    {
+        field(1; "No."; Code[20]) { }
+    }
+}
+
+tableextension 50463 "Rental Customer Ext" extends Customer
+{
+    fields
+    {
+        field(50463; "Loyalty Points"; Integer)
+        {
+            DataClassification = CustomerContent;
+        }
+    }
+}

microsoft/knowledge/appsource/two-level-namespace-replaces-object-affix-not-extension-member-affix.good.al

@@ -0,0 +1,22 @@
+namespace Contoso.Rentals;
+
+table 50460 "Rental Agreement"
+{
+    DataClassification = CustomerContent;
+
+    fields
+    {
+        field(1; "No."; Code[20]) { }
+    }
+}
+
+tableextension 50461 "Rental Customer Ext" extends Customer
+{
+    fields
+    {
+        field(50461; "Loyalty Points RNT"; Integer)
+        {
+            DataClassification = CustomerContent;
+        }
+    }
+}

microsoft/knowledge/appsource/two-level-namespace-replaces-object-affix-not-extension-member-affix.md

@@ -0,0 +1,26 @@
+---
+bc-version: [23..]
+domain: appsource
+keywords: [namespace, two-level, affix, prefix, suffix, as0011, tableextension, pageextension]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# A two-level namespace replaces an object affix, not an extension-member affix
+
+## Description
+
+Current AppSource naming guidance accepts a namespace with at least two levels, such as `Contoso.Rentals`, instead of a registered prefix or suffix on the names of objects the app owns. The namespace does not qualify members added to another publisher's object: fields, keys, controls, and actions introduced through table or page extensions still share the target object's flat member namespace and still need the registered affix.
+
+## Best Practice
+
+Choose one collision strategy for owned objects: a registered affix or a globally meaningful namespace with at least two levels. Regardless of that choice, apply the registered affix to every member added to a base or third-party object. Keep the affix configured for AppSourceCop so member validation remains deterministic.
+
+See sample: `two-level-namespace-replaces-object-affix-not-extension-member-affix.good.al`.
+
+## Anti Pattern
+
+Using `namespace Contoso;` as though one level satisfied the AppSource alternative, or declaring `namespace Contoso.Rentals;` and then adding an unaffixed `Loyalty Points` field to `Customer`. The namespace distinguishes the extension's own objects; it cannot disambiguate members on Customer.
+
+See sample: `two-level-namespace-replaces-object-affix-not-extension-member-affix.bad.al`.

microsoft/knowledge/breaking-changes/deprecate-public-members-with-the-obsolete-lifecycle.good.al

@@ -1,7 +1,7 @@
 codeunit 50305 "Net Amount Api Good"
 {
-    // Old name kept and marked obsolete: callers still compile but get a warning
-    // pointing at the replacement, with a tag recording the removal target version.
+    // Old name kept during the warning window. The tag records when obsoletion
+    // began; a later release deletes the method after consumers have migrated.
     [Obsolete('Use CalculateNetAmount instead.', '25.0')]
     procedure CalcNet(GrossAmount: Decimal; TaxRate: Decimal): Decimal
     begin

microsoft/knowledge/breaking-changes/deprecate-public-members-with-the-obsolete-lifecycle.md

@@ -11,16 +11,16 @@ application-area: [all]
 
 ## Description
 
-Deleting or renaming a published procedure (or object) in a single release is a hard break: dependent extensions that reference it stop compiling the moment they pick up the new version, with no warning window to migrate. AL provides a staged deprecation lifecycle precisely so consumers get advance notice. For a procedure, apply the `[Obsolete('reason', 'tag')]` attribute: the member keeps working but every caller gets a compiler warning naming the replacement and the target version. The member stays through a deprecation window — at least one major release — before it is finally removed. Object- and field-level members use the matching `ObsoleteState = Pending` → `Removed` property progression. LLMs trained to "clean up" code often delete or rename the old member immediately, skipping the window entirely.
+Deleting or renaming a published procedure (or object) in a single release is a hard break: dependent extensions that reference it stop compiling the moment they pick up the new version, with no warning window to migrate. AL provides staged deprecation so consumers get advance notice. A procedure uses `[Obsolete('reason', 'tag')]`: it remains callable but callers receive a compiler warning naming the replacement and the version in which obsoletion began. Methods do not have `ObsoleteState`; after the deprecation window, the method is deleted, commonly through versioned preprocessor cleanup. Objects and fields instead use the `ObsoleteState = Pending` to `Removed` property progression.
 
 ## Best Practice
 
-When a published procedure is superseded, keep it in place and mark it `[Obsolete('Use CalculateNetAmount instead.', '25.0')]`, where the message names the replacement and the tag records the target version for removal. Have the obsolete member forward to the new one so behavior is preserved during the window. Only after the deprecation window has elapsed — a later release — change its state to removed. This gives every dependent app a compile-time signal and time to migrate before anything actually disappears.
+When a published procedure is superseded, keep it in place and mark it `[Obsolete('Use CalculateNetAmount instead.', '25.0')]`, where the message names the replacement and the tag records when the method became obsolete. Have the obsolete member forward to the new one so behavior is preserved during the window. Only after the deprecation window has elapsed should a later release delete the method. For an object or field, use `Pending` during the warning window and `Removed` afterward.
 
 See sample: `deprecate-public-members-with-the-obsolete-lifecycle.good.al`.
 
 ## Anti Pattern
 
-Renaming or deleting the published `CalcNet` procedure in place — replacing it with `CalculateNetAmount` and nothing else — so consumers calling `CalcNet` break immediately with no deprecation notice. Detection: a previously shipped non-`local` procedure that vanished or was renamed between versions with no `[Obsolete]` marker left behind on a kept member. Mark it obsolete and keep it for a window instead.
+Renaming or deleting the published `CalcNet` procedure in place — replacing it with `CalculateNetAmount` and nothing else — so consumers calling `CalcNet` break immediately with no deprecation notice. Detection: a previously shipped non-`local` procedure that vanished or was renamed between versions with no `[Obsolete]` marker left behind during a prior warning window. Do not suggest `ObsoleteState = Removed` for a method; that property belongs to supported object and element types.
 
 See sample: `deprecate-public-members-with-the-obsolete-lifecycle.bad.al`.

microsoft/knowledge/breaking-changes/do-not-expose-sensitive-data-through-public-api.good.al

@@ -1,10 +1,10 @@
 codeunit 50320 "Payment Client Good"
 {
     var
-        AccessToken: Text;
+        AccessToken: SecretText;
 
-    // Credential flows inward through an internal setter and never leaves the object.
-    internal procedure SetAccessToken(NewToken: Text)
+    // Credential remains SecretText as it flows inward and is stored.
+    internal procedure SetAccessToken(NewToken: SecretText)
     begin
         AccessToken := NewToken;
     end;

microsoft/knowledge/breaking-changes/do-not-expose-sensitive-data-through-public-api.md

@@ -1,5 +1,5 @@
 ---
-bc-version: [all]
+bc-version: [23..]
 domain: breaking-changes
 keywords: [sensitive-data, secrettext, token, credential, public-api, access-boundary]
 technologies: [al]

microsoft/knowledge/breaking-changes/namespace-is-part-of-published-object-identity.bad.al

@@ -0,0 +1,9 @@
+// This published object previously used namespace Contoso.Rentals.
+namespace Contoso.RentalManagement;
+
+codeunit 50467 "Rental Agreement Mgt."
+{
+    procedure CreateAgreement()
+    begin
+    end;
+}

microsoft/knowledge/breaking-changes/namespace-is-part-of-published-object-identity.good.al

@@ -0,0 +1,8 @@
+namespace Contoso.Rentals;
+
+codeunit 50466 "Rental Agreement Mgt."
+{
+    procedure CreateAgreement()
+    begin
+    end;
+}

microsoft/knowledge/breaking-changes/namespace-is-part-of-published-object-identity.md

@@ -0,0 +1,26 @@
+---
+bc-version: [23..]
+domain: breaking-changes
+keywords: [namespace, published-object, dependency, breaking-change, as0007, compile-time-identity]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Treat a published namespace as part of object identity
+
+## Description
+
+AL resolves an object by namespace and name. Once an app ships and dependent extensions compile against that identity, changing the namespace breaks their references even when the object name and ID stay unchanged. AppSourceCop AS0007 rejects changing the namespace of published objects; namespaces are therefore not a cosmetic folder-like label that can be reorganized after release.
+
+## Best Practice
+
+Choose a globally meaningful namespace before first publication and keep it stable. Add new functional areas beneath that structure without moving existing published objects. If an identity must move, use the platform's supported move/obsoletion lifecycle rather than a source-only namespace rename.
+
+See sample: `namespace-is-part-of-published-object-identity.good.al`.
+
+## Anti Pattern
+
+Changing `namespace Contoso.Rentals;` to `namespace Contoso.RentalManagement;` as a cleanup while leaving the object name and ID untouched. Every dependent `using` directive and qualified reference targets the old identity and stops compiling.
+
+See sample: `namespace-is-part-of-published-object-identity.bad.al`.

microsoft/knowledge/breaking-changes/obsolete-table-fields-instead-of-deleting-them.bad.al

@@ -3,9 +3,10 @@ table 50311 "Customer Profile Bad"
     fields
     {
         field(1; "No."; Code[20]) { }
-        // Breaking: the published "Email" field was renamed in place. Dependent
-        // extensions that reference "Email" stop compiling, and the data stored in
-        // the old column is orphaned on upgrade.
-        field(2; "Contact Email"; Text[80]) { }
+        // Breaking: the published Email field at ID 3 was renamed while retaining
+        // the ID. The good example keeps Email at ID 3 and adds a separate field.
+        // AppSourceCop AS0005 rejects the compatibility change; retaining the ID
+        // does not by itself mean the stored column was dropped and re-created.
+        field(3; "Contact Email"; Text[80]) { }
     }
 }

microsoft/knowledge/breaking-changes/obsolete-table-fields-instead-of-deleting-them.good.al

@@ -3,10 +3,10 @@ table 50310 "Customer Profile Good"
     fields
     {
         field(1; "No."; Code[20]) { }
-        // Replacement field shipped alongside the old one.
+        // Replacement is a separate field under an otherwise unused ID.
         field(2; "Contact Email"; Text[80]) { }
-        // Old field kept and marked Pending so dependent code keeps compiling and
-        // an upgrade codeunit can copy its data before it is finally removed.
+        // Old field keeps its original ID, name, and type and is marked Pending so
+        // dependent code keeps compiling while an upgrade codeunit migrates its data.
         field(3; "Email"; Text[80])
         {
             ObsoleteState = Pending;

microsoft/knowledge/breaking-changes/obsolete-table-fields-instead-of-deleting-them.md

@@ -7,20 +7,20 @@ countries: [w1]
 application-area: [all]
 ---
 
-# Obsolete published table fields instead of deleting or renaming them
+# Obsolete published table fields instead of deleting, renaming, or renumbering them
 
 ## Description
 
-A table field that has shipped carries two contracts at once: extensions reference it by name, and the database holds data in its column. Deleting the field, or renaming it (which the platform treats as drop-plus-add), breaks dependent code at compile time and discards the stored data — a silent data-loss event on upgrade. The fix is the same staged lifecycle used for objects: set `ObsoleteState = Pending` together with `ObsoleteReason` and an `ObsoleteTag` naming the target version, ship the new field alongside, migrate data during the window, and only switch the old field to `ObsoleteState = Removed` in a later release once nothing depends on it. LLMs often "tidy" a schema by renaming a field in place, not realizing this is both a breaking change and a data-loss risk.
+A shipped table field carries both a source-level contract and persisted data. Renaming a field while retaining its ID is prohibited by AppSourceCop AS0005 and can break dependent extensions, but it is not inherently a drop-and-readd operation and should not be described as automatic data loss. Deleting the field or replacing it under a different ID is the data-loss risk: the old field storage is no longer represented unless data is migrated. The supported path is to keep the old field and obsolete it, add a replacement under a new ID, and migrate values before later removal.
 
 ## Best Practice
 
-Add the replacement field, then mark the old field `ObsoleteState = Pending` with an `ObsoleteReason` that names the replacement and an `ObsoleteTag` carrying the target version (for example `'25.0'`). Keep the obsolete field readable so an upgrade codeunit can copy its data into the new field during the deprecation window. Move it to `ObsoleteState = Removed` only in a later major version, after the window has passed and data has migrated.
+Keep the old field's ID, name, and type unchanged. Add the replacement as a separate field under an unused ID, then mark the old field `ObsoleteState = Pending` with an `ObsoleteReason` that names the replacement and an `ObsoleteTag` recording the obsoletion version. Keep the old field readable so an upgrade codeunit can copy its data during the deprecation window. Move it to `ObsoleteState = Removed` only in a later release, after the window has passed and data has migrated.
 
 See sample: `obsolete-table-fields-instead-of-deleting-them.good.al`.
 
 ## Anti Pattern
 
-Renaming the published `Email` field to `Contact Email` directly in the table — or deleting it — so dependent extensions that reference `Email` break and the column's stored values are orphaned on upgrade. Detection: a previously shipped field removed or renamed in a table or table extension with no `ObsoleteState = Pending` step preserving the original. Obsolete the field through the lifecycle instead.
+Renaming published `Email` to `Contact Email` with the same ID violates the compatibility contract and AS0005, even though the retained ID does not itself imply a fresh empty column. Deleting `Email` or changing its ID additionally risks losing its stored values. Detection: any previously shipped field whose name changes at the same ID, or whose original ID disappears without the unchanged field being retained as `Pending` and its data migrated to a separate replacement field.
 
 See sample: `obsolete-table-fields-instead-of-deleting-them.bad.al`.

microsoft/knowledge/breaking-changes/relocating-a-field-to-a-tableextension-is-not-a-deletion.md

@@ -0,0 +1,18 @@
+---
+bc-version: [all]
+domain: breaking-changes
+keywords: [table-field, tableextension, relocation, field-id, obsoletestate, breaking-change, false-positive]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Relocating a field to a tableextension in the same app is not a deletion
+
+## Description
+
+Moving a field out of a base-table definition (or a base-app layer modification of one) into a tableextension that `extends` the same table, within the same app and keeping the same field ID and name, is a relocation — not a deletion or a rename. After the move the field still exists on the table: `Rec."Field Name"` and the field ID resolve exactly as before, so dependent extensions that reference the field continue to compile. Nothing in the field's public contract is removed or renamed, so the deprecation lifecycle that protects a genuinely removed field does not apply. LLM reviewers frequently misread the two-sided diff — the field disappearing from the base object and reappearing in the tableextension — as a shipped field being deleted and illegally re-added under the same ID, and demand `ObsoleteState = Pending` staging that this refactor does not need.
+
+## Best Practice
+
+Recognize a field that is removed from a base table (or base-app layer) and re-declared in a tableextension of the same table, with the same field ID and name, as a same-app relocation. Do not flag it as a deleted or renamed shipped field, and do not require `ObsoleteState = Pending`, `ObsoleteReason`, `ObsoleteTag`, or a deprecation window for the move itself. The `obsolete-table-fields-instead-of-deleting-them` and `obsolete-pending-to-removed-staging` rules apply to fields that leave the table's contract entirely, not to fields relocated within the same app under an unchanged ID.

microsoft/knowledge/breaking-changes/unreleased-symbol-change-is-not-a-breaking-change.md

@@ -0,0 +1,24 @@
+---
+bc-version: [all]
+domain: breaking-changes
+keywords: [released-baseline, unreleased, rename, renumber, obsolete, api-stability, false-positive]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Changing an unreleased symbol is not a breaking change
+
+## Description
+
+Breaking-change rules protect contracts that have already shipped to customers or are exposed to external extensions. A symbol — an object, field, key, enum value, or procedure — that is new in this app, was introduced and then changed within the same still-unreleased development cycle, or belongs to an app that has no released version yet, can be renamed, renumbered, or removed freely. There is no shipped contract to break, so the change is not a breaking change.
+
+Release status is established from the diff, the app's `app.json` version, or a released baseline. An app whose `app.json` version has no corresponding released baseline (for example a `1.0.0.0` app that has never shipped) has no protected surface.
+
+## Best Practice
+
+Before treating a rename, renumber, or removal as breaking, establish that the affected symbol was present in a released baseline. Do not flag changes to symbols that are new in the current unreleased cycle or that belong to an app with no released version. When release status cannot be established from the diff, `app.json`, or a released baseline, omit the finding rather than assert a break.
+
+## Anti Pattern
+
+Reporting a breaking change for a rename, renumber, or removal without confirming the symbol shipped in a released version — for example flagging a break on an app whose `app.json` version has no released baseline.

microsoft/knowledge/data-modeling/share-mediaset-items-with-insert-not-field-assignment.bad.al

@@ -0,0 +1,26 @@
+table 50441 "Source Media Bad"
+{
+    fields
+    {
+        field(1; Code; Code[20]) { }
+        field(10; Pictures; MediaSet) { }
+    }
+}
+
+table 50442 "Target Media Bad"
+{
+    fields
+    {
+        field(1; Code; Code[20]) { }
+        field(20; Pictures; MediaSet) { }
+    }
+}
+
+codeunit 50443 "Share Media Bad"
+{
+    procedure CopyPictures(Source: Record "Source Media Bad"; var Target: Record "Target Media Bad")
+    begin
+        Target.Pictures := Source.Pictures;
+        Target.Modify(true);
+    end;
+}

microsoft/knowledge/data-modeling/share-mediaset-items-with-insert-not-field-assignment.good.al

@@ -0,0 +1,29 @@
+table 50438 "Source Media Good"
+{
+    fields
+    {
+        field(1; Code; Code[20]) { }
+        field(10; Pictures; MediaSet) { }
+    }
+}
+
+table 50439 "Target Media Good"
+{
+    fields
+    {
+        field(1; Code; Code[20]) { }
+        field(20; Pictures; MediaSet) { }
+    }
+}
+
+codeunit 50440 "Share Media Good"
+{
+    procedure CopyPictures(Source: Record "Source Media Good"; var Target: Record "Target Media Good")
+    var
+        Index: Integer;
+    begin
+        for Index := 1 to Source.Pictures.Count() do
+            Target.Pictures.Insert(Source.Pictures.Item(Index));
+        Target.Modify(true);
+    end;
+}

microsoft/knowledge/data-modeling/share-mediaset-items-with-insert-not-field-assignment.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: data-modeling
+keywords: [mediaset, media, insert, field-assignment, tenant-media, delete-integrity, sharing]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Share MediaSet items with Insert instead of field assignment
+
+## Description
+
+`Media` and `MediaSet` fields store IDs that reference tenant media system tables. When a record is deleted, the runtime looks for other references only in the same table and field index; it does not scan every table. Directly assigning a media-set field between different table types copies the ID without registering a separate media-set reference, so deleting one record can remove media that the other record still appears to reference.
+
+## Best Practice
+
+When sharing media between different tables, iterate the source `MediaSet` and call `Target.MediaSetField.Insert(Source.MediaSetField.Item(Index))`, then modify the target record. Direct field assignment is safe only when source and target are the same record subtype and use the same field ID. This concern is about reference/delete integrity, not the separate performance cost of `ModifyAll` on tables with media fields.
+
+See sample: `share-mediaset-items-with-insert-not-field-assignment.good.al`.
+
+## Anti Pattern
+
+`Target.Picture := Source.Picture;` where the two variables refer to different table types or different media-field IDs. The code copies an opaque ID, but the platform does not know that two independent fields now share the media object.
+
+See sample: `share-mediaset-items-with-insert-not-field-assignment.bad.al`.

microsoft/knowledge/data-modeling/table-relation-extensions-are-additive-and-top-down.bad.al

@@ -0,0 +1,35 @@
+enum 50434 "Relation Type Bad"
+{
+    Extensible = true;
+
+    value(0; Customer) { }
+}
+
+table 50435 "Related Entity Bad"
+{
+    fields
+    {
+        field(1; Type; Enum "Relation Type Bad") { }
+        field(2; "Related No."; Code[20])
+        {
+            // This unconditional relation wins before extension branches run.
+            TableRelation = Customer;
+        }
+    }
+}
+
+enumextension 50436 "Relation Type Bad Ext" extends "Relation Type Bad"
+{
+    value(10; Resource) { }
+}
+
+tableextension 50437 "Related Entity Bad Ext" extends "Related Entity Bad"
+{
+    fields
+    {
+        modify("Related No.")
+        {
+            TableRelation = if (Type = const(Resource)) Resource;
+        }
+    }
+}

microsoft/knowledge/data-modeling/table-relation-extensions-are-additive-and-top-down.good.al

@@ -0,0 +1,37 @@
+enum 50430 "Relation Type Good"
+{
+    Extensible = true;
+
+    value(0; Customer) { }
+    value(1; Item) { }
+}
+
+table 50431 "Related Entity Good"
+{
+    fields
+    {
+        field(1; Type; Enum "Relation Type Good") { }
+        field(2; "Related No."; Code[20])
+        {
+            TableRelation =
+                if (Type = const(Customer)) Customer
+                else if (Type = const(Item)) Item;
+        }
+    }
+}
+
+enumextension 50432 "Relation Type Resource" extends "Relation Type Good"
+{
+    value(10; Resource) { }
+}
+
+tableextension 50433 "Related Entity Resource" extends "Related Entity Good"
+{
+    fields
+    {
+        modify("Related No.")
+        {
+            TableRelation = if (Type = const(Resource)) Resource;
+        }
+    }
+}

microsoft/knowledge/data-modeling/table-relation-extensions-are-additive-and-top-down.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: data-modeling
+keywords: [tablerelation, tableextension, enumextension, additive, top-down, unconditional-relation]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Design TableRelation branches for additive top-down extension
+
+## Description
+
+A `tableextension` can add to an existing `TableRelation`, but the combined relation is evaluated top-down after the original value. The first unconditional relation wins. An extension branch appended after an unconditional base relation is therefore unreachable, even though the extension compiles and appears to describe the new enum value correctly.
+
+## Best Practice
+
+When a relation is designed to follow an extensible enum, express the base cases as conditional branches and leave no unconditional catch-all ahead of future extension branches. An enum extension can then append a condition for its new value. When extending a field you do not own, inspect the original `TableRelation`; do not claim that an appended condition overrides an unconditional relation.
+
+See sample: `table-relation-extensions-are-additive-and-top-down.good.al`.
+
+## Anti Pattern
+
+A base field has an unconditional `TableRelation = Customer;` and a `tableextension` adds `if (Type = const(Resource)) Resource`. The original unconditional branch always wins, so the new enum value still validates and looks up against Customer. The concern is evaluation order, not `ValidateTableRelation`; free-form input is covered separately by security guidance.
+
+See sample: `table-relation-extensions-are-additive-and-top-down.bad.al`.

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

@@ -15,10 +15,13 @@ codeunit 50185 "Collect Errors Good Sample"
             until Item.Next() = 0;
 
         if HasCollectedErrors() then begin
-            CollectedErrors := GetCollectedErrors();
+            // The default is false; true retrieves and clears the collection.
+            CollectedErrors := GetCollectedErrors(true);
+            // This blocking aggregate intentionally retains messages only.
             foreach CollectedError in CollectedErrors do
                 ErrorText += CollectedError.Message() + '\';
-            Message('The following must be fixed before posting:\%1', ErrorText);
+            Error(ErrorInfo.Create(
+                StrSubstNo('The following must be fixed before posting:\%1', ErrorText), false));
         end;
     end;
 }
@@ -30,8 +33,10 @@ codeunit 50186 "Collect Errors Item Check"
     trigger OnRun()
     begin
         if Rec.Description = '' then
-            Error('Item %1 has no description.', Rec."No.");
+            Error(ErrorInfo.Create(
+                StrSubstNo('Item %1 has no description.', Rec."No."), true));
         if Rec."Unit Cost" <= 0 then
-            Error('Item %1 must have a positive unit cost.', Rec."No.");
+            Error(ErrorInfo.Create(
+                StrSubstNo('Item %1 must have a positive unit cost.', Rec."No."), true));
     end;
 }

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

@@ -1,5 +1,5 @@
 ---
-bc-version: [all]
+bc-version: [19..]
 domain: error-handling
 keywords: [collectible-errors, errorbehavior, collect, getcollectederrors, hascollectederrors, validation, batch]
 technologies: [al]
@@ -11,16 +11,16 @@ application-area: [all]
 
 ## 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.
+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 collectible errors occur and gathers them, so all failures can be presented together. `GetCollectedErrors()` returns a `List of [ErrorInfo]` for the handler to inspect, but does not clear the collection by default; pass `true` to retrieve and clear in one call, or call `ClearCollectedErrors()` explicitly after retrieving. A handler can copy record information into a custom error page as Microsoft Learn demonstrates, or deliberately format only the messages into a final blocking error as this article's sample does.
 
 ## 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.
+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()`, retrieve and clear the list with `GetCollectedErrors(true)`, and fail the operation with the collected messages. The sample intentionally produces a text aggregate and does not claim to retain record/field metadata in the final error. If that metadata is needed, map each `ErrorInfo` to a custom error UI before clearing, following the Microsoft Learn pattern. Do not replace validation failure with `Message`: clearing collected errors suppresses the platform failure, so the custom handler must still block the invalid operation.
 
 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.
+Three shapes signal trouble. Hand-rolled accumulation reimplements collection and prevents the handler from receiving individual `ErrorInfo` values. A `Collect` procedure that never handles the collection falls back to the concatenated platform dialog. Finally, code that calls parameterless `GetCollectedErrors()`, assumes it cleared the list, and only shows a `Message` can both leave the errors collected and allow invalid processing to continue.
 
 See sample: `collect-validation-errors-with-errorbehavior.bad.al`.

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

@@ -7,7 +7,6 @@ codeunit 50190 "Error Type Good Sample"
         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;

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

@@ -1,5 +1,5 @@
 ---
-bc-version: [all]
+bc-version: [14..]
 domain: error-handling
 keywords: [errorinfo, errortype, internal, client, telemetry, diagnostics, generic-message]
 technologies: [al]
@@ -15,7 +15,7 @@ application-area: [all]
 
 ## 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`.
+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` 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`.
 

microsoft/knowledge/error-handling/fielderror-default-message-logic.bad.al

@@ -9,7 +9,7 @@ table 50120 "FieldError Default Bad"
 
     procedure ValidateForRelease()
     begin
-        // Re-testing a field and handing FieldError a fully-formed sentence.
+        // This re-tests a field and gives FieldError a fully formed sentence.
         // The framework already prepends the caption and appends the value,
         // so this renders as "Currency Code The Currency Code field must have
         // a value. in ..." — caption repeated, capital letter mid-sentence,

microsoft/knowledge/error-handling/fielderror-default-message-logic.good.al

@@ -9,9 +9,8 @@ table 50120 "FieldError Default Good"
 
     procedure ValidateForRelease()
     begin
-        // Plain required-field gate: TestField checks the condition and raises
-        // the error in one call, with caption and record context supplied by
-        // the framework.
+        // TestField checks this required-field condition and raises the error
+        // with caption and record context supplied by the framework.
         TestField("Currency Code");
 
         // Condition already evaluated: pass only a lowercase predicate so it

microsoft/knowledge/error-handling/fielderror-default-message-logic.md

@@ -8,13 +8,15 @@ application-area: [all]
 ---
 # Rely On FieldError's Auto-Generated Context And Pass Only A Lowercase Predicate
 
-> Contributions welcome — open a PR to refine or extend this article.
-
 ## Description
 `Rec.FieldError(FieldNo)` does not just print the text you give it. Business Central automatically prepends the field caption, appends the current field value (when non-blank), and suffixes the table name and primary-key values for record identification. The optional second argument is only the middle predicate of that sentence — e.g. `"must be unique"`, not a whole self-contained message. Misunderstanding this leads to messages that duplicate the caption and value or read as broken grammar, because the framework's surrounding text is built to join a lowercase fragment.
 
 ## Best Practice
 For a plain required-field check, prefer `TestField`, which tests the condition and raises the error in one call. When the condition is non-trivial and has already been evaluated, call `FieldError(FieldNo)` with no message to get the localized default (`must have a value`, `is not valid`, etc.), or pass a short lowercase predicate such as `FieldError(FieldNo, 'must be a positive number')`. Start the custom text with a lowercase letter so it reads as one sentence with the auto-inserted caption, and use a field-number reference (or the field token) rather than a hard-coded field name so captions and translations stay correct. Let the framework supply the caption, value, table, and key context for you.
 
+See sample: `fielderror-default-message-logic.good.al`.
+
 ## Anti Pattern
 Re-testing a condition you already evaluated, or passing a fully formed sentence like `'The Amount field must be positive.'` to `FieldError`. The result reads as `Amount The Amount field must be positive. in Gen. Journal Line ...` — capital letter mid-sentence, caption and value repeated, and a stray trailing clause. Reviewer signals: a `FieldError` argument that names the field, restates the current value, starts with a capital letter, or ends with a period. Each is a sign the author treated `FieldError` like `Error` instead of as a predicate slotted into framework-generated context.
+
+See sample: `fielderror-default-message-logic.bad.al`.

microsoft/knowledge/error-handling/fielderror-vs-testfield.bad.al

@@ -9,7 +9,7 @@ table 50122 "FieldError vs TestField Bad"
 
     procedure PostDocument()
     begin
-        // FieldError performs no comparison and always raises the moment it is
+        // FieldError performs no comparison and raises as soon as it is
         // reached, so this "check" terminates PostDocument every time — the
         // Posting Date is never actually tested, and the amount rule below is
         // dead code.

microsoft/knowledge/error-handling/fielderror-vs-testfield.good.al

@@ -9,8 +9,8 @@ table 50122 "FieldError vs TestField Good"
 
     procedure PostDocument()
     begin
-        // Simple presence gate: TestField performs the check itself and raises
-        // only when the field is empty. Self-documenting prerequisite.
+        // TestField performs this simple presence check and raises only when
+        // the field is empty. Self-documenting prerequisite.
         TestField("Posting Date");
 
         // Business logic has already determined the value is invalid;

microsoft/knowledge/error-handling/fielderror-vs-testfield.md

@@ -8,13 +8,15 @@ application-area: [all]
 ---
 # Choose `TestField` For Conditional Checks And `FieldError` For Already-Failed Validation
 
-> Contributions welcome — open a PR to refine or extend this article.
-
 ## Description
 `TestField` and `FieldError` look interchangeable but behave differently, and choosing the wrong one produces either dead code or a check that never fires. `TestField` performs the comparison itself and throws only when the field is empty or does not match the supplied value; `FieldError` performs no comparison and always raises an error the moment it is reached. Both attach the field caption and the record's primary-key context to the message automatically, which is why neither should be replaced by a hand-built `Error` call that interpolates the field name as a literal.
 
 ## Best Practice
 Use `TestField` when the condition is a simple presence-or-equality check on a single field — mandatory-field gates and prerequisite checks at the top of a procedure read clearly and self-document intent. Use `FieldError` inside an `OnValidate` trigger or a validation procedure where surrounding business logic has already determined the value is invalid and you want a specific, custom message. Rely on the built-in field-and-record context both methods add rather than re-stating the field name in the text.
 
+See sample: `fielderror-vs-testfield.good.al`.
+
 ## Anti Pattern
 Calling `FieldError` to "test" a field — placing it on a path that is reached unconditionally and expecting it to validate — terminates execution every time because `FieldError` never evaluates a condition. The inverse smell is reaching for `TestField` when the rule needs a tailored message, then bolting a vague generic string onto a check that cannot express the real business reason. A reviewer can spot the first by a `FieldError` that is not guarded by a preceding `if`, and the second by a `TestField` whose intent comment describes a condition more complex than presence or equality.
+
+See sample: `fielderror-vs-testfield.bad.al`.

microsoft/knowledge/error-handling/ignored-tryfunction-return-disables-try-semantics.bad.al

@@ -0,0 +1,17 @@
+codeunit 50301 "Try Return Bad"
+{
+    procedure ImportDocument()
+    begin
+        // Ignoring the Boolean result makes this an ordinary, throwing call.
+        TryImportDocument();
+    end;
+
+    [TryFunction]
+    local procedure TryImportDocument()
+    begin
+        Error(SourceRejectedErr);
+    end;
+
+    var
+        SourceRejectedErr: Label 'The source document was rejected.';
+}

microsoft/knowledge/error-handling/ignored-tryfunction-return-disables-try-semantics.good.al

@@ -0,0 +1,18 @@
+codeunit 50300 "Try Return Good"
+{
+    procedure ImportDocument()
+    begin
+        if not TryImportDocument() then
+            Error(ImportFailedErr);
+    end;
+
+    [TryFunction]
+    local procedure TryImportDocument()
+    begin
+        Error(SourceRejectedErr);
+    end;
+
+    var
+        ImportFailedErr: Label 'The document could not be imported.';
+        SourceRejectedErr: Label 'The source document was rejected.';
+}

microsoft/knowledge/error-handling/ignored-tryfunction-return-disables-try-semantics.md

@@ -0,0 +1,30 @@
+---
+bc-version: [13..]
+domain: error-handling
+keywords: [tryfunction, try-method, boolean-return, ignored-return-value, error-propagation]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Consume a TryFunction return value to enable try semantics
+
+## Description
+
+A procedure marked `[TryFunction]` catches errors only when the caller uses its Boolean return value. An assignment or conditional makes the invocation a try-method call; a bare call is treated as an ordinary procedure call and exposes errors as usual. The attribute alone does not make every invocation non-throwing.
+
+## Best Practice
+
+Consume the result directly: assign it to a Boolean or use the call in an `if` condition. Handle `false` immediately while the last-error state still describes that failure.
+
+See sample: `ignored-tryfunction-return-disables-try-semantics.good.al`.
+
+## Anti Pattern
+
+Calling a `[TryFunction]` procedure as a standalone statement and assuming the attribute suppresses its errors. The call has ordinary error semantics because its Boolean result is ignored.
+
+See sample: `ignored-tryfunction-return-disables-try-semantics.bad.al`.
+
+## See also
+
+`microsoft/knowledge/performance/use-tryfunction-for-error-catching-not-rollback.md` owns transaction rollback expectations after a try method has actually caught an error.

microsoft/knowledge/error-handling/page-boolean-triggers-default-to-true.md

@@ -0,0 +1,24 @@
+---
+bc-version: [all]
+domain: error-handling
+keywords: [oninsertrecord, onmodifyrecord, ondeleterecord, onquerypage, boolean-trigger, exit, false-positive]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Page record triggers return true by default; a missing exit(true) does not block the operation
+
+## Description
+
+The Boolean page record triggers `OnInsertRecord`, `OnModifyRecord`, `OnDeleteRecord`, and `OnQueryClosePage` return `true` by default. When the trigger body omits an explicit return value, the platform treats the result as `true` and the operation proceeds. Only an explicit `exit(false)` — or a reachable code path that returns `false` — cancels the insert, modify, delete, or page close.
+
+This is a defined exception to the ordinary Boolean method rule, where the default return is `false`. Reviewers unfamiliar with the exception sometimes read a page record trigger that has no `exit(true)` and conclude the operation is blocked; it is not.
+
+## Best Practice
+
+Do not claim that a missing `exit(true)` blocks or prevents an insert, modify, or delete, and do not recommend adding `exit(true)` "to let the operation proceed" — that is already the default. Evaluate these triggers only for an explicit or reachable `exit(false)`/false-returning path that would cancel the operation unintentionally.
+
+## Anti Pattern
+
+Flagging `OnInsertRecord`, `OnModifyRecord`, `OnDeleteRecord`, or `OnQueryClosePage` as defective because it "does not return `true`", or asserting that inserts/modifies/deletes will silently fail without an explicit `exit(true)`. The default return already permits the operation.

microsoft/knowledge/error-handling/unchecked-get-throws-when-record-not-found.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: error-handling
+keywords: [get, record-not-found, runtime-error, return-value, boolean-method, false-positive]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# An unchecked Record.Get raises an error when the record is missing; it is not silently ignored
+
+## Description
+
+`Record.Get` returns a Boolean, but its behavior when no record is found depends on whether the return value is consumed. When the return value is used — inside `if Rec.Get(...) then`, or assigned to a variable — a missing record yields `false` and execution continues. When `Rec.Get(...)` is called as a bare statement and the return value is not used, the platform raises a runtime "record not found" error if the record does not exist. A bare `Rec.Get(Key)` therefore acts as an assertion that the record exists: it does not swallow or silently ignore a missing record. This mirrors other AL find methods, where an unconsumed return value lets the platform enforce the not-found error.
+
+## Best Practice
+
+Do not claim that a `Record.Get` whose return value is unused silently ignores a missing record or hides an error. Treat a bare `Rec.Get(...)` statement as an intentional existence assertion that already throws when the record is absent. Recommend an explicit existence check only when the surrounding logic must continue gracefully rather than error out.
+
+## Anti Pattern
+
+Flagging a bare `Rec.Get(Key)` statement as a defect because "the return value is ignored, so a missing record is swallowed", or recommending it be wrapped in `if Rec.Get(...) then ... else Error(...)` to "handle the not-found case" — the unchecked call already raises an error when the record is missing.
+
+## See also
+
+- `ignored-tryfunction-return-disables-try-semantics.md` — a different case where ignoring a Boolean return value changes behavior.

microsoft/knowledge/events/add-new-event-parameters-at-the-end.bad.al

@@ -1,21 +1,14 @@
-// Demonstration-only AL. Not compiled by CI; illustrates the article.
+// Demonstration-only AL. Version 1 exposed PostDocument(SalesHeader).
 codeunit 50251 "Param Append Bad Sample"
 {
-    procedure PostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
-    var
-        IsHandled: Boolean;
+    procedure PostDocument(var SalesHeader: Record "Sales Header")
     begin
-        IsHandled := false;
-        // Anti-pattern: 'CalledFromBatch' was inserted before the existing
-        // IsHandled parameter, shifting it and breaking the argument positions
-        // every existing subscriber relied on.
-        OnBeforePostDocument(SalesHeader, CalledFromBatch, IsHandled);
-        if IsHandled then
-            exit;
+        // Existing callers cannot supply the newly required argument.
+        OnBeforePostDocument(SalesHeader);
     end;
 
     [IntegrationEvent(false, false)]
-    local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean; var IsHandled: Boolean)
+    procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
     begin
     end;
 }

microsoft/knowledge/events/add-new-event-parameters-at-the-end.good.al

@@ -1,4 +1,4 @@
-// Demonstration-only AL. Not compiled by CI; illustrates the article.
+// Demonstration-only AL. Version 1 had SalesHeader and IsHandled parameters.
 codeunit 50250 "Param Append Good Sample"
 {
     procedure PostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
@@ -6,15 +6,24 @@ codeunit 50250 "Param Append Good Sample"
         IsHandled: Boolean;
     begin
         IsHandled := false;
-        // The new 'CalledFromBatch' parameter was appended at the end of the
-        // existing signature, so existing subscribers needed no re-mapping.
-        OnBeforePostDocument(SalesHeader, IsHandled, CalledFromBatch);
+        // Subscribers bind by name, so the new parameter can sit between the
+        // existing parameters without breaking subscribers that omit it.
+        OnBeforePostDocument(SalesHeader, CalledFromBatch, IsHandled);
         if IsHandled then
             exit;
     end;
 
     [IntegrationEvent(false, false)]
-    local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean; CalledFromBatch: Boolean)
+    local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean; var IsHandled: Boolean)
     begin
     end;
 }
+
+codeunit 50252 "Existing Param Subscriber"
+{
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::"Param Append Good Sample", 'OnBeforePostDocument', '', false, false)]
+    local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean)
+    begin
+        IsHandled := SalesHeader."No." = '';
+    end;
+}

microsoft/knowledge/events/add-new-event-parameters-at-the-end.md

@@ -1,26 +1,26 @@
 ---
 bc-version: [all]
 domain: events
-keywords: [event-parameters, signature, backward-compatibility, append, onbefore, integration-event, versioning]
+keywords: [event-parameters, signature, backward-compatibility, public-event, local-event, internal-event, appsourcecop, as0024, as0025]
 technologies: [al]
 countries: [w1]
 application-area: [all]
 ---
 
-# Add new event parameters at the end
+# Event parameter additions depend on publisher access, not position
 
 ## Description
 
-Adding a parameter to an existing event publisher changes its signature. Appending the new parameter at the end of the parameter list keeps the change easy to review and track: existing subscribers still bind to the leading parameters, and the diff is a single clean addition. Inserting a parameter in the middle makes diffs noisy and harder to review, and obscures the history of how the signature evolved. New parameters belong after the existing ones.
+Event subscribers bind publisher parameters by name and can omit parameters they do not use. A `local` or `internal` Business or Integration event can therefore gain a parameter at any position without breaking subscriber-only consumers; appending is not a compatibility requirement. A public event is also a public procedure that dependent extensions can raise, so adding a required parameter anywhere breaks callers under AppSourceCop AS0024.
 
 ## Best Practice
 
-When extending an existing publisher, append the new parameter after all existing ones, including after a trailing `var IsHandled: Boolean` when present. Subscribers that already match keep working against the leading parameters, and the change stays a one-line addition that is trivial to review.
+Add a parameter directly only when the shipped event publisher is `local` or `internal`. Place it where the signature is clearest; existing subscribers continue binding the parameters they name. For a public event, keep the original publisher unchanged and introduce a new event with the expanded contract.
 
 See sample: `add-new-event-parameters-at-the-end.good.al`.
 
 ## Anti Pattern
 
-Inserting a new parameter in the middle of an existing event's signature, shifting every subsequent parameter and making the change noisy and harder to review. Detection: a changed event signature where an added parameter appears before existing parameters rather than at the tail of the list.
+Appending a parameter to a public event and assuming its position makes the change compatible. Existing external callers still lack the new required argument. Conversely, do not flag a parameter inserted among existing parameters on a `local` or `internal` Business or Integration event merely because it was not appended.
 
 See sample: `add-new-event-parameters-at-the-end.bad.al`.

microsoft/knowledge/events/adding-a-parameter-to-an-event-is-not-a-breaking-change.md

@@ -0,0 +1,18 @@
+---
+bc-version: [all]
+domain: events
+keywords: [event-parameters, signature, subscriber-binding, backward-compatibility, integration-event, breaking-change, false-positive]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Adding a parameter to an event is not a breaking change
+
+## Description
+
+Adding a parameter to an existing event publisher does not break existing subscribers. AL binds a subscriber to a publisher by the event name, and the subscriber's parameter list only has to be a subset of the publisher's, matched by name and type. A subscriber that does not declare the new parameter keeps compiling and keeps binding — it simply ignores the addition. This holds for `IntegrationEvent` and `BusinessEvent` publishers, and even more plainly for `local` events. Appending the new parameter at the end keeps the change a clean, reviewable addition (see `add-new-event-parameters-at-the-end`). LLM reviewers often misreport the mere presence of a new event parameter as a "breaking event signature change" that breaks subscribers, which is incorrect.
+
+## Best Practice
+
+Do not flag the addition of a parameter to an event publisher as a breaking or signature-breaking change, and do not claim it breaks existing subscribers. Genuine, separate concerns are covered by their own rules — a parameter inserted in the middle of the list rather than appended (`add-new-event-parameters-at-the-end`), or a parameter that carries no meaningful value — and should be raised on those grounds, not framed as a backward-compatibility break.

microsoft/knowledge/events/do-not-add-ishandled-to-an-existing-event.bad.al

@@ -8,9 +8,9 @@ codeunit 50291 "New OnBefore Bad Sample"
     begin
         Total := 100;
 
-        // Anti-pattern: IsHandled was bolted onto the existing
-        // OnAfterCalculateTotal, changing its contract and breaking every
-        // subscriber that matched the original signature.
+        // Anti-pattern: IsHandled was bolted onto the existing OnAfter event.
+        // Regardless of compiler compatibility, this changes a notification
+        // into an override contract that existing subscribers did not expect.
         OnAfterCalculateTotal(SalesHeader, Total, IsHandled);
     end;
 

microsoft/knowledge/events/do-not-change-shipped-event-attribute-flags.bad.al

@@ -0,0 +1,14 @@
+// Demonstration-only AL. Version 1 used [IntegrationEvent(true, true, false)].
+codeunit 50531 "Shipment Events Bad"
+{
+    procedure NotifyShipment(ShipmentNo: Code[20])
+    begin
+        OnShipmentCreated(ShipmentNo);
+    end;
+
+    // Version 2 mutates all three contract-significant arguments in place.
+    [IntegrationEvent(false, false, true)]
+    local procedure OnShipmentCreated(ShipmentNo: Code[20])
+    begin
+    end;
+}

microsoft/knowledge/events/do-not-change-shipped-event-attribute-flags.good.al

@@ -0,0 +1,21 @@
+// Demonstration-only AL. The Isolated argument requires runtime 9.0 / BC20.
+codeunit 50530 "Shipment Events"
+{
+    procedure NotifyShipment(ShipmentNo: Code[20])
+    begin
+        OnShipmentCreated(ShipmentNo);
+        OnShipmentCreatedIsolated(ShipmentNo);
+    end;
+
+    // Preserve the shipped attribute contract.
+    [IntegrationEvent(true, true, false)]
+    local procedure OnShipmentCreated(ShipmentNo: Code[20])
+    begin
+    end;
+
+    // Publish a new event for different isolation and sender semantics.
+    [IntegrationEvent(false, false, true)]
+    local procedure OnShipmentCreatedIsolated(ShipmentNo: Code[20])
+    begin
+    end;
+}

microsoft/knowledge/events/do-not-change-shipped-event-attribute-flags.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: events
+keywords: [event-attribute, includesender, globalvaraccess, isolated-event, compatibility, integration-event, business-event, appsourcecop, as0021, as0101]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Do not change shipped event attribute flags
+
+## Description
+
+`IncludeSender` and, on Integration events, `GlobalVarAccess` have been event-contract flags since runtime 1.0. Removing sender or global access breaks subscribers, so AppSourceCop AS0021 prevents changing those flags from `true` to `false`. On runtime 9.0 and later (Business Central 2022 release wave 1, BC20), `Isolated` also controls transaction, error, and rollback behavior; AS0101 prevents adding, removing, or changing that argument.
+
+## Best Practice
+
+Keep every available attribute argument exactly as shipped. If new subscribers need different sender/global exposure, publish a new event with the desired flags. Apply the same rule to `Isolated` only on BC20 or later, where that argument exists. Raise both events while the original contract is supported, and choose preferred flags only when designing a new event.
+
+See sample: `do-not-change-shipped-event-attribute-flags.good.al`.
+
+## Anti Pattern
+
+Changing a shipped event's `IncludeSender` or `GlobalVarAccess` to modernize its design, including replacing `IncludeSender` with an explicit parameter. On BC20 or later, adding, removing, or toggling `Isolated` is equally contract-significant. Even a change that leaves old subscribers compiling can alter observable execution or exposure; version the event instead.
+
+See sample: `do-not-change-shipped-event-attribute-flags.bad.al`.

microsoft/knowledge/events/prefer-reusing-or-extending-existing-events.good.al

@@ -8,13 +8,13 @@ codeunit 50260 "Reuse Event Good Sample"
         IsHandled := false;
         // A single event, extended with CustomerNo appended at the end, covers
         // the need; no second event is raised beside it.
-        OnBeforeProcessOrder(SalesHeader, CustomerNo, IsHandled);
+        OnBeforeProcessOrder(SalesHeader, IsHandled, CustomerNo);
         if IsHandled then
             exit;
     end;
 
     [IntegrationEvent(false, false)]
-    local procedure OnBeforeProcessOrder(var SalesHeader: Record "Sales Header"; CustomerNo: Code[20]; var IsHandled: Boolean)
+    local procedure OnBeforeProcessOrder(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean; CustomerNo: Code[20])
     begin
     end;
 }

microsoft/knowledge/events/prefer-this-over-includesender-in-codeunit-events.md

@@ -7,20 +7,20 @@ countries: [w1]
 application-area: [all]
 ---
 
-# Prefer this over IncludeSender in codeunit events
+# Prefer this over IncludeSender in new codeunit events
 
 ## Description
 
-Some publishers set `IncludeSender` to `true` on `[IntegrationEvent]` or `[BusinessEvent]` so subscribers receive the publishing object as an implicit sender parameter. From Business Central 2024 release wave 2, a codeunit can instead pass itself explicitly with the `this` keyword as a normal, strongly-typed `Sender` parameter. Explicit passing is clearer at both the publisher and the subscriber: the sender appears in the signature, it is concretely typed to the publishing codeunit, and it avoids the implicit-parameter mechanics of `IncludeSender`. Reserve `IncludeSender = true` for cases where the sender genuinely cannot be passed explicitly. This guidance applies to code targeting Business Central 2024 release wave 2 or later, where the `this` keyword is available.
+When designing a new publisher, setting `IncludeSender` to `true` on `[IntegrationEvent]` or `[BusinessEvent]` gives subscribers the publishing object as an implicit sender parameter. From Business Central 2024 release wave 2, a codeunit can instead pass itself explicitly with the `this` keyword as a normal, strongly-typed `Sender` parameter. Explicit passing makes the sender visible and typed in the signature. This is new-event design guidance only: never change `IncludeSender` on an event that has already shipped.
 
 ## Best Practice
 
-Declare the publisher `[IntegrationEvent(false, false)]` with an explicit `Sender: Codeunit "…"` parameter and raise it with `this`, for example `OnBeforeProcessOrder(OrderNo, this);`. Subscribers then receive a typed sender they can call directly.
+For a new event, declare the publisher `[IntegrationEvent(false, false)]` with an explicit `Sender: Codeunit "…"` parameter and raise it with `this`, for example `OnBeforeProcessOrder(OrderNo, this);`. Subscribers then receive a typed sender they can call directly.
 
 See sample: `prefer-this-over-includesender-in-codeunit-events.good.al`.
 
 ## Anti Pattern
 
-Relying on `[IntegrationEvent(true, …)]` solely to hand subscribers the publisher instance, where a codeunit could pass `this` explicitly as a typed parameter. Detection: `IncludeSender = true` on a codeunit event whose only purpose is to expose the sender, in code targeting Business Central 2024 release wave 2 or later.
+Designing a new codeunit event with `[IntegrationEvent(true, …)]` solely to hand subscribers the publisher instance, where `this` could be passed explicitly as a typed parameter. Do not apply this rule by mutating a shipped event's attribute flags.
 
 See sample: `prefer-this-over-includesender-in-codeunit-events.bad.al`.

microsoft/knowledge/events/publish-thin-onbefore-onafter-integration-events.good.al

@@ -20,13 +20,14 @@ codeunit 50225 "Reservation Post Good Sample"
     var
         IsHandled: Boolean;
     begin
+        IsHandled := false;
         OnBeforeReserve(ReservationEntry, IsHandled);
-        if IsHandled then
-            exit;
-
+        if not IsHandled then begin
             ReservationEntry.Reserved := true;
             ReservationEntry.Modify(true);
+        end;
 
+        // OnAfter reports completion whether a subscriber or the base body handled it.
         OnAfterReserve(ReservationEntry);
     end;
 

microsoft/knowledge/events/treat-local-and-internal-events-as-subscriber-contracts.bad.al

@@ -0,0 +1,15 @@
+// Demonstration-only AL. Version 1 exposed var Score as an Integer.
+codeunit 50521 "Customer Scoring Events Bad"
+{
+    procedure ScoreCustomer(CustomerNo: Code[20]; ScoreText: Text)
+    begin
+        OnCustomerScored(CustomerNo, ScoreText);
+    end;
+
+    // 'local' limits raising, not subscription. Renaming Score to ScoreText,
+    // changing its type, and removing var all break existing subscribers.
+    [IntegrationEvent(false, false)]
+    local procedure OnCustomerScored(CustomerNo: Code[20]; ScoreText: Text)
+    begin
+    end;
+}

microsoft/knowledge/events/treat-local-and-internal-events-as-subscriber-contracts.good.al

@@ -0,0 +1,24 @@
+// Demonstration-only AL. Version 1 had CustomerNo and var Score parameters.
+codeunit 50520 "Customer Scoring Events"
+{
+    procedure ScoreCustomer(CustomerNo: Code[20]; Reason: Text; var Score: Integer)
+    begin
+        OnCustomerScored(CustomerNo, Reason, Score);
+    end;
+
+    // Adding Reason between existing parameters preserves subscriber bindings.
+    [IntegrationEvent(false, false)]
+    local procedure OnCustomerScored(CustomerNo: Code[20]; Reason: Text; var Score: Integer)
+    begin
+    end;
+}
+
+codeunit 50522 "Existing Scoring Subscriber"
+{
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::"Customer Scoring Events", 'OnCustomerScored', '', false, false)]
+    local procedure OnCustomerScored(CustomerNo: Code[20]; var Score: Integer)
+    begin
+        if CustomerNo = '' then
+            Score := 0;
+    end;
+}

microsoft/knowledge/events/treat-local-and-internal-events-as-subscriber-contracts.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: events
+keywords: [local-event, internal-event, event-subscriber, compatibility, access-modifier, integration-event, business-event, parameter-name, var-parameter, appsourcecop]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Treat local and internal events as subscriber contracts
+
+## Description
+
+The `local` and `internal` access modifiers on Business and Integration event publishers restrict who can raise the procedure; they do not prevent dependent extensions from subscribing. Once shipped, the event name and each existing parameter's name, type/subtype, and value-versus-`var` passing mode are compatibility contracts even when the publisher is not public. Parameter order is not a subscriber contract because subscribers bind the parameters they use by name. This differs from `[InternalEvent]`, which is module-only except for modules named by `internalsVisibleTo`.
+
+## Best Practice
+
+Preserve a shipped Business or Integration event's identity and every existing parameter's name, type/subtype, and passing mode regardless of the procedure access modifier. AS0025 protects names and types, while AS0063 and AS0077 protect removal and addition of `var`. New parameters may be added at any position on a `local` or `internal` event because subscribers can omit them; public event procedures follow the stricter caller contract described by `add-new-event-parameters-at-the-end`.
+
+See sample: `treat-local-and-internal-events-as-subscriber-contracts.good.al`.
+
+## Anti Pattern
+
+Renaming or removing an existing parameter, changing its type/subtype, or adding/removing its `var` modifier because the event publisher procedure is `local` or `internal`. AppSourceCop checks these subscriber-breaking changes because dependent event subscribers can still bind to the event. Reordering unchanged parameters, or inserting a new parameter among them, is not this anti-pattern.
+
+See sample: `treat-local-and-internal-events-as-subscriber-contracts.bad.al`.

microsoft/knowledge/events/use-ishandled-to-make-base-behaviour-overridable.good.al

@@ -7,6 +7,7 @@ codeunit 50220 "Shipping Charge Good Sample"
     begin
         // Give extensions a sanctioned seam to replace the calculation, then
         // skip the default logic when a subscriber has handled it.
+        IsHandled := false;
         OnBeforeCalculateShippingCharge(OrderAmount, Charge, IsHandled);
         if IsHandled then
             exit(Charge);

microsoft/knowledge/interfaces/extend-published-interfaces-dont-edit-them.bad.al

@@ -0,0 +1,16 @@
+// Demonstration-only AL. Version 1 shipped with only CalculateAmount().
+interface "I Shipping Quote Bad"
+{
+    procedure CalculateAmount(): Decimal;
+
+    // Added in version 2: every existing implementer now fails to compile.
+    procedure CalculateDeliveryDate(): Date;
+}
+
+codeunit 50511 "Existing Shipping Quote" implements "I Shipping Quote Bad"
+{
+    procedure CalculateAmount(): Decimal
+    begin
+        exit(10);
+    end;
+}

microsoft/knowledge/interfaces/extend-published-interfaces-dont-edit-them.good.al

@@ -0,0 +1,23 @@
+// Demonstration-only AL. Interface inheritance requires runtime 14.0 / BC25.
+interface "I Shipping Quote"
+{
+    procedure CalculateAmount(): Decimal;
+}
+
+interface "I Shipping Quote V2" extends "I Shipping Quote"
+{
+    procedure CalculateDeliveryDate(): Date;
+}
+
+codeunit 50510 "Shipping Quote V2" implements "I Shipping Quote V2"
+{
+    procedure CalculateAmount(): Decimal
+    begin
+        exit(10);
+    end;
+
+    procedure CalculateDeliveryDate(): Date
+    begin
+        exit(Today() + 1);
+    end;
+}

microsoft/knowledge/interfaces/extend-published-interfaces-dont-edit-them.md

@@ -0,0 +1,26 @@
+---
+bc-version: [16..]
+domain: interfaces
+keywords: [published-interface, interface-method, breaking-change, interface-extends, versioned-interface, appsourcecop, as0066]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Extend published interfaces; do not edit them
+
+## Description
+
+Adding a method to a shipped interface changes the contract every implementing codeunit must satisfy. Implementers can live in dependent extensions, so the addition breaks code the interface publisher cannot update; AppSourceCop reports AS0066. Interface inheritance is available from runtime 14.0 (Business Central 2024 release wave 2, BC25), but the original interface must remain unchanged.
+
+## Best Practice
+
+On BC25 or later, declare a new interface that `extends` the published interface and add the new method there. Existing implementers remain valid for the original contract, while new implementers opt in to the extended contract. For targets BC16 through BC24, where interface inheritance is unavailable, publish a new or versioned sibling interface instead.
+
+See sample: `extend-published-interfaces-dont-edit-them.good.al`.
+
+## Anti Pattern
+
+Adding a procedure directly to an interface that has already shipped. Every dependent implementation must immediately add that procedure, so an otherwise compatible app update breaks its implementers.
+
+See sample: `extend-published-interfaces-dont-edit-them.bad.al`.

microsoft/knowledge/interfaces/handle-unknown-enum-ordinals-with-unknownvalueimplementation.bad.al

@@ -0,0 +1,36 @@
+// Demonstration-only AL. A removed enum-extension value left ordinal 700 in data.
+enum 50503 "Delivery Method Bad" implements "I Delivery Method Bad"
+{
+    Extensible = true;
+    DefaultImplementation = "I Delivery Method Bad" = "Default Delivery Method Bad";
+
+    value(0; Default)
+    {
+    }
+}
+
+interface "I Delivery Method Bad"
+{
+    procedure Deliver();
+}
+
+codeunit 50504 "Default Delivery Method Bad" implements "I Delivery Method Bad"
+{
+    procedure Deliver()
+    begin
+    end;
+}
+
+codeunit 50505 "Delivery Dispatch Bad"
+{
+    procedure DeliverPersistedValue()
+    var
+        DeliveryMethod: Enum "Delivery Method Bad";
+        Delivery: Interface "I Delivery Method Bad";
+    begin
+        DeliveryMethod := 700;
+        // DefaultImplementation does not handle an ordinal that is not declared.
+        Delivery := DeliveryMethod;
+        Delivery.Deliver();
+    end;
+}

microsoft/knowledge/interfaces/handle-unknown-enum-ordinals-with-unknownvalueimplementation.good.al

@@ -0,0 +1,34 @@
+// Demonstration-only AL. UnknownValueImplementation requires runtime 7.0 / BC18.
+interface "I Delivery Method"
+{
+    procedure Deliver();
+}
+
+codeunit 50500 "Unknown Delivery Method" implements "I Delivery Method"
+{
+    procedure Deliver()
+    begin
+        Error(UnknownMethodErr);
+    end;
+
+    var
+        UnknownMethodErr: Label 'The saved delivery method is no longer installed. Select another method.';
+}
+
+codeunit 50501 "Default Delivery Method" implements "I Delivery Method"
+{
+    procedure Deliver()
+    begin
+    end;
+}
+
+enum 50502 "Delivery Method" implements "I Delivery Method"
+{
+    Extensible = true;
+    DefaultImplementation = "I Delivery Method" = "Default Delivery Method";
+    UnknownValueImplementation = "I Delivery Method" = "Unknown Delivery Method";
+
+    value(0; Default)
+    {
+    }
+}

microsoft/knowledge/interfaces/handle-unknown-enum-ordinals-with-unknownvalueimplementation.md

@@ -0,0 +1,26 @@
+---
+bc-version: [18..]
+domain: interfaces
+keywords: [unknownvalueimplementation, unknown-enum-value, persisted-ordinal, enum-extension, extension-uninstall, interface-fallback]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Handle unknown enum ordinals with UnknownValueImplementation
+
+## Description
+
+An enum ordinal can remain in persisted data after the enum extension that declared it is uninstalled. The ordinal is then unknown: it matches no currently declared enum value. `DefaultImplementation` does not cover this case; it covers declared values that have no explicit interface implementation. `UnknownValueImplementation`, available from runtime 7.0 (Business Central 2021 release wave 1, BC18), provides the distinct interface implementation for an unknown ordinal.
+
+## Best Practice
+
+On BC18 or later, set `UnknownValueImplementation = <Interface> = <Codeunit>;` on an enum that implements an interface and can be persisted. Use an implementation that reports a clear domain error or safely contains the unknown state. Keep `DefaultImplementation` separately when declared but unmapped values also need a fallback.
+
+See sample: `handle-unknown-enum-ordinals-with-unknownvalueimplementation.good.al`.
+
+## Anti Pattern
+
+Defining only `DefaultImplementation` and assuming it also handles a stored ordinal whose enum value has disappeared. After an enum extension is uninstalled, converting that unknown ordinal to the interface can produce a technical runtime error instead of controlled handling.
+
+See sample: `handle-unknown-enum-ordinals-with-unknownvalueimplementation.bad.al`.

microsoft/knowledge/interfaces/set-defaultimplementation-on-enum.md

@@ -11,11 +11,11 @@ application-area: [all]
 
 ## Description
 
-An `enum` that `implements` an interface maps each value to a codeunit through the `Implementation` property. But an extensible enum can carry values that set no `Implementation` — values added later by an extension, or a value left intentionally blank. Assigning such a value to an interface variable and calling a method on it fails at runtime unless the enum provides a fallback. The enum-level `DefaultImplementation` property names the codeunit used whenever a value has no explicit `Implementation`, so resolution always yields a usable object. LLMs are generally unaware this property exists and leave the gap open.
+An `enum` that `implements` an interface maps each declared value to a codeunit through the `Implementation` property. A declared value, including one supplied by an enum extension, can omit that mapping. Assigning that value to an interface variable then fails at runtime unless the enum provides `DefaultImplementation`. This property is for declared but unmapped values; an ordinal that is no longer declared is a different case covered by `handle-unknown-enum-ordinals-with-unknownvalueimplementation`.
 
 ## Best Practice
 
-On any extensible enum that implements an interface, set `DefaultImplementation = <Interface> = <Codeunit>;` at the enum level, pointing at a safe implementation that does nothing harmful. Values with their own `Implementation` keep using it; every other value — including ones added later by extensions — resolves to the default instead of failing. For the distinct case of an out-of-range integer that matches no declared value, pair it with `UnknownValueImplementation`. The result is that a consumer can assign any enum value to the interface variable and call through it without a runtime guard.
+On any extensible enum that implements an interface, set `DefaultImplementation = <Interface> = <Codeunit>;` at the enum level, pointing at a safe implementation. Values with their own `Implementation` keep using it; declared values without one resolve to the default. Do not rely on this property for persisted ordinals that match no declared enum value.
 
 See sample: `set-defaultimplementation-on-enum.good.al`.
 

microsoft/knowledge/performance/addloadfields-in-report-onpredataitem.bad.al

@@ -2,13 +2,22 @@ report 50221 "Perf Sample AddLoadFields Bad"
 {
     dataset
     {
-        // No AddLoadFields: every Cust. Ledger Entry column ships per row, even though
-        // only three columns feed the layout.
         dataitem(CustLedgerEntry; "Cust. Ledger Entry")
         {
             column(CustomerNo; "Customer No.") { }
             column(PostingDate; "Posting Date") { }
             column(Amount; Amount) { }
+
+            trigger OnAfterGetRecord()
+            begin
+                // Source Code is not a dataset column, so its first access causes a
+                // just-in-time load and updates the dataitem enumerator.
+                RegisterSourceCode("Source Code");
+            end;
         }
     }
+
+    local procedure RegisterSourceCode(SourceCode: Code[10])
+    begin
+    end;
 }

microsoft/knowledge/performance/addloadfields-in-report-onpredataitem.good.al

@@ -10,8 +10,19 @@ report 50220 "Perf Sample AddLoadFields Good"
 
             trigger OnPreDataItem()
             begin
-                AddLoadFields("Customer No.", "Posting Date", Amount);
+                // Dataset columns are selected by the report compiler. Source Code is
+                // extra because only trigger code reads it.
+                CustLedgerEntry.AddLoadFields("Source Code");
+            end;
+
+            trigger OnAfterGetRecord()
+            begin
+                RegisterSourceCode("Source Code");
             end;
         }
     }
+
+    local procedure RegisterSourceCode(SourceCode: Code[10])
+    begin
+    end;
 }

microsoft/knowledge/performance/addloadfields-in-report-onpredataitem.md

@@ -7,20 +7,20 @@ countries: [w1]
 application-area: [all]
 ---
 
-# In reports, declare the fields the layout needs with AddLoadFields
+# Add trigger-only report fields in OnPreDataItem
 
 ## Description
 
-Reports iterate dataitems on potentially large source tables and pipe rows into a layout. The partial-record optimization is the same idea as `use-setloadfields-for-partial-records.md`, but the API is different: per the upstream guidance, "for reports, use `AddLoadFields()` in `OnPreDataItem` trigger to add fields needed by the layout." `AddLoadFields` is additive — call it for each field the layout consumes — and runs once per dataitem before iteration begins.
+Report dataitem field selection is calculated at compile time and once per dataitem type during execution. Fields referenced by dataset columns are selected automatically; fields used only in triggers are not. Use `AddLoadFields` in `OnPreDataItem` to supplement the automatic selection with normal fields that trigger code needs.
 
 ## Best Practice
 
-In each dataitem's `OnPreDataItem` trigger, list the columns the layout binds to via `AddLoadFields(<field>, <field>, ...)`. The platform then materializes only those columns per row. Treat the layout column list as the spec: every column the layout uses must be added; columns the layout does not use should not be added.
+When a dataitem trigger needs an extra field, add that field in `OnPreDataItem` before iteration starts. This supplements the compiler-selected fields and avoids the first just-in-time load and enumerator update when the trigger reads the extra field.
 
 See sample: `addloadfields-in-report-onpredataitem.good.al`.
 
 ## Anti Pattern
 
-Relying on the dataitem's default to load every field. On a report bound to a ledger-scale table this transfers an entire row per iteration, of which the layout reads a fraction.
+Listing every dataset column in `AddLoadFields`, or omitting a known trigger-only field because the dataset already uses other fields. The former is redundant; the latter causes a just-in-time load on first access and can cause repeated loads when the record is copied or passed by value.
 
 See sample: `addloadfields-in-report-onpredataitem.bad.al`.

microsoft/knowledge/performance/avoid-cloning-records-before-modify-delete-in-loops.bad.al

@@ -0,0 +1,19 @@
+codeunit 50493 "Perf Record Clone Bad"
+{
+    procedure IncreaseCustomerCreditLimits(Percent: Decimal)
+    var
+        Customer: Record Customer;
+        CustomerCopy: Record Customer;
+    begin
+        Customer.SetLoadFields("Credit Limit (LCY)");
+        Customer.SetFilter("Credit Limit (LCY)", '>0');
+        if Customer.FindSet(true) then
+            repeat
+                CustomerCopy.Copy(Customer);
+                CustomerCopy.Validate(
+                    "Credit Limit (LCY)",
+                    Round(CustomerCopy."Credit Limit (LCY)" * (1 + Percent / 100)));
+                CustomerCopy.Modify(true);
+            until Customer.Next() = 0;
+    end;
+}

microsoft/knowledge/performance/avoid-cloning-records-before-modify-delete-in-loops.good.al

@@ -0,0 +1,17 @@
+codeunit 50492 "Perf Record Clone Good"
+{
+    procedure IncreaseCustomerCreditLimits(Percent: Decimal)
+    var
+        Customer: Record Customer;
+    begin
+        Customer.SetLoadFields("Credit Limit (LCY)");
+        Customer.SetFilter("Credit Limit (LCY)", '>0');
+        if Customer.FindSet(true) then
+            repeat
+                Customer.Validate(
+                    "Credit Limit (LCY)",
+                    Round(Customer."Credit Limit (LCY)" * (1 + Percent / 100)));
+                Customer.Modify(true);
+            until Customer.Next() = 0;
+    end;
+}

microsoft/knowledge/performance/avoid-cloning-records-before-modify-delete-in-loops.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: performance
+keywords: [clone, clone-before-write, copy, gettable, by-value, copied-record, writing-helper]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Avoid cloning records before Modify or Delete in loops
+
+## Description
+
+Microsoft's [AL database-method performance guidance](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/administration/optimize-sql-al-database-methods-and-performance-on-server#insert-modify-delete-and-locktable) states that cloning an iterated record before `Modify` or `Delete` restarts the SQL `SELECT` and issues an extra SQL statement for every row. The runtime treats `Record.Copy`, `RecordRef.GetTable`, and passing a record by value to a writing helper as clones in this situation.
+
+## Best Practice
+
+Use `FindSet(true)` when the loop writes the traversed rows, and call `Modify` or `Delete` on that iterating record variable. If generic code is required, open and iterate the `RecordRef` directly instead of calling `GetTable` for each typed record. Keep a per-row loop when validation or row-specific behavior is required; this rule does not imply that `ModifyAll` or `DeleteAll` is equivalent.
+
+See sample: `avoid-cloning-records-before-modify-delete-in-loops.good.al`.
+
+## Anti Pattern
+
+Inside an active traversal, copy the current row, convert it with `RecordRef.GetTable`, or pass it without `var` to a helper, then call `Modify` or `Delete` on that clone. Do not flag read-only snapshots, temporary records, or copies used to write a different target table; the documented extra-statement concern is clone-before-write on the traversed table.
+
+See sample: `avoid-cloning-records-before-modify-delete-in-loops.bad.al`.

microsoft/knowledge/performance/avoid-commit-inside-loops.good.al

@@ -1,21 +1,60 @@
+query 50127 "Perf Customer Chunk"
+{
+    QueryType = Normal;
+    OrderBy = ascending(CustomerNo);
+
+    elements
+    {
+        dataitem(Customer; Customer)
+        {
+            column(CustomerNo; "No.") { }
+        }
+    }
+}
+
 codeunit 50128 "Perf Sample CommitInLoop Good"
 {
     procedure NormalizeCustomerNames()
     var
-        Customer: Record Customer;
-        RowsInChunk: Integer;
-        ChunkSize: Integer;
+        LastCustomerNo: Code[20];
     begin
-        ChunkSize := 500;
-        if Customer.FindSet(true) then
+        // The outer loop owns checkpoints; the per-row loop contains no Commit.
+        while NormalizeNextChunk(LastCustomerNo) do
+            Commit();
+    end;
+
+    local procedure NormalizeNextChunk(var LastCustomerNo: Code[20]): Boolean
+    var
+        Customer: Record Customer;
+        TempCustomer: Record Customer temporary;
+        CustomerChunk: Query "Perf Customer Chunk";
+        LastChunkCustomerNo: Code[20];
+    begin
+        CustomerChunk.TopNumberOfRows(500);
+        if LastCustomerNo <> '' then
+            CustomerChunk.SetFilter(CustomerNo, '>%1', LastCustomerNo);
+        CustomerChunk.Open();
+        while CustomerChunk.Read() do begin
+            TempCustomer.Init();
+            TempCustomer."No." := CustomerChunk.CustomerNo;
+            TempCustomer.Insert();
+            LastChunkCustomerNo := CustomerChunk.CustomerNo;
+        end;
+        CustomerChunk.Close();
+
+        if TempCustomer.IsEmpty() then
+            exit(false);
+
+        Customer.LockTable();
+        if TempCustomer.FindSet() then
             repeat
+                if Customer.Get(TempCustomer."No.") then begin
                     Customer.Name := UpperCase(Customer.Name);
                     Customer.Modify();
-                RowsInChunk += 1;
-                if RowsInChunk >= ChunkSize then begin
-                    Commit();
-                    RowsInChunk := 0;
                 end;
-            until Customer.Next() = 0;
+            until TempCustomer.Next() = 0;
+
+        LastCustomerNo := LastChunkCustomerNo;
+        exit(true);
     end;
 }

microsoft/knowledge/performance/avoid-commit-inside-loops.md

@@ -1,7 +1,7 @@
 ---
 bc-version: [all]
 domain: performance
-keywords: [commit, loop, transaction, lock, checkpoint, codeunit-run]
+keywords: [commit, commit-in-loop, per-row-commit, checkpoint, bounded-checkpoint, watermark, topnumberofrows]
 technologies: [al]
 countries: [w1]
 application-area: [all]
@@ -13,17 +13,16 @@ application-area: [all]
 
 ## Description
 
-Commit ends the current write transaction. Calling it inside a per-row loop produces one transaction per iteration and loses the ability to roll back the whole operation atomically; it also interferes with the platform's ability to batch write operations. Most loops need no explicit Commit at all — AL auto-commits the enclosing code module on successful completion (see `understand-implicit-transaction-boundary.md`). When the batch is too large for one transaction, the fix is not a per-row Commit but bounded checkpoints that each process N rows.
+Commit ends the current write transaction. Calling it inside a per-row loop produces one transaction per iteration and loses the ability to roll back the whole operation atomically; it also interferes with the platform's ability to batch write operations. Most loops need no explicit Commit at all — AL auto-commits the enclosing code module on successful completion (see `understand-implicit-transaction-boundary.md`). When the batch is too large for one transaction, the fix is not a per-row Commit but bounded checkpoints that select an exact list of at most N keys and process only those rows.
 
 ## Best Practice
 
-If the batch is large enough that a single transaction is untenable, process it in checkpoints driven by an outer loop that each time picks up the next N rows. Commit once per checkpoint at a clearly defined safe boundary, not inside the per-row loop. Wrapping each chunk in `Codeunit.Run` gives the same effect with native rollback on failure — see `codeunit-run-as-atomic-sub-operation.md`.
+If the batch is large enough that a single transaction is untenable, use an ordered primary-key watermark and retrieve a bounded next-N key list. `FindSet` is optimized for reading the complete filtered set and isn't implemented as `TOP X`, so calling it over the remaining tail and breaking after N rows does not bound retrieval. The sample uses a query capped by [`TopNumberOfRows`](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/query/queryinstance-topnumberofrows-method) to fill a temporary key buffer, then takes update locks and modifies only those exact keys. It does not reconstruct an inclusive first-to-last range that concurrent inserts could expand. Commit after the bounded inner loop returns and persist its last selected key as the next watermark. Use a stable key and define how a later run handles records inserted at or below an already committed watermark. A `Codeunit.Run` boundary can also own a chunk when its implicit commit and error behavior fit the caller — see `codeunit-run-as-atomic-sub-operation.md`.
 
 See sample: `avoid-commit-inside-loops.good.al`.
 
 ## Anti Pattern
 
-Placing Commit inside `repeat ... until Next() = 0` is almost always a mistake: it is unusual for the correctness of the operation to depend on per-row commits, and the cost of starting a new transaction on every row dominates the work.
+Placing Commit inside `repeat ... until Next() = 0` is almost always a mistake: it is unusual for the correctness of the operation to depend on per-row commits, and the cost of starting a new transaction on every row dominates the work. A capped query that discovers only an upper key and then re-reads an inclusive key range is not exact batching either; concurrent inserts inside that range can enlarge the checkpoint.
 
 See sample: `avoid-commit-inside-loops.bad.al`.
-

microsoft/knowledge/performance/avoid-get-inside-loop-on-large-table.bad.al

@@ -1,13 +1,15 @@
 codeunit 50253 "Perf Sample NPlus1 Bad"
 {
-    procedure SumStdCost(var BOMLine: Record "BOM Component") TotalCost: Decimal
+    procedure SumStdCost(BOMNo: Code[20]; BOMVersionCode: Code[20]) TotalCost: Decimal
     var
+        BOMLine: Record "Production BOM Line";
         Item: Record Item;
     begin
+        BOMLine.SetRange("Production BOM No.", BOMNo);
+        BOMLine.SetRange("Version Code", BOMVersionCode);
         if BOMLine.FindSet() then
             repeat
-                // Full-row Item.Get per BOM line — no partial loading, no caching.
-                Item.Get(BOMLine."No.");
+                if Item.Get(BOMLine."No.") then
                     if Item."Costing Method" = Item."Costing Method"::Standard then
                         TotalCost += Item."Standard Cost" * BOMLine."Quantity per";
             until BOMLine.Next() = 0;

microsoft/knowledge/performance/avoid-get-inside-loop-on-large-table.good.al

@@ -1,15 +1,38 @@
-codeunit 50252 "Perf Sample NPlus1 Good"
+query 50252 "Perf Sample BOM Cost"
 {
-    procedure SumStdCost(var BOMLine: Record "BOM Component") TotalCost: Decimal
+    QueryType = Normal;
+
+    elements
+    {
+        dataitem(ProductionBOMLine; "Production BOM Line")
+        {
+            column(ProductionBOMNo; "Production BOM No.") { }
+            column(VersionCode; "Version Code") { }
+            column(QuantityPer; "Quantity per") { }
+
+            dataitem(Item; Item)
+            {
+                DataItemLink = "No." = ProductionBOMLine."No.";
+                DataItemTableFilter = "Costing Method" = const(Standard);
+                SqlJoinType = InnerJoin;
+
+                column(StandardCost; "Standard Cost") { }
+            }
+        }
+    }
+}
+
+codeunit 50254 "Perf Sample NPlus1 Good"
+{
+    procedure SumStdCost(BOMNo: Code[20]; BOMVersionCode: Code[20]) TotalCost: Decimal
     var
-        Item: Record Item;
+        BOMCost: Query "Perf Sample BOM Cost";
     begin
-        Item.SetLoadFields("Costing Method", "Standard Cost");
-        if BOMLine.FindSet() then
-            repeat
-                if Item.Get(BOMLine."No.") then
-                    if Item."Costing Method" = Item."Costing Method"::Standard then
-                        TotalCost += Item."Standard Cost" * BOMLine."Quantity per";
-            until BOMLine.Next() = 0;
+        BOMCost.SetRange(ProductionBOMNo, BOMNo);
+        BOMCost.SetRange(VersionCode, BOMVersionCode);
+        BOMCost.Open();
+        while BOMCost.Read() do
+            TotalCost += BOMCost.StandardCost * BOMCost.QuantityPer;
+        BOMCost.Close();
     end;
 }

microsoft/knowledge/performance/avoid-get-inside-loop-on-large-table.md

@@ -11,16 +11,16 @@ application-area: [all]
 
 ## Description
 
-A `Get` or `FindFirst` against a different record inside a loop body produces one database round-trip per iteration — the classic N+1 pattern. Per the upstream guidance, "Flag when a `Get()`/`FindFirst()` is called inside a loop for each record — this creates N+1 database round-trips." The cost only matters when the inner table is meaningful: lookups against temporary tables, singleton setup tables, enum-mapping tables, permission objects, or Role IDs are bounded and safe. The pattern to catch is the inner lookup that hits a production-scale table for every outer row.
+A `Get` or `FindFirst` against another persistent table inside a loop can produce an N+1 access pattern: one outer query followed by repeated inner lookups. Server and primary-key caches can satisfy some `Get` calls, so a source-level `Get` is not proof of one SQL round-trip. The concern is an unbounded loop whose lookup keys are not known to repeat or remain cached.
 
 ## Best Practice
 
-When the loop needs values from another record, lift the lookup out of the loop if the rows can be collected up front, or apply `SetLoadFields` so each inner read transfers only the columns the loop actually uses (see `use-setloadfields-for-partial-records.md`). When the inner record is small or bounded, leave the call site alone — the rule targets large-table inner lookups specifically.
+Use a query object to join the outer and inner tables when the relationship and filters can be expressed as one query. If keys repeat, a dictionary cache can reduce lookups to one per distinct key. `SetLoadFields` can reduce the columns transferred by unavoidable inner reads, but it does not eliminate the N+1 shape and must not be presented as doing so.
 
 See sample: `avoid-get-inside-loop-on-large-table.good.al`.
 
 ## Anti Pattern
 
-Iterating BOM lines and calling `Item.Get(BOMLine."No.")` per row to read a costing method, with no `SetLoadFields` on `Item`. Each iteration issues one query against Item (~800k rows) and pulls the entire row to read two fields. The fix is `Item.SetLoadFields("Costing Method", "Standard Cost");` ahead of the loop — still N reads, but each one transfers only the needed columns.
+Iterating production BOM lines and calling `Item.Get(BOMLine."No.")` for each line when the same result can be produced by a query joining Production BOM Line to Item. Partial loading alone is only a payload mitigation for this pattern.
 
 See sample: `avoid-get-inside-loop-on-large-table.bad.al`.

microsoft/knowledge/performance/calcfields-in-both-getrecord-triggers-is-not-redundant.md

@@ -0,0 +1,22 @@
+---
+bc-version: [all]
+domain: performance
+keywords: [calcfields, onaftergetrecord, onaftergetcurrrecord, page-lifecycle, flowfield, false-positive]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# CalcFields in both OnAfterGetRecord and OnAfterGetCurrRecord is not redundant
+
+## Description
+
+`OnAfterGetRecord` fires once per row as the page loads records into the view; `OnAfterGetCurrRecord` fires when a record becomes the active/current record. Calling `CalcFields` in both triggers is not duplicate or redundant work: the two triggers run at different points in the page lifecycle and serve different purposes — populating FlowFields for every displayed row versus refreshing them for the record the user has selected. The same `CalcFields` call appearing in both places is an intentional pattern, not copy-paste waste.
+
+## Best Practice
+
+Do not flag `CalcFields` appearing in both `OnAfterGetRecord` and `OnAfterGetCurrRecord` as duplicate, redundant, or removable. Treat each trigger's `CalcFields` on its own lifecycle merits.
+
+## Anti Pattern
+
+Recommending that a developer delete one of the two `CalcFields` calls because "the field is already calculated in the other trigger". The genuine per-row FlowField cost is addressed by the separate guidance on FlowField calculation in loops and on hidden FlowFields; it is not addressed by removing a lifecycle-correct `CalcFields`.

microsoft/knowledge/performance/choose-maintainsiftindex-by-read-write-ratio.md

@@ -13,11 +13,11 @@ application-area: [all]
 
 ## Description
 
-`MaintainSIFTIndex` on a key decides whether the SIFT aggregate structure is updated on every `INSERT`, `MODIFY`, and `DELETE` that touches the key's fields. With `Yes`, `CalcSums` and FlowField reads are immediate — but every write pays the cost of updating the aggregate. With `No`, writes are cheaper but the first aggregate read after a change has to rebuild. Neither value is universally correct; the right choice depends on how often the aggregate is read versus how often the underlying rows are written.
+`MaintainSIFTIndex` on a key decides whether SQL Server maintains the SIFT indexed view as underlying rows change. With `Yes`, writes that affect the key or sum fields also maintain the indexed aggregate. With `No`, that SIFT indexed view is not maintained, so a compatible `CalcSums` or FlowField calculation is computed from the base table instead and may require scanning many rows. There is no deferred "first read rebuild" of the SIFT structure.
 
 ## Best Practice
 
-Measure read-to-write ratios for the key's SIFT fields under realistic workloads. Set `MaintainSIFTIndex = Yes` only on keys whose aggregates are read far more often than the rows are written (reporting keys on reference tables, dashboards). Set `No` on keys whose rows are written heavily and whose aggregates are read rarely (transactional ledger entries, import-staging tables).
+Measure aggregate-read latency and write cost under realistic filters and volumes. Keep `MaintainSIFTIndex = true` when the maintained aggregate materially benefits frequent `CalcSums` or FlowField reads. Consider `false` when writes dominate and the less-frequent aggregate reads can tolerate calculation from the base table.
 
 See sample: `choose-maintainsiftindex-by-read-write-ratio.good.al`.