Skip to content

Cache file reads for diff rendering#1496

Merged
dgageot merged 1 commit intodocker:mainfrom
krissetto:improve-diff-rendering
Jan 26, 2026
Merged

Cache file reads for diff rendering#1496
dgageot merged 1 commit intodocker:mainfrom
krissetto:improve-diff-rendering

Conversation

@krissetto
Copy link
Copy Markdown
Contributor

@krissetto krissetto commented Jan 25, 2026

Also adds dedicated view for collapsed reasoning blocks (so we only show + - lines on a single line instead of the full diff), and the interface for collapsed reasoning block (or just compact) renders in general so we can differentiate how other tools render when in collapsed reasoning blocks in the future

Screencast (relevant bits at ~1:00):

Screencast.From.2026-01-25.12-54-16.mp4

@krissetto
Copy link
Copy Markdown
Contributor Author

/review

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Found 3 issues in the caching implementation that should be addressed:

  1. Memory Leak (HIGH): Unbounded cache growth - the global cache map is never cleared
  2. Race Condition (MEDIUM): Duplicate expensive work in renderEditFile on concurrent cache misses
  3. Race Condition (MEDIUM): Duplicate expensive work in countDiffLines on concurrent cache misses

The collapsed view interface and nil-safety look good. The main concerns are around the cache implementation needing eviction/cleanup and proper concurrency control.

Comment thread pkg/tui/components/tool/editfile/render.go
Comment thread pkg/tui/components/tool/editfile/render.go
Comment thread pkg/tui/components/tool/editfile/render.go
@krissetto krissetto dismissed github-actions[bot]’s stale review January 25, 2026 11:52

mostly irrelevant in the context of the TUI

@krissetto krissetto marked this pull request as ready for review January 25, 2026 11:58
@krissetto krissetto requested a review from a team as a code owner January 25, 2026 11:58
@krissetto
Copy link
Copy Markdown
Contributor Author

/review

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Reviewed the caching implementation for diff rendering. Found one potential memory management concern worth noting.

Finding

  • Unbounded Cache Growth (Medium severity, line 46 in render.go)

The cache improves performance as intended, but lacks a cleanup mechanism. While the impact is modest for typical sessions, consider adding cache eviction for robustness in long-running sessions.

Comment thread pkg/tui/components/tool/editfile/render.go
also adds dedicated view for collapsed reasoning blocks

Signed-off-by: Christopher Petito <chrisjpetito@gmail.com>
@krissetto krissetto force-pushed the improve-diff-rendering branch from b164b80 to df8514b Compare January 25, 2026 20:51
@dgageot dgageot merged commit ec1aaa0 into docker:main Jan 26, 2026
5 checks passed
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.

2 participants