From 8c3216eec4d5900d3f3827d796df663649d5e35a Mon Sep 17 00:00:00 2001 From: demiliani Date: Fri, 25 Sep 2026 12:05:15 +0200 Subject: [PATCH] Fix locationless finding deduplication --- tools/Test-ReviewContract.ps1 | 41 ++++++++++++++++++ tools/Validate-FindingsReport.ps1 | 72 ++++++++++++++++++++++++++++--- 2 files changed, 107 insertions(+), 6 deletions(-) diff --git a/tools/Test-ReviewContract.ps1 b/tools/Test-ReviewContract.ps1 index 185eb9a..372dab3 100644 --- a/tools/Test-ReviewContract.ps1 +++ b/tools/Test-ReviewContract.ps1 @@ -337,6 +337,20 @@ try { -SourceRoot $tmp -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath, $supportingArticlePath Assert-True (-not $acceptedMergedSuper.normalized) 'overlapping A and B findings may merge into A with B as a supporting reference' + $textOnlyOwnerLeaf = $mergeOwnerLeaf | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $textOnlyOwnerLeaf.findings[0].PSObject.Properties.Remove('suggested-code') + $textOnlySupportingLeaf = $supportingFindingLeaf | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $textOnlySupportingLeaf.findings[0].PSObject.Properties.Remove('suggested-code') + $textOnlyMergedFinding = $mergedFinding | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $textOnlyMergedFinding.PSObject.Properties.Remove('suggested-code') + $textOnlyMergedSuper = $mergedSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $textOnlyMergedSuper.findings = @($textOnlyMergedFinding) + $textOnlyMergedSuper.'sub-results' = @($textOnlyOwnerLeaf, $textOnlySupportingLeaf) + Set-Content -LiteralPath $reportPath -Value ($textOnlyMergedSuper | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $acceptedTextOnlyMerge = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super ` + -SourceRoot $tmp -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath, $supportingArticlePath + Assert-True (-not $acceptedTextOnlyMerge.normalized) 'supporting references permit an overlapping A and B merge with different messages and no suggested code' + $unmergedSupportingFinding = $supportingFindingLeaf.findings[0] | ConvertTo-Json -Depth 20 | ConvertFrom-Json $unmergedSupportingFinding | Add-Member -NotePropertyName 'from-sub-skill' -NotePropertyValue 'al-security-review' $unmergedDuplicates = $mergedSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json @@ -357,6 +371,33 @@ try { -SourceRoot $tmp -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath } + $locationlessLeaf = $styleFindingLeaf | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $locationlessLeaf.findings[0].PSObject.Properties.Remove('location') + $secondLocationlessFinding = $locationlessLeaf.findings[0] | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $locationlessLeaf.findings = @($locationlessLeaf.findings[0], $secondLocationlessFinding) + $locationlessLeaf.summary.counts.minor = 2 + $locationlessRolledFinding = $rolledFinding | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $locationlessRolledFinding.PSObject.Properties.Remove('location') + $locationlessOmission = $deduplicatedSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $locationlessOmission.findings = @($locationlessRolledFinding) + $locationlessOmission.'sub-results' = @($locationlessLeaf, $emptySecurityLeaf) + Set-Content -LiteralPath $reportPath -Value ($locationlessOmission | 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 + } + + $completeLocationlessRollup = $locationlessOmission | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $completeLocationlessRollup.summary.counts.minor = 2 + $completeLocationlessRollup.findings = @( + $locationlessRolledFinding + ($locationlessRolledFinding | ConvertTo-Json -Depth 20 | ConvertFrom-Json) + ) + Set-Content -LiteralPath $reportPath -Value ($completeLocationlessRollup | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $acceptedLocationlessRollup = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super ` + -SourceRoot $tmp -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath + Assert-True (-not $acceptedLocationlessRollup.normalized) 'two locationless leaf occurrences require and accept two distinct rolled findings' + $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 diff --git a/tools/Validate-FindingsReport.ps1 b/tools/Validate-FindingsReport.ps1 index a7d3aa7..186bf84 100644 --- a/tools/Validate-FindingsReport.ps1 +++ b/tools/Validate-FindingsReport.ps1 @@ -122,7 +122,7 @@ function Get-SemanticErrors { return $false } if (-not $firstHasLocation) { - return $true + return $false } if ($First.location.file -cne $Second.location.file) { return $false @@ -171,20 +171,52 @@ function Get-SemanticErrors { [switch] $RequirePrimaryOwner ) + $rolledHasLocation = Test-HasProperty $RolledFinding 'location' + $leafHasLocation = Test-HasProperty $LeafFinding 'location' + $leafReferences = @($LeafFinding.references) + $rolledReferences = @($RolledFinding.references) + if (-not $rolledHasLocation -and -not $leafHasLocation) { + $expectedId = if ($leafReferences.Count) { $LeafFinding.id } else { "${LeafProducerId}:$($LeafFinding.id)" } + if ($RolledFinding.'from-sub-skill' -cne $LeafProducerId -or + $RolledFinding.id -cne $expectedId -or + $RolledFinding.severity -cne $LeafFinding.severity -or + $RolledFinding.confidence -cne $LeafFinding.confidence -or + $RolledFinding.message -cne $LeafFinding.message -or + $rolledReferences.Count -ne $leafReferences.Count -or + -not (Test-ReferencesInclude $rolledReferences $leafReferences)) { + return $false + } + foreach ($name in '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 + } + } + return $true + } + if (-not (Test-LocationsOverlap $RolledFinding $LeafFinding) -or - -not (Test-SameCorrection $RolledFinding $LeafFinding) -or (Get-SeverityRank $RolledFinding.severity) -lt (Get-SeverityRank $LeafFinding.severity) -or (Get-ConfidenceRank $RolledFinding.confidence) -lt (Get-ConfidenceRank $LeafFinding.confidence)) { return $false } - $leafReferences = @($LeafFinding.references) + $sameCorrection = Test-SameCorrection $RolledFinding $LeafFinding + $explicitCrossRuleMerge = $leafReferences.Count -and + $rolledReferences.Count -gt $leafReferences.Count -and + (Test-ReferencesInclude $rolledReferences $leafReferences) + if (-not $sameCorrection -and -not $explicitCrossRuleMerge) { + return $false + } + if (-not $leafReferences.Count) { return $RolledFinding.'from-sub-skill' -ceq $LeafProducerId -and $RolledFinding.id -ceq "${LeafProducerId}:$($LeafFinding.id)" -and - -not @($RolledFinding.references).Count + -not $rolledReferences.Count } - if (-not (Test-ReferencesInclude @($RolledFinding.references) $leafReferences)) { + if (-not (Test-ReferencesInclude $rolledReferences $leafReferences)) { return $false } if ($RequirePrimaryOwner) { @@ -394,7 +426,35 @@ function Get-SemanticErrors { } } - foreach ($eligibleFinding in $eligibleFindings) { + $usedLocationlessFindings = [Collections.Generic.HashSet[int]]::new() + foreach ($eligibleFinding in @($eligibleFindings | Where-Object { -not (Test-HasProperty $_.Finding 'location') })) { + $matchingIndex = -1 + for ($index = 0; $index -lt $findings.Count; $index++) { + if (-not $usedLocationlessFindings.Contains($index) -and + -not (Test-HasProperty $findings[$index] 'location') -and + (Test-RolledFindingRepresents $findings[$index] $eligibleFinding.Finding $eligibleFinding.ProducerId)) { + $matchingIndex = $index + break + } + } + if ($matchingIndex -lt 0) { + Add-Error 'SUPER_FINDING_MISSING' "$ReportPathPrefix.findings" ` + "No distinct rolled-up finding represents a locationless finding from '$($eligibleFinding.ProducerId)'." + } + else { + $usedLocationlessFindings.Add($matchingIndex) | Out-Null + } + } + for ($index = 0; $index -lt $findings.Count; $index++) { + if ($findings[$index].'from-sub-skill' -cne 'agent' -and + -not (Test-HasProperty $findings[$index] 'location') -and + -not $usedLocationlessFindings.Contains($index)) { + Add-Error 'SUPER_FINDING_MISMATCH' "$ReportPathPrefix.findings[$index]" ` + 'No distinct locationless leaf finding corresponds to this rolled-up finding.' + } + } + + foreach ($eligibleFinding in @($eligibleFindings | Where-Object { Test-HasProperty $_.Finding 'location' })) { $represented = @($findings | Where-Object { $_.'from-sub-skill' -cne 'agent' -and (Test-RolledFindingRepresents $_ $eligibleFinding.Finding $eligibleFinding.ProducerId)