Fix AI block crash rendering grouped images#11766
Conversation
Co-Authored-By: Oz <oz-agent@warp.dev>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR replaces the grouped-image tooltip-handle slicing with a helper that safely uses only the available handle suffix, preventing the out-of-bounds panic when grouped markdown images render without enough preallocated handles. The added unit test covers exhausted and partially available handle slices.
Concerns
- For this user-facing change, please include screenshots or a screen recording demonstrating grouped AI block images rendering without crashing end to end.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
lucieleblanc
left a comment
lucieleblanc
left a comment
There was a problem hiding this comment.
Safety first 🚧
Small non-blocking comment on the overall tooltip organization that's out of scope for this cherry-pick PR.
| let handles_for_group = image_tooltip_handles_for_group( | ||
| props.image_section_tooltip_handles, | ||
| *props.starting_image_section_index, | ||
| image_group.images.len(), | ||
| ); |
There was a problem hiding this comment.
Very code-first follow-up: I don't love that we use a contiguous array of handles for these grouped image tooltips. It feels like a C/C++ pattern and I'm pretty sure we could have a more idiomatic Rust construct for this
There was a problem hiding this comment.
Yeah, agreed that it's a little weird. Didn't dig too deeply into why we do this (didn't want to blow up the scope of this PR), but aligned that we should probably fix. ZL's the owner here lol and not sure how high-prio since this isn't a functional issue.
2 participants
Description
Fixes a slice bounds panic in AI block rendering when collapsible reasoning or summarization text includes multiple renderable markdown image groups but has no preallocated persistent tooltip handles.
The grouped image path now treats tooltip handles as best-effort: it selects only the available handle suffix for a group and degrades to no persistent handles when exhausted, preserving image rendering while avoiding out-of-bounds slicing.
Agent Mode conversation: https://staging.warp.dev/conversation/fedec4e9-7678-47da-98b7-6b3547e06320
From Varoon: Some context here, but I'm fairly certain this is related to code changes introduced in ZL's PR here. My hunch is that the user's session included some collapsible block (reasoning/summarization) with Markdown images. Since these changes were merged within the user's noted "regression boundary", seem like a good candidate.
Linked Issue
Fixes #11679
ready-to-specorready-to-implement.Testing
cargo fmt --checkcargo clippy --workspace --all-targets --all-features --tests -- -D warningscargo nextest run -p warp -E 'test(image_tooltip_handles_for_group_uses_available_handles_only)'./script/runAgent Mode
CHANGELOG-BUG-FIX: Fixed a crash when rendering AI reasoning or summaries that contain multiple markdown images.
Co-Authored-By: Oz oz-agent@warp.dev