Skip to content

fix: prevent cursor-overflow panic on PR reload that shrinks the diff - #574

Merged
agavra merged 1 commit into
agavra:mainfrom
joshvito:fix/pr-reload-cursor-overflow
Aug 11, 2026
Merged

agavra merged 1 commit into
agavra:mainfrom
joshvito:fix/pr-reload-cursor-overflow

Conversation

@joshvito

@joshvito joshvito commented Aug 7, 2026 •

Copy link
Copy Markdown
Contributor

fixes #573

A PR :e reload can replace the diff with a shorter one while leaving cursor_line parked past the new end (the same-head reload branch, and a restored overview cursor captured from the taller old diff, never reclamp it). The next cursor_down then clamps the cursor up to the new max_cursor_line — below prev_cursor — underflowing the cursor_line - prev_cursor usize subtraction and panicking with "attempt to subtract with overflow" at navigation.rs.

  • cursor_down: use saturating_sub for the cursor-movement delta, matching the existing cursor_up siblings.
  • finish_pr_reload / reload_pull_request_with_backend: clamp cursor_line into the reloaded diff's bounds after the swap/restore.
  • add a regression test that reproduces the stale-cursor overflow.

A PR `:e` reload can replace the diff with a shorter one while leaving
`cursor_line` parked past the new end (the same-head reload branch, and a
restored overview cursor captured from the taller old diff, never reclamp
it). The next `cursor_down` then clamps the cursor *up* to the new
`max_cursor_line` — below `prev_cursor` — underflowing the
`cursor_line - prev_cursor` usize subtraction and panicking with
"attempt to subtract with overflow" at navigation.rs.

- cursor_down: use `saturating_sub` for the cursor-movement delta, matching
  the existing `cursor_up` siblings.
- finish_pr_reload / reload_pull_request_with_backend: clamp `cursor_line`
  into the reloaded diff's bounds after the swap/restore.
- add a regression test that reproduces the stale-cursor overflow.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@joshvito

joshvito commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

I want to test this before I publish.

Comment thread src/app/tests/scroll_behavior_tests.rs
@joshvito
joshvito force-pushed the fix/pr-reload-cursor-overflow branch from 4c0bc1a to 19875ba Compare August 10, 2026 15:56
@joshvito
joshvito marked this pull request as ready for review August 10, 2026 15:59

@agavra agavra left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

nice catch @joshvito 🙏

@agavra
agavra merged commit 55f1f0c into agavra:main Aug 11, 2026
8 checks passed
@joshvito
joshvito deleted the fix/pr-reload-cursor-overflow branch August 13, 2026 17:12
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.

panic when reloading a PR of shorter length

2 participants