fix: persist previousRunId across page reloads for v1 threads #2288
fix: persist previousRunId across page reloads for v1 threads #2288MichaelMilstead merged 5 commits intomainfrom
Conversation
There was a problem hiding this comment.
The fix is directionally correct, but there are a couple of robustness and maintainability concerns: handleRunFinished ignores RunFinishedEvent.runId, which can drop lastCompletedRunId in edge cases, and ThreadSyncManager’s sync gating can cause repeated metadata fetches/dispatches when message syncing doesn’t complete. Also, overloading LOAD_THREAD_MESSAGES to carry metadata increases coupling and regression risk; a dedicated action would be clearer.
Additional notes (2)
- Maintainability |
react-sdk/src/v1/utils/event-accumulator.ts:673-680
handleRunFinishedpersistslastCompletedRunIdfromthreadState.streaming.runIdand ignores theRunFinishedEventpayload entirely. If there are scenarios where streaming state losesrunId(or never had it) but the event contains it, this misses a reliable source of truth.
Given the goal is robustness across edge cases, it would be safer to incorporate the event’s runId when present rather than relying solely on streaming state.
- Maintainability |
react-sdk/src/v1/hooks/use-tambo-v1-send-message.test.tsx:1046-1125
This new test sets upstateWithLastRunwithlastCompletedRunId: "run_1"and asserts it’s used aspreviousRunId. That’s good, but it doesn’t cover the crucial post-reload path this PR claims to fix: metadata fetch populateslastCompletedRunIdand thenuseTamboV1SendMessageuses it.
Right now you only validate the hook’s fallback behavior, not the end-to-end integration between ThreadSyncManager -> reducer -> hook.
Summary of changes
What changed
This PR fixes post-reload message sending for v1 threads by persisting and rehydrating the run identifier required by the server as previousRunId.
Key updates
- Persist last run in stream state:
- Added
lastCompletedRunId?: stringtoThreadState. RUN_FINISHEDnow storesthreadState.streaming.runIdintolastCompletedRunId(with fallback to existing).
- Added
- Rehydrate after reload:
ThreadSyncManagernow fetches both:client.threads.messages.list()(messages)client.threads.retrieve()(metadata)
- Metadata load dispatches
LOAD_THREAD_MESSAGESwithlastCompletedRunIdto update state without blocking message sync.
- Use persisted run ID on send:
useTamboV1SendMessagenow setspreviousRunIdfromthreadState.streaming.runId ?? threadState.lastCompletedRunId.
- Tests updated/added:
- New hook test asserts
lastCompletedRunIdis used whenstreaming.runIdis absent. - Reducer tests + snapshot updated to cover
lastCompletedRunIdbehavior.
- New hook test asserts
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
alecf
left a comment
There was a problem hiding this comment.
one minor suggested change - I think this will be a little more reliable/current
I do worry a little about accidentally resetting the latestRunId if this request comes back late or something, but I don't have another better approach right now
Problem: After a page reload, switching to an existing thread and sending a message failed with BadRequestError: 400 previousRunId is required. The streaming.runId was only set during active streaming and lost on reload. The server requires
previousRunId when a thread already has messages/runs.
fixes: TAM-1152