[TEST – DO NOT MERGE] Validate BCQuality Copilot PR-review wiring by JesperSchulz · Pull Request #8772 · microsoft/BCApps

JesperSchulz · 2026-06-24T11:32:28Z

⚠️ This is a test PR — please DO NOT MERGE

This PR exists only to verify that the BCQuality Copilot PR-review wiring added in #8700 (Consume BCQuality knowledge base for Copilot PR review) is functioning correctly on microsoft/BCApps. It should be closed once the review agent has run.

What it does

It replicates the exact byte-identical sample diff from microsoft/BCAppsBCQuality#47 (350 additions, 0 deletions) so we can do an apples-to-apples comparison of the review agent's findings against that known baseline.

Changes under src/Apps/W1/Quality Management/app/:

File	Object
`src/Integration/ExternalResults/QltyExtResultImport.Codeunit.al`	codeunit 20584 (new)
`src/Integration/ExternalResults/QltyImportLogEntry.Table.al`	table 20585 (new)
`src/Integration/ExternalResults/QltyImportLog.Page.al`	page 20586 (new)
`src/Integration/ExternalResults/QltyExtResultEvents.Codeunit.al`	codeunit 20590 (new)
`src/Utilities/QltyFileImport.Codeunit.al`	modified

Expected outcome

The sample code is intentionally crafted with reviewable issues (magic numbers, Commit() inside loops, Get/FindFirst results thrown away, PII fields without DataClassification, plain-text credentials, uppercase reserved keywords, spaces before parentheses, an off-by-one loop, PII in telemetry, etc.). A successful wiring should surface these as review comments — mirroring the ~25 findings seen on the #47 baseline.

If the review agent posts findings here, the wiring is working. 🎯

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

…te BCQuality wiring Replicates the exact byte-identical sample diff from microsoft/BCAppsBCQuality#47 (adds external quality result import codeunit 20584, log table 20585, list page 20586, integration events 20590, and extends QltyFileImport.Codeunit.al) to verify that the BCQuality Copilot PR-review wiring added in #8700 triggers and produces findings on microsoft/BCApps. This PR is intentionally crafted with reviewable issues and is NOT meant to be merged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-06-24T11:34:12Z

Could not find a linked ADO work item. Please link one by using the pattern 'AB#' followed by the relevant work item number. You may use the 'Fixes' keyword to automatically resolve the work item when the pull request is merged. E.g. 'Fixes AB#1234'

github-actions · 2026-06-24T12:02:25Z

+        for I := 1 to MaxFiles do begin
+            Clear(ServerFileName);
+            if not PromptAndImportIntoInStream(FilterString, FileInStream, ServerFileName) then
+                exit(ImportedCount);


