Address review feedback: name AS0025, AS0063 and AS0077 for the var and rename cases instead of claiming them in the breaking-changes article, and point at events/treat-local-and-internal-events-as-subscriber-contracts which already owns them.
The first revision justified the carve-out with leading-prefix binding and limited it to parameters appended at the end. Verified against shipping BCApps code that AL binds subscriber parameters by name, not position, so an added parameter is additive wherever it is placed. Also corrects the cross-reference to events/adding-a-parameter-to-an-event-is-not-a-breaking-change, which already states this rule, and drops reordering from the list of edits that break binding.
The article's detection guidance flags 'a parameter added' on any shipped procedure. Applied to an event publisher that is bound to rather than called, this produced a false positive on BCApps PR 10278, where a trailing var parameter was appended to the existing IntegrationEvent OnAfterOpenForRecRef and the developer twice replied that adding a parameter to an existing integration event is not a breaking change.
It also contradicted events/add-new-event-parameters-at-the-end, which already states that existing subscribers still bind to the leading parameters. Scope the rule to called procedures, carve out appended event parameters as additive, and keep every other event signature edit - removal, reorder, retype, var flip - in scope. Point the IsHandled case at events/do-not-add-ishandled-to-an-existing-event, which owns that semantic concern.
Two knowledge false-positive guards addressing bug 642303 (agent FPs on BCApps PR #9607):
- breaking-changes: relocating a field to a tableextension in the same app under the same field ID/name is a relocation, not a deletion/rename; the field still resolves on the table, so it must not be flagged as a deleted shipped field or require ObsoleteState staging. Scoped to the contract axis; silent on data migration.
- events: adding a parameter to an event publisher does not break existing subscribers (subscribers bind by name and match a subset), so the addition itself must not be reported as a breaking signature change.
* 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>
* Complete AL review knowledge readiness
Fill telemetry and Query coverage, strengthen thin review domains, correct audited content defects, and add deterministic cheap-model evaluation and reference-integrity safeguards.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27
* Generalize review fixture discovery
Derive smoke cases from the leaf, domain, and paired-sample conventions so new leaves require no scoring-contract changes. Keep only exceptional selection/context overrides and fail when retrieval metadata cannot rank the selected article.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27
* Preserve published field IDs in sample
Keep the existing Email and Contact Email field IDs unchanged, clarify that the sample represents an independent baseline, and use a local breaking-change rule for the generic smoke evaluation.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27
* Clarify published field identity rules
State explicitly that a published field keeps its ID, name, and type while a replacement is added as a separate field under an unused ID.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27
* Align field obsoletion sample baselines
Use Email field ID 3 as the shared baseline so the bad example demonstrates a same-ID rename while the good example retains the original field and adds a separate replacement.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9825b012-e653-496a-9310-c1f4b6f8ac27
---------
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Seed a new breaking-changes (AL API stability) knowledge domain with six
articles plus good/bad AL samples, a new al-breaking-changes-review leaf
skill, and minimal wiring into al-code-review and the README.
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
