Skip to content

Commit b56a673

Browse files
chore(tables): remove tables-fractional-ordering feature flag
1 parent c864a92 commit b56a673

17 files changed

Lines changed: 115 additions & 395 deletions

File tree

apps/sim/app/api/table/[tableId]/rows/route.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ async function handleBatchInsert(
7474
rows,
7575
workspaceId: validated.workspaceId,
7676
userId,
77-
positions: validated.positions,
7877
orderKeys: validated.orderKeys,
7978
},
8079
table,

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2732,7 +2732,7 @@ export function TableGrid({
27322732

27332733
if (createBatchRows.length > 0) {
27342734
batchCreateRef.current(
2735-
{ rows: createBatchRows, positions: createBatchPositions },
2735+
{ rows: createBatchRows },
27362736
{
27372737
onSuccess: (response) => {
27382738
const createdRows = response?.data?.rows ?? []

apps/sim/hooks/queries/tables.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,7 @@ type BatchCreateTableRowsParams = Omit<BatchInsertTableRowsBodyInput, 'workspace
779779
type BatchCreateTableRowsResponse = ContractJsonResponse<typeof batchCreateTableRowsContract>
780780

781781
/**
782-
* Batch create rows in a table. Supports optional per-row positions for undo restore.
782+
* Batch create rows in a table. Supports optional per-row order keys for undo restore.
783783
*/
784784
export function useBatchCreateTableRows({ workspaceId, tableId }: RowMutationContext) {
785785
const queryClient = useQueryClient()
@@ -793,7 +793,6 @@ export function useBatchCreateTableRows({ workspaceId, tableId }: RowMutationCon
793793
body: {
794794
workspaceId,
795795
rows: variables.rows as RowData[],
796-
positions: variables.positions,
797796
orderKeys: variables.orderKeys,
798797
},
799798
})

apps/sim/hooks/use-table-undo.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,11 @@ export function useTableUndo({
136136
deleteRowMutation.mutate(action.rowId)
137137
} else {
138138
// Redo via the batch path so the saved orderKey restores exact placement.
139-
// The single-insert API has no orderKey field, and under the fractional-ordering
140-
// flag its `position` is read as a rank — a gappy saved position misplaces.
139+
// The single-insert API has no orderKey field, and order_key is authoritative —
140+
// a gappy saved position would misplace the row.
141141
batchCreateRowsMutation.mutate(
142142
{
143143
rows: [action.data ?? {}],
144-
positions: [action.position],
145144
orderKeys: action.orderKey ? [action.orderKey] : undefined,
146145
},
147146
{
@@ -169,7 +168,6 @@ export function useTableUndo({
169168
batchCreateRowsMutation.mutate(
170169
{
171170
rows: action.rows.map((r) => r.data),
172-
positions: action.rows.map((r) => r.position),
173171
orderKeys: action.rows.every((r) => r.orderKey)
174172
? action.rows.map((r) => r.orderKey as string)
175173
: undefined,
@@ -194,7 +192,6 @@ export function useTableUndo({
194192
batchCreateRowsMutation.mutate(
195193
{
196194
rows: action.rows.map((row) => row.data),
197-
positions: action.rows.map((row) => row.position),
198195
orderKeys: action.rows.every((row) => row.orderKey)
199196
? action.rows.map((row) => row.orderKey as string)
200197
: undefined,

apps/sim/lib/api/contracts/tables.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -197,16 +197,9 @@ export const batchInsertTableRowsBodySchema = z
197197
TABLE_LIMITS.MAX_BATCH_INSERT_SIZE,
198198
`Cannot insert more than ${TABLE_LIMITS.MAX_BATCH_INSERT_SIZE} rows per batch`
199199
),
200-
positions: z.array(z.number().int().min(0)).max(TABLE_LIMITS.MAX_BATCH_INSERT_SIZE).optional(),
201-
/** Fractional ordering: exact per-row order keys (undo restore). Takes precedence over `positions`. */
200+
/** Fractional ordering: exact per-row order keys (undo restore). */
202201
orderKeys: z.array(z.string().min(1)).max(TABLE_LIMITS.MAX_BATCH_INSERT_SIZE).optional(),
203202
})
204-
.refine((data) => !data.positions || data.positions.length === data.rows.length, {
205-
message: 'positions array length must match rows array length',
206-
})
207-
.refine((data) => !data.positions || new Set(data.positions).size === data.positions.length, {
208-
message: 'positions must not contain duplicates',
209-
})
210203
.refine((data) => !data.orderKeys || data.orderKeys.length === data.rows.length, {
211204
message: 'orderKeys array length must match rows array length',
212205
})

apps/sim/lib/copilot/tools/server/table/user-table.ts

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -361,20 +361,6 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
361361
return { success: false, message: 'Workspace ID is required' }
362362
}
363363

364-
const positions = args.positions as number[] | undefined
365-
if (positions !== undefined && positions.length !== args.rows.length) {
366-
return {
367-
success: false,
368-
message: `positions length (${positions.length}) must match rows length (${args.rows.length})`,
369-
}
370-
}
371-
if (positions !== undefined && new Set(positions).size !== positions.length) {
372-
return {
373-
success: false,
374-
message: 'positions must not contain duplicate values',
375-
}
376-
}
377-
378364
const table = await getTableById(args.tableId)
379365
if (!table || table.workspaceId !== workspaceId) {
380366
return { success: false, message: `Table not found: ${args.tableId}` }
@@ -390,7 +376,6 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
390376
rows: args.rows.map((r: RowData) => rowDataNameToId(r, idByName)),
391377
workspaceId,
392378
userId: context.userId,
393-
positions,
394379
},
395380
table,
396381
requestId

apps/sim/lib/core/config/env.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ export const env = createEnv({
7272
ENTERPRISE_STORAGE_LIMIT_GB: z.number().optional().default(500), // Default storage limit in GB for enterprise tier (can be overridden per org)
7373
BILLING_ENABLED: z.boolean().optional(), // Enable billing enforcement and usage tracking
7474
FREE_API_DEPLOYMENT_GATE_ENABLED: z.boolean().optional(), // Block free-plan accounts from programmatic execution (API/MCP/A2A/generic webhooks/chat embeds). Requires BILLING_ENABLED. Off by default for dark rollout
75-
TABLES_FRACTIONAL_ORDERING: z.boolean().optional(), // Order table rows by fractional order_key (O(1) insert/delete) instead of integer position
7675

7776
// Table feature limits (per plan). Apply when billing is disabled (free tier defaults) or for billed plans.
7877
FREE_TABLES_LIMIT: z.number().optional(), // Max user tables per workspace on free tier (default: 3)

apps/sim/lib/core/config/feature-flags.test.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ function withAppConfig(doc: unknown) {
4444
}
4545

4646
/**
47-
* `isFeatureEnabled` only accepts registered `FeatureFlagName`s. The registry is
48-
* empty in this PR, so tests reference flags through the AppConfig document and
49-
* cast their throwaway names through this helper.
47+
* `isFeatureEnabled` only accepts registered `FeatureFlagName`s. These tests
48+
* exercise the evaluation logic with throwaway flag names supplied through the
49+
* AppConfig document, cast to `FeatureFlagName` through this helper.
5050
*/
5151
const enabled = (flag: string, ctx?: FeatureFlagContext) =>
5252
isFeatureEnabled(flag as FeatureFlagName, ctx)
@@ -60,7 +60,6 @@ describe('getFeatureFlags', () => {
6060
it('derives flags from fallback secrets when AppConfig is disabled, without fetching', async () => {
6161
const flags = await getFeatureFlags()
6262
// All registered flags should be present, disabled (env vars unset in test env)
63-
expect(flags['tables-fractional-ordering']).toEqual({ enabled: false })
6463
expect(flags['mothership-beta']).toEqual({ enabled: false })
6564
expect(mockFetch).not.toHaveBeenCalled()
6665
})
@@ -86,7 +85,6 @@ describe('getFeatureFlags', () => {
8685
flagRef.isAppConfigEnabled = true
8786
mockFetch.mockResolvedValue(null)
8887
const flags = await getFeatureFlags()
89-
expect(flags['tables-fractional-ordering']).toEqual({ enabled: false })
9088
expect(flags['mothership-beta']).toEqual({ enabled: false })
9189
})
9290

apps/sim/lib/core/config/feature-flags.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,6 @@ interface FeatureFlagDefinition {
6262

6363
/** The single registry of known flags. To add a flag, add one entry here. */
6464
const FEATURE_FLAGS = {
65-
'tables-fractional-ordering': {
66-
description: 'Order table rows by fractional order_key instead of legacy integer position',
67-
fallback: 'TABLES_FRACTIONAL_ORDERING',
68-
},
6965
'mothership-beta': {
7066
description:
7167
'Mothership beta plan/changelog artifact surfaces in the copilot VFS and doc compiler. ' +

apps/sim/lib/table/__tests__/update-row.test.ts

Lines changed: 10 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,6 @@ import { getUniqueColumns } from '@/lib/table/validation'
1616

1717
vi.mock('@sim/db', () => dbChainMock)
1818

19-
// These suites assert flag-off position-shift semantics; pin the flag so they're
20-
// deterministic regardless of a local TABLES_FRACTIONAL_ORDERING env value.
21-
vi.mock('@/lib/core/config/feature-flags', () => ({
22-
isFeatureEnabled: vi.fn().mockResolvedValue(false),
23-
}))
24-
2519
vi.mock('@/lib/table/validation', () => ({
2620
validateRowSize: vi.fn(() => ({ valid: true, errors: [] })),
2721
validateRowAgainstSchema: vi.fn(() => ({ valid: true, errors: [] })),
@@ -177,29 +171,17 @@ describe('insertRow — position race safety (migration 0198 + advisory lock)',
177171
expect(findExecutedSqlContaining('hashtextextended')).toBe(true)
178172
})
179173

180-
it('explicit-position inserts also acquire the advisory lock to serialize position shifts', async () => {
181-
dbChainMockFns.limit.mockResolvedValueOnce([])
182-
dbChainMockFns.returning.mockResolvedValueOnce([
183-
{
184-
id: 'row-1',
185-
tableId: 'tbl-1',
186-
workspaceId: 'ws-1',
187-
data: { name: 'a' },
188-
position: 5,
189-
createdAt: new Date(),
190-
updatedAt: new Date(),
191-
},
192-
])
193-
194-
await insertRow(
195-
{ tableId: 'tbl-1', data: { name: 'a' }, workspaceId: 'ws-1', position: 5 },
196-
TABLE,
197-
'req-1'
198-
)
174+
it('explicit-position inserts also acquire the advisory lock to serialize order-key minting', async () => {
175+
await expect(
176+
insertRow(
177+
{ tableId: 'tbl-1', data: { name: 'a' }, workspaceId: 'ws-1', position: 5 },
178+
TABLE,
179+
'req-1'
180+
)
181+
).rejects.toBeDefined()
199182

200-
// `(table_id, position)` index is non-unique, so concurrent explicit-position
201-
// inserts at the same slot could both skip the shift and duplicate — lock
202-
// serializes them.
183+
// A position-based insert resolves its order_key from the neighbor at that
184+
// rank; the lock serializes concurrent minting at the same slot.
203185
expect(findExecutedSqlContaining('pg_advisory_xact_lock')).toBe(true)
204186
})
205187

@@ -215,42 +197,6 @@ describe('insertRow — position race safety (migration 0198 + advisory lock)',
215197
expect(findExecutedSqlContaining('pg_advisory_xact_lock')).toBe(true)
216198
})
217199

218-
it('batchInsertRows with explicit positions acquires the advisory lock', async () => {
219-
dbChainMockFns.returning.mockResolvedValueOnce([
220-
{
221-
id: 'row-1',
222-
tableId: 'tbl-1',
223-
workspaceId: 'ws-1',
224-
data: { name: 'a' },
225-
position: 3,
226-
createdAt: new Date(),
227-
updatedAt: new Date(),
228-
},
229-
{
230-
id: 'row-2',
231-
tableId: 'tbl-1',
232-
workspaceId: 'ws-1',
233-
data: { name: 'b' },
234-
position: 4,
235-
createdAt: new Date(),
236-
updatedAt: new Date(),
237-
},
238-
])
239-
240-
await batchInsertRows(
241-
{
242-
tableId: 'tbl-1',
243-
rows: [{ name: 'a' }, { name: 'b' }],
244-
workspaceId: 'ws-1',
245-
positions: [3, 4],
246-
},
247-
TABLE,
248-
'req-1'
249-
)
250-
251-
expect(findExecutedSqlContaining('pg_advisory_xact_lock')).toBe(true)
252-
})
253-
254200
it('upsertRow skips the advisory lock on the update path (match found)', async () => {
255201
vi.mocked(getUniqueColumns).mockReturnValue([{ name: 'name', type: 'string', unique: true }])
256202
dbChainMockFns.limit.mockResolvedValueOnce([

0 commit comments

Comments
 (0)