Skip to content

Wave 7: findings - #9

Merged
ThePlenkov merged 4 commits into
wave-6-plannerfrom
wave-7-findings
Aug 11, 2026
Merged

ThePlenkov merged 4 commits into
wave-6-plannerfrom
wave-7-findings

Conversation

@ThePlenkov

@ThePlenkov ThePlenkov commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

User description

Summary

  • Standalone findings package: SARIF normalization, SHA-256 fingerprinting, baseline CRUD + diff, suppression filtering
  • No @Sverka deps, node stdlib only (crypto/fs/path)
  • 88 tests pass (errors 8, fingerprint 14, normalize 25, baseline 22, suppress 12, public-api 7)
  • Spec 07 amended to resolve contradiction: empty rule/checkId are valid fingerprint inputs (SARIF edge case)
  • saveBaseline returns Promise per spec
  • Reviewer APPROVED after rework (both rejections fixed: return type + spec amendment)

Test plan

  • bun run test (findings: 88 tests, full monorepo 16 projects green)
  • bun run typecheck (clean)
  • bun run build (green, findings 10.45kB index.mjs)
  • bun run lint (clean)
  • reviewer approved (sv-fd9, second pass)

Stacked on #8

Generated with Devin


Summary by cubic

Adds standalone @sverka/findings for SARIF 2.1.0 normalization, SHA-256 fingerprints, and baseline tracking with suppressions and only-new filtering. Ships pure ESM (.mjs + .d.mts), uses only Node stdlib, exports SARIF/baseline types, and updates spec 07 (allow empty rule/checkId; saveBaseline returns Promise<void>).

  • New Features

    • Normalize SARIF to canonical Finding.
    • Deterministic fingerprints; validate only file and line range.
    • Baseline: create/update/compare; load/save; suppressions (optional expiry); only-new filter.
    • Public API: normalizeSarif, computeFingerprint, baseline ops, suppression utils; typed errors with codes.
  • Refactors

    • Fixed SonarCloud and Codacy quality gates; added override for error cause.
    • CI: retriggered SonarCloud analysis (no functional changes).

Written for commit 56e0be4. Summary will update on new commits.

Review in cubic


CodeAnt-AI Description

Add SARIF findings normalization with stable baselines and suppressions

What Changed

  • Added a standalone findings package that converts SARIF 2.1.0 results into consistent findings, including severity, source details, locations, snippets, and rule help links.
  • Findings receive stable SHA-256 identifiers that remain unchanged when messages or severity change and normalize Windows file paths.
  • Added baseline creation, updates, comparison, JSON loading, and saving to identify new, unchanged, and resolved findings.
  • Added suppression handling, including expiry dates and filtering for suppressed or only-new findings.
  • Added clear errors for invalid SARIF, missing locations, invalid fingerprint inputs, and baseline file failures.
  • Published the package with ESM module and type declaration entry points and added comprehensive behavior tests.

Impact

✅ Consistent findings across SARIF-emitting tools
✅ Stable issue tracking across repeated scans
✅ Fewer repeated findings through baseline and suppression filtering

🔄 Retrigger CodeAnt AI Review

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@codeant-ai

codeant-ai Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Incremental review completed 274cb9d Aug 11, 2026 · 10:54 10:54
✅ Incremental review completed e886641 Aug 11, 2026 · 08:44 08:45
✅ Incremental review completed 3eef5eb Aug 11, 2026 · 06:24 06:24
✅ Incremental review completed e608504 Aug 11, 2026 · 00:25 00:25
✅ Incremental review completed 1bae0dd Aug 10, 2026 · 20:19 20:20

@coderabbitai

coderabbitai Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@ThePlenkov, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 29 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e531a535-a82f-40af-ab8b-cf5e6a11238a

📥 Commits

Reviewing files that changed from the base of the PR and between 0aa68bf and 56e0be4.

