fix(app-builder): show newest V2 messages first with load-earlier option#4463
Conversation
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| }, [session.info.id]); | ||
|
|
||
| useEffect(() => { | ||
| setVisibleCount(prev => Math.min(prev, sessionState.messages.length)); |
There was a problem hiding this comment.
WARNING: visibleCount only ever shrinks (or resets on session-id change) — it never grows to re-anchor onto a newly-started user turn.
This effect clamps visibleCount down when sessionState.messages.length decreases, but does nothing when it increases (Math.min(prev, newLength) is a no-op for growth). tailFromLastUserTurn is only re-run in the session.info.id effect above, which fires on session switch, not when a new user message is appended to the same session.
Concretely: after the initial turn completes, visibleCount settles at that turn's message count (e.g. 2). When the user sends a new message, the optimistic user message is appended (messages.length +1) before the assistant's reply message exists yet. Because visibleCount wasn't re-derived, sessionState.messages.slice(-visibleCount) now shows a stale mix of the tail-end of the previous turn together with the brand-new user message, instead of just the new turn, until the assistant's message is created and the fixed-size window naturally catches up. This directly undermines the PR's stated goal ("the latest user turn and its ... assistant response are visible").
Consider re-deriving visibleCount via tailFromLastUserTurn whenever a new user-role message appears at the tail (not just clamping downward on shrink), so the window re-anchors to the new turn immediately rather than self-correcting only once the reply arrives.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe follow-up commit fixes the prior CRITICAL finding exactly as suggested: Files Reviewed (1 file)
Previous Review Summaries (2 snapshots, latest commit d8748c7)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit d8748c7)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe follow-up re-anchor fix correctly resolves the prior stale-mix issue on new user turns, but the visible tail window still never grows during a turn's own streaming, so the user's message (and earlier same-turn chunks) drop out of view as soon as the assistant appends more messages. Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous review (commit 87b606d)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe new V2 tail-pagination window in Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Reviewed by claude-sonnet-5 · Input: 48 · Output: 11.2K · Cached: 1.4M Review guidance: REVIEW.md from base branch |
The previous effect clamped visibleCount downward when messages shrank but never re-derived it on growth, so the window's left edge stayed fixed at the previous turn's boundary. When the user sent a new message, the optimistic user message was appended to the message list but messages.slice(-visibleCount) still showed a stale mix of the previous turn's tail plus the new user message until the assistant reply happened to push the fixed-size window past the boundary. Track the id of the tail-most user message in a ref and re-anchor the window (visibleCount = messages.length - lastUserIdx) whenever that id changes, which only happens when a new user turn is appended. Assistant streaming does not change the last-user-message id, so the window correctly stays anchored to the current turn as the reply streams in. The shrink clamp is preserved via a functional setter so a sudden shrink of the message list cannot leave visibleCount pointing past the end. Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
| setVisibleCount(messages.length - lastIdx); | ||
| return; | ||
| } | ||
| setVisibleCount(prev => (prev > messages.length ? messages.length : prev)); |
There was a problem hiding this comment.
CRITICAL: Visible window never grows during streaming, so the current turn's earlier messages (including the user's own prompt) disappear as soon as more messages are appended
visibleMessages = sessionState.messages.slice(-visibleCount) is a fixed-size sliding window. On a new user turn, visibleCount is reset to exactly messages.length - lastIdx (typically 1, just the new user message) — but once the assistant reply starts streaming and appends additional message entries (thinking/tool-call/text chunks, per splitByContiguousPrefix's static/dynamic split), this branch is not taken again for the same turn (the tail-most user id hasn't changed), so execution falls through to the clamp-only branch here, which never grows visibleCount. With visibleCount frozen at 1, slice(-1) now returns only the newest appended message, dropping the user's own message (and any earlier assistant chunks of the same turn) out of view for the rest of the turn.
This is a regression versus the pre-fix behavior: previously the (buggy) frozen visibleCount at least inherited a larger value from the prior turn, giving some accidental buffer before this manifested; now it's reliably set to 1 right at the turn boundary, so the user's own message vanishes on the very first assistant-streamed message of essentially every turn. This directly undermines the PR's stated goal of keeping "the latest user turn and its ... assistant response" visible.
| setVisibleCount(prev => (prev > messages.length ? messages.length : prev)); | |
| setVisibleCount(prev => Math.min(messages.length, Math.max(prev, messages.length - lastIdx))); |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
The previous effect clamped visibleCount downward but never grew it, so on a turn boundary visibleCount was reset to messages.length - lastIdx (typically 1, just the new user message). When the assistant reply then streamed additional message entries (thinking/tool-call/text chunks) at the tail, visibleCount stayed frozen at 1 and slice(-1) returned only the newest chunk, dropping the user's own prompt and earlier chunks of the current turn out of view. Clamp visibleCount to [messages.length - lastIdx, messages.length] during streaming so the window grows with the tail as new chunks arrive, while still preserving any prior Load-earlier expansion and resetting to the new turn's natural tail when a new user message lands. Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
wombatepiclandingstudio
force-pushed
the
fix/app-builder-v2-show-newest-first
branch
from
d8748c7 to
430b1dd
Compare
1 participant
Summary
App Builder V2 sessions currently render every message in chronological order (oldest → newest) and rely on auto-scroll to reach the bottom. On a long conversation, this means a slow initial render and a jarring jump. This change makes V2 sessions render only the tail of the conversation (from the last user turn onwards) and adds a Load earlier messages button at the top that reveals one older user turn per click.
paginateMessagesand show the same button).V2SessionMessages.shouldAutoScroll/ "Scroll to bottom" behavior continue to work because the visible slice always includes the tail where new streaming messages arrive.Follow-up fix in commit
e841fc978— re-anchor on new user turn:The first version of the effect clamped
visibleCountdownward whensessionState.messagesshrank, and re-rantailFromLastUserTurnonly whensession.info.idchanged (i.e. session switch). It did nothing when the message list grew with a new user message, so after the initial turn completed andvisibleCountsettled at e.g. 2, sending a new message would append the optimistic user message andmessages.slice(-visibleCount)would show a stale mix of the previous turn's tail plus the new user message until the assistant reply happened to push the fixed-size window past the boundary.The fix tracks the id of the tail-most user message in a ref and re-anchors the window to the new tail whenever that id changes (which only happens when a brand-new user message is appended at the tail; assistant streaming never changes it). The shrink clamp is preserved via a functional setter.
New helpers in
apps/web/src/components/app-builder/utils/filterMessages.ts:tailFromLastUserTurn(messages, isUser?)— returns the slice from the last user-role message to the end, or the full list if no user message exists.loadEarlierUserTurn(messages, currentVisibleCount, isUser?)— grows the visible window backwards by exactly one user turn.lastUserIndex(messages, isUser)— exported for the component to detect when the tail-most user message changes.Both pagination helpers are generic over the message type via a role-checker callback (defaults to
getMessageRolefor V1CloudMessage[]), and accept a V2StoredMessage[]via amsg => msg.info.rolecallback at the call site.Verification
Manual only — automated checks were run but the full test suite requires a live Postgres which is not available in this sandbox.
pnpm typecheckinapps/web— passes.pnpm linton the changed files — 0 warnings, 0 errors.pnpm format— clean.pnpm test -- src/components/app-builder/utils/filterMessages.test.ts— cannot run here (sandbox lacks Postgres for the web app's global Jest setup). The unit tests cover:tailFromLastUserTurn(no user / first / middle / re-anchor to newly appended user turn / user_feedback),loadEarlierUserTurn(no-op / grows one turn / fallback to full),lastUserIndex(none / middle / user_feedback),getMessageRole, and apaginateMessagesV1 regression check.UX smoke test (to be performed by reviewer locally):
Visual Changes
N/A — no visual change to the message bubbles or layout. The only new element is a "Load earlier messages" button at the top of V2 sessions, which uses the same styling as the existing V1 button (
Button variant="ghost" size="sm").Reviewer Notes
V2SessionMessagesnow usesuseState/useEffect/useCallback/useRefto manage the visible window; all four are already imported at the top of the file.[sessionState.messages, getRole](notvisibleCount), so it re-runs on every messages change but does not loop on its own setters: the functional-setter shrink path returnsprevunchanged when in range, which React bails out of. The new-user-message path returns early after settingvisibleCount.splitByContiguousPrefixstatic/dynamic split runs on the visible slice rather than the full message list, so the streaming boundary is always computed within what's actually shown.childSessionMessagesmap is unaffected — child data is keyed by child session id and only rendered when a visible parent message references it.session.info.idre-anchor effect from the first version was removed; the ref-based effect handles both intra-session new-turn and inter-session re-mounting (the component iskey={session.info.id}keyed, so a session switch already remounts it).