feat: chat + claim verification AsyncStatus integration (MON-216) - #303
Conversation
Replaces chat/page.tsx's bouncing-dots typing indicator with MON-214's
AsyncStatus for both search and verify modes: contextual status text
("Recherche dans les votes et profils des deputes...", the existing
verify-mode text), a cancel affordance for in-flight requests, and a
retry affordance on failure that resubmits the original question/claim.
Adds AbortSignal support to apiPost/api.search/api.verify (optional,
backward compatible) so cancel can actually abort the fetch rather than
just hiding the UI. Follows the existing AbortError handling convention
already used by shareAnswer's navigator.share() call in this same file.
Scope correction: the issue described components/chat/Bubbles.tsx's
TypingIndicator as the target - that file turned out to be entirely
dead code (chat/page.tsx has its own inline bubble implementations,
Bubbles.tsx was never imported anywhere). Removed it along with the
now-unused local Dot/KeyframeStyle bouncing-dots implementation.
Token-level streaming remains out of scope per the issue - the RAG
backend doesn't stream today.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🧪 dbt data-health check — ✅ passedValidates prod data health. A failure means current prod data is stale/broken, not necessarily that this PR is wrong. |
Walid-peach
left a comment
There was a problem hiding this comment.
Attention Score
50/100 - NEEDS YOUR ATTENTION
- Base 100
- -10: 1 Should Fix finding (retry duplicates the failed user message)
- -10: 1 Should Fix finding (cancel leaves a dangling, unexplained user message)
- -10: Testing / Validation Gaps section is non-empty
- -15: could not fully verify the retry/cancel UX end-to-end via automated tests (the gaps below weren't caught by the new test suite)
Reason to take a closer look: both findings are real, user-visible UX defects in the two new interactive affordances this PR adds (retry and cancel), not just polish - worth confirming intent before merge even though nothing crashes or fails CI.
Summary
This PR replaces /chat's bouncing-dots typing indicator with MON-214's AsyncStatus for both search and claim-verification modes, adds real AbortController-backed cancellation, and adds a retry affordance on failure. The scope correction (components/chat/Bubbles.tsx was dead code, deleted along with the file) is well-reasoned and clearly documented. Test coverage is a genuine improvement - this page had zero tests before. Two UX gaps in the new retry/cancel flows are worth fixing before merge.
Must Fix
None.
Should Fix
- Retry duplicates the failed user message.
frontend/src/app/chat/page.tsx'sonRetrycallback (in themsg.role === 'error'branch, ~line 707) callssend(msg.retry.value)/submitClaim(msg.retry.value)directly. Both functions unconditionally dosetMessages(prev => [...prev, { role: 'user', text: q }, ...])- so after a failed attempt + retry, the transcript shows the same question/claim as two separate user bubbles (the original failed one, still in the array since only thetypingrole gets filtered on error - and a second one from the retry). A reviewer clicking retry twice on the same conversation would see three copies of the same message. Since every error message is always immediately preceded by its own user message inmessages(append-only, error/typing only ever follow their own user message), the retry callback can safely strip that pair first:setMessages(prev => prev.filter((_, idx) => idx !== i && idx !== i - 1))before callingsend/submitClaim(using theialready in scope from the.mapclosure). - Cancel leaves a dangling, unexplained user message. In both
sendandsubmitClaim'sAbortErrorcatch branch, only thetypingmessage is filtered out (setMessages(prev => prev.filter(m => m.role !== 'typing'))) - the user message that triggered the request stays in the transcript with no reply and no visible indication it was cancelled. Combined withinputValalready having been cleared at the top ofsend/submitClaimbefore the network call, the user's typed text is gone entirely with no way to recover it. Suggest removing both the trailing user and typing messages on cancel (setMessages(prev => prev.slice(0, -2))is safe here specifically because nothing else can be appended between them whileloadingblocks further submissions) and restoring the text into the input (setInputVal(q)/setInputVal(claim)) so cancelling reads as "return to editing," not "silently swallow what I typed."
Nice to Have
frontend/src/app/chat/page.tsx's error-branchAsyncStatuspassesstatus={msg.text}even thoughstatusis never rendered onceerroris set - harmless (satisfies the required prop) but a reader might wonder why the same string is passed twice; a one-line comment (already present just above, in a different context) or acceptingstatusas optional onAsyncStatuswhenerroris given would remove the redundancy. Not worth reopening MON-214's already-merged API for.- The
apiPost/api.search/api.verifysignature change (adding an optional trailingsignalparam) is backward compatible and low-risk, but worth a quick grep confirmation that no other caller passes a truthy third positional argument that could now be misinterpreted as a signal - a quick check shows none do, just flagging the class of risk for future callers ofapiPostwho might add a third argument without checking this PR's addition.
Testing / Validation Gaps
The two Should Fix findings above weren't caught by the new ChatPage.test.tsx suite: the retry test (shows a retry affordance on failure and resubmits the same question) only asserts mockSearch was called twice and the answer text appears - it doesn't assert on the number of user-bubble elements in the DOM, which is exactly where the duplication would show up. Similarly the cancel test only asserts the status role and an absent retry button, not that the dangling user message and cleared input are handled. Once the two Should Fix items are addressed, tests asserting the DOM message count (not just mock call counts) would close this gap and prevent regression.
Documentation / Reviewer Notes
The manual verification against the live production API (Playwright, screenshot of both modes' loading state) is a strong signal for a page with this much interactive/timing-sensitive behavior and no prior test coverage - same pattern used well in MON-215, good to see it continue here.
Verdict
Needs changes before merge
Retry resubmitted the failed question/claim without removing the original failed attempt's user bubble, so the transcript showed the same message twice. Now strips the error message's own (user, error) pair before resubmitting - safe because error/typing messages are always appended immediately after their own user message. Cancel only removed the typing indicator, leaving the user's message dangling with no reply and no restored input text. Now removes both the user and typing messages (the trailing pair - nothing else can be appended between them while `loading` blocks new submits) and restores the text into the input, so cancelling reads as "back to editing." Both fixes covered by strengthened assertions in ChatPage.test.tsx.
|
Applied both Should Fix findings from the review:
Verified live against the production API: cancelling now correctly returns to the empty state with the question restored in the input box and no orphaned sidebar conversation. Strengthened the two relevant tests in CI green again after the fix. |
What
Wires MON-214's
AsyncStatusprimitive into/chat's search and claim-verification flows, replacing the bouncing-dots typing indicator with contextual status text, a cancel affordance for in-flight requests, and a retry affordance on failure.Why
Chat search showed three bouncing dots with no context and no way to cancel or retry. Claim verification already showed static contextual text, but neither mode exposed
aria-busy, a cancel path, or a retry path - a failed request just left a plain red error string with no way to resubmit without retyping.Re-reading the actual code surfaced a scope correction: the issue named
components/chat/Bubbles.tsx'sTypingIndicatoras the integration target. That component (along withUserBubble,AssistantBubble,ErrorBubblein the same file) turned out to be entirely dead code -chat/page.tsxhas its own inline bubble implementations and never importedBubbles.tsx. Removed the whole file rather than leave misleading dead code that references the exact pattern this issue was meant to replace.Changes
frontend/src/app/chat/page.tsx:AsyncStatuswith contextual text for both modes (SEARCH_LOADING_TEXT, the existingVERIFY_LOADING_TEXT), wired to a newcancelActivecallback.AsyncStatusin its error state, with a retry affordance that resubmits the exact question/claim that failed (stored on the error message itself asretry: { kind, value }, so historical errors in a restored conversation retry correctly even ifmodehas since changed - omitted entirely for errors restored from pre-MON-216 stored conversations, so old data doesn't render a broken retry button).send/submitClaimnow create anAbortControllerper request, threaded throughapi.search/api.verify; both catch blocks special-caseAbortError(silently removes the typing message, no error message) - mirrors the existingAbortErrorhandling already used byshareAnswer'snavigator.share()call in this same file.Dot/KeyframeStylebouncing-dots implementation.frontend/src/lib/api.ts:apiPostandapi.search/api.verifyaccept an optionalAbortSignal(backward compatible - every other caller is unaffected).frontend/src/components/chat/Bubbles.tsx- deleted (dead code, see above); the now-emptycomponents/chat/directory is removed with it.Testing
frontend/__tests__/components/ChatPage.test.tsx(no prior test coverage existed for this page) - 5 tests: loading state showsAsyncStatuswitharia-busyand the right contextual text per mode, cancel aborts the request and clears the loading state with no error, a failed request shows retry and resubmits the same text, and a successful verification clears the loading state.npm test- full suite, 229/229 passing.npm run lint/npm run build- clean./chatquestion and claim-verification flows show the contextual status + cancel affordance while in flight (aria-busy="true"), then settle into a real answer/verdict. Screenshot shared with the requester.Review caught two UX gaps in the new retry/cancel flows, both fixed:
Risks / Notes
apiPost's signature change is additive/optional.rag/chain/rag_chain.py'sask()doesn't stream today, so this delivers contextual progress only, not streamed output.AsyncStatusitself (already tested in MON-214); not re-tested here.Breaking Changes
None.