Address agent review feedback

This commit is contained in:
Stefano Demiliani 2026-08-27 16:33:40 +02:00
parent 38d341eabb
commit e98fb8ec38
11 changed files with 63 additions and 20 deletions

View file

@ -90,7 +90,7 @@ Code examples belong in separate files, not in the knowledge file itself. Knowle
## Scope ## Scope
The current curated corpus is focused on **technical AL code review**: AppSource and compatibility, data modeling, error handling, events, interfaces, performance, privacy, Query objects, security, style, telemetry, testing, UI, upgrade, and web services. These are the domains backed by knowledge files and registered review leaves today. The current curated corpus is focused on **technical AL code review**: Agents, AppSource and compatibility, data modeling, error handling, events, interfaces, performance, privacy, Query objects, security, style, telemetry, testing, UI, upgrade, and web services. These are the domains backed by knowledge files and registered review leaves today.
Business Central functional domains (Finance, Supply Chain Management, Manufacturing, Jobs, Warehousing, Service), PowerShell, pipelines, and Power Platform remain valid future repository scope, but they are **not current coverage claims** until corresponding knowledge and action skills exist. Consumers should derive supported review scope from the live knowledge index and dispatched skills, not from roadmap breadth. Business Central functional domains (Finance, Supply Chain Management, Manufacturing, Jobs, Warehousing, Service), PowerShell, pipelines, and Power Platform remain valid future repository scope, but they are **not current coverage claims** until corresponding knowledge and action skills exist. Consumers should derive supported review scope from the live knowledge index and dispatched skills, not from roadmap breadth.

View file

@ -15,8 +15,7 @@ Instance setup is not a Card or StandardDialog. The toolkit expects `PageType =
## Best Practice ## Best Practice
Declare `PageType = ConfigurationDialog`, host `part(...; "Agent Setup Part")`, and put agent-specific fields in another group. Keep system OK/Cancel. Use a temporary source record and defer persistence until Update, as described in `agent-setup-source-table-is-temporary.md`. Declare `PageType = ConfigurationDialog`, host `part(...; "Agent Setup Part")`, and put agent-specific fields in another group. Keep system OK/Cancel. Use a temporary source record and defer persistence until Update, as described in `agent-setup-source-table-is-temporary.md`. Following Microsoft's agent setup samples, set `Extensible = false`.
The Extensible property of the page must be set to false.
See sample: `agent-setup-page-is-configuration-dialog.good.al`. See sample: `agent-setup-page-is-configuration-dialog.good.al`.

View file

@ -3,14 +3,29 @@ codeunit 50100 "Sales Review Agent Task"
procedure AnalyzeAgentTaskMessage(AgentTaskMessage: Record "Agent Task Message"; var Annotations: Record "Agent Annotation") procedure AnalyzeAgentTaskMessage(AgentTaskMessage: Record "Agent Task Message"; var Annotations: Record "Agent Annotation")
var var
AgentMessage: Codeunit "Agent Message"; AgentMessage: Codeunit "Agent Message";
EmptyMessageMsg: Label 'Message is empty.';
EmptyMessageDetailsTxt: Label 'Provide a sales order task before running the agent.';
NotRelevantMsg: Label 'Message is not a sales order task.'; NotRelevantMsg: Label 'Message is not a sales order task.';
NotRelevantDetailsTxt: Label 'Provide a message related to sales order review.'; NotRelevantDetailsTxt: Label 'Provide a message related to sales order review.';
MessageText: Text;
begin begin
if AgentTaskMessage.Type = AgentTaskMessage.Type::Output then begin if AgentTaskMessage.Type = AgentTaskMessage.Type::Output then begin
AgentMessage.UpdateText(AgentTaskMessage, AgentMessage.GetText(AgentTaskMessage) + '\n\nWritten with the help of AI'); AgentMessage.UpdateText(AgentTaskMessage, AgentMessage.GetText(AgentTaskMessage) + #13#10 + #13#10 + 'Written with the help of AI');
exit; exit;
end; end;
if not IsRelevant(AgentMessage.GetText(AgentTaskMessage)) then begin
MessageText := AgentMessage.GetText(AgentTaskMessage);
if MessageText = '' then begin
Clear(Annotations);
Annotations.Code := 'MESSAGE001';
Annotations.Severity := Annotations.Severity::Error;
Annotations.Message := EmptyMessageMsg;
Annotations.Details := EmptyMessageDetailsTxt;
Annotations.Insert();
exit;
end;
if not IsRelevant(MessageText) then begin
Clear(Annotations);
Annotations.Code := 'RELEVANCE001'; Annotations.Code := 'RELEVANCE001';
Annotations.Severity := Annotations.Severity::Warning; Annotations.Severity := Annotations.Severity::Warning;
Annotations.Message := NotRelevantMsg; Annotations.Message := NotRelevantMsg;
@ -21,6 +36,6 @@ codeunit 50100 "Sales Review Agent Task"
local procedure IsRelevant(MessageText: Text): Boolean local procedure IsRelevant(MessageText: Text): Boolean
begin begin
exit(MessageText <> ''); exit(StrPos(LowerCase(MessageText), 'sales order') > 0);
end; end;
} }

View file

@ -1,3 +1,16 @@
codeunit 50101 "Sales Review Agent Subscribers"
{
Access = Internal;
EventSubscriberInstance = Manual;
SingleInstance = true;
[EventSubscriber(ObjectType::Table, Database::"Sales Header", OnAfterInsertEvent, '', false, false)]
local procedure OnAfterInsertSalesHeader(var Rec: Record "Sales Header")
begin
Message('Keep going, agent.');
end;
}
codeunit 50102 "Agent Session Events" codeunit 50102 "Agent Session Events"
{ {
Access = Internal; Access = Internal;
@ -6,7 +19,7 @@ codeunit 50102 "Agent Session Events"
InherentPermissions = X; InherentPermissions = X;
var var
GlobalAgentEvents: Codeunit "Sales Review Agent Events"; GlobalAgentSubscribers: Codeunit "Sales Review Agent Subscribers";
[EventSubscriber(ObjectType::Codeunit, Codeunit::"System Initialization", OnAfterInitialization, '', false, false)] [EventSubscriber(ObjectType::Codeunit, Codeunit::"System Initialization", OnAfterInitialization, '', false, false)]
local procedure RegisterSubscribersOnAfterInitialization() local procedure RegisterSubscribersOnAfterInitialization()
@ -16,6 +29,6 @@ codeunit 50102 "Agent Session Events"
begin begin
if not AgentSession.IsAgentSession(AgentMetadataProvider) then if not AgentSession.IsAgentSession(AgentMetadataProvider) then
exit; exit;
if BindSubscription(GlobalAgentEvents) then; if BindSubscription(GlobalAgentSubscribers) then;
end; end;
} }

View file

@ -21,17 +21,15 @@ page 50100 "Sales Review Agent Setup"
trigger OnQueryClosePage(CloseAction: Action): Boolean trigger OnQueryClosePage(CloseAction: Action): Boolean
var var
Agent: Codeunit Agent; Agent: Codeunit Agent;
TempAgentAccessControl: Record "Agent Access Control" temporary; AgentSetup: Codeunit "Agent Setup";
TempAgentSetupBuffer: Record "Agent Setup Buffer" temporary;
AgentUserSecurityId: Guid; AgentUserSecurityId: Guid;
begin begin
if CloseAction = CloseAction::Cancel then if CloseAction = CloseAction::Cancel then
exit(true); exit(true);
AgentUserSecurityId := Agent.Create( CurrPage.AgentSetupPart.Page.GetAgentSetupBuffer(TempAgentSetupBuffer);
Enum::"Agent Metadata Provider"::"Sales Review Agent", AgentUserSecurityId := AgentSetup.SaveChanges(TempAgentSetupBuffer);
'SALESREVIEW',
'Sales Review Agent',
TempAgentAccessControl);
Agent.SetInstructions(AgentUserSecurityId, GetInstructions()); Agent.SetInstructions(AgentUserSecurityId, GetInstructions());
Agent.Activate(AgentUserSecurityId); Agent.Activate(AgentUserSecurityId);
exit(true); exit(true);

View file

@ -20,7 +20,7 @@ codeunit 50101 "Sales Review Agent Install"
CopilotCapability.RegisterCapability( CopilotCapability.RegisterCapability(
Enum::"Copilot Capability"::"Sales Review Agent", Enum::"Copilot Capability"::"Sales Review Agent",
Enum::"Copilot Availability"::Preview, Enum::"Copilot Availability"::Preview,
"Copilot Billing Type"::"Microsoft Billed", Enum::"Copilot Billing Type"::"Microsoft Billed",
LearnMoreUrlTxt); LearnMoreUrlTxt);
end; end;
} }

View file

@ -11,7 +11,7 @@ application-area: [all]
## Description ## Description
The agent runtime looks for specific phrases: ask for assistance, request a review, reply, write an email, memorize, set a field, use lookup, invoke an action. Ordinary English such as get a human to look or remember this is weaker. Outbound reply and email always require review; that is platform policy, not optional tone. The agent runtime looks for specific phrases: ask for assistance, request a review, reply, write an email, memorize, `Set field`, use lookup, `Invoke action`. Ordinary English such as get a human to look or remember this is weaker. Outbound reply and email always require review; that is platform policy, not optional tone.
## Best Practice ## Best Practice

View file

@ -4,7 +4,7 @@ enumextension 50100 "Sales Review Agent Metadata" extends "Agent Metadata Provid
{ {
Caption = 'Sales Review Agent'; Caption = 'Sales Review Agent';
Implementation = IAgentFactory = "Sales Review Agent Factory", Implementation = IAgentFactory = "Sales Review Agent Factory",
IAgentMetadata = "Sales Review Agent Metadata", IAgentMetadata = "Sales Review Agent Meta. Impl.",
IAgentTaskExecution = "Sales Review Agent Task"; IAgentTaskExecution = "Sales Review Agent Task";
} }
} }

View file

@ -14,7 +14,9 @@ application-area: [all]
# AL agents review # AL agents review
Reviews AL source changes against the `agents` knowledge domain in BCQuality and emits a findings report. This is a leaf action skill: it invokes no sub-skills. Reviews AL source changes against the `agents` knowledge domain in BCQuality and emits a findings report. This is a leaf action skill: it invokes no sub-skills. It is not one of the skills composed by `al-code-review`; Entry discovers and dispatches it as a top-level peer, so it produces an independent findings report.
Agent findings apply to AL files that implement or invoke Agent SDK surfaces, including agent interfaces, setup, creation, task execution, capability registration, profiles, access controls, instructions, and session-bound subscribers. Return `not-applicable` when the diff contains no AL changes or no Agent SDK implementation or usage.
An orchestrator invokes this skill with either a `pr-diff` or a `file-path`. The skill produces one JSON document conforming to the DO output contract. An orchestrator invokes this skill with either a `pr-diff` or a `file-path`. The skill produces one JSON document conforming to the DO output contract.

View file

@ -4,6 +4,9 @@
"minimumExpectedRecall": 1.0, "minimumExpectedRecall": 1.0,
"minimumCleanRate": 1.0, "minimumCleanRate": 1.0,
"overrides": { "overrides": {
"agents": {
"article": "wire-all-three-agent-interfaces"
},
"appsource": { "appsource": {
"context": "AppSourceCop mandatoryAffixes is configured to ABC." "context": "AppSourceCop mandatoryAffixes is configured to ABC."
}, },

View file

@ -203,7 +203,20 @@ foreach ($domain in $leafDomains) {
} }
$selectedArticle = $articles | Where-Object BaseName -eq $articleName | Select-Object -First 1 $selectedArticle = $articles | Where-Object BaseName -eq $articleName | Select-Object -First 1
if (-not $selectedArticle) { if (-not $selectedArticle) {
$problems.Add("${domain}: override article does not exist: $articleName.md") | Out-Null $articleExists = @(
foreach ($layer in $layers) {
$articleFile = Join-Path $Root "$($layer.Name)/knowledge/$domain/$articleName.md"
if (Test-Path -LiteralPath $articleFile -PathType Leaf) {
$articleFile
}
}
).Count -gt 0
if ($articleExists) {
$problems.Add("${domain}: override article does not have both .good.al and .bad.al companion samples: $articleName.md") | Out-Null
} else {
$problems.Add("${domain}: override article does not exist: $articleName.md") | Out-Null
}
continue
} }
} else { } else {
$selectedArticle = $articles | Select-Object -First 1 $selectedArticle = $articles | Select-Object -First 1
@ -214,7 +227,7 @@ foreach ($domain in $leafDomains) {
} }
$articlePath = [string]$selectedArticle.ArticlePath $articlePath = [string]$selectedArticle.ArticlePath
$sampleDirectory = Split-Path -Parent $articlePath $sampleDirectory = (Split-Path -Parent $articlePath).Replace('\', '/')
$context = if ($override -and ($override.PSObject.Properties.Name -contains 'context')) { $context = if ($override -and ($override.PSObject.Properties.Name -contains 'context')) {
[string]$override.context [string]$override.context
} else { } else {