From f9333addcdf7afae3f385d07346d52b448af1f99 Mon Sep 17 00:00:00 2001 From: Jeremy Vyska Date: Wed, 30 Sep 2026 15:49:13 +0200 Subject: [PATCH] Revert evaluation harness change for multiple articles per domain The harness intentionally evaluates one paired article per domain. Keep it as designed; how the retention pairs join privacy evaluation is left to the maintainers. Co-Authored-By: Claude Opus 5.5 --- evaluation/README.md | 2 +- evaluation/review-fixtures.json | 6 +--- tools/Test-ReviewFixtures.ps1 | 51 +++++++++------------------------ 3 files changed, 15 insertions(+), 44 deletions(-) diff --git a/evaluation/README.md b/evaluation/README.md index a3a661b..7063baf 100644 --- a/evaluation/README.md +++ b/evaluation/README.md @@ -2,7 +2,7 @@ The evaluation is convention-driven. The harness discovers every `/skills/review/al--review.md` leaf across the enabled `microsoft`, `community`, and `custom` layers. Duplicate domains resolve with `custom > community > microsoft` precedence. For each selected leaf, the harness finds paired knowledge across the same layers, applies the same precedence to duplicate article slugs, 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, add context, or list `additionalArticles` whose sample pairs become extra positive and clean cases for that domain, when the generic convention cannot express a scenario. It should remain empty in the normal case. +`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. diff --git a/evaluation/review-fixtures.json b/evaluation/review-fixtures.json index d671ba9..f5c7046 100644 --- a/evaluation/review-fixtures.json +++ b/evaluation/review-fixtures.json @@ -23,11 +23,7 @@ "article": "use-isempty-for-existence-check" }, "privacy": { - "article": "no-pii-in-telemetry-message-string", - "additionalArticles": [ - "register-owned-log-tables-for-retention-policies", - "ship-a-default-retention-policy-setup" - ] + "article": "no-pii-in-telemetry-message-string" }, "style": { "article": "label-comment-explains-placeholders" diff --git a/tools/Test-ReviewFixtures.ps1 b/tools/Test-ReviewFixtures.ps1 index 1a376b9..a9912b7 100644 --- a/tools/Test-ReviewFixtures.ps1 +++ b/tools/Test-ReviewFixtures.ps1 @@ -4,7 +4,7 @@ .DESCRIPTION CI uses the static validation path to prove every registered AL review leaf - has at least one positive and one clean control, every fixture/reference exists, and + has one positive and one clean control, every fixture/reference exists, and the manifest remains internally consistent. For an actual model run, -PrepareDirectory copies inputs to neutral names and @@ -226,49 +226,24 @@ foreach ($domain in $leafDomains) { continue } - # The primary article keeps the domain-level case IDs; additionalArticles add - # further paired cases keyed by slug so existing case hashes stay stable. - $selections = [System.Collections.Generic.List[object]]::new() - $selections.Add([pscustomobject]@{ Article = $selectedArticle; IdPrefix = $domain }) | Out-Null - if ($override -and ($override.PSObject.Properties.Name -contains 'additionalArticles')) { - foreach ($additionalName in @($override.additionalArticles)) { - $additionalName = [string]$additionalName - if ($additionalName.EndsWith('.md')) { - $additionalName = [System.IO.Path]::GetFileNameWithoutExtension($additionalName) - } - if (@($selections | Where-Object { $_.Article.BaseName -eq $additionalName }).Count) { - $problems.Add("${domain}: additional article is already selected: $additionalName.md") | Out-Null - continue - } - $additionalArticle = $articles | Where-Object BaseName -eq $additionalName | Select-Object -First 1 - if (-not $additionalArticle) { - $problems.Add("${domain}: additional article does not exist or lacks .good.al and .bad.al companion samples: $additionalName.md") | Out-Null - continue - } - $selections.Add([pscustomobject]@{ Article = $additionalArticle; IdPrefix = "$domain-$additionalName" }) | Out-Null - } - } - + $articlePath = [string]$selectedArticle.ArticlePath + $sampleDirectory = (Split-Path -Parent $articlePath).Replace('\', '/') $context = if ($override -and ($override.PSObject.Properties.Name -contains 'context')) { [string]$override.context } else { $null } - foreach ($selection in $selections) { - $articlePath = [string]$selection.Article.ArticlePath - $sampleDirectory = (Split-Path -Parent $articlePath).Replace('\', '/') - foreach ($kind in 'bad', 'good') { - $case = [pscustomobject]@{ - id = "$($selection.IdPrefix)-$kind" - domain = $domain - input = "$sampleDirectory/$($selection.Article.BaseName).$kind.al" - expected = if ($kind -eq 'bad') { @($articlePath) } else { @() } - } - if ($context) { - $case | Add-Member -NotePropertyName context -NotePropertyValue $context - } - $caseList.Add($case) | Out-Null + foreach ($kind in 'bad', 'good') { + $case = [pscustomobject]@{ + id = "$domain-$kind" + domain = $domain + input = "$sampleDirectory/$($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)