bcquality/microsoft/knowledge/performance/calcfields-in-both-getrecord-triggers-is-not-redundant.md
Wenjie Fan 1bf5a3b276
Add precision guards for systematic agent false-positive patterns (#112)
* Add precision guards for systematic agent false-positive patterns

Encodes reviewer-confirmed FP guards from the online eval: tooltip-inherited, page-trigger default return, drill-down filter not visible in diff, dual-trigger CalcFields (do.md); and a released-baseline precondition for breaking-change/upgrade findings on never-shipped symbols.

* Scope suggested-code and location to exactly the changed lines

Addresses reviewer-reported misplaced suggestions from the eval: insert-only-property emitting the whole field, single-statement rewrites anchored on the procedure name, and reductive multi-line collapses. The skill now emits a location range that matches precisely the rewritten lines.

* Correct suggested-code scoping guidance to match one-click anchor mechanics

A lone inserted line matches no existing file line and cannot be anchored; bracket the new line with one adjacent unchanged line instead. Reductive collapses omit suggested-code and fall back to a manual snippet.

* Move BC-specific FP guards out of do.md into leaf skills

do.md is the stable action-skill template and must stay domain-agnostic (JesperSchulz review). Relocate the four known false-positive patterns to their domain leaves: ToolTip-inheritance to al-ui-review; drill-down/lookup filtering and CalcFields lifecycle to al-performance-review; page-trigger exit(true) semantics to al-error-handling-review.

* Move suggested-code line-scoping guidance out of do.md into al-code-review

do.md must not carry instructions for how the review skill behaves (JesperSchulz review). Relocate the location/suggested-code precise-span rules to al-code-review's existing Suggested-code guidance section. do.md is now unchanged vs main.

* Move false-positive guards from skills into knowledge files

Keep review skills slim (finders/appliers). The FP guards and released-baseline preconditions previously embedded in leaf skills become negative-clarification knowledge articles in their domains, and the agent-findings policy edits to al-ui/al-privacy are reverted to main. Adds 6 knowledge files: error-handling (page-boolean-triggers-default-to-true), ui (bound-page-field-inherits-source-field-tooltip), performance (calcfields-in-both-getrecord-triggers-is-not-redundant, page-effective-filter-may-live-outside-the-diff), breaking-changes (unreleased-symbol-change-is-not-a-breaking-change), upgrade (unreleased-schema-change-needs-no-upgrade-path).

* Revert branch's suggested-code scoping addition in al-code-review

The three location-span shapes added to al-code-review are output-format mechanics, not domain knowledge: one-click span correctness is the engine's job (Resolve-SuggestionPlacement) and do.md already owns the suggested-code/location contract. The AL concerns the examples illustrate are already covered by existing knowledge (use-isempty-for-existence-check, data-classification-required-on-pii-fields, no-space-before-method-parenthesis). Restores al-code-review to main; the branch now adds only the 6 knowledge files.

* Restore al-ui/al-privacy review skills to base (zero diff in PR)

These two leaf skills carried an accidental net change against the PR merge-base because an earlier revert used the current origin/main (post-#110) instead of the branch base (pre-#110). Restoring them to the merge-base version removes them from the PR diff entirely. Three-way merge still preserves main's #110 suppression.

---------

Co-authored-by: wenjiefan <wenjiefan@microsoft.com>
2026-07-17 14:17:04 +02:00

1.4 KiB

bc-version domain keywords technologies countries application-area
all
performance
calcfields
onaftergetrecord
onaftergetcurrrecord
page-lifecycle
flowfield
false-positive
al
w1
all

CalcFields in both OnAfterGetRecord and OnAfterGetCurrRecord is not redundant

Description

OnAfterGetRecord fires once per row as the page loads records into the view; OnAfterGetCurrRecord fires when a record becomes the active/current record. Calling CalcFields in both triggers is not duplicate or redundant work: the two triggers run at different points in the page lifecycle and serve different purposes — populating FlowFields for every displayed row versus refreshing them for the record the user has selected. The same CalcFields call appearing in both places is an intentional pattern, not copy-paste waste.

Best Practice

Do not flag CalcFields appearing in both OnAfterGetRecord and OnAfterGetCurrRecord as duplicate, redundant, or removable. Treat each trigger's CalcFields on its own lifecycle merits.

Anti Pattern

Recommending that a developer delete one of the two CalcFields calls because "the field is already calculated in the other trigger". The genuine per-row FlowField cost is addressed by the separate guidance on FlowField calculation in loops and on hidden FlowFields; it is not addressed by removing a lifecycle-correct CalcFields.