📒 Files selected for processing (18)
  • engdocs/architecture/wave-07-findings-plan.md
  • packages/findings/package.json
  • packages/findings/project.json
  • packages/findings/src/__tests__/baseline.test.ts
  • packages/findings/src/__tests__/errors.test.ts
  • packages/findings/src/__tests__/fingerprint.test.ts
  • packages/findings/src/__tests__/helpers/fixtures.ts
  • packages/findings/src/__tests__/normalize.test.ts
  • packages/findings/src/__tests__/public-api.test.ts
  • packages/findings/src/__tests__/suppress.test.ts
  • packages/findings/src/baseline.ts
  • packages/findings/src/errors.ts
  • packages/findings/src/fingerprint.ts
  • packages/findings/src/index.ts
  • packages/findings/src/normalize.ts
  • packages/findings/src/suppress.ts
  • packages/findings/src/types.ts
  • specs/07-findings/spec.md

Comment @coderabbitai help to get the list of available commands.

@baz-reviewer

baz-reviewer Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Merger

Needs Review

The shipped code has concrete correctness and reliability defects despite resolved discussions: it ignores context.root, accepts non-integer line numbers, and can emit native TypeErrors for malformed SARIF or baseline JSON instead of documented errors. CI also did not run for this non-trivial change.

Commit 56e0be4 · Evaluated 2026-08-11 15:39 UTC

Review this PR on Baz | Customize your next review

@codeant-ai codeant-ai Bot added the size:XXL This PR changes 1000+ lines, ignoring generated files label Aug 10, 2026

@amazon-q-developer amazon-q-developer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

This PR introduces a standalone findings package with SARIF normalization, SHA-256 fingerprinting, baseline CRUD operations, and suppression filtering. The implementation is well-structured and thoroughly tested (88 passing tests).

Critical Issue Found (1)

  • baseline.ts: Type validation bug that allows null/undefined values to bypass validation before type casting, potentially violating type safety guarantees

The rest of the implementation appears solid with proper error handling, comprehensive test coverage, and good documentation. Once the critical validation issue is addressed, this PR should be ready to merge.


You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

Comment thread packages/findings/src/baseline.ts
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Wave 7: Add standalone findings package (SARIF normalize, fingerprint, baseline)

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Add @sverka/findings core APIs: SARIF normalization, SHA-256 fingerprints, baseline CRUD/diff.
• Implement suppression + only-new filtering driven by baseline suppressions and fingerprints.
• Align spec/docs and package scaffolding; add comprehensive vitest coverage for public API.
Diagram

