Fix stack overflow with many attributed runs#1
Conversation
The reduce in TextBuilder built the result via Text("\(partialResult)\(text)"),
wrapping every attributed run in a LocalizedStringKey-backed Text.
At render time, SwiftUI's LocalizedStringKey.resolve walks the chain
and recurses ~8 stack frames per nesting level, overflowing the
main-thread stack for content that produces a few hundred runs
(e.g. emoji-dense notes where each emoji forms its own run).
Switch to Text + Text concatenation, which produces concat storage
that resolves linearly.
Fixes gonzalezreal#56
|
@claude can you review this PR? |
|
@codex please review this PR - it says it can't be merged automatically, so we need to check why and what to do as well as the PR itself |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a996e51850
ℹ️ 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".
| // reduce step; resolving the resulting N-deep chain recurses ~8 stack frames per | ||
| // level during SwiftUI's render pass and overflows the main-thread stack for | ||
| // content that produces a few hundred attributed runs (e.g. emoji-dense notes). | ||
| self = textValues.reduce(Text(verbatim: ""), +) |
There was a problem hiding this comment.
When this PR is merged into current main, this line conflicts with the existing TextBuilder rewrite there; resolving the conflict by keeping this reduce(..., +) path would drop the plain-run coalescing and balanced concatenation that main already uses. For inputs with thousands of attributed runs, such as large syntax-highlighted code blocks or emoji-dense text, this still builds one left-deep Text concatenation node per run, so SwiftUI can recurse through thousands of ConcatenatedTextStorage nodes and hit the same stack-overflow class this PR is meant to fix. Resolve the conflict by keeping the current main implementation rather than replacing it with this left fold.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a996e51850
ℹ️ 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".
| // reduce step; resolving the resulting N-deep chain recurses ~8 stack frames per | ||
| // level during SwiftUI's render pass and overflows the main-thread stack for | ||
| // content that produces a few hundred attributed runs (e.g. emoji-dense notes). | ||
| self = textValues.reduce(Text(verbatim: ""), +) |
There was a problem hiding this comment.
When this PR is merged into current main, this line conflicts with the existing TextBuilder rewrite there; resolving the conflict by keeping this reduce(..., +) path would drop the plain-run coalescing and balanced concatenation that main already uses. For inputs with thousands of attributed runs, such as large syntax-highlighted code blocks or emoji-dense text, this still builds one left-deep Text concatenation node per run, so SwiftUI can recurse through thousands of ConcatenatedTextStorage nodes and hit the same stack-overflow class this PR is meant to fix. Resolve the conflict by keeping the current main implementation rather than replacing it with this left fold.
Useful? React with 👍 / 👎.
|
@codex Please review the comments you made on this PR and implement fixes |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a996e51850
ℹ️ 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".
| // reduce step; resolving the resulting N-deep chain recurses ~8 stack frames per | ||
| // level during SwiftUI's render pass and overflows the main-thread stack for | ||
| // content that produces a few hundred attributed runs (e.g. emoji-dense notes). | ||
| self = textValues.reduce(Text(verbatim: ""), +) |
There was a problem hiding this comment.
For inputs that still produce many runs, such as a large syntax-highlighted code block built in HighlightedTextFragment.Model.highlight, this left fold constructs an N-deep Text + Text tree. That avoids the LocalizedStringKey recursion from the old interpolation path, but it still leaves SwiftUI with a deeply nested concatenation to resolve during layout, so the same stack-overflow class can recur when there are thousands of token/link/attachment runs. Build the concatenation as a balanced tree (or coalesce plain attributed runs before concatenating) so tree depth is bounded instead of proportional to the number of runs.
Useful? React with 👍 / 👎.
|
@codex Please review the comments you made on this PR and implement fixes |
Summary
Testing
|
|
@codex this PR can't merge - please review and fix the blocking issues |
|
Summary
Testing
|
|
Closing — superseded by commit 8b1e47a ("fix(textfragment): bound Text recursion depth in TextBuilder") which is already on The fix on
As the file header comment on |
2 participants
Cherry-picked from gonzalezreal/textual PR gonzalezreal#57
Fixes stack overflow when rendering text with many attributed runs (e.g., emoji-dense text). Switches from Text interpolation to Text concatenation to avoid deep nesting that causes stack overflow in LocalizedStringKey.resolve.