Skip to content

fix(table): no typewriter flash; Run-row skips completed workflows#4685

Closed
TheodoreSpeaks wants to merge 2 commits into
stagingfrom
fix/table-typewriter-runrow
Closed

fix(table): no typewriter flash; Run-row skips completed workflows#4685
TheodoreSpeaks wants to merge 2 commits into
stagingfrom
fix/table-typewriter-runrow

Conversation

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator

Summary

  • Typewriter flicker: a cell going from running → value briefly flashed the full text for one frame before the animation started. The reset to empty happened in an effect (one frame late) while the render fell back to the full value (revealed ?? kind.text). Now the reset happens synchronously during render when the value changes, and we only animate on actual changes (not first mount) — no flash.
  • Run row re-running completed workflows: the gutter "Run row" (and manual "Run incomplete") only skipped completed and fully-filled groups, so a workflow that completed with any blank output column was treated as incomplete and re-ran. Now manual incomplete runs treat a completed group as done regardless of blank outputs — only "Run all" re-runs completed cells. The auto cascade still re-fills blanks (completedAndFilled). Client optimistic stamp mirrors the rule.

Type of Change

  • Bug fix

Testing

Tested manually on staging; tsc, vitest (lib/table + hooks/queries, 202 passing), and lint all 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)

- Typewriter: reset the revealed text synchronously during render when the
  value changes (not in an effect), so a cell going from running→value no
  longer flashes the full text for one frame before animating.
- Run row / manual incomplete runs now treat a `completed` group as done even
  if an output column is blank — only "Run all" re-runs completed cells. The
  auto cascade keeps re-filling blank outputs (completedAndFilled). Client
  optimistic stamp mirrors: incomplete skips `completed` cells.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 20, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 21, 2026 12:07am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 20, 2026

PR Summary

Medium Risk
Adjusts table run eligibility and server-side bulk-clear behavior for mode: 'incomplete', which affects which workflow executions/data get wiped and re-run; mistakes could skip needed reruns or clear too much state. UI typewriter change is low risk but touches render-time state updates.

Overview
Fixes a table cell UI flicker by changing useTypewriter to reset its revealed text synchronously when text changes, so the full new value never flashes for a frame before the typewriter animation starts.

Changes manual run behavior for mode: 'incomplete' to not re-run workflow groups that are already completed, even if some outputs are blank (only “Run all” re-runs completed). This rule is applied consistently across server eligibility (classifyEligibility), client optimistic stamping in useRunColumn, and the dispatcher’s bulkClearWorkflowGroupCells, which now clears outputs/executions per-group only for re-runnable terminal states (error/cancelled) instead of doing a row-level wipe.

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

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 21, 2026

Greptile Summary

This PR fixes two independent bugs in the table workflow feature: a one-frame typewriter flash when a cell transitions from running to a new value, and "Run row / Run incomplete" incorrectly re-running workflows that had already completed but left an output column blank.

  • Typewriter fix (cell-render.tsx): Replaces the effect-based setRevealed('') reset (one frame late) with a synchronous state update during render using the React derived-state pattern. mountedRef prevents animation on first mount/remount, and animateRef bridges the sync render path to the async rAF effect without reintroducing the flash.
  • Eligibility fix (workflow-columns.ts): For manual mode: 'incomplete' runs, the server now skips any group whose exec.status === 'completed' regardless of whether outputs are blank. The auto-cascade path is unchanged and still uses completedAndFilled. The client optimistic stamp in tables.ts is updated to match.

Confidence Score: 4/5

Both fixes are self-contained and well-reasoned; the client and server eligibility rules are kept in sync, and the typewriter pattern is a documented React escape hatch used correctly.

The logic changes are consistent between server and client. The typewriter hook uses React's render-time setState escape hatch correctly — prevTextRef makes the check idempotent across re-renders, and animateRef cleanly separates the synchronous render path from the async rAF effect. The eligibility predicate change is intentional and the auto-cascade path (isManualRun=false) is unaffected. The only gap is a stale JSDoc on classifyEligibility that now contradicts the new manual-incomplete behaviour.

The JSDoc block at the top of classifyEligibility in workflow-columns.ts still describes the old 'completed + blank output = stale for manual runs' rule and should be updated.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.tsx Typewriter flash fixed by moving state reset to render-time (React derived-state pattern). Logic is sound: prevTextRef guards idempotency, mountedRef skips first-mount animation, animateRef bridges the synchronous render path to the async effect without re-adding the flash.
apps/sim/lib/table/workflow-columns.ts Eligibility predicate updated so manual mode: 'incomplete' skips any group with status === 'completed' regardless of blank outputs. Auto-cascade path (isManualRun=false) is unchanged and still uses completedAndFilled. JSDoc on classifyEligibility now contradicts the new behaviour.
apps/sim/hooks/queries/tables.ts Client-side optimistic stamp mirrors server change: drops areOutputsFilled guard and instead skips completed cells by exec?.status === 'completed', keeping the UI consistent with the new server eligibility rule.

Comments Outside Diff (1)

  1. apps/sim/lib/table/workflow-columns.ts, line 37-46 (link)

    P2 Stale JSDoc after eligibility change

    The docstring still says "Completed" status is treated as stale when any output cell is empty … for manual incomplete-mode runs, but this PR changes exactly that: manual incomplete runs now treat any completed exec as done regardless of blank outputs. A future reader tracing the eligibility logic from this comment will get the wrong picture.

Reviews (1): Last reviewed commit: "fix(table): no typewriter flash; Run-row..." | Re-trigger Greptile

bulkClearWorkflowGroupCells in incomplete mode wiped EVERY targeted group's
output data + exec on any row that wasn't fully filled across all targeted
groups. So Run-row on a row with one completed group and one cancelled group
wiped the completed group's outputs + exec too, and the dispatcher re-ran it.

Now incomplete-mode clears per-group: only error/cancelled groups get their
output columns + exec cleared; completed and in-flight groups are left intact
(never-run groups have nothing to clear and run via eligibility). Combined
with the classifyEligibility guard, a completed workflow is never re-run by
Run-row — only Run-all re-runs it.
@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator Author

Combined into #4687 (its branch already contained these commits; retargeted #4687 to staging).

@waleedlatif1 waleedlatif1 deleted the fix/table-typewriter-runrow branch May 21, 2026 06:45
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