From 8aa54bafd6fea4fea146de416594816009a4e766 Mon Sep 17 00:00:00 2001 From: Javier Armesto <25327760+javiarmesto@users.noreply.github.com> Date: Fri, 16 Oct 2026 16:24:47 +0200 Subject: [PATCH 1/3] knowledge(web-services): describe API pages and queries with AboutText for MCP tools The Business Central MCP server takes the tool description from AboutText, then EntitySetCaption, then EntityName. API pages without AboutText reach agents as a bare entity name. LLMs treat AboutText as a UI teaching-tip property and omit it on API objects; no analyzer flags it. Observed in 13 of 22 generated solutions of a read-only SalesMargin query API. Co-Authored-By: Claude Opus 5.5 --- ...be-api-pages-for-mcp-with-abouttext.bad.al | 46 ++++++++++++++++++ ...e-api-pages-for-mcp-with-abouttext.good.al | 48 +++++++++++++++++++ ...scribe-api-pages-for-mcp-with-abouttext.md | 31 ++++++++++++ 3 files changed, 125 insertions(+) create mode 100644 microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.bad.al create mode 100644 microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.good.al create mode 100644 microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.md diff --git a/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.bad.al b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.bad.al new file mode 100644 index 00000000..f580053e --- /dev/null +++ b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.bad.al @@ -0,0 +1,46 @@ +page 50381 "WS Margin Line API Bad" +{ + PageType = API; + Caption = 'marginLine'; + APIPublisher = 'contoso'; + APIGroup = 'salesMargin'; + APIVersion = 'v1.0'; + EntityName = 'marginLine'; + EntitySetName = 'marginLines'; + EntityCaption = 'Margin Line'; + EntitySetCaption = 'Margin Lines'; + ODataKeyFields = SystemId; + SourceTable = "Sales Line"; + SourceTableView = where(Type = const(Item)); + Editable = false; + InsertAllowed = false; + ModifyAllowed = false; + DeleteAllowed = false; + + layout + { + area(content) + { + repeater(records) + { + field(id; Rec.SystemId) + { + Caption = 'id'; + Editable = false; + } + field(documentNumber; Rec."Document No.") + { + Caption = 'documentNumber'; + } + field(itemNumber; Rec."No.") + { + Caption = 'itemNumber'; + } + field(lineAmount; Rec."Line Amount") + { + Caption = 'lineAmount'; + } + } + } + } +} diff --git a/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.good.al b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.good.al new file mode 100644 index 00000000..968e8fb2 --- /dev/null +++ b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.good.al @@ -0,0 +1,48 @@ +page 50380 "WS Margin Line API Good" +{ + PageType = API; + Caption = 'marginLine'; + APIPublisher = 'contoso'; + APIGroup = 'salesMargin'; + APIVersion = 'v1.0'; + EntityName = 'marginLine'; + EntitySetName = 'marginLines'; + EntityCaption = 'Margin Line'; + EntitySetCaption = 'Margin Lines'; + AboutTitle = 'Sales line margins'; + AboutText = 'One item line of an open sales quote, order or invoice, with its document number, item and line amount. Read-only; credit memos and non-item lines are not included.'; + ODataKeyFields = SystemId; + SourceTable = "Sales Line"; + SourceTableView = where(Type = const(Item)); + Editable = false; + InsertAllowed = false; + ModifyAllowed = false; + DeleteAllowed = false; + + layout + { + area(content) + { + repeater(records) + { + field(id; Rec.SystemId) + { + Caption = 'id'; + Editable = false; + } + field(documentNumber; Rec."Document No.") + { + Caption = 'documentNumber'; + } + field(itemNumber; Rec."No.") + { + Caption = 'itemNumber'; + } + field(lineAmount; Rec."Line Amount") + { + Caption = 'lineAmount'; + } + } + } + } +} diff --git a/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.md b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.md new file mode 100644 index 00000000..8323857c --- /dev/null +++ b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.md @@ -0,0 +1,31 @@ +--- +bc-version: [21..] +domain: web-services +keywords: [api-page, api-query, abouttext, abouttitle, mcp, tool-description, agents, entitysetcaption] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Describe API pages and queries with `AboutText` for MCP tools + +## Description + +The Business Central MCP server exposes API pages and API queries as tools, and an agent picks a tool by reading its description. The server builds that description from `AboutText` first, then falls back to `EntitySetCaption`, then to the camelCase `EntityName`. An API page without `AboutText` therefore reaches the agent as a bare plural noun, with nothing about what the entity holds, which rows it returns or which values are computed. LLMs treat `AboutText` and `AboutTitle` as UI teaching-tip properties and leave them off API objects, because API pages are never shown in the client. The compiler and the analyzers do not flag the omission. + +## Best Practice + +On every `PageType = API` page and `QueryType = API` query that may be exposed as an MCP tool, set `AboutText` to one or two sentences. Say what one record is, which records the endpoint returns (filters, document types, scope) and any value that is computed at read time instead of stored. Set `AboutTitle` as well, since the platform expects the pair. Write it for a reader who only sees the tool list, and keep it consistent with `EntityCaption` and `EntitySetCaption`. + +See sample: [`describe-api-pages-for-mcp-with-abouttext.good.al`](describe-api-pages-for-mcp-with-abouttext.good.al). + +## Anti Pattern + +An API page or API query with no `AboutText`, or with a value that only repeats the entity name, such as "Margin lines". The agent cannot tell the endpoint apart from similar ones, or know that a field like a margin percentage is recalculated live. It then picks the wrong tool or misreads the data. The detection signal: a `PageType = API` page or `QueryType = API` query that does not set `AboutText`. + +See sample: [`describe-api-pages-for-mcp-with-abouttext.bad.al`](describe-api-pages-for-mcp-with-abouttext.bad.al). + +## References + +- [AboutText property](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/properties/devenv-abouttext-property) +- [API page type: describe the API for MCP tools](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/devenv-api-pagetype#describe-the-api-for-mcp-tools) From 38bb24502a4a2806800f12d2d99691e7883b61f6 Mon Sep 17 00:00:00 2001 From: Javier Armesto <25327760+javiarmesto@users.noreply.github.com> Date: Fri, 16 Oct 2026 17:51:20 +0200 Subject: [PATCH 2/3] knowledge(web-services): scope MCP descriptions and register API examples --- evaluation/review-fixtures.json | 3 +- ...be-api-pages-for-mcp-with-abouttext.bad.al | 27 ++++++++++++++++- ...e-api-pages-for-mcp-with-abouttext.good.al | 30 +++++++++++++++++-- ...scribe-api-pages-for-mcp-with-abouttext.md | 22 +++++++++----- .../skills/review/al-web-services-review.md | 5 ++-- 5 files changed, 72 insertions(+), 15 deletions(-) diff --git a/evaluation/review-fixtures.json b/evaluation/review-fixtures.json index 68904529..dfe6dd6d 100644 --- a/evaluation/review-fixtures.json +++ b/evaluation/review-fixtures.json @@ -197,7 +197,8 @@ "check-http-status-before-consuming-response-body", "check-json-null-before-converting-values", "format-exchanged-values-with-standard-format-9", - "api-page-least-privilege-write-access" + "api-page-least-privilege-write-access", + "describe-api-pages-for-mcp-with-abouttext" ] } } diff --git a/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.bad.al b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.bad.al index f580053e..3eedb62d 100644 --- a/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.bad.al +++ b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.bad.al @@ -1,3 +1,4 @@ +// Intended for exposure through the Business Central online MCP server. page 50381 "WS Margin Line API Bad" { PageType = API; @@ -11,7 +12,7 @@ page 50381 "WS Margin Line API Bad" EntitySetCaption = 'Margin Lines'; ODataKeyFields = SystemId; SourceTable = "Sales Line"; - SourceTableView = where(Type = const(Item)); + SourceTableView = where("Document Type" = filter(Quote | Order | Invoice), Type = const(Item)); Editable = false; InsertAllowed = false; ModifyAllowed = false; @@ -44,3 +45,27 @@ page 50381 "WS Margin Line API Bad" } } } + +// This API query is also intended for exposure as an MCP tool. +query 50383 "WS Item Lines Query Bad" +{ + QueryType = API; + APIPublisher = 'contoso'; + APIGroup = 'salesMargin'; + APIVersion = 'v1.0'; + EntityName = 'itemSalesLine'; + EntitySetName = 'itemSalesLines'; + EntityCaption = 'Item Sales Line'; + EntitySetCaption = 'Item Sales Lines'; + // The entity caption omits the material document-type scope. + elements + { + dataitem(SalesLine; "Sales Line") + { + DataItemTableFilter = "Document Type" = filter(Quote | Order | Invoice), Type = const(Item); + column(id; SystemId) { } + column(documentNumber; "Document No.") { } + column(itemNumber; "No.") { } + } + } +} diff --git a/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.good.al b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.good.al index 968e8fb2..ad719a38 100644 --- a/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.good.al +++ b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.good.al @@ -1,3 +1,4 @@ +// Intended for exposure through the Business Central online MCP server. page 50380 "WS Margin Line API Good" { PageType = API; @@ -9,11 +10,10 @@ page 50380 "WS Margin Line API Good" EntitySetName = 'marginLines'; EntityCaption = 'Margin Line'; EntitySetCaption = 'Margin Lines'; - AboutTitle = 'Sales line margins'; - AboutText = 'One item line of an open sales quote, order or invoice, with its document number, item and line amount. Read-only; credit memos and non-item lines are not included.'; + AboutText = 'One item line of an unposted sales quote, order or invoice, with its document number, item and line amount. Read-only; credit memos and non-item lines are not included.'; ODataKeyFields = SystemId; SourceTable = "Sales Line"; - SourceTableView = where(Type = const(Item)); + SourceTableView = where("Document Type" = filter(Quote | Order | Invoice), Type = const(Item)); Editable = false; InsertAllowed = false; ModifyAllowed = false; @@ -46,3 +46,27 @@ page 50380 "WS Margin Line API Good" } } } + +// This API query is also intended for exposure as an MCP tool. +query 50382 "WS Item Lines Query Good" +{ + QueryType = API; + APIPublisher = 'contoso'; + APIGroup = 'salesMargin'; + APIVersion = 'v1.0'; + EntityName = 'itemSalesLine'; + EntitySetName = 'itemSalesLines'; + EntityCaption = 'Item Sales Line'; + EntitySetCaption = 'Item Sales Lines'; + AboutText = 'One item line of an unposted sales quote, order or invoice. Credit memos and non-item lines are excluded; returns the line ID, document number and item number.'; + elements + { + dataitem(SalesLine; "Sales Line") + { + DataItemTableFilter = "Document Type" = filter(Quote | Order | Invoice), Type = const(Item); + column(id; SystemId) { } + column(documentNumber; "Document No.") { } + column(itemNumber; "No.") { } + } + } +} diff --git a/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.md b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.md index 8323857c..d0b20c74 100644 --- a/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.md +++ b/microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.md @@ -1,31 +1,37 @@ --- -bc-version: [21..] +bc-version: [28..] domain: web-services -keywords: [api-page, api-query, abouttext, abouttitle, mcp, tool-description, agents, entitysetcaption] +keywords: [api-page, api-query, abouttext, mcp, tool-description, entitysetcaption, metadata] technologies: [al] countries: [w1] application-area: [all] --- -# Describe API pages and queries with `AboutText` for MCP tools +# Describe API objects intended for Business Central MCP tools ## Description -The Business Central MCP server exposes API pages and API queries as tools, and an agent picks a tool by reading its description. The server builds that description from `AboutText` first, then falls back to `EntitySetCaption`, then to the camelCase `EntityName`. An API page without `AboutText` therefore reaches the agent as a bare plural noun, with nothing about what the entity holds, which rows it returns or which values are computed. LLMs treat `AboutText` and `AboutTitle` as UI teaching-tip properties and leave them off API objects, because API pages are never shown in the client. The compiler and the analyzers do not flag the omission. +For API pages and API queries exposed through the Business Central online MCP server, the documented tool-description precedence is `AboutText`, then `EntitySetCaption`, then a camel-cased `EntityName`. Treating `AboutText` only as a UI teaching-tip property can leave an agent with a caption that omits essential scope or computed-value semantics. This guidance covers BC 28 and later online environments with the relevant MCP API-object support; the earlier MCP preview is outside its scope. ## Best Practice -On every `PageType = API` page and `QueryType = API` query that may be exposed as an MCP tool, set `AboutText` to one or two sentences. Say what one record is, which records the endpoint returns (filters, document types, scope) and any value that is computed at read time instead of stored. Set `AboutTitle` as well, since the platform expects the pair. Write it for a reader who only sees the tool list, and keep it consistent with `EntityCaption` and `EntitySetCaption`. +For an API object intended for MCP use, provide a tool description that explains what a row represents, material filters and any computed values that affect interpretation. Prefer `AboutText` when captions alone cannot convey those facts. Keep the description consistent with the actual filters, fields and allowed operations. Verify the description exposed by the configured MCP tool; metadata text does not enforce filters or permissions. + +The documented extraction order does not use `AboutTitle`. Do not require it solely for MCP: Microsoft ships API queries with `AboutText` and no `AboutTitle`, and the compiler accepts that form. Teaching tips have separate pairing requirements. Do not flag an absent `AboutText` when the effective fallback already describes the tool adequately, or when the object has no established MCP use. Other MCP servers and connectors can choose a different description source; check their contract before applying this rule. See sample: [`describe-api-pages-for-mcp-with-abouttext.good.al`](describe-api-pages-for-mcp-with-abouttext.good.al). ## Anti Pattern -An API page or API query with no `AboutText`, or with a value that only repeats the entity name, such as "Margin lines". The agent cannot tell the endpoint apart from similar ones, or know that a field like a margin percentage is recalculated live. It then picks the wrong tool or misreads the data. The detection signal: a `PageType = API` page or `QueryType = API` query that does not set `AboutText`. +An API object intended for an MCP tool exposes a description that omits material endpoint semantics, such as excluding credit memos or including only item lines, because it relies on a bare entity caption. Another defect is an `AboutText` that promises restrictions the endpoint does not implement. Missing `AboutText` alone is a discovery signal, not sufficient evidence of a finding; establish the intended MCP use and the deficient effective description. See sample: [`describe-api-pages-for-mcp-with-abouttext.bad.al`](describe-api-pages-for-mcp-with-abouttext.bad.al). ## References -- [AboutText property](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/properties/devenv-abouttext-property) -- [API page type: describe the API for MCP tools](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/devenv-api-pagetype#describe-the-api-for-mcp-tools) +- [API page type and MCP description precedence](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/devenv-api-pagetype#describe-the-api-for-mcp-tools) +- [API query type and MCP description precedence](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/devenv-api-querytype#describe-the-api-for-mcp-tools) +- [AboutText property and teaching-tip requirements](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/properties/devenv-abouttext-property) +- [BC 28 release and generally available enhanced MCP server](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/whatsnew/whatsnew-update-28-0) +- [API query support in the 2026 release wave 1 MCP enhancements](https://learn.microsoft.com/dynamics365/release-plan/2026wave1/smb/dynamics365-business-central/unlock-more-business-central-data-mcp-server-enhancements) +- [Microsoft API Finance query with AboutText and no AboutTitle](https://github.com/microsoft/BCApps/blob/e51c552efbeb6821db450b199aae6b015371866c/src/Apps/W1/APIReportsFinance/App/src/APIFinanceGLEntry.Query.al) diff --git a/microsoft/skills/review/al-web-services-review.md b/microsoft/skills/review/al-web-services-review.md index 6d7d71a0..3f86693a 100644 --- a/microsoft/skills/review/al-web-services-review.md +++ b/microsoft/skills/review/al-web-services-review.md @@ -38,14 +38,15 @@ Discard files that are not applicable. Retain conditionally applicable files (an Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against: - The changed AL object names and types — especially pages declared with `PageType = API`, API page `part` controls, queries declared with `QueryType = API`, and procedures that expose bound actions. -- The changed properties and triggers, weighted toward API page metadata (`APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SourceTable`, `SourceTableTemporary`), navigation metadata (`SubPageLink`, `Multiplicity`, and visible singleton or collection semantics), CRUD guards (`InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`), the `OnOpenPage` trigger, and `OnValidate` triggers on exposed fields. +- The changed properties and triggers, weighted toward API page metadata (`APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `AboutText`, `EntitySetCaption`, `ODataKeyFields`, `SourceTable`, `SourceTableTemporary`), navigation metadata (`SubPageLink`, `Multiplicity`, and visible singleton or collection semantics), CRUD guards (`InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`), the `OnOpenPage` trigger, and `OnValidate` triggers on exposed fields. - Outbound HTTP integration code that constructs or sends requests, captures a client method's optional Boolean result, checks an `HttpResponseMessage`, or reads and parses response content. - Integration code that converts values to or from exchanged text: `Format` or `Evaluate` on a request URL, body, or file line, and `JsonToken`/`JsonValue` conversions of payload properties. - Webhook subscriber handlers and subscription lifecycle code, especially code that creates or renews subscriptions, handles `validationToken`, schedules from `expirationDateTime`, or targets resources whose eligibility is visible in the diff. +- An API page or API query has established MCP use and its description metadata is added, changed, or omitted — `describe-api-pages-for-mcp-with-abouttext.md`. Read the article before evaluating the effective tool description. - An API page (`PageType = API`) is added or changed and its `InsertAllowed`/`ModifyAllowed`/`DeleteAllowed` properties are left at their default `true` (or explicitly set `true`) for an operation the endpoint's stated purpose does not need — `api-page-least-privilege-write-access.md`. The signal is an operation left enabled beyond what the endpoint's own described purpose requires, not the mere presence of these properties; a page that genuinely needs full CRUD and grants it deliberately is not a violation. - An API page (`PageType = API`) with `InsertAllowed = true` lists a field in `ODataKeyFields` and that same field control sets `Editable = false` — `api-page-key-fields-must-be-editable-on-insert.md`. A system-generated key such as `SystemId` marked read-only is not this anti-pattern. - An API page exposes a field via `Rec` directly, and that field is a stored value computed from other fields inside an `OnValidate` trigger rather than a FlowField recalculated in `OnAfterGetRecord` — `stored-derived-fields-must-not-be-exposed-directly.md`. Exposing a genuine FlowField, or a value already recalculated in `OnAfterGetRecord`, is not this anti-pattern. -- Tokens extracted from the diff that relate to API surface and behaviour (`PageType`, `QueryType`, `API`, `api-page`, `page-part`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SystemId`, `SubPageLink`, `subpagelink`, `Multiplicity`, `multiplicity`, `Many`, `ZeroOrOne`, `SourceTableTemporary`, `Job Queue Entry`, `webhook`, `webhookSupportedResources`, `webhook-supported-resources`, `subscriptions`, `notificationUrl`, `validationToken`, `validationtoken`, `expirationDateTime`, `expirationdatetime`, `ServiceEnabled`, `WebServiceActionContext`, `SetActionResponse`, `ReadIsolation`, `IsolationLevel`, `ReadCommitted`, `InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`, `SourceTable`, `HttpClient`, `HttpRequestMessage`, `HttpResponseMessage`, `Get`, `Post`, `Put`, `Delete`, `Send`, `IsSuccessStatusCode`, `HttpStatusCode`, `Content`, `ReadAs`, `JsonObject`, `JsonToken`, `JsonValue`, `AsValue`, `IsNull`, `SelectToken`, `Format`, `Evaluate`, `XmlDocument`). +- Tokens extracted from the diff that relate to API surface and behaviour (`PageType`, `QueryType`, `API`, `api-page`, `page-part`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `AboutText`, `EntitySetCaption`, `ODataKeyFields`, `SystemId`, `SubPageLink`, `subpagelink`, `Multiplicity`, `multiplicity`, `Many`, `ZeroOrOne`, `SourceTableTemporary`, `Job Queue Entry`, `webhook`, `webhookSupportedResources`, `webhook-supported-resources`, `subscriptions`, `notificationUrl`, `validationToken`, `validationtoken`, `expirationDateTime`, `expirationdatetime`, `ServiceEnabled`, `WebServiceActionContext`, `SetActionResponse`, `ReadIsolation`, `IsolationLevel`, `ReadCommitted`, `InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`, `SourceTable`, `HttpClient`, `HttpRequestMessage`, `HttpResponseMessage`, `Get`, `Post`, `Put`, `Delete`, `Send`, `IsSuccessStatusCode`, `HttpStatusCode`, `Content`, `ReadAs`, `JsonObject`, `JsonToken`, `JsonValue`, `AsValue`, `IsNull`, `SelectToken`, `Format`, `Evaluate`, `XmlDocument`). A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. From 9279168d397d968dd4185569dbb5e704b1783fed Mon Sep 17 00:00:00 2001 From: Javier Armesto <25327760+javiarmesto@users.noreply.github.com> Date: Tue, 20 Oct 2026 06:24:07 +0200 Subject: [PATCH 3/3] fix(evaluation): preserve MCP applicability in prepared fixtures --- .github/workflows/review-fixtures.yml | 4 + evaluation/README.md | 28 ++++++- evaluation/review-fixtures.json | 10 ++- tools/Test-ReviewFixtureContexts.ps1 | 101 ++++++++++++++++++++++++++ tools/Test-ReviewFixtures.ps1 | 67 ++++++++++++++++- 5 files changed, 204 insertions(+), 6 deletions(-) create mode 100644 tools/Test-ReviewFixtureContexts.ps1 diff --git a/.github/workflows/review-fixtures.yml b/.github/workflows/review-fixtures.yml index 0de9a022..fff80e83 100644 --- a/.github/workflows/review-fixtures.yml +++ b/.github/workflows/review-fixtures.yml @@ -35,3 +35,7 @@ jobs: -ChangedPathsFile "$env:RUNNER_TEMP/changed-paths.txt" ` -CoverageReportPath "$env:RUNNER_TEMP/review-coverage.json" Get-Content -LiteralPath "$env:RUNNER_TEMP/review-coverage.json" + + - name: Check context-dependent fixture regressions + shell: pwsh + run: ./tools/Test-ReviewFixtureContexts.ps1 -Root . diff --git a/evaluation/README.md b/evaluation/README.md index 5863fcc1..be009105 100644 --- a/evaluation/README.md +++ b/evaluation/README.md @@ -15,6 +15,32 @@ is mandatory. Model-facing preparation hashes case IDs, neutralizes `Good`/`Bad` object-name tokens, and removes full-line sample comments so neither the article slug, domain, nor expected outcome reveals the answer. +When applicability depends on deployment facts that AL cannot establish, use +`articleOverrides` within the domain override, keyed by a selected article's +exact slug (without `.md`). Each entry requires a non-empty `context` string. +It replaces the domain-wide context for that article's positive and clean pair +and survives in all prepared request formats. Include all necessary deployment +facts, but no expected finding, article path, fix, or assessment of code quality. +Other selected articles retain their existing domain context, if any. + +An article entry can also supply `cleanContexts`, an object mapping lowercase +hyphenated control names to non-empty context strings. Each control reuses the +article's `.bad.al` source with that context and expects no findings. Its stable +manifest ID is `--clean-`; model-facing IDs remain opaque. +Use this only when changing the deployment prerequisite makes the same source +legitimate. The MCP description article supplies Business Central online 28 and +MCP exposure to both normal cases, then adds a REST-only, non-MCP clean context. +This controls applicability without treating every API lacking `AboutText` as +a defect. Invalid or unselected article overrides fail validation. + +The context regression check runs the real preparer and scorer, verifies all +three request formats, context isolation, unchanged neutralization, mandatory +clean controls, and rejection of findings on the non-MCP input: + +```powershell +pwsh ./tools/Test-ReviewFixtureContexts.ps1 -Root . +``` + The SCM `articles` override deliberately selects every rule in the initial functional domain, producing nine positive cases and nine clean controls. Business context is executable: document/status `TestField` guards, @@ -64,7 +90,7 @@ pwsh ./tools/Test-ReviewFixtures.ps1 -Root . -ChangedPathsFile .changed-paths.tx 2. For a fast/small model, use one fresh invocation per `request-case-*.json`. Each request embeds the exact leaf instructions, that domain's candidate index rows with authoritative paths, and one opaque case. The model opens only matching articles and copies finding IDs from `candidateArticles[].path`. Save each response with the matching `result-case-*.json` name in the same directory. - `request-.json` files provide optional leaf batches containing every selected case for that domain and identify the selected layer-owned skill path; save those as `result-.json`. A normal convention-selected domain has one bad/good pair, while an `articles` override contributes one pair per listed article. Directory scoring prefers `result-case-*.json` when present and otherwise falls back to `result-*.json`. `review-request.json` is an optional all-domains stress test for larger models. Neither batch form is the preferred fast-model profile. + `request-.json` files provide optional leaf batches containing every selected case for that domain and identify the selected layer-owned skill path; save those as `result-.json`. A normal convention-selected domain has one bad/good pair, while an `articles` override contributes one pair per listed article plus any registered context controls. Directory scoring prefers `result-case-*.json` when present and otherwise falls back to `result-*.json`. `review-request.json` is an optional all-domains stress test for larger models. Neither batch form is the preferred fast-model profile. 3. Save only this result shape: diff --git a/evaluation/review-fixtures.json b/evaluation/review-fixtures.json index dfe6dd6d..0a557d50 100644 --- a/evaluation/review-fixtures.json +++ b/evaluation/review-fixtures.json @@ -199,7 +199,15 @@ "format-exchanged-values-with-standard-format-9", "api-page-least-privilege-write-access", "describe-api-pages-for-mcp-with-abouttext" - ] + ], + "articleOverrides": { + "describe-api-pages-for-mcp-with-abouttext": { + "context": "Target Business Central online version 28. The API page and API query in this input are exposed as tools through Business Central's built-in MCP server.", + "cleanContexts": { + "non-mcp": "Target Business Central online version 28. The API page and API query in this input serve a REST integration only and are not exposed through MCP." + } + } + } } } } diff --git a/tools/Test-ReviewFixtureContexts.ps1 b/tools/Test-ReviewFixtureContexts.ps1 new file mode 100644 index 00000000..df5c7812 --- /dev/null +++ b/tools/Test-ReviewFixtureContexts.ps1 @@ -0,0 +1,101 @@ +<# +.SYNOPSIS + Regression checks for context-dependent review fixtures after neutral preparation. +#> +[CmdletBinding()] +param( + [string] $Root = (Resolve-Path (Join-Path $PSScriptRoot '..')), + [string] $WorkDirectory = (Join-Path ([IO.Path]::GetTempPath()) "bcquality-context-tests-$([guid]::NewGuid().ToString('N'))") +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' +$Root = (Resolve-Path -LiteralPath $Root).Path +New-Item -ItemType Directory -Path $WorkDirectory -ErrorAction Stop | Out-Null +$runner = (Get-Process -Id $PID).Path +$harness = Join-Path $Root 'tools/Test-ReviewFixtures.ps1' +$prepared = Join-Path $WorkDirectory 'prepared' +$log = @(& $runner -NoProfile -File $harness -Root $Root -PrepareDirectory $prepared 2>&1) +if ($LASTEXITCODE -ne 0) { throw "Fixture preparation failed: $($log -join [Environment]::NewLine)" } + +function Assert-True { + param([bool] $Condition, [string] $Message) + if (-not $Condition) { throw $Message } +} + +function Get-OpaqueId { + param([string] $ManifestId) + $hash = [Security.Cryptography.SHA256]::HashData([Text.Encoding]::UTF8.GetBytes($ManifestId)) + return 'case-' + [Convert]::ToHexString($hash).Substring(0, 8).ToLowerInvariant() +} + +$slug = 'describe-api-pages-for-mcp-with-abouttext' +$prefix = "web-services-$slug" +$mcpContext = "Target Business Central online version 28. The API page and API query in this input are exposed as tools through Business Central's built-in MCP server." +$restContext = 'Target Business Central online version 28. The API page and API query in this input serve a REST integration only and are not exposed through MCP.' +$ids = @{ + bad = Get-OpaqueId "$prefix-bad" + good = Get-OpaqueId "$prefix-good" + rest = Get-OpaqueId "$prefix-clean-non-mcp" +} +$all = Get-Content -LiteralPath (Join-Path $prepared 'review-request.json') -Raw | ConvertFrom-Json +$domain = Get-Content -LiteralPath (Join-Path $prepared 'request-web-services.json') -Raw | ConvertFrom-Json +foreach ($kind in 'bad', 'good', 'rest') { + $id = $ids[$kind] + $expectedContext = if ($kind -eq 'rest') { $restContext } else { $mcpContext } + $singlePath = Join-Path $prepared "request-$id.json" + Assert-True (Test-Path -LiteralPath $singlePath) "Missing prepared context case: $kind" + $single = Get-Content -LiteralPath $singlePath -Raw | ConvertFrom-Json + foreach ($request in $all, $domain, $single) { + $case = @($request.cases | Where-Object id -eq $id) + Assert-True ($case.Count -eq 1) "Context case $kind must occur once in each request format." + Assert-True ($case[0].PSObject.Properties.Name -contains 'context') "Prepared $kind case lost its deployment context." + Assert-True ($case[0].context -ceq $expectedContext) "Prepared $kind case has the wrong context." + $keys = @($case[0].PSObject.Properties.Name | Sort-Object) -join ',' + Assert-True ($keys -ceq 'context,file,id') "Prepared $kind case reveals internal fixture metadata." + } +} +$badCode = Get-Content -LiteralPath (Join-Path $prepared "$($ids.bad).al") -Raw +$restCode = Get-Content -LiteralPath (Join-Path $prepared "$($ids.rest).al") -Raw +Assert-True ($badCode -ceq $restCode) 'The non-MCP control must use exactly the positive case code; only deployment context changes.' +Assert-True ($badCode -notmatch '(?m)^\s*//') 'Full-line comments must still be removed.' +foreach ($case in @($domain.cases | Where-Object { $_.id -notin @($ids.Values) })) { + Assert-True ($case.PSObject.Properties.Name -notcontains 'context') 'MCP context leaked to another web-services article.' +} +$finance = Get-Content -LiteralPath (Join-Path $prepared 'request-finance.json') -Raw | ConvertFrom-Json +foreach ($case in $finance.cases) { + Assert-True ($case.context -ceq 'Target Business Central 28, worldwide standard application (w1). Review each file as an independent source input.') 'Existing domain-wide context changed.' +} + +# Exercise scoring through the public CLI. Other positive cases intentionally +# have no findings; only the context control's specific diagnostics are asserted. +$results = [pscustomobject]@{ cases = @($all.cases | ForEach-Object { [pscustomobject]@{ id = $_.id; findings = @() } }) } +$resultsPath = Join-Path $WorkDirectory 'results.json' +$results | ConvertTo-Json -Depth 8 | Set-Content -LiteralPath $resultsPath -Encoding utf8 +$score = @(& $runner -NoProfile -File $harness -Root $Root -ResultsPath $resultsPath 2>&1) -join "`n" +Assert-True ($LASTEXITCODE -eq 1) 'Empty findings must fail the positive cases.' +Assert-True (-not $score.Contains("${prefix}-clean-non-mcp:")) 'An empty non-MCP control should be accepted.' +($results.cases | Where-Object id -eq $ids.rest).findings = @('microsoft/knowledge/web-services/describe-api-pages-for-mcp-with-abouttext.md') +$results | ConvertTo-Json -Depth 8 | Set-Content -LiteralPath $resultsPath -Encoding utf8 +$score = @(& $runner -NoProfile -File $harness -Root $Root -ResultsPath $resultsPath 2>&1) -join "`n" +Assert-True ($LASTEXITCODE -eq 1 -and $score.Contains("${prefix}-clean-non-mcp: clean control produced finding(s):")) 'Scoring must reject a finding on the non-MCP control.' +$results.cases = @($results.cases | Where-Object id -ne $ids.rest) +$results | ConvertTo-Json -Depth 8 | Set-Content -LiteralPath $resultsPath -Encoding utf8 +$score = @(& $runner -NoProfile -File $harness -Root $Root -ResultsPath $resultsPath 2>&1) -join "`n" +Assert-True ($LASTEXITCODE -eq 1 -and $score.Contains("Results missing case: ${prefix}-clean-non-mcp")) 'Scoring must require the non-MCP control.' + +foreach ($invalid in 'unselected-article', 'empty-context', 'empty-clean-context', 'invalid-control-name') { + $manifest = Get-Content -LiteralPath (Join-Path $Root 'evaluation/review-fixtures.json') -Raw | ConvertFrom-Json + $entry = $manifest.overrides.'web-services'.articleOverrides.$slug + switch ($invalid) { + 'unselected-article' { $manifest.overrides.'web-services'.articleOverrides | Add-Member -NotePropertyName 'not-selected' -NotePropertyValue $entry } + 'empty-context' { $entry.context = ' ' } + 'empty-clean-context' { $entry.cleanContexts.'non-mcp' = ' ' } + 'invalid-control-name' { $entry.cleanContexts | Add-Member -NotePropertyName '../escape' -NotePropertyValue $restContext } + } + $manifestPath = Join-Path $WorkDirectory "$invalid.json" + $manifest | ConvertTo-Json -Depth 12 | Set-Content -LiteralPath $manifestPath -Encoding utf8 + $validation = @(& $runner -NoProfile -File $harness -Root $Root -ManifestPath $manifestPath 2>&1) -join "`n" + Assert-True ($LASTEXITCODE -eq 1 -and $validation.Contains('articleOverrides')) "Invalid article override was not diagnosed: $invalid" +} +Write-Host 'Review fixture context regression checks PASSED: request formats, isolation, neutralization, clean-control scoring, and invalid overrides.' -ForegroundColor Green diff --git a/tools/Test-ReviewFixtures.ps1 b/tools/Test-ReviewFixtures.ps1 index f9be5127..b6b599a5 100644 --- a/tools/Test-ReviewFixtures.ps1 +++ b/tools/Test-ReviewFixtures.ps1 @@ -281,10 +281,53 @@ foreach ($domain in $leafDomains) { } else { $null } + $articleOverrides = @{} + if ($override -and ($override.PSObject.Properties.Name -contains 'articleOverrides')) { + if ($override.articleOverrides -isnot [pscustomobject]) { + $problems.Add("${domain}: articleOverrides must be an object keyed by selected article slug.") | Out-Null + } else { + foreach ($property in $override.articleOverrides.PSObject.Properties) { + $slug = $property.Name + $entry = $property.Value + $problemCount = $problems.Count + if ($slug -cnotin @($selectedArticles | ForEach-Object BaseName)) { + $problems.Add("${domain}: articleOverrides names an unselected article: $slug") | Out-Null + } + if ($entry -isnot [pscustomobject]) { + $problems.Add("${domain}: articleOverrides.$slug must be an object.") | Out-Null + continue + } + if (($entry.PSObject.Properties.Name -notcontains 'context') -or + ($entry.context -isnot [string]) -or [string]::IsNullOrWhiteSpace($entry.context)) { + $problems.Add("${domain}: articleOverrides.$slug.context must be a non-empty string.") | Out-Null + } + foreach ($name in $entry.PSObject.Properties.Name) { + if ($name -notin 'context', 'cleanContexts') { + $problems.Add("${domain}: unknown articleOverrides.$slug property: $name") | Out-Null + } + } + if ($entry.PSObject.Properties.Name -contains 'cleanContexts') { + if ($entry.cleanContexts -isnot [pscustomobject]) { + $problems.Add("${domain}: articleOverrides.$slug.cleanContexts must be an object keyed by control name.") | Out-Null + } else { + foreach ($control in $entry.cleanContexts.PSObject.Properties) { + if ($control.Name -cnotmatch '^[a-z0-9]+(?:-[a-z0-9]+)*$' -or + $control.Value -isnot [string] -or [string]::IsNullOrWhiteSpace($control.Value)) { + $problems.Add("${domain}: articleOverrides.$slug.cleanContexts requires lowercase hyphenated names and non-empty context strings.") | Out-Null + } + } + } + } + if ($problems.Count -eq $problemCount) { $articleOverrides[$slug] = $entry } + } + } + } for ($articleIndex = 0; $articleIndex -lt $selectedArticles.Count; $articleIndex++) { $selectedArticle = $selectedArticles[$articleIndex] $articlePath = [string]$selectedArticle.ArticlePath $sampleDirectory = (Split-Path -Parent $articlePath).Replace('\', '/') + $articleOverride = $articleOverrides[[string]$selectedArticle.BaseName] + $caseContext = if ($articleOverride) { [string]$articleOverride.context } else { $context } foreach ($kind in 'bad', 'good') { $caseId = if ($articleIndex -eq 0) { "$domain-$kind" @@ -297,11 +340,25 @@ foreach ($domain in $leafDomains) { input = "$sampleDirectory/$($selectedArticle.BaseName).$kind.al" expected = if ($kind -eq 'bad') { @($articlePath) } else { @() } } - if ($context) { - $case | Add-Member -NotePropertyName context -NotePropertyValue $context + if ($caseContext) { + $case | Add-Member -NotePropertyName context -NotePropertyValue $caseContext } $caseList.Add($case) | Out-Null } + if ($articleOverride -and ($articleOverride.PSObject.Properties.Name -contains 'cleanContexts')) { + foreach ($control in $articleOverride.cleanContexts.PSObject.Properties) { + # Reuse the positive source: the changed deployment context alone + # makes the rule inapplicable. Never expose this marker to models. + $caseList.Add([pscustomobject]@{ + id = "$domain-$($selectedArticle.BaseName)-clean-$($control.Name)" + domain = $domain + input = "$sampleDirectory/$($selectedArticle.BaseName).bad.al" + expected = @() + context = [string]$control.Value + contextControl = $true + }) | Out-Null + } + } } } @@ -330,8 +387,10 @@ foreach ($case in $cases) { if ($expected.Count -and $inputRelativePath -notmatch '\.bad\.[^.]+$') { $problems.Add("${id}: positive case must use a .bad sample: $inputRelativePath") | Out-Null } - if (-not $expected.Count -and $inputRelativePath -notmatch '\.good\.[^.]+$') { - $problems.Add("${id}: clean case must use a .good sample: $inputRelativePath") | Out-Null + $isContextControl = ($case.PSObject.Properties.Name -contains 'contextControl') -and $case.contextControl + if (-not $expected.Count -and $inputRelativePath -notmatch '\.good\.[^.]+$' -and + -not ($isContextControl -and $inputRelativePath -match '\.bad\.al$')) { + $problems.Add("${id}: clean case must use a .good sample or a registered context control: $inputRelativePath") | Out-Null } foreach ($reference in $expected) {