$\textbf{🟡\ Medium\ Severity\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

In ImportBatch, TempBlob.CreateInStream(FileInStream) is called immediately after PromptAndImportIntoInStream has populated FileInStream with the uploaded file's content.

CreateInStream overwrites FileInStream with a new, empty InStream backed by TempBlob's empty internal buffer — the uploaded file's data is silently discarded on every iteration. The import count is incremented as if the file were processed. To buffer uploaded content into TempBlob, first create an OutStream from TempBlob, copy the uploaded stream into it, then create the InStream; alternatively, remove TempBlob entirely and process FileInStream directly after the upload succeeds.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:27Z

+
+        if ResultBuffer.FindSet() then
+            repeat
+                // GET inside the loop re-reads the same customer on every iteration.


$\textbf{🟠\ High\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Customer.Get(CustomerNo) is called on every iteration of the repeat...until loop in ImportResults.

Because CustomerNo is a fixed parameter that does not change between iterations, this issues N identical round-trips to the Customer table. The fix is to hoist the Get call to before the FindSet check so the record is loaded exactly once.

Knowledge:

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:28Z

+                ImportLogEntry."Result Value" := ResultBuffer."Result Value";
+                ImportLogEntry.Insert();
+
+                // Commit inside the loop defeats the implicit transaction boundary.


$\textbf{🟠\ High\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Commit() is called inside a repeat...until loop in ImportResults.

This creates one transaction per imported row, defeating the platform's ability to roll back the whole import atomically and adding a transaction-boundary round-trip on every row. Per the referenced guidance, place Commit at a bounded checkpoint after N rows, or wrap each chunk in Codeunit.Run for native rollback on failure.

Knowledge:

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:30Z

+        ImportLogEntry: Record "Qlty. Import Log Entry";
+    begin
+        ImportLogEntry.SetRange("Customer No.", CustomerNo);
+        // Count() materializes a number the caller does not need.


$\textbf{🟠\ High\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

HasImportedResults uses Count() > 0 to test for the existence of any row, then falls through to FindFirst() — which is also only testing existence.

Both operations do more work than needed: Count() materialises a count the caller does not use; FindFirst() materialises a row the caller does not read. Per the referenced guidance, replace both with a single exit(not ImportLogEntry.IsEmpty()) after the SetRange.

Knowledge:

microsoft/knowledge/performance/use-isempty-for-existence-check.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:31Z

+
+            TempBlob.CreateInStream(FileInStream);
+            ImportedCount += 1;
+


$\textbf{🟠\ High\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Commit() is called inside the for-loop in ImportBatch.

Every uploaded file gets its own transaction, removing atomicity and adding lock overhead on each iteration. Per the referenced guidance, commit at a clearly defined safe boundary outside the loop, or use Codeunit.Run for sub-operation atomicity.

Knowledge:

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:33Z

+        Dimensions: Dictionary of [Text, Text];
+    begin
+        // PII (customer name) placed directly in the telemetry message string,
+        // and a placeholder event id that was never registered.


$\textbf{🟠\ High\ Severity\ —\ Privacy} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

LogImportCompleted passes Customer.Name via StrSubstNo as the message text of Session.LogMessage.

The platform sends the message string to telemetry verbatim regardless of the DataClassification argument on the same call, so the customer name is now in Application Insights. Per the referenced guidance, replace the message with a static string such as 'Import completed' and move any structured context to custom dimensions — provided those dimensions carry no PII per featuretelemetry-customdimensions-no-pii.md.

Knowledge:

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:35Z

+            DataClassification::CustomerContent,
+            TelemetryScope::All,
+            'Category', 'QualityImport');
+


$\textbf{🟠\ High\ Severity\ —\ Privacy} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

FeatureTelemetry.LogUsage receives a Dimensions dictionary containing 'CustomerName' -> Customer.Name.

Customer names are CustomerContent-class PII and must not appear in FeatureTelemetry custom dimensions; the platform sends them verbatim to telemetry. Per the referenced guidance, pass only non-personal context such as feature names, status codes, counts, or durations.

Knowledge:

microsoft/knowledge/privacy/featuretelemetry-customdimensions-no-pii.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:36Z

+    local procedure RaiseImportError(CustomerNo: Code[20])
+    begin
+        // Error message composed with StrSubstNo instead of passing the
+        // parameter directly to Error().


$\textbf{🟠\ High\ Severity\ —\ Privacy} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

RaiseImportError calls Error(StrSubstNo('Could not import results for customer %1', CustomerNo)).

The StrSubstNo result is a plain Text with all substitutions already performed; the platform cannot apply DataClassification to its contents when Error runs, so the interpolated customer identifier is logged to telemetry verbatim. Per the referenced guidance, pass the format Label and the argument separately — Error(CouldNotImportErr, CustomerNo) — so the platform classifies each argument individually.

Knowledge:

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:38Z

+        }
+        // PII fields below carry no DataClassification.
+        field(3; "Customer Name"; Text[100])
+        {


$\textbf{🟠\ High\ Severity\ —\ Privacy} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Table field 3 'Customer Name' (Text[100]) stores a personal name but has no DataClassification property.

The platform defaults missing classification to SystemMetadata, which misclassifies customer name data in telemetry, GDPR exports, and admin reports. Per the referenced guidance, set DataClassification = CustomerContent on this field.

Suggested change

{

field(3; "Customer Name"; Text[100])

{

Caption = 'Customer Name';

DataClassification = CustomerContent;

}

Knowledge:

microsoft/knowledge/privacy/data-classification-required-on-pii-fields.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:40Z

+            Caption = 'Customer Name';
+        }
+        field(4; "Contact Email"; Text[80])
+        {


$\textbf{🟠\ High\ Severity\ —\ Privacy} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Table field 4 'Contact Email' (Text[80]) stores an email address but has no DataClassification property.

The platform defaults to SystemMetadata, under-classifying this PII field in telemetry and GDPR exports. Per the referenced guidance, set DataClassification = CustomerContent on this field.

Suggested change

{

field(4; "Contact Email"; Text[80])

{

Caption = 'Contact Email';

DataClassification = CustomerContent;

}

Knowledge:

microsoft/knowledge/privacy/data-classification-required-on-pii-fields.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

Seems like we need to revisit suggestions. It should only insert the DataClassification, not the entire field!

github-actions · 2026-06-24T12:02:42Z

+    /// </summary>
+    [IntegrationEvent(false, false)]
+    procedure OnBeforeCallLabApi(Endpoint: Text; ApiKey: Text; var BearerToken: Text)
+    begin


$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

OnBeforeCallLabApi exposes ApiKey: Text and var BearerToken: Text as IntegrationEvent parameters.

Any extension on the tenant can subscribe to this event and read — or overwrite — both values. Per the referenced guidance, credentials must never flow through event parameters; restrict the payload to non-sensitive context (the business record, an IsHandled flag, or a mutable payload object whose contents the publisher controls). Authentication should be handled entirely within the publisher.

Knowledge:

microsoft/knowledge/security/integrationevent-must-not-expose-secrets.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:43Z

+    end;
+
+    local procedure FetchResultsIntoBuffer(CustomerNo: Code[20]; var ResultBuffer: Record "Qlty. Import Log Entry" temporary)
+    var


$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

ApiKey and BearerToken are declared as Text in both FetchResultsIntoBuffer's var block and DownloadResults's parameter list.

Any value that carries an API key, bearer token, or other secret must be SecretText end-to-end so the compiler prevents silent plain-text exposure and the debugger cannot read the value. Per the referenced guidance, declare both as SecretText and cascade the type change through GetApiKey/GetAccessToken return types and DownloadResults parameters.

Knowledge:

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:45Z

+    local procedure RaiseImportError(CustomerNo: Code[20])
+    begin
+        // Error message composed with StrSubstNo instead of passing the
+        // parameter directly to Error().


$\textbf{🟠\ High\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

RaiseImportError wraps a hardcoded string literal in StrSubstNo and passes the result as the sole argument to Error().

Per the referenced guidance (CodeCop AA0231), Error() accepts a format Label and arguments directly; wrapping with StrSubstNo hides the placeholders from the translation pipeline and removes the format-string identity from the call site. Declare a Label with the 'Err' suffix and call Error(CouldNotImportResultsForCustomerErr, CustomerNo).

Knowledge:

microsoft/knowledge/style/error-passes-parameters-directly-not-strsubstno.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:46Z

+    end;
+
+    // Uppercase reserved keywords throughout this procedure body.
+    local procedure CountValidEntries(var ImportLogEntry: Record "Qlty. Import Log Entry"): Integer


$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

CountValidEntries uses uppercase reserved keywords throughout its body: VAR, BEGIN, IF, REPEAT, UNTIL, EXIT.

Per the referenced guidance (CodeCop AA0241), all AL reserved keywords must be lowercase. Run the AL formatter to normalise this procedure.

Knowledge:

microsoft/knowledge/style/lowercase-reserved-keywords.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:48Z

+    local procedure DescribeEntry(EntryNo: Integer): Text
+    var
+        ImportLogEntry: Record "Qlty. Import Log Entry";
+    begin


$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

DescribeEntry has a space between the method name and its opening parenthesis on three call sites: 'Get (EntryNo)', 'exit (ImportLogEntry...)', and 'exit ('')'.

Per the referenced guidance (CodeCop AA0002), there must be no whitespace between a method name and its '('.

Suggested change

begin

if ImportLogEntry.Get(EntryNo) then

exit(ImportLogEntry."Customer Name");

exit('');

Knowledge:

microsoft/knowledge/style/no-space-before-method-parenthesis.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:49Z

+    begin
+        // PII (customer name) placed directly in the telemetry message string,
+        // and a placeholder event id that was never registered.
+        Session.LogMessage(


$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Session.LogMessage uses the placeholder event ID '0000'.

Per the referenced guidance, event IDs must be stable, unique, and non-placeholder — IDs like '0000' collide with every other extension that uses the same placeholder and make the event unsearchable in Application Insights or KQL dashboards. Assign a real, registered ID from the extension's telemetry catalogue before this code ships.

Knowledge:

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:51Z

+                    ToolTip = 'Specifies the number of the import log entry.';
+                }
+                field("Customer No."; Rec."Customer No.")
+                {


$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Page field 'Customer No.' has no ToolTip property.

Per the referenced guidance (CodeCop AA0218), every field control on a regular page requires a non-empty ToolTip starting with 'Specifies'. AppSource technical validation rejects pages with missing tooltips.

Suggested change

{

field("Customer No."; Rec."Customer No.")

{

ToolTip = 'Specifies the customer number.';

}

Knowledge:

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:53Z

+                {
+                }
+                field("Customer Name"; Rec."Customer Name")
+                {


$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Page field 'Customer Name' has no ToolTip property.

Per the referenced guidance (CodeCop AA0218), every field control on a regular page requires a non-empty ToolTip starting with 'Specifies'.

Suggested change

{

field("Customer Name"; Rec."Customer Name")

{

ToolTip = 'Specifies the customer name.';

}

Knowledge:

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:54Z

+                {
+                }
+                field("Contact Email"; Rec."Contact Email")
+                {


$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Page field 'Contact Email' has no ToolTip property.

Per the referenced guidance (CodeCop AA0218), every field control on a regular page requires a non-empty ToolTip starting with 'Specifies'.

Suggested change

{

field("Contact Email"; Rec."Contact Email")

{

ToolTip = 'Specifies the contact email address.';

}

Knowledge:

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:56Z

+                    ToolTip = 'Specifies the measured result value.';
+                }
+                field("Imported At"; Rec."Imported At")
+                {


$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Page field 'Imported At' has no ToolTip property.

Per the referenced guidance (CodeCop AA0218), every field control on a regular page requires a non-empty ToolTip starting with 'Specifies'.

Suggested change

{

field("Imported At"; Rec."Imported At")

{

ToolTip = 'Specifies the date and time when the result was imported.';

}

Knowledge:

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-06-24T12:02:58Z

+    /// <summary>
+    /// Returns the upload dialog title, looked up with a space before the parenthesis.
+    /// </summary>
+    procedure GetDialogTitle(): Text


$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

GetDialogTitle has a space before the parenthesis in 'exit (ImportFromLbl)'.

Per the referenced guidance (CodeCop AA0002), there must be no whitespace between a method name and its '('.

Suggested change

procedure GetDialogTitle(): Text

exit(ImportFromLbl);

Knowledge:

microsoft/knowledge/style/no-space-before-method-parenthesis.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

This suggestion seems wrong. It suggests to replace the procedure name, while it should replace the exit statement.

github-actions · 2026-06-24T12:03:00Z

Copilot PR Review

Iteration 1 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630

Findings by domain

Findings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).

Domain	Findings	Knowledge-backed	Agent	Inline
Agent	1	0	1	1
Performance	4	4	0	4
Privacy	5	5	0	5
Security	2	2	0	2
Style	9	9	0	9

Totals: 20 knowledge-backed · 1 agent findings.

Orchestrator pre-filter (13 file(s) excluded)

layer-disabled (knowledge) : 13 file(s)

_{Findings produced by the Copilot CLI agent against BCQuality at 822cae1b2771ac25f665f73369f69093bd4fd630. Reply 👎 on any inline comment to flag false positives.}

) ## Problem On [#8772](#8772) two ```suggestion``` blocks were mis-applied, as flagged in review comments: - **`QltyFileImport` (line 68):** the comment anchored `procedure GetDialogTitle(): Text` but the fix `exit(ImportFromLbl);` was meant for line 70. Applying it would overwrite the procedure signature. - **`QltyImportLogEntry` (line 34):** the comment anchored `{` but the suggested code was the whole 4-line field. Applying it replaced `{` with the entire block, duplicating the field instead of just inserting the `DataClassification` line. Root cause: the orchestrator always posted a **single-line** comment at the model's reported `location.line`, but a GitHub suggestion replaces *exactly* the anchored line(s). When the model anchors a different/narrower line than the fix targets, the suggestion corrupts the file. ## Fix `tools/Code Review/scripts/Invoke-CopilotPRReview.ps1` now validates and re-anchors each suggestion against the PR-head file content before posting: - **Single-line fix** → snapped onto the line it actually rewrites (whitespace-insensitive match within a small window; trusts the model's anchor when no unique match exists). - **Multi-line fix** that edits a construct → posted as a **multi-line comment** over the full span it replaces, so inserted lines land in place. - **Unplaceable fix** → the applicable block is dropped and the change is shown as a manual, non-applicable snippet. New helpers: `Get-PrHeadFileLines`, `ConvertTo-LooseLine`, `Test-OrderedSubsequence`, `Resolve-SuggestionPlacement`; `New-ReviewComment` gained `start_line`/`start_side` support; `Build-CommentBody` gained `-SuppressSuggestion`. ## Validation Unit-tested the resolver against both real cases (snaps to 70..70; expands to 33..36) plus edge cases (genuine single-line rewrite trusts anchor, ambiguous match trusts anchor, unplaceable → drop, empty file → drop). Full script parses cleanly. Fixes: [AB#640364](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/640364) Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions Bot added the AL: Apps (W1) Add-on apps for W1 label Jun 24, 2026

JesperSchulz marked this pull request as ready for review June 24, 2026 11:40

JesperSchulz requested a review from a team June 24, 2026 11:40

github-actions Bot reviewed Jun 24, 2026

View reviewed changes

JesperSchulz mentioned this pull request Jun 24, 2026

Anchor Copilot PR-review suggestion blocks to the correct line(s) #8806

Merged

Copilot AI mentioned this pull request Jun 25, 2026

Anchor Copilot PR-review suggestion blocks to the correct line(s) #8815

Closed

JesperSchulz mentioned this pull request Jun 25, 2026

[TEST – DO NOT MERGE] Validate BCQuality suggested inline fixes (re-test of #8772) #8816

Closed

JesperSchulz closed this Jun 25, 2026

JesperSchulz mentioned this pull request Jun 25, 2026

[TEST – DO NOT MERGE] Loyalty Sample across all 11 BCQuality domains #8822

Open

-        {
+        field(4; "Contact Email"; Text[80])
+        {
+            Caption = 'Contact Email';
+            DataClassification = CustomerContent;
+        }

-    begin
+        if ImportLogEntry.Get(EntryNo) then
+            exit(ImportLogEntry."Customer Name");
+        exit('');

Uh oh!

Conversation

JesperSchulz commented Jun 24, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ This is a test PR — please DO NOT MERGE

What it does

Expected outcome

Uh oh!

github-actions Bot commented Jun 24, 2026

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

In ImportBatch, TempBlob.CreateInStream(FileInStream) is called immediately after PromptAndImportIntoInStream has populated FileInStream with the uploaded file's content.

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

Customer.Get(CustomerNo) is called on every iteration of the repeat...until loop in ImportResults.

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

Commit() is called inside a repeat...until loop in ImportResults.

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

HasImportedResults uses Count() > 0 to test for the existence of any row, then falls through to FindFirst() — which is also only testing existence.

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

Commit() is called inside the for-loop in ImportBatch.

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

LogImportCompleted passes Customer.Name via StrSubstNo as the message text of Session.LogMessage.

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

FeatureTelemetry.LogUsage receives a Dimensions dictionary containing 'CustomerName' -> Customer.Name.

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

RaiseImportError calls Error(StrSubstNo('Could not import results for customer %1', CustomerNo)).

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

Table field 3 'Customer Name' (Text[100]) stores a personal name but has no DataClassification property.

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

Table field 4 'Contact Email' (Text[80]) stores an email address but has no DataClassification property.

Uh oh!

JesperSchulz Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

OnBeforeCallLabApi exposes ApiKey: Text and var BearerToken: Text as IntegrationEvent parameters.

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

ApiKey and BearerToken are declared as Text in both FetchResultsIntoBuffer's var block and DownloadResults's parameter list.

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

RaiseImportError wraps a hardcoded string literal in StrSubstNo and passes the result as the sole argument to Error().

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

CountValidEntries uses uppercase reserved keywords throughout its body: VAR, BEGIN, IF, REPEAT, UNTIL, EXIT.

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

DescribeEntry has a space between the method name and its opening parenthesis on three call sites: 'Get (EntryNo)', 'exit (ImportLogEntry...)', and 'exit ('')'.

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

Session.LogMessage uses the placeholder event ID '0000'.

Uh oh!

github-actions Bot Jun 24, 2026

Choose a reason for hiding this comment

Page field 'Customer No.' has no ToolTip property.

JesperSchulz commented Jun 24, 2026 •

edited by github-actions Bot

Loading