Skip to content

fix(tables): count dispatcher pre-stamps in "X running" during active dispatch#4850

Merged
TheodoreSpeaks merged 1 commit into
stagingfrom
fix/table-x-running-flicker
Jun 2, 2026
Merged

fix(tables): count dispatcher pre-stamps in "X running" during active dispatch#4850
TheodoreSpeaks merged 1 commit into
stagingfrom
fix/table-x-running-flicker

Conversation

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator

Summary

  • Follow-up to fix(tables): reliable stop-all, accurate "X running", and rate/usage gating for cell runs #4838. The "X running" badge flickered 20→0 during a run: the live SSE path counts the dispatcher's pending pre-stamps (via isExecInFlight), but the server refetch (countRunningCells) excluded pending + null-executionId cells, so each per-window dispatch SSE → refetch reset the badge to 0.
  • The orphan-exclusion is correct only when no dispatch is active (abandoned pre-stamps). During an active dispatch those pre-stamps are real queued work the dispatcher is about to run. countActiveRunCells now includes them in byRowId, matching the SSE path; the no-dispatch fallback still excludes orphans.

Type of Change

  • Bug fix

Testing

  • Manually: large table run no longer flickers 20→0; badge holds the in-flight count.
  • tsc --noEmit (0 errors) and biome pass.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 2, 2026 7:10pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented Jun 2, 2026

PR Summary

Low Risk
Narrow change to run-state counting logic for the UI badge; no auth, data model, or execution semantics changes beyond aligning counts with existing SSE behavior.

Overview
Fixes the "X running" badge flickering during large table runs when the client refetches run state on each dispatch SSE.

countRunningCells gains an optional includeUnclaimedPreStamps flag so pending rows with no executionId (dispatcher pre-stamps) can be counted or excluded. Orphan pre-stamps should still be excluded when no dispatch is active; during an active dispatch they represent real queued work.

countActiveRunCells now loads the sidecar byRowId via countRunningCells(..., { includeUnclaimedPreStamps: true }) while a dispatch is running, aligning server refetch counts with the live SSE path (isExecInFlight / pending). The no-dispatch path is unchanged and still drops abandoned pre-stamps so the badge does not stick above zero.

Reviewed by Cursor Bugbot for commit e56960e. Bugbot is set up for automated code reviews on this repo. Configure here.

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator Author

@greptile review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 2, 2026

Greptile Summary

This PR fixes a "X running" badge flicker (20→0) during active table dispatches by making countRunningCells aware of unclaimed pre-stamps (pending + null executionId). A new opts.includeUnclaimedPreStamps flag controls whether those pre-stamps are included (active dispatch) or excluded (orphan fallback), aligning the server-refetch path with the live SSE path.

  • countRunningCells gains an opts.includeUnclaimedPreStamps flag; when false (default), the existing orphan-exclusion filter is applied; when true, undefined is passed to drizzle-orm's and() — which safely ignores it — so all pending rows count.
  • countActiveRunCells now calls countRunningCells with includeUnclaimedPreStamps: true when at least one dispatch is active, so byRowId matches the SSE in-flight count; the no-dispatch fallback is unchanged.

Confidence Score: 5/5

The 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

Filename Overview
apps/sim/lib/table/dispatcher.ts Adds includeUnclaimedPreStamps opt to countRunningCells and threads it through countActiveRunCells; logic is correct, default behaviour unchanged, drizzle and(…, undefined) is safe.

Reviews (3): Last reviewed commit: "fix(tables): count dispatcher pre-stamps..." | Re-trigger Greptile

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 2, 2026

Greptile Summary

Fixes a "20→0" badge flicker that occurred during large table runs: the live SSE path counted dispatcher pre-stamps as in-flight (via isExecInFlight), but the server refetch (countRunningCells) excluded pending rows with a null executionId, so each per-window SSE event followed by a refetch briefly zeroed the badge. The fix gates orphan exclusion behind an includeUnclaimedPreStamps flag and passes that flag from countActiveRunCells when a dispatch is active.

  • countRunningCells gains an opts.includeUnclaimedPreStamps flag; when set, the orphan-exclusion WHERE predicate is omitted (Drizzle's and() silently drops undefined conditions), keeping all pending rows in scope.
  • countActiveRunCells passes { includeUnclaimedPreStamps: true } so the per-row byRowId map matches the SSE delta path; the no-dispatch fallback path (active.length === 0) is unchanged and still excludes orphans.

Confidence Score: 4/5

Safe 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

Filename Overview
apps/sim/lib/table/dispatcher.ts Adds includeUnclaimedPreStamps opt to countRunningCells and passes it from countActiveRunCells during active dispatch; logic is sound but there is a narrow window after a dispatch completes where pre-stamps may be momentarily over-counted.

Sequence Diagram

sequenceDiagram
    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
Loading

Comments Outside Diff (1)

  1. apps/sim/lib/table/dispatcher.ts, line 280-285 (link)

    P2 Stale-active-dispatch race can briefly over-count pre-stamps

    listActiveDispatches and countRunningCells run in two separate calls even though they're fired via Promise.all. If a dispatch transitions to complete after listActiveDispatches returns (but before countRunningCells executes), active.length is still > 0, so countRunningCells is called with includeUnclaimedPreStamps: true — causing the just-completed dispatch's pre-stamps (now orphans) to be counted as queued work. The badge would show a non-zero value and self-correct only on the next refetch cycle. The window is narrow and the impact is a momentary over-count rather than under-count, so the user experience is safer than the previous flicker, but it is worth noting as a corner case.

Reviews (2): Last reviewed commit: "fix(tables): count dispatcher pre-stamps..." | Re-trigger Greptile

… 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).
@TheodoreSpeaks TheodoreSpeaks force-pushed the fix/table-x-running-flicker branch from 7a227af to e56960e Compare June 2, 2026 19:10
@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator Author

@greptile review

@TheodoreSpeaks TheodoreSpeaks merged commit 34ee7f9 into staging Jun 2, 2026
14 checks passed
@TheodoreSpeaks TheodoreSpeaks deleted the fix/table-x-running-flicker branch June 2, 2026 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant