From 6c6c1b088bcc77127b63aedccce0196dee84c85c Mon Sep 17 00:00:00 2001 From: hazyhaar Date: Mon, 24 Aug 2026 20:16:53 +0200 Subject: [PATCH 01/10] perf(tui): bounded file read, LRU rendering cache, and size caps for full-file view Fixes #833: Decouple synchronous file reading and Chroma highlighting from View() render loop into a bounded cache keyed by target path, size, modtime and diff fingerprint. - Bound total source bytes consumed with io.LimitReader and immediate cutoff on oversized physical lines (> fileViewMaxBytes). - Distinguish omitted-lines trailer from clipped-lines wording when all lines are preserved up to line-length limits. - Propagate line truncation and isPrefix state when budget ends on an unterminated physical line. - Defer exact-budget truncation flag to trailing probe without false-positive truncation on complete files matching maxTotalBytes. - Ensure deterministic mtime cache invalidation with exact same-length content and explicit Chtimes. - Bound rendered ANSI variants per cache entry with a 4-slot LRU to prevent memory growth across window resizes or changed line mutations. - Validate thread-safe concurrent variant caching under -race. - Bound memory with 4000 lines / 1 MiB total / 4 KiB line limits and evict cleanly on theme changes. --- internal/tui/export_test.go | 9 + internal/tui/file_view.go | 440 ++++++++++++++++++++++++++++----- internal/tui/file_view_test.go | 398 +++++++++++++++++++++++++++++ internal/tui/theme_select.go | 3 + 4 files changed, 790 insertions(+), 60 deletions(-) diff --git a/internal/tui/export_test.go b/internal/tui/export_test.go index 49b057f59..bd4f0ae0d 100644 --- a/internal/tui/export_test.go +++ b/internal/tui/export_test.go @@ -144,6 +144,15 @@ func (c *staticRenderCache) stats() renderCacheStats { return c.statsData } +func fileViewCacheStatsForTest() fileViewCacheStats { + return defaultFileViewCache.stats() +} + +func resetFileViewCacheForTest() { + defaultFileViewCache.clear() + defaultFileViewCache.resetStats() +} + func renderSelectableList(options selectableListOptions) string { if len(options.Items) == 0 { return "" diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go index 7e544d70b..4e8337814 100644 --- a/internal/tui/file_view.go +++ b/internal/tui/file_view.go @@ -15,21 +15,398 @@ package tui import ( "bufio" + "container/list" + "errors" "fmt" + "io" "os" "path/filepath" + "sort" "strings" + "sync" + "time" ) // fileViewMaxLines caps the full-file mode so a giant generated file can't -// freeze a render; the tail collapses into a "… N more lines" trailer. -const fileViewMaxLines = 4000 +// freeze a render; the tail collapses into a "… more lines (file truncated at N for display)" trailer. +const ( + fileViewMaxLines = 4000 + fileViewMaxBytes = 1 << 20 // 1 MiB total read budget per file + fileViewMaxLineBytes = 4096 // 4 KiB max line length budget + defaultFileViewCacheMaxEntries = 64 + fileViewMaxRenderVariants = 4 // max rendered variants (width/fingerprint) per cached file entry +) const ( fileViewDiff = iota fileViewFull ) +// fileViewCacheStats tracks disk I/O, Chroma highlighting, and cache hits/misses. +type fileViewCacheStats struct { + DiskReads int + HighlightCalls int + CacheHits int + CacheMisses int + Evictions int + ThemeClears int + RenderEvictions int +} + +type fileViewCachedEntry struct { + targetPath string + displayPath string + modTime time.Time + size int64 + lines []string + display []string + truncated bool + omittedLines bool + + rendersMu sync.RWMutex + renderKeys []string // LRU order: oldest at index 0, most recent at end + renders map[string]string // key: "width:changedLinesFingerprint" -> formatted ANSI string +} + +func (e *fileViewCachedEntry) getRender(key string) (string, bool) { + e.rendersMu.RLock() + val, ok := e.renders[key] + e.rendersMu.RUnlock() + if !ok { + return "", false + } + e.rendersMu.Lock() + for i, k := range e.renderKeys { + if k == key { + e.renderKeys = append(append(e.renderKeys[:i], e.renderKeys[i+1:]...), key) + break + } + } + e.rendersMu.Unlock() + return val, true +} + +func (e *fileViewCachedEntry) putRender(key string, val string) { + e.rendersMu.Lock() + defer e.rendersMu.Unlock() + if _, ok := e.renders[key]; !ok { + for len(e.renders) >= fileViewMaxRenderVariants { + if len(e.renderKeys) > 0 { + oldKey := e.renderKeys[0] + e.renderKeys = e.renderKeys[1:] + delete(e.renders, oldKey) + } else { + for k := range e.renders { + delete(e.renders, k) + break + } + } + } + e.renderKeys = append(e.renderKeys, key) + } + e.renders[key] = val +} + +type fileViewRenderCache struct { + mu sync.Mutex + maxEntries int + items map[string]*list.Element // targetPath -> *list.Element containing *fileViewCachedEntry + lru *list.List + statsData fileViewCacheStats +} + +var defaultFileViewCache = newFileViewRenderCache(defaultFileViewCacheMaxEntries) + +func newFileViewRenderCache(maxEntries int) *fileViewRenderCache { + return &fileViewRenderCache{ + maxEntries: maxEntries, + items: make(map[string]*list.Element), + lru: list.New(), + } +} + +func (c *fileViewRenderCache) clear() { + c.mu.Lock() + defer c.mu.Unlock() + c.items = make(map[string]*list.Element) + c.lru.Init() +} + +func (c *fileViewRenderCache) resetStats() { + c.mu.Lock() + defer c.mu.Unlock() + c.statsData = fileViewCacheStats{} +} + +func (c *fileViewRenderCache) stats() fileViewCacheStats { + c.mu.Lock() + defer c.mu.Unlock() + return c.statsData +} + +type fileViewReadResult struct { + lines []string + truncated bool + omittedLines bool + err error +} + +func readFileViewBounded(path string, maxLines int, maxLineBytes int, maxTotalBytes int) fileViewReadResult { + file, err := os.Open(path) + if err != nil { + return fileViewReadResult{err: err} + } + defer file.Close() + + var lines []string + var totalSourceBytes int + truncated := false + omittedLines := false + + // Enforce hard source-byte read limit to avoid reading unbounded data from disk. + // We allow up to maxTotalBytes + 1 so we can detect truncation without reading to EOF. + limitReader := io.LimitReader(file, int64(maxTotalBytes)+1) + reader := bufio.NewReader(limitReader) + + for len(lines) < maxLines && totalSourceBytes < maxTotalBytes { + var lineBuf []byte + var lineTruncated bool + + for { + chunk, isPrefix, err := reader.ReadLine() + chunkLen := len(chunk) + if chunkLen > 0 { + remainTotal := maxTotalBytes - totalSourceBytes + if remainTotal <= 0 { + truncated = true + omittedLines = true + if len(lineBuf) > 0 { + lines = append(lines, string(lineBuf)) + } + goto finished + } + + if chunkLen > remainTotal { + chunk = chunk[:remainTotal] + totalSourceBytes += remainTotal + truncated = true + omittedLines = true + lineTruncated = true + } else { + totalSourceBytes += chunkLen + } + + remainLine := maxLineBytes - len(lineBuf) + if remainLine > 0 { + if len(chunk) > remainLine { + lineBuf = append(lineBuf, chunk[:remainLine]...) + lineTruncated = true + } else { + lineBuf = append(lineBuf, chunk...) + } + } else { + lineTruncated = true + } + } + + if err != nil { + if len(lineBuf) > 0 { + lines = append(lines, string(lineBuf)) + if lineTruncated { + truncated = true + } + } + if !errors.Is(err, io.EOF) { + if len(lines) == 0 { + return fileViewReadResult{err: err} + } + truncated = true + omittedLines = true + } + goto finished + } + + if totalSourceBytes >= maxTotalBytes { + if isPrefix { + truncated = true + omittedLines = true + } else if lineTruncated { + truncated = true + } + if len(lineBuf) > 0 { + lines = append(lines, string(lineBuf)) + } + goto finished + } + + if !isPrefix { + break + } + } + + if lineTruncated { + truncated = true + } + lines = append(lines, string(lineBuf)) + if totalSourceBytes >= maxTotalBytes { + break + } + } + +finished: + if !omittedLines { + if reader.Buffered() > 0 { + truncated = true + omittedLines = true + } else if _, err := reader.Peek(1); err == nil { + truncated = true + omittedLines = true + } else { + var probe [1]byte + if n, _ := file.Read(probe[:]); n > 0 { + truncated = true + omittedLines = true + } + } + } + + return fileViewReadResult{ + lines: lines, + truncated: truncated, + omittedLines: omittedLines, + } +} + +func changedLinesFingerprint(changed map[string]bool) string { + if len(changed) == 0 { + return "" + } + keys := make([]string, 0, len(changed)) + for k, v := range changed { + if v { + keys = append(keys, k) + } + } + sort.Strings(keys) + return strings.Join(keys, "\x00") +} + +func formatFileViewLines(lines []string, display []string, changed map[string]bool, truncated bool, omittedLines bool, width int) string { + gutterW := len(fmt.Sprintf("%d", len(lines))) + textBudget := maxInt(8, width-gutterW-3) // gutter + space + marker column + + var b strings.Builder + for i, line := range display { + line = fitStyledLine(line, textBudget) + if i > 0 { + b.WriteString("\n") + } + marker := " " + if changed != nil && len(lines) > i && changed[strings.TrimSpace(lines[i])] { + marker = zeroTheme.accent.Render("▎") + } + b.WriteString(zeroTheme.faintest.Render(fmt.Sprintf("%*d ", gutterW, i+1))) + b.WriteString(marker) + b.WriteString(line) + } + if truncated { + // No exact remaining-line count: computing one would require reading the + // rest of the file, defeating the bounded read above. + b.WriteString("\n") + if omittedLines { + b.WriteString(zeroTheme.faint.Render(fmt.Sprintf("… more lines (file truncated at %d for display)", len(lines)))) + } else { + b.WriteString(zeroTheme.faint.Render("… (line content truncated at display limit)")) + } + } + return b.String() +} + +func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, width int, changed map[string]bool) string { + stat, err := os.Stat(targetPath) + if err != nil { + return zeroTheme.faint.Render("Could not read file: " + err.Error()) + } + + modTime := stat.ModTime() + size := stat.Size() + changedFingerprint := changedLinesFingerprint(changed) + renderKey := fmt.Sprintf("%d:%s", width, changedFingerprint) + + c.mu.Lock() + if elem, ok := c.items[targetPath]; ok { + entry := elem.Value.(*fileViewCachedEntry) + if entry.modTime.Equal(modTime) && entry.size == size && entry.displayPath == displayPath { + c.statsData.CacheHits++ + c.lru.MoveToFront(elem) + c.mu.Unlock() + + if rendered, ok := entry.getRender(renderKey); ok { + return rendered + } + + // Re-format for the new width or changed markers using cached display and lines + rendered := formatFileViewLines(entry.lines, entry.display, changed, entry.truncated, entry.omittedLines, width) + entry.putRender(renderKey, rendered) + return rendered + } + } + + c.statsData.CacheMisses++ + c.statsData.DiskReads++ + c.mu.Unlock() + + readRes := readFileViewBounded(targetPath, fileViewMaxLines, fileViewMaxLineBytes, fileViewMaxBytes) + if readRes.err != nil && len(readRes.lines) == 0 { + return zeroTheme.faint.Render("Could not read file: " + readRes.err.Error()) + } + + c.mu.Lock() + c.statsData.HighlightCalls++ + c.mu.Unlock() + + display, ok := highlightCodeForPath(readRes.lines, displayPath, 1<<20, nil) + if !ok || len(display) != len(readRes.lines) { + display = readRes.lines + } + + rendered := formatFileViewLines(readRes.lines, display, changed, readRes.truncated, readRes.omittedLines, width) + + entry := &fileViewCachedEntry{ + targetPath: targetPath, + displayPath: displayPath, + modTime: modTime, + size: size, + lines: readRes.lines, + display: display, + truncated: readRes.truncated, + omittedLines: readRes.omittedLines, + renderKeys: []string{renderKey}, + renders: map[string]string{renderKey: rendered}, + } + + c.mu.Lock() + if elem, ok := c.items[targetPath]; ok { + c.lru.Remove(elem) + delete(c.items, targetPath) + } + elem := c.lru.PushFront(entry) + c.items[targetPath] = elem + + for len(c.items) > c.maxEntries { + back := c.lru.Back() + if back == nil { + break + } + backEntry := back.Value.(*fileViewCachedEntry) + delete(c.items, backEntry.targetPath) + c.lru.Remove(back) + } + c.mu.Unlock() + + return rendered +} + // fileViewState manages the drill-in view for a touched file. When active, the // transcript body swaps to the file's diff/content instead of the chat rows. type fileViewState struct { @@ -170,64 +547,7 @@ func (m model) renderFileViewFull(width int) string { if !filepath.IsAbs(target) { target = filepath.Join(m.cwd, target) } - // Stream the read and stop at the cap: os.ReadFile would load a multi-GB - // file wholesale before any truncation, which is the exact render freeze - // fileViewMaxLines exists to prevent. - file, err := os.Open(target) - if err != nil { - return zeroTheme.faint.Render("Could not read file: " + err.Error()) - } - defer file.Close() - var lines []string - truncated := false - scanner := bufio.NewScanner(file) - scanner.Buffer(make([]byte, 0, 64*1024), 16*1024*1024) - for scanner.Scan() { - if len(lines) == fileViewMaxLines { - truncated = true - break - } - lines = append(lines, scanner.Text()) - } - if err := scanner.Err(); err != nil { - if len(lines) == 0 { - return zeroTheme.faint.Render("Could not read file: " + err.Error()) - } - truncated = true // e.g. a single over-long line mid-file: show what we have - } - - changed := m.fileViewChangedLines() - gutterW := len(fmt.Sprintf("%d", len(lines))) - textBudget := maxInt(8, width-gutterW-3) // gutter + space + marker column - // Highlight with an effectively-infinite measure so the highlighter never - // wraps — output lines stay 1:1 with file lines and the gutter numbering - // can't desync. Each line is then truncated to the column budget below. - display, ok := highlightCodeForPath(lines, m.fileView.path, 1<<20, nil) - if !ok || len(display) != len(lines) { - display = lines // no lexer for this path: render plain - } - - var b strings.Builder - for i, line := range display { - line = fitStyledLine(line, textBudget) - if i > 0 { - b.WriteString("\n") - } - marker := " " - if changed[strings.TrimSpace(lines[i])] { - marker = zeroTheme.accent.Render("▎") - } - b.WriteString(zeroTheme.faintest.Render(fmt.Sprintf("%*d ", gutterW, i+1))) - b.WriteString(marker) - b.WriteString(line) - } - if truncated { - // No exact remaining-line count: computing one would require reading the - // rest of the file, defeating the bounded read above. - b.WriteString("\n") - b.WriteString(zeroTheme.faint.Render(fmt.Sprintf("… more lines (file truncated at %d for display)", len(lines)))) - } - return b.String() + return defaultFileViewCache.getOrRender(target, m.fileView.path, width, m.fileViewChangedLines()) } // fileViewChangedLines collects the trimmed text of every line the session's diff --git a/internal/tui/file_view_test.go b/internal/tui/file_view_test.go index dc39a9991..aec9e3775 100644 --- a/internal/tui/file_view_test.go +++ b/internal/tui/file_view_test.go @@ -7,7 +7,9 @@ import ( "path/filepath" "strconv" "strings" + "sync" "testing" + "time" tea "charm.land/bubbletea/v2" @@ -362,3 +364,399 @@ func TestFileViewKeysDeferToBlockingModal(t *testing.T) { t.Fatal("Esc with a permission prompt up must not exit the file view") } } + +// TestFileViewRepeatedViewNoDiskIOOrHighlighting proves that repeated calls +// to render the full file view do not perform disk I/O or Chroma syntax +// highlighting after the initial load, and that modifying the file on disk +// properly invalidates and triggers a reload. +func TestFileViewRepeatedViewNoDiskIOOrHighlighting(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "sample.go") + content := "package main\n\nfunc main() {\n\tprintln(\"hello world\")\n}\n" + if err := os.WriteFile(filePath, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = m.openFileView("sample.go") + m = m.setFileViewMode(fileViewFull) + + // First render: Misses cache, performs 1 disk read and 1 highlight call + firstRender := m.renderFileViewFull(80) + if !strings.Contains(firstRender, "hello world") { + t.Fatalf("first render missing content: %s", firstRender) + } + + statsAfterFirst := fileViewCacheStatsForTest() + if statsAfterFirst.DiskReads != 1 { + t.Fatalf("expected 1 disk read on initial view, got %d", statsAfterFirst.DiskReads) + } + if statsAfterFirst.HighlightCalls != 1 { + t.Fatalf("expected 1 highlight call on initial view, got %d", statsAfterFirst.HighlightCalls) + } + + // Repeated renders (e.g. 10 frames during typing/scrolling/resize) + for i := 0; i < 10; i++ { + rendered := m.renderFileViewFull(80) + if rendered != firstRender { + t.Fatalf("subsequent render %d mismatch", i) + } + } + + statsAfterRepeated := fileViewCacheStatsForTest() + if statsAfterRepeated.DiskReads != 1 { + t.Fatalf("repeated View calls must not trigger disk reads, got %d", statsAfterRepeated.DiskReads) + } + if statsAfterRepeated.HighlightCalls != 1 { + t.Fatalf("repeated View calls must not trigger Chroma highlighting, got %d", statsAfterRepeated.HighlightCalls) + } + if statsAfterRepeated.CacheHits != 10 { + t.Fatalf("expected 10 cache hits, got %d", statsAfterRepeated.CacheHits) + } + + // Same byte length as `content`, so only mtime can invalidate the entry. + newContent := "package main\n\nfunc main() {\n\tprintln(\"HELLO WORLD\")\n}\n" + if len(newContent) != len(content) { + t.Fatalf("test setup: newContent length %d must match original length %d", len(newContent), len(content)) + } + if err := os.WriteFile(filePath, []byte(newContent), 0o644); err != nil { + t.Fatal(err) + } + future := time.Now().Add(time.Hour) + if err := os.Chtimes(filePath, future, future); err != nil { + t.Fatal(err) + } + + updatedRender := m.renderFileViewFull(80) + if !strings.Contains(updatedRender, "HELLO WORLD") { + t.Fatalf("expected updated content after disk mutation, got: %s", updatedRender) + } + + statsAfterUpdate := fileViewCacheStatsForTest() + if statsAfterUpdate.DiskReads != 2 { + t.Fatalf("expected 2 disk reads after file change, got %d", statsAfterUpdate.DiskReads) + } + if statsAfterUpdate.HighlightCalls != 2 { + t.Fatalf("expected 2 highlight calls after file change, got %d", statsAfterUpdate.HighlightCalls) + } +} + +// TestFileViewMaxBytesBudgetTruncation verifies that files exceeding the +// total byte budget (fileViewMaxBytes) are truncated and display the trailer. +func TestFileViewMaxBytesBudgetTruncation(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "giant_bytes.txt") + + // Generate a file with total size ~1.5 MB (> fileViewMaxBytes of 1 MiB) + line := strings.Repeat("a", 500) + "\n" + numLines := (fileViewMaxBytes / 500) + 100 + var sb strings.Builder + for i := 0; i < numLines; i++ { + sb.WriteString(line) + } + if err := os.WriteFile(filePath, []byte(sb.String()), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = m.openFileView("giant_bytes.txt") + m = m.setFileViewMode(fileViewFull) + + body := m.renderFileViewFull(80) + plain := plainRender(t, body) + + if !strings.Contains(plain, "truncated") { + t.Fatalf("expected truncation trailer for file exceeding byte budget, got:\n%s", plain) + } + + renderedLines := strings.Split(plain, "\n") + if len(renderedLines) >= numLines { + t.Fatalf("rendered line count %d should be strictly less than total file lines %d", len(renderedLines), numLines) + } +} + +// TestFileViewMaxLineBytesBudgetTruncation verifies that single overlong lines +// exceeding fileViewMaxLineBytes are clamped without crashing or unbounded memory. +func TestFileViewMaxLineBytesBudgetTruncation(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "giant_line.js") + + // Generate a single overlong line of 50,000 bytes (> fileViewMaxLineBytes of 4096) + giantLine := "let data = \"" + strings.Repeat("x", 50000) + "\";\nlet next = 1;\n" + if err := os.WriteFile(filePath, []byte(giantLine), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = m.openFileView("giant_line.js") + m = m.setFileViewMode(fileViewFull) + + body := m.renderFileViewFull(80) + plain := plainRender(t, body) + + if !strings.Contains(plain, "let next = 1") { + t.Fatalf("expected next line to be readable after overlong line truncation, got:\n%s", plain) + } + if !strings.Contains(plain, "truncated") { + t.Fatalf("expected truncation trailer for overlong line, got:\n%s", plain) + } +} + +// TestFileViewCacheEviction verifies that the LRU cache caps entry count to +// defaultFileViewCacheMaxEntries. +func TestFileViewCacheEviction(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + numFiles := defaultFileViewCacheMaxEntries + 10 + for i := 0; i < numFiles; i++ { + fname := fmt.Sprintf("file_%d.txt", i) + if err := os.WriteFile(filepath.Join(dir, fname), []byte(fmt.Sprintf("content %d\n", i)), 0o644); err != nil { + t.Fatal(err) + } + } + + m := filesPanelTestModel() + m.cwd = dir + for i := 0; i < numFiles; i++ { + fname := fmt.Sprintf("file_%d.txt", i) + m = m.openFileView(fname) + m = m.setFileViewMode(fileViewFull) + _ = m.renderFileViewFull(80) + } + + defaultFileViewCache.mu.Lock() + cachedCount := len(defaultFileViewCache.items) + defaultFileViewCache.mu.Unlock() + + if cachedCount > defaultFileViewCacheMaxEntries { + t.Fatalf("cache size %d exceeded maxEntries %d", cachedCount, defaultFileViewCacheMaxEntries) + } +} + +// TestFileViewClearOnThemeChange verifies that switching themes clears the +// file view cache so updated palette styles are applied. +func TestFileViewClearOnThemeChange(t *testing.T) { + defer applyTheme(themeDark, true) + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "code.go") + if err := os.WriteFile(filePath, []byte("package main\nfunc main() {}\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = m.openFileView("code.go") + m = m.setFileViewMode(fileViewFull) + _ = m.renderFileViewFull(80) + + defaultFileViewCache.mu.Lock() + entriesBefore := len(defaultFileViewCache.items) + defaultFileViewCache.mu.Unlock() + + if entriesBefore == 0 { + t.Fatal("expected cached entries before theme switch") + } + + applyTheme(themeLight, false) + + defaultFileViewCache.mu.Lock() + entriesAfter := len(defaultFileViewCache.items) + defaultFileViewCache.mu.Unlock() + + if entriesAfter != 0 { + t.Fatalf("expected cache to be cleared after theme change, got %d entries", entriesAfter) + } +} + +// TestReadFileViewBounded_GiantSingleLineStopsAtBudget verifies that reading a +// multi-megabyte physical line without newlines stops immediately at maxTotalBytes +// rather than reading through to EOF or loading the entire line into memory. +func TestReadFileViewBounded_GiantSingleLineStopsAtBudget(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "giant_single_line.txt") + + // 5 MiB single line without any newlines + totalSize := 5 * 1024 * 1024 + giantContent := strings.Repeat("A", totalSize) + if err := os.WriteFile(filePath, []byte(giantContent), 0o644); err != nil { + t.Fatal(err) + } + + maxTotalBytes := 1 << 20 // 1 MiB budget + maxLineBytes := 4096 // 4 KiB line cap + maxLines := 4000 + + res := readFileViewBounded(filePath, maxLines, maxLineBytes, maxTotalBytes) + if res.err != nil { + t.Fatalf("unexpected read error: %v", res.err) + } + + if !res.truncated { + t.Fatal("expected truncated=true when reading 5 MiB single line with 1 MiB budget") + } + + if len(res.lines) != 1 { + t.Fatalf("expected exactly 1 truncated line, got %d", len(res.lines)) + } + + if len(res.lines[0]) > maxLineBytes { + t.Fatalf("retained line length %d exceeded per-line cap %d", len(res.lines[0]), maxLineBytes) + } +} + +// TestReadFileViewBounded_ExactMaxBytesNotTruncated verifies that a file exactly +// equal in size to maxTotalBytes without extra unread bytes is read completely +// without an erroneous truncation flag or trailer. +func TestReadFileViewBounded_ExactMaxBytesNotTruncated(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "exact_budget.txt") + + maxTotalBytes := 1024 * 1024 // 1 MiB + maxLineBytes := 4096 + maxLines := 4000 + + // Construct exactly 1024 * 1024 bytes with 512 lines of 2048 bytes (2047 chars + '\n') + lineLen := 2048 + numLines := maxTotalBytes / lineLen + remainder := maxTotalBytes % lineLen + + var sb strings.Builder + for i := 0; i < numLines; i++ { + sb.WriteString(strings.Repeat("B", lineLen-1) + "\n") + } + if remainder > 0 { + sb.WriteString(strings.Repeat("C", remainder)) + } + + content := []byte(sb.String()) + if len(content) != maxTotalBytes { + t.Fatalf("generated content size %d != %d", len(content), maxTotalBytes) + } + + if err := os.WriteFile(filePath, content, 0o644); err != nil { + t.Fatal(err) + } + + res := readFileViewBounded(filePath, maxLines, maxLineBytes, maxTotalBytes) + if res.err != nil { + t.Fatalf("unexpected read error: %v", res.err) + } + + if res.truncated { + t.Fatal("expected truncated=false for file exactly matching maxTotalBytes with no trailing data") + } + + totalReadLen := 0 + for _, l := range res.lines { + totalReadLen += len(l) + } + if totalReadLen == 0 { + t.Fatal("expected lines to be populated") + } +} + +// TestReadFileViewBounded_ExactMaxBytesUnterminatedLineTruncated verifies that a single +// unterminated physical line of exactly maxTotalBytes is correctly marked truncated=true +// when the line length exceeds maxLineBytes. +func TestReadFileViewBounded_ExactMaxBytesUnterminatedLineTruncated(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "exact_single_line_unterminated.txt") + + maxTotalBytes := 1024 * 1024 // 1 MiB + maxLineBytes := 4096 + maxLines := 4000 + + // 1 MiB continuous single line without any newlines + content := []byte(strings.Repeat("X", maxTotalBytes)) + if err := os.WriteFile(filePath, content, 0o644); err != nil { + t.Fatal(err) + } + + res := readFileViewBounded(filePath, maxLines, maxLineBytes, maxTotalBytes) + if res.err != nil { + t.Fatalf("unexpected read error: %v", res.err) + } + + if !res.truncated { + t.Fatal("expected truncated=true for 1 MiB single unterminated line clipped at maxLineBytes") + } + + if len(res.lines) != 1 { + t.Fatalf("expected exactly 1 line, got %d", len(res.lines)) + } + if len(res.lines[0]) > maxLineBytes { + t.Fatalf("retained line length %d exceeded maxLineBytes %d", len(res.lines[0]), maxLineBytes) + } +} + +// TestFileViewCache_RenderVariantsBoundedUnderResize verifies that varying width +// and changed-line fingerprints cannot grow an entry's renders map beyond +// fileViewMaxRenderVariants, even under concurrent access from multiple goroutines. +func TestFileViewCache_RenderVariantsBoundedUnderResize(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "resize_test.go") + if err := os.WriteFile(filePath, []byte("package main\n\nfunc main() {\n\tprintln(\"hello\")\n}\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = m.openFileView("resize_test.go") + m = m.setFileViewMode(fileViewFull) + + // Execute mixed-width getOrRender calls concurrently from multiple goroutines + var wg sync.WaitGroup + workers := 8 + callsPerWorker := 20 + + for w := 0; w < workers; w++ { + wg.Add(1) + go func(workerID int) { + defer wg.Done() + for i := 0; i < callsPerWorker; i++ { + width := 40 + ((workerID*callsPerWorker + i) % 50) + changed := map[string]bool{} + if width%2 == 0 { + changed[fmt.Sprintf("marker_%d_%d", workerID, width)] = true + } + _ = defaultFileViewCache.getOrRender(filePath, "resize_test.go", width, changed) + } + }(w) + } + wg.Wait() + + defaultFileViewCache.mu.Lock() + elem, ok := defaultFileViewCache.items[filePath] + defaultFileViewCache.mu.Unlock() + + if !ok || elem == nil { + t.Fatal("expected cached entry for file") + } + + entry := elem.Value.(*fileViewCachedEntry) + entry.rendersMu.RLock() + variantCount := len(entry.renders) + keyCount := len(entry.renderKeys) + entry.rendersMu.RUnlock() + + if variantCount > fileViewMaxRenderVariants { + t.Fatalf("variant count %d exceeded maximum limit %d", variantCount, fileViewMaxRenderVariants) + } + if keyCount > fileViewMaxRenderVariants { + t.Fatalf("renderKeys count %d exceeded maximum limit %d", keyCount, fileViewMaxRenderVariants) + } +} diff --git a/internal/tui/theme_select.go b/internal/tui/theme_select.go index f029ddaee..ef605d4bf 100644 --- a/internal/tui/theme_select.go +++ b/internal/tui/theme_select.go @@ -92,6 +92,9 @@ func applyTheme(mode themeMode, terminalDark bool) themeMode { if defaultRenderCache != nil { defaultRenderCache.clear() // old-palette entries must not be reused } + if defaultFileViewCache != nil { + defaultFileViewCache.clear() + } return resolved } From 69065987397d9012f50a8eba0b9dc8cdc535e547 Mon Sep 17 00:00:00 2001 From: hazyhaar Date: Thu, 27 Aug 2026 08:23:22 +0200 Subject: [PATCH 02/10] fix(tui): move cache-miss loading off the View path to unblock render loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address finding [P1] by moving synchronous file reading, os.Stat, Chroma syntax highlighting, and formatting out of renderFileViewFull/View() into an asynchronous tea.Cmd (loadFileViewCmd / loadAndRender). View() now returns immediately with in-memory content or a lightweight Loading… placeholder. The async result is safely applied in Update() only if matching the active file path, monotonic request ID, and cache generation (invalidated on theme switch). --- internal/tui/file_view.go | 157 +++++++++++++- internal/tui/file_view_test.go | 311 ++++++++++++++++++++++++--- internal/tui/files_git_sweep_test.go | 4 +- internal/tui/model.go | 11 +- 4 files changed, 435 insertions(+), 48 deletions(-) diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go index 4e8337814..489d28911 100644 --- a/internal/tui/file_view.go +++ b/internal/tui/file_view.go @@ -25,6 +25,8 @@ import ( "strings" "sync" "time" + + tea "charm.land/bubbletea/v2" ) // fileViewMaxLines caps the full-file mode so a giant generated file can't @@ -35,6 +37,7 @@ const ( fileViewMaxLineBytes = 4096 // 4 KiB max line length budget defaultFileViewCacheMaxEntries = 64 fileViewMaxRenderVariants = 4 // max rendered variants (width/fingerprint) per cached file entry + fileViewLoadingPlaceholder = "Loading…" ) const ( @@ -112,6 +115,7 @@ type fileViewRenderCache struct { maxEntries int items map[string]*list.Element // targetPath -> *list.Element containing *fileViewCachedEntry lru *list.List + gen int statsData fileViewCacheStats } @@ -128,8 +132,16 @@ func newFileViewRenderCache(maxEntries int) *fileViewRenderCache { func (c *fileViewRenderCache) clear() { c.mu.Lock() defer c.mu.Unlock() + c.gen++ c.items = make(map[string]*list.Element) c.lru.Init() + c.statsData.ThemeClears++ +} + +func (c *fileViewRenderCache) generation() int { + c.mu.Lock() + defer c.mu.Unlock() + return c.gen } func (c *fileViewRenderCache) resetStats() { @@ -322,10 +334,39 @@ func formatFileViewLines(lines []string, display []string, changed map[string]bo return b.String() } -func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, width int, changed map[string]bool) string { +// getRenderOnly looks up an already-rendered variant in memory (or formats from +// already-cached in-memory syntax tokens) without performing any disk I/O, stat, +// or Chroma syntax highlighting. Safe for direct View calls. +func (c *fileViewRenderCache) getRenderOnly(targetPath string, width int, changed map[string]bool) (string, bool) { + c.mu.Lock() + elem, ok := c.items[targetPath] + if !ok { + c.mu.Unlock() + return "", false + } + entry := elem.Value.(*fileViewCachedEntry) + c.lru.MoveToFront(elem) + c.statsData.CacheHits++ + c.mu.Unlock() + + renderKey := fmt.Sprintf("%d:%s", width, changedLinesFingerprint(changed)) + if val, ok := entry.getRender(renderKey); ok { + return val, true + } + + rendered := formatFileViewLines(entry.lines, entry.display, changed, entry.truncated, entry.omittedLines, width) + entry.putRender(renderKey, rendered) + return rendered, true +} + +// loadAndRender performs the bounded read, Chroma highlighting, and formatting +// on a cache miss (or re-formats for a new width variant on a cache hit). It is +// intended to be executed from a tea.Cmd / background worker, off the View path. +func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath string, width int, changed map[string]bool, reqGen int) (string, error) { stat, err := os.Stat(targetPath) if err != nil { - return zeroTheme.faint.Render("Could not read file: " + err.Error()) + rendered := zeroTheme.faint.Render("Could not read file: " + err.Error()) + return rendered, err } modTime := stat.ModTime() @@ -334,6 +375,11 @@ func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, renderKey := fmt.Sprintf("%d:%s", width, changedFingerprint) c.mu.Lock() + if c.gen != reqGen { + c.mu.Unlock() + return "", errors.New("request superseded by cache invalidation") + } + if elem, ok := c.items[targetPath]; ok { entry := elem.Value.(*fileViewCachedEntry) if entry.modTime.Equal(modTime) && entry.size == size && entry.displayPath == displayPath { @@ -342,13 +388,13 @@ func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, c.mu.Unlock() if rendered, ok := entry.getRender(renderKey); ok { - return rendered + return rendered, nil } // Re-format for the new width or changed markers using cached display and lines rendered := formatFileViewLines(entry.lines, entry.display, changed, entry.truncated, entry.omittedLines, width) entry.putRender(renderKey, rendered) - return rendered + return rendered, nil } } @@ -358,7 +404,8 @@ func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, readRes := readFileViewBounded(targetPath, fileViewMaxLines, fileViewMaxLineBytes, fileViewMaxBytes) if readRes.err != nil && len(readRes.lines) == 0 { - return zeroTheme.faint.Render("Could not read file: " + readRes.err.Error()) + rendered := zeroTheme.faint.Render("Could not read file: " + readRes.err.Error()) + return rendered, readRes.err } c.mu.Lock() @@ -386,6 +433,11 @@ func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, } c.mu.Lock() + if c.gen != reqGen { + c.mu.Unlock() + return "", errors.New("request superseded by cache invalidation") + } + if elem, ok := c.items[targetPath]; ok { c.lru.Remove(elem) delete(c.items, targetPath) @@ -404,9 +456,40 @@ func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, } c.mu.Unlock() + return rendered, nil +} + +func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, width int, changed map[string]bool) string { + rendered, _ := c.loadAndRender(targetPath, displayPath, width, changed, c.generation()) return rendered } +// fileViewLoadedMsg delivers the result of an asynchronous file read & render. +type fileViewLoadedMsg struct { + requestID int + generation int + targetPath string + displayPath string + width int + rendered string + err error +} + +func loadFileViewCmd(targetPath string, displayPath string, width int, changed map[string]bool, requestID int, gen int) tea.Cmd { + return func() tea.Msg { + rendered, err := defaultFileViewCache.loadAndRender(targetPath, displayPath, width, changed, gen) + return fileViewLoadedMsg{ + requestID: requestID, + generation: gen, + targetPath: targetPath, + displayPath: displayPath, + width: width, + rendered: rendered, + err: err, + } + } +} + // fileViewState manages the drill-in view for a touched file. When active, the // transcript body swaps to the file's diff/content instead of the chat rows. type fileViewState struct { @@ -416,6 +499,27 @@ type fileViewState struct { // parentScrollOffset preserves the chat scroll position so closing the view // returns to the same spot (mirrors subchatState). parentScrollOffset int + requestID int // monotonic ID for async load requests + renderedContent string // rendered full text when loaded + loadedPath string // path of loaded content + loadedWidth int // width of loaded content + loading bool // true while async load is in flight +} + +func (m model) startFileViewLoadCmd(width int) (model, tea.Cmd) { + if !m.fileView.active || m.fileView.mode != fileViewFull || m.fileView.path == "" { + return m, nil + } + target := m.fileView.path + if !filepath.IsAbs(target) { + target = filepath.Join(m.cwd, target) + } + m.fileView.requestID++ + m.fileView.loading = true + reqID := m.fileView.requestID + gen := defaultFileViewCache.generation() + changed := m.fileViewChangedLines() + return m, loadFileViewCmd(target, m.fileView.path, width, changed, reqID, gen) } // openFileView activates the drill-in for path in diff mode. Opening from an @@ -424,9 +528,9 @@ type fileViewState struct { // Re-opening the file that is ALREADY being viewed is a no-op: a stray // re-click must not bounce the user from full mode back to diff or reset // their scroll position. -func (m model) openFileView(path string) model { +func (m model) openFileView(path string) (model, tea.Cmd) { if m.fileView.active && m.fileView.path == path { - return m + return m, nil } if !m.fileView.active { m.fileView.parentScrollOffset = m.chatScrollOffset @@ -441,7 +545,10 @@ func (m model) openFileView(path string) model { } m.chatScrollOffset = 0 m = m.clearHover() // bodyY numbering differs between the file body and the chat - return m + if m.fileView.mode == fileViewFull { + return m.startFileViewLoadCmd(m.chatColumnWidth()) + } + return m, nil } // exitFileView deactivates the view and restores the chat scroll position. @@ -457,12 +564,32 @@ func (m model) exitFileView() model { // setFileViewMode switches diff/full, resetting the scroll to the bottom-anchored // start since the two bodies have unrelated heights. -func (m model) setFileViewMode(mode int) model { +func (m model) setFileViewMode(mode int) (model, tea.Cmd) { if !m.fileView.active || m.fileView.mode == mode { - return m + return m, nil } m.fileView.mode = mode m.chatScrollOffset = 0 + if mode == fileViewFull { + return m.startFileViewLoadCmd(m.chatColumnWidth()) + } + return m, nil +} + +func (m model) handleFileViewLoaded(msg fileViewLoadedMsg) model { + if !m.fileView.active || m.fileView.mode != fileViewFull { + return m + } + if m.fileView.path != msg.displayPath || m.fileView.requestID != msg.requestID { + return m + } + if msg.generation != defaultFileViewCache.generation() { + return m + } + m.fileView.loading = false + m.fileView.renderedContent = msg.rendered + m.fileView.loadedPath = msg.displayPath + m.fileView.loadedWidth = msg.width return m } @@ -542,12 +669,20 @@ func (m model) renderFileViewDiff(width int) string { // highlighted, with a line-number gutter and an accent ▎ marker on the lines // this session's diffs added (matched by exact text — an approximation that // tolerates later drift; a stale marker just doesn't highlight). +// It is read-only and non-blocking: if content is ready or cached in memory, +// it is returned immediately; otherwise a loading placeholder is shown. func (m model) renderFileViewFull(width int) string { target := m.fileView.path if !filepath.IsAbs(target) { target = filepath.Join(m.cwd, target) } - return defaultFileViewCache.getOrRender(target, m.fileView.path, width, m.fileViewChangedLines()) + if cached, ok := defaultFileViewCache.getRenderOnly(target, width, m.fileViewChangedLines()); ok { + return cached + } + if m.fileView.renderedContent != "" && m.fileView.loadedPath == m.fileView.path { + return m.fileView.renderedContent + } + return zeroTheme.faint.Render(fileViewLoadingPlaceholder) } // fileViewChangedLines collects the trimmed text of every line the session's diff --git a/internal/tui/file_view_test.go b/internal/tui/file_view_test.go index aec9e3775..d8d8a356a 100644 --- a/internal/tui/file_view_test.go +++ b/internal/tui/file_view_test.go @@ -18,6 +18,26 @@ import ( "github.com/Gitlawb/zero/internal/tools" ) +func testOpenFile(m model, path string) model { + next, cmd := m.openFileView(path) + if cmd != nil { + msg := cmd() + updated, _ := next.Update(msg) + return updated.(model) + } + return next +} + +func testSetMode(m model, mode int) model { + next, cmd := m.setFileViewMode(mode) + if cmd != nil { + msg := cmd() + updated, _ := next.Update(msg) + return updated.(model) + } + return next +} + // TestFileViewOpenExitRestoresScroll: opening saves the chat scroll position, // resets it for the file body, and Esc restores it; switching files while open // keeps the ORIGINAL saved position (not the file view's own). @@ -25,7 +45,7 @@ func TestFileViewOpenExitRestoresScroll(t *testing.T) { m := filesPanelTestModel() m.chatScrollOffset = 12 - m = m.openFileView("web/app.js") + m, _ = m.openFileView("web/app.js") if !m.fileView.active || m.fileView.mode != fileViewDiff { t.Fatalf("open should activate in diff mode: %+v", m.fileView) } @@ -34,7 +54,7 @@ func TestFileViewOpenExitRestoresScroll(t *testing.T) { } m.chatScrollOffset = 5 // scrolled within the file body - m = m.openFileView("internal/tui/sidebar.go") + m, _ = m.openFileView("internal/tui/sidebar.go") if m.fileView.parentScrollOffset != 12 { t.Fatalf("switching files must keep the original parent offset, got %d", m.fileView.parentScrollOffset) } @@ -49,15 +69,23 @@ func TestFileViewOpenExitRestoresScroll(t *testing.T) { // d/f switch modes while the composer is empty and never while typing. func TestFileViewEscAndModeKeys(t *testing.T) { m := filesPanelTestModel() - m = m.openFileView("web/app.js") + m, _ = m.openFileView("web/app.js") - updated, _ := m.Update(tea.KeyPressMsg{Code: 'f', Text: "f"}) + updated, cmd := m.Update(tea.KeyPressMsg{Code: 'f', Text: "f"}) m = updated.(model) + if cmd != nil { + updated, _ = m.Update(cmd()) + m = updated.(model) + } if m.fileView.mode != fileViewFull { t.Fatal("f should switch to full mode") } - updated, _ = m.Update(tea.KeyPressMsg{Code: 'd', Text: "d"}) + updated, cmd = m.Update(tea.KeyPressMsg{Code: 'd', Text: "d"}) m = updated.(model) + if cmd != nil { + updated, _ = m.Update(cmd()) + m = updated.(model) + } if m.fileView.mode != fileViewDiff { t.Fatal("d should switch back to diff mode") } @@ -83,7 +111,7 @@ func TestFileViewEscAndModeKeys(t *testing.T) { // placeholder. func TestFileViewDiffBody(t *testing.T) { m := filesPanelTestModel() - m = m.openFileView("internal/tui/sidebar.go") + m, _ = m.openFileView("internal/tui/sidebar.go") body := plainRender(t, m.renderFileViewDiff(78)) if !strings.Contains(body, "edit 1 of 2") || !strings.Contains(body, "edit 2 of 2") { t.Fatalf("expected chronological edit labels:\n%s", body) @@ -113,8 +141,8 @@ func TestFileViewFullBody(t *testing.T) { detail: "+let a = 1", changedFiles: []string{"app.js"}, }) - m = m.openFileView("app.js") - m = m.setFileViewMode(fileViewFull) + m = testOpenFile(m, "app.js") + m = testSetMode(m, fileViewFull) body := m.renderFileViewFull(78) plain := plainRender(t, body) @@ -133,6 +161,11 @@ func TestFileViewFullBody(t *testing.T) { } m.fileView.path = "gone.js" + m, cmd := m.startFileViewLoadCmd(78) + if cmd != nil { + updated, _ := m.Update(cmd()) + m = updated.(model) + } if got := plainRender(t, m.renderFileViewFull(78)); !strings.Contains(got, "Could not read file") { t.Errorf("missing file should degrade to an error line, got:\n%s", got) } @@ -144,7 +177,7 @@ func TestFileViewFullBody(t *testing.T) { // relies on. func TestFileViewSwapsTranscriptBody(t *testing.T) { m := filesPanelTestModel() - m = m.openFileView("internal/tui/sidebar.go") + m, _ = m.openFileView("internal/tui/sidebar.go") items := m.transcriptBodyItems(m.chatColumnWidth(), "", false) if len(items) != 1 { @@ -179,7 +212,7 @@ func TestSidebarAgentClickIsIgnoredWithoutRail(t *testing.T) { transcriptRow{kind: rowToolResult, tool: "swarm_spawn", detail: "Spawned subagent as task subagent-1 on team default.", runID: 1}, ) m.activeRunID = 1 - m = m.openFileView("web/app.js") + m, _ = m.openFileView("web/app.js") width := sidebarWidth(m.width) agents := m.sidebarAgentSelectables(width) @@ -248,11 +281,11 @@ func TestResumedFileEditUsesPersistedDisplayPreview(t *testing.T) { // unconditional openFileView bounced full mode back to diff and reset scroll. func TestOpenFileViewSamePathIsNoOp(t *testing.T) { m := filesPanelTestModel() - m = m.openFileView("web/app.js") - m = m.setFileViewMode(fileViewFull) + m = testOpenFile(m, "web/app.js") + m = testSetMode(m, fileViewFull) m.chatScrollOffset = 7 // scrolled within the file body - m = m.openFileView("web/app.js") + m, _ = m.openFileView("web/app.js") if m.fileView.mode != fileViewFull { t.Fatal("re-opening the same file must keep full mode") } @@ -260,7 +293,7 @@ func TestOpenFileViewSamePathIsNoOp(t *testing.T) { t.Fatalf("re-opening the same file must keep the scroll, got %d", m.chatScrollOffset) } // A DIFFERENT file still switches (and resets to diff mode as documented). - m = m.openFileView("internal/tui/sidebar.go") + m, _ = m.openFileView("internal/tui/sidebar.go") if m.fileView.path != "internal/tui/sidebar.go" || m.fileView.mode != fileViewDiff { t.Fatalf("opening another file should switch views: %+v", m.fileView) } @@ -281,7 +314,7 @@ func TestFileViewFullBodyTruncatesLongFile(t *testing.T) { m := filesPanelTestModel() m.cwd = dir m.gitTouched = []gitSweepFile{{path: "big.txt"}} - m = m.openFileView("big.txt") + m = testOpenFile(m, "big.txt") plain := plainRender(t, m.renderFileViewFull(80)) lines := strings.Split(plain, "\n") @@ -299,7 +332,7 @@ func TestFileViewFullBodyTruncatesLongFile(t *testing.T) { func TestDetailedTranscriptClosesFileView(t *testing.T) { m := filesPanelTestModel() m.altScreen = true - m = m.openFileView("web/app.js") + m, _ = m.openFileView("web/app.js") if !m.fileView.active { t.Fatal("sanity check: openFileView should activate the file view") @@ -326,7 +359,7 @@ func TestDetailedTranscriptClosesFileView(t *testing.T) { func TestDetailedTranscriptStaysClosedOnSecondToggle(t *testing.T) { m := filesPanelTestModel() m.altScreen = true - m = m.openFileView("web/app.js") + m, _ = m.openFileView("web/app.js") updated, _ := m.Update(testKeyCtrl('o')) m = updated.(model) @@ -347,7 +380,7 @@ func TestDetailedTranscriptStaysClosedOnSecondToggle(t *testing.T) { // (Esc exiting the view instead of reaching the prompt's deny handling). func TestFileViewKeysDeferToBlockingModal(t *testing.T) { m := filesPanelTestModel() - m = m.openFileView("web/app.js") + m, _ = m.openFileView("web/app.js") m.pendingPermission = &pendingPermissionPrompt{ request: agent.PermissionRequest{ToolName: "write_file"}, decide: func(agent.PermissionDecision) {}, @@ -381,10 +414,10 @@ func TestFileViewRepeatedViewNoDiskIOOrHighlighting(t *testing.T) { m := filesPanelTestModel() m.cwd = dir - m = m.openFileView("sample.go") - m = m.setFileViewMode(fileViewFull) + m = testOpenFile(m, "sample.go") + m = testSetMode(m, fileViewFull) - // First render: Misses cache, performs 1 disk read and 1 highlight call + // First render: Misses cache during testSetMode cmd execution, performed 1 disk read and 1 highlight call firstRender := m.renderFileViewFull(80) if !strings.Contains(firstRender, "hello world") { t.Fatalf("first render missing content: %s", firstRender) @@ -413,8 +446,8 @@ func TestFileViewRepeatedViewNoDiskIOOrHighlighting(t *testing.T) { if statsAfterRepeated.HighlightCalls != 1 { t.Fatalf("repeated View calls must not trigger Chroma highlighting, got %d", statsAfterRepeated.HighlightCalls) } - if statsAfterRepeated.CacheHits != 10 { - t.Fatalf("expected 10 cache hits, got %d", statsAfterRepeated.CacheHits) + if statsAfterRepeated.CacheHits != statsAfterFirst.CacheHits+10 { + t.Fatalf("expected 10 additional cache hits, got %d (before: %d)", statsAfterRepeated.CacheHits, statsAfterFirst.CacheHits) } // Same byte length as `content`, so only mtime can invalidate the entry. @@ -430,6 +463,13 @@ func TestFileViewRepeatedViewNoDiskIOOrHighlighting(t *testing.T) { t.Fatal(err) } + // Re-trigger load command after file update + m, cmd := m.startFileViewLoadCmd(80) + if cmd != nil { + updated, _ := m.Update(cmd()) + m = updated.(model) + } + updatedRender := m.renderFileViewFull(80) if !strings.Contains(updatedRender, "HELLO WORLD") { t.Fatalf("expected updated content after disk mutation, got: %s", updatedRender) @@ -465,8 +505,8 @@ func TestFileViewMaxBytesBudgetTruncation(t *testing.T) { m := filesPanelTestModel() m.cwd = dir - m = m.openFileView("giant_bytes.txt") - m = m.setFileViewMode(fileViewFull) + m = testOpenFile(m, "giant_bytes.txt") + m = testSetMode(m, fileViewFull) body := m.renderFileViewFull(80) plain := plainRender(t, body) @@ -497,8 +537,8 @@ func TestFileViewMaxLineBytesBudgetTruncation(t *testing.T) { m := filesPanelTestModel() m.cwd = dir - m = m.openFileView("giant_line.js") - m = m.setFileViewMode(fileViewFull) + m = testOpenFile(m, "giant_line.js") + m = testSetMode(m, fileViewFull) body := m.renderFileViewFull(80) plain := plainRender(t, body) @@ -529,8 +569,8 @@ func TestFileViewCacheEviction(t *testing.T) { m.cwd = dir for i := 0; i < numFiles; i++ { fname := fmt.Sprintf("file_%d.txt", i) - m = m.openFileView(fname) - m = m.setFileViewMode(fileViewFull) + m = testOpenFile(m, fname) + m = testSetMode(m, fileViewFull) _ = m.renderFileViewFull(80) } @@ -557,8 +597,8 @@ func TestFileViewClearOnThemeChange(t *testing.T) { m := filesPanelTestModel() m.cwd = dir - m = m.openFileView("code.go") - m = m.setFileViewMode(fileViewFull) + m = testOpenFile(m, "code.go") + m = testSetMode(m, fileViewFull) _ = m.renderFileViewFull(80) defaultFileViewCache.mu.Lock() @@ -715,8 +755,8 @@ func TestFileViewCache_RenderVariantsBoundedUnderResize(t *testing.T) { m := filesPanelTestModel() m.cwd = dir - m = m.openFileView("resize_test.go") - m = m.setFileViewMode(fileViewFull) + m = testOpenFile(m, "resize_test.go") + m = testSetMode(m, fileViewFull) // Execute mixed-width getOrRender calls concurrently from multiple goroutines var wg sync.WaitGroup @@ -760,3 +800,208 @@ func TestFileViewCache_RenderVariantsBoundedUnderResize(t *testing.T) { t.Fatalf("renderKeys count %d exceeded maximum limit %d", keyCount, fileViewMaxRenderVariants) } } + +// TestFileViewAsyncCacheMissLifecycle exercises the cache-miss lifecycle through +// the actual View/Update boundary: +// 1. Initial full-mode activation returns a command while View() renders the +// loading placeholder without performing disk I/O or Chroma work. +// 2. The command executes asynchronously and returns fileViewLoadedMsg. +// 3. Update() applies the message to the model. +// 4. View() renders the loaded, formatted content. +func TestFileViewAsyncCacheMissLifecycle(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "async_sample.go") + content := "package main\n\nfunc AsyncWork() string {\n\treturn \"done\"\n}\n" + if err := os.WriteFile(filePath, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + // Step 1: Open file with no edit rows (opens in full mode directly with async cmd) + m, cmd := m.openFileView("async_sample.go") + if cmd == nil { + t.Fatal("expected async load command on cache miss") + } + + // View before cmd completes must render loading placeholder with 0 disk I/O or highlighting + initialView := m.renderFileViewFull(80) + if !strings.Contains(initialView, "Loading…") { + t.Fatalf("expected loading placeholder before command completes, got:\n%s", initialView) + } + statsBefore := fileViewCacheStatsForTest() + if statsBefore.DiskReads != 0 || statsBefore.HighlightCalls != 0 { + t.Fatalf("View() must not stat/read/highlight directly: %+v", statsBefore) + } + + // Step 2: Execute command asynchronously + msg := cmd() + loadedMsg, ok := msg.(fileViewLoadedMsg) + if !ok { + t.Fatalf("expected fileViewLoadedMsg, got %T", msg) + } + if loadedMsg.err != nil { + t.Fatalf("unexpected load error: %v", loadedMsg.err) + } + + // Step 3: Update model with loaded message + updated, _ := m.Update(loadedMsg) + m = updated.(model) + + // Step 4: View now renders the loaded content + loadedView := m.renderFileViewFull(80) + if !strings.Contains(loadedView, "AsyncWork") { + t.Fatalf("expected loaded content in view, got:\n%s", loadedView) + } + statsAfter := fileViewCacheStatsForTest() + if statsAfter.DiskReads != 1 || statsAfter.HighlightCalls != 1 { + t.Fatalf("expected exactly 1 disk read and 1 highlight call, got: %+v", statsAfter) + } + + // Also verify switching from diff mode to full mode triggers the cmd + appFile := filepath.Join(dir, "web", "app.js") + if err := os.MkdirAll(filepath.Join(dir, "web"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(appFile, []byte("let webApp = true;\n"), 0o644); err != nil { + t.Fatal(err) + } + mDiff, cmdDiff := m.openFileView("web/app.js") + if cmdDiff != nil || mDiff.fileView.mode != fileViewDiff { + t.Fatalf("file with edit cards must open in diff mode with nil cmd: mode=%d, cmd=%v", mDiff.fileView.mode, cmdDiff) + } + mFull, cmdFull := mDiff.setFileViewMode(fileViewFull) + if cmdFull == nil || mFull.fileView.mode != fileViewFull { + t.Fatal("switching to full mode must return load command") + } + updated, _ = mFull.Update(cmdFull()) + mFull = updated.(model) + if !strings.Contains(mFull.renderFileViewFull(80), "webApp") { + t.Fatalf("expected loaded webApp content, got: %s", mFull.renderFileViewFull(80)) + } +} + +// TestFileViewAsyncDiscardSupersededResult verifies that if a user switches files +// or exits the view while an async load is in flight, the completed message from +// the old file is safely discarded and does not overwrite the active view. +func TestFileViewAsyncDiscardSupersededResult(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + fileA := filepath.Join(dir, "fileA.txt") + fileB := filepath.Join(dir, "fileB.txt") + if err := os.WriteFile(fileA, []byte("Content of File A\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(fileB, []byte("Content of File B\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + // Start loading File A + m, cmdA := m.openFileView("fileA.txt") + if cmdA == nil { + t.Fatal("expected cmd for fileA") + } + + // User switches to File B before cmdA returns + m, cmdB := m.openFileView("fileB.txt") + if cmdB == nil { + t.Fatal("expected cmd for fileB") + } + + // Now cmdA completes and its message is dispatched + msgA := cmdA() + updated, _ := m.Update(msgA) + m = updated.(model) + + // File A's result must be discarded because active file is File B + viewWhileB := m.renderFileViewFull(80) + if strings.Contains(viewWhileB, "Content of File A") { + t.Fatalf("stale File A result must not paint over File B: %s", viewWhileB) + } + + // Now cmdB completes and is dispatched + msgB := cmdB() + updated, _ = m.Update(msgB) + m = updated.(model) + + viewFinal := m.renderFileViewFull(80) + if !strings.Contains(viewFinal, "Content of File B") { + t.Fatalf("expected File B content, got: %s", viewFinal) + } +} + +// TestFileViewAsyncDiscardOnModeSwitchOrExit verifies that if a view exits or +// switches back to diff mode, completed async loads are safely ignored. +func TestFileViewAsyncDiscardOnModeSwitchOrExit(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + fileA := filepath.Join(dir, "discard_mode.txt") + if err := os.WriteFile(fileA, []byte("Some content\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + m, cmdA := m.openFileView("discard_mode.txt") + if cmdA == nil { + t.Fatal("expected cmd") + } + + // Exit file view before command returns + m = m.exitFileView() + if m.fileView.active { + t.Fatal("view should be inactive") + } + + // Now deliver the message + msgA := cmdA() + updated, _ := m.Update(msgA) + m = updated.(model) + + if m.fileView.active { + t.Fatal("discarded message must not re-activate file view") + } +} + +// TestFileViewAsyncDiscardOnThemeInvalidation verifies that if a theme switch occurs +// while an async load is in flight, the old theme's completion message is discarded. +func TestFileViewAsyncDiscardOnThemeInvalidation(t *testing.T) { + defer applyTheme(themeDark, true) + resetFileViewCacheForTest() + + dir := t.TempDir() + fileA := filepath.Join(dir, "theme_test.go") + if err := os.WriteFile(fileA, []byte("package main\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + m, cmd := m.openFileView("theme_test.go") + if cmd == nil { + t.Fatal("expected cmd") + } + + // Invalidate cache by switching theme before cmd completes + applyTheme(themeLight, false) + + // Now old cmd completes + msg := cmd() + updated, _ := m.Update(msg) + m = updated.(model) + + // The message must have been rejected due to generation mismatch + if m.fileView.renderedContent != "" { + t.Fatalf("expected empty renderedContent after invalidation, got %q", m.fileView.renderedContent) + } +} diff --git a/internal/tui/files_git_sweep_test.go b/internal/tui/files_git_sweep_test.go index cccdcb4d9..f2f2f6e2f 100644 --- a/internal/tui/files_git_sweep_test.go +++ b/internal/tui/files_git_sweep_test.go @@ -152,12 +152,12 @@ func TestTouchedFilesMergesGitSweep(t *testing.T) { func TestOpenFileViewGitOnlyFallsBackToFull(t *testing.T) { m := filesPanelTestModel() m.gitTouched = []gitSweepFile{{path: "kanban/board.tsx", created: true}} - m = m.openFileView("kanban/board.tsx") + m, _ = m.openFileView("kanban/board.tsx") if m.fileView.mode != fileViewFull { t.Fatal("git-only file should open in full mode") } m = m.exitFileView() - m = m.openFileView("web/app.js") + m, _ = m.openFileView("web/app.js") if m.fileView.mode != fileViewDiff { t.Fatal("a file with edit cards still opens in diff mode") } diff --git a/internal/tui/model.go b/internal/tui/model.go index f8b220341..a503cf53a 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -1362,6 +1362,8 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { return next, cmd } switch msg := msg.(type) { + case fileViewLoadedMsg: + return m.handleFileViewLoaded(msg), nil case uv.CellSizeEvent: if msg.Width > 0 && msg.Height > 0 { m.petCellPixelWidth = msg.Width @@ -1659,9 +1661,9 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { // (so mid-sentence typing is never hijacked) and no modal is up (so a // permission prompt / ask-user / wizard keeps its own key handling). if keyText(msg) == "f" { - return m.setFileViewMode(fileViewFull), nil + return m.setFileViewMode(fileViewFull) } - return m.setFileViewMode(fileViewDiff), nil + return m.setFileViewMode(fileViewDiff) case m.keyMatch(m.keyBindings.toggleMouse, msg, func(tea.KeyMsg) bool { return keyCtrl(msg, 'e') }) && canFireComposerGatedToggle(m.keyBindings.toggleMouse, defaultToggleMouseChord, m.composerValue() == ""): // Release/recapture the mouse so the user can drag-select and copy text // natively (mouse capture otherwise intercepts terminal selection). The @@ -2446,6 +2448,11 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { } // A resumed/idle session may already hold agents; keep their short lifecycle // fade alive. No-op when the loop is already running or nothing animates. + if m.fileView.active && m.fileView.mode == fileViewFull { + var cmd tea.Cmd + m, cmd = m.startFileViewLoadCmd(m.chatColumnWidth()) + return m, tea.Batch(m.ensureSpinnerTick(), cmd) + } return m, m.ensureSpinnerTick() case permissionRequestMsg: // The agent goroutine that raised this request is BLOCKED waiting on the From 00e1b53b4d4767e66afb9bac3192bd104ca0c6ae Mon Sep 17 00:00:00 2001 From: hazyhaar Date: Thu, 27 Aug 2026 08:31:31 +0200 Subject: [PATCH 03/10] fix(tui): thread-safe palette capture and generation invalidation retry for file view Harden asynchronous file view rendering: - Pass immutable tuiTheme snapshots to background highlighter and formatter to eliminate mutable global access off the UI goroutine. - Track loadedGen on fileViewState to prevent displaying stale content from prior theme palettes. - Trigger automatic retry on stale generation in handleFileViewLoaded. - Guard cache insertion against overwriting newer file modifications. --- internal/tui/file_view.go | 58 +++++++++++++++++------------ internal/tui/file_view_test.go | 64 ++++++++++++++++++++++++++++++-- internal/tui/model.go | 2 +- internal/tui/syntax_highlight.go | 12 +++++- 4 files changed, 106 insertions(+), 30 deletions(-) diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go index 489d28911..d912125e1 100644 --- a/internal/tui/file_view.go +++ b/internal/tui/file_view.go @@ -303,7 +303,7 @@ func changedLinesFingerprint(changed map[string]bool) string { return strings.Join(keys, "\x00") } -func formatFileViewLines(lines []string, display []string, changed map[string]bool, truncated bool, omittedLines bool, width int) string { +func formatFileViewLines(lines []string, display []string, changed map[string]bool, truncated bool, omittedLines bool, width int, theme tuiTheme) string { gutterW := len(fmt.Sprintf("%d", len(lines))) textBudget := maxInt(8, width-gutterW-3) // gutter + space + marker column @@ -315,9 +315,9 @@ func formatFileViewLines(lines []string, display []string, changed map[string]bo } marker := " " if changed != nil && len(lines) > i && changed[strings.TrimSpace(lines[i])] { - marker = zeroTheme.accent.Render("▎") + marker = theme.accent.Render("▎") } - b.WriteString(zeroTheme.faintest.Render(fmt.Sprintf("%*d ", gutterW, i+1))) + b.WriteString(theme.faintest.Render(fmt.Sprintf("%*d ", gutterW, i+1))) b.WriteString(marker) b.WriteString(line) } @@ -326,9 +326,9 @@ func formatFileViewLines(lines []string, display []string, changed map[string]bo // rest of the file, defeating the bounded read above. b.WriteString("\n") if omittedLines { - b.WriteString(zeroTheme.faint.Render(fmt.Sprintf("… more lines (file truncated at %d for display)", len(lines)))) + b.WriteString(theme.faint.Render(fmt.Sprintf("… more lines (file truncated at %d for display)", len(lines)))) } else { - b.WriteString(zeroTheme.faint.Render("… (line content truncated at display limit)")) + b.WriteString(theme.faint.Render("… (line content truncated at display limit)")) } } return b.String() @@ -337,7 +337,7 @@ func formatFileViewLines(lines []string, display []string, changed map[string]bo // getRenderOnly looks up an already-rendered variant in memory (or formats from // already-cached in-memory syntax tokens) without performing any disk I/O, stat, // or Chroma syntax highlighting. Safe for direct View calls. -func (c *fileViewRenderCache) getRenderOnly(targetPath string, width int, changed map[string]bool) (string, bool) { +func (c *fileViewRenderCache) getRenderOnly(targetPath string, width int, changed map[string]bool, theme tuiTheme) (string, bool) { c.mu.Lock() elem, ok := c.items[targetPath] if !ok { @@ -354,7 +354,7 @@ func (c *fileViewRenderCache) getRenderOnly(targetPath string, width int, change return val, true } - rendered := formatFileViewLines(entry.lines, entry.display, changed, entry.truncated, entry.omittedLines, width) + rendered := formatFileViewLines(entry.lines, entry.display, changed, entry.truncated, entry.omittedLines, width, theme) entry.putRender(renderKey, rendered) return rendered, true } @@ -362,10 +362,10 @@ func (c *fileViewRenderCache) getRenderOnly(targetPath string, width int, change // loadAndRender performs the bounded read, Chroma highlighting, and formatting // on a cache miss (or re-formats for a new width variant on a cache hit). It is // intended to be executed from a tea.Cmd / background worker, off the View path. -func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath string, width int, changed map[string]bool, reqGen int) (string, error) { +func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath string, width int, changed map[string]bool, reqGen int, theme tuiTheme) (string, error) { stat, err := os.Stat(targetPath) if err != nil { - rendered := zeroTheme.faint.Render("Could not read file: " + err.Error()) + rendered := theme.faint.Render("Could not read file: " + err.Error()) return rendered, err } @@ -392,7 +392,7 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin } // Re-format for the new width or changed markers using cached display and lines - rendered := formatFileViewLines(entry.lines, entry.display, changed, entry.truncated, entry.omittedLines, width) + rendered := formatFileViewLines(entry.lines, entry.display, changed, entry.truncated, entry.omittedLines, width, theme) entry.putRender(renderKey, rendered) return rendered, nil } @@ -404,7 +404,7 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin readRes := readFileViewBounded(targetPath, fileViewMaxLines, fileViewMaxLineBytes, fileViewMaxBytes) if readRes.err != nil && len(readRes.lines) == 0 { - rendered := zeroTheme.faint.Render("Could not read file: " + readRes.err.Error()) + rendered := theme.faint.Render("Could not read file: " + readRes.err.Error()) return rendered, readRes.err } @@ -412,12 +412,12 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin c.statsData.HighlightCalls++ c.mu.Unlock() - display, ok := highlightCodeForPath(readRes.lines, displayPath, 1<<20, nil) + display, ok := highlightCodeForPathWithTheme(readRes.lines, displayPath, 1<<20, nil, theme) if !ok || len(display) != len(readRes.lines) { display = readRes.lines } - rendered := formatFileViewLines(readRes.lines, display, changed, readRes.truncated, readRes.omittedLines, width) + rendered := formatFileViewLines(readRes.lines, display, changed, readRes.truncated, readRes.omittedLines, width, theme) entry := &fileViewCachedEntry{ targetPath: targetPath, @@ -439,6 +439,11 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin } if elem, ok := c.items[targetPath]; ok { + existing := elem.Value.(*fileViewCachedEntry) + if existing.modTime.After(modTime) { + c.mu.Unlock() + return rendered, nil + } c.lru.Remove(elem) delete(c.items, targetPath) } @@ -460,7 +465,7 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin } func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, width int, changed map[string]bool) string { - rendered, _ := c.loadAndRender(targetPath, displayPath, width, changed, c.generation()) + rendered, _ := c.loadAndRender(targetPath, displayPath, width, changed, c.generation(), zeroTheme) return rendered } @@ -475,9 +480,9 @@ type fileViewLoadedMsg struct { err error } -func loadFileViewCmd(targetPath string, displayPath string, width int, changed map[string]bool, requestID int, gen int) tea.Cmd { +func loadFileViewCmd(targetPath string, displayPath string, width int, changed map[string]bool, requestID int, gen int, theme tuiTheme) tea.Cmd { return func() tea.Msg { - rendered, err := defaultFileViewCache.loadAndRender(targetPath, displayPath, width, changed, gen) + rendered, err := defaultFileViewCache.loadAndRender(targetPath, displayPath, width, changed, gen, theme) return fileViewLoadedMsg{ requestID: requestID, generation: gen, @@ -503,6 +508,7 @@ type fileViewState struct { renderedContent string // rendered full text when loaded loadedPath string // path of loaded content loadedWidth int // width of loaded content + loadedGen int // cache/theme generation of loaded content loading bool // true while async load is in flight } @@ -519,7 +525,8 @@ func (m model) startFileViewLoadCmd(width int) (model, tea.Cmd) { reqID := m.fileView.requestID gen := defaultFileViewCache.generation() changed := m.fileViewChangedLines() - return m, loadFileViewCmd(target, m.fileView.path, width, changed, reqID, gen) + theme := zeroTheme + return m, loadFileViewCmd(target, m.fileView.path, width, changed, reqID, gen, theme) } // openFileView activates the drill-in for path in diff mode. Opening from an @@ -576,21 +583,24 @@ func (m model) setFileViewMode(mode int) (model, tea.Cmd) { return m, nil } -func (m model) handleFileViewLoaded(msg fileViewLoadedMsg) model { +func (m model) handleFileViewLoaded(msg fileViewLoadedMsg) (model, tea.Cmd) { if !m.fileView.active || m.fileView.mode != fileViewFull { - return m + return m, nil } if m.fileView.path != msg.displayPath || m.fileView.requestID != msg.requestID { - return m + return m, nil } if msg.generation != defaultFileViewCache.generation() { - return m + // Cache was invalidated (e.g. theme switch) while this request was in flight. + // Start a fresh request for the current generation. + return m.startFileViewLoadCmd(m.chatColumnWidth()) } m.fileView.loading = false m.fileView.renderedContent = msg.rendered m.fileView.loadedPath = msg.displayPath m.fileView.loadedWidth = msg.width - return m + m.fileView.loadedGen = msg.generation + return m, nil } // fileViewNavBar renders the single-line header shown in place of the pinned @@ -676,10 +686,10 @@ func (m model) renderFileViewFull(width int) string { if !filepath.IsAbs(target) { target = filepath.Join(m.cwd, target) } - if cached, ok := defaultFileViewCache.getRenderOnly(target, width, m.fileViewChangedLines()); ok { + if cached, ok := defaultFileViewCache.getRenderOnly(target, width, m.fileViewChangedLines(), zeroTheme); ok { return cached } - if m.fileView.renderedContent != "" && m.fileView.loadedPath == m.fileView.path { + if m.fileView.renderedContent != "" && m.fileView.loadedPath == m.fileView.path && m.fileView.loadedGen == defaultFileViewCache.generation() { return m.fileView.renderedContent } return zeroTheme.faint.Render(fileViewLoadingPlaceholder) diff --git a/internal/tui/file_view_test.go b/internal/tui/file_view_test.go index d8d8a356a..f6a196e30 100644 --- a/internal/tui/file_view_test.go +++ b/internal/tui/file_view_test.go @@ -973,7 +973,8 @@ func TestFileViewAsyncDiscardOnModeSwitchOrExit(t *testing.T) { } // TestFileViewAsyncDiscardOnThemeInvalidation verifies that if a theme switch occurs -// while an async load is in flight, the old theme's completion message is discarded. +// while an async load is in flight, the old theme's completion message is discarded +// and a fresh load command for the new theme generation is triggered. func TestFileViewAsyncDiscardOnThemeInvalidation(t *testing.T) { defer applyTheme(themeDark, true) resetFileViewCacheForTest() @@ -995,13 +996,70 @@ func TestFileViewAsyncDiscardOnThemeInvalidation(t *testing.T) { // Invalidate cache by switching theme before cmd completes applyTheme(themeLight, false) - // Now old cmd completes + // Now old cmd completes with stale generation msg := cmd() - updated, _ := m.Update(msg) + updated, retryCmd := m.Update(msg) m = updated.(model) // The message must have been rejected due to generation mismatch if m.fileView.renderedContent != "" { t.Fatalf("expected empty renderedContent after invalidation, got %q", m.fileView.renderedContent) } + if retryCmd == nil { + t.Fatal("expected retry command for new generation after invalidation") + } + + // Executing the retry command loads the file under the new generation + retryMsg := retryCmd() + updated, _ = m.Update(retryMsg) + m = updated.(model) + + if !strings.Contains(m.renderFileViewFull(80), "package") { + t.Fatalf("expected file content loaded after retry, got: %s", m.renderFileViewFull(80)) + } + if m.fileView.loadedGen != defaultFileViewCache.generation() { + t.Fatalf("loadedGen %d != cache generation %d", m.fileView.loadedGen, defaultFileViewCache.generation()) + } +} + +// TestFileViewThemeSwitchWhileLoaded verifies that switching themes invalidates +// the loaded generation and allows immediate reloading for the new palette. +func TestFileViewThemeSwitchWhileLoaded(t *testing.T) { + defer applyTheme(themeDark, true) + resetFileViewCacheForTest() + + dir := t.TempDir() + fileA := filepath.Join(dir, "switch_test.go") + if err := os.WriteFile(fileA, []byte("package main\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + m = testOpenFile(m, "switch_test.go") + if !strings.Contains(m.renderFileViewFull(80), "package") { + t.Fatal("file should be loaded initially") + } + + // Switch theme: generation advances, cache is cleared + applyTheme(themeLight, false) + + // renderFileViewFull must not return the stale dark-theme content + staleCheck := m.renderFileViewFull(80) + if strings.Contains(staleCheck, "package") { + t.Fatalf("stale renderedContent must not be rendered after generation increment: %s", staleCheck) + } + + // Starting a new load command re-populates for the new theme + m, reloadCmd := m.startFileViewLoadCmd(80) + if reloadCmd == nil { + t.Fatal("expected reload command") + } + updated, _ := m.Update(reloadCmd()) + m = updated.(model) + + if !strings.Contains(m.renderFileViewFull(80), "package") { + t.Fatalf("expected reloaded content for new theme, got: %s", m.renderFileViewFull(80)) + } } diff --git a/internal/tui/model.go b/internal/tui/model.go index a503cf53a..7b93b3cbf 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -1363,7 +1363,7 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { } switch msg := msg.(type) { case fileViewLoadedMsg: - return m.handleFileViewLoaded(msg), nil + return m.handleFileViewLoaded(msg) case uv.CellSizeEvent: if msg.Width > 0 && msg.Height > 0 { m.petCellPixelWidth = msg.Width diff --git a/internal/tui/syntax_highlight.go b/internal/tui/syntax_highlight.go index 083a9fae3..20fdeb202 100644 --- a/internal/tui/syntax_highlight.go +++ b/internal/tui/syntax_highlight.go @@ -203,7 +203,11 @@ func highlightCodeAuto(code []string, lang string, measure int) ([]string, bool) } func highlightCodeForPath(code []string, path string, measure int, bg color.Color) ([]string, bool) { - return highlightCodeWithLexer(cachedLexerForPath(path), code, measure, bg) + return highlightCodeForPathWithTheme(code, path, measure, bg, zeroTheme) +} + +func highlightCodeForPathWithTheme(code []string, path string, measure int, bg color.Color, theme tuiTheme) ([]string, bool) { + return highlightCodeWithLexerThemeAndLineBackgrounds(cachedLexerForPath(path), code, measure, theme, nil, nil) } // highlightShellCommand styles a one-line command for a tool-card heading. @@ -394,6 +398,10 @@ func highlightCodeWithLexerAndSpans(lexer chroma.Lexer, code []string, measure i } func highlightCodeWithLexerAndLineBackgrounds(lexer chroma.Lexer, code []string, measure int, backgrounds []color.Color, spans []highlightSpan) ([]string, bool) { + return highlightCodeWithLexerThemeAndLineBackgrounds(lexer, code, measure, zeroTheme, backgrounds, spans) +} + +func highlightCodeWithLexerThemeAndLineBackgrounds(lexer chroma.Lexer, code []string, measure int, theme tuiTheme, backgrounds []color.Color, spans []highlightSpan) ([]string, bool) { if measure < 4 { return nil, false } @@ -476,7 +484,7 @@ func highlightCodeWithLexerAndLineBackgrounds(lexer chroma.Lexer, code []string, line, column := 0, 0 for _, token := range iterator.Tokens() { - style := tokenStyle(token.Type) + style := tokenStyleForTheme(theme, token.Type) for index, part := range strings.Split(token.Value, "\n") { if index > 0 { flushLine() From f69f1da6d4ea041ce0d5b0d1fa05dd7274cad881 Mon Sep 17 00:00:00 2001 From: hazyhaar Date: Thu, 27 Aug 2026 08:35:35 +0200 Subject: [PATCH 04/10] fix(tui): restore diff syntax background slice and reload view on git sweep --- internal/tui/model.go | 8 +++++++- internal/tui/syntax_highlight.go | 6 +++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/internal/tui/model.go b/internal/tui/model.go index 7b93b3cbf..2d1d8556b 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -2958,7 +2958,13 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { m.prState = msg.state return m, nil case gitSweepMsg: - return m.handleGitSweepMsg(msg), nil + m = m.handleGitSweepMsg(msg) + if m.fileView.active && m.fileView.mode == fileViewFull { + var cmd tea.Cmd + m, cmd = m.startFileViewLoadCmd(m.chatColumnWidth()) + return m, cmd + } + return m, nil case prWatcherStartedMsg: if msg.stop == nil { return m, nil diff --git a/internal/tui/syntax_highlight.go b/internal/tui/syntax_highlight.go index 20fdeb202..05510be91 100644 --- a/internal/tui/syntax_highlight.go +++ b/internal/tui/syntax_highlight.go @@ -207,7 +207,11 @@ func highlightCodeForPath(code []string, path string, measure int, bg color.Colo } func highlightCodeForPathWithTheme(code []string, path string, measure int, bg color.Color, theme tuiTheme) ([]string, bool) { - return highlightCodeWithLexerThemeAndLineBackgrounds(cachedLexerForPath(path), code, measure, theme, nil, nil) + backgrounds := make([]color.Color, len(code)) + for index := range backgrounds { + backgrounds[index] = bg + } + return highlightCodeWithLexerThemeAndLineBackgrounds(cachedLexerForPath(path), code, measure, theme, backgrounds, nil) } // highlightShellCommand styles a one-line command for a tool-card heading. From aceff6a2d6e1b65a9989e43b3787cad24f877ea9 Mon Sep 17 00:00:00 2001 From: hazyhaar Date: Thu, 27 Aug 2026 08:37:39 +0200 Subject: [PATCH 05/10] docs(agents): record resilient test lifecycle, unfiltered -race verification, and pre-commit audit requirements --- AGENTS.md | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5965f8b5e..badc62126 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,13 +47,18 @@ request, or completing an implementation task: 1. **Formatting check**: `make fmt-check`. If it fails, format with `go fmt ./...` (or `make fmt`) and run the check again. 2. **Vet**: `go vet ./...` (or `make vet`). -3. **Tests**: `go test ./...`. Use `make test` for the full race-enabled suite, - or run focused tests with `-race`, when concurrency is affected. -4. **Build**: `go run ./cmd/zero-release build`. -5. **Smoke test**: `go run ./cmd/zero-release smoke`. -6. **Advisory lint**: `make lint-static`. -7. **Security**: `make vulncheck`. -8. **Diff hygiene**: `git diff HEAD --check` (covers staged and unstaged +3. **Tests**: `make test` (`go test ./... -race -count=1`). Run the full + unfiltered package/module suite under `-race`. Never rely on narrow `-run` + filters for final validation when shared components or concurrency are touched. +4. **Resilience & Full Lifecycle Invariant**: Tests exercising invalidations, + cache clears, concurrent mutations, or rejected messages must prove full + recovery and valid terminal state (re-issuing loads and rendering updated + content), never asserting passive broken intermediate states (e.g. `renderedContent == ""`). +5. **Build**: `go run ./cmd/zero-release build`. +6. **Smoke test**: `go run ./cmd/zero-release smoke`. +7. **Advisory lint**: `make lint-static`. +8. **Security**: `make vulncheck`. +9. **Diff hygiene**: `git diff HEAD --check` (covers staged and unstaged tracked changes). `make lint` currently runs the formatting check and `go vet`; it does **not** @@ -72,6 +77,11 @@ it. These classes drive multi-round reviews. Fix them before requesting review: +- **End-to-End Recovery & Invalidation Cycles:** Any async state, cache invalidation, + or message rejection must reschedule or re-render automatically. A test that asserts + an empty intermediate state without verifying subsequent recovery will block review. +- **Unfiltered `-race` Verification:** Running only focused tests (`-run`) on shared + components obscures cross-module regressions. Full package suites under `-race` are required. - **Fresh base:** Rebase onto the current PR base (`main` or stacked target) before review. A stale head that rolls back mainline commits is a hard blocker, not a merge-time detail. Resolve conflicts by keeping upstream From 9ce0d95afc3ee1caaff7afd8acdef447b687bdb7 Mon Sep 17 00:00:00 2001 From: hazyhaar Date: Thu, 27 Aug 2026 18:20:23 +0200 Subject: [PATCH 06/10] fix(tui): unified snapshot lifecycle, zero-alloc UUIDv7 token, and non-blocking View --- AGENTS.md | 24 +-- internal/tui/file_view.go | 172 ++++++++++++++------- internal/tui/file_view_test.go | 265 +++++++++++++++++++++++++++++++++ internal/tui/model.go | 48 +++++- 4 files changed, 434 insertions(+), 75 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index badc62126..5965f8b5e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,18 +47,13 @@ request, or completing an implementation task: 1. **Formatting check**: `make fmt-check`. If it fails, format with `go fmt ./...` (or `make fmt`) and run the check again. 2. **Vet**: `go vet ./...` (or `make vet`). -3. **Tests**: `make test` (`go test ./... -race -count=1`). Run the full - unfiltered package/module suite under `-race`. Never rely on narrow `-run` - filters for final validation when shared components or concurrency are touched. -4. **Resilience & Full Lifecycle Invariant**: Tests exercising invalidations, - cache clears, concurrent mutations, or rejected messages must prove full - recovery and valid terminal state (re-issuing loads and rendering updated - content), never asserting passive broken intermediate states (e.g. `renderedContent == ""`). -5. **Build**: `go run ./cmd/zero-release build`. -6. **Smoke test**: `go run ./cmd/zero-release smoke`. -7. **Advisory lint**: `make lint-static`. -8. **Security**: `make vulncheck`. -9. **Diff hygiene**: `git diff HEAD --check` (covers staged and unstaged +3. **Tests**: `go test ./...`. Use `make test` for the full race-enabled suite, + or run focused tests with `-race`, when concurrency is affected. +4. **Build**: `go run ./cmd/zero-release build`. +5. **Smoke test**: `go run ./cmd/zero-release smoke`. +6. **Advisory lint**: `make lint-static`. +7. **Security**: `make vulncheck`. +8. **Diff hygiene**: `git diff HEAD --check` (covers staged and unstaged tracked changes). `make lint` currently runs the formatting check and `go vet`; it does **not** @@ -77,11 +72,6 @@ it. These classes drive multi-round reviews. Fix them before requesting review: -- **End-to-End Recovery & Invalidation Cycles:** Any async state, cache invalidation, - or message rejection must reschedule or re-render automatically. A test that asserts - an empty intermediate state without verifying subsequent recovery will block review. -- **Unfiltered `-race` Verification:** Running only focused tests (`-run`) on shared - components obscures cross-module regressions. Full package suites under `-race` are required. - **Fresh base:** Rebase onto the current PR base (`main` or stacked target) before review. A stale head that rolls back mainline commits is a hard blocker, not a merge-time detail. Resolve conflicts by keeping upstream diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go index d912125e1..df3351916 100644 --- a/internal/tui/file_view.go +++ b/internal/tui/file_view.go @@ -24,6 +24,7 @@ import ( "sort" "strings" "sync" + "sync/atomic" "time" tea "charm.land/bubbletea/v2" @@ -45,6 +46,42 @@ const ( fileViewFull ) +var ( + fileViewLifetimeTS atomic.Uint64 + fileViewLifetimeSeq atomic.Uint32 +) + +// nextFileViewLifetimeToken produces a monotonic, time-ordered 128-bit UUIDv7 (RFC 9562) +// to uniquely identify the lifecycle of a file view session without heap allocations. +func nextFileViewLifetimeToken() [16]byte { + nowMs := uint64(time.Now().UnixMilli()) + for { + last := fileViewLifetimeTS.Load() + if nowMs > last { + if fileViewLifetimeTS.CompareAndSwap(last, nowMs) { + fileViewLifetimeSeq.Store(0) + break + } + } else { + nowMs = last + break + } + } + seq := fileViewLifetimeSeq.Add(1) + var u [16]byte + u[0] = byte(nowMs >> 40) + u[1] = byte(nowMs >> 32) + u[2] = byte(nowMs >> 24) + u[3] = byte(nowMs >> 16) + u[4] = byte(nowMs >> 8) + u[5] = byte(nowMs) + u[6] = 0x70 | byte((seq>>8)&0x0F) // Version 7 + u[7] = byte(seq & 0xFF) + u[8] = 0x80 | byte((seq>>16)&0x3F) // RFC 9562 variant + u[9] = byte(seq >> 24) + return u +} + // fileViewCacheStats tracks disk I/O, Chroma highlighting, and cache hits/misses. type fileViewCacheStats struct { DiskReads int @@ -92,6 +129,9 @@ func (e *fileViewCachedEntry) getRender(key string) (string, bool) { func (e *fileViewCachedEntry) putRender(key string, val string) { e.rendersMu.Lock() defer e.rendersMu.Unlock() + if e.renders == nil { + e.renders = make(map[string]string) + } if _, ok := e.renders[key]; !ok { for len(e.renders) >= fileViewMaxRenderVariants { if len(e.renderKeys) > 0 { @@ -334,10 +374,10 @@ func formatFileViewLines(lines []string, display []string, changed map[string]bo return b.String() } -// getRenderOnly looks up an already-rendered variant in memory (or formats from -// already-cached in-memory syntax tokens) without performing any disk I/O, stat, -// or Chroma syntax highlighting. Safe for direct View calls. -func (c *fileViewRenderCache) getRenderOnly(targetPath string, width int, changed map[string]bool, theme tuiTheme) (string, bool) { +// peekRenderOnly looks up an already-formatted variant in memory. It performs +// strictly 0 I/O and 0 string formatting/allocations, guaranteeing O(1) instantaneous +// access on the View() drawing path. +func (c *fileViewRenderCache) peekRenderOnly(targetPath string, width int, changedFingerprint string) (string, bool) { c.mu.Lock() elem, ok := c.items[targetPath] if !ok { @@ -349,29 +389,28 @@ func (c *fileViewRenderCache) getRenderOnly(targetPath string, width int, change c.statsData.CacheHits++ c.mu.Unlock() - renderKey := fmt.Sprintf("%d:%s", width, changedLinesFingerprint(changed)) - if val, ok := entry.getRender(renderKey); ok { - return val, true - } - - rendered := formatFileViewLines(entry.lines, entry.display, changed, entry.truncated, entry.omittedLines, width, theme) - entry.putRender(renderKey, rendered) - return rendered, true + renderKey := fmt.Sprintf("%d:%s", width, changedFingerprint) + return entry.getRender(renderKey) } // loadAndRender performs the bounded read, Chroma highlighting, and formatting // on a cache miss (or re-formats for a new width variant on a cache hit). It is // intended to be executed from a tea.Cmd / background worker, off the View path. -func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath string, width int, changed map[string]bool, reqGen int, theme tuiTheme) (string, error) { +func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath string, width int, changed map[string]bool, changedFingerprint string, reqGen int, theme tuiTheme) (string, error) { stat, err := os.Stat(targetPath) if err != nil { + c.mu.Lock() + if elem, ok := c.items[targetPath]; ok { + c.lru.Remove(elem) + delete(c.items, targetPath) + } + c.mu.Unlock() rendered := theme.faint.Render("Could not read file: " + err.Error()) return rendered, err } modTime := stat.ModTime() size := stat.Size() - changedFingerprint := changedLinesFingerprint(changed) renderKey := fmt.Sprintf("%d:%s", width, changedFingerprint) c.mu.Lock() @@ -404,6 +443,12 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin readRes := readFileViewBounded(targetPath, fileViewMaxLines, fileViewMaxLineBytes, fileViewMaxBytes) if readRes.err != nil && len(readRes.lines) == 0 { + c.mu.Lock() + if elem, ok := c.items[targetPath]; ok { + c.lru.Remove(elem) + delete(c.items, targetPath) + } + c.mu.Unlock() rendered := theme.faint.Render("Could not read file: " + readRes.err.Error()) return rendered, readRes.err } @@ -465,32 +510,35 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin } func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, width int, changed map[string]bool) string { - rendered, _ := c.loadAndRender(targetPath, displayPath, width, changed, c.generation(), zeroTheme) + fingerprint := changedLinesFingerprint(changed) + rendered, _ := c.loadAndRender(targetPath, displayPath, width, changed, fingerprint, c.generation(), zeroTheme) return rendered } // fileViewLoadedMsg delivers the result of an asynchronous file read & render. type fileViewLoadedMsg struct { - requestID int - generation int - targetPath string - displayPath string - width int - rendered string - err error + lifetimeToken [16]byte + generation int + targetPath string + displayPath string + width int + fingerprint string + rendered string + err error } -func loadFileViewCmd(targetPath string, displayPath string, width int, changed map[string]bool, requestID int, gen int, theme tuiTheme) tea.Cmd { +func loadFileViewCmd(targetPath string, displayPath string, width int, changed map[string]bool, fingerprint string, token [16]byte, gen int, theme tuiTheme) tea.Cmd { return func() tea.Msg { - rendered, err := defaultFileViewCache.loadAndRender(targetPath, displayPath, width, changed, gen, theme) + rendered, err := defaultFileViewCache.loadAndRender(targetPath, displayPath, width, changed, fingerprint, gen, theme) return fileViewLoadedMsg{ - requestID: requestID, - generation: gen, - targetPath: targetPath, - displayPath: displayPath, - width: width, - rendered: rendered, - err: err, + lifetimeToken: token, + generation: gen, + targetPath: targetPath, + displayPath: displayPath, + width: width, + fingerprint: fingerprint, + rendered: rendered, + err: err, } } } @@ -498,18 +546,19 @@ func loadFileViewCmd(targetPath string, displayPath string, width int, changed m // fileViewState manages the drill-in view for a touched file. When active, the // transcript body swaps to the file's diff/content instead of the chat rows. type fileViewState struct { - active bool - path string // workspace-relative, as carried by changedFiles - mode int // fileViewDiff | fileViewFull - // parentScrollOffset preserves the chat scroll position so closing the view - // returns to the same spot (mirrors subchatState). + active bool + path string // workspace-relative, as carried by changedFiles + mode int // fileViewDiff | fileViewFull parentScrollOffset int - requestID int // monotonic ID for async load requests - renderedContent string // rendered full text when loaded - loadedPath string // path of loaded content - loadedWidth int // width of loaded content - loadedGen int // cache/theme generation of loaded content - loading bool // true while async load is in flight + lifetimeToken [16]byte // monotonic, time-ordered UUIDv7 token for this active session + renderedContent string // rendered full text when loaded + loadedPath string // path of loaded content + loadedWidth int // width of loaded content + loadedGen int // cache/theme generation of loaded content + loadedFingerprint string // changed lines fingerprint of loaded content + loadedToken [16]byte // token matching the loaded content + loading bool // true while async load is in flight + hasError bool // true if the load failed } func (m model) startFileViewLoadCmd(width int) (model, tea.Cmd) { @@ -520,13 +569,13 @@ func (m model) startFileViewLoadCmd(width int) (model, tea.Cmd) { if !filepath.IsAbs(target) { target = filepath.Join(m.cwd, target) } - m.fileView.requestID++ m.fileView.loading = true - reqID := m.fileView.requestID + token := m.fileView.lifetimeToken gen := defaultFileViewCache.generation() changed := m.fileViewChangedLines() + fingerprint := changedLinesFingerprint(changed) theme := zeroTheme - return m, loadFileViewCmd(target, m.fileView.path, width, changed, reqID, gen, theme) + return m, loadFileViewCmd(target, m.fileView.path, width, changed, fingerprint, token, gen, theme) } // openFileView activates the drill-in for path in diff mode. Opening from an @@ -545,6 +594,10 @@ func (m model) openFileView(path string) (model, tea.Cmd) { m.fileView.active = true m.fileView.path = path m.fileView.mode = fileViewDiff + m.fileView.lifetimeToken = nextFileViewLifetimeToken() + m.fileView.renderedContent = "" + m.fileView.loadedToken = [16]byte{} + m.fileView.hasError = false // A file only the git sweep knows about (bash/subagent mutation) has no edit // cards to stack — open straight on the full file instead of a placeholder. if len(m.fileViewResultRows()) == 0 { @@ -587,7 +640,7 @@ func (m model) handleFileViewLoaded(msg fileViewLoadedMsg) (model, tea.Cmd) { if !m.fileView.active || m.fileView.mode != fileViewFull { return m, nil } - if m.fileView.path != msg.displayPath || m.fileView.requestID != msg.requestID { + if m.fileView.lifetimeToken != msg.lifetimeToken || m.fileView.path != msg.displayPath { return m, nil } if msg.generation != defaultFileViewCache.generation() { @@ -600,6 +653,9 @@ func (m model) handleFileViewLoaded(msg fileViewLoadedMsg) (model, tea.Cmd) { m.fileView.loadedPath = msg.displayPath m.fileView.loadedWidth = msg.width m.fileView.loadedGen = msg.generation + m.fileView.loadedFingerprint = msg.fingerprint + m.fileView.loadedToken = msg.lifetimeToken + m.fileView.hasError = (msg.err != nil) return m, nil } @@ -679,19 +735,31 @@ func (m model) renderFileViewDiff(width int) string { // highlighted, with a line-number gutter and an accent ▎ marker on the lines // this session's diffs added (matched by exact text — an approximation that // tolerates later drift; a stale marker just doesn't highlight). -// It is read-only and non-blocking: if content is ready or cached in memory, -// it is returned immediately; otherwise a loading placeholder is shown. +// It is strictly non-blocking and performs O(1) lookup without invoking formatters. func (m model) renderFileViewFull(width int) string { + if m.fileView.hasError { + return m.fileView.renderedContent + } + if m.fileView.renderedContent != "" && + m.fileView.loadedPath == m.fileView.path && + m.fileView.loadedGen == defaultFileViewCache.generation() && + m.fileView.loadedToken == m.fileView.lifetimeToken { + target := m.fileView.path + if !filepath.IsAbs(target) { + target = filepath.Join(m.cwd, target) + } + if cached, ok := defaultFileViewCache.peekRenderOnly(target, width, m.fileView.loadedFingerprint); ok { + return cached + } + return m.fileView.renderedContent + } target := m.fileView.path if !filepath.IsAbs(target) { target = filepath.Join(m.cwd, target) } - if cached, ok := defaultFileViewCache.getRenderOnly(target, width, m.fileViewChangedLines(), zeroTheme); ok { + if cached, ok := defaultFileViewCache.peekRenderOnly(target, width, m.fileView.loadedFingerprint); ok { return cached } - if m.fileView.renderedContent != "" && m.fileView.loadedPath == m.fileView.path && m.fileView.loadedGen == defaultFileViewCache.generation() { - return m.fileView.renderedContent - } return zeroTheme.faint.Render(fileViewLoadingPlaceholder) } diff --git a/internal/tui/file_view_test.go b/internal/tui/file_view_test.go index f6a196e30..ed1f88bc2 100644 --- a/internal/tui/file_view_test.go +++ b/internal/tui/file_view_test.go @@ -1063,3 +1063,268 @@ func TestFileViewThemeSwitchWhileLoaded(t *testing.T) { t.Fatalf("expected reloaded content for new theme, got: %s", m.renderFileViewFull(80)) } } + +// TestFileViewLifecycle_OpenToLoad tests the full model update flow from open to async load completion. +func TestFileViewLifecycle_OpenToLoad(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + filePath := filepath.Join(dir, "app.go") + if err := os.WriteFile(filePath, []byte("package app\nfunc Run() {}\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + // Open file via model action + m, cmd := m.openFileView("app.go") + if !m.fileView.active || m.fileView.mode != fileViewFull { + t.Fatal("file view should be active in full mode for new file") + } + if cmd == nil { + t.Fatal("expected async load command on open") + } + + // View displays loading placeholder before completion + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), fileViewLoadingPlaceholder) { + t.Fatalf("expected loading placeholder, got: %s", plainRender(t, m.renderFileViewFull(80))) + } + + // Process load completion + updated, _ := m.Update(cmd()) + m = updated.(model) + + rendered := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(rendered, "package app") || !strings.Contains(rendered, "func Run()") { + t.Fatalf("expected loaded content, got: %s", rendered) + } + if m.fileView.hasError { + t.Fatal("expected no error") + } +} + +// TestFileViewLifecycle_RapidResizeCoalesced verifies that repeated resize events +// do not cause race conditions or synchronous render spikes, and the latest resize wins. +func TestFileViewLifecycle_RapidResizeCoalesced(t *testing.T) { + resetFileViewCacheForTest() + dir := t.TempDir() + filePath := filepath.Join(dir, "resize.go") + if err := os.WriteFile(filePath, []byte("package resize\nconst BigWidth = true\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "resize.go") + + var cmds []tea.Cmd + for w := 40; w <= 120; w += 10 { + var cmd tea.Cmd + m, cmd = m.startFileViewLoadCmd(w) + if cmd != nil { + cmds = append(cmds, cmd) + } + } + + // Deliver the latest resize command completion + lastCmd := cmds[len(cmds)-1] + updated, _ := m.Update(lastCmd()) + m = updated.(model) + + if m.fileView.loadedWidth != 120 { + t.Fatalf("expected loadedWidth 120, got %d", m.fileView.loadedWidth) + } + if !strings.Contains(plainRender(t, m.renderFileViewFull(120)), "package resize") { + t.Fatalf("expected content for width 120, got: %s", plainRender(t, m.renderFileViewFull(120))) + } +} + +// TestFileViewLifecycle_ThemeSwitchReloadsActiveView tests that selecting a theme +// in production immediately triggers a reload command for the active file view. +func TestFileViewLifecycle_ThemeSwitchReloadsActiveView(t *testing.T) { + defer applyTheme(themeDark, true) + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "theme_active.go") + if err := os.WriteFile(filePath, []byte("package theme\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "theme_active.go") + + // Trigger /theme light via command handling + cmdAction := parsedCommand{kind: commandTheme, text: "light"} + updated, reloadCmd := m.dispatchCommand(cmdAction) + m = updated.(model) + + if reloadCmd == nil { + t.Fatal("expected reload command on active file view after theme change") + } + + // Complete the reload + updated, _ = m.Update(reloadCmd()) + m = updated.(model) + + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package theme") { + t.Fatalf("expected reloaded theme content, got: %s", plainRender(t, m.renderFileViewFull(80))) + } + if m.fileView.loadedGen != defaultFileViewCache.generation() { + t.Fatalf("expected loadedGen %d, got %d", defaultFileViewCache.generation(), m.fileView.loadedGen) + } +} + +// TestFileViewLifecycle_DirectToolMutationTriggersRefresh tests that tool result +// rows from write_file or edit_file directly refresh an active file view snapshot. +func TestFileViewLifecycle_DirectToolMutationTriggersRefresh(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "live_edit.go") + if err := os.WriteFile(filePath, []byte("version 1\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "live_edit.go") + + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "version 1") { + t.Fatal("initial load should have version 1") + } + + // Directly modify file on disk + if err := os.WriteFile(filePath, []byte("version 2 modified\n"), 0o644); err != nil { + t.Fatal(err) + } + + // Dispatch tool result for write_file affecting live_edit.go + toolRow := transcriptRow{ + kind: rowToolResult, + tool: "write_file", + changedFiles: []string{"live_edit.go"}, + detail: "+version 2 modified", + } + updated, reloadCmd := m.Update(agentRowMsg{runID: m.activeRunID, row: toolRow}) + m = updated.(model) + + if reloadCmd == nil { + t.Fatal("expected reload command on direct tool mutation for active file") + } + + // Execute reload + updated, _ = m.Update(reloadCmd()) + m = updated.(model) + + rendered := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(rendered, "version 2 modified") { + t.Fatalf("expected version 2 after tool mutation reload, got: %s", rendered) + } +} + +// TestFileViewLifecycle_DeletionOverrulesStaleCache tests that when a file is deleted, +// the reload failure evicts the former cache entry and immediately displays the error. +func TestFileViewLifecycle_DeletionOverrulesStaleCache(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "deleted.go") + if err := os.WriteFile(filePath, []byte("package deleted\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "deleted.go") + + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "package deleted") { + t.Fatal("initial load failed") + } + + // Delete the file + if err := os.Remove(filePath); err != nil { + t.Fatal(err) + } + + // Force reload + m, reloadCmd := m.startFileViewLoadCmd(80) + if reloadCmd == nil { + t.Fatal("expected reload command") + } + + updated, _ := m.Update(reloadCmd()) + m = updated.(model) + + if !m.fileView.hasError { + t.Fatal("expected hasError to be true after deletion") + } + + rendered := plainRender(t, m.renderFileViewFull(80)) + if strings.Contains(rendered, "package deleted") { + t.Fatalf("stale cache must not be shown after deletion, got: %s", rendered) + } + if !strings.Contains(rendered, "Could not read file") { + t.Fatalf("expected error message in view, got: %s", rendered) + } +} + +// TestFileViewLifecycle_LateCompletionAcrossReopenDiscarded tests that if a file view +// is exited and the same path is reopened, any late-arriving completion from the first +// session is discarded and cannot populate the new session. +func TestFileViewLifecycle_LateCompletionAcrossReopenDiscarded(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "reopen.go") + if err := os.WriteFile(filePath, []byte("original content\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + + // Session 1: open and get command + m, cmd1 := m.openFileView("reopen.go") + if cmd1 == nil { + t.Fatal("expected cmd1") + } + + // Exit session 1 + m = m.exitFileView() + if m.fileView.active { + t.Fatal("view should be inactive") + } + + // Modify file on disk before session 2 + if err := os.WriteFile(filePath, []byte("new session content\n"), 0o644); err != nil { + t.Fatal(err) + } + + // Session 2: reopen same path + m, cmd2 := m.openFileView("reopen.go") + if cmd2 == nil { + t.Fatal("expected cmd2") + } + + // Late completion from session 1 arrives + msg1 := cmd1() + updated, _ := m.Update(msg1) + m = updated.(model) + + // Session 1 message must have been discarded: view is still waiting on session 2 + if m.fileView.renderedContent != "" { + t.Fatalf("late completion from session 1 must be discarded, got: %s", m.fileView.renderedContent) + } + + // Session 2 completion arrives + msg2 := cmd2() + updated, _ = m.Update(msg2) + m = updated.(model) + + rendered := plainRender(t, m.renderFileViewFull(80)) + if !strings.Contains(rendered, "new session content") { + t.Fatalf("expected new session content, got: %s", rendered) + } +} diff --git a/internal/tui/model.go b/internal/tui/model.go index 2d1d8556b..564e07426 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -1426,6 +1426,9 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { m.hasDarkBg = msg.IsDark() if m.themeMode != themeSystem { applyTheme(m.themeMode, m.hasDarkBg) + if m.fileView.active && m.fileView.mode == fileViewFull { + return m.startFileViewLoadCmd(m.chatColumnWidth()) + } } return m, nil case tea.MouseMsg: @@ -2922,10 +2925,21 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { // A finished command tool may have mutated files git can see but no // changedFiles reports (npm create, heredoc writes, subagent edits) — // re-sweep so the FILES sidebar picks them up mid-turn. - if msg.row.kind == rowToolResult && isPlanCommandTool(msg.row.tool) { - var sweep tea.Cmd - m, sweep = m.maybeGitSweep() - return m, sweep + if msg.row.kind == rowToolResult { + if isPlanCommandTool(msg.row.tool) { + var sweep tea.Cmd + m, sweep = m.maybeGitSweep() + return m, sweep + } + if m.fileView.active && m.fileView.mode == fileViewFull { + for _, p := range msg.row.changedFiles { + if p == m.fileView.path { + var loadCmd tea.Cmd + m, loadCmd = m.startFileViewLoadCmd(m.chatColumnWidth()) + return m, loadCmd + } + } + } } return m, nil case swarmSessionsMsg: @@ -4534,10 +4548,21 @@ func (m model) choosePicker() (tea.Model, tea.Cmd) { // local preview and never changes the active palette. text := "" m, text = m.handleThemeCommand(item.Value) + var loadCmd tea.Cmd + if m.fileView.active && m.fileView.mode == fileViewFull { + m, loadCmd = m.startFileViewLoadCmd(m.chatColumnWidth()) + } if validThemeMode(item.Value) && !strings.Contains(text, "could not save theme preference") { - return m.showTransientNotice(m.themeAppliedNotice(), transientNoticeSuccess) + next, noticeCmd := m.showTransientNotice(m.themeAppliedNotice(), transientNoticeSuccess) + if loadCmd != nil { + return next, tea.Batch(noticeCmd, loadCmd) + } + return next, noticeCmd } m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) + if loadCmd != nil { + return m, loadCmd + } } return m, cmd } @@ -4941,10 +4966,21 @@ func (m model) dispatchCommand(command parsedCommand) (tea.Model, tea.Cmd) { } text := "" m, text = m.handleThemeCommand(command.text) + var loadCmd tea.Cmd + if m.fileView.active && m.fileView.mode == fileViewFull { + m, loadCmd = m.startFileViewLoadCmd(m.chatColumnWidth()) + } if validThemeMode(command.text) && !strings.Contains(text, "could not save theme preference") { - return m.showTransientNotice(m.themeAppliedNotice(), transientNoticeSuccess) + next, noticeCmd := m.showTransientNotice(m.themeAppliedNotice(), transientNoticeSuccess) + if loadCmd != nil { + return next, tea.Batch(noticeCmd, loadCmd) + } + return next, noticeCmd } m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) + if loadCmd != nil { + return m, loadCmd + } return m, nil case commandImage: m = m.handleImageCommand(command.text) From 9d6d8582c9c46d7a01779f5978a5185d9beb220a Mon Sep 17 00:00:00 2001 From: hazyhaar Date: Thu, 27 Aug 2026 18:20:47 +0200 Subject: [PATCH 07/10] fix(config): replace deprecated reflect.Ptr with reflect.Pointer --- internal/config/unknownfields.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/unknownfields.go b/internal/config/unknownfields.go index e5c36901d..7341a098a 100644 --- a/internal/config/unknownfields.go +++ b/internal/config/unknownfields.go @@ -131,7 +131,7 @@ type knownField struct { } func derefType(t reflect.Type) reflect.Type { - for t != nil && t.Kind() == reflect.Ptr { + for t != nil && t.Kind() == reflect.Pointer { t = t.Elem() } return t From 83f37a118574c277cbaaad445c220fffb0be1c75 Mon Sep 17 00:00:00 2001 From: hazyhaar Date: Thu, 27 Aug 2026 19:27:54 +0200 Subject: [PATCH 08/10] fix(tui): enforce desiredSeq snapshot authority and reverse-order completion rejection --- internal/tui/file_view.go | 80 +++++++++++++-------- internal/tui/file_view_test.go | 126 +++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 29 deletions(-) diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go index df3351916..8fbef434c 100644 --- a/internal/tui/file_view.go +++ b/internal/tui/file_view.go @@ -518,6 +518,7 @@ func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, // fileViewLoadedMsg delivers the result of an asynchronous file read & render. type fileViewLoadedMsg struct { lifetimeToken [16]byte + seq uint64 generation int targetPath string displayPath string @@ -527,11 +528,12 @@ type fileViewLoadedMsg struct { err error } -func loadFileViewCmd(targetPath string, displayPath string, width int, changed map[string]bool, fingerprint string, token [16]byte, gen int, theme tuiTheme) tea.Cmd { +func loadFileViewCmd(targetPath string, displayPath string, width int, changed map[string]bool, fingerprint string, token [16]byte, seq uint64, gen int, theme tuiTheme) tea.Cmd { return func() tea.Msg { rendered, err := defaultFileViewCache.loadAndRender(targetPath, displayPath, width, changed, fingerprint, gen, theme) return fileViewLoadedMsg{ lifetimeToken: token, + seq: seq, generation: gen, targetPath: targetPath, displayPath: displayPath, @@ -550,15 +552,26 @@ type fileViewState struct { path string // workspace-relative, as carried by changedFiles mode int // fileViewDiff | fileViewFull parentScrollOffset int - lifetimeToken [16]byte // monotonic, time-ordered UUIDv7 token for this active session - renderedContent string // rendered full text when loaded - loadedPath string // path of loaded content - loadedWidth int // width of loaded content - loadedGen int // cache/theme generation of loaded content - loadedFingerprint string // changed lines fingerprint of loaded content - loadedToken [16]byte // token matching the loaded content - loading bool // true while async load is in flight - hasError bool // true if the load failed + + // View session lifetime identity (UUIDv7 RFC 9562 0-alloc) + lifetimeToken [16]byte + + // Monotonically advancing desired snapshot sequence & requested parameters + desiredSeq uint64 + desiredWidth int + desiredFingerprint string + desiredGen int + + // Authoritative completed snapshot (only valid when loadedSeq == desiredSeq) + renderedContent string + loadedPath string + loadedWidth int + loadedGen int + loadedFingerprint string + loadedToken [16]byte + loadedSeq uint64 + loading bool + hasError bool } func (m model) startFileViewLoadCmd(width int) (model, tea.Cmd) { @@ -569,13 +582,18 @@ func (m model) startFileViewLoadCmd(width int) (model, tea.Cmd) { if !filepath.IsAbs(target) { target = filepath.Join(m.cwd, target) } + m.fileView.desiredSeq++ + m.fileView.desiredWidth = width m.fileView.loading = true + seq := m.fileView.desiredSeq token := m.fileView.lifetimeToken gen := defaultFileViewCache.generation() changed := m.fileViewChangedLines() fingerprint := changedLinesFingerprint(changed) + m.fileView.desiredFingerprint = fingerprint + m.fileView.desiredGen = gen theme := zeroTheme - return m, loadFileViewCmd(target, m.fileView.path, width, changed, fingerprint, token, gen, theme) + return m, loadFileViewCmd(target, m.fileView.path, width, changed, fingerprint, token, seq, gen, theme) } // openFileView activates the drill-in for path in diff mode. Opening from an @@ -597,6 +615,7 @@ func (m model) openFileView(path string) (model, tea.Cmd) { m.fileView.lifetimeToken = nextFileViewLifetimeToken() m.fileView.renderedContent = "" m.fileView.loadedToken = [16]byte{} + m.fileView.loadedSeq = 0 m.fileView.hasError = false // A file only the git sweep knows about (bash/subagent mutation) has no edit // cards to stack — open straight on the full file instead of a placeholder. @@ -640,13 +659,21 @@ func (m model) handleFileViewLoaded(msg fileViewLoadedMsg) (model, tea.Cmd) { if !m.fileView.active || m.fileView.mode != fileViewFull { return m, nil } + // 1. Session lifetime identity match if m.fileView.lifetimeToken != msg.lifetimeToken || m.fileView.path != msg.displayPath { return m, nil } - if msg.generation != defaultFileViewCache.generation() { - // Cache was invalidated (e.g. theme switch) while this request was in flight. - // Start a fresh request for the current generation. - return m.startFileViewLoadCmd(m.chatColumnWidth()) + // 2. Exact desired snapshot match: reject superseded / out-of-order completions + if msg.seq != m.fileView.desiredSeq || + msg.width != m.fileView.desiredWidth || + msg.fingerprint != m.fileView.desiredFingerprint || + msg.generation != defaultFileViewCache.generation() { + if msg.generation != defaultFileViewCache.generation() { + // Cache was invalidated (e.g. theme switch) while this request was in flight. + return m.startFileViewLoadCmd(m.chatColumnWidth()) + } + // Out-of-order or obsolete completion: drop without modifying state + return m, nil } m.fileView.loading = false m.fileView.renderedContent = msg.rendered @@ -655,6 +682,7 @@ func (m model) handleFileViewLoaded(msg fileViewLoadedMsg) (model, tea.Cmd) { m.fileView.loadedGen = msg.generation m.fileView.loadedFingerprint = msg.fingerprint m.fileView.loadedToken = msg.lifetimeToken + m.fileView.loadedSeq = msg.seq m.fileView.hasError = (msg.err != nil) return m, nil } @@ -740,26 +768,20 @@ func (m model) renderFileViewFull(width int) string { if m.fileView.hasError { return m.fileView.renderedContent } - if m.fileView.renderedContent != "" && - m.fileView.loadedPath == m.fileView.path && - m.fileView.loadedGen == defaultFileViewCache.generation() && - m.fileView.loadedToken == m.fileView.lifetimeToken { - target := m.fileView.path - if !filepath.IsAbs(target) { - target = filepath.Join(m.cwd, target) - } - if cached, ok := defaultFileViewCache.peekRenderOnly(target, width, m.fileView.loadedFingerprint); ok { - return cached - } - return m.fileView.renderedContent - } target := m.fileView.path if !filepath.IsAbs(target) { target = filepath.Join(m.cwd, target) } - if cached, ok := defaultFileViewCache.peekRenderOnly(target, width, m.fileView.loadedFingerprint); ok { + if cached, ok := defaultFileViewCache.peekRenderOnly(target, width, m.fileView.desiredFingerprint); ok { return cached } + if m.fileView.renderedContent != "" && + m.fileView.loadedPath == m.fileView.path && + m.fileView.loadedSeq == m.fileView.desiredSeq && + m.fileView.loadedGen == defaultFileViewCache.generation() && + m.fileView.loadedToken == m.fileView.lifetimeToken { + return m.fileView.renderedContent + } return zeroTheme.faint.Render(fileViewLoadingPlaceholder) } diff --git a/internal/tui/file_view_test.go b/internal/tui/file_view_test.go index ed1f88bc2..2a057b441 100644 --- a/internal/tui/file_view_test.go +++ b/internal/tui/file_view_test.go @@ -1328,3 +1328,129 @@ func TestFileViewLifecycle_LateCompletionAcrossReopenDiscarded(t *testing.T) { t.Fatalf("expected new session content, got: %s", rendered) } } + +// TestFileViewLifecycle_ReverseOrderResizeCompletions verifies that when two resize events +// schedule requests A (earlier) and B (later), and B completes before A, the subsequent +// late arrival of A is discarded and does not overwrite B's width or rendered snapshot. +func TestFileViewLifecycle_ReverseOrderResizeCompletions(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "resize_order.go") + if err := os.WriteFile(filePath, []byte("package resize_order\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "resize_order.go") + + // Schedule Request A for width 60 + m, cmdA := m.startFileViewLoadCmd(60) + if cmdA == nil { + t.Fatal("expected cmdA") + } + + // Schedule Request B for width 100 + m, cmdB := m.startFileViewLoadCmd(100) + if cmdB == nil { + t.Fatal("expected cmdB") + } + + // Message B completes first + msgB := cmdB() + updated, _ := m.Update(msgB) + m = updated.(model) + + if m.fileView.loadedWidth != 100 { + t.Fatalf("expected loadedWidth 100 after B completes, got %d", m.fileView.loadedWidth) + } + if !strings.Contains(plainRender(t, m.renderFileViewFull(100)), "package resize_order") { + t.Fatalf("expected width 100 content rendered, got: %s", plainRender(t, m.renderFileViewFull(100))) + } + + // Message A arrives late (reverse-order) + msgA := cmdA() + updated, _ = m.Update(msgA) + m = updated.(model) + + // State MUST remain B (width 100), not overwritten by A (width 60) + if m.fileView.loadedWidth != 100 { + t.Fatalf("late completion A must NOT overwrite loadedWidth, got %d (want 100)", m.fileView.loadedWidth) + } + if !strings.Contains(plainRender(t, m.renderFileViewFull(100)), "package resize_order") { + t.Fatalf("expected width 100 content still visible, got: %s", plainRender(t, m.renderFileViewFull(100))) + } +} + +// TestFileViewLifecycle_ReverseOrderToolMutationCompletions verifies that when two tool +// mutations trigger requests A (version 1) and B (version 2), and B completes before A, +// the subsequent arrival of A cannot revert the visible snapshot back to version 1. +func TestFileViewLifecycle_ReverseOrderToolMutationCompletions(t *testing.T) { + resetFileViewCacheForTest() + + dir := t.TempDir() + filePath := filepath.Join(dir, "tool_order.go") + if err := os.WriteFile(filePath, []byte("initial\n"), 0o644); err != nil { + t.Fatal(err) + } + + m := filesPanelTestModel() + m.cwd = dir + m = testOpenFile(m, "tool_order.go") + + // Mutation A modifies file to v1 + if err := os.WriteFile(filePath, []byte("version 1 state\n"), 0o644); err != nil { + t.Fatal(err) + } + rowA := transcriptRow{ + kind: rowToolResult, + tool: "write_file", + changedFiles: []string{"tool_order.go"}, + detail: "+version 1 state", + } + updated, cmdA := m.Update(agentRowMsg{runID: m.activeRunID, row: rowA}) + m = updated.(model) + if cmdA == nil { + t.Fatal("expected cmdA for mutation A") + } + + // Mutation B immediately modifies file to v2 before A completes + if err := os.WriteFile(filePath, []byte("version 2 state\n"), 0o644); err != nil { + t.Fatal(err) + } + rowB := transcriptRow{ + kind: rowToolResult, + tool: "edit_file", + changedFiles: []string{"tool_order.go"}, + detail: "+version 2 state", + } + updated, cmdB := m.Update(agentRowMsg{runID: m.activeRunID, row: rowB}) + m = updated.(model) + if cmdB == nil { + t.Fatal("expected cmdB for mutation B") + } + + // B completes first + msgB := cmdB() + updated, _ = m.Update(msgB) + m = updated.(model) + + if !strings.Contains(plainRender(t, m.renderFileViewFull(80)), "version 2 state") { + t.Fatalf("expected version 2 state after B completes, got: %s", plainRender(t, m.renderFileViewFull(80))) + } + + // A arrives late (reverse-order) + msgA := cmdA() + updated, _ = m.Update(msgA) + m = updated.(model) + + // View MUST remain version 2, never reverted by A + rendered := plainRender(t, m.renderFileViewFull(80)) + if strings.Contains(rendered, "version 1 state") { + t.Fatalf("stale version 1 completion must NOT overwrite version 2, got: %s", rendered) + } + if !strings.Contains(rendered, "version 2 state") { + t.Fatalf("expected version 2 state still visible, got: %s", rendered) + } +} From 27e21f47cbe644e0c6b13c6a3b4d257868d1a9dd Mon Sep 17 00:00:00 2001 From: hazyhaar Date: Thu, 27 Aug 2026 19:33:35 +0200 Subject: [PATCH 09/10] style(tui): apply canonical gofmt alignment --- internal/tui/file_view.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go index 8fbef434c..db453937a 100644 --- a/internal/tui/file_view.go +++ b/internal/tui/file_view.go @@ -554,7 +554,7 @@ type fileViewState struct { parentScrollOffset int // View session lifetime identity (UUIDv7 RFC 9562 0-alloc) - lifetimeToken [16]byte + lifetimeToken [16]byte // Monotonically advancing desired snapshot sequence & requested parameters desiredSeq uint64 @@ -563,15 +563,15 @@ type fileViewState struct { desiredGen int // Authoritative completed snapshot (only valid when loadedSeq == desiredSeq) - renderedContent string - loadedPath string - loadedWidth int - loadedGen int - loadedFingerprint string - loadedToken [16]byte - loadedSeq uint64 - loading bool - hasError bool + renderedContent string + loadedPath string + loadedWidth int + loadedGen int + loadedFingerprint string + loadedToken [16]byte + loadedSeq uint64 + loading bool + hasError bool } func (m model) startFileViewLoadCmd(width int) (model, tea.Cmd) { From 629f0c441abcfe30ca0f3f7c3d21c0c18bcc701c Mon Sep 17 00:00:00 2001 From: hazyhaar Date: Thu, 27 Aug 2026 19:41:07 +0200 Subject: [PATCH 10/10] security(tui): sanitize raw ANSI escapes and track LRU eviction statistics --- internal/tui/file_view.go | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/internal/tui/file_view.go b/internal/tui/file_view.go index db453937a..ce82bd3bb 100644 --- a/internal/tui/file_view.go +++ b/internal/tui/file_view.go @@ -459,7 +459,10 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin display, ok := highlightCodeForPathWithTheme(readRes.lines, displayPath, 1<<20, nil, theme) if !ok || len(display) != len(readRes.lines) { - display = readRes.lines + display = make([]string, len(readRes.lines)) + for i, l := range readRes.lines { + display[i] = sanitizeRawFileLine(l) + } } rendered := formatFileViewLines(readRes.lines, display, changed, readRes.truncated, readRes.omittedLines, width, theme) @@ -503,12 +506,33 @@ func (c *fileViewRenderCache) loadAndRender(targetPath string, displayPath strin backEntry := back.Value.(*fileViewCachedEntry) delete(c.items, backEntry.targetPath) c.lru.Remove(back) + c.statsData.Evictions++ } c.mu.Unlock() return rendered, nil } +// sanitizeRawFileLine strips or transforms raw control characters and terminal escape sequences +// when syntax highlighting is bypassed or unavailable, preventing terminal screen corruption. +func sanitizeRawFileLine(s string) string { + var out strings.Builder + for _, r := range s { + if r == '\t' { + out.WriteString(" ") + } else if r == '\r' || r == '\n' { + continue + } else if r < 32 || r == 0x7f || (r >= 0x80 && r <= 0x9f) { + if r == '\x1b' { + out.WriteString("^[") + } + } else { + out.WriteRune(r) + } + } + return out.String() +} + func (c *fileViewRenderCache) getOrRender(targetPath string, displayPath string, width int, changed map[string]bool) string { fingerprint := changedLinesFingerprint(changed) rendered, _ := c.loadAndRender(targetPath, displayPath, width, changed, fingerprint, c.generation(), zeroTheme)