diff --git a/.wiki/Architecture.md b/.wiki/Architecture.md index dab695b5..decc3dda 100644 --- a/.wiki/Architecture.md +++ b/.wiki/Architecture.md @@ -71,7 +71,16 @@ module mocking. now calls the package's own stateless `killQuery` directly, and there is exactly one generic ClickHouse HTTP transport implementation left in the repository. -3. `JSONStringsEachRowWithProgress` is folded line by line by pure stream logic. +3. `JSONStringsEachRowWithProgress` is folded line by line by pure stream logic + (`core/stream.js`'s `applyStreamLine`). A `meta` line establishes named/ + typed columns when the server sends one. **#627**: a meta-less first `row` + (ClickHouse 24.8 and earlier never emit `meta` for ordinary queries) + instead establishes name-only columns from that row's own keys, with the + unknown-type sentinel `type: ''` — never a value-based type guess. Row + values are stored in whichever order was established first, meta or not. + This fallback belongs to `core/stream.js` as SQL Browser result policy; + the package's own stream reading (`streamLines`) is unchanged and never + synthesizes metadata. 4. Results resolve through the panel registry to table, chart, logs, KPI, filter, text, or graph-oriented renderers. 5. One auth refresh is attempted for expired/denied tokens. diff --git a/.wiki/Decisions-and-Roadmap.md b/.wiki/Decisions-and-Roadmap.md index 7db1352c..c659d8af 100644 --- a/.wiki/Decisions-and-Roadmap.md +++ b/.wiki/Decisions-and-Roadmap.md @@ -163,6 +163,15 @@ Two roadmap tracks are current: `Response`/rejection while the caller's `AbortSignal` controls the real fetch, or a deliberate renegotiation of the transport contract's cancellation semantics themselves. + **Current state (#627, landed):** the general meta-line compatibility bug + called out above is resolved — `core/stream.ts`'s `applyStreamLine` now + establishes name-only columns (`type: ''`) from the first row when a + stream never sends `meta`, instead of silently discarding every row. + ClickHouse 24.8 is now limited support: query execution and Table results + work; automatic typed-result parity (charts/KPI/logs/type-aware + formatting) remains outside the 24.8 guarantee, since those servers still + never provide result-type metadata. This is independent of ADR-0005, which + **remains Rejected**. - **#630 — extract the SQL Browser's own Fetch-native transport mechanics into a first-party package.** Independent of the #585/ADR-0005 track above diff --git a/CHANGELOG.md b/CHANGELOG.md index 730f7086..4581ee83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -483,6 +483,13 @@ auto-generated per-PR notes; this file is the curated, human-readable history. changed at any point across either amendment. ### Fixed +- **#627: preserve Table results from ClickHouse 24.8 streams that omit result + metadata.** The first meta-less row now establishes column names in response + object-key order with the explicit unknown-type sentinel `type: ''`, and + later rows retain that established order without value-based type inference. + Meta-first streams keep their existing typed behavior. ClickHouse 24.8 is + documented as limited support: query execution and Table results are + supported, while typed-result features may degrade when metadata is absent. - **#642: `check:arch`'s generic layering rules (and Rule B) now fail closed on a computed dynamic `import(...)` instead of silently skipping it.** `extractSpecifiers` (renamed `extractStaticSpecifiers`) used to include a diff --git a/README.md b/README.md index c4817635..b974c095 100644 --- a/README.md +++ b/README.md @@ -782,10 +782,6 @@ panel-sizing spec. The full system-requirements matrix — minimum browser versions, supported ClickHouse server versions, and IdP/OAuth requirements — is tracked in #71. -A known ClickHouse-version compatibility gap that any such matrix needs to -account for is tracked in #627: query results render silently empty on -servers predating a 2025 ClickHouse streaming-format change (see -`docs/ADR-0005-clickhouse-web-client.md`). One feature is narrower than the rest of the app: [**Export**](#export) needs the File System Access API, which today is **Chromium-only** (Chrome/Edge) over @@ -793,6 +789,18 @@ HTTPS or `localhost`. On Firefox, Safari, or plain HTTP, the Export button stays visible but disabled with a tooltip explaining why — no other feature is affected. +## ClickHouse server compatibility + +**ClickHouse 24.8: Limited support — query execution and Table results are +supported. Typed result features such as automatic charts, KPI +interpretation, logs detection, and type-aware formatting may be unavailable +because these servers do not provide result metadata in the streaming format +used by SQL Browser.** + +SQL Browser does not infer ClickHouse result types from returned values. The +exact first 25.x release that supplies the missing streaming metadata has +not been established. + ## Development For source development, testing, end-to-end checks, and release workflows, see diff --git a/docs/ADR-0005-clickhouse-web-client.md b/docs/ADR-0005-clickhouse-web-client.md index b8349fcb..4f201a8d 100644 --- a/docs/ADR-0005-clickhouse-web-client.md +++ b/docs/ADR-0005-clickhouse-web-client.md @@ -105,8 +105,10 @@ reason: `{"meta":[...]}` column-header line on ClickHouse 24.8 — that capability was added by ClickHouse GitHub PR #74181 ("JSONEachRowWithProgress format will include meta, totals, and extremes"), merged 2025-01-06, postdating - 24.8. `src/core/stream.ts`'s `applyStreamLine()` has no meta-less - fallback, so every row silently maps to an empty/null value — + 24.8. `src/core/stream.ts`'s `applyStreamLine()` had no meta-less + fallback at the time of this evidence (since fixed in #627 — see the + "#627 production compatibility follow-up" note below), so every row + silently mapped to an empty/null value — **identically** for the current transport and the candidate: on both 24.8 rows, the live precision corpus shows `currentMatchesOfficial: true` (both adapters agree with each other) while both disagree with the @@ -147,10 +149,22 @@ semantics. This ADR's final Rejected decision rests on that new finding, not on either of the two gates above (both remain exactly as characterized by the 2026-08-07 methodology amendment). -**This does not mean ClickHouse 24.8 is newly supported.** #627 is -unaffected by this decision either way — the current transport and the -candidate share the identical meta-line defect, and fixing it is -independent, ongoing work tracked on its own. +**This did not mean ClickHouse 24.8 was newly supported at the time of this +decision.** #627 was unaffected by this decision either way — the current +transport and the candidate shared the identical meta-line defect, and +fixing it was independent, ongoing work tracked on its own. See the +"#627 production compatibility follow-up" note immediately below: that work +has since landed. + +**#627 production compatibility follow-up:** ClickHouse 24.8 now has +limited, data-safe support for ordinary query execution and Table results. +When result metadata is absent, SQL Browser establishes column names from +the first row and records the unknown ClickHouse result type as `type: ''`, +preserving returned values without inventing type semantics. This resolves +the production compatibility defect observed by the #585 spike. It does +**not** adopt, authorize, or reopen `@clickhouse/client-web`; ADR-0005 +remains **Rejected**. The committed #585 matrix remains historical evidence +of the pre-#627 behavior. **The current custom transport (`src/net/ch-client.ts`) remains authoritative — no cutover was ever attempted or is now authorized.** No @@ -266,9 +280,11 @@ the "Decision" section above). `@clickhouse/client-web` need" — it does not, by itself, set or change SQL Browser's own general ClickHouse-version support floor. That remains a separate, open question: #71 tracks the documented support matrix, and #627 -tracks fixing the underlying meta-line bug this derivation surfaced, which -affects the *current* transport regardless of this ADR's outcome or of -which client SQL Browser eventually ships. +tracked fixing the underlying meta-line bug this derivation surfaced (since +fixed — see the "#627 production compatibility follow-up" note under +"## Decision: Rejected" above), which affected the *current* transport +regardless of this ADR's outcome or of which client SQL Browser eventually +ships. ## Exact server matrix @@ -858,10 +874,12 @@ surfaced that they fail for two very different *kinds* of reasons: - `supported-server matrix`'s failure is **symmetric**: the current transport and the candidate read back the same wrong (empty) values on 24.8, because the root cause (`src/core/stream.ts`'s `applyStreamLine()` - has no fallback for a ClickHouse response that never sends a `meta` - line — a real, pre-existing, general SQL Browser defect, now tracked as - #627) lives entirely on the SQL Browser side, unrelated to which HTTP - client issues the request. A candidate that fails identically to the + had no fallback for a ClickHouse response that never sends a `meta` + line — a real, pre-existing, general SQL Browser defect, tracked as #627 + and since fixed there; see the "#627 production compatibility follow-up" + note under "## Decision: Rejected" above) lived entirely on the SQL + Browser side, unrelated to which HTTP client issues the request. A + candidate that fails identically to the status quo isn't *worse* than the status quo on this axis — the original Phase 0 evidence run's mechanical rule (any required hard gate failing → Rejected) didn't distinguish "the candidate regressed something" from diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0dd86f07..c35dcf85 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -237,6 +237,20 @@ section below). A single automatic token refresh on 401/403/ `authenticatedRequest()` (#630 Phase 6): before `authConfirmed` flips, an auth failure signs out; after, it is a query error. +`applyStreamLine` normalizes both shapes a progress stream can take. A +meta-first stream (a `{meta:[...]}` line before any row) establishes +server-provided names/types directly. **#627**: if a `row` line arrives +before any columns exist — ClickHouse 24.8 and earlier never emit `meta` for +ordinary queries — SQL Browser establishes name-only columns from that +row's object keys instead, using the unknown-type sentinel `type: ''`, and +every subsequent row maps through that established name order. This +fallback is SQL Browser result POLICY owned by `core/stream.ts`, not +package protocol parsing — `packages/clickhouse-http` never synthesizes +metadata; it only decodes whatever the server actually sent. Typed result +consumers (chart auto-detection, KPI/logs interpretation, type-aware +formatting) degrade to their existing generic/fail-closed behavior when a +column's type is unknown, rather than inferring a type from values. + ### Transport seam (#585 Phase 1) and the clickhouse-http package (#630 Phases 2-4) Generic request construction and stream mechanics are split out behind a diff --git a/docs/evidence/585/README.md b/docs/evidence/585/README.md index 0297e423..05cffaac 100644 --- a/docs/evidence/585/README.md +++ b/docs/evidence/585/README.md @@ -4,4 +4,23 @@ Generated by `tests/spike/clickhouse-client/run-matrix.mjs`. See `decision-table the canonical hard-gate table (generated from `results.json`, never hand-edited), `results.json` for the full machine-readable evidence, and `critical-questions.md` / `support-minimum-analysis.md` / `deletion-estimate.md` / `compatibility-matrix.md` for the -plan's named deliverables. Validate with `npm run check:client-spike:evidence`. +plan's named deliverables. + +This directory is immutable historical #585 evidence. It was generated and validated by +the then-present `tests/spike/clickhouse-client` tooling; that executable harness and its +`check:client-spike:evidence` npm script were retired in #630 Phase 8. Do not regenerate +these historical results. Current ClickHouse 24.8 production compatibility evidence for +#627 lives in `../627/`. + +## Post-spike production follow-up + +* #585's pinned 24.8 rows correctly record the pre-#627 failure (see + `compatibility-matrix.md`) — those rows are truthful historical observations and are not + rewritten to "passed". +* #627 is the independent production compatibility fix: `src/core/stream.ts`'s + `applyStreamLine()` now establishes name-only columns (`type: ''`) from the first row + when a stream omits `meta` entirely, instead of silently discarding every row. +* Current live verification against the same two pinned 24.8 images lives in + `docs/evidence/627/`. +* ADR-0005 remains **Rejected** — this follow-up does not adopt, authorize, or reopen + `@clickhouse/client-web`. diff --git a/docs/evidence/627/README.md b/docs/evidence/627/README.md new file mode 100644 index 00000000..bf3304a1 --- /dev/null +++ b/docs/evidence/627/README.md @@ -0,0 +1,179 @@ +# Live ClickHouse 24.8 evidence — issue #627 + +Committed evidence for the two immutable historical-generation ClickHouse 24.8 images +named in issue #627 and the #585 spike's compatibility matrix. This is a one-off, +manually-run verification (see "Procedure" below); there is no permanent Docker-matrix +test harness for it, by design (the #585 spike harness was retired in #630 Phase 8, and +these two rows are pinned, immutable digests, not a maintained-forever regression suite). + +## Tested commit + +The live Docker capture below (raw `*.ndjson` bytes, `docker pull`/`docker run`, +image digests, and asserted `SELECT version()`) was performed against +`036760afe565db89a43f112fa37db3bf7f192257` (branch `wip/627-metaless-stream-columns` +— the same commit/branch as this repo's `fix/627-metaless-stream-columns`, not a +diverged fork). At that commit, `src/core/stream.ts`'s meta-less fallback had been +unchanged since this branch's first commit, `fe19ab7` (`fix(#627): preserve Table +results from ClickHouse 24.8 meta-less streams`). + +That is **not** the branch's only later commit. A subsequent commit on this same +branch, `edcb8ba7` (`fix(#627): restore row-width invariant for zero-key rows...`, +hereafter "the zero-key-guard commit"), added a guard to the exact `json.row` +fallback arm this evidence exercises: a zero-key `{}` row arriving before columns +are established is now declined rather than used to "establish" zero-length +columns (see that arm's comment in `src/core/stream.ts` for the invariant it +protects). Since acceptance criterion 7 is evidence-backed, this document must +attest to the code at whichever commit is currently checked out, not only the +commit the live capture happened to run against — so the distinction matters: + +* The zero-key-guard commit **does not require re-running the live Docker capture** + to validate: it is provably inert against the two `*.ndjson` captures committed + here, because both carry only 3-key rows (`id`/`precise`/`lexical`) in every `row` + line — never a zero-key `{}` row — so `keys.length === 0` never fires for this + corpus, and replaying these exact bytes through the zero-key-guard commit's + `stream.ts` reproduces the committed `*.normalized.json` files' structural + content exactly. +* That replay is not just asserted here — it is a real, permanent regression test: + `tests/unit/evidence-627-replay.test.ts` feeds both committed raw `*.ndjson` + captures through the production `streamLines()` -> `applyStreamLine()` path at + whatever checkout/HEAD `npm test` is run against, and asserts the result matches + the committed `*.normalized.json` files exactly (and that the guard's zero-key + branch never fires on this corpus). That test passes at the zero-key-guard commit + and every commit after it that leaves the corpus/guard behavior unchanged — it + validates whichever checkout is currently under test, not one SHA pinned as + permanently final. + +In short: the pinned image digests, `SELECT version()` assertions, and raw +`*.ndjson` bytes below are provenance of the **live capture**, dated to `036760a`; +the **decoder/accumulator behavior** they were run through is attested by the +committed replay test above against whichever commit is currently checked out — +not frozen at the live-capture commit, and not pinned to any single SHA as a +permanently "final" head. + +## Execution date + +2026-08-12 (UTC). + +## Environment + +* `DOCKER_NETWORK=iso-altinity` (created for this run; both containers attached to it). +* Docker server 29.4.0. + +## Images and asserted versions + +| Row | Image digest | Asserted `SELECT version()` | +|---|---|---| +| ClickHouse OSS | `clickhouse/clickhouse-server@sha256:1ffa82edee000a42c09313bd9f1293d94c570aee74babc1b3ca9983a35fa597b` | `24.8.14.39` | +| Altinity Stable | `altinity/clickhouse-server@sha256:d0c456453ddc5220bc96e37c9b1f81eb210ca22fc0d6877dc9e71722ff43fa8f` | `24.8.14.10547.altinitystable` | + +Both digests pulled cleanly on the first `docker pull` attempt (no registry-outage +retries were needed for this run). + +## Query + +```sql +SELECT + if(number = 0, 'row-a', 'row-b') AS id, + if( + number = 0, + '9007199254740993.12345678901234567890', + '-9007199254740993.00000000000000000001' + ) AS precise, + if(number = 0, '001.2300', '0002') AS lexical +FROM numbers(2) +ORDER BY number +``` + +Requested with `default_format=JSONStringsEachRowWithProgress` over each container's +mapped `8123` port. + +## Verification commands + +A one-off verifier (not committed — built fresh in `$TMPDIR`, per the plan) ran, per image: + +```sh +docker pull "$IMAGE" +docker run -d --rm --name "$NAME" --network="$DOCKER_NETWORK" \ + -p 127.0.0.1::8123 -e CLICKHOUSE_SKIP_USER_SETUP=1 "$IMAGE" +curl -fsS --data-binary 'SELECT version()' "http://127.0.0.1:${PORT}/" +curl -fsS --data-binary "$QUERY" \ + "http://127.0.0.1:${PORT}/?default_format=JSONStringsEachRowWithProgress" \ + > "$KEY.ndjson" +node verify-627.mjs "$KEY.ndjson" > "$KEY.normalized.json" +``` + +`verify-627.mjs` is an esbuild bundle of a small Node script that feeds the raw captured +bytes through the REAL production decoder/accumulator — +`@altinity/clickhouse-http`'s `streamLines()` (unmodified protocol mechanics) followed by +`src/core/stream.ts`'s `newResult()`/`applyStreamLine()` (the #627 result-policy fallback +under test) — then asserts the result against independently declared expected literals +(not read back from the result itself) before printing the normalized JSON committed here +as `*.normalized.json`. + +## Independently declared expected columns/rows + +```json +{ + "columns": [ + {"name":"id","type":""}, + {"name":"precise","type":""}, + {"name":"lexical","type":""} + ], + "rows": [ + ["row-a","9007199254740993.12345678901234567890","001.2300"], + ["row-b","-9007199254740993.00000000000000000001","0002"] + ] +} +``` + +## Results + +Both rows pass every assertion in the plan's live pass/fail definition: + +| Assertion | OSS 24.8.14.39 | Altinity Stable 24.8.14.10547 | +|---|---|---| +| Exact image digest pulled | yes | yes | +| `SELECT version()` matches expected | yes | yes | +| Raw captured stream reaches EOF with **no** `meta` record | confirmed — see `*.ndjson`; no `"meta"` line present | confirmed — see `*.ndjson`; no `"meta"` line present | +| Production `streamLines()` parses the actual captured bytes | yes (no parse error) | yes (no parse error) | +| Production `applyStreamLine()` yields the exact 3 expected columns, all `type: ''` | yes | yes | +| Both rows match the independently declared literals exactly (leading/trailing lexical precision preserved) | yes | yes | +| `result.error === null` | yes | yes | +| `result.capped === false` | yes | yes | + +See `oss-24.8.14.39.normalized.json` / `altinity-24.8.14.10547.normalized.json` for the +exact printed `{metaSeen, columns, rows, error, capped}` object from each run, and +`oss-24.8.14.39.ndjson` / `altinity-24.8.14.10547.ndjson` for the exact raw bytes each +server sent (progress lines, then two `row` lines, never a `meta` line). + +The separate accumulator-to-grid unit regression +(`tests/unit/grid-render.test.ts`'s "faithfully renders a meta-less accumulated result +(#627)..." case) independently confirms these same literals render unchanged as Table +cell text through the real `renderGrid()` — see that test for the DOM-level proof; it is +not re-run against these captured bytes here, since it already covers the +`StreamResult -> renderGrid` half of the pipeline with its own independently declared +literals. + +`tests/unit/evidence-627-replay.test.ts` is the ongoing, head-tracking counterpart +to the one-off verifier above: it replays these exact two committed `*.ndjson` +files through the real `streamLines()`/`applyStreamLine()` production path at +whatever commit `npm test` runs against, and fails the suite if that ever stops +matching the committed `*.normalized.json` files — so this evidence's attestation +does not silently go stale the next time `src/core/stream.ts`'s fallback changes. + +## Transient pull retries + +None needed for this run — both exact digests pulled successfully on the first attempt. + +## Files in this directory + +```text +README.md this file +oss-24.8.14.39.ndjson raw captured JSONStringsEachRowWithProgress bytes (OSS) +oss-24.8.14.39.normalized.json production decoder/accumulator output (OSS) +altinity-24.8.14.10547.ndjson raw captured bytes (Altinity Stable) +altinity-24.8.14.10547.normalized.json production decoder/accumulator output (Altinity Stable) +``` + +The temporary verifier source/bundle used to produce these files was **not** committed, +per the plan — only its output. diff --git a/docs/evidence/627/altinity-24.8.14.10547.ndjson b/docs/evidence/627/altinity-24.8.14.10547.ndjson new file mode 100644 index 00000000..865582f3 --- /dev/null +++ b/docs/evidence/627/altinity-24.8.14.10547.ndjson @@ -0,0 +1,5 @@ +{"progress":{"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"0","result_rows":"0","result_bytes":"0","elapsed_ns":"0"}} +{"progress":{"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"2","result_rows":"0","result_bytes":"0","elapsed_ns":"0"}} +{"progress":{"read_rows":"2","read_bytes":"16","written_rows":"0","written_bytes":"0","total_rows_to_read":"2","result_rows":"0","result_bytes":"0","elapsed_ns":"0"}} +{"row":{"id":"row-a","precise":"9007199254740993.12345678901234567890","lexical":"001.2300"}} +{"row":{"id":"row-b","precise":"-9007199254740993.00000000000000000001","lexical":"0002"}} diff --git a/docs/evidence/627/altinity-24.8.14.10547.normalized.json b/docs/evidence/627/altinity-24.8.14.10547.normalized.json new file mode 100644 index 00000000..ffd9b10a --- /dev/null +++ b/docs/evidence/627/altinity-24.8.14.10547.normalized.json @@ -0,0 +1,31 @@ +{ + "metaSeen": false, + "columns": [ + { + "name": "id", + "type": "" + }, + { + "name": "precise", + "type": "" + }, + { + "name": "lexical", + "type": "" + } + ], + "rows": [ + [ + "row-a", + "9007199254740993.12345678901234567890", + "001.2300" + ], + [ + "row-b", + "-9007199254740993.00000000000000000001", + "0002" + ] + ], + "error": null, + "capped": false +} diff --git a/docs/evidence/627/oss-24.8.14.39.ndjson b/docs/evidence/627/oss-24.8.14.39.ndjson new file mode 100644 index 00000000..865582f3 --- /dev/null +++ b/docs/evidence/627/oss-24.8.14.39.ndjson @@ -0,0 +1,5 @@ +{"progress":{"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"0","result_rows":"0","result_bytes":"0","elapsed_ns":"0"}} +{"progress":{"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"2","result_rows":"0","result_bytes":"0","elapsed_ns":"0"}} +{"progress":{"read_rows":"2","read_bytes":"16","written_rows":"0","written_bytes":"0","total_rows_to_read":"2","result_rows":"0","result_bytes":"0","elapsed_ns":"0"}} +{"row":{"id":"row-a","precise":"9007199254740993.12345678901234567890","lexical":"001.2300"}} +{"row":{"id":"row-b","precise":"-9007199254740993.00000000000000000001","lexical":"0002"}} diff --git a/docs/evidence/627/oss-24.8.14.39.normalized.json b/docs/evidence/627/oss-24.8.14.39.normalized.json new file mode 100644 index 00000000..ffd9b10a --- /dev/null +++ b/docs/evidence/627/oss-24.8.14.39.normalized.json @@ -0,0 +1,31 @@ +{ + "metaSeen": false, + "columns": [ + { + "name": "id", + "type": "" + }, + { + "name": "precise", + "type": "" + }, + { + "name": "lexical", + "type": "" + } + ], + "rows": [ + [ + "row-a", + "9007199254740993.12345678901234567890", + "001.2300" + ], + [ + "row-b", + "-9007199254740993.00000000000000000001", + "0002" + ] + ], + "error": null, + "capped": false +} diff --git a/src/core/stream.ts b/src/core/stream.ts index a171d071..3062da79 100644 --- a/src/core/stream.ts +++ b/src/core/stream.ts @@ -1,12 +1,20 @@ // Pure result accumulator for ClickHouse's JSONStringsEachRowWithProgress // streaming format. Each newline-delimited JSON object is one of: -// { meta: [{name,type}, ...] } — column headers (once, first) +// { meta: [{name,type}, ...] } — column headers (usually first, but may +// be absent entirely — see below) // { row: { col: value, ... } } — one data row // { progress: {...} } — incremental progress stats // { exception: "..." } — server-side error // `applyStreamLine` folds one parsed object into a mutable result; keeping it // pure (no fetch, no DOM) makes the streaming parser fully unit-testable. // +// Issue #627 — ClickHouse 24.8 and earlier streams omit `meta` entirely for +// ordinary queries. When the first `row` line arrives before any columns are +// established, `applyStreamLine` establishes name-only columns from that +// row's object keys, with the unknown-type sentinel `type: ''` (never a +// value-based type guess). This is SQL Browser result policy, not protocol +// parsing — see the fallback in the `json.row` arm below. +// // Issue #630 Phase 3 — the canonical progress-line wire type (`StreamLine`) // and the generic stream/exception-parsing primitives (`splitBuffer`, // `parseExceptionText`, `ExceptionFrame`, `findExceptionFrame`) moved to @@ -23,7 +31,10 @@ // imports the package type either — it narrows the fields it needs from the // open record instead. -/** One streamed result column, as reported by a `{meta}` line. */ +/** One streamed result column, as reported by a `{meta}` line — or, when a + * stream omits `meta` entirely (ClickHouse 24.8 and earlier, issue #627), + * a name-only column established from the first `row`'s object keys, with + * `type: ''` as the unknown-type sentinel. */ export interface StreamColumn { name: string; type: string; @@ -87,6 +98,13 @@ export function newResult(fmt: string, rowLimit = 0): StreamResult { * (`meta`/`row`/`progress`/`exception`), narrowing each locally, without * re-exporting a second declared wire contract. Unrecognized records are a * no-op — the module doc above lists the four shapes a line can take. + * + * Issue #627: when a `row` line arrives before `result.columns` has been + * established (no `meta` line ever arrived, or none will), this is SQL + * Browser result policy — not protocol parsing — establishing name-only + * columns from that first row's object keys (`type: ''`, no value-based + * inference) so meta-less ClickHouse 24.8-and-earlier streams still produce + * a usable Table result instead of silently discarding every row. */ export function applyStreamLine(json: Record, result: StreamResult): StreamResult { if (json.meta) { @@ -94,6 +112,25 @@ export function applyStreamLine(json: Record, result: StreamRes result.columns = meta.map((m) => ({ name: m.name, type: m.type })); } else if (json.row) { const row = json.row as Record; + const keys = Object.keys(row); + + if (result.columns.length === 0) { + // A zero-key row (`{}`) arriving before columns are established is + // deliberately never stored, and never used to "establish" columns: + // establishing zero-length columns from it would make + // `result.columns.length === 0` mean both "not yet established" and + // "established with zero columns" — indistinguishable sentinels. A + // later real row would then re-establish columns out from under this + // row's already-pushed (necessarily zero-width) entry, breaking the + // invariant this module must hold for its whole lifetime: every stored + // row's width equals `result.columns.length`. A zero-key row carries no + // values, so declining to store it discards no query data. Not + // reachable from a real ClickHouse SELECT (a projection always has at + // least one column) — issue #627. + if (keys.length === 0) return result; + result.columns = keys.map((name) => ({ name, type: '' })); + } + // At the cap: drop the row (block-boundary overage from `break`) and flag it. if (result.rowLimit > 0 && result.rows.length >= result.rowLimit) { result.capped = true; diff --git a/tests/unit/chart-data.test.ts b/tests/unit/chart-data.test.ts index df5d6a21..efb4fbcb 100644 --- a/tests/unit/chart-data.test.ts +++ b/tests/unit/chart-data.test.ts @@ -76,6 +76,17 @@ describe('autoChart', () => { expect(autoChart([{ name: 'carrier', type: 'String' }, { name: 'monthly_total', type: 'Float64' }])) .toEqual({ type: 'hbar', x: 0, y: [1], series: null }); }); + // #627: a meta-less ClickHouse 24.8 stream carries `type: ''` for every + // column — no value-based inference means there is no measure, so auto + // chart detection must fail closed to null rather than guessing a numeric + // axis from the column name/values. + it('a meta-less result (every column type "") never auto-charts', () => { + expect(chartRole({ name: 'total', type: '' })).toBe('category'); + expect(autoChart([ + { name: 'when', type: '' }, + { name: 'total', type: '' }, + ])).toBeNull(); + }); }); describe('schemaKey', () => { diff --git a/tests/unit/evidence-627-replay.test.ts b/tests/unit/evidence-627-replay.test.ts new file mode 100644 index 00000000..1140a2a0 --- /dev/null +++ b/tests/unit/evidence-627-replay.test.ts @@ -0,0 +1,93 @@ +// The committed live-capture evidence in `docs/evidence/627/*.ndjson` / +// `*.normalized.json` attests to decoder/accumulator behavior. That +// attestation has to stay bound to the code currently under test, not to +// the commit the capture happened to run against — otherwise a later +// change to `streamLines()`/`applyStreamLine()` would silently invalidate +// the committed evidence instead of failing this suite. +// +// This spec is what enforces that binding: it replays the two raw +// committed NDJSON captures through the REAL production path at whatever +// checkout runs it — the package's `streamLines()` (unmodified protocol +// mechanics) followed by `src/core/stream.ts`'s +// `newResult()`/`applyStreamLine()` (the #627 result-policy fallback, +// including its zero-key-row guard) — and requires the result to still be +// deeply/structurally equal to the committed `*.normalized.json` files +// (deep value equality on the parsed objects, not a byte-for-byte file +// comparison). +// +// The zero-key-row guard `edcb8ba` added to that fallback arm is inert for +// this corpus: both committed captures carry only 3-key rows +// (id/precise/lexical) in every `row` line, never a zero-key `{}` row, so +// the guard's `keys.length === 0` branch never fires here. The per-case +// column assertions below are the evidence for that inertness claim, not +// just an assertion of it in prose. +import { describe, expect, it } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { streamLines } from '@altinity/clickhouse-http'; +import { newResult, applyStreamLine, type StreamResult } from '../../src/core/stream.js'; + +// `join(dirname(...), ...)`, not `resolve` — this repo's ambient `node:path` +// shim (`tests/types/node-fs-url.d.ts`, ADR-0002's no-@types/node decision) +// declares only `dirname`/`join`. +const evidenceDir = join(dirname(fileURLToPath(import.meta.url)), '..', '..', 'docs', 'evidence', '627'); + +/** Turn a whole NDJSON string into a single-chunk byte stream — sufficient + * here since this replay only needs `streamLines`' line-splitting/parsing, + * not its chunk-boundary reassembly (already covered directly in + * `packages/clickhouse-http/test/unit/progress-stream.test.ts`). */ +function wholeBodyStream(text: string): ReadableStream { + const bytes = new TextEncoder().encode(text); + let sent = false; + return new ReadableStream({ + pull(controller) { + if (!sent) { + sent = true; + controller.enqueue(bytes); + } else { + controller.close(); + } + }, + }); +} + +/** Replay one raw captured NDJSON body through the real production + * streamLines() -> applyStreamLine() path, at the current head. */ +async function replay(ndjson: string): Promise<{ metaSeen: boolean; result: StreamResult }> { + const result = newResult('Table'); + let metaSeen = false; + await streamLines(wholeBodyStream(ndjson), { + onLine: (line) => { + if (line.meta) metaSeen = true; + applyStreamLine(line, result); + }, + }); + return { metaSeen, result }; +} + +describe('#627 evidence replay — production path reproduces the committed captures', () => { + it.each([ + ['oss-24.8.14.39', 'ClickHouse OSS 24.8.14.39'], + ['altinity-24.8.14.10547', 'Altinity Stable 24.8.14.10547.altinitystable'], + ])('%s (%s): replaying the committed raw capture reproduces the committed normalized output exactly', async (key) => { + const rawNdjson = readFileSync(join(evidenceDir, `${key}.ndjson`), 'utf8'); + const expected = JSON.parse(readFileSync(join(evidenceDir, `${key}.normalized.json`), 'utf8')); + + const { metaSeen, result } = await replay(rawNdjson); + + // No `meta` line in either committed capture — the whole point of #627. + expect(metaSeen).toBe(false); + expect(metaSeen).toBe(expected.metaSeen); + expect(result.columns).toEqual(expected.columns); + expect(result.rows).toEqual(expected.rows); + expect(result.error).toBe(expected.error); + expect(result.capped).toBe(expected.capped); + + // The guard `edcb8ba` added (declining to establish columns/store a + // zero-key `{}` row) never fires against this corpus: every captured + // `row` line carries exactly the 3 keys id/precise/lexical, never zero. + expect(result.columns).toHaveLength(3); + expect(result.columns.map((c) => c.name)).toEqual(['id', 'precise', 'lexical']); + }); +}); diff --git a/tests/unit/grid-render.test.ts b/tests/unit/grid-render.test.ts index 285127e1..37421b35 100644 --- a/tests/unit/grid-render.test.ts +++ b/tests/unit/grid-render.test.ts @@ -6,6 +6,7 @@ import { import type { GridColumn, RenderGridArgs, Widths } from '../../src/ui/grid-render.js'; import type { ResultSort } from '../../src/state.js'; import { h } from '../../src/ui/dom.js'; +import { newResult, applyStreamLine } from '../../src/core/stream.js'; const click = (el: Element) => el.dispatchEvent(new Event('click', { bubbles: true })); @@ -104,6 +105,48 @@ describe('renderGrid', () => { expect(capped.querySelectorAll('tbody tr')).toHaveLength(1); expect(capped.textContent).toContain('+ 1 more rows truncated'); }); + it('faithfully renders a meta-less accumulated result (#627) with no fabricated type and no numeric coercion', () => { + const result = newResult('Table'); + applyStreamLine({ + row: { + id: 'row-a', + precise: '9007199254740993.12345678901234567890', + lexical: '001.2300', + }, + }, result); + applyStreamLine({ + row: { + id: 'row-b', + precise: '-9007199254740993.00000000000000000001', + lexical: '0002', + }, + }, result); + + const el = renderGrid(gridArgs({ columns: result.columns, rows: result.rows })); + + const headers = el.querySelectorAll('thead th'); + // Header type titles are empty — no fabricated String/Decimal/UInt64 etc. + expect(headers[1].getAttribute('title')).toBe(''); + expect(headers[2].getAttribute('title')).toBe(''); + expect(headers[3].getAttribute('title')).toBe(''); + + const rows = el.querySelectorAll('tbody tr'); + const row0 = rows[0].querySelectorAll('td.cell'); + const row1 = rows[1].querySelectorAll('td.cell'); + + // Independently declared expected literals — not read back from `result`. + expect(row0[0].textContent).toBe('row-a'); + expect(row0[1].textContent).toBe('9007199254740993.12345678901234567890'); + expect(row0[2].textContent).toBe('001.2300'); + expect(row1[0].textContent).toBe('row-b'); + expect(row1[1].textContent).toBe('-9007199254740993.00000000000000000001'); + expect(row1[2].textContent).toBe('0002'); + + // Unknown-type cells never get the numeric `.num` class. + expect(el.querySelectorAll('td.num')).toHaveLength(0); + expect(el.textContent).not.toMatch(/\b(String|Decimal|UInt64|Int\d+|Float\d+)\b/); + }); + it('reapplies stored widths (fixed layout) on render', () => { const el = renderGrid(gridArgs({ widths: { idx: 36, 0: 90, 1: 70 } })); const table = el.querySelector('.res-table')!; diff --git a/tests/unit/kpi.test.ts b/tests/unit/kpi.test.ts index 46b3afeb..a0f3011e 100644 --- a/tests/unit/kpi.test.ts +++ b/tests/unit/kpi.test.ts @@ -135,4 +135,21 @@ describe('readKpiFields', () => { expect(tupleString.diagnostics.map((item) => item.code)).toEqual(['kpi-server-named-tuple-unsupported', 'kpi-no-eligible-fields']); expect(tupleString.diagnostics[0].message).toContain('ClickHouse 24.3'); }); + // #627: a meta-less ClickHouse 24.8 stream reports every column as + // `type: ''`. Numeric-looking scalar values and tuple-shaped objects must + // never be accepted as KPI fields solely from their values — parsing/ + // numeric-eligibility both fail closed, and the existing diagnostics fire + // instead of a fabricated numeric type or a dereference. + it('never accepts a KPI field from values alone when column type is "" (meta-less #627 result)', () => { + const columns = [ + { name: 'requests', type: '' }, + { name: 'availability', type: '' }, + { name: 'region', type: '' }, + ]; + const row = [42, { value: '99.95', delta: '0.1' }, 'EU']; + expect(() => readKpiFields({ columns, row, rowCount: 1 })).not.toThrow(); + const out = readKpiFields({ columns, row, rowCount: 1 }); + expect(out.items).toEqual([]); + expect(out.diagnostics.map((d) => d.code)).toEqual(['kpi-unsupported-field', 'kpi-unsupported-field', 'kpi-unsupported-field', 'kpi-no-eligible-fields']); + }); }); diff --git a/tests/unit/logs.test.ts b/tests/unit/logs.test.ts index b2e5c242..11cd2575 100644 --- a/tests/unit/logs.test.ts +++ b/tests/unit/logs.test.ts @@ -24,6 +24,17 @@ describe('detectLogsView', () => { expect(detectLogsView([{ name: 'ts', type: 'DateTime' }, { name: 'host', type: 'String' }])).toBeNull(); expect(detectLogsView(undefined)).toBeNull(); }); + // #627: a meta-less ClickHouse 24.8 stream reports `type: ''` for every + // column. Even highly suggestive names (event_time/message/level) must not + // manufacture typed log semantics from names alone — the type regexes all + // reject '', so detection fails closed to null. + it('highly suggestive column names with type "" never qualify as a logs shape (meta-less #627 result)', () => { + expect(detectLogsView([ + { name: 'event_time', type: '' }, + { name: 'message', type: '' }, + { name: 'level', type: '' }, + ])).toBeNull(); + }); it('strips nested Nullable(LowCardinality(...)) wrappers before the type check', () => { const shape = detectLogsView([ { name: 'ts', type: 'Nullable(DateTime)' }, diff --git a/tests/unit/panel-cfg.test.ts b/tests/unit/panel-cfg.test.ts index ae639b15..8196baf4 100644 --- a/tests/unit/panel-cfg.test.ts +++ b/tests/unit/panel-cfg.test.ts @@ -99,6 +99,26 @@ describe('resolveLogsShape', () => { expect(resolveLogsShape({ type: 'logs' }, strCols)).toBeNull(); expect(resolveLogsShape({ type: 'logs' }, [])).toBeNull(); }); + it('#627: an explicit cfg resolves by column NAME alone against meta-less columns (type: \'\'), unlike convention detection which fails closed on \'\'', () => { + // A meta-less ClickHouse 24.8 stream (#627) establishes columns with the + // unknown-type sentinel `type: ''`. Convention detection (findTimeColumn/ + // findMsgColumn) type-checks via TIME_TYPE_RE/MSG_TYPE_RE and fails closed + // on '' — but an *explicit* cfg.time/cfg.msg resolves purely by name + // (idxOf), never consulting column.type, so it still succeeds here. This + // is intended, pre-existing name-based-path behavior (not a #627 + // regression): #627's degraded-functionality contract permits rendering + // against unknown-typed columns, it only forbids discarding row values or + // fabricating a type — this path does neither. + const metalessCols = [ + { name: 'event_time', type: '' }, + { name: 'message', type: '' }, + ]; + expect(resolveLogsShape({ type: 'logs', time: 'event_time', msg: 'message' }, metalessCols)) + .toEqual({ time: 0, msg: 1, level: null, extras: [] }); + // Convention detection alone (no explicit names) fails closed on the same + // meta-less columns — the contrast this test pins. + expect(resolveLogsShape({ type: 'logs' }, metalessCols)).toBeNull(); + }); }); describe('panelCfgValid', () => { @@ -169,6 +189,15 @@ describe('autoPanel', () => { expect(autoPanel(strCols).cfg).toEqual({ type: 'table' }); expect(autoPanel([]).cfg).toEqual({ type: 'table' }); }); + // #627: a meta-less ClickHouse 24.8 result reports `type: ''` for every + // column, even when names/values plausibly resemble logs (event_time/ + // message) or a KPI (single numeric-looking row). All three typed paths + // must fail closed on empty types, leaving the universal Table fallback. + it('falls back to Table for a one-row meta-less result whose names/values resemble logs/KPI/chart data', () => { + const cols = [{ name: 'event_time', type: '' }, { name: 'message', type: '' }, { name: 'requests', type: '' }]; + const out = autoPanel({ columns: cols, rows: [['2026-01-01 00:00:00', 'boom', 42]] }); + expect(out.cfg).toEqual({ type: 'table' }); + }); }); describe('switchPanelType', () => { diff --git a/tests/unit/results.test.ts b/tests/unit/results.test.ts index fd1831ee..9250e9a2 100644 --- a/tests/unit/results.test.ts +++ b/tests/unit/results.test.ts @@ -504,6 +504,18 @@ describe('openCellDetail', () => { expect(qs(panel, '.cd-type')).toBeNull(); expect(qs(panel, '.cd-pre').textContent).toBe(''); }); + // #627: a meta-less ClickHouse 24.8 stream column carries `type: ''` — the + // unknown-type sentinel, not "no type at all". The detail drawer must not + // throw, must keep the full value visible, and must never synthesize a + // type label for it. + it('type: "" (meta-less #627 column) → no exception, full value visible, no type chip, no synthetic type text', () => { + const app = makeApp(); + expect(() => openCellDetail(app, 'precise', '', '9007199254740993.12345678901234567890')).not.toThrow(); + const panel = qs(app.dom.inspectorHost, '.cd-panel'); + expect(qs(panel, '.cd-type')).toBeNull(); + expect(qs(panel, '.cd-pre').textContent).toBe('9007199254740993.12345678901234567890'); + expect(panel.textContent).not.toMatch(/\b(String|Decimal|UInt64|Int\d+|Float\d+)\b/); + }); it('HTML value → Rendered (sandboxed iframe srcdoc) ↔ Source toggle', () => { const app = makeApp(); openCellDetail(app, 'html', 'String', 'hi'); diff --git a/tests/unit/spec-editor.test.ts b/tests/unit/spec-editor.test.ts index 1b04091c..46366e5b 100644 --- a/tests/unit/spec-editor.test.ts +++ b/tests/unit/spec-editor.test.ts @@ -407,6 +407,25 @@ describe('Spec editor adapter', () => { .toMatchObject({ documentation: 'x' }); expect(createSpecCompletionSources().resultColumnIndexes({ context: { tab: { lastSuccessfulResultColumns: [{ name: 'x' }] } } })[0]) .toMatchObject({ detail: 'x', documentation: 'x' }); + // #627: a meta-less ClickHouse 24.8 column carries the explicit unknown-type + // sentinel `type: ''`, not an absent `type` field — the falsy check above + // must treat it identically: name/value completion still appears, and + // documentation/detail stay name-only with no separator or fabricated type. + const emptyTypeColumn = { name: 'unknown', type: '' }; + const emptyTypeResultColumn = createSpecCompletionSources().resultColumns({ + context: { tab: { lastSuccessfulResultColumns: [emptyTypeColumn] } }, + })[0]; + expect(emptyTypeResultColumn).toMatchObject({ + label: 'unknown', value: 'unknown', detail: '', documentation: 'unknown', + }); + expect(emptyTypeResultColumn.documentation).not.toContain('·'); + const emptyTypeIndexColumn = createSpecCompletionSources().resultColumnIndexes({ + context: { tab: { lastSuccessfulResultColumns: [emptyTypeColumn] } }, + })[0]; + expect(emptyTypeIndexColumn).toMatchObject({ + label: '0', value: 0, detail: 'unknown', documentation: 'unknown', + }); + expect(emptyTypeIndexColumn.documentation).not.toContain('·'); expect(createSpecCompletionSources().queryParameters({ context: { tab: { sqlDraft: '' } } })).toEqual([]); expect(createSpecCompletionSources().queryParameters({ context: { tab: { sqlDraft: 'CREATE VIEW v AS SELECT {ddl_only:String}, {mixed:String}; SELECT {mixed:UInt8}', diff --git a/tests/unit/stream.test.ts b/tests/unit/stream.test.ts index 10892817..a39d54e7 100644 --- a/tests/unit/stream.test.ts +++ b/tests/unit/stream.test.ts @@ -79,6 +79,111 @@ describe('applyStreamLine', () => { }); }); +describe('applyStreamLine — meta-less streams (#627)', () => { + it('establishes name-only columns from the first row and preserves values through a true EOF with no meta', () => { + const r = newResult('Table'); + applyStreamLine({ + row: { + id: 'row-a', + precise: '9007199254740993.12345678901234567890', + label: '001.2300', + }, + }, r); + // No `meta` line is ever applied — this is the real EOF-with-no-meta failure mode. + + expect(r.columns).toEqual([ + { name: 'id', type: '' }, + { name: 'precise', type: '' }, + { name: 'label', type: '' }, + ]); + expect(r.rows).toEqual([[ + 'row-a', + '9007199254740993.12345678901234567890', + '001.2300', + ]]); + }); + + it('keeps the first row\'s key order stable even when later rows insert keys in a different order', () => { + const r = newResult('Table'); + applyStreamLine({ row: { alpha: 'a1', beta: 'b1', gamma: 'c1' } }, r); + applyStreamLine({ row: { gamma: 'c2', alpha: 'a2', beta: 'b2' } }, r); + + expect(r.columns).toEqual([ + { name: 'alpha', type: '' }, + { name: 'beta', type: '' }, + { name: 'gamma', type: '' }, + ]); + expect(r.rows).toEqual([ + ['a1', 'b1', 'c1'], + ['a2', 'b2', 'c2'], + ]); + }); + + it('never fabricates a value-based type for synthetic-looking meta-less values', () => { + const r = newResult('Table'); + applyStreamLine({ + row: { + big_int: '9223372036854775807123', // beyond JS safe integer range + decimal: '001.2300', + date_time: '2024-01-02 03:04:05', + uuid: '550e8400-e29b-41d4-a716-446655440000', + bool: 'true', + enum_like: 'ACTIVE', + }, + }, r); + + for (const column of r.columns) { + expect(column.type).toBe(''); + } + }); + + it('caps meta-less rows the same way as meta-first rows', () => { + const r = newResult('Table', 1); + applyStreamLine({ row: { a: '1' } }, r); + applyStreamLine({ row: { a: '2' } }, r); + + expect(r.columns).toEqual([{ name: 'a', type: '' }]); + expect(r.rows).toEqual([['1']]); + expect(r.capped).toBe(true); + }); + + it('keeps meta-first columns and real types authoritative even when metadata order differs from row-object order', () => { + const r = newResult('Table'); + applyStreamLine({ meta: [{ name: 'b', type: 'String' }, { name: 'a', type: 'UInt64' }] }, r); + applyStreamLine({ row: { a: '1', b: 'x' } }, r); + + expect(r.columns).toEqual([{ name: 'b', type: 'String' }, { name: 'a', type: 'UInt64' }]); + expect(r.rows).toEqual([['x', '1']]); + }); + + it('declines to establish columns or store a zero-key row, so a later real row leaves every stored row width-matched to result.columns', () => { + const r = newResult('Table'); + applyStreamLine({ row: {} }, r); + applyStreamLine({ row: { host: 'srv-7', status: 'ok' } }, r); + + expect(r.columns).toEqual([ + { name: 'host', type: '' }, + { name: 'status', type: '' }, + ]); + expect(r.rows).toEqual([['srv-7', 'ok']]); + for (const storedRow of r.rows) { + expect(storedRow.length).toBe(r.columns.length); + } + }); + + it('preserves progress/exception folding once meta-less columns are established', () => { + const r = newResult('Table'); + applyStreamLine({ row: { a: '1' } }, r); + applyStreamLine({ progress: { read_rows: '50', read_bytes: '500', elapsed_ns: '1000', total_rows_to_read: '100' } }, r); + expect(r.progress).toEqual({ rows: 50, bytes: 500, elapsed_ns: 1000, total_rows: 100 }); + expect(r.pct).toBe(50); + + applyStreamLine({ exception: 'boom' }, r); + expect(r.error).toBe('boom'); + expect(r.columns).toEqual([{ name: 'a', type: '' }]); + }); +}); + describe('parseErrorPos', () => { it('returns the 0-based caret offset from "position N" (1-based in the message)', () => { expect(parseErrorPos('Syntax error: failed at position 18 (BEWEEN): …')).toBe(17); diff --git a/tests/unit/variable-options.test.ts b/tests/unit/variable-options.test.ts index aa0b1c1d..1198f5e6 100644 --- a/tests/unit/variable-options.test.ts +++ b/tests/unit/variable-options.test.ts @@ -354,6 +354,16 @@ describe('validateOptionColumns', () => { expect(validateOptionColumns([{ name: 'v', type: 'String' }, { name: 'l', type: 'String' }])) .toBeNull(); }); + + // #627: a meta-less ClickHouse 24.8 probe response reports `type: ''` for + // both columns. `''` must not be treated as String — it fails the existing + // type check, with the existing diagnostic, not a throw and not acceptance. + it('rejects type: "" the same as any other non-String type (meta-less #627 probe response)', () => { + expect(() => validateOptionColumns([{ name: 'v', type: '' }, { name: 'l', type: '' }])).not.toThrow(); + const found = validateOptionColumns([{ name: 'v', type: '' }, { name: 'l', type: '' }])!; + expect(found.code).toBe('variable-option-column-type'); + expect(found.message).toContain('this returns and '); + }); }); describe('validateOptionRowCount', () => {