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

evaluation/README.md

@@ -1,6 +1,10 @@
 # AL review evaluation
 
-`review-fixtures.json` contains one positive and one clean control for every AL review leaf. Inputs are existing companion samples, but 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.
+The evaluation is convention-driven. For every `microsoft/skills/review/al-<domain>-review.md` leaf, the harness finds `microsoft/knowledge/<domain>/`, selects the first article (by filename) with both `.bad.al` and `.good.al` companions, and derives the expected positive and clean control automatically. Adding a conforming leaf requires no scoring-contract edit.
+
+`review-fixtures.json` contains only global thresholds and optional exceptional overrides. An override may select a different article or add context when the generic convention cannot express a scenario. It should remain empty in the normal case.
+
+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.
 
 ## Validate the corpus
 
@@ -8,7 +12,7 @@
 pwsh ./tools/Test-ReviewFixtures.ps1 -Root .
 ```
 
-This is credential-free and runs in CI. It validates fixture paths, expected article references, unique IDs, and positive/clean coverage for every registered leaf.
+This credential-free check proves every registered leaf maps to a same-named knowledge domain with at least one complete AL sample pair and that all configured overrides are valid.
 
 ## Run a fast-model evaluation
 
@@ -18,6 +22,8 @@ This is credential-free and runs in CI. It validates fixture paths, expected art
    pwsh ./tools/Test-ReviewFixtures.ps1 -Root . -PrepareDirectory ./.evaluation-run
    ```
 
+   This is also the CI path. It derives all cases, builds the current index, requires the convention-selected article to rank naturally into the candidate cutoff, and prepares the neutral requests.
+
 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-<domain>.json` files provide optional two-case leaf batches; save those as `result-<domain>.json`. 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.
@@ -47,4 +53,4 @@ This is credential-free and runs in CI. It validates fixture paths, expected art
 
    For a single combined stress-test result, use `-ResultsPath` instead.
 
-The committed gate requires full expected recall and no findings on clean controls. `allowedAdditional` in the manifest records known, independently valid overlaps without weakening the required primary finding.
+The committed gate requires full expected recall, the exact convention-derived article ID, and no findings on clean controls.

evaluation/review-fixtures.json

@@ -1,237 +1,40 @@
 {
-  "version": 1,
+  "version": 2,
+  "selection": "first-paired-al-article",
   "minimumExpectedRecall": 1.0,
   "minimumCleanRate": 1.0,
-  "cases": [
+  "overrides": {
-    {
+    "appsource": {
-      "id": "appsource-bad",
+      "context": "AppSourceCop mandatoryAffixes is configured to ABC."
-      "domain": "appsource",
-      "input": "microsoft/knowledge/appsource/object-affixes-prevent-collisions.bad.al",
-      "context": "AppSourceCop mandatoryAffixes is configured to ABC.",
-      "expected": ["microsoft/knowledge/appsource/object-affixes-prevent-collisions.md"],
-      "allowedAdditional": []
     },
-    {
+    "breaking-changes": {
-      "id": "appsource-good",
+      "article": "obsolete-table-fields-instead-of-deleting-them",
-      "domain": "appsource",
+      "context": "Baseline version: field ID 2 was named Email. Review the provided changed version."
-      "input": "microsoft/knowledge/appsource/object-affixes-prevent-collisions.good.al",
-      "context": "AppSourceCop mandatoryAffixes is configured to ABC.",
-      "expected": [],
-      "allowedAdditional": []
     },
-    {
+    "events": {
-      "id": "breaking-changes-bad",
+      "article": "initialize-ishandled-to-false-before-publishing"
-      "domain": "breaking-changes",
-      "input": "microsoft/knowledge/breaking-changes/obsolete-table-fields-instead-of-deleting-them.bad.al",
-      "context": "Baseline version: field ID 2 was named Email. Review the provided changed version.",
-      "expected": ["microsoft/knowledge/breaking-changes/obsolete-table-fields-instead-of-deleting-them.md"],
-      "allowedAdditional": []
     },
-    {
+    "interfaces": {
-      "id": "breaking-changes-good",
+      "article": "set-defaultimplementation-on-enum"
-      "domain": "breaking-changes",
-      "input": "microsoft/knowledge/breaking-changes/obsolete-table-fields-instead-of-deleting-them.good.al",
-      "context": "Baseline version: field ID 2 was named Email. Review the provided changed version.",
-      "expected": [],
-      "allowedAdditional": []
     },
-    {
+    "performance": {
-      "id": "data-modeling-bad",
+      "article": "use-isempty-for-existence-check"
-      "domain": "data-modeling",
-      "input": "microsoft/knowledge/data-modeling/setup-table-is-a-singleton.bad.al",
-      "expected": ["microsoft/knowledge/data-modeling/setup-table-is-a-singleton.md"],
-      "allowedAdditional": []
     },
-    {
+    "privacy": {
-      "id": "data-modeling-good",
+      "article": "no-pii-in-telemetry-message-string"
-      "domain": "data-modeling",
-      "input": "microsoft/knowledge/data-modeling/setup-table-is-a-singleton.good.al",
-      "expected": [],
-      "allowedAdditional": []
     },
-    {
+    "style": {
-      "id": "error-handling-bad",
+      "article": "label-comment-explains-placeholders"
-      "domain": "error-handling",
-      "input": "microsoft/knowledge/error-handling/ignored-tryfunction-return-disables-try-semantics.bad.al",
-      "expected": ["microsoft/knowledge/error-handling/ignored-tryfunction-return-disables-try-semantics.md"],
-      "allowedAdditional": []
     },
-    {
+    "telemetry": {
-      "id": "error-handling-good",
+      "article": "telemetry-event-id-stable-unique"
-      "domain": "error-handling",
-      "input": "microsoft/knowledge/error-handling/ignored-tryfunction-return-disables-try-semantics.good.al",
-      "expected": [],
-      "allowedAdditional": []
     },
-    {
+    "upgrade": {
-      "id": "events-bad",
+      "article": "initvalue-does-not-update-existing-rows",
-      "domain": "events",
+      "context": "The extended table existed in the previous app version and already contains rows."
-      "input": "microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.bad.al",
-      "expected": ["microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.md"],
-      "allowedAdditional": []
     },
-    {
+    "web-services": {
-      "id": "events-good",
+      "article": "expose-systemid-as-the-api-key"
-      "domain": "events",
-      "input": "microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.good.al",
-      "expected": [],
-      "allowedAdditional": []
-    },
-    {
-      "id": "interfaces-bad",
-      "domain": "interfaces",
-      "input": "microsoft/knowledge/interfaces/set-defaultimplementation-on-enum.bad.al",
-      "expected": ["microsoft/knowledge/interfaces/set-defaultimplementation-on-enum.md"],
-      "allowedAdditional": []
-    },
-    {
-      "id": "interfaces-good",
-      "domain": "interfaces",
-      "input": "microsoft/knowledge/interfaces/set-defaultimplementation-on-enum.good.al",
-      "expected": [],
-      "allowedAdditional": []
-    },
-    {
-      "id": "performance-bad",
-      "domain": "performance",
-      "input": "microsoft/knowledge/performance/use-isempty-for-existence-check.bad.al",
-      "expected": ["microsoft/knowledge/performance/use-isempty-for-existence-check.md"],
-      "allowedAdditional": []
-    },
-    {
-      "id": "performance-good",
-      "domain": "performance",
-      "input": "microsoft/knowledge/performance/use-isempty-for-existence-check.good.al",
-      "expected": [],
-      "allowedAdditional": []
-    },
-    {
-      "id": "privacy-bad",
-      "domain": "privacy",
-      "input": "microsoft/knowledge/privacy/no-pii-in-telemetry-message-string.bad.al",
-      "expected": ["microsoft/knowledge/privacy/no-pii-in-telemetry-message-string.md"],
-      "allowedAdditional": []
-    },
-    {
-      "id": "privacy-good",
-      "domain": "privacy",
-      "input": "microsoft/knowledge/privacy/no-pii-in-telemetry-message-string.good.al",
-      "expected": [],
-      "allowedAdditional": []
-    },
-    {
-      "id": "query-bad",
-      "domain": "query",
-      "input": "microsoft/knowledge/query/set-query-filters-before-open.bad.al",
-      "expected": ["microsoft/knowledge/query/set-query-filters-before-open.md"],
-      "allowedAdditional": []
-    },
-    {
-      "id": "query-good",
-      "domain": "query",
-      "input": "microsoft/knowledge/query/set-query-filters-before-open.good.al",
-      "expected": [],
-      "allowedAdditional": []
-    },
-    {
-      "id": "security-bad",
-      "domain": "security",
-      "input": "microsoft/knowledge/security/secrettext-with-httpclient.bad.al",
-      "expected": ["microsoft/knowledge/security/secrettext-with-httpclient.md"],
-      "allowedAdditional": ["microsoft/knowledge/security/secrettext-for-credentials.md"]
-    },
-    {
-      "id": "security-good",
-      "domain": "security",
-      "input": "microsoft/knowledge/security/secrettext-with-httpclient.good.al",
-      "expected": [],
-      "allowedAdditional": []
-    },
-    {
-      "id": "style-bad",
-      "domain": "style",
-      "input": "microsoft/knowledge/style/label-comment-explains-placeholders.bad.al",
-      "expected": ["microsoft/knowledge/style/label-comment-explains-placeholders.md"],
-      "allowedAdditional": []
-    },
-    {
-      "id": "style-good",
-      "domain": "style",
-      "input": "microsoft/knowledge/style/label-comment-explains-placeholders.good.al",
-      "expected": [],
-      "allowedAdditional": []
-    },
-    {
-      "id": "telemetry-bad",
-      "domain": "telemetry",
-      "input": "microsoft/knowledge/telemetry/telemetry-event-id-stable-unique.bad.al",
-      "expected": ["microsoft/knowledge/telemetry/telemetry-event-id-stable-unique.md"],
-      "allowedAdditional": []
-    },
-    {
-      "id": "telemetry-good",
-      "domain": "telemetry",
-      "input": "microsoft/knowledge/telemetry/telemetry-event-id-stable-unique.good.al",
-      "expected": [],
-      "allowedAdditional": []
-    },
-    {
-      "id": "testing-bad",
-      "domain": "testing",
-      "input": "microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.bad.al",
-      "expected": ["microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.md"],
-      "allowedAdditional": []
-    },
-    {
-      "id": "testing-good",
-      "domain": "testing",
-      "input": "microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.good.al",
-      "expected": [],
-      "allowedAdditional": []
-    },
-    {
-      "id": "ui-bad",
-      "domain": "ui",
-      "input": "microsoft/knowledge/ui/show-caption-on-editable-fields.bad.al",
-      "expected": ["microsoft/knowledge/ui/show-caption-on-editable-fields.md"],
-      "allowedAdditional": []
-    },
-    {
-      "id": "ui-good",
-      "domain": "ui",
-      "input": "microsoft/knowledge/ui/show-caption-on-editable-fields.good.al",
-      "expected": [],
-      "allowedAdditional": []
-    },
-    {
-      "id": "upgrade-bad",
-      "domain": "upgrade",
-      "input": "microsoft/knowledge/upgrade/initvalue-does-not-update-existing-rows.bad.al",
-      "context": "The extended table existed in the previous app version and already contains rows.",
-      "expected": ["microsoft/knowledge/upgrade/initvalue-does-not-update-existing-rows.md"],
-      "allowedAdditional": []
-    },
-    {
-      "id": "upgrade-good",
-      "domain": "upgrade",
-      "input": "microsoft/knowledge/upgrade/initvalue-does-not-update-existing-rows.good.al",
-      "context": "The extended table existed in the previous app version and already contains rows.",
-      "expected": [],
-      "allowedAdditional": []
-    },
-    {
-      "id": "web-services-bad",
-      "domain": "web-services",
-      "input": "microsoft/knowledge/web-services/expose-systemid-as-the-api-key.bad.al",
-      "expected": ["microsoft/knowledge/web-services/expose-systemid-as-the-api-key.md"],
-      "allowedAdditional": []
-    },
-    {
-      "id": "web-services-good",
-      "domain": "web-services",
-      "input": "microsoft/knowledge/web-services/expose-systemid-as-the-api-key.good.al",
-      "expected": [],
-      "allowedAdditional": []
     }
-  ]
+  }
 }

skills/write.md

@@ -89,6 +89,6 @@ Before opening a pull request:
 - File is in the correct layer and domain folder.
 - Name is kebab-case and descriptive.
 - Every companion sample is referenced by filename from the article, and every referenced sample exists.
-- A changed review domain has a positive and clean control in `evaluation/review-fixtures.json`.
+- Every review-leaf domain has at least one article with both `.good.al` and `.bad.al` companions; the evaluation harness derives positive and clean controls from that convention automatically.
 
 Agents scaffolding new files SHOULD run this checklist programmatically before emitting the file.

tools/Test-ReviewFixtures.ps1

@@ -36,7 +36,6 @@ if (-not (Test-Path -LiteralPath $ManifestPath)) {
 }
 
 $manifest = Get-Content -LiteralPath $ManifestPath -Raw | ConvertFrom-Json
-$cases = @($manifest.cases)
 $problems = [System.Collections.Generic.List[string]]::new()
 
 function Get-ModelCaseId {
@@ -97,11 +96,11 @@ function Get-RankedArticles {
     )
 }
 
-if ($manifest.version -ne 1) {
+if ($manifest.version -ne 2) {
     $problems.Add("Unsupported manifest version: $($manifest.version)") | Out-Null
 }
-if (-not $cases.Count) {
+if ($manifest.selection -ne 'first-paired-al-article') {
-    $problems.Add('Manifest has no cases.') | Out-Null
+    $problems.Add("Unsupported selection strategy: $($manifest.selection)") | Out-Null
 }
 if (([double]$manifest.minimumExpectedRecall -lt 0) -or ([double]$manifest.minimumExpectedRecall -gt 1)) {
     $problems.Add('minimumExpectedRecall must be between 0 and 1.') | Out-Null
@@ -117,13 +116,80 @@ $leafDomains = @(
         Sort-Object -Unique
 )
 
+$overrides = @{}
+if ($manifest.PSObject.Properties.Name -contains 'overrides') {
+    foreach ($property in $manifest.overrides.PSObject.Properties) {
+        $overrides[$property.Name] = $property.Value
+    }
+}
+foreach ($overrideDomain in $overrides.Keys) {
+    if ($leafDomains -notcontains $overrideDomain) {
+        $problems.Add("Override domain '$overrideDomain' has no registered al-$overrideDomain-review leaf.") | Out-Null
+    }
+}
+
+$caseList = [System.Collections.Generic.List[object]]::new()
+foreach ($domain in $leafDomains) {
+    $knowledgeDirectory = Join-Path $Root "microsoft/knowledge/$domain"
+    if (-not (Test-Path -LiteralPath $knowledgeDirectory -PathType Container)) {
+        $problems.Add("${domain}: no Microsoft knowledge directory exists.") | Out-Null
+        continue
+    }
+
+    $override = if ($overrides.ContainsKey($domain)) { $overrides[$domain] } else { $null }
+    $selectedArticle = $null
+    if ($override -and ($override.PSObject.Properties.Name -contains 'article')) {
+        $articleName = [string]$override.article
+        if ($articleName.EndsWith('.md')) {
+            $articleName = [System.IO.Path]::GetFileNameWithoutExtension($articleName)
+        }
+        $candidate = Join-Path $knowledgeDirectory "$articleName.md"
+        if (Test-Path -LiteralPath $candidate -PathType Leaf) {
+            $selectedArticle = Get-Item -LiteralPath $candidate
+        } else {
+            $problems.Add("${domain}: override article does not exist: $articleName.md") | Out-Null
+        }
+    } else {
+        $selectedArticle = Get-ChildItem -LiteralPath $knowledgeDirectory -File -Filter '*.md' |
+            Sort-Object Name |
+            Where-Object {
+                (Test-Path -LiteralPath (Join-Path $knowledgeDirectory "$($_.BaseName).good.al") -PathType Leaf) -and
+                (Test-Path -LiteralPath (Join-Path $knowledgeDirectory "$($_.BaseName).bad.al") -PathType Leaf)
+            } |
+            Select-Object -First 1
+    }
+    if (-not $selectedArticle) {
+        $problems.Add("${domain}: no article has both .good.al and .bad.al companion samples.") | Out-Null
+        continue
+    }
+
+    $articlePath = "microsoft/knowledge/$domain/$($selectedArticle.Name)"
+    $context = if ($override -and ($override.PSObject.Properties.Name -contains 'context')) {
+        [string]$override.context
+    } else {
+        $null
+    }
+    foreach ($kind in 'bad', 'good') {
+        $case = [pscustomobject]@{
+            id = "$domain-$kind"
+            domain = $domain
+            input = "microsoft/knowledge/$domain/$($selectedArticle.BaseName).$kind.al"
+            expected = if ($kind -eq 'bad') { @($articlePath) } else { @() }
+        }
+        if ($context) {
+            $case | Add-Member -NotePropertyName context -NotePropertyValue $context
+        }
+        $caseList.Add($case) | Out-Null
+    }
+}
+$cases = @($caseList)
+
 $seenIds = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::Ordinal)
 foreach ($case in $cases) {
     $id = [string]$case.id
     $domain = [string]$case.domain
     $input = [string]$case.input
     $expected = @($case.expected)
-    $allowed = @($case.allowedAdditional)
 
     if ([string]::IsNullOrWhiteSpace($id)) {
         $problems.Add('Case with empty id.') | Out-Null
@@ -145,7 +211,7 @@ foreach ($case in $cases) {
         $problems.Add("${id}: clean case must use a .good sample: $input") | Out-Null
     }
 
-    foreach ($reference in @($expected + $allowed)) {
+    foreach ($reference in $expected) {
         $referencePath = Join-Path $Root ([string]$reference)
         if (-not (Test-Path -LiteralPath $referencePath -PathType Leaf)) {
             $problems.Add("${id}: referenced article does not exist: $reference") | Out-Null
@@ -275,13 +341,13 @@ if ($PrepareDirectory) {
             }
             $rankedArticles = @(Get-RankedArticles -Articles $domainArticles -CaseText $caseText)
             $manifestCase = $manifestCaseByModelId[[string]$requestCase.id]
-            $expectedPaths = @($manifestCase.expected | ForEach-Object { [string]$_ })
+            $selectedArticlePath = ([string]$manifestCase.input) -replace '\.(?:good|bad)\.al$', '.md'
-            if ($expectedPaths.Count) {
+            $rankedPaths = @($rankedArticles | ForEach-Object { [string]$_.path })
-                $rankedPaths = @($rankedArticles | ForEach-Object { [string]$_.path })
+            if ($rankedPaths -notcontains $selectedArticlePath) {
-                if ($rankedPaths -notcontains $expectedPaths[0]) {
+                throw "$($manifestCase.id): deterministic ranking omitted selected article '$selectedArticlePath'. Improve its retrieval metadata or choose an exceptional override article."
-                    throw "$($manifestCase.id): deterministic ranking omitted expected primary article '$($expectedPaths[0])'."
-                }
             }
+            # Candidate order must not reveal which article owns the fixture.
+            $rankedArticles = @($rankedArticles | Sort-Object path)
             [pscustomobject]@{
                 protocol = "Run only $leafPath over this case. Follow leafInstructions exactly, evaluate the ranked candidate article rows, open matching articles in full, and copy every finding id verbatim from candidateArticles[].path."
                 skill = $leafPath
@@ -377,12 +443,11 @@ foreach ($case in $cases) {
         } | Where-Object { -not [string]::IsNullOrWhiteSpace($_) } | Sort-Object -Unique
     )
     $expected = @($case.expected | ForEach-Object { [string]$_ })
-    $allowed = @($case.allowedAdditional | ForEach-Object { [string]$_ })
 
     if ($expected.Count) {
         $positiveTotal++
         $missing = @($expected | Where-Object { $findingIds -notcontains $_ })
-        $unexpected = @($findingIds | Where-Object { ($expected + $allowed) -notcontains $_ })
+        $unexpected = @($findingIds | Where-Object { $expected -notcontains $_ })
         if (-not $missing.Count -and -not $unexpected.Count) {
             $positivePassed++
         } else {