fix(tables): count dispatcher pre-stamps in "X running" during active dispatch#4850
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview
Reviewed by Cursor Bugbot for commit e56960e. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
@greptile review |
Greptile SummaryThis PR fixes a "X running" badge flicker (20→0) during active table dispatches by making
Confidence Score: 5/5The change is narrow and well-scoped with correct default behaviour preserved and no altered write paths. The fix correctly threads the includeUnclaimedPreStamps flag through both call sites, the no-dispatch fallback is untouched, and drizzle-orm's and(…, undefined) safely drops the clause. No files require special attention beyond the single changed file. Important Files Changed
Reviews (3): Last reviewed commit: "fix(tables): count dispatcher pre-stamps..." | Re-trigger Greptile |
Greptile SummaryFixes a "20→0" badge flicker that occurred during large table runs: the live SSE path counted dispatcher pre-stamps as in-flight (via
Confidence Score: 4/5Safe to merge; the fix correctly aligns the server refetch with the SSE live path and is well-guarded by the no-dispatch fallback. The core logic is correct: Drizzle's and() safely drops undefined conditions, so the SQL produced when includeUnclaimedPreStamps is true omits the orphan-exclusion predicate as intended. The no-dispatch fallback path is unchanged. The one edge case is a narrow timing window where a dispatch completes between listActiveDispatches and countRunningCells, leading to a momentary over-count of pre-stamps that self-corrects on the next refetch. apps/sim/lib/table/dispatcher.ts — specifically the countActiveRunCells two-phase DB read (dispatch list then sidecar count) which can diverge across a dispatch-completion boundary. Important Files Changed
Sequence DiagramsequenceDiagram
participant C as Client
participant SSE as SSE Stream
participant D as Dispatcher
participant DB as DB (countRunningCells)
D->>DB: stampQueuedForBatch() writes pending + null executionId
D->>SSE: dispatch event status dispatching
SSE->>C: "delta badge = 20 isExecInFlight counts pending"
C->>DB: countActiveRunCells(tableId)
Note over DB: listActiveDispatches active > 0
Note over DB: countRunningCells includeUnclaimedPreStamps true
DB->>C: byRowId includes pre-stamps badge stays 20
|
… dispatch The live SSE path counts pending pre-stamps (isExecInFlight) but countRunningCells excluded them, so each per-window refetch reset the badge from ~20 to 0 (visible flicker now that the control stays shown via hasActiveDispatch). Include unclaimed pre-stamps in byRowId when a dispatch is active; keep excluding them only in the no-dispatch fallback (orphan case).
7a227af to
e56960e
Compare
|
@greptile review |
Summary
pendingpre-stamps (viaisExecInFlight), but the server refetch (countRunningCells) excludedpending+ null-executionId cells, so each per-windowdispatchSSE → refetch reset the badge to 0.countActiveRunCellsnow includes them inbyRowId, matching the SSE path; the no-dispatch fallback still excludes orphans.Type of Change
Testing
tsc --noEmit(0 errors) and biome pass.Checklist