Skip to content

check-term-links: treat a line the diff also deletes as moved, not new - #1187

Merged
d-morrison merged 2 commits into
mainfrom
term-links-moved-lines
Oct 23, 2026
Merged

d-morrison merged 2 commits into
mainfrom
term-links-moved-lines

Conversation

@d-morrison

Copy link
Copy Markdown
Collaborator

Splitting a file puts every line in new files, so check-term-links reflagged each first use the original already carried. It failed pds#244, the first of the one-div-per-subfile splits for gha#1176. The sibling diff-scoped checks (check-literal-math-letters, check-local-macros, check-fform-paren) already exempt moved lines; this adds the same exemption: an added line whose exact text, ignoring surrounding whitespace, the same diff also deletes is left out.

Hunk counts now decide which diff lines are body lines, so a body line that starts with ++ or -- is never read as a file header.

Tests: a real move between files is not flagged, an edited line that still carries the first use is, and the parser drops moved lines and ignores header-looking body lines. Replacing the exemption with if True turns two of them red (test_cli_moved_line_is_not_new, test_parse_added_lines_drops_lines_the_diff_also_deletes). The 40 tests in the folder pass.

The first check-term-links test here that only git mvs a file would have passed for the wrong reason: a pure rename has no hunks, so it never reaches the parser. I replaced it with a copy plus an emptied original.

The notes repos pin this check at a tag, so the fix reaches them when the tag slides.


Generated by Claude Code

Splitting a file adds every line to new files, so the check reflagged each
first use the original already carried. The sibling diff-scoped checks
already exempt moved lines.

Part of #1176.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V6MvQvUgPePB2ndiLAgpHs
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V6MvQvUgPePB2ndiLAgpHs
@github-actions

Copy link
Copy Markdown
Contributor

Claude finished review — View run


I found no defects in this PR. I read the parser change in check-term-links/check-term-links.py and the new tests, and ran the suite: 40 passed.

  • Moved-line exemption: an added line is dropped only when its exact text, stripped of surrounding whitespace, was also deleted in the same diff. A genuinely edited line still has different text, so it is still flagged. The new test test_cli_edited_line_with_the_same_first_use_is_still_flagged covers that.
  • Hunk-count parsing: the counts bound how many body lines are consumed. A body line starting with ++ or -- is therefore not read as a file header. This matches the model check-new-line-breaks already uses.
  • +++ /dev/null (deleted files): the stale current file is harmless, because new_left is 0 for those hunks, so nothing is attributed to the previous file.
  • Blank lines: they are kept as added (not text), so they cannot all be treated as moved.
  • Documentation: the module docstring and a changelog.d fragment are included.

One residual: a moved line that also appears elsewhere in the base is exempted by text alone. The PR accepts that trade-off and matches the check-new-line-breaks precedent in CLAUDE.md.

I did not review the macro rules: the diff adds no math. I found no hallucinated symbols or URLs; the one external reference is gha#1176, which I did not check.

Verdict

Ready for merge

Structured Review Data (JSON)
{
  "schema_version": "1.1",
  "reviewer": "claude",
  "commit_sha": "778571d81c51ab75caea0d531f0d21a24a20f2d1",
  "verdict": "CLEAN",
  "findings": [],
  "detailed_assessment": "Traced parse_added_lines hunk-count handling, deleted-file diffs, blank lines and edited-line cases; the suite of 40 tests passes and no defect was found.",
  "holistic_assessment": "The change matches the documented moved-line diff model used by sibling checks, keeps skip behavior intact, and includes docs, a changelog fragment and regression tests."
}

Reviewed commit: 778571d

@github-actions

Copy link
Copy Markdown
Contributor

💰 Cost: $0.6876 (review) — run

@d-morrison
d-morrison merged commit f2f5fc4 into main Oct 23, 2026
202 of 204 checks passed
@d-morrison
d-morrison deleted the term-links-moved-lines branch October 23, 2026 08:57
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