Sync knowledge articles with review agent instructions

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

@@ -11,16 +11,16 @@ application-area: [all]
 
 ## Description
 
-Every field on every AL table and table extension must carry an explicit `DataClassification` property. 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 `DataClassification` defaults to `ToBeClassified`, which is a compliance gap, not a neutral state.
+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. When uncertain between two values, pick the stronger protection.
+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, or omitting the property entirely (which resolves to the same default). Code in this state fails compliance audits and breaks the subject-access-request and retention tooling that depends on the property being set correctly.
+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/performance/keep-event-subscribers-lightweight.good.al

@@ -0,0 +1,20 @@
+codeunit 50930 "Perf Sample Subscriber Good"
+{
+    [EventSubscriber(ObjectType::Table, Database::"Sales Line", 'OnAfterValidateEvent', 'No.', false, false)]
+    local procedure OnAfterValidateSalesLineNo(var Rec: Record "Sales Line")
+    var
+        Item: Record Item;
+    begin
+        if Rec.Type <> Rec.Type::Item then
+            exit;
+
+        Item.SetLoadFields("Costing Method");
+        if Item.Get(Rec."No.") then
+            if Item."Costing Method" = Item."Costing Method"::Specific then
+                UpdateSpecificCostingState(Rec);
+    end;
+
+    local procedure UpdateSpecificCostingState(var SalesLine: Record "Sales Line")
+    begin
+    end;
+}

microsoft/knowledge/performance/keep-event-subscribers-lightweight.md

@@ -17,7 +17,9 @@ Event subscribers run synchronously on the publisher's thread. If a subscriber d
 
 ## Best Practice
 
-Keep subscribers small: guard early with inexpensive checks, defer heavy work to a task queue or a background session, and cache results across invocations when the data is stable.
+Keep subscribers small: guard early with inexpensive checks on the publisher record before doing any database work, defer heavy work to a task queue or a background session, and cache results across invocations when the data is stable. In hot events, a cheap `Type`/`Status`/`IsTemporary` exit before a `Get` or `FindFirst` is often the difference between a rare lookup and an N+1 query across every posted line.
+
+See sample: `keep-event-subscribers-lightweight.good.al`.
 
 ## Anti Pattern
 

microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.good.al

