diff --git a/apps/dev-playground/client/src/features/smart-dashboard/components/kpi-cards.tsx b/apps/dev-playground/client/src/features/smart-dashboard/components/kpi-cards.tsx index 07677d98d..58d0c658f 100644 --- a/apps/dev-playground/client/src/features/smart-dashboard/components/kpi-cards.tsx +++ b/apps/dev-playground/client/src/features/smart-dashboard/components/kpi-cards.tsx @@ -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); @@ -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; @@ -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( - (a, b) => a + (Number.isFinite(b) ? Number(b) : 0), + (a, b) => a + (Number.isFinite(b) ? b : 0), 0, ), )} revenue` diff --git a/docs/docs/development/type-generation.md b/docs/docs/development/type-generation.md index 8430a275d..17646e290 100644 --- a/docs/docs/development/type-generation.md +++ b/docs/docs/development/type-generation.md @@ -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: diff --git a/packages/appkit/src/connectors/sql-warehouse/client.ts b/packages/appkit/src/connectors/sql-warehouse/client.ts index 6afca34f8..32cad1f40 100644 --- a/packages/appkit/src/connectors/sql-warehouse/client.ts +++ b/packages/appkit/src/connectors/sql-warehouse/client.ts @@ -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 | 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 @@ -1036,6 +1049,8 @@ export class SQLWarehouseConnector { result: { ...restResult, data: transformedData, + columnNames: arrowColumnNames(response), + columnTypes: columnTypeNames(response), }, }; } @@ -1083,6 +1098,7 @@ export class SQLWarehouseConnector { ...(response.result as ResultData & { attachment?: string; columnNames?: string[]; + columnTypes?: Array; }), // `statementId` is a top-level field, not on `ResultData` — carry it // onto the result (as the EXTERNAL_LINKS path does) so the route can @@ -1091,6 +1107,7 @@ export class SQLWarehouseConnector { // contract), sourced from the modular SDK's camelCase `statementId`. statement_id: response.statementId, columnNames, + columnTypes: columnTypeNames(response), }, }; } diff --git a/packages/appkit/src/connectors/sql-warehouse/tests/client.test.ts b/packages/appkit/src/connectors/sql-warehouse/tests/client.test.ts index 8344df2fc..4cb8cdb5b 100644 --- a/packages/appkit/src/connectors/sql-warehouse/tests/client.test.ts +++ b/packages/appkit/src/connectors/sql-warehouse/tests/client.test.ts @@ -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)", () => { diff --git a/packages/appkit/src/plugins/analytics/analytics.ts b/packages/appkit/src/plugins/analytics/analytics.ts index 21d0985d9..86b2ac211 100644 --- a/packages/appkit/src/plugins/analytics/analytics.ts +++ b/packages/appkit/src/plugins/analytics/analytics.ts @@ -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; @@ -760,12 +762,14 @@ export class AnalyticsPlugin extends Plugin implements ToolProvider { | Record | undefined, signal?: AbortSignal, + options?: { coerceScalars?: boolean }, ): Promise { const result = await deliverJsonResult( executor, query, processedParams, signal, + options, ); return makeResultMessage(result.data, { status: result.status, diff --git a/packages/appkit/src/plugins/analytics/result-delivery.ts b/packages/appkit/src/plugins/analytics/result-delivery.ts index 3f40439d2..66d25ccfd 100644 --- a/packages/appkit/src/plugins/analytics/result-delivery.ts +++ b/packages/appkit/src/plugins/analytics/result-delivery.ts @@ -41,6 +41,7 @@ export interface QueryExecutor { data?: Record[]; external_links?: ExternalLink[]; columnNames?: string[]; + columnTypes?: Array; statement_id?: string; status?: unknown; refreshChunkLink?: RefreshChunkLink; @@ -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 | undefined, signal?: AbortSignal, + options: { coerceScalars?: boolean } = {}, ): Promise { + const coerce = ( + rows: Record[] | undefined, + result: Awaited>, + ) => + options.coerceScalars + ? coerceScalarCells(rows, result?.columnNames, result?.columnTypes) + : rows; + try { const result = await executor.query( query, @@ -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, }; @@ -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[] | undefined, + columnNames: string[] | undefined, + columnTypes: Array | undefined, +): Record[] | 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; +} + // ──────────────────────────────────────────────────────────────────────────── // Server-side Arrow → JSON row materializer (JSON fallback only) // ──────────────────────────────────────────────────────────────────────────── diff --git a/packages/appkit/src/plugins/analytics/tests/analytics.test.ts b/packages/appkit/src/plugins/analytics/tests/analytics.test.ts index 3104d710b..2e7cc7a0f 100644 --- a/packages/appkit/src/plugins/analytics/tests/analytics.test.ts +++ b/packages/appkit/src/plugins/analytics/tests/analytics.test.ts @@ -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(); diff --git a/packages/appkit/src/plugins/analytics/tests/metric.test.ts b/packages/appkit/src/plugins/analytics/tests/metric.test.ts index 07b83e449..bf7e1c82b 100644 --- a/packages/appkit/src/plugins/analytics/tests/metric.test.ts +++ b/packages/appkit/src/plugins/analytics/tests/metric.test.ts @@ -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, diff --git a/packages/appkit/src/plugins/analytics/tests/result-delivery.test.ts b/packages/appkit/src/plugins/analytics/tests/result-delivery.test.ts index 06438b5a5..f4408a184 100644 --- a/packages/appkit/src/plugins/analytics/tests/result-delivery.test.ts +++ b/packages/appkit/src/plugins/analytics/tests/result-delivery.test.ts @@ -6,6 +6,7 @@ import { type ArrowChunkStreamer, arrowDeliveryUnsupported, classifyDispositionRejection, + coerceScalarCells, decodeArrowAttachmentToRows, deliverArrowBytes, deliverJsonResult, @@ -424,3 +425,88 @@ describe("decodeArrowAttachmentToRows — captured Reyden fixtures", () => { expect(rows).toEqual([REYDEN_NESTED_FIXTURE.expected]); }); }); + +describe("deliverJsonResult — coerceScalars", () => { + const nativeResult = () => ({ + data: [ + { big: "42", dec: "1.50", dbl: "2.5", ok: "true", name: "7", nul: null }, + { big: "-1", dec: "0.00", dbl: "1e3", ok: "false", name: "x", nul: null }, + ], + columnNames: ["big", "dec", "dbl", "ok", "name", "nul"], + columnTypes: ["LONG", "DECIMAL", "DOUBLE", "BOOLEAN", "STRING", "LONG"], + }); + + test("converts numeric and BOOLEAN cells on the native JSON_ARRAY path", async () => { + const { executor } = executorFrom(async () => nativeResult()); + const result = await deliverJsonResult( + executor, + "SELECT 1", + undefined, + undefined, + { coerceScalars: true }, + ); + expect(result.data).toEqual([ + { big: 42, dec: 1.5, dbl: 2.5, ok: true, name: "7", nul: null }, + { big: -1, dec: 0, dbl: 1000, ok: false, name: "x", nul: null }, + ]); + }); + + test("leaves cells as delivered without the option", async () => { + const { executor } = executorFrom(async () => nativeResult()); + const result = await deliverJsonResult(executor, "SELECT 1", undefined); + expect(result.data).toEqual(nativeResult().data); + }); + + test("converts ARROW_STREAM fallback rows the same way; temporal and nested cells stay as delivered", async () => { + const { executor } = executorFrom(async (fp) => { + if (fp.format === "JSON_ARRAY") { + throw reject( + "INVALID_PARAMETER_VALUE", + "Inline disposition only supports ARROW_STREAM format.", + ); + } + return { + attachment: REYDEN_SCALAR_FIXTURE.attachment, + columnNames: [...REYDEN_SCALAR_FIXTURE.columns], + columnTypes: [ + "DECIMAL", + "DECIMAL", + "TIMESTAMP", + "TIMESTAMP_NTZ", + "DATE", + "STRUCT", + "ARRAY", + ], + }; + }); + const result = await deliverJsonResult( + executor, + "SELECT 1", + undefined, + undefined, + { coerceScalars: true }, + ); + expect(result.data).toEqual([ + { ...REYDEN_SCALAR_FIXTURE.expected, amt: 123.45, neg: -0.99 }, + ]); + }); + + test("returns rows unchanged when the manifest names or types are missing", () => { + const rows = [{ n: "1" }]; + expect(coerceScalarCells(rows, undefined, ["LONG"])).toEqual([{ n: "1" }]); + expect(coerceScalarCells(rows, ["n"], undefined)).toEqual([{ n: "1" }]); + }); + + test("keeps NaN and ±Infinity as the warehouse strings (JSON has no number for them)", () => { + const rows = [ + { d: "NaN", f: "NaN" }, + { d: "Infinity", f: "1.25" }, + { d: "-Infinity", f: "1.0E20" }, + ]; + expect(coerceScalarCells(rows, ["d", "f"], ["DOUBLE", "FLOAT"])).toEqual([ + { d: "NaN", f: "NaN" }, + { d: "Infinity", f: 1.25 }, + { d: "-Infinity", f: 1e20 }, + ]); + }); +});