Skip to content

fix(report): preserve occurrence columns in JSON and SARIF - #584

Merged
rng1995 merged 7 commits into
mainfrom
fix/report-occurrence-columns
Sep 21, 2026
Merged

rng1995 merged 7 commits into
mainfrom
fix/report-occurrence-columns

Conversation

@zozozeezee

Copy link
Copy Markdown
Collaborator

Repeated findings could inherit the first occurrence's columns in JSON reports, while SARIF reports omitted columns. Preserve each occurrence's source span, emit one-based Unicode code-point columns in SARIF, and add regression coverage for JSON, SARIF, and MCP reports.

Signed-off-by: Chandrashekar Ramachandran <cramachandra@nvidia.com>

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Approved after reviewing the complete three-file diff and relevant surrounding code at commit 47d6275af8368dc927f8a87b47487de96047b87f, with independent checks of SARIF compatibility and column producer/consumer consistency. No required correctness, security, compatibility, or test changes found.

Review details:

  • _expand_occurrences() now takes columns from each occurrence. Missing or explicitly unknown columns no longer inherit the representative finding's span; a finding without an occurrence list retains its own optional columns.
  • Both active and baseline-suppressed SARIF results apply the same conversion. The internal zero-based, end-exclusive convention correctly becomes one-based, end-exclusive SARIF coordinates. columnKind: "unicodeCodePoints" matches static character offsets and the existing conversion of Python AST byte offsets into character columns.
  • Deduplication, immutable source provenance, and match fingerprints remain intact. The JSON expansion agrees with the compact findings returned by MCP; this change does not alter risk scoring or suppression decisions.
  • Reviewed all 11 added regression cases: repeated same-line occurrences, multiple files, optional/unknown columns, multiline ranges, a non-BMP Unicode prefix, normalized/projected source text, suppressed SARIF results, and MCP embedded JSON. The assertions directly exercise the original incorrect-column behavior.

Validation: static source and test inspection only; contributor code was not executed locally. GitHub reports five completed checks passing (changes, lint, OpenCode TypeScript Tests, DCO, and Docker smoke); test-unit is still running for this commit. Wait for that job to pass before merging. There were no prior review threads to resolve.

@rng1995
rng1995 merged commit 0f176d5 into main Sep 21, 2026
6 checks passed
@rng1995
rng1995 deleted the fix/report-occurrence-columns branch September 21, 2026 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants