kte/docs/audits/claude-findings-20260923.md

# Bug and Performance Audit

**Project:** kte (Kyle's Text Editor)
**Date:** 2026-09-23
**Auditor:** Claude Code (automated review, four parallel reviewers plus
spot verification)
**Baseline:** `d8d2ae8` (debug build clean, `kte_tests` 0 failures)

---

## Method

The tree was split into four review areas: core text model (PieceTable,
Buffer, OptimizedSearch), undo and swap (UndoSystem/Tree/Node, Swap,
ErrorRecovery, SyscallWrappers), commands (Command.cc, KKeymap), and
frontends/syntax (Terminal, ImGui, Qt, Editor, `syntax/`). Every finding
below was traced to a concrete scenario. Findings marked **[repro]** were
reproduced with a small test program; the rest were confirmed by reading
the code.

Finding IDs are stable and are referenced by fix commits:

- **M** — memory safety (crash, out-of-bounds, use-after-free, data race)
- **D** — data integrity (undo, swap journal, text corruption)
- **B** — incorrect behaviour
- **P** — performance

---

## M — Memory safety

### M1. PieceTable assignment keeps a stale line index **[repro]**

`PieceTable.cc:47-98`. Copy and move assignment copy `pieces_`, `add_`,
and `total_size_`, but leave the destination's `line_index_` and
`line_index_dirty_` untouched. `Editor::CloseBuffer` erases from
`std::vector<Buffer>`, which move-assigns every later buffer into the
previous slot.

Scenario: open A (`"line1\nline2\nline3\n"`), open B (`"x"`), render
both, close A. Slot 0 now holds B's text with A's index: `LineCount()`
returns 4 and `GetLineView(1)` builds a view over bytes 6–12 of a 1-byte
buffer.

Fix: mark the line index dirty (and clear it) in both assignment
operators.

### M2. Background highlighter thread races buffer mutation

`syntax/HighlighterEngine.cc:184-249`. `PrefetchViewport` (called from
the terminal and ImGui renderers) queues rows for a worker thread which
calls `buf.GetLineString`, walking `pieces_` and reading `add_`. The main
thread's `PieceTable::Insert`/`Delete` mutate both without any shared
lock; `add_.append` may reallocate. The worker also stores a raw
`Buffer*` (`pending_.buf`) which dangles when `AddBuffer`'s `push_back`
reallocates or `CloseBuffer`'s `erase` shifts buffers.

Fix: remove the background warm-up, or feed it an immutable snapshot.

### M3. Per-window SwapManager leaves dangling recorders (ImGui multi-window)

`ImGuiFrontend.cc:381-385`, `Editor.cc:205-213`. Each window's `Editor`
owns its own `SwapManager` but shares the buffer vector. A buffer created
from a secondary window keeps a `swap_rec_` pointing into that window's
manager; closing the window destroys it, and the next edit in the main
window calls into freed memory. `Rehome` from the wrong manager also
returns `nullptr`, silently disabling journaling for the primary's
buffers.

Fix: share one `SwapManager` across windows, and detach every buffer an
`Editor` attached when that `Editor` is destroyed.

### M4. Recursive undo-tree free overflows the stack **[repro]**

`UndoTree.cc:12` (`free_node_graph`), also `UndoSystem.cc:354`
(`is_descendant`, `dfs_find_parent`). Recursion depth equals history
length. About 300k nodes segfaults a debug build on an 8 MB stack;
replace-all over ~150k matches (two nodes each) followed by close or
reload is enough.

Fix: free iteratively with an explicit stack.

### M5. Jump-to-mark places the cursor past the end of the buffer

`Command.cc:3720`. Marks are not adjusted on edits and
`cmd_jump_to_mark` does not clamp. After deleting lines below a mark,
C-k j puts the cursor on a nonexistent row; `cmd_move_left` and
`cmd_move_up` (`:3866`, `:3988`) then index `rows[y]` out of bounds.

Fix: clamp in `cmd_jump_to_mark` and in `compute_mark_region`.

---

## D — Data integrity

### D1. Smart-newline undo deletes the wrong text **[repro]**

`Command.cc:3139-3146`. The indent is inserted and the cursor moved past
it before `u->Begin(UndoType::Insert)`; `UndoSystem::Begin` records the
cursor position, so the node is placed `indent.size()` columns too far
right. `"    foobar"` + SmartNewline + two undos yields
`"    foo    next"` (the following line's text is spliced in and `bar`
is lost).

Fix: `Begin` at `new_x`, then insert, then move the cursor; group the
newline and the indent.

### D2. Cursor column left past end-of-line corrupts undo

`cmd_scroll_up`/`cmd_scroll_down` (`Command.cc:4178`, `:4215`, bound to
the mouse wheel), replace-all's cursor restore (`:2580`), and
`cmd_jump_to_mark` call `SetCursor` without clamping x. `delete_text`
does not clamp; it treats the excess as crossing the newline.
Backspace then joins lines with an empty (dropped) undo node, and
typing followed by undo deletes a newline instead of the typed
character.

Fix: clamp x to the line length in `Buffer::SetCursor`.

### D3. `delete_row` on a final line without trailing newline leaves a stray `\n`

`Buffer.cc:803`. Affects `delete_region` (C-w, M-d, M-Backspace), regex
replace (`Command.cc:2996`), reflow (`:4884`), and the kill ring.
`"foo\nbar"` + M-d at end of `foo` yields `"foo\n"`.

### D4. Phantom empty row after a trailing newline

`PieceTable::LineCount()` counts the empty row after a final `\n`.
C-k C-d on that row records a `DeleteRow("")` whose undo inserts a
line (`Command.cc:3516`); regex replace of `^` matches it and appends
an extra line (`:2985`).

### D5. Missing or wrong undo grouping

- Plain replace-all (`Command.cc:2537-2577`) has no group: N
  replacements need 2N undos, and the final Insert stays pending so the
  next typed characters coalesce into it.
- Visual-line newline (`:3043-3075`) records no undo at all.

### D6. Reflow on a blank line merges the adjacent paragraphs

`Command.cc:4592`. `"aaa\n\nbbb"` with the cursor on the blank line and
ESC q gives `"aaa bbb"`.

### D7. Swap journal is fragile exactly where it matters

- `Swap.cc:1344-1373`: a torn final record (the power-loss case) makes
  the whole file unreadable instead of replaying the valid prefix.
- `Swap.cc:1134-1157`, `:1187`, `:1224`: after any dropped record
  (breaker open, >16 MB payload, partial write) later records still
  replay against a different base state; a partial write leaves bytes
  that later records are appended behind.
- `Swap.cc:1040` vs `:1187`: checkpoints allow 4 GB but the record
  length is 24 bits; buffers over 16 MB are never checkpointed and the
  journal grows without bound.
- `Swap.cc:644`, `:708`: a failed header write leaves an fd with no
  header; answering N to "delete corrupt swap?" appends new edits to the
  corrupt file.

### D8. Circuit breaker failure window never expires

`ErrorRecovery.cc:77`. `last_failure_time_` is updated before
`IsWindowExpired()` reads it, so elapsed time is always zero; five
failures hours apart still open the breaker.

### D9. Stale syntax highlighting after undo-to-clean or reload

`Buffer.h:391-400`. `version_` is bumped only in `SetDirty(true)`. Undo
back to the saved node calls `SetDirty(false)`, and `OpenFromFile`
(reload) never bumps it, so `HighlighterEngine` keeps serving spans for
the old text.

---

## B — Behaviour

### B1. `set_escdelay` is compiled out

`TerminalFrontend.cc:46`. `set_escdelay` is a function in ncurses, not a
macro, so `#ifdef set_escdelay` is always false and ESC waits the
1000 ms default.

### B2. Terminal resize runs C-g

`TerminalInputHandler.cc:167-171` maps `KEY_RESIZE` to
`CommandId::Refresh`, the cancel handler: it clears the mark, cancels
prompts (including the swap-recovery prompt, so the file is never
opened), and resets the universal argument.

### B3. ESC-as-Meta flag is not cleared on most paths

`TerminalInputHandler.cc:56-279`. ESC followed by an arrow, Enter, Tab,
or a control chord leaves `esc_meta` set; the next plain `d` runs M-d.
Same in `ImGuiInputHandler.cc:43-73`.

### B4. C-h shadows Backspace in the terminal

The generic control lookup (`TerminalInputHandler.cc:264`) maps ^H to
SearchReplace before the Backspace branch (`:307`) runs.

### B5. Byte-based editing splits UTF-8

`Command.cc:3259`, `:3352`, `:3861-3947`. Backspace, delete, and
horizontal motion step by byte: Backspace on `é` leaves a lone `0xC3`,
and C-b/C-f land inside code points.

### B6. Horizontal scroll counts bytes, renderer counts cells

`compute_render_x` (`Command.cc:52-60`) counts bytes while
`TerminalRenderer.cc:269-322` skips display cells: a long CJK line
scrolls to blank with the cursor off-screen.

### B7. GUI draws non-ASCII text as replacement glyphs

Highlighters emit one span per non-ASCII byte and `ImGuiRenderer.cc`
(~510) / `QtFrontend.cc` (~333) decode each span separately.

### B8. ImGui drops the character after Ctrl+V

`ImGuiInputHandler.cc:362`, `:382`, `:401` set a suppress-next-text flag
for chords that produce no text event.

### B9. Control characters mis-measured in the terminal

`TerminalRenderer.cc:314`, `:364`, `:413` count C0 controls as width 1;
ncurses draws them as `^X`.

### B10. Qt tab width disagrees with the command layer

`QtFrontend.cc:138` uses 4; `Command.cc` uses 8.

### B11. Smart newline edits the old buffer while a prompt is open

`Command.cc:3131`. With a prompt active, `cmd_newline` accepts it and
the saved indent is inserted through a possibly stale `Buffer*`.

### B12. Line count stale after merged appends **[repro]**

`PieceTable.cc:220-260`. `addPieceBack`/`addPieceFront` merge paths
skip `InvalidateLineIndex()`. Latent today.

### B13. Syntax highlighter edge cases

Go raw strings and Rust multi-line strings do not carry state across
lines; shell `$#` is highlighted as a comment; a C++ `#define` line
opening `/*` does not carry the comment.

---

## P — Performance

### P1. Whole-file `Rows()` rebuild per frame

Renderers call `buf->Rows()` every frame (`TerminalRenderer.cc:45`,
`ImGuiRenderer.cc:69`, `QtFrontend.cc:477`), and
`ensure_cursor_visible` (`Command.cc:91`) after every command. After an
edit this rebuilds a `std::string` per line, each via two `GetRange`
calls.

### P2. Whole-buffer materialization for one line

`Buffer::GetLineView` (`Buffer.cc:633`) calls `content_.Data()`, which
copies the entire file; the line index is rebuilt byte-by-byte after
every edit.

### P3. Re-highlight from row 0 on every keystroke

`SetDirty(true)` calls `InvalidateFrom(0)`; stateful highlighters then
re-lex from the top of the file to the viewport.

### P4. Quadratic region commands

Replace-all, `delete_region`, indent/unindent, reflow, and repeated
backspace re-fetch `Rows()` after each mutation inside their loops.

### P5. Opening a file checkpoints into a journal that is then deleted

`Swap.cc:493` snapshots, writes, and fsyncs the whole file into
`unnamed-N.swp` before removing it.

### P6. Boyer–Moore bad-character shift degenerates to 1

`OptimizedSearch.cc:63`, `:96`: when the mismatched byte is absent from
the pattern the shift is 1 instead of `j`. Test-only today.

---

## Recommended order

1. M1 (one-line fix)
2. M2 (remove or snapshot the warm-up thread)
3. D2 + M5 (clamp in `SetCursor` and jump-to-mark)
4. D1 (smart-newline undo order)
5. D7 (accept a torn final swap record)
6. P1/P2 (`Nrows()` + `GetLineString` in hot paths)

---

## Fix status

Updated as fixes land on `claude/funny-fermat-gj2vm5`.

| ID | Status |
|----|--------|
| M1 | open |
| M2 | open |
| M4 | open |
| M5 | open |
| D1 | open |
| D2 | open |
| D6 | open |
| D7 (torn tail) | open |
| D8 | open |
| D9 | open |
| B1 | open |
| B2 | open |
| B12 | open |