Repository navigation
Conversation
Cluster names are not guaranteed unique: parse_issues.md Step 3(c)
documents the "Other clusters" overflow slot, which two clustering
runs can both legitimately produce. annotate_clusters_json recovered
each sorted cluster's members/severity_breakdown via
next(r for r in raw if r.get("name") == c.name), so two same-named
clusters both resolved to the first match, and the second cluster's
real findings vanished from the output.
Pair each Cluster with its source dict by identity at construction
time instead, since sort_clusters reorders the same objects rather
than copying them.
Adds a regression test that fails on main (both entries return the
first cluster's members; the second's Critical findings disappear)
and passes on this change.
|
|
Author
|
Fixes a data-corruption bug: two clusters sharing a display name silently overwrite each other's members in the report (repro and a differential test in the PR body). CI's held pending a first-run workflow approval; the other checks (CLA, Socket Security) all pass. |
schenksj
approved these changes
Oct 5, 2026
schenksj
left a comment
schenksj
left a comment
Contributor
There was a problem hiding this comment.
Verified locally: full vulnhunter-fix suite passes (550 passed, 10 skipped, py3.12), and the new test fails against main's cluster_score.py, confirming it reproduces the duplicate-name collision. LGTM.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pairs each
Clusterwith its source dict by identity instead of by name, so the annotation step can't cross-wire two clusters that share a name.sort_clustersreturns the sameClusterobjects it was given, just reordered, sozip(clusters, raw)at construction time gives an exactid(cluster) -> originalmapping that survives the sort. Swapped the post-sortnext(r for r in raw if r.get("name") == c.name)for a lookup into that mapping.Added
test_duplicate_names_keep_their_own_members, which builds two"Other clusters"entries (the documented 4th-slot overflow name fromparse_issues.mdStep 3(c)) with different members and severities. It fails onmain: both output entries came back with the first entry's data, and the second entry's two Critical findings never appeared anywhere in the output.pytest -k cluster_scoreis 31 passed on the branch, same as before this change plus the new one.Fixes #35