Address performance retrieval review

Retrieve Commit-in-loop guidance precisely, process exact checkpoint key lists, and narrow clone-before-write discovery.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 85be3fc4-5253-47b8-ba1b-6b8fd188fcea

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

@@ -1,7 +1,7 @@
 ---
 bc-version: [all]
 domain: performance
-keywords: [clone, copy, recordref, gettable, by-value, modify, delete, loop]
+keywords: [clone, clone-before-write, copy, gettable, by-value, copied-record, writing-helper]
 technologies: [al]
 countries: [w1]
 application-area: [all]

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

@@ -26,36 +26,33 @@ codeunit 50128 "Perf Sample CommitInLoop Good"
     local procedure NormalizeNextChunk(var LastCustomerNo: Code[20]): Boolean
     var
         Customer: Record Customer;
+        TempCustomer: Record Customer temporary;
         CustomerChunk: Query "Perf Customer Chunk";
-        FirstCustomerNo: Code[20];
         LastChunkCustomerNo: Code[20];
     begin
         CustomerChunk.TopNumberOfRows(500);
         if LastCustomerNo <> '' then
             CustomerChunk.SetFilter(CustomerNo, '>%1', LastCustomerNo);
         CustomerChunk.Open();
-        if not CustomerChunk.Read() then begin
-            CustomerChunk.Close();
-            exit(false);
-        end;
-
-        FirstCustomerNo := CustomerChunk.CustomerNo;
-        repeat
+        while CustomerChunk.Read() do begin
+            TempCustomer.Init();
+            TempCustomer."No." := CustomerChunk.CustomerNo;
+            TempCustomer.Insert();
             LastChunkCustomerNo := CustomerChunk.CustomerNo;
-        until not CustomerChunk.Read();
+        end;
         CustomerChunk.Close();
 
-        Customer.SetCurrentKey("No.");
-        Customer.SetRange("No.", FirstCustomerNo, LastChunkCustomerNo);
-        if not Customer.FindSet(true) then begin
-            LastCustomerNo := LastChunkCustomerNo;
-            exit(true);
-        end;
+        if TempCustomer.IsEmpty() then
+            exit(false);
 
-        repeat
-            Customer.Name := UpperCase(Customer.Name);
-            Customer.Modify();
-        until Customer.Next() = 0;
+        Customer.LockTable();
+        if TempCustomer.FindSet() then
+            repeat
+                if Customer.Get(TempCustomer."No.") then begin
+                    Customer.Name := UpperCase(Customer.Name);
+                    Customer.Modify();
+                end;
+            until TempCustomer.Next() = 0;
 
         LastCustomerNo := LastChunkCustomerNo;
         exit(true);

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

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

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

@@ -38,15 +38,18 @@ Discard files that are not applicable. Retain conditionally applicable files (an
 Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against:
 
 - The changed AL object names and types — especially tables, pages with SourceTable bindings, reports, queries, and codeunits performing record iteration.
-- The changed procedures and triggers, weighted toward those that perform loops, Find/FindSet/FindFirst calls, CalcFields, SetAutoCalcFields, CalcSums, FlowField access, record copying, RecordRef conversion, Modify/Delete calls, or cross-table navigation.
-- Tokens extracted from the diff that relate to data access and hot-path costs (`SetRange`, `SetFilter`, `SetLoadFields`, `SetCurrentKey`, `FindSet`, `ReadIsolation`, `LockTable`, `ModifyAll`, `DeleteAll`, `Modify`, `Delete`, `Copy`, `RecordRef`, `GetTable`, `TextBuilder`, `Dictionary`, `temporary`, `repeat`, `until`, `CalcFields`, `SetAutoCalcFields`, `CalcSums`).
+- The changed procedures and triggers, weighted toward those that perform loops, Find/FindSet/FindFirst calls, CalcFields, SetAutoCalcFields, CalcSums, FlowField access, Commit calls, checkpoint helpers, record copying, RecordRef conversion, Modify/Delete calls, or cross-table navigation.
+- Tokens extracted from the diff that relate to data access and hot-path costs (`SetRange`, `SetFilter`, `SetLoadFields`, `SetCurrentKey`, `FindSet`, `ReadIsolation`, `LockTable`, `ModifyAll`, `DeleteAll`, `Modify`, `Delete`, `Commit`, `checkpoint`, `Copy`, `RecordRef`, `GetTable`, `TextBuilder`, `Dictionary`, `temporary`, `repeat`, `until`, `CalcFields`, `SetAutoCalcFields`, `CalcSums`).
 
 A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
 
 Apply these targeted cues even when simple token overlap would rank the article below the worklist cutoff:
 
 - Worklist `use-setautocalcfields-for-per-row-flowfields.md` when a record loop calls `CalcFields`, or when every row reads the same FlowField for a comparison, branch, or per-record action. Worklist `calcsums-instead-of-calcfields-in-loop.md` instead when the loop only accumulates one set total.
-- Worklist `avoid-cloning-records-before-modify-delete-in-loops.md` when an iteration calls `Copy` or `RecordRef.GetTable` before `Modify`/`Delete`, or passes the iterated record without `var` to a helper that writes that record. Do not match a read-only copy, a temporary record, a different target table, or a `RecordRef` opened and iterated directly.
+- Worklist `avoid-commit-inside-loops.md` only when `Commit()` is inside a record-iteration body or a helper invoked once per row. Do not match one `Commit()` after a bounded checkpoint helper returns, a `Commit()` outside iteration, or comments and documentation that merely mention commits.
+- Worklist `avoid-cloning-records-before-modify-delete-in-loops.md` when an iteration calls `Copy` or `RecordRef.GetTable` before `Modify`/`Delete`, or passes the iterated record without `var` to a helper that writes that record. Do not worklist it from `Modify`, `Delete`, or `RecordRef` alone; exclude a direct write on the iterator, a read-only copy, a temporary record, a different target table, and a `RecordRef` opened and iterated directly.
+
+These targeted inclusions and exclusions override generic token overlap. Do not retain an excluded article solely because the diff contains one of its keywords.
 
 Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`.