Skip to content

Commit a76d6d7

Browse files
committed
fix: second QA pass — header sniff, reshape guards, worker/DuckDB aliases
- Treat single-row CSV as headerless; warn on missing primary source - validateReshape + reset reshape on file/sample load; safer pivot wider path - DuckDB planner resolves duplicate source names (orders.csv (2) → orders.csv) - Harden JSONL jq parse, explode invalid JSON, worker URL client validation - Fix smoke-duckdb artifact (browserExecutable); clarify >1GB escalation copy - Add reshape, duckdb alias, and single-row regression tests
1 parent 510d94a commit a76d6d7

14 files changed

Lines changed: 292 additions & 28 deletions

apps/worker/src/url-source.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@ function inferFormatFromName(name: string): FileFormat {
1212
return 'tsv';
1313
}
1414

15-
if (lower.endsWith('.jsonl') || lower.endsWith('.ndjson')) {
15+
if (lower.endsWith('.ndjson')) {
16+
return 'ndjson';
17+
}
18+
19+
if (lower.endsWith('.jsonl')) {
1620
return 'jsonl';
1721
}
1822

packages/core/src/duckdb-plan.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,15 @@ export function buildDuckDbQueryPlan(
344344
sources: DuckDbPlanSource[],
345345
): DuckDbQueryPlan {
346346
const primarySourceName = chain.input[0]?.ref ?? sources[0]?.name ?? '';
347-
const primaryIndex = sources.findIndex((source) => source.name === primarySourceName);
347+
let primaryIndex = sources.findIndex((source) => source.name === primarySourceName);
348+
349+
if (primaryIndex < 0) {
350+
primaryIndex = sources.findIndex((source) => {
351+
const duplicateAlias = source.name.match(/^(.+) \((\d+)\)$/);
352+
353+
return duplicateAlias?.[1] === primarySourceName;
354+
});
355+
}
348356

349357
if (primaryIndex < 0) {
350358
return {
@@ -368,6 +376,14 @@ export function buildDuckDbQueryPlan(
368376
registeredSources.map((source) => [source.sourceName, source.tableName]),
369377
);
370378

379+
for (const source of registeredSources) {
380+
const duplicateAlias = source.sourceName.match(/^(.+) \((\d+)\)$/);
381+
382+
if (duplicateAlias && !tableMap.has(duplicateAlias[1]!)) {
383+
tableMap.set(duplicateAlias[1]!, source.tableName);
384+
}
385+
}
386+
371387
let currentSql = `SELECT * FROM ${registeredSources[primaryIndex].tableName}`;
372388

373389
for (const step of chain.verbs) {

packages/core/src/execution.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -876,6 +876,10 @@ export function executeVerbChain(chain: VerbChain, sources: ChainSource[]): Exec
876876
let current = parsedSources.get(primarySourceName) ?? { columns: [], rows: [] };
877877
const warnings: string[] = [];
878878

879+
if (primarySourceName && !parsedSources.has(primarySourceName)) {
880+
warnings.push(`Primary source "${primarySourceName}" was not loaded.`);
881+
}
882+
879883
for (const step of chain.verbs) {
880884
switch (step.kind) {
881885
case 'cat':

packages/core/src/input.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,14 @@ function splitDelimitedRow(line: string, delimiter: string): string[] {
147147
}
148148

149149
function inferHeader(rows: string[][]): boolean {
150-
if (rows.length < 2) {
150+
if (rows.length === 0) {
151151
return true;
152152
}
153153

154+
if (rows.length === 1) {
155+
return false;
156+
}
157+
154158
const firstRow = rows[0];
155159
const secondRow = rows[1];
156160
const headerish = /^[A-Za-z_][\w .-]*$/;

packages/core/src/json-query.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,16 @@ function previewFromRows(rows: DataRow[]): PreviewTable {
116116
}
117117

118118
export function applyJsonQuery(text: string, query: string): JsonQueryResult {
119-
const rows = splitLines(text).map((line) => JSON.parse(line) as DataRow);
120119
const warnings: string[] = [];
120+
const rows: DataRow[] = [];
121+
122+
for (const line of splitLines(text)) {
123+
try {
124+
rows.push(JSON.parse(line) as DataRow);
125+
} catch {
126+
warnings.push(`Skipped invalid JSON line: ${line.slice(0, 120)}`);
127+
}
128+
}
121129
const operations = query
122130
.split('|')
123131
.map((part) => part.trim())

packages/core/src/reshape.ts

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,15 @@ export function explodeField(rows: DataRow[], field: string): DataRow[] {
124124
const trimmed = value.trim();
125125

126126
if (trimmed.startsWith('[') && trimmed.endsWith(']')) {
127-
const parsed = JSON.parse(trimmed) as DataValue[];
128-
return parsed.map((item) => ({ ...row, [field]: item }));
127+
try {
128+
const parsed = JSON.parse(trimmed) as DataValue[];
129+
130+
if (Array.isArray(parsed)) {
131+
return parsed.map((item) => ({ ...row, [field]: item }));
132+
}
133+
} catch {
134+
// Fall through to comma-split or single-row output.
135+
}
129136
}
130137

131138
if (trimmed.includes(',')) {
@@ -137,6 +144,56 @@ export function explodeField(rows: DataRow[], field: string): DataRow[] {
137144
});
138145
}
139146

147+
export function validateReshape(rows: DataRow[], config: ReshapeConfig): string | null {
148+
if (config.mode === 'none') {
149+
return null;
150+
}
151+
152+
if (config.mode === 'longer') {
153+
const fields = parseFields(config.fields);
154+
155+
if (fields.length === 0) {
156+
return 'Pivot longer requires at least one field name.';
157+
}
158+
159+
if (!fields.some((field) => rows.some((row) => field in row))) {
160+
return `Pivot longer fields (${fields.join(', ')}) were not found in the current preview.`;
161+
}
162+
163+
return null;
164+
}
165+
166+
if (config.mode === 'wider') {
167+
if (config.namesFrom && rows.some((row) => config.namesFrom! in row)) {
168+
return null;
169+
}
170+
171+
const pivotFields = parseFields(config.fields);
172+
173+
if (!pivotFields.some((field) => rows.some((row) => field in row))) {
174+
return 'Pivot wider needs a long-form column or recognizable month columns in the current preview.';
175+
}
176+
177+
return null;
178+
}
179+
180+
if (config.mode === 'explode') {
181+
const field = config.field?.trim() ?? '';
182+
183+
if (!field) {
184+
return 'Explode requires a field name.';
185+
}
186+
187+
if (!rows.some((row) => field in row)) {
188+
return `Explode field "${field}" was not found in the current preview.`;
189+
}
190+
191+
return null;
192+
}
193+
194+
return null;
195+
}
196+
140197
export function applyReshape(rows: DataRow[], config: ReshapeConfig): ReshapeResult {
141198
let nextRows = rows;
142199

packages/core/test/duckdb-plan.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,4 +139,39 @@ describe('duckdb query planner', () => {
139139

140140
expect(plan.supported).toBe(false);
141141
});
142+
143+
it('resolves duplicate source aliases for joins', () => {
144+
const chain: VerbChain = {
145+
input: [{ format: 'csv', ref: 'orders.csv' }],
146+
verbs: [
147+
{
148+
kind: 'join',
149+
opts: {
150+
leftKey: 'id',
151+
rightKey: 'id',
152+
rightSource: 'users.csv',
153+
},
154+
},
155+
],
156+
output: { format: 'csv' },
157+
};
158+
159+
const plan = buildDuckDbQueryPlan(chain, [
160+
{
161+
dialect: null,
162+
format: 'csv',
163+
name: 'orders.csv (2)',
164+
text: 'id,amount\n1,10\n',
165+
},
166+
{
167+
dialect: null,
168+
format: 'csv',
169+
name: 'users.csv',
170+
text: 'id,name\n1,Asha\n',
171+
},
172+
]);
173+
174+
expect(plan.supported).toBe(true);
175+
expect(plan.sql).toContain('LEFT JOIN input_1 AS joined_stream');
176+
});
142177
});

packages/core/test/index.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,18 @@ describe('sample registry', () => {
4646
});
4747
});
4848

49+
it('treats a single data row as headerless', () => {
50+
const dialect = sniffDialect('Alice,30\n');
51+
const inspection = inspectInput(new TextEncoder().encode('Alice,30\n'), 'csv');
52+
53+
expect(dialect.hasHeader).toBe(false);
54+
expect(inspection.preview.rows).toHaveLength(1);
55+
expect(inspection.preview.rows[0]).toMatchObject({
56+
column_1: 'Alice',
57+
column_2: '30',
58+
});
59+
});
60+
4961
it('detects BOM-based UTF-16LE content', () => {
5062
const bytes = new Uint8Array([0xff, 0xfe, 0x61, 0x00, 0x62, 0x00]);
5163

packages/core/test/reshape.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { applyReshape, validateReshape } from '../src/reshape';
4+
5+
describe('reshape validation', () => {
6+
it('skips pivot wider when month columns are absent', () => {
7+
const rows = [
8+
{ order_id: '1001', category: 'books', total: '42.5' },
9+
{ order_id: '1002', category: 'electronics', total: '129.99' },
10+
];
11+
12+
expect(
13+
validateReshape(rows, {
14+
mode: 'wider',
15+
fields: 'jan,feb,mar',
16+
namesFrom: 'month',
17+
valuesFrom: 'value',
18+
groupBy: 'region',
19+
}),
20+
).toMatch(/long-form column|recognizable month columns/i);
21+
});
22+
23+
it('allows pivot wider after an automatic longer step on wide-form data', () => {
24+
const rows = [
25+
{ region: 'north', jan: '120', feb: '140' },
26+
{ region: 'south', jan: '98', feb: '111' },
27+
];
28+
29+
const longer = applyReshape(rows, {
30+
mode: 'longer',
31+
fields: 'jan,feb',
32+
namesTo: 'month',
33+
valuesTo: 'sales',
34+
});
35+
36+
expect(
37+
validateReshape(longer.rows, {
38+
mode: 'wider',
39+
namesFrom: 'month',
40+
valuesFrom: 'sales',
41+
groupBy: 'region',
42+
}),
43+
).toBeNull();
44+
});
45+
46+
it('falls back when explode field contains invalid bracket JSON', () => {
47+
const rows = [{ tags: '[not valid json' }];
48+
49+
expect(() =>
50+
applyReshape(rows, {
51+
mode: 'explode',
52+
field: 'tags',
53+
}),
54+
).not.toThrow();
55+
56+
expect(
57+
applyReshape(rows, {
58+
mode: 'explode',
59+
field: 'tags',
60+
}).rows,
61+
).toHaveLength(1);
62+
});
63+
});

packages/web/scripts/smoke-duckdb.mjs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,22 +106,23 @@ async function runScenario(page, scenario) {
106106
};
107107
}
108108

109-
const edgeCandidates = [
109+
const browserCandidates = [
110+
process.env.CSVSHAPE_BROWSER_EXECUTABLE,
110111
process.env.CSVSHAPE_EDGE_PATH,
111112
'C:\\Program Files (x86)\\Microsoft\\Edge\\Application\\msedge.exe',
112113
'C:\\Program Files\\Microsoft\\Edge\\Application\\msedge.exe',
113114
].filter(Boolean);
114115

115-
const edgeExecutable = edgeCandidates.find((candidate) => existsSync(candidate));
116+
const browserExecutable = browserCandidates.find((candidate) => existsSync(candidate));
116117

117-
if (!edgeExecutable) {
118-
throw new Error('Microsoft Edge executable was not found for the DuckDB-WASM smoke test.');
119-
}
120-
121-
const browser = await chromium.launch({
122-
executablePath: edgeExecutable,
123-
headless: true,
124-
});
118+
const browser = browserExecutable
119+
? await chromium.launch({
120+
executablePath: browserExecutable,
121+
headless: true,
122+
})
123+
: await chromium.launch({
124+
headless: true,
125+
});
125126

126127
const consoleMessages = [];
127128
let page;
@@ -156,7 +157,7 @@ try {
156157
{
157158
baseUrl,
158159
consoleMessages,
159-
edgeExecutable,
160+
browserExecutable: browserExecutable ?? null,
160161
results,
161162
status: 'ok',
162163
},
@@ -172,7 +173,7 @@ try {
172173
artifactPath,
173174
baseUrl,
174175
consoleMessages,
175-
edgeExecutable,
176+
browserExecutable: browserExecutable ?? null,
176177
results,
177178
status: 'ok',
178179
},

0 commit comments

Comments
 (0)