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
 

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/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/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 field was renamed while retaining ID 2.
+        // 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(2; "Contact Email"; Text[80]) { }
+        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,7 +7,7 @@ countries: [w1]
 application-area: [all]
 ---
 
-# Obsolete published table fields instead of deleting or renumbering them
+# Obsolete published table fields instead of deleting, renaming, or renumbering them
 
 ## Description
 
@@ -15,12 +15,12 @@ A shipped table field carries both a source-level contract and persisted data. R
 
 ## Best Practice
 
-Add the replacement field under a new 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.
+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 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 moving the replacement to another ID without migration additionally risks losing its stored values. Detection: a previously shipped field removed, renumbered, or renamed with no retained `Pending` field and migration path.
+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/fielderror-default-message-logic.md

@@ -14,5 +14,9 @@ application-area: [all]
 ## 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.
+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.md

@@ -14,5 +14,9 @@ application-area: [all]
 ## 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.md

@@ -24,3 +24,7 @@ See sample: `ignored-tryfunction-return-disables-try-semantics.good.al`.
 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/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/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/hidden-flowfields-still-calculate-before-bc26-opt-in.bad.al

@@ -0,0 +1,18 @@
+pageextension 50445 "Customer Balance Hidden" extends "Customer Card"
+{
+    layout
+    {
+        addlast(General)
+        {
+            field(Balance; Rec.Balance)
+            {
+                ApplicationArea = All;
+                ToolTip = 'Specifies the customer balance.';
+                Visible = ShowBalance;
+            }
+        }
+    }
+
+    var
+        ShowBalance: Boolean;
+}

microsoft/knowledge/performance/hidden-flowfields-still-calculate-before-bc26-opt-in.good.al

@@ -0,0 +1,29 @@
+pageextension 50444 "Customer Balance Lazy" extends "Customer Card"
+{
+    layout
+    {
+        addlast(General)
+        {
+            field("Balance Preview"; BalancePreview)
+            {
+                ApplicationArea = All;
+                Caption = 'Balance Preview';
+                ToolTip = 'Specifies the balance when balance details are enabled.';
+                Visible = ShowBalance;
+            }
+        }
+    }
+
+    trigger OnAfterGetCurrRecord()
+    begin
+        Clear(BalancePreview);
+        if not ShowBalance then
+            exit;
+        Rec.CalcFields(Balance);
+        BalancePreview := Rec.Balance;
+    end;
+
+    var
+        BalancePreview: Decimal;
+        ShowBalance: Boolean;
+}

microsoft/knowledge/performance/hidden-flowfields-still-calculate-before-bc26-opt-in.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: performance
+keywords: [flowfield, visible, page-control, calculate-only-visible-flowfields, feature-management, hidden-field]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Hidden page FlowFields still calculate unless visible-only calculation is enabled
+
+## Description
+
+By default, a FlowField used directly as a page control's source is calculated when the page loads even when `Visible = false` or its visibility expression evaluates to false. The hidden control can therefore issue an aggregate query that no user sees. Business Central 26 introduced the **Calculate only visible FlowFields** feature-management option; only environments with that option enabled skip calculation for controls that are not visible.
+
+## Best Practice
+
+On BC 26 and later, enable and verify the visible-only FlowField feature before relying on `Visible` to suppress calculation. When the target environment does not guarantee that option, avoid binding an expensive FlowField directly to a usually-hidden control: calculate it only in the branch that displays it and bind the page control to a variable. Do not flag a hidden FlowField when the v26 feature is known to be enabled or the FlowField is cheap and intentionally preloaded.
+
+See sample: `hidden-flowfields-still-calculate-before-bc26-opt-in.good.al`.
+
+## Anti Pattern
+
+Adding a costly Sum or Lookup FlowField to a page with `Visible = SomeRareMode` and assuming the hidden state prevents its query on all supported versions. The review signal is the direct FlowField source plus conditional or false visibility, not visibility alone.
+
+See sample: `hidden-flowfields-still-calculate-before-bc26-opt-in.bad.al`.

microsoft/knowledge/performance/onaftergetcurrrecord-is-not-per-row.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: performance
+keywords: [onaftergetcurrrecord, onaftergetrecord, calcfields, n-plus-one, page-lifecycle, false-positive]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Database work in OnAfterGetCurrRecord is not a per-row or N+1 cost
+
+## Description
+
+`OnAfterGetCurrRecord` fires only when the current/active record changes — typically once when the page opens and once each time the user selects a different row — not once for every row rendered in a list. Database work placed there, such as `CalcFields`, `Get`, or a lookup, therefore runs a bounded number of times driven by user navigation, not multiplied by the number of visible rows. This is unlike `OnAfterGetRecord`, which fires once per row as the page loads records and can create a genuine N+1 pattern. Reviewers sometimes see `CalcFields` or a database call inside a page trigger and assume it runs for every row; the trigger name determines whether that assumption holds.
+
+## Best Practice
+
+Before flagging `CalcFields`, `Get`, or a similar database call in a page trigger as a per-row or N+1 problem, confirm the trigger is `OnAfterGetRecord`, which runs per row. Do not flag the same work in `OnAfterGetCurrRecord`: that trigger runs on current-record change, not for every displayed row.
+
+## Anti Pattern
+
+Reporting `CalcFields` or another database call inside `OnAfterGetCurrRecord` as an N+1 or per-row performance defect, or recommending it be moved out "to avoid running once per row". The trigger does not run per row.
+
+## See also
+
+- `calcfields-in-both-getrecord-triggers-is-not-redundant.md` — the lifecycle distinction between the two triggers.

microsoft/knowledge/performance/page-effective-filter-may-live-outside-the-diff.md

@@ -0,0 +1,22 @@
+---
+bc-version: [all]
+domain: performance
+keywords: [filter, drilldown, lookup, sourcetableview, tablerelation, setrange, false-positive]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# A page or lookup's effective filter may be defined outside the changed hunk
+
+## Description
+
+The effective filter on a drill-down, lookup, or list result set is frequently defined outside any single changed hunk — on the table via a `SourceTableView` property or a `TableRelation`, or through `SetRange`/`SetFilter` calls in unchanged code that runs before the result is shown. The absence of a filter within the changed lines of a diff is therefore not evidence that the result set is unfiltered or that it will load an entire table.
+
+## Best Practice
+
+Do not assert that a drill-down, lookup, or list is "unfiltered" based only on the changed hunk. Confirm the effective filter by checking the page's `SourceTableView`, the field's `TableRelation`, and any `SetRange`/`SetFilter` in the surrounding (possibly unchanged) code before raising a finding about an unbounded result set.
+
+## Anti Pattern
+
+Concluding that a lookup or drill-down loads an unfiltered, full-table result set solely because no `SetRange`/`SetFilter` appears in the changed lines, when the filter is defined on the table, in a `TableRelation`, or in unchanged setup code.

microsoft/knowledge/performance/primary-key-get-in-loop-is-transaction-cached.md

@@ -0,0 +1,24 @@
+---
+bc-version: [all]
+domain: performance
+keywords: [get, primary-key, record-cache, transaction, n-plus-one, dictionary-cache, over-engineering, false-positive]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# A primary-key Get() in a per-row helper is not an N+1 to cache manually
+
+## Description
+
+The Business Central server caches primary-key reads within a transaction. Repeated `Record.Get(<primary key>)` calls for the same key are served from that cache rather than re-queried, so a guarded `if not Rec.Get(...) then exit;` inside a per-row helper is not a genuine N+1 pattern. When each row legitimately carries a distinct key — for example one `Bin Content` row per bin, so `Bin.Get` and `BinType.Get` see a different bin each iteration — the `Get` must run per row regardless, and there is nothing to hoist.
+
+Reviewers sometimes see two `Get` calls inside a routine that runs once per row and recommend wrapping them in a `Dictionary` cache. That is over-engineering: it duplicates the server's built-in record cache, adds state that must be invalidated, and breaks the surrounding extension's established pattern of direct guarded `Get` calls.
+
+## Best Practice
+
+Treat a primary-key `Get()` — especially a guarded `if not Rec.Get(...) then exit;` — as a cheap, transaction-cached read. Do not recommend a manual `Dictionary` cache around per-row primary-key `Get` calls. Reserve N+1 concerns for genuinely repeated non-keyed queries (`FindSet`/`FindFirst` with filters, `Count`) that re-hit the database each iteration.
+
+## Anti Pattern
+
+Reporting repeated primary-key `Get` calls (such as `Bin.Get` and `BinType.Get`) inside a per-row helper as a performance defect, or recommending they be cached in a `Dictionary`. The reads are already cached by the server within the transaction, and per-row keys often differ so the calls cannot be hoisted.

