Cherry-picks for release/v0.6.3 (round 2)#24412
Conversation
The word count threshold was arbitrary and prevented the skill from being used for shorter content that would still benefit from the document editor. Co-authored-by: Claude <noreply@anthropic.com>
…Stack to fix motionVectors hang (LUM-795) (#24375) * perf: replace .move(edge: .bottom) transitions with .opacity in LazyVStack to fix motionVectors hang (LUM-795) Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai> * docs: add LazyVStack transition anti-pattern rule to AGENTS.md and inline warning Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: ashlee@vellum.ai <ashlee@vellum.ai>
…fix 134s hang (LUM-740) (#24321) Move .frame(maxWidth: VSpacing.chatColumnMaxWidth) and .frame(maxWidth: .infinity) from inside the ScrollView content (MessageListContentView) to the ScrollView itself (MessageListView). These modifiers compile to _FlexFrameLayout which calls sizeThatFits on children. When placed between ScrollView and LazyVStack, new message insertions trigger initialPlacement → motionVectors → measureEstimates over ALL ForEach items, defeating laziness entirely (O(n) where each cell is complex). Moving them outside the ScrollView removes _FlexFrameLayout from the scroll content measurement path. The ScrollView is a greedy container that accepts its proposed size without measuring content, so width constraints on its frame work correctly without triggering child measurement. Padding stays inside scroll content for correct insets. Visual alignment is preserved: chatColumnMaxWidth = chatBubbleMaxWidth + 2*xl, and the internal padding of xl on each side still yields chatBubbleMaxWidth for bubble content. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: ashlee@vellum.ai <ashlee@vellum.ai>
…AssistantProgressView) (#23982) * [LUM-718] perf: flatten inner view hierarchies to reduce layout depth Reduce per-cell layout depth by flattening redundant modifier chains and view wrappers in chat bubble inner views: - Remove unnecessary Group wrappers around switch statements in CompactPermissionChip and ChatBubbleToolStatusView.compactPermissionChip (ViewBuilder handles switches natively since Swift 5.3) - Remove single-child VStack wrapper around stepTitle Text in StepDetailRow - Flatten nested VStack > VStack in stepDetailContent technical details (both had identical spacing: VSpacing.xs) - Combine .background() + .clipShape() + .overlay() into single .background {} with fill+stroke in outputBlock - Replace VStack > HStack > Spacer with HStack + .frame(alignment:) in ChatBubbleToolStatusView trailing status - Merge two .padding() calls into single EdgeInsets in CodeBlockView header Estimated depth reduction: ~8-10 layout nodes per cell. Co-Authored-By: Jason Zhou <jasonczhou3@gmail.com> * [LUM-718] Restore .clipShape() to prevent content overflow at rounded corners The .clipShape(RoundedRectangle) is needed to clip scrollable content (e.g., 500+ line outputs in the ScrollView) to the rounded background. Without it, text near corners visually bleeds past the rounded border. Co-Authored-By: Jason Zhou <jasonczhou3@gmail.com> * [LUM-718] Separate stroke into overlay to prevent clipping The stroke was inside .background {} which gets clipped by .clipShape(), halving the visible border width (0.25pt instead of 0.5pt). Move the stroke to .overlay() after .clipShape() so the full border is drawn on top, matching the existing pattern at line 758-762. Co-Authored-By: Jason Zhou <jasonczhou3@gmail.com> * [LUM-718] Add hit testing and accessibility guards to opacity-hidden copy buttons Pre-existing issue: VCopyButton instances used .opacity(isHovered ? 1 : 0) without .allowsHitTesting or .accessibilityHidden guards. When opacity is 0, the button was still tappable and visible to VoiceOver. Added both guards to the two VCopyButton instances in CodeBlockView (language header and no-language overlay). Co-Authored-By: Jason Zhou <jasonczhou3@gmail.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Jason Zhou <jasonczhou3@gmail.com>
…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.
…-cycle mutation (#24332) onChange handlers for isSending and messages.count can fire during the same SwiftUI update pass that triggered them. Writing to ScrollPosition synchronously trips SwiftUI's "Modifying state during view update" runtime guard. Introduce scheduleDeferredBottomPin() which coalesces repeated requests via a generation counter and defers the actual ScrollPosition write to the next main-queue turn. Reset and cancelAll properly invalidate pending deferred pins.
… wrappers (#24337) Replace direct withAnimation(VAnimation.fast) wrappers on emoji/slash menu state toggles with a ComposerMenuRefreshScheduler that defers updates to the next run-loop tick. This coalesces rapid onChange firings (e.g. from inputText + cursorPosition changing together) into a single menu state update, eliminating redundant SwiftUI animation cycles and preventing potential layout thrash. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ffloading computation (#24274) * fix: cache inferCategory results and move computation off main thread (LUM-498) SkillsManager.rebuildCategoryMap now reuses cached values from the existing categoryMap, only calling inferCategory for skills not already in the cache. This eliminates redundant string allocation and keyword matching on every objectWillChange notification. IdentityPanel.fetchSkills now runs the inferCategory loop in a Task.detached so it executes off the main actor, preventing the main thread hang during initial skill load. Co-Authored-By: tkheyfets <timur@vellum.ai> * fix: invalidate category cache when skill metadata changes Track a fingerprint (name + description) alongside each cached category entry so that rebuildCategoryMap re-runs inferCategory when a skill's classification inputs change while keeping the same ID. Co-Authored-By: tkheyfets <timur@vellum.ai> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: tkheyfets <timur@vellum.ai>
#24402) Co-authored-by: Claude <noreply@anthropic.com>
![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: f3d3b92423
ℹ️ 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".
| } | ||
| if anchorMessageId == nil { | ||
| scrollState.requestPinToBottom(animated: true) | ||
| scrollState.scheduleDeferredBottomPin(animated: true) |
There was a problem hiding this comment.
This schedules a second deferred bottom-pin with default flags immediately after handleSendingChanged() already queued one with forceFollowingBottom: true. Because scheduleDeferredBottomPin coalesces by generation and only executes the latest request, the forced transition can be dropped when isSending and messages.count change in the same update cycle. In that case, users who send while in freeBrowsing can fail to reattach to bottom because the surviving request runs without forcing .followingBottom, and requestPinToBottom then no-ops under mode checks.
Useful? React with 👍 / 👎.
| if let reason = toolCall.reasonDescription, !reason.isEmpty { | ||
| Text(toolCall.actionDescription) | ||
| .font(VFont.labelDefault) | ||
| .foregroundStyle(VColor.contentSecondary) | ||
| if !resolvedInputFull.isEmpty { |
There was a problem hiding this comment.
The input/details rendering is now nested under if let reason = toolCall.reasonDescription, so tool calls that do not provide a reason no longer show their input block in expanded details. Most tool calls don't populate reasonDescription, so this regresses debuggability by hiding the primary technical context users previously saw for completed steps.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if let reason = toolCall.reasonDescription, !reason.isEmpty { | ||
| Text(toolCall.actionDescription) | ||
| .font(VFont.labelDefault) | ||
| .foregroundStyle(VColor.contentSecondary) | ||
| if !resolvedInputFull.isEmpty { | ||
| let inputIsLong = cachedInputIsLong ?? false | ||
|
|
||
| if inputIsLong { | ||
| ScrollView { | ||
| Text(resolvedInputFull) | ||
| .font(VFont.bodySmallDefault) | ||
| .foregroundStyle(VColor.contentSecondary) | ||
| .frame(maxWidth: .infinity, alignment: .leading) | ||
| } | ||
| .frame(height: 300) | ||
| .clipShape(RoundedRectangle(cornerRadius: VRadius.sm)) | ||
| } else { | ||
| Text(resolvedInputFull) | ||
| .font(VFont.bodySmallDefault) | ||
| .foregroundStyle(VColor.contentSecondary) | ||
| } | ||
| outputBlock( | ||
| text: resolvedInputFull, | ||
| attributedText: nil, | ||
| copyText: resolvedInputFull, | ||
| copyLabel: "Copy input" | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 Refactoring incorrectly gates friendlyName removal and resolvedInputFull display behind reasonDescription check
When the inner VStack was removed from stepDetailContent, two previously unconditional elements were lost or incorrectly nested. In the old code (git show af737113b), Text(toolCall.friendlyName) was always rendered and resolvedInputFull was shown whenever non-empty — both lived as siblings of the if let reason guard inside a shared VStack. In the new code, friendlyName is completely removed, and resolvedInputFull is now nested inside the if let reason = toolCall.reasonDescription, !reason.isEmpty guard. This means for any tool call without a reasonDescription (which is the common case for most tools), the expanded "Technical details" section displays only the header label with zero content below it — users see no tool name identifier and no input details.
Old code structure (before refactoring)
VStack {
if let reason = toolCall.reasonDescription, !reason.isEmpty {
Text(toolCall.actionDescription) // conditional
}
Text(toolCall.friendlyName) // UNCONDITIONAL
if !resolvedInputFull.isEmpty { // UNCONDITIONAL
// input text
}
}Prompt for agents
In stepDetailContent (AssistantProgressView.swift around line 971), the refactoring that removed the inner VStack incorrectly nested resolvedInputFull inside the if-let-reason guard and dropped toolCall.friendlyName entirely. In the original code (visible via git show af737113b), three elements were siblings inside a VStack: (1) conditionally, Text(toolCall.actionDescription) when reasonDescription exists, (2) unconditionally, Text(toolCall.friendlyName), and (3) unconditionally, the resolvedInputFull text block when non-empty. The fix should restore the original semantic structure: keep Text(toolCall.actionDescription) conditional on reasonDescription, but move Text(toolCall.friendlyName) and the resolvedInputFull outputBlock outside the if-let-reason guard so they are always displayed in the Technical details section. The friendlyName provides the raw tool identifier and the resolvedInputFull shows the tool input — both are essential for the expanded detail view regardless of whether a reasonDescription exists.
Was this helpful? React with 👍 or 👎 to provide feedback.
| .padding(.bottom, VSpacing.sm) | ||
| .textSelection(.enabled) | ||
| .onAppear { | ||
| if cachedColoredResult == nil, | ||
| let result = toolCall.result, !result.isEmpty { | ||
| cachedColoredResult = coloredOutput(result, isError: toolCall.isError) | ||
| } | ||
| if cachedInputIsLong == nil && !resolvedInputFull.isEmpty { | ||
| let lines = resolvedInputFull.utf8.reduce(1) { c, b in b == 0x0A ? c + 1 : c } | ||
| cachedInputIsLong = lines > 30 || (lines == 1 && resolvedInputFull.utf8.count > 50_000) | ||
| } | ||
| } | ||
| .onChange(of: toolCall.result) { _, newResult in | ||
| if let result = newResult, !result.isEmpty { | ||
| cachedColoredResult = coloredOutput(result, isError: toolCall.isError) | ||
| } else { | ||
| cachedColoredResult = nil | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🚩 Removal of .textSelection(.enabled) from stepDetailContent reduces user capability
The old code had .textSelection(.enabled) on the stepDetailContent VStack (previously around line 1039), allowing users to select and copy arbitrary portions of text from expanded tool details. This was removed in the refactoring. The outputBlock still provides a whole-block copy button, so users can copy entire outputs, but they can no longer select specific text spans. This is a usability regression but may be intentional to simplify hit-testing or avoid interaction conflicts with the copy button overlay.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return nil | ||
| } | ||
|
|
||
| private func scheduleComposerMenuRefresh() { |
There was a problem hiding this comment.
🚩 Slash/emoji menu animations removed entirely — popups now appear/disappear without transition
Both withAnimation(VAnimation.fast) wrappers around showSlashMenu/showEmojiMenu state changes AND the parent .animation(VAnimation.fast, value: showSlashMenu/showEmojiMenu) modifiers were removed from ComposerView.swift:144-145. The popup views (SlashCommandPopup, EmojiPickerPopup) still declare .transition(.opacity.combined(with: .move(edge: .bottom))), but without any animation context these transitions are inert — the menus appear and disappear instantly. This may be intentional (the commit says "remove withAnimation wrappers" to fix state-mutation timing issues), but it's a visual regression from the previous animated behavior. If animation is desired, an implicit .animation() modifier scoped to only these values could be re-added without the withAnimation timing problems.
(Refers to lines 114-124)
Was this helpful? React with 👍 or 👎 to provide feedback.
3 participants
Summary
Cherry-picks the following commits from
mainontorelease/v0.6.3:Conflict resolution
One conflict in
clients/macos/vellum-assistant/Features/Chat/AssistantProgressView.swiftduring the[LUM-718]commit. The incoming commit flattened an innerVStackwrapper and retained the olderoutputBlock-based technical details rendering, butrelease/v0.6.3already had a different implementation (from #24019) using inlineScrollView/Textthat achieved similar goals. Kept the HEAD version. The follow-up commit2d57cd909then restructured that section further, landing cleanly on top.Test plan
swift buildpasses locally