@@ -2,7 +2,8 @@ codeunit 51204 "Perf Sample LockTable Good"
 {
     procedure GetOrCreate(var AgentStatus: Record "Integer"): Boolean
     begin
-        // Read path: no lock.
+        // Read path: consistent read on this record instance only.
+        AgentStatus.ReadIsolation := IsolationLevel::ReadCommitted;
         if AgentStatus.Get(1) then
             exit(true);
 

microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.md

@@ -11,11 +11,11 @@ application-area: [all]
 
 ## Description
 
-LockTable takes an exclusive write lock on the affected table for the remainder of the transaction. In a helper that is called from many read-only sites and a few write sites, placing LockTable unconditionally at the top serializes every reader on every other reader's lock — the helper becomes a system-wide contention point. The correct shape is a conditional structure: try the read-only path first, and only fall through to LockTable when the code genuinely needs to modify the table.
+LockTable causes reads against the table to use update locks for the remainder of the transaction. In a helper that is called from many read-only sites and a few write sites, placing LockTable unconditionally at the top serializes every reader on every other reader's lock — the helper becomes a system-wide contention point. The correct shape is a conditional structure: try the read-only path first, and only fall through to LockTable when the code genuinely needs to modify the table.
 
 ## Best Practice
 
-For paths that are read-only, prefer `ReadIsolation` over `LockTable`. Setting `Rec.ReadIsolation := IsolationLevel::ReadCommitted` on a record variable gives fine-grained, per-instance control over the isolation level without taking an update lock on the table for the rest of the transaction. Use `LockTable` only for paths that genuinely write to the table.
+For paths that are read-only, prefer `ReadIsolation` over `LockTable`. Setting `Rec.ReadIsolation := IsolationLevel::ReadCommitted` on a record variable gives fine-grained, per-instance control over the isolation level without taking an update lock on the table for the rest of the transaction. Use `ReadCommitted` as the normal read-only choice; move to `RepeatableRead`, `Serializable`, or an update lock only when the code has a concrete consistency invariant that requires it. Use `LockTable` only for paths that genuinely write to the table.
 
 For helpers that may or may not modify records, factor the code so readers return immediately without a lock and only writers reach the LockTable call. A common pattern: attempt `Rec.Get()` first; if it returns the row, exit with the value; otherwise LockTable and proceed with the Insert. Document the pattern in a comment on the helper so callers understand why the LockTable is inside a branch.
 

microsoft/knowledge/performance/table-event-subscribers-disable-bulk-modifyall-and-deleteall.md

@@ -11,14 +11,14 @@ application-area: [all]
 
 ## Description
 
-`ModifyAll` and `DeleteAll` normally compile to a single set-based SQL UPDATE or DELETE. That optimization is conditional: if any subscriber is bound to the table's modify or delete events — `OnBeforeModifyEvent`, `OnAfterModifyEvent`, `OnBeforeDeleteEvent`, `OnAfterDeleteEvent`, and their Rec counterparts — the server must invoke AL per affected row so the subscriber sees each record. The operation falls back to a row-by-row loop, one SQL statement per row, inside the same transaction.
+`ModifyAll` and `DeleteAll` normally compile to a single set-based SQL UPDATE or DELETE. That optimization is conditional: the server falls back to row-by-row execution when it must invoke AL per affected row. Common causes are global table delete triggers, table modify/delete event subscribers, and Media or MediaSet fields added to the table or a table extension.
 
 The slowdown is invisible in the caller's source: the call site still reads as a bulk operation. It only shows up under load, and adding an apparently cheap subscriber (even an empty one, or one that guards on a condition and returns) is enough to trigger the fallback for every caller of ModifyAll/DeleteAll on that table across the system. Central tables — Item Ledger Entry, G/L Entry, Sales Line — are the worst places to attach such subscribers because every extension's bulk operation pays the cost.
 
 ## Best Practice
 
-Before subscribing to a table's modify or delete events, consider whether the logic can live elsewhere — on the triggering action, on a specific OnValidate, or on a business-event publisher. If the subscriber is unavoidable, scope it as narrowly as possible and document that it forces row-by-row execution so future maintainers understand the cost. Watch PRs that add such subscribers to heavily-modified tables.
+Before subscribing to a table's modify or delete events, consider whether the logic can live elsewhere — on the triggering action, on a specific OnValidate, or on a business-event publisher. If the subscriber, global trigger, or Media/MediaSet field is unavoidable, document that the table may no longer support set-based ModifyAll/DeleteAll. When a table has not regressed, prefer a small number of ModifyAll/DeleteAll calls; they are still commonly 10-50x faster than a manual loop.
 
 ## Anti Pattern
 
-An empty or nearly-empty `OnAfterModifyEvent` subscriber on `Sales Line` added as a placeholder for future integration. Every `ModifyAll` on `Sales Line` — in the base app, in every extension, in every tenant — now runs one SQL UPDATE per row.
+An empty or nearly-empty `OnAfterModifyEvent` subscriber on `Sales Line` added as a placeholder for future integration. Every `ModifyAll` on `Sales Line` — in the base app, in every extension, in every tenant — can now run one SQL UPDATE per row. The same regression can come from a global delete trigger or from adding a Media field to the table.

microsoft/knowledge/performance/use-dictionary-for-temporary-identity-lookups.bad.al

@@ -0,0 +1,16 @@
+codeunit 50934 "Perf Sample TempLookup Bad"
+{
+    procedure MarkSeenCustomers(var SalesLine: Record "Sales Line")
+    var
+        TempCustomer: Record Customer temporary;
+    begin
+        if SalesLine.FindSet() then
+            repeat
+                if not TempCustomer.Get(SalesLine."Sell-to Customer No.") then begin
+                    TempCustomer.Init();
+                    TempCustomer."No." := SalesLine."Sell-to Customer No.";
+                    TempCustomer.Insert();
+                end;
+            until SalesLine.Next() = 0;
+    end;
+}

microsoft/knowledge/performance/use-dictionary-for-temporary-identity-lookups.good.al

@@ -0,0 +1,13 @@
+codeunit 50933 "Perf Sample Dictionary Good"
+{
+    procedure MarkSeenCustomers(var SalesLine: Record "Sales Line")
+    var
+        SeenCustomerNos: Dictionary of [Code[20], Boolean];
+    begin
+        if SalesLine.FindSet() then
+            repeat
+                if not SeenCustomerNos.ContainsKey(SalesLine."Sell-to Customer No.") then
+                    SeenCustomerNos.Add(SalesLine."Sell-to Customer No.", true);
+            until SalesLine.Next() = 0;
+    end;
+}

microsoft/knowledge/performance/use-dictionary-for-temporary-identity-lookups.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: performance
+keywords: [dictionary, temporary-table, lookup, identity, o1]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Use Dictionary for temporary identity lookups
+
+## Description
+
+A temporary record is useful when code needs record semantics: filters, keys, FlowFields, or table-shaped buffers. When the only operation is "have I seen this key?" or "what value belongs to this key?", a `Dictionary` is the simpler and faster structure. Dictionary lookup is O(1) by key, while a temporary table still pays record and key-management overhead.
+
+## Best Practice
+
+Use `Dictionary` for in-memory lookup sets and maps whose keys fit in memory and whose access pattern is by identity. Keep temporary tables for data that needs table APIs, multiple keys, filter expressions, or later processing as records.
+
+See sample: `use-dictionary-for-temporary-identity-lookups.good.al`.
+
+## Anti Pattern
+
+Creating a temporary table solely to call `Get` or `FindFirst` by a single key in a loop. The code looks familiar to AL developers, but it is heavier than the lookup problem requires.
+
+See sample: `use-dictionary-for-temporary-identity-lookups.bad.al`.

microsoft/knowledge/performance/use-findset-readonly-by-default.md

@@ -11,11 +11,11 @@ application-area: [all]
 
 ## Description
 
-FindSet has two modes: FindSet() and FindSet(false) are read-only and take no write lock; FindSet(true) calls LockTable before fetching. Write locks are expensive and hold for the remainder of the transaction, so passing `true` when you do not intend to modify the records increases contention under load.
+FindSet has two modes: FindSet() and FindSet(false) are read-only and take no update lock; FindSet(true) sets update-lock read isolation on the record before fetching. Update locks are expensive and hold for the lock scope, so passing `true` when you do not intend to modify the records increases contention under load.
 
 ## Best Practice
 
-Call FindSet with no arguments when the loop only reads field values. Pass `true` only when the same loop is expected to call Modify, Delete, or Rename on the record, and the correctness of the operation depends on the table being locked for the full iteration.
+Call FindSet with no arguments when the loop only reads field values. Pass `true` only when the same loop is expected to call Modify, Delete, or Rename on the record, and the correctness of the operation depends on the matching rows being locked for the iteration.
 
 See sample: `use-findset-readonly-by-default.good.al`.
 

microsoft/knowledge/performance/use-setloadfields-for-partial-records.md

@@ -15,7 +15,7 @@ SetLoadFields instructs the platform to hydrate only the listed fields on a reco
 
 ## Best Practice
 
-Call SetLoadFields before FindSet, FindFirst, or Get whenever the code path only reads a subset of fields. List every field that is read or written during the operation, including fields used in calculations and downstream function calls. Omitting a field that is later accessed triggers a second round-trip.
+Call SetLoadFields before FindSet, FindFirst, or Get when the table is wide enough to matter (roughly 10+ fields) and the code path reads a small subset (roughly under 60%) across a material number of rows. Short loops over narrow tables usually do not earn the extra coupling; see `skip-setloadfields-on-narrow-tables-and-short-loops` for that exception. List every field that is read or written during the operation, including fields used in calculations and downstream function calls. Omitting a field that is later accessed triggers a second round-trip.
 
 Fields that appear **only** in SetRange or SetFilter calls do not need to be included — the database resolves the filter using the index without hydrating the value into AL memory. Including filter-only fields wastes bandwidth and is not required.
 

microsoft/knowledge/performance/use-textbuilder-for-loop-string-assembly.bad.al

@@ -0,0 +1,14 @@
+codeunit 50932 "Perf Sample TextConcat Bad"
+{
+    procedure BuildItemList(var Item: Record Item): Text
+    var
+        Result: Text;
+    begin
+        if Item.FindSet() then
+            repeat
+                Result += StrSubstNo('%1,%2', Item."No.", Item.Description);
+            until Item.Next() = 0;
+
+        exit(Result);
+    end;
+}

microsoft/knowledge/performance/use-textbuilder-for-loop-string-assembly.good.al

@@ -0,0 +1,14 @@
+codeunit 50931 "Perf Sample TextBuilder Good"
+{
+    procedure BuildItemList(var Item: Record Item): Text
+    var
+        Builder: TextBuilder;
+    begin
+        if Item.FindSet() then
+            repeat
+                Builder.AppendLine(StrSubstNo('%1,%2', Item."No.", Item.Description));
+            until Item.Next() = 0;
+
+        exit(Builder.ToText());
+    end;
+}

microsoft/knowledge/performance/use-textbuilder-for-loop-string-assembly.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: performance
+keywords: [textbuilder, string-concatenation, loop, text, allocation]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Use TextBuilder for loop-based string assembly
+
+## Description
+
+Repeated `Text := Text + ...` concatenation inside a loop reallocates and copies the growing string on every iteration. In AL, `TextBuilder` is the platform type for constructing larger text payloads incrementally. `StrSubstNo` remains appropriate for formatting one message; TextBuilder is for many appends, especially inside loops.
+
+## Best Practice
+
+Use `TextBuilder.Append` or `AppendLine` when assembling CSV rows, log payloads, JSON-ish diagnostic text, or other multi-line strings from repeated loop iterations. Convert to Text once, after the loop, with `ToText()`.
+
+See sample: `use-textbuilder-for-loop-string-assembly.good.al`.
+
+## Anti Pattern
+
+Appending to the same Text variable on every iteration of a large loop. Each append copies the accumulated prefix again, so the cost grows with both row count and final string length.
+
+See sample: `use-textbuilder-for-loop-string-assembly.bad.al`.

microsoft/knowledge/privacy/classify-data-at-migration-destination.bad.al

@@ -0,0 +1,14 @@
+table 50936 "Migrated Employee"
+{
+    fields
+    {
+        field(1; "Employee No."; Code[20])
+        {
+            DataClassification = ToBeClassified;
+        }
+        field(2; "Tax Identification No."; Text[30])
+        {
+            DataClassification = SystemMetadata;
+        }
+    }
+}

microsoft/knowledge/privacy/classify-data-at-migration-destination.good.al

@@ -0,0 +1,14 @@
+table 50935 "Migrated Employee"
+{
+    fields
+    {
+        field(1; "Employee No."; Code[20])
+        {
+            DataClassification = EndUserPseudonymousIdentifiers;
+        }
+        field(2; "Tax Identification No."; Text[30])
+        {
+            DataClassification = EndUserIdentifiableInformation;
+        }
+    }
+}

microsoft/knowledge/privacy/classify-data-at-migration-destination.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: privacy
+keywords: [migration, dataclassification, hybrid, destination, pii]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Classify migrated data at the destination field
+
+## Description
+
+Hybrid migration codeunits such as HybridSL, HybridGP, and HybridBC legitimately process sensitive source data: tax IDs, employee identifiers, financial balances, and customer records. The privacy concern is not that the migration code touches the data. The concern is where the data lands: the destination table field must have a DataClassification value that matches the migrated content.
+
+## Best Practice
+
+When reviewing migration code, follow the assignment to the destination field and verify that the destination table declares an appropriate field-level or inherited DataClassification. Treat the migration procedure itself as expected business functionality; flag only missing or understated classification on the persistent destination.
+
+See sample: `classify-data-at-migration-destination.good.al`.
+
+## Anti Pattern
+
+Flagging a migration procedure merely because it copies tax IDs or names from a source system. That creates false positives and misses the real issue: a destination field with no classification, `ToBeClassified`, or `SystemMetadata` for customer or employee data.
+
+See sample: `classify-data-at-migration-destination.bad.al`.