Skip to content

feat(api): expand the public v2 tables surface - #6188

Open
TheodoreSpeaks wants to merge 5 commits into
improvement/v2-endpointsfrom
feat/v2-tables-coverage
Open

feat(api): expand the public v2 tables surface#6188
TheodoreSpeaks wants to merge 5 commits into
improvement/v2-endpointsfrom
feat/v2-tables-coverage

Conversation

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator

Summary

  • Adds 16 operations to /api/v2/tables so a public caller can do what the internal surface can — previously it could read and write rows but not rename a table, restore one, manage views, run an enrichment column, or import/export
  • PATCH /tables/[tableId] (rename/move/lock — neither v1 nor v2 had it), POST /restore, saved views (GET/POST /views, GET/PATCH/DELETE /views/[viewId]), POST /columns/run + POST /rows/[rowId]/enrichment/[groupId], GET /groups, POST /rows/find
  • Import/export ship with job control so nothing can be started that can't be observed and stopped: POST /import, /import-async, /import-csv; GET /export, POST /export-async, GET /export/download; GET /jobs, POST /job/cancel, POST /cancel-runs
  • Extracts lib/table/orchestration/import.ts and lib/table/export-stream.ts out of the first-party routes and repoints those routes at them, so v1 and v2 can't drift on what an import or export does
  • events/stream, metadata and dispatches stay internal — editor state, not public API

Notes for review

  • Table gains folderId and locks (toApiTable, v2ApiTableSchema, OpenAPI Table). Without them PATCH could write two fields the surface couldn't read back. Additive; existing response examples updated.
  • PATCH carries the first-party permission split: name/folderId need write, locks needs admin + the table-locks feature. Clearing a lock still works with the feature off, so a locked table is never stranded.
  • runColumnBodySchema and cancelTableRunsBodySchema were split into un-refined base objects plus shared refine helpers (following the existing insertTableRowBodyBaseSchema / rowAnchorMutexRefine precedent) — Zod forbids .extend() on a refined schema and v2 narrows filter to predicate-only. Internal behavior unchanged.
  • GET /export is the one v2 success body that isn't { data } — it's the file. mode: 'stream' contract, rate-limit headers set by hand.
  • Multipart routes bypass parseRequest (no body on the contract); form fields are parsed against contract form schemas, workspaceId is required ahead of the file part, and the 10 MB proxy cap is enforced — without it Next silently truncates and a partial import reports success.
  • Caught one regression during the refactor: the 423 body's lock field was being dropped. The orchestration now threads the lock kind through, so v1 renders { error, lock } again (no details, which would make the client swallow the toast) and v2 surfaces it as details: { lock }.
  • Based on improvement/v2-endpoints, same as feat(api): complete the v2 workflows resource with versions and CRUD #6184 and feat(cli): Sim CLI with AWS-style profiles and a platform key exchange #6147.

Type of Change

  • New feature (non-breaking change which adds functionality)

Testing

  • bun run check:api-validation:strict — passes (route baseline 1046 → 1062)
  • bun run check:openapi — passes, 109 operations / 100 contracts cross-checked
  • bun run type-check, bun run lint:check — pass
  • bunx vitest run app/api/v2/tables app/api/table lib/table — 82 files, 1079 tests pass
  • Full suite: 1327/1329 files pass; the 2 failures (lib/billing/storage/tracking.test.ts, executor/handlers/pi/cloud-review-tools.test.ts) reproduce on a clean tree with these changes stashed — pre-existing and unrelated
  • One route.test.ts per new route: gate off → 404, invalid body → 400, access denied → 403, rate limited → 429, happy path asserting the exact data shape and the lib call

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)

Adds 16 operations so a v2 caller can do what the internal surface can:
rename/move/lock a table, restore it, manage saved views, run enrichment
columns, look up rows, and import/export with observable job control.

Extracts lib/table/orchestration/import.ts (performTableCsvImport,
performCreateTableFromCsv) and lib/table/export-stream.ts from the
first-party routes, then repoints those routes at them, so v1 and v2
cannot drift on what an import or export actually does.

events/stream, metadata and dispatches stay internal — they are editor
state, not public API.
@vercel

vercel Bot commented Aug 2, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview Aug 2, 2026 5:25am

