perf: cap ForEach item count and eliminate cell-level FlexFrames (LUM-945)

[devin-ai-integration]

Summary

Addresses the remaining main-thread hang after PR #26053 eliminated the ancestor-level _FlexFrameLayout cascade. Spindump analysis of a local build with #26053 applied showed a ~29s residual hang from two independent layers: (1) unbounded ForEach item count when displayedMessageCount escalated to Int.max, and (2) cell-level _FlexFrameLayout alignment queries on video/audio embeds.

Three changes, ordered by impact:

1. Replace .frame(maxWidth: 360) → .widthCap(360) on media embeds (HIGHEST IMPACT)

InlineVideoAttachmentView and InlineAudioAttachmentView used .frame(maxWidth: 360), which creates _FlexFrameLayout — triggering explicitAlignment queries through the LazyVStack subtree on every cell measurement. WidthCapLayout (Layout protocol, O(1)) is already used in 5+ places in the codebase (PRs #24589, #26007). PR #25947 previously reverted this specific change, reasoning that "bounded maxWidth: 360 is NOT the problematic .infinity pattern" — but any _FlexFrameLayout with a maxWidth parameter still triggers alignment cascade queries through the cell hierarchy. The difference is magnitude, not kind.

Also made WidthCapLayout (struct, init, methods, and widthCap extension) public for cross-module access — the media embed views are in VellumAssistantLib while WidthCapLayout is in VellumAssistantShared.

2. Replace Int.max sentinel with isShowAllMode: Bool

displayedMessageCount = Int.max was used as a sentinel meaning "show all messages." This conflated two concerns:

• Behavioral: "don't shrink the window when new messages arrive"
• Sizing: "how many items go into the ForEach"

When active, all messages entered the ForEach — making LazyVStack measure every cell. The new isShowAllMode flag decouples these. Updated across all sites in ChatPaginationState, ChatViewModel, iOS ChatContentView, ChatVisibleMessageFilter, and test call sites in ConversationForkNavigationIOSTests.

All 4 trim/reset paths in ChatViewModel (trimOldMessagesIfNeeded, trimForBackground, memory pressure handler, populateFromHistory initial load) now also reset isShowAllMode = false alongside displayedMessageCount = messagePageSize to keep pagination state consistent.

3. Cap paginatedVisibleMessages at 400 items (macOS)

New maxPaginatedWindowSize = 400 constant. In show-all mode, recomputePaginatedSuffix() applies min(visible.count, 400) so the macOS ForEach is always bounded. Users see the latest 400 messages; older messages are accessible via pagination. Follow-up issue LUM-952 tracks sliding window logic so cap-hidden messages become reachable when conversations exceed 400 visible messages.

4. Update clients/macos/AGENTS.md FlexFrame guidance

• Expanded the FlexFrame rule to explicitly cover all maxWidth/maxHeight variants (not just maxWidth:alignment:), and clarified that bounded values like .frame(maxWidth: 360) are equally problematic — the difference is magnitude, not kind.
• Added .widthCap(N) as the first-listed safe alternative (it was missing entirely — agents could only discover it by reading source).
• Clarified the "Do NOT use a custom Layout barrier" caveat to specify it applies between the outer modifier chain and the ScrollView — not to Layout-protocol modifiers used inside cells (like WidthCapLayout). An agent reading the old wording could misinterpret it as "don't use custom Layout at all in lazy containers."

---

Root Cause Analysis

1. How did the code get into this state?

The Int.max sentinel was introduced in PR #7852 (Feb 2026, Timur + Claude). The original pagination (PR #7770) capped displayedMessageCount at displayedMessages.count, but this caused a regression: when new messages arrived via streaming, the window shrank back to a suffix, hiding messages the user had already scrolled through. Int.max was a quick fix — "just make the count bigger than any possible message list."

This worked fine initially because conversations were small. But as daemon history pagination was added (PR #9235), conversations could accumulate hundreds or thousands of messages. Each time the daemon said "no more pages," it set displayedMessageCount = Int.max — putting ALL messages into the ForEach permanently.

The cell-level FlexFrames (.frame(maxWidth: 360)) on video/audio views were introduced when those views were first created. When PR #25947 audited FlexFrames, it initially changed these to .frame(width: 360), but review feedback noted this would clip on narrow windows, so it was reverted with the incorrect reasoning that bounded maxWidth wasn't problematic.

2. What mistakes or decisions led to it?

1. Using a magic sentinel value instead of a separate flag. Int.max overloaded the meaning of displayedMessageCount — it simultaneously meant "don't shrink" and "show everything." A boolean flag would have been the correct abstraction from the start.

2. Incomplete understanding of _FlexFrameLayout during PR fix(chat): Remove remaining FlexFrame anti-patterns from LazyVStack cell views #25947 audit. The review concluded bounded maxWidth was safe because it's "not the .infinity pattern." But _FlexFrameLayout is created for ANY .frame(maxWidth:) call — it's the modifier type, not the parameter, that determines alignment cascade behavior.

3. No upper bound on ForEach item count. The pagination system had no defensive cap. The implicit assumption was that displayedMessageCount would always be "reasonable," but Int.max violated that by definition.

3. Were there warning signs we missed?

• The Int.max sentinel was documented in comments as a known oddity across multiple files, but nobody flagged the performance implication.
• PR fix(chat): Remove remaining FlexFrame anti-patterns from LazyVStack cell views #25947's review cycle was itself a warning sign — it went through three iterations trying to find a correct fix for bounded maxWidth, without discovering WidthCapLayout (introduced 3 weeks earlier in PR fix: eliminate _FlexFrameLayout anti-pattern from chat cell hierarchy #24589 for exactly this pattern).
• The hang was masked by the larger ancestor-level cascade. The 29s cell-level cost was dominated by the 105s ancestor cost until PR fix: replace FlexFrame with Layout protocol to eliminate main-thread hang (LUM-944) #26053 removed that layer.

4. What can we do to prevent this pattern from recurring?

• Never use Int.max (or similar magic values) as sentinel states for collection sizes. Use a separate boolean or enum. Magic sentinels that interact with collection APIs (suffix(), min()) create subtle bugs because the extreme value flows through arithmetic silently.
• AGENTS.md updated in this PR — the FlexFrame guidance in clients/macos/AGENTS.md now explicitly lists .widthCap() as the preferred safe alternative, clarifies that bounded maxWidth values are equally dangerous, and disambiguates the custom Layout caveat so agents don't avoid WidthCapLayout.

---

Test plan

• No Swift linter configured in repo; TypeScript lint is N/A for these changes
• CI runs security checks (macOS Build/Tests/Lint are skipped — no Xcode runner)
• Requires local macOS build verification — the function signature changes to resolvePendingChatAnchor and nextPendingChatAnchorSearchStep will fail at compile time if any call site was missed. The WidthCapLayout access level was already caught and fixed via local build.
• iOS test file (ConversationForkNavigationIOSTests) updated with new isShowAllMode parameter at all 6 call sites

Human review checklist

• iOS has no 400-item cap in show-all mode: The maxPaginatedWindowSize = 400 cap is applied in recomputePaginatedSuffix() (macOS path). iOS computes its own visibleMessages inline and returns ALL messages when isShowAllMode is true — unbounded. Decide whether iOS needs a matching cap or if iOS conversations are small enough in practice.
• 400-item cap interaction with active pagination (macOS): When the user has explicitly paginated back and isShowAllMode is true, recomputePaginatedSuffix caps at suffix(400). Older loaded messages outside that window are invisible and unreachable (LUM-952 tracks sliding window fix). Is this acceptable for now?
• populateFromHistory when isShowAllMode && hasMore: The new code skips incrementing displayedMessageCount entirely in this branch. The intent is that the Combine pipeline will re-derive paginatedVisibleMessages from the updated messages array. Verify the prepended (older) messages actually appear — they'd be at the start of the array, and suffix(400) takes from the end, so they may be outside the window.
• Double-mutation in iOS ChatContentView:829-830: Setting isShowAllMode = true then displayedMessageCount = ... triggers recomputeVisibleMessages twice via the ChatViewModel setter. Verify no problematic intermediate state.
• No remaining Int.max in pagination paths: grep confirms remaining Int.max usages are in unrelated files (emoji tests, conversation display order, queue position, hash function).
• AGENTS.md update: Review the updated FlexFrame guidance in clients/macos/AGENTS.md for accuracy and completeness.

Related PRs

PR	Description	Relationship
#26053	BottomAlignedMinHeightLayout — ancestor-level FlexFrame fix (LUM-944)	Prerequisite; exposed this residual hang
#25947	Cell-level FlexFrame audit	Incorrectly preserved .frame(maxWidth: 360) — this PR fixes that
#24589	Original WidthCapLayout introduction	Created the Layout protocol struct we use here
#26007	FlexFrame batch 3 — expanded WidthCapLayout usage	Same pattern adopted here
#7852	Original Int.max show-all mode	Introduced the sentinel this PR replaces

Follow-up issues

• LUM-952 — Sliding window pagination so cap-hidden messages (beyond 400) are reachable

Closes LUM-945

Apple refs checked (2026-04-16): Layout protocol, WWDC22 — Compose custom layouts with SwiftUI

Link to Devin session: https://app.devin.ai/sessions/ffe548bed9ae491baf803baf3f73ba7f
Requested by: @ashleeradka

---

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1797b6927a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Preserve access to older messages after capping show-all

In show-all mode this now always renders suffix(min(visible.count, 200)), so when older history is prepended the window remains pinned to the newest 200 items instead of revealing the newly loaded page. Once hasMoreHistory becomes false, pagination stops and already-loaded older messages are no longer reachable (and previously visible older rows can disappear after a load). For conversations longer than 200 messages, upward pagination effectively breaks.

Useful? React with 👍 / 👎.

[devin-ai-integration]

Valid concern. The 200-item cap in recomputePaginatedSuffix means that in show-all mode, only the 200 most recent messages are in the ForEach. However, this is the macOS path — paginatedVisibleMessages is consumed by the macOS MessageListView. The pagination sentinel still triggers when the user scrolls up, which calls loadPreviousMessagePage() to fetch more from the daemon. The cap doesn't prevent fetching — it just bounds what's in the ForEach at any given time.

That said, this is flagged in the "Human review checklist" in the PR description as an open UX question for the reviewer to decide on. The cap value (200) can be adjusted.

[chatgpt-codex-connector]

Keep show-all window stable when new messages arrive on iOS

When show-all mode is entered, displayedMessageCount is now set to the current total instead of an unbounded sentinel. iOS still computes visibleMessages using displayedMessageCount < all.count ? suffix(displayedMessageCount) : all (clients/ios/Views/ChatContentView.swift), so after any new incoming message the count becomes stale and older visible messages start dropping again. This reintroduces the “expanded window collapses on new messages” behavior for users who paged through history.

Useful? React with 👍 / 👎.

[devin-ai-integration]

Confirmed — this is a real bug. iOS visibleMessages was computing its own suffix inline and wasn't updated to check isShowAllMode. Fixed in commit 5dcd451: the guard now checks !viewModel.isShowAllMode so in show-all mode, all messages are returned regardless of displayedMessageCount.

[devin-ai-integration]

Devin Review found 3 potential issues.

View 3 additional findings in Devin Review.

[devin-ai-integration]

🚩 resetMessagePagination() has zero external callers

ChatViewModel.resetMessagePagination() at line 902-903 properly resets both isShowAllMode and displayedMessageCount, but it is never called from outside ChatViewModel (or even within it — no internal callers either). The four trim/history sites that need pagination resets (trimOldMessagesIfNeeded, trimForBackground, memory pressure handler, populateFromHistory) all set displayedMessageCount directly instead. This method was likely intended to be the single point of reset but was never wired in. Consider whether these sites should use resetMessagePagination() instead of manual field assignment.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Confirmed — all 4 sites now reset isShowAllMode = false alongside displayedMessageCount = Self.messagePageSize. Fixed in commit 5dcd451.

Regarding resetMessagePagination(): you're right that it has zero callers. However, it resets additional state (historyCursor, hasMoreHistory, isLoadingMoreMessages) that isn't appropriate for all 4 trim sites — e.g., trimOldMessagesIfNeeded() explicitly sets hasMoreHistory = true (because trimmed messages are re-fetchable from the daemon), which resetMessagePagination() would undo. The explicit field assignment is intentional at each site.

[devin-ai-integration]

Devin Review found 1 new potential issue.

View 5 additional findings in Devin Review.

[devin-ai-integration]

🟡 iOS visibleMessages bypasses maxPaginatedWindowSize cap in show-all mode

The PR introduces maxPaginatedWindowSize = 400 (ChatPaginationState.swift:23) specifically to cap ForEach item counts and prevent unbounded LazyVStack sizes. On macOS, recomputePaginatedSuffix() at ChatPaginationState.swift:146-148 correctly applies min(visible.count, Self.maxPaginatedWindowSize) when isShowAllMode is true. However, the iOS ChatContentView.visibleMessages computed property returns ALL messages when isShowAllMode is true — it short-circuits the guard and returns the full displayedMessages array with no cap. This means the LazyVStack at line 200 gets an unbounded item count on iOS in show-all mode, defeating the optimization this PR was designed to apply. This is particularly impactful on iOS where memory is more constrained.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

This is already flagged in the PR description's "Human review checklist" as the first item:

iOS has no 200-item cap in show-all mode: The maxPaginatedWindowSize = 200 cap is applied in recomputePaginatedSuffix() (macOS path). iOS computes its own visibleMessages inline and returns ALL messages when isShowAllMode is true — unbounded. Decide whether iOS needs a matching cap or if iOS conversations are small enough in practice.

This was an intentional decision to leave for the reviewer. The iOS path has its own inline visibleMessages that predates this PR — it was not introduced here. Option 2 (removing the duplicate slicing and using paginatedVisibleMessages) would be a larger refactor that changes the iOS pagination architecture, which felt out of scope for this fix. Applying the cap inline (option 1) is straightforward if the reviewer decides iOS needs it.

[vex-assistant-bot]

✦ Approved — solid design, clean execution

What this does

Three changes addressing the residual ~29s hang after PR #26053 removed the ancestor-level FlexFrame cascade:

1. .frame(maxWidth: 360) → .widthCap(360) on video/audio embeds — eliminates cell-level _FlexFrameLayout alignment queries. WidthCapLayout is already proven across 10+ sites in the codebase. Access level bump to public is the correct fix for cross-module usage (VellumAssistantLib → VellumAssistantShared).

2. Int.max sentinel → isShowAllMode: Bool — separates the behavioral concern ("don't shrink the window") from the sizing concern ("how many items in ForEach"). This is the right abstraction. The sentinel overloading was documented in comments across 4 files as a known oddity — now it's properly modeled.

3. maxPaginatedWindowSize = 200 cap — bounds the macOS ForEach even in show-all mode. 200 is reasonable (4× page size, covers any practical viewport).

KB cross-reference

• Anti-patterns KB confirms: ANY .frame(maxWidth:) creates _FlexFrameLayout regardless of the bound value. PR #25947 kept .frame(maxWidth: 360) with the reasoning that "bounded maxWidth is safe" — this was incorrect. The modifier type determines cascade behavior, not the parameter value. This PR corrects that gap.
• WidthCapLayout is the established pattern — used in InlineSurfaceRouter (5 sites), ModelListBubble, CommandListBubble. Consistent adoption here.
• No circular fix risk — Int.max was introduced in PR #7852 and never reverted/re-added. This is a clean replacement.
• No conflict with in-flight work — ChatPaginationState last touched in PR #23464 (April). ChatViewModel last substantive pagination change was the same PR.

Bugs caught and fixed

Devin Review found 2 real bugs in commit 1, both fixed in commit 2:

1. iOS visibleMessages not checking isShowAllMode — would hide new messages
2. Four trim/reset sites missing isShowAllMode = false — would leave stale show-all state

Both confirmed fixed. Good self-review loop.

Items for human review

Agreeing with the PR's own checklist — two items worth deciding:

1. iOS has no 200-cap in show-all mode. The macOS path caps at 200 via recomputePaginatedSuffix, but iOS computes its own visibleMessages inline and returns ALL messages when isShowAllMode is true. If iOS ever gets conversations with 500+ messages, same hang potential. Worth a follow-up ticket.

2. populateFromHistory with isShowAllMode && hasMore — when prepending older pages in show-all mode, displayedMessageCount isn't incremented (the if !isShowAllMode guard skips it). The prepended messages land at the start of the array, and suffix(200) takes from the end. So the newly fetched older messages won't be visible until the user scrolls up past the 200 boundary. This is correct behavior (they paginated to see older messages, the view already shows the newest 200), but worth verifying in QA that the pagination sentinel still triggers to reveal them.

Minor observation

The paired isShowAllMode = false; displayedMessageCount = Self.messagePageSize pattern triggers recomputeVisibleMessages twice through the ChatViewModel setters (each setter calls it). Not a correctness issue — the intermediate state is consistent — but if it ever becomes a hot path, could be collapsed into a single direct mutation on paginationState + one recompute call.

Verdict

Clean separation of concerns, correct use of established patterns, thorough root cause analysis, and good self-review catching real bugs. The AGENTS.md guideline suggestion in the PR description is also good — should land as a follow-up.

Ship it ✦