refactor(tables): consolidate row data-access in service.ts

[TheodoreSpeaks]

Summary

• Route both row-GET endpoints (internal + public v1) and the copilot tool through the single service.queryRows instead of three inline query copies; add a withExecutions option so v1 still omits executions.
• Run COUNT(*) and the page fetch concurrently in queryRows (was two serial round-trips).
• Move CSV-import transaction ownership out of the API route into importAppendRows / importReplaceRows so routes never hold a trx.
• Extract row position mechanics (reserve / shift / compact) into named private helpers in service.ts — readable, and a single locus for the planned fractional-indexing change.
• Net: one module does table work (service.ts); no separate wrapper layer.

Type of Change

• Refactor (no behavior change)

Testing

Tested manually. 244 table / import / row-route / copilot tests pass; type-check, bun run lint:check, and bun run check:api-validation:strict all clean.

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]

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Jun 4, 2026 10:02pm

[cursor]

PR Summary

Medium Risk
Touches core row ordering, bulk delete compaction, and import transactions; behavior should be equivalent but mistakes could affect concurrency, pagination totals, or import atomicity.

Overview
Centralizes user table row reads, ordering, deletes, and CSV import writes in lib/table/service.ts so API routes and copilot stop duplicating Drizzle/SQL.

Row listing: Internal and v1 GET handlers now call shared queryRows instead of inline filter/sort/pagination (and execution sidecar loading). queryRows runs count + page in parallel and adds withExecutions (v1 passes false).

CSV import: The import route delegates to importAppendRows / importReplaceRows, which own a single transaction for optional column adds plus batched or replace row writes; tests mock these entry points instead of low-level *WithTx helpers.

Position & deletes: Shared helpers (acquireRowOrderLock, reserveInsertPosition, reserveBatchPositions, compactPositions, insertOrderedRow, deleteOrderedRow, deleteOrderedRowsByIds, buildOrderedRowValues) back insert/batch/replace/upsert/delete paths; copilot table writes use buildOrderedRowValues for contiguous positions after replace-all.

^{Reviewed by Cursor Bugbot for commit 77d6a14. Bugbot is set up for automated code reviews on this repo. Configure here.}

[TheodoreSpeaks]

@greptile review

[greptile-apps]

Greptile Summary

This refactor centralizes all user_table_rows read, position-write, and import operations into service.ts, eliminating three duplicate query-build + execution-sidecar implementations across the internal route, v1 route, and copilot tool.

• Read path: queryRows gains a withExecutions toggle and runs COUNT(*) + SELECT concurrently via Promise.all when includeTotal is true; both GET routes delegate to it and drop ~200 lines of inline SQL.
• Write path: acquireRowOrderLock, reserveInsertPosition, reserveBatchPositions, compactPositions, and buildOrderedRowValues replace scattered inline position-shift SQL; importAppendRows / importReplaceRows own the import transaction so routes no longer hold a trx directly.
• Tests: 15 new table-wrapper.test.ts cases and updated import-route tests align with the new service surface.

Confidence Score: 5/5

Safe to merge; the refactor is behavior-preserving across all read, write, and import paths.

All route wire shapes, transaction boundaries, advisory-lock usage, and position compaction logic are faithfully reproduced in the new helpers. The only notable side effect is a redundant advisory-lock round-trip in upsertRow's insert path, which is harmless due to PostgreSQL's re-entrant transaction-level advisory locks.

apps/sim/lib/table/service.ts — the upsertRow insert path now acquires the advisory lock twice; worth a one-line fix before the next performance-sensitive follow-up.

Important Files Changed

Filename	Overview
apps/sim/lib/table/service.ts	Core of the refactor: introduces acquireRowOrderLock, nextRowPosition, reserveInsertPosition, reserveBatchPositions, compactPositions, and the import wrapper functions. Logic is equivalent to the removed code, but upsertRow's insert path acquires the advisory lock twice (once explicitly, once inside reserveInsertPosition).
apps/sim/lib/table/types.ts	Adds withExecutions flag to QueryOptions; clean addition with good JSDoc.
apps/sim/app/api/table/[tableId]/rows/route.ts	Replaces ~115 lines of inline query-build / execution-sidecar logic with a single queryRows call; shape of the response JSON is identical.
apps/sim/app/api/v1/tables/[tableId]/rows/route.ts	Same route-simplification as the internal route; passes withExecutions: false so the v1 response correctly omits the executions sidecar join.
apps/sim/app/api/table/[tableId]/import/route.ts	Route no longer holds a trx; delegates to importAppendRows / importReplaceRows, which own the transaction atomically. Behavior preserved.
apps/sim/lib/copilot/request/tools/tables.ts	Replaces two identical inline chunk.map blocks with buildOrderedRowValues; position arithmetic is identical (startPosition + i == old i + j).
apps/sim/app/api/table/[tableId]/import/route.test.ts	Test mocks updated to match the new importAppendRows / importReplaceRows surface; helper functions appendRows / appendAdditions make argument assertions cleaner.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Route or Tool] -->|read| B[queryRows]
    B --> C{includeTotal?}
    C -->|yes| D["Promise.all: COUNT + SELECT"]
    C -->|no| E[SELECT only]
    D --> F{withExecutions?}
    E --> F
    F -->|true| G[loadExecutionsByRow]
    F -->|false| H[skip sidecar join]
    G --> I[QueryResult]
    H --> I

    A2[Import Route] -->|append| J[importAppendRows]
    A2 -->|replace| K[importReplaceRows]
    J --> L["db.transaction: addTableColumnsWithTx + batchInsertRowsWithTx batches"]
    K --> M["db.transaction: addTableColumnsWithTx + replaceTableRowsWithTx"]
    L --> N[acquireRowOrderLock + reserveBatchPositions]
    M --> N

    A3["insertRow / upsertRow"] --> O["insertOrderedRow / reserveInsertPosition"]
    O --> N

    A4[deleteRow] --> P["deleteOrderedRow: shiftRowsDownAfter"]
    A5["deleteRowsByIds / deleteRowsByFilter"] --> Q["deleteOrderedRowsByIds: compactPositions"]

