Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,8 @@ function formatNumber(n: number): string {

/** Percent delta between the last `tail` window and the previous window. */
function windowedTrend(values: number[], tail: number): number | undefined {
// Drop nulls/undefined/NaN (e.g. days with no trips after a fare filter) and
// coerce everything to Number defensively — some drivers hand back DECIMAL
// columns as strings, and `0 + "12.35"` would silently string-concat and
// render "NaN%" once we tried to divide.
const clean = values.map((v) => Number(v)).filter(Number.isFinite);
// Drop nulls/undefined/NaN (e.g. days with no trips after a fare filter).
const clean = values.filter(Number.isFinite);
if (clean.length < tail * 2) return undefined;
const recent = clean.slice(-tail);
const prior = clean.slice(-tail * 2, -tail);
Expand All @@ -189,16 +186,10 @@ function windowedTrend(values: number[], tail: number): number | undefined {
}

export function KPICards({ data, sparklines, isLoading }: KPICardsProps) {
// Coerce on intake so downstream sparkline paths and trend math stay purely
// numeric — avoids surprises if a driver ever hands back DECIMAL-as-string.
const toNum = (v: unknown) => {
const n = Number(v);
return Number.isFinite(n) ? n : 0;
};
const tripSeries = sparklines.map((r) => toNum(r.trip_count));
const fareSeries = sparklines.map((r) => toNum(r.avg_fare));
const distSeries = sparklines.map((r) => toNum(r.avg_distance));
const revenueSeries = sparklines.map((r) => toNum(r.total_revenue));
const tripSeries = sparklines.map((r) => r.trip_count);
const fareSeries = sparklines.map((r) => r.avg_fare);
const distSeries = sparklines.map((r) => r.avg_distance);
const revenueSeries = sparklines.map((r) => r.total_revenue);

const TREND_WINDOW = 7;

Expand Down Expand Up @@ -229,11 +220,8 @@ export function KPICards({ data, sparklines, isLoading }: KPICardsProps) {
subtitle={
revenueSeries.length > 0
? `$${formatNumber(
// Explicit Number() wrap on each accumulator step defends
// against a single stray string in the series silently
// turning the whole sum into a concatenated blob.
revenueSeries.reduce<number>(
(a, b) => a + (Number.isFinite(b) ? Number(b) : 0),
(a, b) => a + (Number.isFinite(b) ? b : 0),
0,
),
)} revenue`
Expand Down
13 changes: 13 additions & 0 deletions docs/docs/development/type-generation.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,19 @@ resolve a real table. The sample value is used only at type-generation time; the
runtime query still binds the actual parameter. See
[SQL parameters → Sample values](../plugins/analytics.md#sample-values-for-type-generation).

### Result value types

Numeric columns (`TINYINT`, `SMALLINT`, `INT`, `BIGINT`, `FLOAT`, `DOUBLE`,
`DECIMAL`) are typed `number` and `BOOLEAN` columns `boolean`. The warehouse
returns every JSON_ARRAY cell as a string, so the analytics query route converts
these columns to match. `NaN` and `±Infinity` in `DOUBLE` and `FLOAT` columns
have no JSON number, so they stay the strings `"NaN"`, `"Infinity"` and
`"-Infinity"`. Date and timestamp columns stay ISO strings. A `BIGINT`
beyond `Number.MAX_SAFE_INTEGER` or a `DECIMAL` with more than ~15 significant
digits loses precision as a JS number; `CAST` such a column to `STRING` in SQL to
keep it exact (it is then typed `string`). Metric-view rows keep the warehouse's
string values; see [Metric-view types](#metric-view-types).

## Using generated types

Once types are generated, your IDE will provide autocomplete and type checking:
Expand Down
17 changes: 17 additions & 0 deletions packages/appkit/src/connectors/sql-warehouse/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,19 @@ function arrowColumnNames(response: StatementResponse): string[] | undefined {
);
}

/**
* Manifest type names (`LONG`, `DECIMAL`, `BOOLEAN`, …), parallel to
* {@link arrowColumnNames}. Lets the analytics query route turn JSON_ARRAY's
* all-string cells into the JS types the generated query types promise.
*/
function columnTypeNames(
response: StatementResponse,
): Array<string | undefined> | undefined {
const cols = response.manifest?.schema?.columns;
if (!cols || cols.length === 0) return undefined;
return cols.map((c) => c.typeName);
}

/**
* Coerce the modular SDK's `bigint` row/byte counts back to `number` (the type
* the legacy SDK used). AppKit never does arithmetic on these — they are purely
Expand Down Expand Up @@ -1036,6 +1049,8 @@ export class SQLWarehouseConnector {
result: {
...restResult,
data: transformedData,
columnNames: arrowColumnNames(response),
columnTypes: columnTypeNames(response),
},
};
}
Expand Down Expand Up @@ -1083,6 +1098,7 @@ export class SQLWarehouseConnector {
...(response.result as ResultData & {
attachment?: string;
columnNames?: string[];
columnTypes?: Array<string | undefined>;
}),
// `statementId` is a top-level field, not on `ResultData` — carry it
// onto the result (as the EXTERNAL_LINKS path does) so the route can
Expand All @@ -1091,6 +1107,7 @@ export class SQLWarehouseConnector {
// contract), sourced from the modular SDK's camelCase `statementId`.
statement_id: response.statementId,
columnNames,
columnTypes: columnTypeNames(response),
},
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,31 @@ describe("SQLWarehouseConnector._transformDataArray", () => {
const result = await transform(connector, response);
expect(result.result.data[0].metadata).toEqual({ key: "value" });
});

test("attaches manifest column names and types, leaving cells as delivered", async () => {
const connector = createConnector();
const response = {
statementId: "stmt-1",
status: { state: "SUCCEEDED" },
manifest: {
format: "JSON_ARRAY",
schema: {
columns: [
{ name: "trips", typeText: "BIGINT", typeName: "LONG" },
{ name: "", typeText: "BOOLEAN", typeName: "BOOLEAN" },
],
},
},
result: {
dataArray: [["42", "true"]],
},
} as unknown as StatementResponse;

const result = await transform(connector, response);
expect(result.result.data).toEqual([{ trips: "42", column_1: "true" }]);
expect(result.result.columnNames).toEqual(["trips", "column_1"]);
expect(result.result.columnTypes).toEqual(["LONG", "BOOLEAN"]);
});
});

describe("classic warehouse (EXTERNAL_LINKS + ARROW_STREAM)", () => {
Expand Down
6 changes: 5 additions & 1 deletion packages/appkit/src/plugins/analytics/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,12 +399,14 @@ export class AnalyticsPlugin extends Plugin implements ToolProvider {
// warehouse only accepts ARROW_STREAM for INLINE, retries as
// ARROW_STREAM and decodes server-side — returning the SSE
// `result` message with plain rows. (ARROW_STREAM requests are
// handled earlier via `_handleArrowStreamQuery`.)
// handled earlier via `_handleArrowStreamQuery`.) Numeric and
// BOOLEAN cells are coerced to match the generated query types.
return await self._executeJsonArrayPath(
executor,
query,
processedParams,
sig,
{ coerceScalars: true },
);
} catch (err) {
originalError = err;
Expand Down Expand Up @@ -760,12 +762,14 @@ export class AnalyticsPlugin extends Plugin implements ToolProvider {
| Record<string, SQLTypeMarker | null | undefined>
| undefined,
signal?: AbortSignal,
options?: { coerceScalars?: boolean },
): Promise<AnalyticsSseMessage> {
const result = await deliverJsonResult(
executor,
query,
processedParams,
signal,
options,
);
return makeResultMessage(result.data, {
status: result.status,
Expand Down
70 changes: 68 additions & 2 deletions packages/appkit/src/plugins/analytics/result-delivery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export interface QueryExecutor {
data?: Record<string, unknown>[];
external_links?: ExternalLink[];
columnNames?: string[];
columnTypes?: Array<string | undefined>;
statement_id?: string;
status?: unknown;
refreshChunkLink?: RefreshChunkLink;
Expand Down Expand Up @@ -294,13 +295,27 @@ interface JsonResult {
* attachment to plain rows server-side so the caller's JSON contract holds.
*
* EXTERNAL_LINKS is never used for the JSON fallback.
*
* Both paths yield the warehouse's all-string scalar cells. `coerceScalars`
* converts numeric and BOOLEAN columns to JS numbers/booleans (see
* {@link coerceScalarCells}); the query route sets it, the metric route keeps
* its documented `string | null` wire values.
*/
export async function deliverJsonResult(
executor: QueryExecutor,
query: string,
processedParams: Record<string, SQLTypeMarker | null | undefined> | undefined,
signal?: AbortSignal,
options: { coerceScalars?: boolean } = {},
): Promise<JsonResult> {
const coerce = (
rows: Record<string, unknown>[] | undefined,
result: Awaited<ReturnType<QueryExecutor["query"]>>,
) =>
options.coerceScalars
? coerceScalarCells(rows, result?.columnNames, result?.columnTypes)
: rows;

try {
const result = await executor.query(
query,
Expand All @@ -309,7 +324,7 @@ export async function deliverJsonResult(
signal,
);
return {
data: result?.data,
data: coerce(result?.data, result),
status: result?.status,
statement_id: result?.statement_id,
};
Expand All @@ -336,12 +351,63 @@ export async function deliverJsonResult(
arrowResult.columnNames,
);
return {
data: rows,
data: coerce(rows, arrowResult),
status: arrowResult.status,
statement_id: arrowResult.statement_id,
};
}

/** Manifest `typeName`s whose JSON_ARRAY string cells parse to a JS number. */
const NUMERIC_TYPE_NAMES = new Set([
"BYTE",
"SHORT",
"INT",
"LONG",
"FLOAT",
"DOUBLE",
"DECIMAL",
]);

/**
* Convert JSON_ARRAY's string cells for numeric and BOOLEAN columns to JS
* numbers/booleans so rows match the generated query types (`number` /
* `boolean`). `columnNames`/`columnTypes` are the parallel manifest arrays the
* connector attaches; without them rows are returned unchanged. Mutates the
* freshly built rows in place. NULL cells stay `null`; nested types stay as
* delivered. DOUBLE/FLOAT `"NaN"`, `"Infinity"` and `"-Infinity"` stay strings:
* JSON has no number for them, and the SSE JSON would send `null` (SQL NULL).
*
* shortcut: LONG beyond 2^53 and DECIMAL beyond ~15 significant digits lose
* precision as JS numbers; switch those to string/bigint if apps need exact values.
*/
export function coerceScalarCells(
rows: Record<string, unknown>[] | undefined,
columnNames: string[] | undefined,
columnTypes: Array<string | undefined> | undefined,
): Record<string, unknown>[] | undefined {
if (!rows || !columnNames || !columnTypes) return rows;
const toNumber = (v: string) => {
const n = Number(v);
return Number.isFinite(n) ? n : v;
};
const coercions: Array<[string, (v: string) => number | boolean | string]> =
[];
columnNames.forEach((name, i) => {
const typeName = columnTypes[i];
if (typeName === "BOOLEAN") coercions.push([name, (v) => v === "true"]);
else if (typeName && NUMERIC_TYPE_NAMES.has(typeName))
coercions.push([name, toNumber]);
});
if (coercions.length === 0) return rows;
for (const row of rows) {
for (const [name, convert] of coercions) {
const value = row[name];
if (typeof value === "string") row[name] = convert(value);
}
}
return rows;
}
Comment thread
fjakobs marked this conversation as resolved.
Resolved

// ────────────────────────────────────────────────────────────────────────────
// Server-side Arrow → JSON row materializer (JSON fallback only)
// ────────────────────────────────────────────────────────────────────────────
Expand Down
37 changes: 37 additions & 0 deletions packages/appkit/src/plugins/analytics/tests/analytics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,43 @@ describe("Analytics Plugin", () => {
});
});

test("/query/:query_key converts JSON_ARRAY numeric and BOOLEAN cells to match the generated query types", async () => {
const plugin = new AnalyticsPlugin(config);
const { router, getHandler } = createMockRouter();
(plugin as any).app.getAppQuery = vi.fn().mockResolvedValue({
query: "SELECT * FROM test",
isAsUser: false,
});
// Connector output for a real JSON_ARRAY response: every cell a string,
// plus the manifest names/types the connector attaches.
(plugin as any).SQLClient.executeStatement = vi.fn().mockResolvedValue({
result: {
data: [
{ trips: "42", fare: "12.50", ok: "true", zip: "10001", r: "NaN" },
],
columnNames: ["trips", "fare", "ok", "zip", "r"],
columnTypes: ["LONG", "DECIMAL", "BOOLEAN", "STRING", "DOUBLE"],
},
});

plugin.injectRoutes(router);
const handler = getHandler("POST", "/query/:query_key");
const mockRes = createMockResponse();
await handler(
createMockRequest({
params: { query_key: "test_query" },
body: { parameters: {} },
}),
mockRes,
);

expect(mockRes.write).toHaveBeenCalledWith(
expect.stringContaining(
'"data":[{"trips":42,"fare":12.5,"ok":true,"zip":"10001","r":"NaN"}]',
),
);
});

test("/query/:query_key falls back ARROW_STREAM INLINE→EXTERNAL_LINKS and streams the preserved links in-context", async () => {
const plugin = new AnalyticsPlugin(config);
const { router, getHandler } = createMockRouter();
Expand Down
36 changes: 36 additions & 0 deletions packages/appkit/src/plugins/analytics/tests/metric.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,42 @@ describe("analytics metric route", () => {
expect(mockRes.end).toHaveBeenCalled();
});

test("keeps JSON_ARRAY string cells (metric rows are typed string | null)", async () => {
const plugin = pluginForDir(
config,
registryDir({
revenue: {
key: "revenue",
source: "cat.sch.revenue_metrics",
lane: "sp",
},
}),
);
const { router, getHandler } = createMockRouter();
(plugin as any).SQLClient.executeStatement = vi.fn().mockResolvedValue({
result: {
data: [{ arr: "1234" }],
columnNames: ["arr"],
columnTypes: ["LONG"],
},
});

plugin.injectRoutes(router);
const handler = getHandler("POST", "/metric/:key");
const mockRes = createMockResponse();
await handler(
createMockRequest({
params: { key: "revenue" },
body: { measures: ["arr"] },
}),
mockRes,
);

expect(mockRes.write).toHaveBeenCalledWith(
expect.stringContaining('"data":[{"arr":"1234"}]'),
);
});

test("emits warehouse_status before result", async () => {
const plugin = pluginForDir(
config,
Expand Down
Loading
Loading