Request Review

@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Large public API expansion plus shared import/export orchestration affects data writes, background jobs, and table locks; regressions could change client behavior or leave partial imports/exports, though tests and OpenAPI checks are in place.

Overview
Expands the public v2 Tables OpenAPI with many new operations and schemas: PATCH table (rename/move/locks with partial-success semantics), restore, saved views CRUD, workflow groups listing, column/enrichment runs, row find, CSV import/create-from-CSV, sync/async export, job listing/download, job cancel, and cancel-runs. Table response examples and the Table model now include folderId, locks, and job, plus shared error responses (Conflict, Locked, PayloadTooLarge, Gone).

Refactors first-party table routes so v1 and v2 share behavior: streaming export moves to lib/table/export-stream (createTableExportStream, filename/content-type helpers), and synchronous CSV import / create-from-CSV delegate to lib/table/orchestration (performTableCsvImport, performCreateTableFromCsv) instead of inlined pipeline logic in the route handlers.

v2 error plumbing now accepts optional details on orchestration failures (e.g. lock kind) via v2ErrorForOrchestration; the sync import route returns 423 with { error, lock } (no details) for UI toast behavior. v1 middleware gains route kinds for the new table capabilities.

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

@greptile-apps

greptile-apps Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR substantially expands the public v2 Tables API and extracts shared import/export orchestration for reuse by first- and second-generation routes. The follow-up changes also address the previously reported PATCH consistency issue.

  • Adds table updates, restore, views, enrichment, search, import/export, and job-control operations.
  • Extends public table responses with folder and lock state.
  • Prevalidates deterministic PATCH rejections before writes and reports any operations committed before a later runtime failure.
  • Signals schema changes whenever at least one PATCH operation commits.

Confidence Score: 5/5

The PR appears safe to merge.

The previously reported PATCH issue is resolved: deterministic rejection checks now occur before mutation, every successful sequential write is recorded before a later failure, and schema-change signaling runs whenever any operation commits, so no blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/app/api/v2/tables/[tableId]/route.ts Implements PATCH prevalidation, sequential mutation tracking, partial-success reporting, and schema-change signaling; the previously reported stale-client path is addressed.
apps/sim/lib/table/orchestration/tables.ts Centralizes table mutation outcomes and preserves structured errors for the expanded v2 surface.
apps/sim/lib/table/orchestration/import.ts Extracts shared import orchestration so first-party and public routes use the same behavior.
apps/sim/lib/table/export-stream.ts Extracts shared streaming export behavior for reuse across API versions.
apps/sim/lib/api/contracts/v2/tables.ts Defines the expanded v2 Tables request and response contracts, including update, view, enrichment, import/export, and job operations.
apps/docs/openapi-v2-tables.json Documents the expanded public Tables API and its explicit PATCH partial-success contract.

Sequence Diagram

sequenceDiagram
  participant Client
  participant Route as PATCH /api/v2/tables/{tableId}
  participant Auth as Workspace authorization
  participant DB as Table services
  participant Events as Table event stream

  Client->>Route: name / folderId / locks
  Route->>Auth: Check write/admin access
  Route->>Route: Prevalidate feature and folder constraints
  alt Rejected before mutation
    Route-->>Client: 4xx with no changes
  else Prevalidation succeeds
    Route->>DB: Apply locks
    Route->>DB: Apply rename
    Route->>DB: Apply folder move
    Route->>Events: Signal when any operation committed
    alt Later operation fails
      Route-->>Client: Error with details.applied
    else All operations succeed
      Route->>DB: Re-read table
      Route-->>Client: Updated table
    end
  end
Loading

Reviews (5): Last reviewed commit: "feat(api): make v2 table PATCH state whi..." | Re-trigger Greptile

Comment thread apps/sim/app/api/v2/tables/[tableId]/route.ts Outdated
Comment thread apps/sim/app/api/v2/tables/utils.ts
…ry 423

Greptile P1: PATCH applied locks, rename and move as three sequential
transactions, so a folder rejected mid-request left the earlier writes
persisted while the response reported failure — and the schema-changed
signal was skipped, leaving open clients on stale state. Every rejectable
condition now runs before the first write, and the signal fires whenever
anything did land.

