# 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`, 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 |