Skip to content

fix(core): use header counts for hunkLineRange so context lines are in range#244

Open
aldevv wants to merge 1 commit intomodem-dev:mainfrom
aldevv:fix/hunk-line-range-includes-context
Open

fix(core): use header counts for hunkLineRange so context lines are in range#244
aldevv wants to merge 1 commit intomodem-dev:mainfrom
aldevv:fix/hunk-line-range-includes-context

Conversation

@aldevv
Copy link
Copy Markdown

@aldevv aldevv commented May 8, 2026

User-visible: } / { and --next-comment silently skip comments — visiting some, missing others. Repros whenever a comment is anchored on a line past the leading context of its hunk.

Steps to reproduce (on main)

  1. hunk diff <range> on a diff that contains a hunk where the actual + / - line sits past leading context — e.g. a header like @@ -28,20 +28,17 @@ whose first added line is around line 40.
  2. Attach a comment to that hunk by hunk number (so the daemon resolves the anchor via firstCommentTargetForHunk):
    echo '{"comments":[{"filePath":"<file-with-context-heavy-hunk>","hunkNumber":1,"summary":"x","rationale":"y"}]}' \
      | hunk session comment apply --repo <repo> --stdin
  3. Press } (or run hunk session navigate --repo <repo> --next-comment) repeatedly.
  4. The hunk that the comment was applied to is silently skipped — the cursor list never includes it.

Fix: hunkLineRange now uses the per-side header total (additionCount / deletionCount, includes context), not the addition/deletion-only count, so comment-anchor lines fall inside the hunk's range and the hunks make it into the navigation cursor list. Also consolidates the duplicated copy in agentAnnotations.ts into a single import, and adds a regression test.

No user-facing docs or workflows change.

Fixes #235.

…n range

hunkLineRange used additionLines/deletionLines (count of '+' / '-' rows)
as the per-side extent. Pierre exposes those as the change counts only —
the per-side header total (which includes context) is in additionCount /
deletionCount. With many context rows around few changes the additions-only
extent produced range = [start, start], so:

- Comments anchored past leading context (resolved by firstCommentTargetForHunk
  walking hunkContent) fell outside the hunk's range, annotationOverlapsHunk
  returned false, getAnnotatedHunkIndices left the hunk unflagged, and the
  hunk silently disappeared from buildAnnotatedHunkCursors. } / { skipped
  it during comment navigation.
- findHunkIndexForLine returned -1 for context lines, even when those lines
  belong to a hunk in the diff stream.

Fix: switch hunkLineRange to additionCount / deletionCount and consolidate
the duplicate copy in agentAnnotations.ts to a single import. Add a
regression test with one addition surrounded by context that fails under
the old additions-only extent.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 8, 2026

Greptile Summary

This PR fixes a silent comment-navigation regression where hunks containing few changes surrounded by many context lines were dropped from annotatedHunkCursors. The root cause was hunkLineRange using additionLines/deletionLines (only +/- row counts) instead of additionCount/deletionCount (the per-side header total, which includes context), causing the computed hunk extent to collapse to a point range and making context-anchored comments invisible to annotationOverlapsHunk.

  • src/core/liveComments.ts: hunkLineRange now uses additionCount/deletionCount from the hunk header, with an expanded JSDoc explaining why the *Lines variant is wrong.
  • src/ui/lib/agentAnnotations.ts: The stale duplicate hunkLineRange implementation is removed; the file now imports the single canonical version from liveComments.
  • src/core/liveComments.test.ts: A regression test verifies that a 1-addition hunk with 25 lines of context reports a range covering all context rows, and that findHunkIndexForLine resolves lines deep into the trailing context.

Confidence Score: 5/5

Safe to merge — the change is a targeted two-field swap with a matching regression test and no new surface area.

The fix is minimal and well-scoped: one function, two field names changed, a stale duplicate removed, and a regression test that exercises the exact failure scenario described in the PR. The additionCount/deletionCount fields are documented in the pierre Hunk type, and the existing test suite already exercises the non-degenerate path. No logic outside of hunkLineRange was touched.

No files require special attention.

Important Files Changed

Filename Overview
src/core/liveComments.ts Core fix: swaps additionLines/deletionLines for additionCount/deletionCount in hunkLineRange so context lines are included in the computed extent.
src/core/liveComments.test.ts Adds a focused regression test with one addition surrounded by heavy context; validates that hunkLineRange and findHunkIndexForLine both cover the full span.
src/ui/lib/agentAnnotations.ts Deduplicates hunkLineRange by importing the single canonical implementation from liveComments.ts; no logic changes in this file.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["hunkLineRange(hunk)"] --> B{"Before fix"}
    A --> C{"After fix"}
    B --> D["newEnd = additionStart + additionLines − 1\n(only '+' rows)"]
    C --> E["newEnd = additionStart + additionCount − 1\n(context + '+' rows)"]
    D --> F["Range collapses: [28, 28]\nfor 16-line hunk with 1 addition"]
    E --> G["Range correct: [28, 44]\nfor 16-line hunk with 1 addition"]
    F --> H["annotationOverlapsHunk → false\nfor comments past leading context"]
    G --> I["annotationOverlapsHunk → true\nall comments in hunk found"]
    H --> J["Hunk silently dropped from\ngetAnnotatedHunkIndices /\nbuildAnnotatedHunkCursors"]
    I --> K["All 4 cursors present\n} / { navigation works"]
Loading

Reviews (1): Last reviewed commit: "fix(core): use header counts for hunkLin..." | Re-trigger Greptile

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.

next/prev-comment keybinding jumps backward when selection is on a non-annotated hunk

1 participant