graph TD
  Sarif{{"SARIF log"}} --> Norm["normalizeSarif"] --> Fp["computeFingerprint"] --> Findings["Finding[]"]
  Findings --> BaseOps["baseline ops"] --> BaseFile[("baseline.json")]
  Findings --> Supp["suppression filters"] --> Filtered["only-new / unsuppressed"]

  subgraph Legend
    direction LR
    _ext{{"External input"}} ~~~ _proc["Function/module"] ~~~ _db[("Stored file")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Prefer SARIF-provided fingerprints when present
  • ➕ Potentially matches tool-specific stability semantics (e.g., CodeQL)
  • ➕ May improve cross-version stability when tools already compute robust fingerprints
  • ➖ Harder to guarantee determinism/consistency across different SARIF producers
  • ➖ Requires defining precedence/merging rules between tool fingerprints and Sverka’s canonical fingerprinting
2. Validate SARIF via JSON Schema library
  • ➕ More exhaustive structural validation with clearer error reporting
  • ➕ Less hand-rolled validation code to maintain
  • ➖ Adds a dependency, conflicting with the PR’s goal of stdlib-only portability
  • ➖ Schema validation can be slower and still needs custom mapping/normalization logic
3. Store full Finding objects in the baseline (not just fingerprints)
  • ➕ Enables richer diffs (e.g., severity/message changes) without re-running normalization
  • ➕ Can support reporting resolved findings with full context
  • ➖ Baseline becomes larger and more brittle across schema changes
  • ➖ Increases compatibility surface; spec currently intends fingerprints-only storage

Recommendation: The chosen approach (canonical SHA-256 fingerprinting + fingerprints-only baseline + minimal hand-rolled SARIF validation) is a good fit for a v1, dependency-free package. Consider a future enhancement to optionally prefer SARIF tool-provided fingerprints (when present and trusted), but keep the current canonical fingerprint as the default to preserve cross-tool consistency and determinism.

Files changed (18) +2392 / -247

Enhancement (7) +637 / -0
baseline.tsImplement baseline model, diffing, and file I/O +199/-0

Implement baseline model, diffing, and file I/O

• Adds Baseline/Suppression/BaselineDiff data models plus create/update/compare operations based on fingerprint sets. Implements loadBaseline/saveBaseline with schema checks, JSON parsing, and BaselineError codes for read/parse/version/write failures.

packages/findings/src/baseline.ts

errors.tsAdd typed error classes and codes for findings operations +44/-0

Add typed error classes and codes for findings operations

• Introduces NormalizationError and BaselineError with explicit code unions and 'override'd 'cause' to satisfy noImplicitOverride. Centralizes error-code vocab used by fingerprinting, normalization, and baseline I/O.

packages/findings/src/errors.ts

fingerprint.tsImplement deterministic SHA-256 fingerprinting +39/-0

Implement deterministic SHA-256 fingerprinting

• Implements computeFingerprint over a canonical string, normalizing Windows path separators and validating required fields. Allows empty rule/checkId (SARIF edge case) while throwing NormalizationError for invalid file/line inputs.

packages/findings/src/fingerprint.ts

index.tsDefine findings public exports per spec +12/-0

Define findings public exports per spec

• Exports the package’s public types, functions, and error classes from a single entrypoint. Mirrors spec-defined interface surface for consumers.

packages/findings/src/index.ts

normalize.tsImplement SARIF 2.1.0 normalization into Finding[] +211/-0

Implement SARIF 2.1.0 normalization into Finding[]

• Defines minimal SARIF types and normalizes SARIF runs/results into Findings with runtime validation and stable id assignment. Resolves rules via ruleId or ruleIndex, maps SARIF levels to Severity, expands multi-location results, and computes fingerprints per location.

packages/findings/src/normalize.ts

suppress.tsImplement baseline-driven suppression and only-new filters +52/-0

Implement baseline-driven suppression and only-new filters

• Adds suppression evaluation with expiry handling, plus helpers to include/exclude suppressed findings. Implements only-new filtering by excluding baseline fingerprints and any suppressed matches.

packages/findings/src/suppress.ts

types.tsAdd canonical Finding and normalization context types +80/-0

Add canonical Finding and normalization context types

• Defines Finding, Severity, FindingSource, NormalizeContext, and FingerprintInput types used throughout the package and exported publicly.

packages/findings/src/types.ts

Tests (7) +1197 / -0
baseline.test.tsAdd baseline CRUD/diff and I/O tests +282/-0

Add baseline CRUD/diff and I/O tests

• Adds tests for create/update/compare semantics including deduping fingerprints, timestamp behavior, suppression cleanup, and diff categories. Covers load/save behavior and BaselineError codes for not-found/invalid/write-failed scenarios using temp files.

packages/findings/src/tests/baseline.test.ts

errors.test.tsAdd tests for NormalizationError and BaselineError +73/-0

Add tests for NormalizationError and BaselineError

• Validates construction, code typing coverage, name assignment, and cause chaining behavior for both error classes.

packages/findings/src/tests/errors.test.ts

fingerprint.test.tsAdd SHA-256 fingerprint determinism and validation tests +102/-0

Add SHA-256 fingerprint determinism and validation tests

• Verifies fingerprint determinism, discrimination by rule/file/lines/checkId, Windows path normalization, and hex formatting. Confirms SARIF edge-case allowance for empty rule/checkId while rejecting empty file and non-positive line numbers.

packages/findings/src/tests/fingerprint.test.ts

fixtures.tsAdd test fixtures for SARIF builders and temp file helpers +111/-0

Add test fixtures for SARIF builders and temp file helpers

• Provides minimal SARIF object builders (log/run/result/rule) and a default NormalizeContext. Adds temp directory and file helpers to support baseline I/O tests.

packages/findings/src/tests/helpers/fixtures.ts

normalize.test.tsAdd SARIF normalization behavior and edge-case tests +328/-0

Add SARIF normalization behavior and edge-case tests

• Covers basic mapping to Finding fields, checkId/id construction, severity mapping (including rule defaultConfiguration), rule resolution (ruleId vs ruleIndex), and multi-location expansion. Adds determinism assertions and validates thrown NormalizationError codes for malformed SARIF and missing locations.

packages/findings/src/tests/normalize.test.ts

public-api.test.tsAdd public API surface verification tests +112/-0

Add public API surface verification tests

• Asserts that index exports required functions, error classes, and all spec-defined types (compile-time importability). Acts as a guardrail against accidental public API drift.

packages/findings/src/tests/public-api.test.ts

suppress.test.tsAdd suppression and only-new filtering tests +189/-0

Add suppression and only-new filtering tests

• Tests isSuppressed behavior for matching, expired, and non-expiring suppressions. Verifies filterSuppressed include/exclude behavior and filterOnlyNew baseline membership + suppression exclusion semantics.

packages/findings/src/tests/suppress.test.ts

Documentation (2) +552 / -241
wave-07-findings-plan.mdAdd Wave 7 findings implementation plan +231/-0

Add Wave 7 findings implementation plan

• Introduces an architecture/implementation plan outlining module layout, invariants (determinism, stdlib-only), error codes, and TDD sequencing. Documents key edge cases (empty rule/checkId, multi-location, baseline forward-compat).

engdocs/architecture/wave-07-findings-plan.md

spec.mdAmend Wave 7 findings spec to match v1 scope and edge cases +321/-241

Amend Wave 7 findings spec to match v1 scope and edge cases

• Updates goals/non-goals to reflect SARIF-only v1, baseline-based suppressions, and removal of registry/inline suppression plans. Clarifies fingerprint input validation (empty rule/checkId allowed), baseline diff semantics (resolvedFingerprints), and documents the public exports and test plan changes.

specs/07-findings/spec.md

Other (2) +6 / -6
package.jsonFix findings package export paths for ESM build outputs +5/-5

Fix findings package export paths for ESM build outputs

• Updates main/module/types and exports to point to generated .mjs and .d.mts artifacts, aligning with sibling packages’ build conventions.

packages/findings/package.json

project.jsonUpdate findings lint command for ESLint 9 flat config +1/-1

Update findings lint command for ESLint 9 flat config

• Removes the deprecated/incorrect '--ext .ts' flag and lints the src directory directly via bun + eslint.

packages/findings/project.json

@codacy-production

codacy-production Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 82 complexity · 0 duplication

Metric Results
Complexity 82
Duplication 0

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

Comment thread packages/findings/src/fingerprint.ts
Comment thread packages/findings/src/normalize.ts Outdated
Comment thread packages/findings/src/normalize.ts Outdated
Comment thread packages/findings/src/suppress.ts

@codacy-production codacy-production Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request is currently not up to standards. The implementation contains a major logic bug in normalizeSarif where context.root is ignored; this prevents fingerprints from being stable across different file systems (e.g., local vs. CI), which is a core requirement of the package. Additionally, Codacy reports over 500 new issues, and the primary normalization function has a cyclomatic complexity of 32, far exceeding the threshold of 10. These issues must be addressed before merging to ensure maintainability and functional correctness.

About this PR

  • The PR introduces 527 new quality issues according to static analysis. Please review the linting and formatting rules for the new @sverka/findings package to ensure it aligns with project standards before merging.

Test suggestions

  • SARIF normalization: verify mapping of levels to severities and rule resolution via ruleId or ruleIndex.
  • Multi-location results: verify that one Finding is produced per location.
  • Fingerprint computation: verify SHA-256 determinism and path normalization (Windows backslashes).
  • Fingerprint validation: ensure empty file or non-positive lines throw INVALID_FINGERPRINT_INPUT while empty rule/checkId are accepted.
  • Baseline CRUD: verify creating, updating (merging/pruning), and comparing (new/resolved/unchanged).
  • Suppression: verify that findings matching non-expired baseline suppression entries are filtered out.
  • Baseline I/O: verify loading/saving from JSON and error handling for missing or invalid files.
  • Public API: verify all required types, functions, and error classes are exported from the index.

TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback

Comment thread packages/findings/src/normalize.ts
Comment thread packages/findings/src/normalize.ts Outdated
@qodo-code-review

qodo-code-review Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (7) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. SARIF paths remain machine-specific 🐞 Bug ≡ Correctness
Description
normalizeSarif fingerprints and returns the raw artifact URI without using context.root;
absolute URIs therefore produce different baseline identities across checkout roots. Windows
separators also remain in Finding.file, violating its root-relative, forward-slash contract.
Code

packages/findings/src/normalize.ts[156]

+          file: uri,
Evidence
The normalizer extracts uri and passes it unchanged to both fingerprinting and Finding.file,
while the public model and specification require a root-relative forward-slash path and explicitly
assign this conversion to normalizeSarif.

packages/findings/src/normalize.ts[135-170]
packages/findings/src/types.ts[19-20]
packages/findings/src/types.ts[61-63]
specs/07-findings/spec.md[446-449]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Normalize each SARIF artifact URI to a project-root-relative, forward-slash path before using it in a fingerprint or `Finding.file`.

## Issue Context
`NormalizeContext.root` exists for path resolution, and the findings contract requires portable root-relative paths. Preserve already-relative paths while handling absolute filesystem paths consistently.

## Fix Focus Areas
- packages/findings/src/normalize.ts[135-170]
- packages/findings/src/types.ts[61-63]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Malformed SARIF leaks TypeErrors 🐞 Bug ☼ Reliability
Description
The runtime validator dereferences the top-level value and each result before verifying they are
non-null objects. Parsed inputs such as null or {version:"2.1.0", runs:[{...results:[null]}]}
throw native TypeError instead of NormalizationError with INVALID_SARIF.
Code

packages/findings/src/normalize.ts[84]

+  if (sarif.version !== "2.1.0") {
Evidence
The first validation reads sarif.version directly, and result processing passes each array element
into resolveRule without an object guard. The plan and specification promise runtime structural
validation and controlled INVALID_SARIF errors.

packages/findings/src/normalize.ts[83-96]
packages/findings/src/normalize.ts[119-126]
packages/findings/src/normalize.ts[196-210]
engdocs/architecture/wave-07-findings-plan.md[92-93]
specs/07-findings/spec.md[405-410]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Reject non-object SARIF values, runs, results, rules, and locations with `NormalizationError("INVALID_SARIF")` before dereferencing them.

## Issue Context
The input originates from parsed external JSON, so TypeScript annotations provide no runtime protection. Preserve `MISSING_LOCATION` specifically for otherwise valid results lacking locations.

## Fix Focus Areas
- packages/findings/src/normalize.ts[83-133]
- packages/findings/src/normalize.ts[196-210]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Baseline elements bypass validation 🐞 Bug ☼ Reliability
Description
loadBaseline checks only that the two fields are arrays and then casts their contents to
string[] and Suppression[]. For example, suppressions: [null] loads successfully and later
crashes isSuppressed while reading s.fingerprint.
Code

packages/findings/src/baseline.ts[R167-168]

+    fingerprints: obj.fingerprints as string[],
+    suppressions: obj.suppressions as Suppression[],
Evidence
The public interfaces require string fingerprints and structured suppression objects, but the loader
validates only the outer arrays. Suppression filtering unconditionally dereferences each loaded
entry, demonstrating the downstream crash.

packages/findings/src/baseline.ts[8-34]
packages/findings/src/baseline.ts[152-170]
packages/findings/src/suppress.ts[9-20]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Validate fingerprint element types and every required suppression field before returning a `Baseline`; reject malformed entries with `BASELINE_INVALID`.

## Issue Context
The baseline file is external JSON, so casting array contents makes the returned public type unsound and defers failures into suppression filtering.

## Fix Focus Areas
- packages/findings/src/baseline.ts[152-170]
- packages/findings/src/suppress.ts[9-20]
- packages/findings/src/__tests__/baseline.test.ts[247-254]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Missing region yields wrong error code 🐞 Bug ≡ Correctness
Description
When a SARIF location's optional region field is absent, normalizeSarif defaults
startLine/endLine to 0 instead of validating the region exists, causing computeFingerprint to
throw the unrelated INVALID_FINGERPRINT_INPUT code instead of the documented MISSING_LOCATION
semantics for an incomplete location.
Code

packages/findings/src/normalize.ts[R138-140]

+        const region = phys?.region;
+        const startLine = region?.startLine ?? 0;
+        const endLine = region?.endLine ?? startLine;
Evidence
SarifLocation.physicalLocation.region is typed as optional in the same PR (normalize.ts lines
49-60), but normalizeSarif does not check for its presence before deriving startLine/endLine, so a
location with a missing region silently becomes startLine=0/endLine=0 and surfaces as a fingerprint
validation error rather than a location error.

packages/findings/src/normalize.ts[136-140]
packages/findings/src/fingerprint.ts[23-33]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When a SARIF result's location has no `region` object (which is optional per the `SarifLocation` type), `normalizeSarif` defaults `startLine` and `endLine` to `0` instead of treating the location as incomplete. This causes `computeFingerprint` to throw `INVALID_FINGERPRINT_INPUT` ("startLine must be > 0") rather than the semantically correct `MISSING_LOCATION` error that the rest of the function uses for missing/empty `locations` arrays.

## Issue Context
`SarifLocation.physicalLocation.region` is optional. The normalizer only validates that the `locations` array itself is non-empty, but never validates that each location has a usable `region` with a `startLine`. This produces confusing, semantically wrong errors for realistic SARIF inputs (e.g. tools that report file-level results without a region).

## Fix Focus Areas
- packages/findings/src/normalize.ts[135-160]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Non-integer lines accepted 🐞 Bug ≡ Correctness
Description
computeFingerprint only rejects line values at or below zero, so NaN, Infinity, and positive
fractions are accepted and hashed. These invalid source positions should produce
INVALID_FINGERPRINT_INPUT under the positive-integer contract.
Code

packages/findings/src/fingerprint.ts[23]

+  if (input.startLine <= 0) {
Evidence
The implementation has only <= 0 guards, whereas the specification explicitly requires positive
integers; JavaScript comparisons allow NaN, positive infinity, and positive fractions through
these guards.

packages/findings/src/fingerprint.ts[16-38]
specs/07-findings/spec.md[505-518]
packages/findings/src/tests/fingerprint.test.ts[91-101]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Validate both line values with `Number.isInteger(value) && value > 0` before hashing and throw `INVALID_FINGERPRINT_INPUT` otherwise.

## Issue Context
This rejects `NaN`, infinities, fractions, zero, and negatives consistently. Add focused tests for all non-integer cases on both fields.

## Fix Focus Areas
- packages/findings/src/fingerprint.ts[23-33]
- packages/findings/src/__tests__/fingerprint.test.ts[91-101]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View review recommended (1)
6. Null baseline leaks TypeError 🐞 Bug ☼ Reliability
Description
loadBaseline casts the result of JSON.parse to an object without verifying it is a non-null,
non-array object, then immediately reads obj.version. As a result, a syntactically valid baseline
file containing null (or other primitive/array roots) throws a native TypeError instead of the
documented BaselineError with BASELINE_INVALID.
Code

packages/findings/src/baseline.ts[R145-146]

+  const obj = parsed as Record<string, unknown>;
+  if (obj.version !== BASELINE_VERSION) {
Evidence
Because JSON.parse can legally return null (e.g., parsing the literal "null"), the value
assigned to parsed is unknown and is then blindly cast to Record<string, unknown> around line
145, after which the code accesses obj.version. If the parsed value is actually null, that
property access triggers a TypeError that escapes the intended error-wrapping flow, contradicting
the loader’s documented contract and package specification that schema/malformed baseline content
should be reported as BaselineError with BASELINE_INVALID (with BASELINE_NOT_FOUND reserved
for missing files).

packages/findings/src/baseline.ts[121-150]
specs/07-findings/spec.md[510-514]
packages/findings/src/baseline.ts[138-151]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Ensure `loadBaseline` validates that the parsed JSON root is a non-null, non-array object before accessing baseline fields like `version`, and convert invalid roots (including `null`, primitives, and arrays) into a `BaselineError` with code `BASELINE_INVALID` rather than allowing a raw `TypeError` to escape.

## Issue Context
A TypeScript cast does not perform runtime validation, so casting `unknown` from `JSON.parse` to `Record<string, unknown>` can mask `null` and other non-object roots until property access crashes. The function is documented to throw only `BaselineError` with codes `BASELINE_NOT_FOUND` or `BASELINE_INVALID`; allowing a native `TypeError` to escape can break callers that only catch `BaselineError`. Add/extend tests to cover `null` and other primitive JSON roots to preserve the documented error behavior.

## Fix Focus Areas
- packages/findings/src/baseline.ts[138-151]
- packages/findings/src/__tests__/baseline.test.ts[223-254]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

7. loadBaseline read errors misclassified as invalid 🐞 Bug ☼ Reliability
Description
loadBaseline treats any readFile failure other than ENOENT (e.g. EACCES permission errors,
EISDIR) as BASELINE_INVALID, conflating filesystem access failures with actual schema/JSON
validation failures and misleading callers about the true cause.
Code

packages/findings/src/baseline.ts[R130-136]

+  } catch (e) {
+    const err = e as NodeJS.ErrnoException;
+    if (err.code === "ENOENT") {
+      throw new BaselineError(`baseline file not found: ${path}`, "BASELINE_NOT_FOUND", e);
+    }
+    throw new BaselineError(`cannot read baseline file: ${path}`, "BASELINE_INVALID", e);
+  }
Evidence
The catch block in loadBaseline only special-cases ENOENT; every other readFile error (permission
denied, reading a directory, etc.) is thrown as BASELINE_INVALID, which per the spec's error
documentation is meant for 'invalid JSON or wrong schema version', not I/O access failures. The
current BaselineErrorCode union (BASELINE_NOT_FOUND, BASELINE_INVALID, BASELINE_WRITE_FAILED) has no
dedicated code for this case, so this is a taxonomy gap rather than a simple bug.

packages/findings/src/baseline.ts[126-136]
packages/findings/src/errors.ts[41-44]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`loadBaseline` currently classifies every `readFile` error other than `ENOENT` as `BASELINE_INVALID`, even though `BASELINE_INVALID` is documented as meaning 'invalid JSON or wrong schema version'. Permission errors (`EACCES`) or attempting to read a directory (`EISDIR`) are file-system access failures unrelated to the baseline's content validity.

## Issue Context
The `BaselineErrorCode` union only defines `BASELINE_NOT_FOUND`, `BASELINE_INVALID`, `BASELINE_WRITE_FAILED`. Fixing this properly requires either introducing a new code (spec change) or explicitly documenting/accepting that non-ENOENT read errors map to `BASELINE_INVALID` with a clearer message that indicates it's a read failure, not a schema failure.

## Fix Focus Areas
- packages/findings/src/baseline.ts[126-136]
- packages/findings/src/errors.ts[41-44]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context
Review mode: 🧠 Deep: This introduces substantial new runtime logic across SARIF normalization, hashing, baseline I/O/diffing, suppression, errors, and public APIs, with 27 independent edit sites and many edge cases where redundant review can catch defects.

Grey Divider

Tip of the day
💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread packages/findings/src/normalize.ts Outdated
Comment thread packages/findings/src/normalize.ts
Comment thread packages/findings/src/fingerprint.ts
Comment thread packages/findings/src/baseline.ts Outdated
Comment thread packages/findings/src/baseline.ts
Comment thread packages/findings/src/normalize.ts Outdated
Comment thread packages/findings/src/baseline.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 18 files

Tip: instead of fixing issues one by one fix them all with cubic
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

Comment thread packages/findings/src/fingerprint.ts
Comment thread engdocs/architecture/wave-07-findings-plan.md
Comment thread packages/findings/src/normalize.ts Outdated
Comment thread packages/findings/src/suppress.ts
Comment thread packages/findings/src/fingerprint.ts
Comment thread packages/findings/src/baseline.ts Outdated
Comment thread packages/findings/src/normalize.ts Outdated
Comment thread packages/findings/src/__tests__/baseline.test.ts
Comment thread specs/07-findings/spec.md
Comment thread packages/findings/src/normalize.ts Outdated
This was referenced Aug 10, 2026
@codeant-ai codeant-ai Bot added size:XXL This PR changes 1000+ lines, ignoring generated files and removed size:XXL This PR changes 1000+ lines, ignoring generated files labels Aug 10, 2026
@codeant-ai codeant-ai Bot added the size:XXL This PR changes 1000+ lines, ignoring generated files label Aug 11, 2026
@codeant-ai codeant-ai Bot added size:XXL This PR changes 1000+ lines, ignoring generated files and removed size:XXL This PR changes 1000+ lines, ignoring generated files labels Aug 11, 2026
@ThePlenkov
ThePlenkov force-pushed the wave-7-findings branch 2 times, most recently from 0272712 to f9d995b Compare August 11, 2026 14:16
@ThePlenkov
ThePlenkov force-pushed the wave-7-findings branch 2 times, most recently from 213ffb0 to d43b68d Compare August 11, 2026 14:50
@ThePlenkov
ThePlenkov force-pushed the wave-7-findings branch 2 times, most recently from 19a97a7 to f2775b6 Compare August 11, 2026 15:07
ThePlenkov and others added 3 commits August 11, 2026 15:29
Standalone findings package: SARIF normalization, SHA-256 fingerprinting,
baseline CRUD + diff, suppression filtering. No @Sverka deps, node stdlib
only (crypto/fs/path). 88 tests pass. Spec 07 amended to resolve
contradiction: empty rule/checkId are valid fingerprint inputs (SARIF
edge case), only file and line range are validated. saveBaseline returns
Promise<void> per spec. Reviewer APPROVED after rework (both rejections
fixed: return type + spec amendment).

<details>
- 88 tests pass (errors 8, fingerprint 14, normalize 25, baseline 22, suppress 12, public-api 7)
- typecheck clean
- build green (findings 10.45kB index.mjs)
- lint clean
- reviewer approved (sv-fd9, second pass)
</details>

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
@sonarqubecloud

Copy link
Copy Markdown

@ThePlenkov
ThePlenkov merged commit 3e0abc6 into wave-4-runtime-host Aug 11, 2026
4 checks passed
@ThePlenkov
ThePlenkov deleted the wave-7-findings branch September 23, 2026 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

baz: needs review size:XXL This PR changes 1000+ lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant