From 9b56300cd6aae30b012444c79863ad16f9d22636 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikolaj=20Jakobi=20N=C3=B8ttrup?= Date: Fri, 23 Oct 2026 16:38:57 +0200 Subject: [PATCH 1/2] Evaluation: keep sample comments for the two articles whose rule is about comments Test-ReviewFixtures.ps1 strips every full-line comment from each sample on purpose (evaluation/README.md: "removes full-line sample comments so neither the article slug, domain, nor expected outcome reveals the answer"). Two articles are about the comments themselves, so they cannot be evaluated that way: - testing/test-feature-scenario-tags: the [FEATURE]/[SCENARIO]/[GIVEN]/[WHEN]/[THEN] tags are full-line comments and are removed from the good sample. - style/al-comments-must-not-restate-what-code-already-shows: its bad sample is made of comments and ends with none, so the rule can never fire. A new optional keepComments list in review-fixtures.json (article path + reason) makes preparation keep the comments for that article's two samples; Good/Bad tokens are still neutralized. Nothing changes for any other article. Co-Authored-By: Claude Opus 5.5 --- evaluation/README.md | 2 ++ evaluation/review-fixtures.json | 10 ++++++++++ tools/Test-ReviewFixtures.ps1 | 33 ++++++++++++++++++++++++++++++++- 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/evaluation/README.md b/evaluation/README.md index 55323235..8cbc5a74 100644 --- a/evaluation/README.md +++ b/evaluation/README.md @@ -23,6 +23,8 @@ clean control deliberately writes UTF-8 text and downloads the existing bytes through a default-encoding InStream: transport alone does not transcode text and must not trigger the encoding rule. +An article whose rule is about comments themselves cannot be evaluated once its samples lose their full-line comments. List it under `keepComments` in `review-fixtures.json` (an `article` path and a non-empty `reason`) and preparation keeps that article's sample comments; `Good`/`Bad` tokens are still neutralized. Use it only when the rule is about the comments, and prefer `coverageWaivers` when keeping them would reveal the expected outcome. + 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, diff --git a/evaluation/review-fixtures.json b/evaluation/review-fixtures.json index 5fc0cfbb..8245f759 100644 --- a/evaluation/review-fixtures.json +++ b/evaluation/review-fixtures.json @@ -13,6 +13,16 @@ "reason": "The defect is the expectation that the layout becomes the default; the code alone is identical to a legitimate optional layout once comments are removed." } ], + "keepComments": [ + { + "article": "microsoft/knowledge/testing/test-feature-scenario-tags.md", + "reason": "The rule is about the [FEATURE]/[SCENARIO]/[GIVEN]/[WHEN]/[THEN] comment tags themselves; stripping full-line comments removes the tags from the good sample." + }, + { + "article": "microsoft/knowledge/style/al-comments-must-not-restate-what-code-already-shows.md", + "reason": "The rule is about comments themselves; stripping full-line comments leaves its bad sample with no comments, so the rule can never fire." + } + ], "overrides": { "copilot": { "articles": [ diff --git a/tools/Test-ReviewFixtures.ps1 b/tools/Test-ReviewFixtures.ps1 index f9be5127..2d494f11 100644 --- a/tools/Test-ReviewFixtures.ps1 +++ b/tools/Test-ReviewFixtures.ps1 @@ -162,6 +162,22 @@ foreach ($overrideDomain in $overrides.Keys) { } } +$keepCommentsArticles = @{} +$keepCommentsSeen = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::Ordinal) +if ($manifest.PSObject.Properties.Name -contains 'keepComments') { + foreach ($entry in @($manifest.keepComments)) { + if (-not $entry.article -or -not $entry.reason) { + $problems.Add('Each keepComments entry requires non-empty article and reason values.') | Out-Null + continue + } + if ($keepCommentsArticles.ContainsKey([string]$entry.article)) { + $problems.Add("Duplicate keepComments entry: $($entry.article)") | Out-Null + continue + } + $keepCommentsArticles[[string]$entry.article] = [string]$entry.reason + } +} + $coverageWaivers = @{} if ($manifest.PSObject.Properties.Name -contains 'coverageWaivers') { foreach ($waiver in @($manifest.coverageWaivers)) { @@ -300,12 +316,21 @@ foreach ($domain in $leafDomains) { if ($context) { $case | Add-Member -NotePropertyName context -NotePropertyValue $context } + if ($keepCommentsArticles.ContainsKey($articlePath)) { + $case | Add-Member -NotePropertyName keepComments -NotePropertyValue $true + $keepCommentsSeen.Add($articlePath) | Out-Null + } $caseList.Add($case) | Out-Null } } } $cases = @($caseList) +foreach ($keepCommentsArticle in $keepCommentsArticles.Keys) { + if (-not $keepCommentsSeen.Contains($keepCommentsArticle)) { + $problems.Add("keepComments article is not a selected evaluation article: $keepCommentsArticle") | Out-Null + } +} $seenIds = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::Ordinal) foreach ($case in $cases) { @@ -464,7 +489,13 @@ if ($PrepareDirectory) { # Good, Bad, or Anti-pattern. Strip full-line comments and neutralize those # object-name tokens so model-facing fixtures do not reveal the expected # outcome while preserving executable AL structure and references. - $neutralText = [regex]::Replace($sourceText, '(?m)^\s*//.*(?:\r?\n|$)', '') + # An article whose rule is about the comments themselves is listed under + # keepComments in review-fixtures.json and keeps them; its samples carry no + # answer-revealing labels (Good/Bad is still neutralized below). + $neutralText = $sourceText + if (-not ($case.PSObject.Properties.Name -contains 'keepComments' -and $case.keepComments)) { + $neutralText = [regex]::Replace($neutralText, '(?m)^\s*//.*(?:\r?\n|$)', '') + } $neutralText = [regex]::Replace($neutralText, '\b(?:Good|Bad)\b', 'Eval') Set-Content -LiteralPath (Join-Path $PrepareDirectory $neutralName) -Value $neutralText -Encoding UTF8 $requestCase = [pscustomobject]@{ id = $modelId; file = $neutralName } From 12a59ca97adb73b6ec394e38977dc3a82e7978ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikolaj=20Jakobi=20N=C3=B8ttrup?= Date: Fri, 23 Oct 2026 16:38:57 +0200 Subject: [PATCH 2/2] Remove the answer-revealing comment from test-feature-scenario-tags.bad.al With keepComments the sample's comments reach the model, and this one stated the expected finding ("no FEATURE/SCENARIO/GIVEN/WHEN/THEN tags"). The code itself shows the missing tags, and the article's Anti Pattern section already explains them. Co-Authored-By: Claude Opus 5.5 --- microsoft/knowledge/testing/test-feature-scenario-tags.bad.al | 1 - 1 file changed, 1 deletion(-) diff --git a/microsoft/knowledge/testing/test-feature-scenario-tags.bad.al b/microsoft/knowledge/testing/test-feature-scenario-tags.bad.al index 06e5d54f..040247c7 100644 --- a/microsoft/knowledge/testing/test-feature-scenario-tags.bad.al +++ b/microsoft/knowledge/testing/test-feature-scenario-tags.bad.al @@ -18,7 +18,6 @@ codeunit 50102 "Item Price Testing" SalesHeader: Record "Sales Header"; SalesLine: Record "Sales Line"; begin - // setup mixed with assertions, no clear layers, no FEATURE/SCENARIO/GIVEN/WHEN/THEN tags LibrarySales.CreateCustomer(Customer); LibraryInventory.CreateItem(Item); LibraryPriceCalculation.CreatePriceHeader(