Cursor: v2TableLockError dropped the lock kind, so async import, column
run, enrichment and table mutations returned a bare LOCKED. A table has
four independent locks, so the caller could not tell which to clear.
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/api/v2/tables/[tableId]/route.ts
…n ones

The previous commit named the lock only where the rejection was thrown and
caught at the route boundary. Where it instead arrives as a classified
`errorCode: 'locked'` outcome — delete table, delete row, update column,
and the table mutations — the kind was dropped, so those 423s stayed
unactionable while their neighbours improved.

The orchestration results now carry `lock`, and a shared
`v2TableOrchestrationError` renders both arrival paths into the same
`{ code, message, details: { lock } }` body. `details` is omitted rather
than sent null when the kind is unknown, so a caller branching on it sees
absence instead of a phantom value.
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/docs/openapi-v2-tables.json Outdated
`POST /import-async` pointed callers at `GET /api/v2/tables/jobs` to track
progress, but that endpoint filters to `type = 'export'` — imports are
derived onto the table itself, one write job at a time, and exports get a
separate list precisely because they are excluded from that derivation.
The public Table shape omitted those derived fields, so an async import
could be started and cancelled but never observed to completion, failure,
or progress. That is the gap the import/export/job-control set was meant
to close.

Table now carries `job` — id, type, status, rowsProcessed, error, or null
when idle — and the import-async docs point at the table rather than the
export list.
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

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 83e04cb. Configure here.

Greptile held the PR at 4/5 on the residual non-atomicity and named two
acceptable resolutions: make PATCH atomic, or have the contract adopt and
expose partial-success explicitly. Atomicity would mean threading one
transaction through renameTable, moveTableToFolder and updateTableLocks —
three shared service functions with four non-test callers including the
first-party route and two copilot tools — and deferring their per-operation
audits to commit time. That is a refactor of shared write paths well
outside this PR.

So the contract states it instead. Every rejectable condition is already
pre-validated, so a failure here is a genuine fault; when one follows a
successful operation the error now carries `details.applied` listing what
is live. Absent when nothing applied, so its presence always means "these
changes took effect despite the error". Documented on the operation.

`v2ErrorForOrchestration` gained the optional `details` this needs.
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

Addressing the 4/5 hold on non-atomic PATCH (b25c426) — the finding is fair, and I took the second of the two resolutions you named.

Why not atomicity. It would require one transaction spanning renameTable, moveTableToFolder and updateTableLocks. Those are shared service functions with four non-test callers — the v2 route, the first-party PATCH /api/table/[tableId], and two copilot tool handlers (table/user-table.ts, vfs-mutate.ts) — and each currently opens its own transaction and emits its own audit post-commit. Making them composable means adding WithTx variants across all three and deferring their audits to the outer commit, since an audit describing a write that later rolls back is worse than the partial state it would be fixing. That is a refactor of shared write paths, with its own failure modes, and it does not belong in a PR about the public v2 surface.

What landed instead. The endpoint now adopts partial-success explicitly rather than implying atomicity it does not have:

  • Every rejectable condition is already pre-validated (previous commit), so no bad request can produce partial state. What remains is only a genuine fault: a lost race, the table archived mid-request, a database error.
  • When such a fault follows a successful operation, the error carries error.details.applied listing what is nevertheless live — ["locks"], ["name"], ["folderId"], or a combination. A caller reconciles from that instead of re-reading and diffing to discover it.
  • The field is absent when nothing applied, so its presence always means "these changes took effect despite the error" — the distinction that makes it actionable rather than decorative.
  • Documented on the operation in the OpenAPI spec, so it is part of the published contract rather than undocumented behaviour.

Two tests pin both halves: a move failing after a successful rename asserts details equals { applied: ["name"] }, and a first-operation failure asserts details is undefined and that no later operation ran.

This required giving v2ErrorForOrchestration an optional details parameter, which it arguably should have had — the lock-kind work in the previous commit had to route around its absence.

Worth flagging for the human reviewer: the first-party PATCH /api/table/[tableId] has the same non-atomic shape and no such reporting. This PR does not otherwise touch that file, so I have left it rather than expanding scope — happy to follow up separately.

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile

@TheodoreSpeaks

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 b25c426. Configure here.

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