microsoft/knowledge/performance/use-tryfunction-for-error-catching-not-rollback.md

@@ -11,11 +11,11 @@ application-area: [all]
 
 ## Description
 
-`[TryFunction]` annotates a method so that errors raised inside it can be caught by the caller instead of propagating. Per the platform reference, "changes to the database that are made with a try method aren't rolled back" — the attribute catches the error; it does not unwind database state. This is the critical distinction from `Codeunit.Run`, which does roll back on error (see `codeunit-run-as-atomic-sub-operation.md`). A try function also only catches when its return value is used: "If the return variable for a call to a function, which is attributed with [TryFunction] isn't used, then the call isn't considered a try function call." `DoTry();` propagates errors normally; only `ok := DoTry();` or `if DoTry() then ...` catches. The return type is forced to Boolean; user-defined return types are not allowed, and the value isn't accessible inside the try method itself. On Business Central on-premises, writes inside a try method are blocked by default and raise a runtime error unless `DisableWriteInsideTryFunctions` is set to `false` on the server — SaaS has no such restriction.
+`[TryFunction]` lets a caller catch an error, but database changes made before that error are not rolled back. The attribute catches; it does not unwind transaction state. This is the critical distinction from `Codeunit.Run`, which can provide an atomic rollback boundary (see `codeunit-run-as-atomic-sub-operation.md`). On Business Central on-premises, writes inside a try method are blocked by default unless `DisableWriteInsideTryFunctions` is set to `false`; SaaS does not provide that server setting.
 
 ## Best Practice
 
-Reach for `[TryFunction]` when you want to catch a failure without unwinding the transaction — HTTP calls whose non-2xx responses should surface a user-friendly message, .NET interop whose exceptions you want to translate, validation or parsing routines whose errors you intend to log and continue past. Always capture the return: `if MyTry() then ... else HandleFailure(GetLastErrorText());`. When the work is transactional — writes that must either fully apply or fully revert — use `Codeunit.Run` instead. The two primitives solve different problems: one catches errors, the other bounds a rollback scope.
+Reach for `[TryFunction]` when you want to catch a failure without unwinding the transaction — for example, third-party interop or parsing whose error you intend to translate. When writes must either fully apply or fully revert, use `Codeunit.Run` instead. The two primitives solve different problems: one catches errors, the other bounds a rollback scope.
 
 Use `[TryFunction]` sparingly. Each caught error writes to the session-wide `GetLastErrorText` and `GetLastErrorCallStack` buffers, and every subsequent catch overwrites the earlier state — a helper that reads `GetLastErrorText` later may see a different error than the one it intended to inspect. Prefer explicit checks (non-throwing predicates, guard conditions, upfront validation) for operations with predictable failure modes; reserve `[TryFunction]` for genuinely unpredictable failures such as network calls, third-party interop, or evaluation of user-supplied expressions. When you do catch, read `GetLastErrorText` immediately after the failed call, and call `ClearLastError` before the call if an earlier catch in the same scope could have left state behind — per the platform reference, "If you call the GetLastErrorText method immediately after you call the ClearLastError method, then an empty string is returned."
 
@@ -23,6 +23,10 @@ See sample: `use-tryfunction-for-error-catching-not-rollback.good.al`.
 
 ## Anti Pattern
 
-Wrapping database writes in `[TryFunction]` expecting the writes to roll back when the method errors. They do not: the writes that succeeded before the error remain, the caller receives `false`, and the corrupted-state bug surfaces in production. A related anti-pattern is calling a try function without capturing the return (`DoTry();`), which silently strips the error-catching behavior and lets the error propagate — the code looks defensive but behaves identically to an unwrapped call. A third is defensive sprinkling: wrapping every operation that *could* theoretically error in `[TryFunction]` on the theory that catching is always safer than propagating. Each extra catch pollutes the shared error buffer and makes the diagnostic signal harder to find when something real does fail.
+Wrapping database writes in `[TryFunction]` and expecting successful writes before the error to roll back. They remain, the caller receives `false`, and partially applied state can escape. Defensive sprinkling is also unsafe: every catch overwrites the session error buffer and can hide the failure a later helper intended to inspect.
 
 See sample: `use-tryfunction-for-error-catching-not-rollback.bad.al`.
+
+## See also
+
+`microsoft/knowledge/error-handling/ignored-tryfunction-return-disables-try-semantics.md` owns the separate call-site rule that a try method's Boolean result must be consumed.

microsoft/knowledge/privacy/avoid-strsubstno-prebuild-before-error.md

