test(pty): cover add-note stack and deletion rows

[benvinegar]

Summary

• add Tuistory coverage for saving [+] notes in stack layout
• add Tuistory coverage for saving [+] notes on deletion-only rows
• add a small deletion-only PTY fixture and row-targeting helper

Tests

• bun run typecheck
• bun run lint
• bun test test/pty/ui-integration.test.ts -t "add-note|add-note drafts"

[greptile-apps]

Greptile Summary

This PR adds PTY integration test coverage for two previously untested [+] add-note entry points: the stack layout mode and deletion-only diff rows. It introduces a small createDeletionOnlyFilePair fixture in the harness and two shared test helpers (lineIndexOf, revealAddNoteNear) to locate and trigger the hover-only affordance.

• createDeletionOnlyFilePair produces a minimal before/after pair where one export is removed, exercising the deletion-row code path through the full save flow.
• revealAddNoteNear probes a 3×3 grid of terminal cells around the target row, tolerating off-by-one differences between PTY snapshots and wrapped line rendering.
• The stack-mode test discards the waitForText return value and takes a second text() snapshot, which is inconsistent with the deletion-only test's pattern and introduces a small race window.

Confidence Score: 4/5

Safe to merge — changes are test-only and add new coverage without touching production code.

The stack-mode test discards the waitForText result and re-reads the terminal snapshot with a separate text() call, creating a small race window not present in the deletion-only test. This inconsistency could cause intermittent CI failures if a redraw fires between the two calls.

test/pty/ui-integration.test.ts — the stack-mode test snapshot-acquisition pattern is worth aligning with the deletion-only test.

Important Files Changed

Filename	Overview
test/pty/harness.ts	Adds createDeletionOnlyFilePair() fixture returning a before/after pair where one export is removed; wired into the harness export object.
test/pty/ui-integration.test.ts	Adds lineIndexOf/revealAddNoteNear helpers and two new PTY tests for stack-mode and deletion-only add-note flows; stack-mode test discards waitForText return value and calls text() again, introducing a small inconsistency vs. the deletion-only pattern.

Sequence Diagram

sequenceDiagram
    participant Test
    participant harness
    participant PTY as PTY Session

    Note over Test,PTY: stack-mode add-note flow
    Test->>harness: createLongWrapFilePair()
    Test->>PTY: launchHunk(--mode stack)
    PTY-->>Test: waitForText(nav bar)
    Test->>Test: lineIndexOf(initial, "this is a very long")
    Test->>PTY: revealAddNoteNear(session, targetRow)
    loop probe 3x3 grid
        Test->>PTY: moveMouse(x, y)
        PTY-->>Test: waitForText([+]) 200ms
    end
    Test->>PTY: click([+])
    PTY-->>Test: waitForText("Draft note")
    Test->>PTY: type("Save this stack draft.")
    Test->>PTY: press(ctrl+s)
    PTY-->>Test: waitForText("Your note")

    Note over Test,PTY: deletion-only add-note flow
    Test->>harness: createDeletionOnlyFilePair()
    Test->>PTY: launchHunk(--mode split)
    PTY-->>Test: waitForText("removeMe")
    Test->>Test: lineIndexOf(initial, "removeMe")
    Test->>PTY: revealAddNoteNear(session, targetRow)
    Test->>PTY: click([+])
    PTY-->>Test: waitForText("Draft note")
    Test->>PTY: type("Save this deletion draft.")
    Test->>PTY: press(ctrl+s)
    PTY-->>Test: waitForText("Your note")

Loading

Comments Outside Diff (1)

1. test/pty/ui-integration.test.ts, line 657-661 (link)

   The waitForText return value is discarded and then session.text({ immediate: true }) is called separately to obtain the snapshot used by lineIndexOf. This introduces a small race window between the two calls — a UI redraw fired in that gap could mean initial no longer contains "this is a very long", causing targetRow to be -1 and the subsequent expect to fail spuriously. The deletion-only test directly below avoids this by assigning the waitForText result.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: test/pty/ui-integration.test.ts
   Line: 657-661
   
   Comment:
   The `waitForText` return value is discarded and then `session.text({ immediate: true })` is called separately to obtain the snapshot used by `lineIndexOf`. This introduces a small race window between the two calls — a UI redraw fired in that gap could mean `initial` no longer contains `"this is a very long"`, causing `targetRow` to be `-1` and the subsequent `expect` to fail spuriously. The deletion-only test directly below avoids this by assigning the `waitForText` result.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
test/pty/ui-integration.test.ts:657-661
The `waitForText` return value is discarded and then `session.text({ immediate: true })` is called separately to obtain the snapshot used by `lineIndexOf`. This introduces a small race window between the two calls — a UI redraw fired in that gap could mean `initial` no longer contains `"this is a very long"`, causing `targetRow` to be `-1` and the subsequent `expect` to fail spuriously. The deletion-only test directly below avoids this by assigning the `waitForText` result.

```suggestion
      const initial = await session.waitForText(/View\s+Navigate\s+Theme\s+Agent\s+Help/, {
        timeout: 15_000,
      });
      const targetRow = lineIndexOf(initial, "this is a very long");
```

_{Reviews (1): Last reviewed commit: "test(pty): cover add-note stack and dele..." | Re-trigger Greptile}