Skip to content

Commit a003274

Browse files
refactor: use API enum names (JSON_ARRAY, ARROW_STREAM) and simplify format model
Address Mario's review: collapse three formats (JSON, ARROW, ARROW_STREAM) into two that match the Databricks API enums. ARROW_STREAM supports both INLINE and EXTERNAL_LINKS dispositions with automatic fallback. Default remains JSON_ARRAY per reviewer request. Co-authored-by: Isaac
1 parent 055cd41 commit a003274

9 files changed

Lines changed: 104 additions & 216 deletions

File tree

packages/appkit-ui/src/react/charts/__tests__/types.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ describe("isQueryProps", () => {
9393
const props = {
9494
queryKey: "test_query",
9595
parameters: { limit: 100 },
96-
format: "json" as const,
96+
format: "json_array" as const,
9797
};
9898

9999
expect(isQueryProps(props as any)).toBe(true);

packages/appkit-ui/src/react/charts/types.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { Table } from "apache-arrow";
55
// ============================================================================
66

77
/** Supported data formats for analytics queries */
8-
export type DataFormat = "json" | "arrow" | "arrow_stream" | "auto";
8+
export type DataFormat = "json_array" | "arrow_stream" | "auto";
99

1010
/** Chart orientation */
1111
export type Orientation = "vertical" | "horizontal";
@@ -77,8 +77,8 @@ export interface QueryProps extends ChartBaseProps {
7777
parameters?: Record<string, unknown>;
7878
/**
7979
* Data format to use
80-
* - "json": Use JSON format (smaller payloads, simpler)
81-
* - "arrow": Use Arrow format (faster for large datasets)
80+
* - "json_array": Use JSON format (smaller payloads, simpler)
81+
* - "arrow_stream": Use Arrow format (faster for large datasets)
8282
* - "auto": Automatically select based on expected data size
8383
* @default "auto"
8484
*/

packages/appkit-ui/src/react/hooks/__tests__/use-chart-data.test.ts

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ describe("useChartData", () => {
7272
});
7373

7474
describe("format selection", () => {
75-
test("uses JSON format when explicitly specified", () => {
75+
test("uses JSON_ARRAY format when explicitly specified", () => {
7676
mockUseAnalyticsQuery.mockReturnValue({
7777
data: [],
7878
loading: false,
@@ -82,18 +82,18 @@ describe("useChartData", () => {
8282
renderHook(() =>
8383
useChartData({
8484
queryKey: "test",
85-
format: "json",
85+
format: "json_array",
8686
}),
8787
);
8888

8989
expect(mockUseAnalyticsQuery).toHaveBeenCalledWith(
9090
"test",
9191
undefined,
92-
expect.objectContaining({ format: "JSON" }),
92+
expect.objectContaining({ format: "JSON_ARRAY" }),
9393
);
9494
});
9595

96-
test("uses ARROW format when explicitly specified", () => {
96+
test("uses ARROW_STREAM format when explicitly specified", () => {
9797
mockUseAnalyticsQuery.mockReturnValue({
9898
data: [],
9999
loading: false,
@@ -103,18 +103,18 @@ describe("useChartData", () => {
103103
renderHook(() =>
104104
useChartData({
105105
queryKey: "test",
106-
format: "arrow",
106+
format: "arrow_stream",
107107
}),
108108
);
109109

110110
expect(mockUseAnalyticsQuery).toHaveBeenCalledWith(
111111
"test",
112112
undefined,
113-
expect.objectContaining({ format: "ARROW" }),
113+
expect.objectContaining({ format: "ARROW_STREAM" }),
114114
);
115115
});
116116

117-
test("auto-selects ARROW for large limit", () => {
117+
test("auto-selects ARROW_STREAM for large limit", () => {
118118
mockUseAnalyticsQuery.mockReturnValue({
119119
data: [],
120120
loading: false,
@@ -132,11 +132,11 @@ describe("useChartData", () => {
132132
expect(mockUseAnalyticsQuery).toHaveBeenCalledWith(
133133
"test",
134134
{ limit: 1000 },
135-
expect.objectContaining({ format: "ARROW" }),
135+
expect.objectContaining({ format: "ARROW_STREAM" }),
136136
);
137137
});
138138

139-
test("auto-selects ARROW for date range queries", () => {
139+
test("auto-selects ARROW_STREAM for date range queries", () => {
140140
mockUseAnalyticsQuery.mockReturnValue({
141141
data: [],
142142
loading: false,
@@ -157,7 +157,7 @@ describe("useChartData", () => {
157157
expect(mockUseAnalyticsQuery).toHaveBeenCalledWith(
158158
"test",
159159
expect.objectContaining({ startDate: "2025-01-01" }),
160-
expect.objectContaining({ format: "ARROW" }),
160+
expect.objectContaining({ format: "ARROW_STREAM" }),
161161
);
162162
});
163163

@@ -179,7 +179,7 @@ describe("useChartData", () => {
179179
expect(mockUseAnalyticsQuery).toHaveBeenCalledWith(
180180
"test",
181181
expect.anything(),
182-
expect.objectContaining({ format: "JSON" }),
182+
expect.objectContaining({ format: "JSON_ARRAY" }),
183183
);
184184
});
185185

@@ -201,11 +201,11 @@ describe("useChartData", () => {
201201
expect(mockUseAnalyticsQuery).toHaveBeenCalledWith(
202202
"test",
203203
expect.anything(),
204-
expect.objectContaining({ format: "ARROW" }),
204+
expect.objectContaining({ format: "ARROW_STREAM" }),
205205
);
206206
});
207207

208-
test("auto-selects ARROW_STREAM by default when no heuristics match", () => {
208+
test("auto-selects JSON_ARRAY by default when no heuristics match", () => {
209209
mockUseAnalyticsQuery.mockReturnValue({
210210
data: [],
211211
loading: false,
@@ -223,11 +223,11 @@ describe("useChartData", () => {
223223
expect(mockUseAnalyticsQuery).toHaveBeenCalledWith(
224224
"test",
225225
{ limit: 100 },
226-
expect.objectContaining({ format: "ARROW_STREAM" }),
226+
expect.objectContaining({ format: "JSON_ARRAY" }),
227227
);
228228
});
229229

230-
test("defaults to auto format (ARROW_STREAM) when format is not specified", () => {
230+
test("defaults to auto format (JSON_ARRAY) when format is not specified", () => {
231231
mockUseAnalyticsQuery.mockReturnValue({
232232
data: [],
233233
loading: false,
@@ -243,7 +243,7 @@ describe("useChartData", () => {
243243
expect(mockUseAnalyticsQuery).toHaveBeenCalledWith(
244244
"test",
245245
undefined,
246-
expect.objectContaining({ format: "ARROW_STREAM" }),
246+
expect.objectContaining({ format: "JSON_ARRAY" }),
247247
);
248248
});
249249
});
@@ -353,29 +353,29 @@ describe("useChartData", () => {
353353
expect(result.current.isArrow).toBe(false);
354354
});
355355

356-
test("isArrow reflects requested ARROW format when data is null", () => {
356+
test("isArrow reflects requested ARROW_STREAM format when data is null", () => {
357357
mockUseAnalyticsQuery.mockReturnValue({
358358
data: null,
359359
loading: true,
360360
error: null,
361361
});
362362

363363
const { result } = renderHook(() =>
364-
useChartData({ queryKey: "test", format: "arrow" }),
364+
useChartData({ queryKey: "test", format: "arrow_stream" }),
365365
);
366366

367367
expect(result.current.isArrow).toBe(true);
368368
});
369369

370-
test("isArrow reflects requested JSON format when data is null", () => {
370+
test("isArrow reflects requested JSON_ARRAY format when data is null", () => {
371371
mockUseAnalyticsQuery.mockReturnValue({
372372
data: null,
373373
loading: true,
374374
error: null,
375375
});
376376

377377
const { result } = renderHook(() =>
378-
useChartData({ queryKey: "test", format: "json" }),
378+
useChartData({ queryKey: "test", format: "json_array" }),
379379
);
380380

381381
expect(result.current.isArrow).toBe(false);

packages/appkit-ui/src/react/hooks/types.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { Table } from "apache-arrow";
55
// ============================================================================
66

77
/** Supported response formats for analytics queries */
8-
export type AnalyticsFormat = "JSON" | "ARROW" | "ARROW_STREAM";
8+
export type AnalyticsFormat = "JSON_ARRAY" | "ARROW_STREAM";
99

1010
/**
1111
* Typed Arrow Table - preserves row type information for type inference.
@@ -33,9 +33,9 @@ export interface TypedArrowTable<
3333

3434
/** Options for configuring an analytics SSE query */
3535
export interface UseAnalyticsQueryOptions<
36-
F extends AnalyticsFormat = "ARROW_STREAM",
36+
F extends AnalyticsFormat = "JSON_ARRAY",
3737
> {
38-
/** Response format - "ARROW_STREAM" (default) uses inline Arrow, "JSON" returns typed arrays, "ARROW" uses external links */
38+
/** Response format - "JSON_ARRAY" (default) returns typed arrays, "ARROW_STREAM" uses Arrow (inline or external links) */
3939
format?: F;
4040

4141
/** Maximum size of serialized parameters in bytes */
@@ -122,7 +122,9 @@ export type InferResultByFormat<
122122
T,
123123
K,
124124
F extends AnalyticsFormat,
125-
> = F extends "ARROW" ? TypedArrowTable<InferRowType<K>> : InferResult<T, K>;
125+
> = F extends "ARROW_STREAM"
126+
? TypedArrowTable<InferRowType<K>>
127+
: InferResult<T, K>;
126128

127129
/**
128130
* Infers parameters type from QueryRegistry[K]["parameters"]

packages/appkit-ui/src/react/hooks/use-analytics-query.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ function getArrowStreamUrl(id: string) {
2727
* Integration hook between client and analytics plugin.
2828
*
2929
* The return type is automatically inferred based on the format:
30-
* - `format: "JSON"` (default): Returns typed array from QueryRegistry
31-
* - `format: "ARROW"`: Returns TypedArrowTable with row type preserved
30+
* - `format: "JSON_ARRAY"` (default): Returns typed array from QueryRegistry
31+
* - `format: "ARROW_STREAM"`: Returns TypedArrowTable with row type preserved
3232
*
3333
* Note: User context execution is determined by query file naming:
3434
* - `queryKey.obo.sql`: Executes as user (OBO = on-behalf-of / user delegation)
@@ -39,28 +39,28 @@ function getArrowStreamUrl(id: string) {
3939
* @param options - Analytics query settings including format
4040
* @returns Query result state with format-appropriate data type
4141
*
42-
* @example JSON format (default)
42+
* @example JSON_ARRAY format (default)
4343
* ```typescript
4444
* const { data } = useAnalyticsQuery("spend_data", params);
4545
* // data: Array<{ group_key: string; cost_usd: number; ... }> | null
4646
* ```
4747
*
48-
* @example Arrow format
48+
* @example ARROW_STREAM format
4949
* ```typescript
50-
* const { data } = useAnalyticsQuery("spend_data", params, { format: "ARROW" });
50+
* const { data } = useAnalyticsQuery("spend_data", params, { format: "ARROW_STREAM" });
5151
* // data: TypedArrowTable<{ group_key: string; cost_usd: number; ... }> | null
5252
* ```
5353
*/
5454
export function useAnalyticsQuery<
5555
T = unknown,
5656
K extends QueryKey = QueryKey,
57-
F extends AnalyticsFormat = "ARROW_STREAM",
57+
F extends AnalyticsFormat = "JSON_ARRAY",
5858
>(
5959
queryKey: K,
6060
parameters?: InferParams<K> | null,
6161
options: UseAnalyticsQueryOptions<F> = {} as UseAnalyticsQueryOptions<F>,
6262
): UseAnalyticsQueryResult<InferResultByFormat<T, K, F>> {
63-
const format = options?.format ?? "ARROW_STREAM";
63+
const format = options?.format ?? "JSON_ARRAY";
6464
const maxParametersSize = options?.maxParametersSize ?? 100 * 1024;
6565
const autoStart = options?.autoStart ?? true;
6666

packages/appkit-ui/src/react/hooks/use-chart-data.ts

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ export interface UseChartDataOptions {
1717
parameters?: Record<string, unknown>;
1818
/**
1919
* Data format preference
20-
* - "json": Force JSON format
21-
* - "arrow": Force Arrow format
20+
* - "json_array": Force JSON format
21+
* - "arrow_stream": Force Arrow format
2222
* - "auto": Auto-select based on heuristics
2323
* @default "auto"
2424
*/
@@ -50,33 +50,32 @@ export interface UseChartDataResult {
5050
function resolveFormat(
5151
format: DataFormat,
5252
parameters?: Record<string, unknown>,
53-
): "JSON" | "ARROW" | "ARROW_STREAM" {
53+
): "JSON_ARRAY" | "ARROW_STREAM" {
5454
// Explicit format selection
55-
if (format === "json") return "JSON";
56-
if (format === "arrow") return "ARROW";
55+
if (format === "json_array") return "JSON_ARRAY";
5756
if (format === "arrow_stream") return "ARROW_STREAM";
5857

5958
// Auto-selection heuristics
6059
if (format === "auto") {
6160
// Check for explicit hint in parameters
62-
if (parameters?._preferArrow === true) return "ARROW";
63-
if (parameters?._preferJson === true) return "JSON";
61+
if (parameters?._preferArrow === true) return "ARROW_STREAM";
62+
if (parameters?._preferJson === true) return "JSON_ARRAY";
6463

6564
// Check limit parameter as data size hint
6665
const limit = parameters?.limit;
6766
if (typeof limit === "number" && limit > ARROW_THRESHOLD) {
68-
return "ARROW";
67+
return "ARROW_STREAM";
6968
}
7069

7170
// Check for date range queries (often large)
7271
if (parameters?.startDate && parameters?.endDate) {
73-
return "ARROW";
72+
return "ARROW_STREAM";
7473
}
7574

76-
return "ARROW_STREAM";
75+
return "JSON_ARRAY";
7776
}
7877

79-
return "ARROW_STREAM";
78+
return "JSON_ARRAY";
8079
}
8180

8281
// ============================================================================
@@ -98,7 +97,7 @@ function resolveFormat(
9897
* // Force Arrow format
9998
* const { data } = useChartData({
10099
* queryKey: "big_query",
101-
* format: "arrow"
100+
* format: "arrow_stream"
102101
* });
103102
* ```
104103
*/
@@ -111,7 +110,7 @@ export function useChartData(options: UseChartDataOptions): UseChartDataResult {
111110
[format, parameters],
112111
);
113112

114-
const isArrowFormat = resolvedFormat === "ARROW";
113+
const isArrowFormat = resolvedFormat === "ARROW_STREAM";
115114

116115
// Fetch data using the analytics query hook
117116
const {

0 commit comments

Comments
 (0)