fix(check): scope signature-change detection to exported symbols

[carlos-alm]

Summary

• Fix checkNoSignatureChanges (src/features/check.ts) to scope signature-change detection to exported symbols only, matching the check's actual purpose of protecting external callers.
• Fix parseDiffOutput to walk each diff hunk body and track only lines actually marked -, instead of trusting the raw hunk-header span (which always includes 3 lines of unchanged context and was sweeping up untouched declarations as false positives).

Titan Audit Context

• Phase: quality_fix (codegraph tool bug)
• Domain: cli/check
• Commits: 1
• Depends on: none

Changes

• src/cli/commands/check.ts
• src/features/check.ts
• tests/integration/check.test.ts

Background

Discovered while adopting the leiden typed-array helpers extracted earlier in this Titan run: removing a private, unexported, zero-caller duplicate fget/iget pair was incorrectly blocked by codegraph check --staged's signatures predicate, for two independent reasons (both fixed here).

Metrics Impact

codegraph check --staged's signatures predicate now only fires on symbols with real external callers, eliminating a source of false-positive gate failures for internal refactors.

Test plan

• CI passes (lint + build + tests)
• codegraph check --cycles --boundaries passes
• No new functions above complexity thresholds

Closes #1760

[greptile-apps]

Greptile Summary

This PR fixes two independent false-positive sources in checkNoSignatureChanges: it scopes the SQL query to exported = 1 so private file-local helpers no longer trip the check, and it replaces the raw hunk-header span approach with DiffLineTracker, a new class that walks each hunk body line-by-line to record only lines actually marked - or +.

