Skip to content

List each strict violation once in JSON and CSV output - #63

Merged
dduugg merged 1 commit into
mainfrom
dedupe-strict-violations-in-json-csv
Sep 27, 2026
Merged

dduugg merged 1 commit into
mainfrom
dedupe-strict-violations-in-json-csv

Conversation

@dduugg

@dduugg dduugg commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

Summary

pks check -o json and -o csv emitted every unrecorded strict violation twice, as byte-identical entries. On contains_strict_violations, main prints two identical CSV rows, and the JSON has two identical violations with violation_count: 2 next to strict_violation_count: 1.

The cause is that an unrecorded strict violation is in both reportable_violations and strict_mode_violations, and both formatters built their list with chain! over the two sets. They now take the sets' union, so each violation appears once and violation_count counts distinct violations. That also makes the schema's description of strict_violation_count as a subset of violation_count true.

Text output and CheckAllResult are deliberately unchanged. Text lists the violation in the report and again as a strict-mode message, which matches packwerk: OffenseCollection#add_offense puts every unlisted offense in new_violations whether or not it's strict, and CheckCommand shows both outstanding_offenses and unlisted_strict_mode_violations. Only the two formatters merged the sets into one list.

This is one of the two follow-ups from the #43 review. The other is #64, which moves strict onto Violation. The two branches merge cleanly in either order.

Tests

  • test_check_with_strict_mode_output_csv now compares whole lines. With contains, it passed while the duplicate row was there.
  • New: JSON on contains_strict_violations, plus CSV and JSON on uses_strict_mode with --ignore-recorded-violations, where recorded strict violations are in both sets.
  • All four fail against a main build: two identical rows, violation_count 2 instead of 1, four rows instead of 2, and violation_count 4 instead of 2.

Test plan

  • cargo test, 269 passed, plus fmt and clippy.
  • Compared against main on simple_app, the strict fixtures, contains_package_todo and contains_stale_violations, with and without --ignore-recorded-violations. Text output is identical. JSON and CSV contain the same distinct entries, now with no duplicates.
  • CI passes.

An unrecorded strict violation is in both `reportable_violations` and
`strict_mode_violations`, and the JSON and CSV formatters chained the two
sets, so they emitted it twice as identical entries. JSON's
`violation_count` counted both. Emit their union instead.

Text output is unchanged. It lists the violation in the report and again as
a strict-mode message, which matches packwerk: `add_offense` puts every
unlisted offense in `new_violations`, and `check` also shows
`unlisted_strict_mode_violations`.

The CSV test now compares whole lines, since `contains` passed with the
duplicate row present. New tests cover JSON on the same fixture and both
formats under --ignore-recorded-violations. All four fail on main.
@dduugg
dduugg requested a review from a team as a code owner September 27, 2026 03:12
@github-project-automation github-project-automation Bot moved this to Triage in Modularity Sep 27, 2026
@dduugg
dduugg merged commit 64544dc into main Sep 27, 2026
15 checks passed
@dduugg
dduugg deleted the dedupe-strict-violations-in-json-csv branch September 27, 2026 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant