From b57a391a0fa9173a2aed037700731e2c37e53def Mon Sep 17 00:00:00 2001 From: demiliani Date: Thu, 24 Sep 2026 11:40:05 +0200 Subject: [PATCH] Fix findings report rollup validation --- docs/standalone-runner.md | 5 +- skills/do.md | 1 + tools/Test-ReviewContract.ps1 | 78 ++++++++++++++++++++++++++++ tools/Validate-FindingsReport.ps1 | 84 +++++++++++++++++++++++++++++-- 4 files changed, 161 insertions(+), 7 deletions(-) diff --git a/docs/standalone-runner.md b/docs/standalone-runner.md index c8dcd6a..e5e2a0c 100644 --- a/docs/standalone-runner.md +++ b/docs/standalone-runner.md @@ -60,8 +60,9 @@ only result. bounded optional-range case, record that normalization separately in private telemetry, and accept the candidate only if the entire copy passes the unchanged strict gate. Use `tools/Validate-FindingsReport.ps1`, passing the - exact source paths and fully retrieved article paths. The accepted report - contains no undeclared telemetry fields. + exact source paths and fully retrieved article paths; pass `-SkillKind super` + for the final rolled-up report. The accepted report contains no undeclared + telemetry fields. 6. Collect each accepted findings-report into `sub-results` in the declared `sub-skills` order, not completion order. Run the super-skill self-review only after all leaves have finished. diff --git a/skills/do.md b/skills/do.md index d37605b..db0d634 100644 --- a/skills/do.md +++ b/skills/do.md @@ -224,6 +224,7 @@ as a findings-report, a coordinator or host MUST validate it deterministically: Hosts SHOULD execute `tools/Validate-FindingsReport.ps1` with the exact source scope and the leaf's recorded set of fully retrieved article paths. Pass +`-SkillKind super` when validating a super-skill's rolled-up report. Pass `-AllowBoundedNormalization` only when the host preserves the immutable raw payload and records `removedRanges` in private telemetry as required above. diff --git a/tools/Test-ReviewContract.ps1 b/tools/Test-ReviewContract.ps1 index b1784ef..a937ef8 100644 --- a/tools/Test-ReviewContract.ps1 +++ b/tools/Test-ReviewContract.ps1 @@ -234,6 +234,84 @@ try { -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath } + $completedUndercoverage = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $completedUndercoverage.summary.coverage.'items-evaluated' = 0 + Set-Content -LiteralPath $reportPath -Value ($completedUndercoverage | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-ThrowsLike -Pattern '*COMPLETED_COVERAGE_INCOMPLETE*' -Action { + & $validator -ReportPath $reportPath -BCQualityRoot $Root -SourceRoot $tmp ` + -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath + } + + $partialFullCoverage = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $partialFullCoverage.outcome = 'partial' + $partialFullCoverage | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'Stopped early.' + Set-Content -LiteralPath $reportPath -Value ($partialFullCoverage | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-ThrowsLike -Pattern '*PARTIAL_COVERAGE_INVALID*' -Action { + & $validator -ReportPath $reportPath -BCQualityRoot $Root -SourceRoot $tmp ` + -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath + } + + $completedLeaf = [ordered]@{ + skill = [ordered]@{ id = 'al-style-review'; version = 1 } + outcome = 'completed' + summary = [ordered]@{ + counts = [ordered]@{ blocker = 0; major = 0; minor = 0; info = 0 } + coverage = [ordered]@{ 'worklist-size' = 1; 'items-evaluated' = 1 } + } + findings = @() + suppressed = @() + } + $leafWithSubResults = $completedLeaf | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $leafWithSubResults | Add-Member -NotePropertyName 'sub-results' -NotePropertyValue @($completedLeaf) + Set-Content -LiteralPath $reportPath -Value ($leafWithSubResults | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-ThrowsLike -Pattern '*LEAF_COMPOSITION_INVALID*' -Action { + & $validator -ReportPath $reportPath -BCQualityRoot $Root + } + + $validSuperReport = [ordered]@{ + skill = [ordered]@{ id = 'al-code-review'; version = 1 } + outcome = 'completed' + summary = [ordered]@{ + counts = [ordered]@{ blocker = 0; major = 0; minor = 0; info = 0 } + coverage = [ordered]@{ 'worklist-size' = 2; 'items-evaluated' = 2 } + } + findings = @() + suppressed = @() + 'sub-results' = @($completedLeaf, $completedLeaf) + } + Set-Content -LiteralPath $reportPath -Value ($validSuperReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $acceptedSuper = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super + Assert-True (-not $acceptedSuper.normalized) 'valid super-skill report is accepted' + + $failedLeaf = $completedLeaf | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $failedLeaf.outcome = 'failed' + $failedLeaf | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'Validation failed.' + $failedLeaf.summary.coverage.'items-evaluated' = 0 + $partialSuperReport = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $partialSuperReport.outcome = 'partial' + $partialSuperReport | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'One sub-skill failed.' + $partialSuperReport.summary.coverage.'worklist-size' = 1 + $partialSuperReport.summary.coverage.'items-evaluated' = 1 + $partialSuperReport.'sub-results' = @($completedLeaf, $failedLeaf) + Set-Content -LiteralPath $reportPath -Value ($partialSuperReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $acceptedPartialSuper = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super + Assert-True (-not $acceptedPartialSuper.normalized) 'partial super-skill excludes failed coverage from its rollup' + + $incorrectOutcome = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $incorrectOutcome.outcome = 'not-applicable' + Set-Content -LiteralPath $reportPath -Value ($incorrectOutcome | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-ThrowsLike -Pattern '*SUPER_OUTCOME_MISMATCH*' -Action { + & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super + } + + $incorrectRollup = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $incorrectRollup.summary.coverage.'worklist-size' = 1 + $incorrectRollup.summary.coverage.'items-evaluated' = 1 + Set-Content -LiteralPath $reportPath -Value ($incorrectRollup | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-ThrowsLike -Pattern '*SUPER_COVERAGE_MISMATCH*' -Action { + & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super + } + Set-Content -LiteralPath $reportPath -Value ($validReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM Assert-ThrowsLike -Pattern '*REFERENCE_NOT_RETRIEVED*' -Action { & $validator -ReportPath $reportPath -BCQualityRoot $Root -SourceRoot $tmp -SourcePaths $sourcePath diff --git a/tools/Validate-FindingsReport.ps1 b/tools/Validate-FindingsReport.ps1 index 1797960..682b904 100644 --- a/tools/Validate-FindingsReport.ps1 +++ b/tools/Validate-FindingsReport.ps1 @@ -10,6 +10,8 @@ param( [string] $SourceRoot, [string[]] $SourcePaths = @(), [string[]] $RetrievedArticlePaths = @(), + [ValidateSet('leaf', 'super')] + [string] $SkillKind = 'leaf', [switch] $AllowBoundedNormalization ) @@ -76,8 +78,38 @@ function Get-SemanticErrors { $errors.Add([pscustomobject]@{ Code = $Code; Path = $Path; Message = $Message }) | Out-Null } + function Get-DerivedSuperOutcome { + param([object[]] $SubResults) + + if (-not $SubResults.Count) { + return 'not-applicable' + } + + $outcomes = @($SubResults | ForEach-Object { $_.outcome }) + if (-not @($outcomes | Where-Object { $_ -cne 'failed' }).Count) { + return 'failed' + } + if (($outcomes -ccontains 'partial') -or + (($outcomes -ccontains 'failed') -and @($outcomes | Where-Object { $_ -cne 'failed' }).Count)) { + return 'partial' + } + if (-not @($outcomes | Where-Object { $_ -cne 'not-applicable' }).Count) { + return 'not-applicable' + } + if (($outcomes -ccontains 'no-knowledge') -and + -not @($outcomes | Where-Object { $_ -cnotin @('no-knowledge', 'not-applicable') }).Count) { + return 'no-knowledge' + } + return 'completed' + } + function Test-Report { - param([object] $Current, [string] $ReportPathPrefix) + param( + [object] $Current, + [string] $ReportPathPrefix, + [ValidateSet('leaf', 'super')] + [string] $CurrentSkillKind + ) $findings = @($Current.findings) foreach ($severity in 'blocker', 'major', 'minor', 'info') { @@ -86,14 +118,41 @@ function Get-SemanticErrors { Add-Error 'COUNT_MISMATCH' "$ReportPathPrefix.summary.counts.$severity" "Expected $actual." } } - if ($Current.summary.coverage.'items-evaluated' -gt $Current.summary.coverage.'worklist-size') { + $worklistSize = $Current.summary.coverage.'worklist-size' + $itemsEvaluated = $Current.summary.coverage.'items-evaluated' + if ($itemsEvaluated -gt $worklistSize) { Add-Error 'COVERAGE_INVALID' "$ReportPathPrefix.summary.coverage" 'items-evaluated exceeds worklist-size.' } + elseif ($Current.outcome -ceq 'completed' -and $itemsEvaluated -ne $worklistSize) { + Add-Error 'COMPLETED_COVERAGE_INCOMPLETE' "$ReportPathPrefix.summary.coverage" 'A completed report must evaluate its full worklist.' + } + elseif ($CurrentSkillKind -ceq 'leaf' -and $Current.outcome -ceq 'partial' -and + ($itemsEvaluated -le 0 -or $itemsEvaluated -ge $worklistSize)) { + Add-Error 'PARTIAL_COVERAGE_INVALID' "$ReportPathPrefix.summary.coverage" 'A partial report must evaluate a non-zero proper subset of its worklist.' + } + + $hasSubResults = Test-HasProperty $Current 'sub-results' + $hasSkippedSubSkills = Test-HasProperty $Current 'skipped-sub-skills' + if ($CurrentSkillKind -ceq 'leaf') { + if ($hasSubResults -or $hasSkippedSubSkills) { + Add-Error 'LEAF_COMPOSITION_INVALID' $ReportPathPrefix 'A leaf report must not contain sub-results or skipped-sub-skills.' + } + } + elseif (-not $hasSubResults) { + Add-Error 'SUPER_SUB_RESULTS_REQUIRED' $ReportPathPrefix 'A super-skill report must contain sub-results.' + } for ($index = 0; $index -lt $findings.Count; $index++) { $finding = $findings[$index] $findingPath = "$ReportPathPrefix.findings[$index]" $references = @($finding.references) + $hasProducer = Test-HasProperty $finding 'from-sub-skill' + if ($CurrentSkillKind -ceq 'leaf' -and $hasProducer) { + Add-Error 'LEAF_PRODUCER_INVALID' "$findingPath.from-sub-skill" 'A leaf finding must not contain from-sub-skill.' + } + elseif ($CurrentSkillKind -ceq 'super' -and -not $hasProducer) { + Add-Error 'SUPER_PRODUCER_REQUIRED' $findingPath 'A super-skill finding must identify its producer in from-sub-skill.' + } if (-not $references.Count) { if ($finding.id -cnotmatch '(^|:)agent:[a-z0-9]+(?:-[a-z0-9]+)*$') { Add-Error 'AGENT_ID_INVALID' "$findingPath.id" 'An agent finding id must contain an agent: slug marker.' @@ -149,15 +208,30 @@ function Get-SemanticErrors { } } - if (Test-HasProperty $Current 'sub-results') { + if ($CurrentSkillKind -ceq 'super' -and $hasSubResults) { $subResults = @($Current.'sub-results') for ($index = 0; $index -lt $subResults.Count; $index++) { - Test-Report $subResults[$index] "$ReportPathPrefix.sub-results[$index]" + Test-Report $subResults[$index] "$ReportPathPrefix.sub-results[$index]" 'leaf' + } + + $expectedOutcome = Get-DerivedSuperOutcome $subResults + if ($Current.outcome -cne $expectedOutcome) { + Add-Error 'SUPER_OUTCOME_MISMATCH' "$ReportPathPrefix.outcome" "Expected '$expectedOutcome' from sub-results." + } + + $includedSubResults = @($subResults | Where-Object outcome -CNE 'failed') + $expectedWorklistSize = ($includedSubResults | Measure-Object -Property { $_.summary.coverage.'worklist-size' } -Sum).Sum + $expectedItemsEvaluated = ($includedSubResults | Measure-Object -Property { $_.summary.coverage.'items-evaluated' } -Sum).Sum + if ($null -eq $expectedWorklistSize) { $expectedWorklistSize = 0 } + if ($null -eq $expectedItemsEvaluated) { $expectedItemsEvaluated = 0 } + if ($worklistSize -ne $expectedWorklistSize -or $itemsEvaluated -ne $expectedItemsEvaluated) { + Add-Error 'SUPER_COVERAGE_MISMATCH' "$ReportPathPrefix.summary.coverage" ` + "Expected worklist-size $expectedWorklistSize and items-evaluated $expectedItemsEvaluated from non-failed sub-results." } } } - Test-Report $Candidate '$' + Test-Report $Candidate '$' $SkillKind if ($PermitRangeStartMismatch) { return @($errors | Where-Object Code -CNE 'RANGE_START_MISMATCH') }