Skip to content

fix(browse): refresh the table once an import's job completes - #1938

Merged
dawsontoth merged 2 commits into
stagefrom
fix/import-refreshes-browse
Oct 13, 2026
Merged

dawsontoth merged 2 commits into
stagefrom
fix/import-refreshes-browse

Conversation

@dawsontoth

@dawsontoth dawsontoth commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Problem

A CSV import into the open table follows its job, but when the job succeeds and the dialog closes, the grid, its count and the database overview still show the rows from before the import until a manual Refresh (found by Dawson hand-testing the release candidate on stage).

Every Browse read has a 60s staleTime, and the import only refetched describe_all and navigated to the table already on screen, which fetches nothing. #1880 did not cause this. #1471 (125b6e57d) did: when it moved the import modal from DatabaseTableView into DatabaseActionModals, it dropped the table view's refreshTable() call.

Solution

useImportDataMutation refreshes from its own onSettled, so the refresh still happens after the modal unmounts (React Query skips a mutate call's own callbacks then). It invalidates [entityId, database, table] (page, filtered search, exact count, describe_table), the open record's search_by_id key, and describe_all.

The refresh is not awaited, so the import settles and the modal closes at the job's verdict, as before. onImported still waits for describe_all before navigating, or resolveDatabasesRedirect would send a table the import just created back to the overview. It now joins the mutation's refetch (awaitDatabaseMapRefresh, cancelRefetch: false) instead of starting a second one.

Every outcome refetches, failures included, because any of them can have written rows. The strongest case is a submission with no verdict (no response, or a 502/503/504): its toast says "Outcome unknown, check whether it took effect", and a grid left on cached rows would invite a retry that duplicates every record without an id. When the instance is really down, the re-reads add their own error toasts; that is the accepted cost.

Every outcome that can write, and why a failed re-read is safe
  • A job that completes with skipped rows (successfully loaded 3 of 5 records, or a JSON insert's (N skipped)).
  • A job that ended in ERROR: csv_url_load inserts in 2 MB chunks (harper dataLayer/bulkLoad.ts, insertChunk), so the chunks before the failing one stay written.
  • A wait that timed out on a load still running.
  • A load refused after the import's own create_table made the table.
  • A submission with no verdict, for example a central-manager 504 on a long insert that Harper went on to apply.

invalidateQueries swallows fetch failures unless throwOnError is set (query-core 5.104.1 src/queryClient.ts:520-521), so a failed re-read is not an unhandled rejection. A throw from the refresh itself is caught, so it cannot turn a completed import into a failure.

No other job follower has this gap. waitForJob is used only by the import, and followRestartJob's callers await it through mutateAsync and then invalidate.

The databases DESIGN.md records where the refresh lives and why.

Verification

Test cases, mutants and review history

Tests:

  • a completed job with skipped rows refetches the page, describe_table and describe_all after the modal unmounted. It marks the filtered search, exact count and open record stale, and leaves another table alone.
  • each failure above refetches too, plus polls that stopped answering;
  • re-reads that fail end in error state without an unhandled rejection;
  • the mutation settles while describe_all is in flight, and the map wait joins that refetch;
  • a throwing refresh leaves the import successful;
  • onImported navigates only once describe_all is back.

The 11 mutants:

  • no refresh;
  • success only;
  • no refresh without a response;
  • each of the three invalidations dropped;
  • no guard;
  • cancelling the in-flight refetch;
  • awaiting the refresh;
  • onImported not waiting;
  • throwOnError: true, where vitest fails the run with an unhandled AxiosError: Network Error.

Cross-model review, five rounds (codex, gemini, Cursor, Harper domain adjudication). The closing full round ran on e59d02f37, and the delta round on 3d8f58efc converged.

Changed from review:

  • the refresh is not awaited;
  • a refusal after create_table refetches;
  • a no-verdict failure refetches;
  • search_by_id is invalidated;
  • the DESIGN note was added.

Declined:

  • .ts import extensions: none of the 1,369 relative imports in src has one, and tsconfig doesn't allow them.
  • the async settleQuietly and join-order concerns: progress.onSuccess is a synchronous toast, and onImported runs after onSettled.
  • two test comments: both state a reason.

Pre-existing, worth a separate issue:

  • waitForJob giving up on a running load reports a plain failure, not "Outcome unknown" (getJob.ts:67-68, :89-90).
  • a failed describe_all refetch still lets onImported navigate against the old map.

Related PRs: #1864 overlaps (already in stage; same DatabaseActionModals files, but the Add Records path, not the import refresh), 4 others independent

🤖 Generated with Claude Code

Review-Coverage: authored=claude; ran=cursor-composer,gemini,codex,cursor-muse; adjudicated=domain; declined=cursor-grok,cursor-kimi,muse; rounds=5; full=2 @ 3d8f58e

Review-Attention: read ~3m (decisions: refetch-after-any-outcome, describe-all-refetch-all-on-failure, join-relies-on-callback-order, do-less-alternative) @ 3d8f58e

An import into a table that is already open left the grid, its count
and the database overview on the rows from before the import, until a
manual Refresh. The import only refetched describe_all and navigated to
the table. Navigating to the table already on screen fetches nothing,
because every Browse read has a 60s staleTime. This regressed in #1471,
which moved the import modal into DatabaseActionModals and dropped the
table view's own refreshTable() call for the same-table case. #1880's
job-following toast did not change it.

useImportDataMutation now refreshes from the mutation's own onSettled,
so the refresh does not depend on the modal still being mounted. It
invalidates the table's queries ([entityId, database, table]: the page,
a filtered search, the exact count, describe_table) and describe_all.
The refresh is not awaited, so the import still settles, and the modal
still closes, at the job's verdict. onImported waits for describe_all
before it navigates, as it did before, but now joins the mutation's
refetch (cancelRefetch: false) instead of starting another.

Every outcome invalidates, because any of them can have written: a job
that skipped rows, a job that ended in ERROR (csv_url_load inserts chunk
by chunk, so earlier chunks stay written), a wait that timed out on a
load still running, a load refused after the import's create_table. A
failure with no verdict (no response, or a 502/503/504) only marks the
reads stale instead of refetching them, since they would fail too and
add their own error toasts. The databases DESIGN.md records where the
refresh lives and why.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dawsontoth
dawsontoth marked this pull request as ready for review October 13, 2026 16:02
@dawsontoth
dawsontoth requested a review from a team as a code owner October 13, 2026 16:02
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 77.59% 13391 / 17258
🔵 Statements 77.54% 14215 / 18332
🔵 Functions 72.44% 3523 / 4863
🔵 Branches 72.53% 10514 / 14495
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/features/instance/databases/components/DatabaseActionModals.tsx 80.85% 45.61% 53.84% 80.85% 103-104, 108-109, 118-147
src/integrations/api/instance/database/importData.ts 78.94% 34.78% 92.3% 78.78% 41-44, 74-75, 106
Generated in workflow #2336 for commit 3d8f58e by the Vitest Coverage Report Action

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the database import flow to trigger Browse refreshes directly from the useImportDataMutation's onSettled callback rather than the modal, ensuring that data is refreshed even if the modal unmounts. It introduces logic to refetch queries when the instance responds, or mark them stale when there is no verdict (e.g., network errors or 502/503/504 statuses). Additionally, comprehensive tests and design documentation have been added. Feedback on the changes suggests adding .catch() handlers to the fire-and-forget queryClient.invalidateQueries calls in refreshImportedTable to prevent unhandled promise rejections.

Comment thread src/integrations/api/instance/database/importData.ts Outdated
The import refresh only marked Browse stale, without refetching, when a
failure had no verdict (no response, or a 502/503/504). But that is the
case whose toast says "Outcome unknown" and asks the user to check
whether the import took effect. For example, central manager can answer
504 on a long insert that Harper goes on to apply. The open grid then
kept the cached rows from before the import, so the check found nothing
new, and a retry would insert every record without an id a second time.

Every outcome now refetches what is on screen. When the instance really
is down, the re-reads fail too and add their own error toasts, which is
cheaper than a grid that invites a duplicating retry. invalidateQueries
does not reject on a failed refetch unless throwOnError is set, so a
failed re-read is not an unhandled rejection; a test pins that.

The refresh also invalidates the open record's search_by_id key, which
the table prefix does not reach. Every import only inserts, so this only
matters for a cached "not found" for an id the import then wrote.
NO_VERDICT_STATUSES goes back to being private to unansweredWrite.ts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dawsontoth
dawsontoth merged commit 8adf303 into stage Oct 13, 2026
3 checks passed
@dawsontoth
dawsontoth deleted the fix/import-refreshes-browse branch October 13, 2026 17:13
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