Skip to content

Fix NameError crash on arrow keys in interactive review - #14

Merged
VIVAAN-DHAWAN merged 1 commit into
mainfrom
fix/interactive-arrow-nameerror
Aug 23, 2026
Merged

VIVAAN-DHAWAN merged 1 commit into
mainfrom
fix/interactive-arrow-nameerror

Conversation

@VIVAAN-DHAWAN

Copy link
Copy Markdown
Owner

Why

Running the interactive review on any PR crashes the moment an arrow key is pressed:

File "diffly_cli/cli.py", line 410, in interactive_view
    sequence = _read_escape_sequence()
NameError: name '_read_escape_sequence' is not defined

The redesigned interactive menu (v1.0.0) calls _read_escape_sequence() to read the bytes following an Escape keypress, but the helper was never defined — so ↑/↓ navigation, one of the two primary controls shown in the menu footer, is guaranteed to crash after the user has already waited for the full analysis. Reproduced on macOS (Python 3.14) reviewing #12.

What users will see / Surface area

  • src/diffly_cli/cli.py: defines _read_escape_sequence(). It uses select.select() with a short timeout to drain the CSI bytes that follow Escape ([A for ↑, [B for ↓), so:
    • Arrow keys move the cursor again.
    • A lone Escape press returns an empty sequence instead of blocking the loop (preserving the v1.0.0 fix documented in the changelog).
    • Read/OS errors degrade to an empty sequence rather than crashing.
  • No behavior change outside the interactive menu; JSON, Markdown, wizard, and local mode are untouched.

Validation

  • New regression tests in tests/test_regressions.py:
    • test_interactive_menu_handles_arrow_keys_without_crashing drives the real interactive_view loop over a pty with ↑ ↓ Enter keypresses — this exact test fails with NameError on main.
    • test_escape_sequence_reader_returns_arrow_codes asserts [A is returned when those bytes follow Escape.
    • test_escape_sequence_reader_treats_a_lone_escape_as_empty asserts a quiet input yields "" without hanging.
  • Full suite: pytest -q → 56 passed.
  • Duplicate check: no open PRs or issues cover this crash; the bug is present on current main (0e9cf1d).

The interactive menu called _read_escape_sequence() but the helper was
never defined, so pressing an arrow key crashed with NameError right
after the analysis completed. Define the reader: it drains the bytes
following Escape via select() with a short timeout, so arrow keys move
the cursor and a lone Escape returns an empty sequence without
blocking. Adds pty-backed regression coverage for both arrow branches
and the lone-Escape path.
@github-actions

Copy link
Copy Markdown

Diffly verdict: QUARANTINE

#14 · 3 files · 81 lines changed · checks: PENDING

Risk flags: NO_TEST_COVERAGE (low), CHECKS_PENDING (medium)

Risk flags and reasoning
  • NO_TEST_COVERAGE (low): Changed production files have no obvious neighboring or repository test coverage; this is a review hint, not a verdict gate. — src/diffly_cli/cli.py
  • CHECKS_PENDING (medium): Required status checks are still pending. — combined commit status; diffly; test (3.10); test (3.12); test (3.13)
  • QUARANTINE because required checks are still running.
Blast-radius summary
File Change Symbols Direct callers Tests
CHANGELOG.md +4/-0 <top-level changes> — —
src/diffly_cli/cli.py +22/-0 <top-level changes> — —
tests/test_regressions.py +55/-0 <top-level changes> — —

Deterministic triage is authoritative; any optional LLM explanation cannot change the verdict.

@VIVAAN-DHAWAN
VIVAAN-DHAWAN merged commit 2910f45 into main Aug 23, 2026
9 checks passed
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.

1 participant