Skip to content

Commit 4ece5b7

Browse files
feat(cli): default tables import to a new table named after the file
`sim tables import people.csv` now does the obvious thing rather than demanding a target. Requiring one guarded the wrong direction: a forgotten flag creating a new table is visible and easily undone, while the outcome worth protecting — writing into an existing table — is the one that now has to be asked for by name. --to-table becomes --table-id, and --mode/--mapping/--create-columns apply only alongside it. Passing one without it is an error rather than a no-op: silently ignoring `--mode replace` would let it read as honoured while a new table was created beside the one it was meant to overwrite. The reverse is also refused, since --table-id already names the destination. The derived name is sanitized, because table names are identifiers: the obvious basename would reject most real files, so `2026-quarterly sales.csv` imports as `_2026_quarterly_sales` instead of failing. --name overrides it, and is required for --file-id, where there is no file name to take one from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0148a65fkPhP4N8tgGPYtRTU
1 parent 39a4a4f commit 4ece5b7

2 files changed

Lines changed: 83 additions & 46 deletions

File tree

‎packages/sim-cli/src/commands/hand-written.test.ts‎

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -84,31 +84,35 @@ describe('tables import argument guards', () => {
8484
await importCommand().parseAsync(['node', 'sim', 'tables', 'import', ...argv])
8585
}
8686

87-
it('refuses to guess the target', async () => {
88-
// Defaulting to a new table would turn a forgotten `--to-table` into a
89-
// silent second copy of the data.
90-
await expect(run(['f.csv'])).rejects.toThrow(/exactly one of --new-table/)
91-
await expect(run(['f.csv', '--new-table', 'a', '--to-table', 't'])).rejects.toThrow(
92-
/exactly one of --new-table/
93-
)
87+
it('refuses to guess the source', async () => {
88+
// A new table is a safe default; where the bytes are is not inferable.
89+
await expect(run([])).rejects.toThrow(/exactly one of <path>/)
90+
await expect(run(['f.csv', '--file-id', 'w_1'])).rejects.toThrow(/exactly one of <path>/)
9491
})
9592

96-
it('refuses to guess the source', async () => {
97-
await expect(run(['--new-table', 'a'])).rejects.toThrow(/exactly one of <path>/)
98-
await expect(run(['f.csv', '--new-table', 'a', '--file-id', 'w_1'])).rejects.toThrow(
99-
/exactly one of <path>/
100-
)
93+
it('rejects existing-table flags when creating one', async () => {
94+
// Ignoring these would let `--mode replace` read as honoured while a new
95+
// table is created beside the one it was meant to overwrite.
96+
await expect(run(['f.csv', '--mode', 'replace'])).rejects.toThrow(/applies to --table-id/)
97+
await expect(run(['f.csv', '--mapping', '{}'])).rejects.toThrow(/applies to --table-id/)
98+
await expect(run(['f.csv', '--create-columns', '{}'])).rejects.toThrow(/applies to --table-id/)
10199
})
102100

103-
it('names the flag when mapping is paired with a new table', async () => {
104-
// The server rejects this too, but as a message about the request body.
105-
await expect(run(['f.csv', '--new-table', 'a', '--mapping', '{}'])).rejects.toThrow(
106-
/--to-table only/
101+
it('rejects new-table flags when importing into an existing one', async () => {
102+
await expect(run(['f.csv', '--table-id', 't', '--name', 'x'])).rejects.toThrow(
103+
/--table-id already names the destination/
107104
)
105+
await expect(run(['f.csv', '--table-id', 't', '--folder-id', 'f'])).rejects.toThrow(
106+
/--table-id already names the destination/
107+
)
108+
})
109+
110+
it('asks for a name when there is no file name to take one from', async () => {
111+
await expect(run(['--file-id', 'w_1'])).rejects.toThrow(/--name <name>/)
108112
})
109113

110114
it('checks all of that before touching the filesystem', async () => {
111115
// `f.csv` does not exist; a "cannot read" error would mean a guard ran late.
112-
await expect(run(['f.csv'])).rejects.toThrow(/exactly one of/)
116+
await expect(run(['f.csv', '--mode', 'append'])).rejects.toThrow(/applies to --table-id/)
113117
})
114118
})

‎packages/sim-cli/src/commands/hand-written.ts‎

Lines changed: 62 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -294,9 +294,9 @@ interface TableImport {
294294
}
295295

296296
interface ImportOptions {
297-
newTable?: string
298-
toTable?: string
299-
mode: string
297+
name?: string
298+
tableId?: string
299+
mode?: string
300300
folderId?: string
301301
fileId?: string
302302
mapping?: string
@@ -306,6 +306,22 @@ interface ImportOptions {
306306
wait: boolean
307307
}
308308

309+
/**
310+
* Turns a file name into a legal table name.
311+
*
312+
* Table names are identifiers — `^[A-Za-z_][A-Za-z0-9_]*$`, 128 max — so the
313+
* obvious `basename(path)` would reject most real files: `2026-sales.csv` and
314+
* `customer data.csv` both fail. Runs of anything else collapse to a single
315+
* underscore, and a leading digit gets one in front, so a default derived from
316+
* the file is a name the server actually accepts.
317+
*/
318+
function tableNameFrom(fileName: string): string {
319+
const stem = fileName.replace(/\.[^.]+$/, '')
320+
const cleaned = stem.replace(/[^A-Za-z0-9]+/g, '_').replace(/^_+|_+$/g, '')
321+
if (!cleaned) return 'imported_table'
322+
return (/^[0-9]/.test(cleaned) ? `_${cleaned}` : cleaned).slice(0, 128)
323+
}
324+
309325
/** How often to ask an in-progress import where it got to. */
310326
const IMPORT_POLL_MS = 1500
311327

@@ -414,37 +430,49 @@ export function attachHandWritten(program: Command): void {
414430
)
415431

416432
// ── tables import ── a transfer, then an async job to watch ──────────────
417-
const tablesGroup = group(program, 'tables')
418-
tablesGroup
433+
group(program, 'tables')
419434
.command('import [path]')
420-
.description('Import a CSV into a new or existing table')
421-
.option('--new-table <name>', 'Create a table with this name')
422-
.option('--to-table <tableId>', 'Import into an existing table')
423-
.option('--mode <append|replace>', 'How to write into an existing table', 'append')
424-
.option('--folder-id <id>', 'Folder for a new table')
435+
.description('Import a CSV, into a new table by default')
436+
.option('--name <name>', 'Name for the new table (defaults to the file name)')
437+
.option('--table-id <id>', 'Import into this existing table instead of creating one')
438+
.option('--mode <append|replace>', 'How to write into --table-id (default: append)')
439+
.option('--folder-id <id>', 'Folder for the new table')
425440
.option('--file-id <id>', 'Import a file already in the workspace instead of a local path')
426-
.option('--mapping <json|@file>', 'Column mapping (existing table only)')
427-
.option('--create-columns <json|@file>', 'Columns to create (existing table only)')
441+
.option('--mapping <json|@file>', 'Column mapping (--table-id only)')
442+
.option('--create-columns <json|@file>', 'Columns to create (--table-id only)')
428443
.option('--timezone <iana>', 'Timezone for date parsing, e.g. America/New_York')
429444
.option('--no-wait', 'Return once the import is queued instead of watching it')
430445
.action(async (path: string | undefined, options: ImportOptions, command: Command) => {
431446
const { client } = clientFrom(command)
432447
const workspaceId = client.requireWorkspace()
433448

434-
// Both target choices are stated, never inferred. Defaulting to a new
435-
// table would turn a forgotten `--to-table` into a second copy of the
436-
// data, which is not something to discover afterwards.
437-
if (Boolean(options.newTable) === Boolean(options.toTable)) {
438-
throw new SimApiError('Pass exactly one of --new-table <name> or --to-table <id>', 0)
439-
}
449+
// The one thing that cannot be inferred: the bytes are either local or
450+
// already in the workspace, and neither implies the other.
440451
if (Boolean(path) === Boolean(options.fileId)) {
441452
throw new SimApiError('Pass exactly one of <path> or --file-id <id>', 0)
442453
}
443-
// The server rejects these against a new table; saying so here names the
444-
// flag rather than returning a validation error about the request body.
445-
if (options.newTable && (options.mapping || options.createColumns)) {
454+
455+
const intoExisting = Boolean(options.tableId)
456+
457+
// Flags that only mean something for one target. Silently ignoring them
458+
// would let `--mode replace` read as honoured while a new table is
459+
// created beside the one it was meant to overwrite.
460+
const misplaced = intoExisting
461+
? ([
462+
['--name', options.name],
463+
['--folder-id', options.folderId],
464+
] as const)
465+
: ([
466+
['--mode', options.mode],
467+
['--mapping', options.mapping],
468+
['--create-columns', options.createColumns],
469+
] as const)
470+
for (const [flag, value] of misplaced) {
471+
if (value === undefined) continue
446472
throw new SimApiError(
447-
'--mapping and --create-columns apply to --to-table only: a new table takes its columns from the CSV',
473+
intoExisting
474+
? `${flag} applies to a new table; --table-id already names the destination`
475+
: `${flag} applies to --table-id: a new table takes its name and columns from the CSV`,
448476
0
449477
)
450478
}
@@ -459,13 +487,18 @@ export function attachHandWritten(program: Command): void {
459487
}
460488
: { type: 'workspace_file', fileId: options.fileId }
461489

462-
const target = options.toTable
463-
? { type: 'existing', tableId: options.toTable, mode: options.mode }
464-
: {
465-
type: 'new',
466-
name: options.newTable,
467-
...(options.folderId ? { folderId: options.folderId } : {}),
468-
}
490+
let target: Record<string, unknown>
491+
if (intoExisting) {
492+
target = { type: 'existing', tableId: options.tableId, mode: options.mode ?? 'append' }
493+
} else {
494+
// A local file names the table; a workspace file id does not, and
495+
// guessing one from an id would produce nonsense.
496+
const name = options.name ?? (local ? tableNameFrom(local.name) : undefined)
497+
if (!name) {
498+
throw new SimApiError('Pass --name <name> to say what the new table is called', 0)
499+
}
500+
target = { type: 'new', name, ...(options.folderId ? { folderId: options.folderId } : {}) }
501+
}
469502

470503
const started = await client.request<{ data: TableImport }>('/api/v2/tables/imports', {
471504
method: 'POST',

0 commit comments

Comments
 (0)