Skip to content

fix(tables): re-check the find match at the reveal, not just before paging - #6750

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/table-find-reveal-recheck
Aug 16, 2026
Merged

fix(tables): re-check the find match at the reveal, not just before paging#6750
waleedlatif1 merged 2 commits into
stagingfrom
fix/table-find-reveal-recheck

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #6733, which merged while the last review round was still in flight — this is the one fix from that round that didn't make it in.

#6733 added a guard in goToMatch so a match removed while its row was paging isn't revealed. Greptile's next round pointed out one more window: between goToMatch queueing the reveal and the reveal effect running is another commit, and a refetch there can remove the match after the guard has already passed.

  • Extracts the membership check into isStillAMatch(match) and applies it at the reveal too — the one place the selection actually moves.
  • When the match is gone, the pending jump is dropped and the cursor is released (activeMatchRef nulled, cursorIsOnMatchRef false), so the next step lands on the clamped replacement instead of skipping it.

Without this, a row write or SSE update landing in that window selects a cell that no longer matches and marks the cursor as sitting on a result.

Type of Change

  • Bug fix

Testing

  • Typecheck, bun run lint:check, bun run check:audits (27 audits), 1313 table tests — all pass on top of current staging.
  • The async race itself isn't unit-covered: the existing tests exercise the find bar, and driving this needs the 4,600-line grid mounted with a mid-flight refetch. The invariant is now enforced at both seams that can observe it (post-await in goToMatch, and the reveal effect).

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

vercel Bot commented Aug 16, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 16, 2026 12:18am

Request Review

@cursor

cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Localized bug fix in table find navigation with no auth, data-model, or API changes; risk is limited to find next/prev behavior under concurrent refetches.

Overview
Follow-up to find-navigation guards: #6733 already dropped stale targets after row paging, but a find refetch could still remove a match after that check while a pending reveal was waiting for its row—so the grid could select a cell that no longer matches and treat the cursor as on a hit, making the next step skip the replacement match.

This PR centralizes membership in isStillAMatch and runs it again in the reveal effect (the last step before selection moves). If the match is gone, the pending jump is dropped; activeMatchRef and cursorIsOnMatchRef are cleared only when this reveal still owns the active match, so a newer jump started while the row was loading is not stranded.

Reviewed by Cursor Bugbot for commit d91a700. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This follow-up closes the remaining race between queueing and applying a table-find reveal.

  • Extracts a shared live-result membership check.
  • Revalidates the pending match immediately before moving the selection.
  • Drops stale reveal work while preserving cursor ownership claimed by a newer jump.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains in the previously reported table-find reveal race.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx Adds the final stale-match gate to the deferred reveal effect and conditionally releases cursor state only when that reveal still owns it.

Reviews (2): Last reviewed commit: "fix(tables): only release the find curso..." | Re-trigger Greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d91a700. Configure here.

@waleedlatif1
waleedlatif1 merged commit 0077f6f into staging Aug 16, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/table-find-reveal-recheck branch August 16, 2026 00:50
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