Skip to content

fix(macOS): replace @State caches with NSCache and remove nested ScrollViews in StepDetailRow#24336

Merged
siddseethepalli merged 1 commit into
mainfrom
sidd/step-detail-nscache-scroll-cleanup
Apr 8, 2026
Merged

fix(macOS): replace @State caches with NSCache and remove nested ScrollViews in StepDetailRow#24336
siddseethepalli merged 1 commit into
mainfrom
sidd/step-detail-nscache-scroll-cleanup

Conversation

@siddseethepalli
Copy link
Copy Markdown
Contributor

Summary

  • Replace per-view @State cached properties with a shared static NSCache<NSString, StepDetailAttributedStringCacheEntry> (128-entry limit) so colored-output memoization lives outside SwiftUI's observation graph — eliminates state mutations during render cycles
  • Remove nested ScrollView wrappers for long tool output/input; the outer transcript handles scrolling, avoiding hit-testing and responder churn
  • Unify input text rendering through the existing outputBlock helper, removing duplicated long/short branching logic

…llViews in StepDetailRow

The previous approach used @State properties to cache colored output and
input-length flags, which could mutate SwiftUI state during the render
cycle. Replace with a static NSCache keyed by content hash so caching
lives outside SwiftUI's observation graph entirely.

Remove per-block nested ScrollViews for long tool output — the outer
transcript already scrolls, and nested scroll views cause hit-testing
and responder churn. Use fixedSize(horizontal: false, vertical: true)
so text self-sizes correctly. Unify input rendering through the existing
outputBlock helper.
@siddseethepalli siddseethepalli merged commit b8ab3a4 into main Apr 8, 2026
@siddseethepalli siddseethepalli deleted the sidd/step-detail-nscache-scroll-cleanup branch April 8, 2026 17:19
devin-ai-integration Bot added a commit that referenced this pull request Apr 8, 2026
…ache

Builds on the NSCache architecture from #24336 to bound worst-case
coloredOutput() cost. For results exceeding 500 lines, only the first
500 are processed into AttributedString segments — the rest are counted
but skipped. A truncation footer tells the user the full output is
available via the copy button.

Key changes:
- coloredOutput() now accepts a lineLimit parameter and uses String.Index
  iteration instead of components(separatedBy:) to avoid N substring allocs
- StepDetailAttributedStringCacheEntry stores totalLines alongside the
  attributed value so truncation state is cached with the output
- outputBlock() accepts an optional truncationFooter parameter
- cachedColoredEntry computed property returns the full cache entry so
  both the attributed string and truncation info are available

Co-Authored-By: tkheyfets <timur@vellum.ai>
@siddseethepalli
Copy link
Copy Markdown
Contributor Author

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8ab3a428e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

noanflaherty pushed a commit that referenced this pull request Apr 8, 2026
…llViews in StepDetailRow (#24336)

The previous approach used @State properties to cache colored output and
input-length flags, which could mutate SwiftUI state during the render
cycle. Replace with a static NSCache keyed by content hash so caching
lives outside SwiftUI's observation graph entirely.

Remove per-block nested ScrollViews for long tool output — the outer
transcript already scrolls, and nested scroll views cause hit-testing
and responder churn. Use fixedSize(horizontal: false, vertical: true)
so text self-sizes correctly. Unify input rendering through the existing
outputBlock helper.
devin-ai-integration Bot added a commit that referenced this pull request Apr 9, 2026
…ache

Builds on the NSCache architecture from #24336 to bound worst-case
coloredOutput() cost. For results exceeding 500 lines, only the first
500 are processed into AttributedString segments — the rest are counted
but skipped. A truncation footer tells the user the full output is
available via the copy button.

Key changes:
- coloredOutput() now accepts a lineLimit parameter and uses String.Index
  iteration instead of components(separatedBy:) to avoid N substring allocs
- StepDetailAttributedStringCacheEntry stores totalLines alongside the
  attributed value so truncation state is cached with the output
- outputBlock() accepts an optional truncationFooter parameter
- cachedColoredEntry computed property returns the full cache entry so
  both the attributed string and truncation info are available

Co-Authored-By: tkheyfets <timur@vellum.ai>
@siddseethepalli
Copy link
Copy Markdown
Contributor Author

Addressed in #25048

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