Validate findings report rollups

2026-10-05 06:36:55 +01:00 · 2026-09-24 16:17:49 +02:00 · 2026-09-24 16:17:49 +02:00 · e847951adf
commit e847951adf
parent b57a391a0f
2 changed files with 164 additions and 0 deletions
--- a/tools/Test-ReviewContract.ps1
+++ b/tools/Test-ReviewContract.ps1
@ -283,7 +283,47 @@ try {
    $acceptedSuper = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super
    Assert-True (-not $acceptedSuper.normalized) 'valid super-skill report is accepted'

+    $styleFindingLeaf = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json
+    $securityFindingLeaf = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json
+    $securityFindingLeaf.skill.id = 'al-security-review'
+    $rolledFinding = $validReport.findings[0] | ConvertTo-Json -Depth 20 | ConvertFrom-Json
+    $rolledFinding | Add-Member -NotePropertyName 'from-sub-skill' -NotePropertyValue 'al-style-review'
+    $deduplicatedSuperReport = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json
+    $deduplicatedSuperReport.summary.counts.minor = 1
+    $deduplicatedSuperReport.findings = @($rolledFinding)
+    $deduplicatedSuperReport.'sub-results' = @($styleFindingLeaf, $securityFindingLeaf)
+    Set-Content -LiteralPath $reportPath -Value ($deduplicatedSuperReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM
+    $acceptedDeduplicatedSuper = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super `
+        -SourceRoot $tmp -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath
+    Assert-True (-not $acceptedDeduplicatedSuper.normalized) 'one top-level finding may deduplicate the same citation from two leaves'
+
+    $omittedLeafFinding = $deduplicatedSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json
+    $omittedLeafFinding.summary.counts.minor = 0
+    $omittedLeafFinding.findings = @()
+    Set-Content -LiteralPath $reportPath -Value ($omittedLeafFinding | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM
+    Assert-ThrowsLike -Pattern '*SUPER_FINDING_MISSING*' -Action {
+        & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super `
+            -SourceRoot $tmp -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath
+    }
+
+    $nonexistentProducer = $deduplicatedSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json
+    $nonexistentProducer.findings[0].'from-sub-skill' = 'al-missing-review'
+    Set-Content -LiteralPath $reportPath -Value ($nonexistentProducer | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM
+    Assert-ThrowsLike -Pattern '*SUPER_PRODUCER_INVALID*' -Action {
+        & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super `
+            -SourceRoot $tmp -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath
+    }
+
+    $rewrittenLeafFinding = $deduplicatedSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json
+    $rewrittenLeafFinding.findings[0].message = 'A rewritten rollup message.'
+    Set-Content -LiteralPath $reportPath -Value ($rewrittenLeafFinding | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM
+    Assert-ThrowsLike -Pattern '*SUPER_FINDING_MISMATCH*' -Action {
+        & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super `
+            -SourceRoot $tmp -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath
+    }
+
    $failedLeaf = $completedLeaf | ConvertTo-Json -Depth 20 | ConvertFrom-Json
+    $failedLeaf.skill.id = 'al-security-review'
    $failedLeaf.outcome = 'failed'
    $failedLeaf | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'Validation failed.'
    $failedLeaf.summary.coverage.'items-evaluated' = 0
@ -297,6 +337,16 @@ try {
    $acceptedPartialSuper = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super
    Assert-True (-not $acceptedPartialSuper.normalized) 'partial super-skill excludes failed coverage from its rollup'

+    $failedLeafLeakage = $partialSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json
+    $failedLeafLeakage.summary.counts.minor = 1
+    $failedLeafLeakage.findings = @($rolledFinding | ConvertTo-Json -Depth 20 | ConvertFrom-Json)
+    $failedLeafLeakage.findings[0].'from-sub-skill' = 'al-security-review'
+    Set-Content -LiteralPath $reportPath -Value ($failedLeafLeakage | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM
+    Assert-ThrowsLike -Pattern '*SUPER_FAILED_FINDING_LEAKAGE*' -Action {
+        & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super `
+            -SourceRoot $tmp -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath
+    }
+
    $incorrectOutcome = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json
    $incorrectOutcome.outcome = 'not-applicable'
    Set-Content -LiteralPath $reportPath -Value ($incorrectOutcome | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM
--- a/tools/Validate-FindingsReport.ps1
+++ b/tools/Validate-FindingsReport.ps1
@ -103,6 +103,69 @@ function Get-SemanticErrors {
        return 'completed'
    }

+    function Get-RolledFindingId {
+        param([object] $Finding, [string] $ProducerId)
+
+        if (@($Finding.references).Count) {
+            return $Finding.id
+        }
+        return "${ProducerId}:$($Finding.id)"
+    }
+
+    function Test-RolledFindingMatches {
+        param([object] $RolledFinding, [object] $LeafFinding, [string] $ProducerId)
+
+        if ($RolledFinding.id -cne (Get-RolledFindingId $LeafFinding $ProducerId)) {
+            return $false
+        }
+        foreach ($name in 'severity', 'message', 'confidence', 'domain', 'suggested-code', 'suggested-code-omission-reason') {
+            $rolledHasProperty = Test-HasProperty $RolledFinding $name
+            $leafHasProperty = Test-HasProperty $LeafFinding $name
+            if ($rolledHasProperty -ne $leafHasProperty -or
+                ($rolledHasProperty -and $RolledFinding.$name -cne $LeafFinding.$name)) {
+                return $false
+            }
+        }
+
+        $rolledHasLocation = Test-HasProperty $RolledFinding 'location'
+        $leafHasLocation = Test-HasProperty $LeafFinding 'location'
+        if ($rolledHasLocation -ne $leafHasLocation) {
+            return $false
+        }
+        if ($rolledHasLocation) {
+            if ($RolledFinding.location.file -cne $LeafFinding.location.file -or
+                $RolledFinding.location.line -ne $LeafFinding.location.line) {
+                return $false
+            }
+            $rolledHasRange = Test-HasProperty $RolledFinding.location 'range'
+            $leafHasRange = Test-HasProperty $LeafFinding.location 'range'
+            if ($rolledHasRange -ne $leafHasRange -or
+                ($rolledHasRange -and
+                    ($RolledFinding.location.range.'start-line' -ne $LeafFinding.location.range.'start-line' -or
+                     $RolledFinding.location.range.'end-line' -ne $LeafFinding.location.range.'end-line'))) {
+                return $false
+            }
+        }
+
+        $rolledReferences = @($RolledFinding.references)
+        $leafReferences = @($LeafFinding.references)
+        if ($rolledReferences.Count -ne $leafReferences.Count) {
+            return $false
+        }
+        for ($index = 0; $index -lt $rolledReferences.Count; $index++) {
+            if ($rolledReferences[$index].path -cne $leafReferences[$index].path) {
+                return $false
+            }
+            $rolledHasSha = Test-HasProperty $rolledReferences[$index] 'sha'
+            $leafHasSha = Test-HasProperty $leafReferences[$index] 'sha'
+            if ($rolledHasSha -ne $leafHasSha -or
+                ($rolledHasSha -and $rolledReferences[$index].sha -cne $leafReferences[$index].sha)) {
+                return $false
+            }
+        }
+        return $true
+    }
+
    function Test-Report {
        param(
            [object] $Current,
@ -228,6 +291,57 @@ function Get-SemanticErrors {
                Add-Error 'SUPER_COVERAGE_MISMATCH' "$ReportPathPrefix.summary.coverage" `
                    "Expected worklist-size $expectedWorklistSize and items-evaluated $expectedItemsEvaluated from non-failed sub-results."
            }
+
+            $failedProducerIds = @($subResults | Where-Object outcome -CEQ 'failed' | ForEach-Object { $_.skill.id })
+            $eligibleFindingsById = [Collections.Generic.Dictionary[string, object]]::new([StringComparer]::Ordinal)
+            foreach ($subResult in $includedSubResults) {
+                foreach ($finding in @($subResult.findings)) {
+                    $rolledId = Get-RolledFindingId $finding $subResult.skill.id
+                    if (-not $eligibleFindingsById.ContainsKey($rolledId)) {
+                        $eligibleFindingsById[$rolledId] = [Collections.Generic.HashSet[string]]::new([StringComparer]::Ordinal)
+                    }
+                    $eligibleFindingsById[$rolledId].Add([string]$subResult.skill.id) | Out-Null
+                }
+            }
+
+            $validRolledIds = [Collections.Generic.HashSet[string]]::new([StringComparer]::Ordinal)
+            for ($index = 0; $index -lt $findings.Count; $index++) {
+                $finding = $findings[$index]
+                $producerId = [string]$finding.'from-sub-skill'
+                if ($producerId -ceq 'agent') {
+                    continue
+                }
+                if ($producerId -cin $failedProducerIds) {
+                    Add-Error 'SUPER_FAILED_FINDING_LEAKAGE' "$ReportPathPrefix.findings[$index].from-sub-skill" `
+                        "Finding is attributed to failed sub-skill '$producerId'."
+                    continue
+                }
+                if (-not $eligibleFindingsById.ContainsKey($finding.id) -or
+                    -not $eligibleFindingsById[$finding.id].Contains($producerId)) {
+                    Add-Error 'SUPER_PRODUCER_INVALID' "$ReportPathPrefix.findings[$index].from-sub-skill" `
+                        "Sub-skill '$producerId' did not emit finding '$($finding.id)' in a non-failed result."
+                    continue
+                }
+                $producerLeafFindings = @(
+                    $includedSubResults |
+                        Where-Object { $_.skill.id -ceq $producerId } |
+                        ForEach-Object { $_.findings } |
+                        Where-Object { (Get-RolledFindingId $_ $producerId) -ceq $finding.id }
+                )
+                if (-not @($producerLeafFindings | Where-Object { Test-RolledFindingMatches $finding $_ $producerId }).Count) {
+                    Add-Error 'SUPER_FINDING_MISMATCH' "$ReportPathPrefix.findings[$index]" `
+                        "Rolled-up finding '$($finding.id)' does not preserve the finding emitted by '$producerId'."
+                    continue
+                }
+                $validRolledIds.Add([string]$finding.id) | Out-Null
+            }
+
+            foreach ($rolledId in $eligibleFindingsById.Keys) {
+                if (-not $validRolledIds.Contains($rolledId)) {
+                    Add-Error 'SUPER_FINDING_MISSING' "$ReportPathPrefix.findings" `
+                        "No valid rolled-up finding represents non-failed leaf finding '$rolledId'."
+                }
+            }
        }
    }