Skip to content

pr-review-canvas renderer.js: import filtering and whitespace collapsing happen before line numbering #472

Description

@Authentis

Observed at commit 2eb7ed4: cursor-team-kit/skills/pr-review-canvas/renderer.js.

The renderer drops and rewrites diff lines before line numbers are computed, so the displayed review is neither the diff nor correctly numbered.

1. Import lines are removed before numbering

Lines 10-13 and 95-98:

function isImport(line) { ... s.startsWith('import ') || s.startsWith('import{') || s.startsWith('} from ') }
var filtered = lines.filter(function(l) { ...; return !isImport(l); });

Every added, removed or context line that looks like an import is deleted, including side-effect imports such as import './polyfill', and changes to them become invisible. The line counters oL and nL (lines 117-135) are only reset from @@ headers, so every line after a dropped import within the same hunk is numbered too low.

2. Whitespace-only detection is whitespace-insensitive

Lines 15-17 and 107-111: isWhitespaceOnly strips all whitespace (replace(/\s/g, '')) from both sides, and matching delete/add blocks are rewritten as context lines. This also hides meaningful changes: "a b" to "ab", "a b" string literals, and indentation changes in Python/YAML/Makefiles.

Minimal reproduction

Diff hunk:

@@ -1,3 +1,3 @@
 import a from 'a'
-import b from 'b'
+import c from 'c'
 const x = 1

Both changed import lines are filtered out, and const x = 1 is displayed as line 1 instead of line 3 on both sides.

Another: - return "a b" / + return "ab" is collapsed into an unchanged context line.

Suggested fix

  • Parse and number the full raw diff first; apply any filtering at render time so line numbers come from the original hunk headers.
  • Make the filters opt-in and visibly marked (for example a collapsed "N import lines hidden" row), never silent.
  • Limit whitespace-only collapsing to cases where the changed lines are identical after trimming leading/trailing whitespace only, or drop it for whitespace-sensitive file types.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions