[Master]-Opening price list from Customer Card is extremely slow on large datasets (~220s) due to AddAllSourceType call

[neeleshsinghal]

AB#641061

[github-actions]

Copilot PR Review

Iteration 2 · Outcome: completed

All 16 sub-skills completed; no knowledge-backed violations or cross-cutting defects found. The diff optimizes BuildSourceFilters to avoid unconditional Mark-based filtering (using IsEmpty existence checks and SetView for the 0/1-match cases), which was validated against BCQuality performance guidance and found compliant.

Knowledge source: https://github.com/microsoft/BCQuality@186d8a131465475c79244d994acb872cd5c0d4bf

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

• layer-disabled (knowledge) : 2 file(s)

_{Findings produced by the AL review agent v1.7.3. Reply 👎 on any inline comment to flag false positives.}

[qasimikram]

S1 (blocking): Please preserve an empty result when none of the price sources match. In BuildSourceFilters, NonEmptyCount = 0 currently leaves MarkingIsUsed false and then calls ClearSourceFilters. The caller consequently skips MarkedOnly(true), leaving only the status, price type, and amount type filters and potentially showing unrelated price lines. Please handle the zero-match case explicitly and add a regression test where every source is empty.

Question on performance coverage: The new fast path avoids marking only when exactly one source has matching rows. When both All Customers and a customer-specific or group source contain rows, NonEmptyCount reaches 2 and the implementation returns to marking every matching row, including a potentially very large All Customers set. Could you confirm one of the following?

1. The production reproduction has only one non-empty source, with before/after timing showing that this change meets the <5-second target.
2. The multi-source scenario has also been tested at scale and remains acceptably fast when both All Customers and customer/group rows exist.

[qasimikram]

The zero-match regression is addressed: NonEmptyCount = 0 now forces MarkedOnly(true), and T222 covers the case where unrelated price lines exist but none match the current customer's sources.

One performance-coverage question remains. The fast path avoids marking only when exactly one source has matching rows. If both All Customers and a customer-specific or group source contain rows, NonEmptyCount reaches 2 and the implementation still marks every matching row, including a potentially large All Customers set. Could you confirm one of the following?

1. The production reproduction has only one non-empty source, with before/after timing showing that this change meets the <5-second target.
2. The multi-source scenario has also been tested at scale and remains acceptably fast when both All Customers and customer/group rows exist.

Non-blocking test suggestion: use Validate for the customer group fields and Modify(true) in T221/T222 so the fixtures exercise the same validation path as production setup.

[qasimikram]

The blocking zero-match regression is fixed and covered by T222. I found no remaining correctness issue in the updated diff. Approving with the open performance-coverage thread left for confirmation before merge.

[github-actions]

⚠️ Stale Status Check Deleted

The Pull Request Build workflow run for this PR was older than 72 hours and has been deleted.

📋 Why was it deleted?

Status checks that are too old may no longer reflect the current state of the target branch. To ensure this PR is validated against the latest code and passes up-to-date checks, a fresh build is required.

---

🔄 How to trigger a new status check:

1. 📤 Push a new commit to the PR branch, or
2. 🔁 Close and reopen the PR

This will automatically trigger a new Pull Request Build workflow run.