Skip to content

feat(browse): guide an empty table to importing or seeding data - #1705

Merged
dawsontoth merged 4 commits into
stagefrom
claude/empty-table-state-f33fe5
Sep 12, 2026
Merged

feat(browse): guide an empty table to importing or seeding data#1705
dawsontoth merged 4 commits into
stagefrom
claude/empty-table-state-f33fe5

Conversation

@dawsontoth

@dawsontoth dawsontoth commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1703, from the conversation at the end of it: with Import Data moving into
the "..." menu, the empty table itself becomes the place to offer it — and, per that
thread, Import or Seed, because they are two different ideas.

So an empty table now gets two separate invitations rather than one button:

card opens the Import Data modal on
Import your data file (or url when that is the only source the role has)
Seed some data sample

with a quieter "Or write one yourself: Add New Record(s)" underneath.

Both still land in the one modal — ShowImportData just carries the method the card
asked for, and ImportDataModal prefers it over its own tableName ? 'file' : 'sample'
default. That is the entire change to the modal.

Only a table that can be filled is invited to fill itself

The interesting half of this PR is everything the empty state deliberately does not
say. Each of these was a way to tell the user something untrue:

  • A page past the end → "No records on this page". Checked first, because on page 3
    of a filtered set "nothing matched" is wrong too.
  • Filters matched nothing → "No records match these filters" + Clear Filters. The
    table may be full; the filters are the way out.
  • An empty page against a non-zero count → "No records came back", and a pointer at
    Refresh / Only If Cached. search_by_value answers a 404 with { data: [] }
    (getSearchByValue.ts), and Only If Cached — on by default — can produce one on a
    cache miss. Claiming "no records yet" there invites a duplicating import.
  • Seed with nothing to seed. methods.sample and methods.file share one path
    (IMPORT_METHOD_PATHS), so a bare insert grant makes both true. An insert-only
    role on a fresh primary-key-only table would get a Seed card whose dataset dropdown is
    empty and whose submit dead-ends on datasetId validation. Each card is now gated on
    the source it would actually use: bundled datasets need csv-data, random records need
    json-records plus columns to model values on.
  • Seed promising the wrong table. Picking a bundled dataset retargets the form to the
    dataset's own table (dog/book/animal), so the card says "load a ready-made sample
    dataset into a table of its own"; only the random-records path claims it fills the
    table on screen.
  • Copy outrunning the role. "Two ways to get some in" renders only when both cards
    do, and the Import card describes only the sources this role can reach — a URL-only
    role no longer reads file-upload copy.

Why the panel is outside the table

It renders as a sibling of the scroll container, not in a colSpan cell. The grid is
table-fixed and can be far wider than the viewport, so a centred message inside it sits
off past the last column, reachable only by scrolling right.

The empty panel appears on data === undefined, not on isFetching: neither list query
keeps the previous page's rows, so an empty array is always this result set's own settled
answer — and gating on isFetching swapped a settled empty panel back to the spinner cell
on every manual Refresh.

dark:text-violet-300 on the accents is not decoration: --primary is #403b8a, which
is invisible against the table's near-black in dark mode. It is the pairing ClusterHome
already uses.

Tests

  • EmptyResultSet.test.tsx — the two invitations are separate and call their own handler;
    each card is dropped when its source is closed; the copy tracks the role; and none of
    the three "not actually an empty table" cases offers to import.
  • DatabaseTableView.test.tsx — the wiring, not just the component: each card through to
    the ShowImportData launch it fires and the method it carries, the url fallback, and
    Seed withheld on a primary-key-only table with no csv-data grant.
  • TableView.test.tsx — the panel renders outside <table>; the plain "No results." cell
    survives when no empty state is passed; nothing claims a table is empty before rows
    arrive; and a settled empty panel stays put through a refetch.

Every guard was mutation-checked — delete it, confirm exactly one test goes red.

Verification

  • tsc -b, oxlint, dprint check — clean (exit codes captured directly, not through a pipe).
  • vitest run — 343 files, 3060 passed / 11 skipped.
  • Rendered against the real CSS cascade, light and dark, at every permission combination
    and all three withheld-invitation cases.

Review

Four rounds of cross-model review (codex + gemini + cursor-grok + Harper domain
adjudication, independent=true). Round 2 converged; rounds 2 and 3 were spent fixing
defects the review found in my own fixes:

round found now
1 Seed offered to a role with nothing to seed; Seed promising the wrong table; copy outrunning the role; refresh flash; untested wiring fixed in 852d11ed
2 unknown count read as "empty"; seed gate and modal reading different schema sources; unmemoized scan; initialMethod untested fixed in 6bfafab3
3 the unknown-count heading still asserting the table was empty fixed in 436494b1

Writing round 2's initialMethod test found a real gap the same way: nothing had
exercised the modal consuming it, so a dropped ?? would have shipped silently.

