Retire deterministic compiler and analyzer duplicates, remove their review routing, and clarify the admission test for contextual analyzer knowledge.
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* feat(community/agents): add AL agent quality guidance
- add 20 agent knowledge rules with good and bad AL samples
- clarify setup dialog shape, temporary persistence, permissions, profiles, instructions, capability registration, and interface wiring
- add the community-owned AL agents review skill
- make review fixture discovery layer-aware with custom, community, and Microsoft precedence
- document layer-aware evaluation behavior
* fix(community/agents): align setup and permission samples
- mark agent setup pages as non-extensible where required
- narrow the agent profile by hiding an unrelated sales-order field
- define a dedicated read-only permission set for the sales review agent
- assign AL-defined permission sets with system scope and the owning app ID
- clarify the permission scope guidance for default access controls
* Address agent review feedback
The narrowed UI-handler guidance still stated the execution rule without the
qualifier the linked Microsoft reference uses. The article said every listed
handler must execute at least once, and the testing leaf skill asked for
`[HandlerFunctions(...)]` to match the invoked handlers exactly. The reference
says every *nonoptional* listed handler must execute, and that send-notification
and recall-notification handlers can be optional. As written, an agent could
flag a deliberately unused optional notification handler.
The discriminator is narrower than the handler type. Both
`[SendNotificationHandler([HandlerIsOptional: Boolean])]` and
`[RecallNotificationHandler([HandlerIsOptional: Boolean])]` take an explicit
optionality argument, so `[SendNotificationHandler(true)]` is exempt while the
same attribute written without the argument stays nonoptional like every other
handler type. Keying the exemption on the argument rather than the type keeps it
checkable from the diff and avoids the opposite false positive, where an agent
stops flagging genuinely nonoptional notification handlers.
Changes:
- The article now states the nonoptional qualifier, explains that optionality is
declared rather than inferred, and adds an explicit do-not-flag clause. That
clause also forbids proposing removal, because the listed entry is what keeps
the test passing on the runs where the notification does fire.
- The testing leaf skill carries the same boundary in its `ui-handlers-in-tests`
cue, and its mechanical-fix list no longer allows removing a listed optional
notification handler as a one-click suggestion.
- `SendNotificationHandler` and `RecallNotificationHandler` were missing from the
skill's testing token list, so notification handlers were not reliably
surfaced to the relevance step at all. Both are now listed.
- The good sample gains a test that lists an unreached
`[SendNotificationHandler(true)]`; the bad sample gains the mirror image, an
unreached `[SendNotificationHandler]` with no optionality argument. The pair
differs only by that argument, which is the point.
- `evaluation/review-fixtures.json` pins the testing domain to
`ui-handlers-in-tests` so the boundary is exercised: the good sample is the
clean control at `minimumCleanRate` 1.0 and the bad sample is the expected
finding. Keywords were retagged with `notification` and `optional-handler`.
validate_frontmatter.py reports 0 errors; Test-ReviewFixtures.ps1 passes with
32 cases across 16 leaf domains and resolves the testing fixture to this article.
The article was rewritten to say a reset is required only when the value can
carry over, and its H1 was updated to match, but three artefacts still carried
the old "always initialize to false" premise:
- The slug still read `initialize-ishandled-to-false-before-publishing`, which
contradicts the body. The slug is not cosmetic: Build-KnowledgeIndex.ps1
ranks candidates on keywords, frontmatter dimensions, domain, path and title,
so a stale path pushes selection back toward the behaviour this change
narrows. Renamed to `reset-ishandled-only-when-the-value-can-carry-over`,
following the existing precedent for conditional slugs such as
`unreleased-symbol-change-is-not-a-breaking-change`.
- Keywords still listed `initialization` and `deterministic` and omitted
`false-positive`, the tag this repository uses for suppression articles.
Replaced with `carry-over` and `loop-iteration` and added `false-positive`.
- The good sample demonstrated only the "prefer separate fresh locals" clause
and contained no reset at all, so the article's headline case had no positive
example. It was also asymmetric with the bad sample, which gained a loop
procedure showing a local that carries `true` into the next iteration.
Added the matching loop procedure to the good sample: a local declared outside
the loop is reset at the top of each iteration. That case cannot be solved by
introducing another local, because AL has no block scope, so it is the only
shape that demonstrates the reset the article still requires. It also gives the
engine the correct `suggested-code` shape for the loop finding; without it the
one-click fix adapted from the good sample would propose splitting the variable
rather than adding one line.
Also renamed the sample codeunits from "IsHandled Init ..." to
"IsHandled Carry Over ...", and updated the two references to the old slug:
the events leaf skill cue and the events pin in evaluation/review-fixtures.json.
validate_frontmatter.py reports 0 errors; Test-ReviewFixtures.ps1 passes with
32 cases across 16 leaf domains and resolves the events fixture to the renamed
article.
* 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>