• Exported-only filter: checkNoSignatureChanges now queries AND exported = 1, correctly ignoring symbols whose every call-site is already within the same diff.
• DiffLineTracker class: replaces pushHunkRanges and its raw +start,+count header arithmetic with precise per-line cursor tracking for both the old side (oldRanges) and new side (changedRanges), closing the latent gap for any diff with non-zero context lines.
• Tests: five new integration tests cover the private-symbol pass-through, the mixed exported/private file, the context-line regression (codegraph check --signatures: false positive from diff hunk context lines #1760), and the new symmetric old/new-side tracker directly.

Confidence Score: 5/5

Safe to merge — both fixes are scoped to the signature-check predicate and the diff parser, with no impact on other predicates or the DB schema.

The DiffLineTracker logic is sound and verified by hand-traced test cases covering context-line edge cases on both sides. The exported = 1 SQL filter is backed by migration 14 which already populates the column. Five new integration tests directly target the fixed behaviours. The only observations are style-level visibility modifiers on two methods already inaccessible outside the module.

No files require special attention.

Important Files Changed

Filename	Overview
src/features/check.ts	Replaces pushHunkRanges with DiffLineTracker for line-precise old/new range tracking; adds AND exported = 1 to the signature-check SQL query. Core logic is correct and thoroughly covered by new tests.
src/cli/commands/check.ts	One-line help-text update to describe the tightened scope of --signatures. No logic change.
tests/integration/check.test.ts	Adds exported parameter to insertNode helper and inserts a private roundHalfEven fixture; adds five targeted tests covering private-symbol pass-through, mixed file, context-line regression, and symmetric old/new-side tracker.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["parseDiffOutput(diffOutput)"] --> B["split on newlines, iterate"]
    B --> C{"--- /dev/null?"}
    C -- yes --> D["prevIsDevNull = true, continue"]
    C -- no --> E{"--- prefix?"}
    E -- yes --> F["skip source-file header, continue"]
    E -- no --> G{"'+++ b/' match?"}
    G -- yes --> H["tracker.flush(prevFile)\nswitch currentFile\nmark newFile if prevIsDevNull"]
    G -- no --> I{"currentFile set?"}
    I -- no --> J["continue"]
    I -- yes --> K{"@@ hunk header?"}
    K -- yes --> L["tracker.flush(currentFile)\ntracker.startHunk(oldStart, newStart)"]
    K -- no --> M["tracker.consume(line)"]
    M --> N{"line starts with '-'?"}
    N -- yes --> O["flushAdded\nextend removedRun\noldLineCursor++"]
    N -- no --> P{"line starts with '+'?"}
    P -- yes --> Q["flushRemoved\nextend addedRun\nnewLineCursor++"]
    P -- no --> R{"line starts with ' '?"}
    R -- yes --> S["flushRemoved + flushAdded\nboth cursors++"]
    R -- no --> T["flushRemoved + flushAdded\n(no cursor advance)"]
    B --> U["EOF: tracker.flush(currentFile)"]
    U --> V["return { changedRanges, oldRanges, newFiles }"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["parseDiffOutput(diffOutput)"] --> B["split on newlines, iterate"]
    B --> C{"--- /dev/null?"}
    C -- yes --> D["prevIsDevNull = true, continue"]
    C -- no --> E{"--- prefix?"}
    E -- yes --> F["skip source-file header, continue"]
    E -- no --> G{"'+++ b/' match?"}
    G -- yes --> H["tracker.flush(prevFile)\nswitch currentFile\nmark newFile if prevIsDevNull"]
    G -- no --> I{"currentFile set?"}
    I -- no --> J["continue"]
    I -- yes --> K{"@@ hunk header?"}
    K -- yes --> L["tracker.flush(currentFile)\ntracker.startHunk(oldStart, newStart)"]
    K -- no --> M["tracker.consume(line)"]
    M --> N{"line starts with '-'?"}
    N -- yes --> O["flushAdded\nextend removedRun\noldLineCursor++"]
    N -- no --> P{"line starts with '+'?"}
    P -- yes --> Q["flushRemoved\nextend addedRun\nnewLineCursor++"]
    P -- no --> R{"line starts with ' '?"}
    R -- yes --> S["flushRemoved + flushAdded\nboth cursors++"]
    R -- no --> T["flushRemoved + flushAdded\n(no cursor advance)"]
    B --> U["EOF: tracker.flush(currentFile)"]
    U --> V["return { changedRanges, oldRanges, newFiles }"]

Loading

_{Reviews (4): Last reviewed commit: "Merge branch 'main' into fix/titan-check..." | Re-trigger Greptile}

[greptile-apps]

Asymmetric precision: changedRanges still uses raw hunk header span

oldRanges is now precisely line-tracked via RemovedLineTracker, but changedRanges is still derived from the raw +start,+count hunk header (lines 123-127). With getGitDiff's --unified=0 this is harmless today — the header span and the actual + lines are identical. But parseDiffOutput is a public export and, if ever called with a context-carrying diff (e.g., a stored diff from a CI artifact or a direct test fixture), changedRanges would silently include context lines on the new side, potentially causing checkMaxBlastRadius to flag functions whose bodies were untouched. A symmetric new-side tracker (or at minimum a code comment acknowledging this invariant) would remove the latent inconsistency.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[carlos-alm]

Fixed — extended the line tracker to walk the new side of each hunk body symmetrically with the old side, so changedRanges now only includes lines actually marked + instead of the raw +start,+count header span. Added a symmetric regression test (new-side ranges exclude unchanged context lines around an addition) mirroring the existing old-side test. Behavior for the current --unified=0 caller is unchanged; this closes the latent gap for any future caller that passes a context-carrying diff.

[github-actions]

Codegraph Impact Analysis

8 functions changed → 8 callers affected across 4 files

• DiffLineTracker in src/features/check.ts:56 (4 transitive callers)
• DiffLineTracker.startHunk in src/features/check.ts:64 (4 transitive callers)
• DiffLineTracker.consume in src/features/check.ts:77 (4 transitive callers)
• DiffLineTracker.flushRemoved in src/features/check.ts:107 (4 transitive callers)
• DiffLineTracker.flushAdded in src/features/check.ts:116 (4 transitive callers)
• DiffLineTracker.flush in src/features/check.ts:125 (4 transitive callers)
• parseDiffOutput in src/features/check.ts:135 (4 transitive callers)
• checkNoSignatureChanges in src/features/check.ts:285 (4 transitive callers)

[carlos-alm]

@greptileai

[greptile-apps]

+++ /dev/null bypasses all filters and corrupts the previous file's ranges

When a deleted file follows a modified file in the diff, the +++ /dev/null header is not caught by any guard: isDevNullSourceLine only matches --- /dev/null, and extractNewFileName requires a b/ prefix. The line falls through to tracker.consume, where line.startsWith('+') evaluates to true, so it is recorded as an added source line in changedRanges for the previous file. The subsequent @@ -N,M +0,0 @@ hunk header from the deleted file then flushes and restarts the tracker under the wrong file, causing the deleted file's removed lines to land in oldRanges of the preceding file.

Before this PR, pushHunkRanges received the same line but the HUNK_RE match failed, so it silently returned — effectively a safe no-op. The new consume path actively processes any +-prefixed line, introducing the regression. The doc comment on consume states that `+++` headers are "already filtered out by the caller" — that holds for +++ b/… but not for +++ /dev/null. The fix is to add a symmetric guard in parseDiffOutput before tracker.consume is reached:

if (line.startsWith('+++ ')) continue; // +++ /dev/null (deleted-file header) not caught by extractNewFileName

The test suite covers new-file creation (--- /dev/null → +++ b/…) but has no test for file deletion (--- a/… → +++ /dev/null), so this path is currently uncovered.