@@ -15,7 +15,7 @@ Error method trace telemetry includes the AL error string only when the first `E
 
 ## Best Practice
 
-Declare the complete message as a `Label` or `TextConst` and pass it directly to `Error`, followed by substitution values. The client receives the formatted message while telemetry can retain the static message template without using the dynamic values as its message. See `error-direct-substitution-safe-for-telemetry.md`.
+Declare the complete message as a `Label` or `TextConst` and pass it directly to `Error`, followed by substitution values. The client receives the formatted message while telemetry retains the static message template without using the dynamic values as its message. Independently review whether each substitution value is appropriate to show to the current user.
 
 See sample: `avoid-strsubstno-prebuild-before-error.good.al`.
 

microsoft/knowledge/privacy/error-direct-substitution-safe-for-telemetry.good.al

@@ -1,10 +0,0 @@
-codeunit 50205 "Privacy Sample Direct Error"
-{
-    procedure ValidateCustomer(var Customer: Record Customer)
-    var
-        InvalidEmailErr: Label 'Customer %1 has an invalid e-mail address: %2.', Comment = '%1 = Customer No., %2 = E-Mail';
-    begin
-        if not Customer."E-Mail".Contains('@') then
-            Error(InvalidEmailErr, Customer."No.", Customer."E-Mail");
-    end;
-}

microsoft/knowledge/privacy/error-direct-substitution-safe-for-telemetry.md

@@ -1,24 +0,0 @@
----
-bc-version: [20..]
-domain: privacy
-keywords: [error, strsubstno, direct-substitution, telemetry, classification, label]
-technologies: [al]
-countries: [w1]
-application-area: [all]
----
-
-# Use a Label or TextConst for the Error telemetry message
-
-## Description
-
-For Error method trace telemetry, the platform includes the AL error string only when `Error` receives a `Label` or `TextConst` as its first argument. Substitution values format the client message, but the static label supplies the telemetry message and preserves its classification context. A string literal, local `Text`, `StrSubstNo` result, or concatenation is not equivalent: telemetry substitutes generic guidance instead of that dynamic string.
-
-## Best Practice
-
-Define the complete error template as a `Label` with placeholder comments, pass the label directly as the first argument, and pass values separately. Independently review whether those values are appropriate to show to the current user.
-
-See sample: `error-direct-substitution-safe-for-telemetry.good.al`.
-
-## Anti Pattern
-
-Assuming that any direct format string is telemetry-safe, or that a `StrSubstNo`/concatenated first argument is logged verbatim. The required telemetry shape is specifically a directly supplied `Label` or `TextConst`; see `avoid-strsubstno-prebuild-before-error.md`.

microsoft/knowledge/privacy/no-pii-in-telemetry-message-string.bad.al

@@ -2,20 +2,20 @@ codeunit 50213 "Privacy Sample Telemetry Bad"
 {
     procedure LogCustomerProcessed(var Customer: Record Customer)
     begin
-        Session.LogMessage('0000', StrSubstNo('Processed %1', Customer.Name), Verbosity::Normal,
+        Session.LogMessage('PRIV0001', StrSubstNo('Processed %1', Customer.Name), Verbosity::Normal,
             DataClassification::SystemMetadata, TelemetryScope::All,
             'Category', 'Privacy');
     end;
 
     procedure LogFileError(FileName: Text)
     begin
-        Session.LogMessage('0001', StrSubstNo('Error processing file %1', FileName), Verbosity::Error,
+        Session.LogMessage('PRIV0002', StrSubstNo('Error processing file %1', FileName), Verbosity::Error,
             DataClassification::SystemMetadata, TelemetryScope::All);
     end;
 
     procedure LogEmployeeUpdate(EmployeeCode: Code[20])
     begin
-        Session.LogMessage('0002', StrSubstNo('Employee %1 updated record', EmployeeCode), Verbosity::Normal,
+        Session.LogMessage('PRIV0003', StrSubstNo('Employee %1 updated record', EmployeeCode), Verbosity::Normal,
             DataClassification::SystemMetadata, TelemetryScope::All);
     end;
 }

microsoft/knowledge/privacy/no-pii-in-telemetry-message-string.good.al

@@ -2,14 +2,14 @@ codeunit 50212 "Privacy Sample Telemetry Good"
 {
     procedure LogCustomerProcessed(var Customer: Record Customer)
     begin
-        Session.LogMessage('0000', 'Customer record processed', Verbosity::Normal,
+        Session.LogMessage('PRIV0001', 'Customer record processed', Verbosity::Normal,
             DataClassification::SystemMetadata, TelemetryScope::All,
             'Category', 'Privacy');
     end;
 
     procedure LogFileError()
     begin
-        Session.LogMessage('0001', 'Error processing uploaded file', Verbosity::Error,
+        Session.LogMessage('PRIV0002', 'Error processing uploaded file', Verbosity::Error,
             DataClassification::SystemMetadata, TelemetryScope::All);
     end;
 }

microsoft/knowledge/privacy/session-logmessage-requires-dataclassification.bad.al

@@ -2,6 +2,6 @@ codeunit 50211 "Privacy Sample LogMessage Bad"
 {
     procedure LogCompleted()
     begin
-        Session.LogMessage('0003', 'Operation completed', Verbosity::Normal);
+        Session.LogMessage('PRIV0004', 'Operation completed', Verbosity::Normal);
     end;
 }

microsoft/knowledge/privacy/session-logmessage-requires-dataclassification.good.al

@@ -2,7 +2,7 @@ codeunit 50210 "Privacy Sample LogMessage Good"
 {
     procedure LogCompleted()
     begin
-        Session.LogMessage('0003', 'Operation completed', Verbosity::Normal,
+        Session.LogMessage('PRIV0004', 'Operation completed', Verbosity::Normal,
             DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher);
     end;
 }

microsoft/knowledge/query/reopening-query-resets-cursor-but-keeps-filters.bad.al

@@ -0,0 +1,28 @@
+query 50426 "Query Reuse Bad"
+{
+    QueryType = Normal;
+
+    elements
+    {
+        dataitem(Customer; Customer)
+        {
+            column(CustomerNo; "No.") { }
+        }
+    }
+}
+
+codeunit 50427 "Query Reuse Bad"
+{
+    procedure ReadAgain(CustomerNoFilter: Code[20])
+    var
+        CustomerQuery: Query "Query Reuse Bad";
+    begin
+        CustomerQuery.SetRange(CustomerNo, CustomerNoFilter);
+        CustomerQuery.Open();
+        if CustomerQuery.Read() then;
+
+        // Reopening resets to the first row and retains CustomerNo.
+        CustomerQuery.Open();
+        if CustomerQuery.Read() then;
+    end;
+}

microsoft/knowledge/query/reopening-query-resets-cursor-but-keeps-filters.good.al

@@ -0,0 +1,39 @@
+query 50424 "Query Reuse Good"
+{
+    QueryType = Normal;
+
+    elements
+    {
+        dataitem(Customer; Customer)
+        {
+            column(CustomerNo; "No.") { }
+        }
+    }
+}
+
+codeunit 50425 "Query Reuse Good"
+{
+    procedure ReadTwoIndependentSets(FirstNo: Code[20]; SecondNo: Code[20])
+    var
+        CustomerQuery: Query "Query Reuse Good";
+    begin
+        CustomerQuery.SetRange(CustomerNo, FirstNo);
+        ReadAll(CustomerQuery);
+
+        Clear(CustomerQuery);
+        CustomerQuery.SetRange(CustomerNo, SecondNo);
+        ReadAll(CustomerQuery);
+    end;
+
+    local procedure ReadAll(var CustomerQuery: Query "Query Reuse Good")
+    begin
+        CustomerQuery.Open();
+        while CustomerQuery.Read() do
+            ProcessCustomer(CustomerQuery.CustomerNo);
+        CustomerQuery.Close();
+    end;
+
+    local procedure ProcessCustomer(CustomerNo: Code[20])
+    begin
+    end;
+}

microsoft/knowledge/query/reopening-query-resets-cursor-but-keeps-filters.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: query
+keywords: [query, open, close, clear, cursor, filters, reuse]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Reopening a Query resets its cursor but keeps its filters
+
+## Description
+
+Calling `Open()` on an already open query first closes the current dataset and opens it again. The next `Read()` starts at the first row; it does not continue from the previous cursor. Reopening also retains filters previously applied to the query variable. Only `Clear(QueryVariable)` resets those filters, so reuse can unexpectedly reread the first row or carry an old filter into a logically separate operation.
+
+## Best Practice
+
+Open once for one read pass. Close after the pass, and call `Clear(QueryVariable)` before reusing the variable for a logically independent query whose filters must start empty. Set the next pass's filters explicitly before reopening.
+
+See sample: `reopening-query-resets-cursor-but-keeps-filters.good.al`.
+
+## Anti Pattern
+
+Calling `Open()` inside or between reads to "advance" or "start fresh", or reusing the same query variable for a new operation while assuming `Open()` cleared old filters. The code compiles but can repeatedly process the first row or silently omit rows behind a retained filter.
+
+See sample: `reopening-query-resets-cursor-but-keeps-filters.bad.al`.

microsoft/knowledge/query/set-query-filters-before-open.bad.al

@@ -0,0 +1,30 @@
+query 50422 "Query Customer Sales Bad"
+{
+    QueryType = Normal;
+
+    elements
+    {
+        dataitem(Customer; Customer)
+        {
+            column(CustomerNo; "No.") { }
+            column(CustomerName; Name) { }
+        }
+    }
+}
+
+codeunit 50423 "Query Filter Order Bad"
+{
+    procedure ReadCustomer(CustomerNoFilter: Code[20])
+    var
+        CustomerSales: Query "Query Customer Sales Bad";
+    begin
+        CustomerSales.Open();
+        CustomerSales.SetRange(CustomerNo, CustomerNoFilter);
+        while CustomerSales.Read() do
+            ProcessCustomer(CustomerSales.CustomerNo);
+    end;
+
+    local procedure ProcessCustomer(CustomerNo: Code[20])
+    begin
+    end;
+}

microsoft/knowledge/query/set-query-filters-before-open.good.al

@@ -0,0 +1,31 @@
+query 50420 "Query Customer Sales Good"
+{
+    QueryType = Normal;
+
+    elements
+    {
+        dataitem(Customer; Customer)
+        {
+            column(CustomerNo; "No.") { }
+            column(CustomerName; Name) { }
+        }
+    }
+}
+
+codeunit 50421 "Query Filter Order Good"
+{
+    procedure ReadCustomer(CustomerNoFilter: Code[20])
+    var
+        CustomerSales: Query "Query Customer Sales Good";
+    begin
+        CustomerSales.SetRange(CustomerNo, CustomerNoFilter);
+        CustomerSales.Open();
+        while CustomerSales.Read() do
+            ProcessCustomer(CustomerSales.CustomerNo);
+        CustomerSales.Close();
+    end;
+
+    local procedure ProcessCustomer(CustomerNo: Code[20])
+    begin
+    end;
+}

microsoft/knowledge/query/set-query-filters-before-open.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: query
+keywords: [query, setfilter, setrange, open, read, dataset, filter-order]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Set Query filters before Open
+
+## Description
+
+`Query.SetFilter` and `Query.SetRange` automatically close an open query dataset. A call placed after `Open()` therefore does not refine the rows already being read; it ends that dataset. The next `Read()` has no open dataset unless the code explicitly calls `Open()` again, so a plausible filter change can turn a working loop into an empty or failing read sequence without a compiler diagnostic.
+
+## Best Practice
+
+Apply every filter before `Open()`, then read the dataset to completion and call `Close()`. When a later branch needs different filters, close or clear the query, set the new filters, and open a new dataset deliberately.
+
+See sample: `set-query-filters-before-open.good.al`.
+
+## Anti Pattern
+
+`Query.Open()` followed by `SetFilter` or `SetRange` and then `Read()` under the assumption that the filter updates the open cursor. Refiltering after `Open()` is valid only when the code intentionally opens a fresh dataset afterward.
+
+See sample: `set-query-filters-before-open.bad.al`.

microsoft/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; }
-    }
-}

microsoft/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; }
-    }
-}

microsoft/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`.

microsoft/knowledge/security/internal-access-is-not-a-security-boundary.bad.al

@@ -0,0 +1,15 @@
+codeunit 50471 "Unprotected Setup Action"
+{
+    Access = Internal;
+
+    trigger OnRun()
+    begin
+        // Internal does not prevent another extension from invoking this OnRun
+        // through Codeunit.Run.
+        UpdateSensitiveSetup();
+    end;
+
+    local procedure UpdateSensitiveSetup()
+    begin
+    end;
+}

microsoft/knowledge/security/internal-access-is-not-a-security-boundary.good.al

@@ -0,0 +1,39 @@
+table 50468 "Sensitive Setup"
+{
+    DataClassification = CustomerContent;
+
+    fields
+    {
+        field(1; "Primary Key"; Code[10]) { }
+    }
+}
+
+codeunit 50469 "Setup Authorization"
+{
+    procedure CanManageSetup(): Boolean
+    var
+        SensitiveSetup: Record "Sensitive Setup";
+    begin
+        exit(SensitiveSetup.WritePermission());
+    end;
+}
+
+codeunit 50470 "Protected Setup Action"
+{
+    Access = Internal;
+
+    trigger OnRun()
+    begin
+        if not SetupAuthorization.CanManageSetup() then
+            Error(NotAuthorizedErr);
+        UpdateSensitiveSetup();
+    end;
+
+    local procedure UpdateSensitiveSetup()
+    begin
+    end;
+
+    var
+        SetupAuthorization: Codeunit "Setup Authorization";
+        NotAuthorizedErr: Label 'You are not authorized to manage this setup.';
+}

microsoft/knowledge/security/internal-access-is-not-a-security-boundary.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: security
+keywords: [access, internal, internalsvisibleto, recordref, codeunit-run, security-boundary, authorization]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Access Internal is API hygiene, not an authorization boundary
+
+## Description
+
+`Access = Internal` controls compile-time symbol visibility. It does not prevent runtime access through mechanisms such as `RecordRef`, `TransferFields`, or `Codeunit.Run`, and `internalsVisibleTo` deliberately grants compile-time access to named companion apps. Microsoft explicitly documents that access modifiers cannot be used as a security boundary.
+
+## Best Practice
+
+Use `internal` to keep implementation details out of the supported API, but enforce sensitive operations with permissions, entitlements, and explicit authorization checks appropriate to the operation. Treat `internalsVisibleTo` as a same-publisher development/testability relationship, not as a trust grant for secrets or elevated data access.
+
+See sample: `internal-access-is-not-a-security-boundary.good.al`.
+
+## Anti Pattern
+
+Placing privileged work in an internal codeunit and claiming that other extensions cannot invoke it, or exposing an app to a different publisher through `internalsVisibleTo` because `internal` is assumed to protect the underlying operation. The access modifier narrows supported callers; it does not authenticate runtime callers.
+
+See sample: `internal-access-is-not-a-security-boundary.bad.al`.

microsoft/knowledge/security/secrets-isolated-storage.md

@@ -17,6 +17,10 @@ API keys, OAuth tokens, client secrets, and connection strings must not be store
 
 Persist every credential in `IsolatedStorage`, write it at the point of capture, and read it only when needed. Prefer `SetEncrypted` when the value fits its documented length limit. On BC24 and later, carry the value through the `SecretText` overloads; on earlier releases, keep any required `Text` handling inside a `[NonDebuggable]` boundary. Choose the `DataScope` that matches the credential's lifetime. See `isolatedstorage-datascope-module-vs-company`, `isolatedstorage-setencrypted-for-sensitive-values`, and `secrettext-for-credentials` for those separate concerns.
 
+See sample: `secrets-isolated-storage.good.al`.
+
 ## Anti Pattern
 
 A "Setup" or "Connection" table carrying a `Text` field named `API Key`, `Password`, or `Client Secret`. The value is now readable by any object with table permission, ships in RapidStart packages and Excel exports, and appears in record snapshots — a credential disclosure that no amount of encryption-in-transit elsewhere makes up for. Reviewer signal: a secret-shaped field declared on a table instead of an `IsolatedStorage` call.
+
+See sample: `secrets-isolated-storage.bad.al`.

microsoft/knowledge/security/secrettext-for-credentials.md

@@ -19,4 +19,4 @@ Declare credential-carrying parameters and variables as `SecretText` from the ca
 
 ## Anti Pattern
 
-Holding a credential in a `Text` variable (`BearerToken: Text`), concatenating it into a header, then passing it to `HttpClient`. The token is visible in the debugger and in any error that prints the variable, and the compiler offers no help because the type was wrong from the start. Reviewers should flag any local or parameter named like a secret (`ApiKey`, `Token`, `Password`, `ClientSecret`) whose type is `Text` or `Code`. See sample: `secrettext-for-credentials.bad.al`.
+Holding a credential in a `Text` variable (`BearerToken: Text`) makes it visible in the debugger and in any error that prints the variable, and the compiler offers no help because the type was wrong from the start. Reviewers should flag any local or parameter named like a secret (`ApiKey`, `Token`, `Password`, `ClientSecret`) whose type is `Text` or `Code`. When the same value is visibly sent through an HTTP URI, header, or body, `secrettext-with-httpclient.md` is the more specific primary rule. See sample: `secrettext-for-credentials.bad.al`.

microsoft/knowledge/security/secrettext-with-httpclient.md

@@ -19,4 +19,4 @@ Compose a secret URI with `SecretStrSubstNo`, call `Request.SetSecretRequestUri(
 
 ## Anti Pattern
 
-Holding a credential in `Text`, interpolating it with `StrSubstNo` or concatenation, and passing that plain text to `HttpClient.Get` or `HttpHeaders.Add`. The secret-aware request and header APIs remove the need to materialize the value as `Text`. See sample: `secrettext-with-httpclient.bad.al`.
+Holding a credential in `Text`, interpolating it with `StrSubstNo` or concatenation, and passing that plain text to `HttpClient.Get` or `HttpHeaders.Add`. The secret-aware request and header APIs remove the need to materialize the value as `Text`. This HTTP-sink rule supersedes the generic `secrettext-for-credentials.md` rule at the same location. See sample: `secrettext-with-httpclient.bad.al`.

microsoft/knowledge/style/caption-required-on-page-fields.bad.al

@@ -1,13 +1,24 @@
-table 50253 "Sample Caption Bad"
+page 50253 "Sample Caption Bad"
 {
-    fields
+    PageType = Card;
+    SourceTable = Customer;
+
+    layout
     {
-        field(1; "Customer No."; Code[20])
+        area(Content)
         {
-        }
-        field(2; "Is Active"; Boolean)
-        {
-            Caption = '';
+            group(General)
+            {
+                field("Customer No."; Rec."No.")
+                {
+                    ApplicationArea = All;
+                }
+                field("Customer Name"; Rec.Name)
+                {
+                    ApplicationArea = All;
+                    Caption = '';
+                }
+            }
         }
     }
 }

microsoft/knowledge/style/caption-required-on-page-fields.good.al

@@ -1,17 +1,28 @@
-table 50252 "Sample Caption Good"
+page 50252 "Sample Caption Good"
 {
-    fields
+    PageType = Card;
+    SourceTable = Customer;
+
+    layout
     {
-        field(1; "Customer No."; Code[20])
+        area(Content)
         {
-            Caption = 'Customer No.';
-        }
-        field(2; "Enabled"; Boolean)
-        {
-        }
-        field(3; Amount; Decimal)
-        {
-            CaptionClass = '3,5,' + 'USD';
+            group(General)
+            {
+                Caption = 'General';
+                field("Customer No."; Rec."No.")
+                {
+                    ApplicationArea = All;
+                    Caption = 'Customer No.';
+                    ToolTip = 'Specifies the customer number.';
+                }
+                field("Customer Name"; Rec.Name)
+                {
+                    ApplicationArea = All;
+                    Caption = 'Customer Name';
+                    ToolTip = 'Specifies the customer name.';
+                }
+            }
         }
     }
 }

microsoft/knowledge/style/temporary-variable-temp-prefix.md

@@ -15,12 +15,12 @@ A `Record` variable declared with the `temporary` modifier behaves nothing like
 
 ## Best Practice
 
-Every variable of type `Record X temporary` must start with `Temp`. The same applies to parameters: a procedure that receives a temporary record as a buffer names the parameter `TempBuffer`, `TempSalesLine`, and so on. The convention extends naturally to derived names — `TempJobWIPBufferCopy`, `TempSourceSalesLine` — anything that starts with `Temp` is in-memory.
+Every local or global variable of type `Record X temporary` must start with `Temp`. Ordinary procedure parameters follow the same convention. Event publisher parameters are owned by the events-domain rule `prefix-temporary-record-event-parameters-with-temp.md`; the style leaf must not emit a second finding for the same event parameter.
 
 See sample: `temporary-variable-temp-prefix.good.al`.
 
 ## Anti Pattern
 
-`WIPBuffer: Record "Job WIP Buffer" temporary;` reads at the call site as if it were a database operation: `WIPBuffer.Insert()` looks identical to a write to the underlying table. The reader has to scroll back to the declaration to discover that this is in-memory, every time.
+`WIPBuffer: Record "Job WIP Buffer" temporary;` as a local, global, or ordinary procedure parameter reads at the call site as if it were a database operation. Exclude event publisher parameters here so the events leaf remains their single owner.
 
 See sample: `temporary-variable-temp-prefix.bad.al`.

microsoft/knowledge/style/tooltip-required-on-page-fields.md

@@ -15,9 +15,11 @@ CodeCop AA0218 requires a non-empty `ToolTip` property on every field control on
 
 Acceptable exceptions: table fields inside `Upgrade`, `Migration`, `HybridBC14`, `HybridSL`, and `HybridGP` codeunits and tables are allowed to omit the tooltip — those types are not surfaced to users.
 
+AA0218 is a compiler analyzer, but its severity is configured per app in the ruleset and is frequently downgraded to `info`/`None` or disabled entirely. PR review therefore cannot assume the compiler will surface the gap: it is the last line of defence for a missing tooltip and should flag it independently. The one case review must *not* flag is a bound field that inherits a `ToolTip` from its source table field — see `bound-page-field-inherits-source-field-tooltip`.
+
 ## Best Practice
 
-Every field control on a regular page carries `ToolTip = 'Specifies …';` (or a clear alternative phrasing). Compose the text in the form "what this value shows" rather than "what the user does with it".
+Every field control on a regular page carries `ToolTip = 'Specifies …';` (or a clear alternative phrasing). Compose the text in the form "what this value shows" rather than "what the user does with it". In review, raise a `medium`-severity finding for a field that has neither an inline nor an inherited tooltip, independently of whether AA0218 is active in the app's ruleset.
 
 See sample: `tooltip-required-on-page-fields.good.al`.
 

microsoft/knowledge/telemetry/choose-telemetry-scope-by-audience.bad.al

@@ -0,0 +1,26 @@
+codeunit 50401 "Telemetry Scope Bad"
+{
+    procedure LogIntegrationFailure()
+    begin
+        // Tenant operators cannot see an actionable integration failure.
+        Session.LogMessage(
+            'TLM0004',
+            'Document exchange failed',
+            Verbosity::Error,
+            DataClassification::SystemMetadata,
+            TelemetryScope::ExtensionPublisher,
+            'Operation', 'DocumentExchange');
+    end;
+
+    procedure LogCacheMiss()
+    begin
+        // Environment telemetry receives publisher-only implementation noise.
+        Session.LogMessage(
+            'TLM0005',
+            'Internal cache entry missed',
+            Verbosity::Verbose,
+            DataClassification::SystemMetadata,
+            TelemetryScope::All,
+            'Cache', 'ExchangeMetadata');
+    end;
+}

microsoft/knowledge/telemetry/choose-telemetry-scope-by-audience.good.al

@@ -0,0 +1,24 @@
+codeunit 50400 "Telemetry Scope Good"
+{
+    procedure LogIntegrationFailure()
+    begin
+        Session.LogMessage(
+            'TLM0002',
+            'Document exchange failed',
+            Verbosity::Error,
+            DataClassification::SystemMetadata,
+            TelemetryScope::All,
+            'Operation', 'DocumentExchange');
+    end;
+
+    procedure LogCacheMiss()
+    begin
+        Session.LogMessage(
+            'TLM0003',
+            'Internal cache entry missed',
+            Verbosity::Verbose,
+            DataClassification::SystemMetadata,
+            TelemetryScope::ExtensionPublisher,
+            'Cache', 'ExchangeMetadata');
+    end;
+}

microsoft/knowledge/telemetry/choose-telemetry-scope-by-audience.md

@@ -0,0 +1,26 @@
+---
+bc-version: [17..]
+domain: telemetry
+keywords: [telemetryscope, extensionpublisher, all, audience, logmessage, application-insights]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Choose TelemetryScope by who must receive the signal
+
+## Description
+
+`TelemetryScope::ExtensionPublisher` sends a custom trace only to the Application Insights resource configured by the extension publisher. `TelemetryScope::All` also sends it to the environment's telemetry, where the customer or partner operating the tenant can query it. The compiler accepts either value, so a plausible-looking scope can silently hide an actionable signal from tenant operators or expose publisher-only implementation noise to them.
+
+## Best Practice
+
+Use `ExtensionPublisher` for internal diagnostics that only the app publisher can interpret, such as cache behavior or private algorithm state. Use `All` for signals the tenant operator can act on, such as an integration failure, quota warning, or setup problem. Decide the audience independently from `DataClassification`; privacy guidance still governs whether the payload may be emitted at all.
+
+See sample: `choose-telemetry-scope-by-audience.good.al`.
+
+## Anti Pattern
+
+Defaulting every call to `All`, including low-level implementation diagnostics, or defaulting every call to `ExtensionPublisher` and thereby hiding customer-actionable failures from environment telemetry. Review only when the message and surrounding branch make the intended audience clear; an ambiguous diagnostic is not enough to infer the wrong scope.
+
+See sample: `choose-telemetry-scope-by-audience.bad.al`.

microsoft/knowledge/telemetry/feature-uptake-transitions-in-order.bad.al

@@ -0,0 +1,11 @@
+codeunit 50405 "Feature Uptake Bad"
+{
+    procedure FeatureOpened()
+    var
+        FeatureTelemetry: Codeunit "Feature Telemetry";
+    begin
+        // The first uptake state skips Discovered and is not emitted.
+        FeatureTelemetry.LogUptake(
+            'TLM0011', 'Document exchange', Enum::"Feature Uptake Status"::Used);
+    end;
+}

microsoft/knowledge/telemetry/feature-uptake-transitions-in-order.good.al

@@ -0,0 +1,26 @@
+codeunit 50404 "Feature Uptake Good"
+{
+    procedure FeatureDiscovered()
+    var
+        FeatureTelemetry: Codeunit "Feature Telemetry";
+    begin
+        FeatureTelemetry.LogUptake(
+            'TLM0008', 'Document exchange', Enum::"Feature Uptake Status"::Discovered);
+    end;
+
+    procedure FeatureSetUp()
+    var
+        FeatureTelemetry: Codeunit "Feature Telemetry";
+    begin
+        FeatureTelemetry.LogUptake(
+            'TLM0009', 'Document exchange', Enum::"Feature Uptake Status"::"Set up");
+    end;
+
+    procedure FeatureUsed()
+    var
+        FeatureTelemetry: Codeunit "Feature Telemetry";
+    begin
+        FeatureTelemetry.LogUptake(
+            'TLM0010', 'Document exchange', Enum::"Feature Uptake Status"::Used);
+    end;
+}

microsoft/knowledge/telemetry/feature-uptake-transitions-in-order.md

@@ -0,0 +1,26 @@
+---
+bc-version: [18..]
+domain: telemetry
+keywords: [featuretelemetry, loguptake, discovered, set-up, used, uptake-status, lifecycle]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Emit FeatureTelemetry uptake states in lifecycle order
+
+## Description
+
+`FeatureTelemetry.LogUptake` accepts `Discovered`, `Set up`, `Used`, and `Undiscovered`, but the platform records the forward transition only as `Discovered` to `Set up` to `Used`. If the first call for a feature is `Set up` or `Used`, no uptake telemetry is emitted. `Undiscovered` is the explicit reset from any state.
+
+## Best Practice
+
+Log `Discovered` when the user encounters the feature, `Set up` after its setup is completed, and `Used` when the user attempts it. Keep the same feature name throughout the funnel. Review ordering only when the changed repository context shows the feature's lifecycle; a single isolated `Used` call cannot prove that earlier states are absent elsewhere.
+
+See sample: `feature-uptake-transitions-in-order.good.al`.
+
+## Anti Pattern
+
+Introducing a feature whose only uptake call jumps directly to `Set up` or `Used`, or using different feature-name literals for successive states. The calls compile and run, but the funnel silently omits the invalid transition.
+
+See sample: `feature-uptake-transitions-in-order.bad.al`.

microsoft/knowledge/telemetry/feature-usage-only-after-success.bad.al

@@ -0,0 +1,23 @@
+codeunit 50407 "Feature Usage Bad"
+{
+    procedure ExchangeDocument(ShouldFail: Boolean)
+    var
+        FeatureTelemetry: Codeunit "Feature Telemetry";
+    begin
+        FeatureTelemetry.LogUsage(
+            'TLM0014', 'Document exchange', 'Document exchanged');
+
+        if not TryExchangeDocument(ShouldFail) then
+            exit;
+    end;
+
+    [TryFunction]
+    local procedure TryExchangeDocument(ShouldFail: Boolean)
+    begin
+        if ShouldFail then
+            Error(ExchangeFailedErr);
+    end;
+
+    var
+        ExchangeFailedErr: Label 'Exchange failed.';
+}

microsoft/knowledge/telemetry/feature-usage-only-after-success.good.al

@@ -0,0 +1,27 @@
+codeunit 50406 "Feature Usage Good"
+{
+    procedure ExchangeDocument(ShouldFail: Boolean)
+    var
+        FeatureTelemetry: Codeunit "Feature Telemetry";
+    begin
+        if not TryExchangeDocument(ShouldFail) then begin
+            FeatureTelemetry.LogError(
+                'TLM0012', 'Document exchange', 'Exchanging document',
+                GetLastErrorText(true), GetLastErrorCallStack());
+            exit;
+        end;
+
+        FeatureTelemetry.LogUsage(
+            'TLM0013', 'Document exchange', 'Document exchanged');
+    end;
+
+    [TryFunction]
+    local procedure TryExchangeDocument(ShouldFail: Boolean)
+    begin
+        if ShouldFail then
+            Error(ExchangeFailedErr);
+    end;
+
+    var
+        ExchangeFailedErr: Label 'Exchange failed.';
+}

microsoft/knowledge/telemetry/feature-usage-only-after-success.md

@@ -0,0 +1,26 @@
+---
+bc-version: [18..]
+domain: telemetry
+keywords: [featuretelemetry, logusage, logerror, success, tryfunction, feature-usage]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Call FeatureTelemetry.LogUsage only after successful use
+
+## Description
+
+`FeatureTelemetry.LogUsage` means that a user successfully used the feature. An attempt belongs in the uptake funnel, while a failed operation belongs in `LogError`. Logging usage before checking the result inflates adoption metrics with failed attempts and makes usage telemetry disagree with the actual business outcome.
+
+## Best Practice
+
+Call `LogUsage` only after the operation has completed successfully. On a failure path, call `LogError` with the captured error text and call stack when the failure must be emitted explicitly. Use a past-tense event name for usage and a present-tense scenario name for errors.
+
+See sample: `feature-usage-only-after-success.good.al`.
+
+## Anti Pattern
+
+Calling `LogUsage` before a Boolean result, `TryFunction`, `Codeunit.Run`, or HTTP status has been checked, or calling it in both success and failure branches. Do not flag an attempt recorded with `LogUptake(...Used)`; unlike `LogUsage`, that state intentionally records an attempt.
+
+See sample: `feature-usage-only-after-success.bad.al`.

microsoft/knowledge/telemetry/keep-custom-dimension-schema-stable.bad.al

@@ -0,0 +1,14 @@
+codeunit 50412 "Telemetry Dimension Bad"
+{
+    procedure LogBatchResult(RecordCount: Integer)
+    var
+        CustomDimensions: Dictionary of [Text, Text];
+    begin
+        CustomDimensions.Add('record count', Format(RecordCount));
+        CustomDimensions.Add('result_code', 'Success');
+        Session.LogMessage(
+            'TLM0015', 'Order processing completed', Verbosity::Normal,
+            DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher,
+            CustomDimensions);
+    end;
+}

microsoft/knowledge/telemetry/keep-custom-dimension-schema-stable.good.al

@@ -0,0 +1,14 @@
+codeunit 50411 "Telemetry Dimension Good"
+{
+    procedure LogBatchResult(RecordCount: Integer)
+    var
+        CustomDimensions: Dictionary of [Text, Text];
+    begin
+        CustomDimensions.Add('RecordCount', Format(RecordCount));
+        CustomDimensions.Add('Result', 'Success');
+        Session.LogMessage(
+            'TLM0015', 'Order processing completed', Verbosity::Normal,
+            DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher,
+            CustomDimensions);
+    end;
+}

microsoft/knowledge/telemetry/keep-custom-dimension-schema-stable.md

@@ -0,0 +1,26 @@
+---
+bc-version: [17..]
+domain: telemetry
+keywords: [customdimensions, dimension-key, schema, pascalcase, kql, breaking-change]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Treat custom dimension keys as a stable telemetry schema
+
+## Description
+
+Business Central prefixes AL custom-dimension keys with `al` in Application Insights, so an AL key named `Result` becomes `alResult`. Microsoft guidance treats telemetry definitions as an API: changing or removing a custom dimension can break dashboards and alerts. PascalCase keys without spaces also compose cleanly in KQL; spaces force awkward bracket access and make queries harder to maintain.
+
+## Best Practice
+
+Choose stable PascalCase keys such as `Operation`, `Result`, and `RecordCount`. Keep the key set and meaning stable for a shipped event ID; add a new event ID or coordinate a schema migration when the meaning must change. Privacy guidance separately governs whether a dimension value may contain customer data.
+
+See sample: `keep-custom-dimension-schema-stable.good.al`.
+
+## Anti Pattern
+
+Keys such as `'order no'` or `'result_code'`, or renaming/removing a key while retaining the same shipped event ID. A naming-only issue is advisory; changing an existing event's schema is the material compatibility defect. New keys on a new event ID are not a breaking change.
+
+See sample: `keep-custom-dimension-schema-stable.bad.al`.

microsoft/knowledge/telemetry/match-verbosity-to-signal-severity.bad.al

@@ -0,0 +1,24 @@
+codeunit 50403 "Telemetry Verbosity Bad"
+{
+    procedure RunExchange()
+    begin
+        if TryExchange() then
+            exit;
+
+        Session.LogMessage(
+            'TLM0007',
+            'Document exchange failed',
+            Verbosity::Normal,
+            DataClassification::SystemMetadata,
+            TelemetryScope::All);
+    end;
+
+    [TryFunction]
+    local procedure TryExchange()
+    begin
+        Error(ExchangeFailedErr);
+    end;
+
+    var
+        ExchangeFailedErr: Label 'Exchange failed.';
+}

microsoft/knowledge/telemetry/match-verbosity-to-signal-severity.good.al

@@ -0,0 +1,24 @@
+codeunit 50402 "Telemetry Verbosity Good"
+{
+    procedure RunExchange()
+    begin
+        if TryExchange() then
+            exit;
+
+        Session.LogMessage(
+            'TLM0006',
+            'Document exchange failed',
+            Verbosity::Error,
+            DataClassification::SystemMetadata,
+            TelemetryScope::All);
+    end;
+
+    [TryFunction]
+    local procedure TryExchange()
+    begin
+        Error(ExchangeFailedErr);
+    end;
+
+    var
+        ExchangeFailedErr: Label 'Exchange failed.';
+}

microsoft/knowledge/telemetry/match-verbosity-to-signal-severity.md

@@ -0,0 +1,26 @@
+---
+bc-version: [17..]
+domain: telemetry
+keywords: [verbosity, severitylevel, critical, error, warning, normal, verbose, logmessage]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Match telemetry Verbosity to the signal's actual severity
+
+## Description
+
+`Verbosity` becomes the Application Insights `severityLevel` and participates in on-premises diagnostic trace filtering. `Critical` represents abnormal termination, `Error` a severe error, `Warning` a warning, `Normal` a non-error event, and `Verbose` detailed tracing. Logging a caught failure as `Normal` is not cosmetic: severity-based alerts miss it, and an on-premises service configured to emit only warnings and above can drop it completely.
+
+## Best Practice
+
+Use `Error` for failed operations that need investigation and `Critical` only for abnormal termination or equivalent loss of service. Use `Warning` for degraded but completed behavior, `Normal` for successful business events, and `Verbose` for detailed diagnostics. Judge the outcome, not the procedure name: an expected optional lookup miss can legitimately remain `Normal` or `Verbose`.
+
+See sample: `match-verbosity-to-signal-severity.good.al`.
+
+## Anti Pattern
+
+A `Session.LogMessage` in a failed `TryFunction`, failed `Codeunit.Run`, unsuccessful HTTP response, or other explicit failure branch that uses `Verbosity::Normal` or `Verbose` without evidence that the failure is expected and benign.
+
+See sample: `match-verbosity-to-signal-severity.bad.al`.

microsoft/knowledge/telemetry/register-one-telemetry-logger-per-publisher.bad.al

@@ -0,0 +1,37 @@
+codeunit 50409 "First Telemetry Logger" implements "Telemetry Logger"
+{
+    Access = Internal;
+
+    procedure LogMessage(EventId: Text; Message: Text; Verbosity: Verbosity; DataClassification: DataClassification; TelemetryScope: TelemetryScope; CustomDimensions: Dictionary of [Text, Text])
+    begin
+        Session.LogMessage(
+            EventId, Message, Verbosity, DataClassification, TelemetryScope, CustomDimensions);
+    end;
+
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::"Telemetry Loggers", 'OnRegisterTelemetryLogger', '', true, true)]
+    local procedure RegisterFirst(var Sender: Codeunit "Telemetry Loggers")
+    var
+        Logger: Codeunit "First Telemetry Logger";
+    begin
+        Sender.Register(Logger);
+    end;
+}
+
+codeunit 50410 "Second Telemetry Logger" implements "Telemetry Logger"
+{
+    Access = Internal;
+
+    procedure LogMessage(EventId: Text; Message: Text; Verbosity: Verbosity; DataClassification: DataClassification; TelemetryScope: TelemetryScope; CustomDimensions: Dictionary of [Text, Text])
+    begin
+        Session.LogMessage(
+            EventId, Message, Verbosity, DataClassification, TelemetryScope, CustomDimensions);
+    end;
+
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::"Telemetry Loggers", 'OnRegisterTelemetryLogger', '', true, true)]
+    local procedure RegisterSecond(var Sender: Codeunit "Telemetry Loggers")
+    var
+        Logger: Codeunit "Second Telemetry Logger";
+    begin
+        Sender.Register(Logger);
+    end;
+}

microsoft/knowledge/telemetry/register-one-telemetry-logger-per-publisher.good.al

@@ -0,0 +1,18 @@
+codeunit 50408 "Sample Telemetry Logger" implements "Telemetry Logger"
+{
+    Access = Internal;
+
+    procedure LogMessage(EventId: Text; Message: Text; Verbosity: Verbosity; DataClassification: DataClassification; TelemetryScope: TelemetryScope; CustomDimensions: Dictionary of [Text, Text])
+    begin
+        Session.LogMessage(
+            EventId, Message, Verbosity, DataClassification, TelemetryScope, CustomDimensions);
+    end;
+
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::"Telemetry Loggers", 'OnRegisterTelemetryLogger', '', true, true)]
+    local procedure OnRegisterTelemetryLogger(var Sender: Codeunit "Telemetry Loggers")
+    var
+        SampleTelemetryLogger: Codeunit "Sample Telemetry Logger";
+    begin
+        Sender.Register(SampleTelemetryLogger);
+    end;
+}

microsoft/knowledge/telemetry/register-one-telemetry-logger-per-publisher.md

@@ -0,0 +1,26 @@
+---
+bc-version: [18..]
+domain: telemetry
+keywords: [telemetry-logger, interface, register, publisher, featuretelemetry, onregistertelemetrylogger]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Register exactly one Telemetry Logger implementation per publisher
+
+## Description
+
+The `Telemetry` and `Feature Telemetry` codeunits reach an extension publisher's telemetry through an implementation of the `"Telemetry Logger"` interface registered with `"Telemetry Loggers".OnRegisterTelemetryLogger`. The platform requires exactly one registration per app publisher. No registration prevents the module from working as expected; multiple registrations make the destination ambiguous and produce platform error telemetry.
+
+## Best Practice
+
+Place one internal logger implementation in one app for the publisher, forward its `LogMessage` method to `Session.LogMessage`, and register it from one event subscriber. Companion apps with the same publisher reuse that registration instead of each adding another. Evaluate absence only with repository or app-family context; a single-file diff cannot prove that no logger exists elsewhere.
+
+See sample: `register-one-telemetry-logger-per-publisher.good.al`.
+
+## Anti Pattern
+
+Adding `FeatureTelemetry` calls to a complete app with no logger registration, or registering two logger implementations for apps that share the same publisher. The calls compile, but the telemetry module reports the missing or duplicate registration instead of behaving as intended.
+
+See sample: `register-one-telemetry-logger-per-publisher.bad.al`.

microsoft/knowledge/telemetry/telemetry-event-id-stable-unique.bad.al

@@ -1,4 +1,4 @@
-codeunit 50260 "Sample Telemetry Id Bad"
+codeunit 50260 "Telemetry Event Id Bad"
 {
     procedure LogCustomerProcessed(var Customer: Record Customer)
     begin

microsoft/knowledge/telemetry/telemetry-event-id-stable-unique.good.al

@@ -1,4 +1,4 @@
-codeunit 50261 "Sample Telemetry Id Good"
+codeunit 50261 "Telemetry Event Id Good"
 {
     procedure LogCustomerProcessed(var Customer: Record Customer)
     begin

microsoft/knowledge/telemetry/telemetry-event-id-stable-unique.md

@@ -1,6 +1,6 @@
 ---
-bc-version: [all]
-domain: style
+bc-version: [17..]
+domain: telemetry
 keywords: [telemetry, logmessage, event-id, sessionlogmessage, observability]
 technologies: [al]
 countries: [w1]

microsoft/knowledge/testing/permission-tests-must-lower-the-execution-context.bad.al

@@ -0,0 +1,26 @@
+codeunit 50483 "Protected Setup Action Bad"
+{
+    trigger OnRun()
+    var
+        Customer: Record Customer;
+    begin
+        Customer.Init();
+        Customer."No." := 'SUPER-INSERT';
+        Customer.Insert();
+    end;
+}
+
+codeunit 50484 "Permission Test Bad"
+{
+    Subtype = Test;
+    TestPermissions = Disabled;
+
+    [Test]
+    procedure LimitedUserCannotRunSetup()
+    var
+        SetupAction: Codeunit "Protected Setup Action Bad";
+    begin
+        // Disabled runs as SUPER; no limited-user boundary is exercised.
+        asserterror SetupAction.Run();
+    end;
+}

microsoft/knowledge/testing/permission-tests-must-lower-the-execution-context.good.al

@@ -0,0 +1,43 @@
+permissionset 50480 "LIMITED USER"
+{
+    Assignable = false;
+    Permissions =
+        tabledata Customer = R,
+        codeunit "Protected Setup Action Test" = X;
+}
+
+codeunit 50481 "Protected Setup Action Test"
+{
+    trigger OnRun()
+    var
+        Customer: Record Customer;
+    begin
+        Customer.Init();
+        Customer."No." := 'NO-INSERT';
+        Customer.Insert();
+    end;
+}
+
+codeunit 50482 "Permission Test Good"
+{
+    Subtype = Test;
+    TestPermissions = Restrictive;
+
+    [Test]
+    procedure LimitedUserCannotRunSetup()
+    var
+        PermissionsMock: Codeunit "Permissions Mock";
+        SetupAction: Codeunit "Protected Setup Action Test";
+    begin
+        PermissionsMock.Start();
+        PermissionsMock.SetExactPermissionSet('LIMITED USER');
+
+        asserterror SetupAction.Run();
+        Assert.ExpectedError('permission');
+
+        PermissionsMock.Stop();
+    end;
+
+    var
+        Assert: Codeunit "Library Assert";
+}

microsoft/knowledge/testing/permission-tests-must-lower-the-execution-context.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: testing
+keywords: [testpermissions, restrictive, disabled, permissions-mock, lower-permissions, super, permission-test]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Permission tests must actually lower the execution context
+
+## Description
+
+`TestPermissions` describes how a test runner should establish the permission context; the enum value does not itself assign the business permission set being tested. `Restrictive` is the default and starts from D365 Full Access, requiring the test to lower permissions. `Disabled` leaves the test running as `SUPER`. A test that expects access to be denied while still running with either broad context can pass or fail for the wrong reason and never exercise the intended boundary.
+
+## Best Practice
+
+Use `TestPermissions::Restrictive` for a permission-sensitive test and lower the current test user with the test framework's `"Permissions Mock"` or `"Library - Lower Permissions"` before invoking the protected operation. Assign the exact permission set the scenario claims to test and restore or stop the mock afterward. Use `Disabled` only for suites that do not assert permission behavior.
+
+See sample: `permission-tests-must-lower-the-execution-context.good.al`.
+
+## Anti Pattern
+
+Setting `TestPermissions = Disabled` or leaving the effective D365 Full Access context in place while asserting that a limited user is denied, or adding a `[TestPermissions(...)]` attribute without any runner/test-library code that applies the intended permission set.
+
+See sample: `permission-tests-must-lower-the-execution-context.bad.al`.

microsoft/knowledge/testing/testisolation-belongs-on-the-test-runner.bad.al

@@ -0,0 +1,22 @@
+codeunit 50452 "Isolated Test Runner Bad"
+{
+    Subtype = TestRunner;
+    TestIsolation = Disabled;
+}
+
+codeunit 50453 "Committed Write Test Bad"
+{
+    Subtype = Test;
+
+    [Test]
+    [TransactionModel(TransactionModel::AutoCommit)]
+    procedure TestCommittedWrite()
+    var
+        Customer: Record Customer;
+    begin
+        Customer.Init();
+        Customer."No." := 'PERSISTS';
+        Customer.Insert();
+        Commit();
+    end;
+}

microsoft/knowledge/testing/testisolation-belongs-on-the-test-runner.good.al

@@ -0,0 +1,22 @@
+codeunit 50450 "Isolated Test Runner Good"
+{
+    Subtype = TestRunner;
+    TestIsolation = Codeunit;
+}
+
+codeunit 50451 "Committed Write Test Good"
+{
+    Subtype = Test;
+
+    [Test]
+    [TransactionModel(TransactionModel::AutoCommit)]
+    procedure TestCommittedWrite()
+    var
+        Customer: Record Customer;
+    begin
+        Customer.Init();
+        Customer."No." := 'ISOLATED';
+        Customer.Insert();
+        Commit();
+    end;
+}

microsoft/knowledge/testing/testisolation-belongs-on-the-test-runner.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: testing
+keywords: [testisolation, testrunner, autocommit, commit, rollback, test-order, database-state]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Configure TestIsolation on the test runner
+
+## Description
+
+`TestIsolation` is a property of a `Subtype = TestRunner` codeunit, not of the test codeunit being executed. Its default is `Disabled`. `Codeunit` rolls back database changes after each test codeunit and `Function` after each test method, including changes that the code under test explicitly committed. Without runner isolation, an `AutoCommit` test can leave data behind and make later tests order-dependent.
+
+## Best Practice
+
+Run independent suites with `TestIsolation = Codeunit` or `Function`, choosing the narrowest boundary the runner supports. Pair this with the appropriate method-level `TransactionModel`: `AutoCommit` permits code under test to commit, while runner isolation still restores the database afterward. Keep isolation disabled only for an intentionally shared-state suite whose ordering and cleanup are explicit.
+
+See sample: `testisolation-belongs-on-the-test-runner.good.al`.
+
+## Anti Pattern
+
+An `AutoCommit` test exercises committed writes under a test runner that omits `TestIsolation` or sets it to `Disabled`, then assumes the database is restored automatically. This article owns runner-level rollback; `transactionmodel-attribute-governs-test-transactions.md` separately owns the method attribute.
+
+See sample: `testisolation-belongs-on-the-test-runner.bad.al`.

microsoft/knowledge/ui/bound-page-field-inherits-source-field-tooltip.md

@@ -0,0 +1,24 @@
+---
+bc-version: [all]
+domain: ui
+keywords: [tooltip, page-field, source-field, inheritance, aa0218, false-positive]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# A page field bound to a table field inherits that field's ToolTip
+
+## Description
+
+A page field bound to a table field inherits the source field's `ToolTip` at runtime: the control shows the table field's `ToolTip` even when the page control declares none of its own. A page field without an inline `ToolTip` is therefore not, by itself, a missing-tooltip defect — the text may be supplied by the bound source field.
+
+The genuinely-missing case is different: a bound field whose source table field *also* carries no `ToolTip`, or an unbound control, has no text to inherit and is a real accessibility gap. The compiler analyzer AA0218 detects this mechanically, but its severity is set by each app's ruleset and is routinely downgraded or disabled — so it cannot be relied on as the only net. PR review is the last line of defence and should raise this case independently.
+
+## Best Practice
+
+Do not raise a missing-`ToolTip` finding for a bound page field whose source table field supplies a `ToolTip`; assume the control inherits it. Do raise a `medium`-severity finding when the field has no inline `ToolTip` **and** no inherited one — that is, a bound field whose source field is also tooltip-less, or an unbound control — rather than assuming AA0218 will catch it downstream.
+
+## Anti Pattern
+
+Two opposite failures: (1) flagging every page field that has no inline `ToolTip` as a violation, ignoring that a bound field inherits its source field's tooltip; and (2) staying silent on a field that has neither an inline nor an inherited tooltip on the assumption that the compiler's AA0218 will report it — a ruleset that downgrades or disables AA0218 then lets a genuine gap ship unflagged.

microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.good.al

@@ -0,0 +1,38 @@
+page 50210 "UI Sample Caption Case"
+{
+    PageType = List;
+    ApplicationArea = All;
+    SourceTable = "Sales Line";
+
+    layout
+    {
+        area(Content)
+        {
+            repeater(Lines)
+            {
+                field("Document No."; Rec."Document No.")
+                {
+                    ToolTip = 'Specifies the document number.';
+                }
+            }
+        }
+    }
+
+    actions
+    {
+        area(Processing)
+        {
+            action(ShowSourceDocument)
+            {
+                Caption = 'Show source document';
+                Image = ViewSourceDocumentLine;
+                ToolTip = 'Open the related source document.';
+
+                trigger OnAction()
+                begin
+                    Message('%1', Rec."Document No.");
+                end;
+            }
+        }
+    }
+}

microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: ui
+keywords: [caption, capitalization, sentence-case, title-case, action, noun-phrase, false-positive]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Sentence-phrase captions use sentence case, not title case
+
+## Description
+
+Business Central caption capitalization depends on whether the caption reads as a **noun phrase** or a **sentence/verb phrase**. Following the Microsoft writing-style guideline, a caption that reads as an imperative sentence — most action captions, such as `'Show source document'`, `'Post and print'`, or `'Copy from last inspection'` — uses **sentence case**: only the first word and any proper nouns are capitalized. Title case (`'Show Source Document'`) is the older convention and is not required for these captions.
+
+Noun-phrase captions (object names, field labels such as `'Source Document No.'`) follow their own capitalization; that is a separate case and is not what this article covers. Reviewers sometimes see a lower-cased word in an action caption (`'Show source document'`) and flag it as inconsistent title case, but a sentence-phrase action caption is correct as written.
+
+## Best Practice
+
+For an action `Caption` that reads as a sentence or verb phrase, capitalize only the first word and proper nouns (sentence case). Do not require every significant word to be capitalized. Before flagging a caption as "should be title case", confirm it is a noun phrase; leave imperative/sentence-phrase action captions in sentence case.
+
+See sample: `caption-capitalization-noun-phrase-vs-sentence-phrase.good.al`.
+
+## Anti Pattern
+
+Reporting a sentence-case action caption such as `'Show source document'` as a style defect and recommending title case (`'Show Source Document'`), or calling it inconsistent with BC conventions. Sentence case is the current guideline for sentence-phrase captions.

microsoft/knowledge/ui/page-background-tasks.md

@@ -1,5 +1,5 @@
 ---
-bc-version: [all]
+bc-version: [15..]
 domain: ui
 keywords: [enqueuebackgroundtask, async-calculation, child-session, factbox, cue-tile, onaftergetcurrrecord, responsive-page, read-only]
 technologies: [al]

microsoft/knowledge/upgrade/do-not-block-upgrade-on-data-errors.good.al

@@ -13,7 +13,7 @@ codeunit 50206 "Upgrade Graceful"
     begin
         if not Customer.Get(CustomerNo) then begin
             Session.LogMessage(
-                '0000ABC',
+                'UPG0001',
                 'Customer not found during upgrade',
                 Verbosity::Warning,
                 DataClassification::SystemMetadata,