Loading

_{Reviews (5): Last reviewed commit: "refactor(tables): consolidate row data-a..." | Re-trigger Greptile}

[greptile-apps]

Greptile Summary

This PR centralizes all user_table_rows read, order, and position-write logic into a new lib/table/table-wrapper.ts module. All three consumer paths — the internal GET route, the v1 public GET route, and the copilot tool — are routed through the shared queryRows / position-primitive helpers, eliminating ~3 duplicate implementations of the order+paginate pattern and the inline position-shift SQL.

• table-wrapper.ts (new): Exports queryRows (ordered/paginated read with withExecutions toggle), acquireRowOrderLock, nextRowPosition, openInsertSlot, shiftRowsAtOrAfter, decrementPositionsAfter, recompactPositions, and buildOrderedRowValues.
• service.ts / route.ts / v1/route.ts / copilot/tools/tables.ts: Each now delegates to the wrapper, removing the duplicated SQL. The copilot buildOrderedRowValues call also fixes the id-generation and position-assignment loop.
• table-wrapper.test.ts (new): 15 unit tests covering position primitives and queryRows branching.

Confidence Score: 4/5

Safe to merge; the refactor is behavior-preserving and all position primitives map 1-to-1 with the code they replaced.

All SQL in the new wrapper is identical to what it replaces, workspace-scoping and advisory-lock ordering are unchanged, and the 15 new unit tests cover the key branching paths. The one change worth watching is that queryRows now runs the COUNT(*) and the main SELECT sequentially — the old v1 route ran them in parallel via Promise.all when includeTotal=true, so paginated requests on the public API will take slightly longer than before.

apps/sim/lib/table/table-wrapper.ts — the sequential count+select path for includeTotal=true is the only deviation from the previous implementation worth a second look.

Important Files Changed

Filename	Overview
apps/sim/lib/table/table-wrapper.ts	New module exporting the centralized row query and position-write primitives; logic is correct and matches the removed implementations, though queryRows runs COUNT(*) and the main SELECT sequentially where the old v1 route ran them in parallel.
apps/sim/lib/table/service.ts	All inline position helpers (acquireTablePositionLock, nextAutoPosition, recompactPositions) and loadExecutionsByRow removed and replaced with table-wrapper imports; the public queryRows signature is unchanged and delegates cleanly.
apps/sim/app/api/table/[tableId]/rows/route.ts	Internal GET handler replaced with a single queryRows call (withExecutions defaults to true); response shape and workspaceId guards are unchanged.
apps/sim/app/api/v1/tables/[tableId]/rows/route.ts	Public v1 GET handler now uses queryRows with withExecutions:false; previously ran COUNT(*) and the main SELECT in parallel via Promise.all when includeTotal=true — now sequential, a minor latency regression on paginated requests.
apps/sim/lib/copilot/request/tools/tables.ts	Two identical chunk-insert loops now use buildOrderedRowValues; position assignment and id generation are equivalent to the removed inline maps.
apps/sim/lib/table/tests/table-wrapper.test.ts	New test file with 15 cases covering position primitives and queryRows branching (sort/filter/includeTotal/withExecutions); good coverage of the new seam.

Sequence Diagram

sequenceDiagram
    participant IR as Internal Route
    participant V1 as v1 Route
    participant SVC as service.ts queryRows()
    participant TW as table-wrapper.ts queryRows()
    participant DB as Postgres

    IR->>TW: "queryRows(table, {withExecutions: true})"
    V1->>TW: "queryRows(table, {withExecutions: false})"
    SVC->>TW: queryRowsWrapper(table, options)

    TW->>DB: "COUNT(*) WHERE tableId+workspaceId+filter [if includeTotal]"
    DB-->>TW: totalCount
    TW->>DB: "SELECT * FROM user_table_rows ORDER BY position/sort LIMIT/OFFSET"
    DB-->>TW: rows[]
    TW->>DB: "SELECT * FROM table_row_executions WHERE rowId IN (...) [if withExecutions]"
    DB-->>TW: executions[]
    TW-->>IR: QueryResult rows, rowCount, totalCount, executions
    TW-->>V1: "QueryResult rows, rowCount, totalCount, executions={}"
    TW-->>SVC: QueryResult

Loading

_{Reviews (2): Last reviewed commit: "refactor(tables): centralize row read/or..." | Re-trigger Greptile}

[TheodoreSpeaks]

@greptile review

[TheodoreSpeaks]

@greptile review

[TheodoreSpeaks]

@greptile review