Skip to content

fix(tables): stop insert-row flicker and return order_key from rows list#4918

Merged
TheodoreSpeaks merged 2 commits into
stagingfrom
fix/insert-row-flicker
Jun 9, 2026
Merged

fix(tables): stop insert-row flicker and return order_key from rows list#4918
TheodoreSpeaks merged 2 commits into
stagingfrom
fix/insert-row-flicker

Conversation

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator

Summary

  • Stop the rows insert flicker: useCreateTableRow.onSettled invalidated tableKeys.detail(tableId), which prefix-matches the nested rowsRoot rows query and forced an un-cancelled refetch on every insert — a late offset refetch could resolve after the optimistic splice and clobber freshly-inserted rows. Now invalidate detail with exact: true (+ lists()) so the count surfaces refresh without cascading into the rows query.
  • Compare order keys bytewise in reconcileCreatedRow (was localeCompare) to match the server's COLLATE "C" ordering and the existing fitsAfter checks.
  • Include order_key in the GET /rows list response — it was dropped in the route's row mapping (the POST/batch handlers already returned it), so the client never saw keys and reconcileCreatedRow always fell back to the position path. Surfacing it activates the key-ordered path and makes insert-above/below place correctly with no refetch.

Type of Change

  • Bug fix

Testing

Tested manually via Chrome DevTools against a live table:

  • 5 rapid inserts → 0 rows refetches (was 1 per insert), row count grew monotonically with no dips — no flicker.
  • Insert-above on a keyed table → new row got a key between its neighbors and landed directly above the anchor, POST only (no GET refetch).
  • bun run lint clean; bun run check:api-validation:strict passes.

Note: this is the client half of fractional ordering. It's correct in the fully-converged state (migration 0228 applied → backfill complete → flag on). Order the rollout so order_keys exist before the flag flips; in flag-off / null-keyed tables the position path still matches the server.

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)

The rows insert flicker came from useCreateTableRow.onSettled invalidating
tableKeys.detail(tableId), which prefix-matches the nested rowsRoot rows query
and forces an un-cancelled refetch on every insert. A late offset refetch could
resolve after the optimistic splice and clobber freshly-inserted rows.

- invalidate detail with exact:true (+ lists) so the count surfaces refresh
  without cascading into the rows query
- compare order keys bytewise in reconcileCreatedRow to match the server's
  COLLATE "C" ordering and fitsAfter (was localeCompare)
- include order_key in the GET /rows list response; it was dropped in the
  route mapping, so the client never saw keys and reconcileCreatedRow always
  fell back to the position path
@vercel

vercel Bot commented Jun 9, 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 Jun 9, 2026 2:58am

Request Review

@cursor

cursor Bot commented Jun 9, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes client cache invalidation and optimistic row reconciliation for inserts; wrong predicates could leave filtered views stale or reintroduce flicker, but scope is limited to table row queries.

Overview
Fixes insert-row flicker and improves fractional row ordering on the client.

GET /rows now includes orderKey in each row (POST/batch already did), so the client can use key-based optimistic placement instead of always falling back to position.

useCreateTableRow stops invalidating the paginated rows cache on settle: new invalidateRowCountSurfaces refreshes table detail (exact: true) and the tables list only, avoiding prefix invalidation that refetched rowsRoot and let late offset responses overwrite optimistically spliced rows. reconcileCreatedRow runs only on default-order (unfiltered, unsorted) queries via isDefaultOrderRowsQuery; filtered/sorted caches are invalidated on success instead. Order keys are compared bytewise (not localeCompare) to match server COLLATE "C" ordering.

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

@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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9c011f6. Configure here.

Comment thread apps/sim/hooks/queries/tables.ts
@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes three interconnected issues causing insert-row flicker and incorrect key-ordered placement in user tables. Together, the fixes complete the client-side half of fractional ordering by making reconcileCreatedRow the authoritative cache source for row inserts.

  • Flicker fix: useCreateTableRow.onSettled now calls invalidateRowCountSurfaces instead of invalidateRowCount, using exact: true on the detail key to prevent prefix-cascade into the nested rowsRoot rows query — eliminating the race between optimistic splices and late-resolving offset refetches.
  • Bytewise sort: reconcileCreatedRow's sortRows replaces localeCompare with JS string </> operators to match the server's COLLATE \"C\" ordering and the pre-existing fitsAfter >= checks.
  • Missing order_key in GET: The GET /rows response mapping now includes orderKey: r.orderKey ?? undefined, matching what the POST/batch handlers already returned and activating the key-ordered insert path on the client.

Confidence Score: 5/5

Safe to merge; all three changes are narrowly scoped bug fixes with no new code paths introduced for un-keyed or mixed-key tables.

The invalidation fix is conservative (adds exact: true — a narrower constraint) and well-bounded by the key hierarchy. The bytewise sort change aligns the client with the server collation and the pre-existing fitsAfter comparisons, so no inconsistency is introduced. The GET orderKey addition is a one-line mapping omission fix whose absence was the root cause of the client always falling back to the position path.

No files require special attention; the changes are straightforward and internally consistent.

Important Files Changed

Filename Overview
apps/sim/hooks/queries/tables.ts Adds invalidateRowCountSurfaces (exact-only detail invalidation) to replace the broad invalidateRowCount in onSettled, and fixes sortRows to use bytewise string comparison; logic is consistent with fitsAfter's existing >= comparisons.
apps/sim/app/api/table/[tableId]/rows/route.ts Adds orderKey: r.orderKey ?? undefined to the GET response row mapping, aligning it with what POST and batch handlers already returned; the ?? undefined correctly drops null keys from JSON serialization.

Sequence Diagram

sequenceDiagram
    participant UI
    participant useMutation as useCreateTableRow
    participant QueryCache as React Query Cache
    participant Server

    UI->>useMutation: mutate(rowData)
    useMutation->>Server: POST /api/table/[id]/rows
    Server-->>useMutation: "{ row: { id, data, position, orderKey, ... } }"

    Note over useMutation: onSuccess
    useMutation->>QueryCache: "reconcileCreatedRow()<br/>(bytewise sort by orderKey<br/>OR position fallback)"
    QueryCache-->>UI: "optimistic row appears<br/>(no refetch, no flicker)"

    Note over useMutation: onSettled (success OR error)
    useMutation->>QueryCache: "invalidateRowCountSurfaces()<br/>exact:true on detail(tableId)<br/>+ lists()"

    Note over QueryCache: rows query NOT invalidated<br/>(exact:true prevents cascade<br/>into rowsRoot prefix)

    QueryCache->>Server: refetch detail(tableId) only
    Server-->>QueryCache: updated rowCount
    QueryCache-->>UI: count badge updated
Loading

Reviews (1): Last reviewed commit: "fix(tables): stop insert-row flicker and..." | Re-trigger Greptile

reconcileCreatedRow patched every rows query under rowsRoot, but its
orderKey/position heuristic only matches the unfiltered, unsorted server order.
Under a filter or column sort the splice could show wrong rows, wrong order, or
an inflated totalCount, and without the prior rowsRoot refetch it persisted
until the query went stale.

- patch only the default-order rows queries (filter and sort both absent)
- refetch the filtered/sorted variants on insert; active ones update now,
  inactive ones on next view. The default view stays optimistic (no flicker)
- the find/write subtrees are excluded from the splice (different shape)
@TheodoreSpeaks TheodoreSpeaks merged commit efeacb9 into staging Jun 9, 2026
14 checks passed
@TheodoreSpeaks TheodoreSpeaks deleted the fix/insert-row-flicker branch June 9, 2026 05:12
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