feat(web): group consecutive tool-use cards#604
feat(web): group consecutive tool-use cards#604junxin367 wants to merge 4 commits intotiann:mainfrom
Conversation
Add a web-only visible projection that groups consecutive root-level execution tools into expandable cards. Keep approval and question tools standalone, reuse older-history loading on expand, and add regression coverage for grouping and UI behavior.
There was a problem hiding this comment.
Review mode: initial
Findings
- [Major]
needsOlderHistoryis only set when the grouped run starts at index 0, so a tool group that is the oldest visible group but follows a text/event block never auto-loads older pages when expanded. That breaks the promised oldest-incomplete-group hydration on pages that begin with non-tool content.web/src/chat/toolGroups.ts:254-265
Suggested fix:let sawGroup = false for (let index = 0; index < blocks.length; index += 1) { ... const needsOlderHistory = options.hasMoreMessages && !sawGroup sawGroup = true ... }
Summary
- Review mode: initial. One blocking behavior gap found; no other issues confirmed in the diff.
Testing
- Not run (bun is unavailable in this environment).
HAPI Bot
| continue | ||
| } | ||
|
|
||
| const needsOlderHistory = options.hasMoreMessages && index === 0 |
There was a problem hiding this comment.
[Major] needsOlderHistory is only set when the grouped run starts at index 0, so a tool group that is the oldest visible group but follows a text/event block never auto-loads older pages when expanded. That breaks the promised oldest-incomplete-group hydration on pages that begin with non-tool content.
Suggested fix:
let sawGroup = false
for (let index = 0; index < blocks.length; index += 1) {
...
const needsOlderHistory = options.hasMoreMessages && !sawGroup
sawGroup = true
...
}There was a problem hiding this comment.
Fixed in cd6f0b5. needsOlderHistory now follows the first visible grouped run instead of index === 0, so oldest visible groups still hydrate when leading visible blocks are text/event content. Added regression coverage in web/src/chat/toolGroups.test.ts.
Mark needsOlderHistory on the first visible grouped tool run even when earlier visible blocks are non-tool content, and add regression coverage for the boundary.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Findings
- [Major] Incomplete tool groups can stop hydrating after the first older-page request.
ToolGroupCardstarts a load, but the same effect depends onctx.isLoadingMoreMessages; when the shared loader flips that flag, React runs the cleanup and marks the in-flight request as cancelled. The eventual promise resolution then cannot clearisHydratingHistory, and the next effect pass returns early onisHydratingHistory, so a multi-page incomplete group can stay stuck instead of loading until its boundary is complete. Evidence:web/src/components/ToolCard/ToolGroupCard.tsx:174.
Suggested fix:useEffect(() => { if (!open || !props.block.needsOlderHistory || historyExhausted) return if (!ctx.hasMoreMessages) { setHistoryExhausted(true) return } let cancelled = false setIsHydratingHistory(true) void ctx.loadOlderMessagesPreservingScroll() .then((loaded) => { if (cancelled) return setIsHydratingHistory(false) if (!loaded) setHistoryExhausted(true) }) .catch(() => { if (cancelled) return setIsHydratingHistory(false) setHistoryExhausted(true) }) return () => { cancelled = true } }, [ open, props.block, ctx.hasMoreMessages, ctx.loadOlderMessagesPreservingScroll, historyExhausted, ])
Summary
- Review mode: follow-up after new commits. The prior bot finding about marking the oldest visible group appears addressed, but grouped expansion still has a remaining hydration regression for incomplete history.
Testing
- Not run (automation;
bunis unavailable in this environment:/bin/bash: bun: command not found).
HAPI Bot
| setHistoryExhausted(false) | ||
| return | ||
| } | ||
| if (isHydratingHistory || ctx.isLoadingMoreMessages) { |
There was a problem hiding this comment.
[Major] Incomplete tool groups can stop hydrating after the first older-page request. This effect starts a load, but it also depends on ctx.isLoadingMoreMessages; when the shared loader flips that flag, React runs the cleanup and marks the in-flight request as cancelled. The eventual promise resolution then cannot clear isHydratingHistory, and the next effect pass returns early here, so a multi-page incomplete group can stay stuck instead of loading until its boundary is complete.
Suggested fix:
useEffect(() => {
if (!open || !props.block.needsOlderHistory || historyExhausted) return
if (!ctx.hasMoreMessages) {
setHistoryExhausted(true)
return
}
let cancelled = false
setIsHydratingHistory(true)
void ctx.loadOlderMessagesPreservingScroll()
.then((loaded) => {
if (cancelled) return
setIsHydratingHistory(false)
if (!loaded) setHistoryExhausted(true)
})
.catch(() => {
if (cancelled) return
setIsHydratingHistory(false)
setHistoryExhausted(true)
})
return () => {
cancelled = true
}
}, [
open,
props.block,
ctx.hasMoreMessages,
ctx.loadOlderMessagesPreservingScroll,
historyExhausted,
])There was a problem hiding this comment.
Fixed in e64ec84. Tool-group older-history chaining no longer depends on the shared isLoadingMoreMessages flag; ToolGroupCard now tracks hydration runs internally so in-flight loads are not cancelled by intermediate provider state flips, and it can continue loading multiple older pages until the boundary is complete or history is exhausted. Added regression coverage for multi-page hydration in web/src/components/ToolCard/ToolGroupCard.test.tsx.
Decouple ToolGroupCard older-history chaining from the shared loading flag, invalidate stale hydration runs safely, and add regression coverage for multi-page hydration.
…uping # Conflicts: # web/src/components/ToolCard/ToolCard.tsx
Summary
Reason
Scope
Test
cd web && bun run test -- src/chat/toolGroups.test.ts src/components/ToolCard/ToolGroupCard.test.tsx src/components/AssistantChat/HappyThread.test.tsxcd web && bun run typecheckRisk / Rollback
Links
Reviewer Notes
web/src/chat/toolGroups.tsToolGroupCardandHappyThread