Skip to content

K8s sandbox recording fix + TUI render-cache with collision prevention - #506

Merged
rschardosin merged 4 commits into
mainfrom
tui-perf-render-cache
Sep 8, 2026
Merged

K8s sandbox recording fix + TUI render-cache with collision prevention#506
rschardosin merged 4 commits into
mainfrom
tui-perf-render-cache

Conversation

@rschardosin

@rschardosin rschardosin commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Overview

This PR contains two complementary features: a K8s sandbox recording fix and a TUI transcript render-cache optimization with collision prevention.

1. K8s Sandbox Recording Fix (K8s xdpyinfo error)

Problem: When running run_drill with browser recording enabled on Kubernetes, drills failed with:

ERR browser_start_recording: probe display size: no dimensions in xdpyinfo output: ""

Root cause: kubectl exec runs commands in the pod's base namespace (thin Debian image), not inside the chroot overlay at /sandbox/rootfs where xdpyinfo, ffmpeg, and other tools are installed.

Solution: Added backendShellCommand() helper that wraps shell commands through /usr/local/bin/astonish-shell on K8s backends (which chroots into the overlay), while using plain sh -c on Docker (where overlay is root). Mirrors the pattern in backend_mcp_transport.go.

2. TUI Render-Cache with Collision Prevention

Performance optimization: Added item render cache that memoizes fully-rendered transcript blocks. Reduces rendering cost from O(all items) to O(new/changed items) on each viewport refresh. Includes smart caching guards for streaming/interactive items and per-item selection application.

Visual-correctness fixes: Identified and fixed two collision bugs from PR review:

  • Activity collision: Two activities with identical summaries but different steps (e.g., "Read 1 file" reading different files) could display the wrong block. Fixed by including step digest in cache key.
  • FileDiff collision: Two file-diffs with empty DiffVerification but different args could collide. Fixed by including ToolName and args digest in cache key when using fallback rendering path.

Added regression tests:

  • TestActivityCollisionPrevention verifies activities with same summary but different steps don't collide.
  • TestFileDiffCollisionPrevention verifies file-diffs with different args don't collide.

Changes

K8s Recording Fix

  • pkg/sandbox/backend_browser_wire.go: Added backendShellCommand() helper, wrapped 4 exec calls in startBackendRecording()
  • pkg/sandbox/backend_browser_wire_test.go: Added unit tests for wrapper behavior and end-to-end K8s recording test

TUI Render-Cache

  • pkg/tui/app.go: Added cache digest helpers, included digests in activity and file-diff cache keys
  • pkg/tui/app_render_test.go: Added collision regression tests

Backward Compatibility

✅ Docker backend behavior unchanged
✅ OpenShell backend unaffected
✅ Non-cached item types unaffected
✅ Breaking changes: None

Tests

  • All sandbox package tests pass
  • All TUI tests pass including new collision regression tests
  • Full build verified clean
  • No linting issues

Performance fix for TUI slowdown in long sessions. Three changes:

1. Per-item render cache (app.go)
   - Add renderedBlock struct caching padded+painted block, plain-text
     lines, and content spans per finalized transcript item
   - itemRenderCache map[string]renderedBlock on model, keyed by
     width+kind+content+expanded+routing
   - Streaming/provisional items bypass the cache
   - Cache cleared on WindowSizeMsg (terminal resize)
   - Eliminates re-running padBlock->applySelectionToBlock->
     paintTranscriptBlock->ANSI-strip for every historical item on
     every refreshViewport()

2. Selection-scoped highlight application (selection.go, app.go)
   - Add selectionIntersectsLines() helper method
   - Gate applySelectionToBlock() to only blocks whose line range
     intersects the active selection
   - Drag selection is now O(selected blocks) not O(all items)

3. Mouse-motion refresh debounce (app.go)
   - Rate-limit refreshViewport() in handleMouseMotion to 16ms (~60fps)
   - Prevents CPU saturation from high-frequency terminal mouse events

Benchmark (200-item session, Apple M4 Pro):
  BenchmarkRenderTranscript200Items/warm_cache:   ~2.3ms/op
  BenchmarkRenderTranscriptWithSelection:          ~1.25ms/op

Tests: all pkg/tui/... pass; new TestSelectionIntersectsLines added;
TestWindowResizeClearsMarkdownCache updated to cover itemRenderCache.
Fixes the xdpyinfo error 'probe display size: no dimensions in xdpyinfo output'
when running drills with browser_start_recording on Kubernetes. The root cause
is that kubectl exec runs commands in the pod base namespace (thin Debian image)
rather than inside the chroot overlay at /sandbox/rootfs where xdpyinfo, ffmpeg,
and other tools are installed.

Solution: Added backendShellCommand() helper that wraps shell commands through
/usr/local/bin/astonish-shell on K8s backends (which chroots into /sandbox/rootfs),
while using plain sh -c on Docker (where overlay is root). This mirrors the pattern
already established in backend_mcp_transport.go for MCP transport.

Applied wrapper to all 4 exec calls in startBackendRecording:
- Display probe (xdpyinfo)
- mkdir for recording output directory
- ffmpeg start script
- ffmpeg stop script

Added unit tests:
- TestBackendShellCommand_Docker: Docker uses plain sh -c
- TestBackendShellCommand_K8s: K8s uses astonish-shell wrapper
- TestBackendShellCommand_NilBackend: nil backend defaults to Docker mode
- TestStartBackendRecording_K8sUsesAstonishShell: E2E test of recording flow

All sandbox package tests pass.
Fixes visual-correctness issues identified in PR review where two activities with
identical summaries but different steps (e.g., 'Read 1 file' reading different
files) could display the wrong transcript block due to cache-key collisions.

Changes:
- Add stepsCacheDigest(), argsCacheDigest(), resultCacheDigest() helpers that
  compute stable FNV-1a hashes of step fields for cache-key inclusion.
- Include step digest in activity cache key to prevent collisions on identical
  summaries.
- Include ToolName and args digest in file-diff cache key when DiffVerification
  is empty (fallback path to DiffFromToolArgs).
- Add TestActivityCollisionPrevention regression test verifying two activities
  with same summary but different steps have different cache keys and render
  distinct output.
- Add TestFileDiffCollisionPrevention regression test verifying two file-diffs
  with same content but different args have different cache keys and render
  distinct output.

All TUI tests pass; no regressions.
@rschardosin rschardosin changed the title Fix: K8s run_drill recording via astonish-shell wrapper K8s sandbox recording fix + TUI render-cache with collision prevention Sep 8, 2026
Four changes addressing review comments on #506:

1. De-duplicate agent badge rendering (Medium issue)
   Extract applyRoutingBadge(md, cw, it) helper method. Both the streaming
   bypass path and the finalized/cacheable path now call this single helper,
   guaranteeing the streaming and finalized bubbles render identically.
   Eliminates 25-line copy-paste that could silently diverge.

2. Bound/evict itemRenderCache after each render pass (Medium issue)
   Track usedCacheKeys in each renderTranscript call. After the for loop,
   delete any map entry not referenced in this pass. This prevents unbounded
   growth from orphaned entries (opposite expand/collapse states, items
   removed by /compact). The cache is now bounded to at most len(tr.Items)
   entries after every render.

3. Harden TestWindowResizeClearsMarkdownCache (Minor)
   Replace the t.Skip("cache key format changed") guard with an explicit
   prefix scan: any key starting with the old-width prefix ("80\x00") in
   either mdCache or itemRenderCache after resize fails the test loudly.
   A future key-format change now errors instead of silently skipping.

4. Fix hash formatting and rune-safe truncation (Minor)
   - strconv.FormatInt(int64(hash), 16) -> FormatUint(hash, 16): avoids
     negative-looking hex for high uint64 values.
   - argsCacheDigest / resultCacheDigest truncation now uses []rune slicing
     to avoid splitting a multi-byte UTF-8 rune mid-character.
@rschardosin
rschardosin merged commit e7c4261 into main Sep 8, 2026
5 checks passed
@rschardosin
rschardosin deleted the tui-perf-render-cache branch September 8, 2026 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant