[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 4 · 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	Fallback
Agent	1	0	1	0	0

Totals: 0 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.}

[alexei-dobriansky]

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR removes MarkedOnly from the line review source filtering path, builds native filters for Source Type, Parent Source No., and Source No., and adds a small UI test for All Customers lines. It is another PR for the same AB#641061 work as PR 9219 and PR 9265: PR 9219 is closed and had the same core OR-filter issue; PR 9265 is still open and uses a different SetLoadFields plus Mark approach.

The performance goal is valid, but this implementation still does not preserve the exact set of source tuples. It ORs each source field independently, so a line can match a combination that was never present in PriceSourceList. It also calls OnBuildSourceFiltersOnBeforeFindLines, but filters added by subscribers are not carried into the final filters used by the page.

Suggestions

S1 - Preserve exact source tuples
BuildSourceFilters must preserve each (Source Type, Parent Source No., Source No.) tuple. The current three OR filters can match mixed combinations that were never in PriceSourceList. Use a tuple-safe strategy, or validate each candidate line against the source list before it reaches the page.

S2 - Keep event-added filters
The final filters only use values from PriceSource. Filters added by OnBuildSourceFiltersOnBeforeFindLines affect the IsEmpty path, but not the final line review filter. Keep subscriber filters in the final result, or keep a per-source path when subscribers add filters.

S3 - Strengthen regression and performance coverage
The new test uses only 10-20 All Customers lines. It does not catch tuple mixing or prove the large-data performance path. Add a mixed-source regression test, and add a performance-size guard if the framework supports it.

Risk assessment and necessity

Risk: This code filters price list lines for Customer, Vendor, Job, Contact, and related review flows. If the filter admits mixed source combinations, the page can show price list lines for sources the caller did not ask for. The event contract is also risky because subscriber-added filters no longer affect the final result.

Necessity: The work item is valid and important: opening Sales Prices from Customer Card can take about 220 seconds with 61k+ price lines. A fix is needed, but PR 9269 should not merge as-is. It is an unsafe alternative/replacement for the same AB#641061 work as closed PR 9219 and open PR 9265.

---

[AI-PR-REVIEW] version=1 system=github pr=9269 round=1 by=alexei-dobriansky sha=52fe3ca32c78911605d38a84f2b10a78573000a6

[alexei-dobriansky]

you might hit max filter length (2100)

[Copilot]

Pull request overview

Improves performance when opening Price List Line Review from the Customer Card by avoiding expensive record marking/materialization when it isn’t required, while adding a UI test to validate the expected “All Customers” lines appear.

Changes:

• Update Price List Management to only use MarkedOnly(true) when multiple source result sets require marking.
• Switch source filtering existence checks from FindSet() to IsEmpty() to avoid unnecessary result-set materialization.
• Add a UI test covering opening Sales Prices from Customer Card and verifying all “All Customers” lines are shown.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/Layers/W1/Tests/ERM/PriceListsUI.Codeunit.al	Adds a UI regression test validating “All Customers” price lines are shown when opening from Customer Card.
src/Layers/W1/BaseApp/Pricing/PriceList/PriceListManagement.Codeunit.al	Avoids MarkedOnly(true)/marking when filtering can be done via a single source filter set, improving performance on large datasets.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[alexei-dobriansky]

Agentic PR Review - Round 2

Recommendation: Request Changes

What this PR does

Since round 1, the PR changed BuildSourceFilters to keep native filters only when exactly one source tuple has matching lines. It falls back to MarkedOnly when more than one source tuple has lines, and it keeps the tuple checks per source.

This addresses the independent OR-filter bug and mostly addresses subscriber-added filters, because rows are checked after OnBuildSourceFiltersOnBeforeFindLines runs. The remaining problem is that the real Customer Card source list can contain customer, price group, discount group, and All Customers sources. If more than one of those has lines, the code still takes the old marking path, so the large-data performance problem is not proven fixed.

Status of previous suggestions

ID	Title	Status	Author response
S1	Preserve exact source tuples	Addressed	Changed in 45cc21a. The independent OR filters are gone; the result is now built per exact source tuple or left on one exact tuple filter.
S2	Keep event-added filters	Addressed	Changed in 45cc21a. Records are checked or marked after OnBuildSourceFiltersOnBeforeFindLines runs, and the single-source path keeps the final event-added filters.
S3	Strengthen regression and performance coverage	Not addressed	No author response. No test changes were pushed since round 1, and the existing small test does not cover mixed sources or a performance-size data set.

New observations (commits since round 1)

S4 - Avoid marking mixed source result sets
The new shortcut only avoids MarkedOnly when exactly one source tuple has lines. Customer Card can have customer-specific, price group, discount group, and All Customers lines, and that still uses the old marking path. Keep the tuple-safe result, but avoid materializing large mixed-source result sets or prove this path is fast.

Risk assessment and necessity

Risk: The changed code is in shared Price List Line Review filtering, so it affects Customer, Vendor, Job, Contact, and Campaign price/discount views. No public event signature changed, but the performance risk remains for mixed-source data because the code still uses MarkedOnly when more than one source has records.

Necessity: The linked Bug is clear and important: opening the page can take about 220 seconds on large data. The fix is needed, but it must cover or measure the mixed-source case that this page can produce.

---

[AI-PR-REVIEW] version=1 system=github pr=9269 round=2 by=alexei-dobriansky sha=45cc21ae5474b2b887e96fc89776046555bc58fd

[github-actions]

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

CheckIfPriceListLineMarkingIsNeededForSources (new in this PR) re-raises the published IntegrationEvent OnBuildSourceFiltersOnBeforeFindLines while pre-checking how many source groups have matching Price List Lines, and BuildSourceFilters raises the same event again for the real pass.

Any external subscriber to this event will now fire twice per source whenever SearchIfPriceExists is false (the normal 'set filters for display' path), instead of once as before this change. Note this diverges from the file's own established twin pattern: the pre-existing asset equivalent (CheckIfPriceListLineMarkingIsNeeded, added earlier for BuildAssetFilters) deliberately raises a separate, dedicated event OnCheckIfPriceListLineMarkingIsNeededOnBeforeFindLines for its check pass so OnBuildAssetFiltersOnBeforeFindLines still fires exactly once per source. If a subscriber to OnBuildSourceFiltersOnBeforeFindLines does anything beyond idempotent filter narrowing (e.g. counts invocations, appends to an accumulator, or performs a side-effecting call), the doubled firing will silently change its behavior for any AppSource extension using this event -- an impact that would normally be major/blocker for a published extensibility contract, but is capped to minor here because it is not backed by a BCQuality knowledge article; consider promoting this to a knowledge-backed rule about not re-using a 'build' IntegrationEvent from a separate 'check-only' pass.

Recommendation:

• introduce a dedicated event (e.g. OnCheckSourceFiltersMarkingIsNeededOnBeforeFindLines) for the check pass, mirroring the asset implementation, so OnBuildSourceFiltersOnBeforeFindLines keeps firing once per source as it did before this change.

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

[alexei-dobriansky]

Agentic PR Review - Round 3

Recommendation: Request Changes

What this PR does

Since round 2, the PR adds SetLoadFields("Price List Code", "Line No.") before the marking loops in the asset and source filter paths. This reduces the fields loaded while marking, but it does not avoid the MarkedOnly path when more than one source tuple has matching lines.

The linked bug is specifically about large Customer Card datasets. That page can include customer-specific, price group, discount group, and All Customers sources. If more than one of those sources has records, the current code still materializes and marks the large result set, so the performance fix is still not proven for the real scenario.

This is also still a duplicate-PR situation for AB#641061: PR 9219 is closed, but PR 9265 and PR 9269 are both open. One of PR 9265/9269 should be closed.

Status of previous suggestions

ID	Title	Status	Author response
S1	Preserve exact source tuples	Addressed	Still addressed. The current code keeps per-tuple filters or uses marking instead of independent OR filters.
S2	Keep event-added filters	Addressed	Still addressed for filter preservation. The final pass still applies subscriber-added filters before finding lines.
S3	Strengthen regression and performance coverage	Not addressed	No test changes were pushed since round 2. The existing test still uses only 10-20 All Customers lines and no mixed-source or performance-size data set.
S4	Avoid marking mixed source result sets	Not addressed	Commit d526b80 only reduces loaded fields before marking. It still uses FindSet, Mark(true), and MarkedOnly(true) when more than one source result set has lines.

New observations (commits since round 2)

S3 - Strengthen regression and performance coverage
No test changes were added since round 2. The current test still uses only 10-20 All Customers lines, so it does not cover mixed sources or prove the large-data path.

S4 - Avoid marking mixed source result sets
The new commit only narrows fields loaded during marking. Customer Card can still produce multiple source sets, and that still loops and marks the large result set.

Risk assessment and necessity

Risk: This code is shared by Customer, Vendor, Job, Contact, Campaign, and related price/discount review flows. Loading fewer fields may help memory and SQL payload, but it does not prove that the 61k+ mixed-source scenario loads in the expected time.

Necessity: AB#641061 is clear and important because opening Sales Prices from Customer Card can take about 220 seconds. A fix is needed, but it must cover the mixed-source case or include a performance guard. Also, PR 9265 is still open for the same work item, so one duplicate PR should be closed before merge.

---

[AI-PR-REVIEW] version=1 system=github pr=9269 round=3 by=alexei-dobriansky sha=d526b802b9ae6900ec41ca325c45268db0ba0d16

[alexei-dobriansky]

Agentic PR Review - Round 4

Recommendation: Request Changes

What this PR does

Since round 3, BuildSourceFilters was rewritten. It no longer marks records. It loops the price sources, applies OnBuildSourceFiltersOnBeforeFindLines, and counts how many source tuples have lines. It then builds the final filter from the count: 0 uses marking, 1 sets the exact tuple range, and more than one sets three separate OR filters on Source Type, Parent Source No., and Source No..

Removing marking for the multi-source case helps performance, but it brings back the tuple-mixing problem from round 1. The three OR filters are independent, so the final query is Source Type IN (...) AND Parent Source No. IN (...) AND Source No. IN (...). This can match a source tuple that was never in the price source list. This is the exact scenario the bug is about, because a Customer Card usually has several sources (customer, price group, discount group, All Customers).

Status of previous suggestions

ID	Title	Status	Author response
S1	Preserve exact source tuples	Not addressed (reopened)	The multi-source else branch again uses three independent OR filters. This can match combinations that were never a real source tuple, the same problem fixed in round 2. Keep the result tuple-safe: OR only the field(s) that actually vary and keep the others as exact ranges, or fall back to marking with only the key fields loaded for the multi-source case.
S2	Keep event-added filters	Not addressed (reopened)	After the loop, ClearSourceFilters runs and the final filter is rebuilt only from PriceSource values. Filters added by OnBuildSourceFiltersOnBeforeFindLines are no longer in the final result handed to the caller.
S3	Strengthen regression and performance coverage	Not addressed	No test changes since round 2. Only the codeunit changed. The test still uses a small All Customers set and no mixed-source or large-data case.
S4	Avoid marking mixed source result sets	Addressed	The multi-source path no longer uses FindSet + Mark + MarkedOnly. It now uses server-side filters instead.

New observations (commits since round 3)

None beyond the reopened items above. The rewrite fixed S4 but reintroduced S1 and S2.

Risk assessment and necessity

Risk: The mixed-source path can now return price list lines for sources the caller did not ask for. This code is shared by Customer, Vendor, Job, Contact, and Campaign price/discount views. Dropping event-added filters can also show more lines than a subscriber intended. Both are correctness risks on a widely used page.

Necessity: AB#641061 is clear and important, because opening Sales Prices from Customer Card can take about 220 seconds. A fast fix is needed, but it must keep the exact source tuples and the subscriber-added filters. The duplicate PR 9265 is now closed, so that earlier concern is resolved.

---

[AI-PR-REVIEW] version=1 system=github pr=9269 round=4 by=alexei-dobriansky at=2026-07-10T05:41:43Z lastSha=9c7a5c66a048bab2063b6ca9349fcbe7307235be suggestions=S1:notaddressed,S2:notaddressed,S3:notaddressed,S4:addressed parentRound=3