Two findings were left alone, deliberately:

  • Delete-to-empty from page ≥ 1 shows "No records on this page" until you page back.
    Adjudicated to a nit across two rounds — the message is accurate and recovery is one
    click. Resetting pageIndex on an empty page is a behaviour change beyond this diff.
  • A useCallback dependency finding was dropped as factually wrong:
    useInstanceImportCapabilities returns a useMemo'd object keyed on permission, so
    its identity is stable.

Coverage caveat on the last round: the gemini leg failed on a headless permission denial
(0 bytes), and the cursor/domain legs were auto-pruned as a narrow low-risk delta, so
round 3 is codex-only. Rounds 1 and 2 carried the full four-lens set with adjudication.

Note on the base

Branched from stage, not from #1703 — the empty state stands on its own and the two
diffs touch different parts of DatabaseTableView.tsx.

🤖 Generated with Claude Code

An empty result set said "No results." and left the user to find Import Data in the
toolbar (which #1703 moves into the "..." menu). The table view is the first view of a
database, so the moment it is empty is the moment to offer a way to fill it.

Import and Seed are offered as two cards rather than one button: they are different
intentions -- bring the data you already have, or get *some* data to look at -- even
though both open the Import Data modal. `ShowImportData` now carries the method each
card wants, and the modal prefers it over its own `tableName ? 'file' : 'sample'`
default.

An empty result set is not an empty table, so the invitation is withheld where it would
be untrue: filters that matched nothing get "No records match these filters" and Clear
Filters, and a page past the end gets "No records on this page" (checked first -- on
page 3 of a filtered set, "nothing matched" is wrong too). Each card is also gated on
its own permission, since `csv_url_load` and `insert`/`csv_data_load` are granted
separately.

The panel renders outside the scrolling table rather than in a spanning cell: the table
can be far wider than the viewport, which would centre the message off past the last
column.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@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 introduces a new EmptyResultSet component to handle empty states in the database table view, offering distinct actions for importing data, seeding data, or adding records manually based on user permissions. It also updates TableView to render this custom empty state outside the scrollable table area, and updates ImportDataModal to support preselecting a specific import method. Feedback was provided to make the guidance text in EmptyResultSet dynamic so that it correctly reflects whether one or both of the import/seed options are available to the user.

Comment thread src/features/instance/databases/components/EmptyResultSet.tsx Outdated
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 62.4% 8643 / 13849
🔵 Statements 62.76% 9242 / 14724
🔵 Functions 55.11% 2176 / 3948
🔵 Branches 55.99% 6229 / 11125
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/features/instance/databases/components/DatabaseActionModals.tsx 0% 0% 0% 0% 29-112
src/features/instance/databases/components/DatabaseTableView.tsx 58.66% 45.55% 34.14% 58.29% 217-230, 234-236, 261, 266-269, 273, 331-334, 441-442, 453-455, 460-473, 483-489, 503-519, 523-531, 540-548, 554-565, 574-579, 583-598, 605-609, 612-639, 643-644, 647-650, 655, 659, 677, 696-697, 816-826
src/features/instance/databases/components/EmptyResultSet.tsx 95.45% 94.44% 100% 95.45% 144
src/features/instance/databases/components/TableView.tsx 72% 57.14% 84.61% 71.42% 139, 144-157, 285-293, 301, 328-334
src/features/instance/databases/modals/ImportDataModal.tsx 37.28% 32.72% 52% 36.84% 69-89, 204-230, 234-238, 243, 247-316, 326-327, 427-440
Generated in workflow #1892 for commit 436494b by the Vitest Coverage Report Action

@cb1kenobi cb1kenobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the empty-table guidance change at 7dfbd5d against the PR bundle. The state precedence, per-permission gating of the Import and Seed cards, modal method preselection, and the loading-race guard all hold up, and the non-empty table rendering is unchanged. CI is green on this commit and Gemini's copy comment is already on file. No blocking issues found.


Reviewed 7dfbd5d

@cb1kenobi cb1kenobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was a little on the fence about the seed data thing, but it's AWESOME. Super clean UI. Nice work!

dawsontoth and others added 2 commits September 11, 2026 19:39
Cross-model review found four ways the first cut told the user something that
wasn't true.

Seed was gated on `methods.sample`, which shares its path with `file` -- so a bare
`insert` grant made it true, and an insert-only role on a fresh primary-key-only table
got a Seed card whose dataset dropdown was empty and whose submit dead-ended on
validation. Both cards are now gated on the source they would actually use: bundled
datasets need `csv-data`, random records need `json-records` plus columns to model
values on.

A bundled dataset retargets the import to the dataset's own table, so the Seed card no
longer promises it fills the table on screen -- only the random-records path does, and
the copy now says which. "Two ways to get some in" appears only when both cards do, and
the Import card describes only the sources this role can reach.

An empty page against a non-zero record count is not an empty table: `search_by_value`
answers a 404 with `{ data: [] }` (getSearchByValue.ts), which Only If Cached -- on by
default -- can produce on a cache miss. That case now says so instead of inviting a
duplicating import.

Also: a manual Refresh of a settled empty table no longer swaps the panel out for the
spinner cell. `data === undefined` is the real "no answer yet" signal, since neither list
query keeps the previous page's rows.

Tests cover the wiring the review said was untested -- each card through to the launch it
fires and the method it carries -- and the comment claiming a role could seed without
importing is gone, because the permission model cannot produce one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Round-2 review findings, both introduced by the previous commit's own fix.

`!!totalRecords` read an *unknown* count as an empty table. `describe_all` carries no
counts and `describe_table` backfills with `retry: false`, so the count is undefined on
first paint and stays undefined if that fetch failed -- and an Only-If-Cached 404, which
`search_by_value` turns into `{ data: [] }`, would then be told it was an empty table.
The heading now asserts emptiness only once the count has said zero; below that it says
"No records in <table>", which is true of the page either way. The invitations stand in
both cases deliberately: withholding them until the count lands would flicker the common
case, a table that really is new and empty.

The seed-random gate read `instanceTable` (describe first) while the modal's Random Data
option reads `instanceDatabaseMap` only, so in the window where describe has attributes
the map lacks, Seed reopened the empty-dropdown dead end the gate exists to close. Both
now read the same map, and the scan is memoized rather than re-running per render.

Tests: the modal's consumption of `initialMethod` was untested, so a dropped `??` would
have shipped -- covered now, including that a requested method the role cannot run still
falls back to an allowed one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@cb1kenobi cb1kenobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-reviewed the two new commits against the full diff at 6bfafab. The recordCount guards distinguish zero, unknown, and non-zero correctly, the seed gate now reads the same attribute map the modal reads so it fails toward withholding a card rather than opening an empty dropdown, and the modal's method preselection still refuses a method the role cannot run. Gemini's copy comment was already addressed and CI is green; no blocking issues found.


Reviewed 6bfafab

…unknown

"No records in <table>" still asserted the table was empty, which is the one thing an
unknown count cannot support. It now reads "No records to show", which is true of the
page whether or not the table holds records; the certain wording stays for a count that
has actually said zero.

Also trims the comments three review rounds have flagged as narration, keeping only the
constraints that are not visible in the code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@cb1kenobi cb1kenobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The empty-table invitations stay behind the page, filter, and non-zero-count guards, and Seed is withheld unless a real sample or random source exists. The latest commit only stops the unknown-count heading from claiming the table is empty. Existing discussion already covered the two-ways copy, which is now gated on both cards. No blocking defects on the changed lines.


Reviewed 436494b

@cb1kenobi cb1kenobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No blocking defects on the changed lines at 436494b. The empty-state precedence (page, then filters, then non-zero count) is correct, both cards are gated on the import source they would actually use rather than the shared method path, and the modal still refuses a preselected method the role cannot run. Two things could not be verified from the bundle alone: whether an open-but-empty filter row sets isFiltered, and what the modal's sample page defaults to for a json-records-only role.


Reviewed 436494b

@dawsontoth

Copy link
Copy Markdown
Contributor Author

Answering the two items @cb1kenobi couldn't verify from the bundle — both check out, no change needed.

An open-but-empty filter row does not set isFiltered. The flag is useFilteredList = filtersToggled && !!appliedSearchConditions (DatabaseTableView.tsx:343), and applyFilters stores null, not [], when nothing was typed: translateFilterValues skips blank values (if (value?.length), line 220) and the result is conditions.length ? conditions : null (line 235). So showing the filter row — or typing and clearing it — leaves an empty table on the invitations rather than on "No records match these filters". Typing without applying does the same, which is consistent: the grid is still running the unfiltered query.

A json-records-only role gets a sample page with exactly one option: Random Data. The bundled-dataset group is gated on allowsSource('csv-data') and the Random Data item on canGenerateRandom && allowsSource('json-records') (ImportDataModal.tsx, the method === 'sample' block). datasetId defaults to '', so the select opens on its "Select a dataset..." placeholder and the user picks the one item present. The Seed card's gate is deliberately the same predicate — allowsSource('json-records') plus a non-empty randomizableAttributes — so the card cannot appear over an empty dropdown. On a primary-key-only table that role gets no Seed card at all, which is the regression test withholds Seed when no dataset and no random records are available.

Worth noting the dropdown does not preselect Random Data even when it is the only choice — pre-existing modal behavior, and a small follow-up if we want Seed to be one click for that role.

🤖 Addressed by Claude Code

@cb1kenobi cb1kenobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The empty-state guards still withhold Import and Seed on later pages, filters, and a non-zero table count, and each card is still gated on the source it would open. The latest heading change only stops an unknown count from claiming the table is empty. The author's follow-up matches the code: an empty filter row does not count as filtered, and a json-records-only Seed card cannot open an empty dropdown. No blocking defects on the changed lines.


Reviewed 436494b

@dawsontoth
dawsontoth added this pull request to the merge queue Sep 12, 2026
Merged via the queue into stage with commit 6320a01 Sep 12, 2026
3 checks passed
@dawsontoth
dawsontoth deleted the claude/empty-table-state-f33fe5 branch September 12, 2026 02:29
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.

2 participants