From fe19ab76394357837f23ee44ad0dd7ca814f23e4 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Wed, 12 Aug 2026 21:48:29 +0200 Subject: [PATCH 1/7] fix(#627): preserve Table results from ClickHouse 24.8 meta-less streams applyStreamLine's json.row arm now establishes name-only StreamColumns (type: '') from the first row's object keys when no meta line has ever arrived, so ordinary ClickHouse 24.8-and-earlier query results reach the Table view instead of being silently discarded at EOF. No value-based type inference, no auxiliary metadata query, no App-shape change. Adds stream.ts regression coverage for the true-EOF-with-no-meta case, stable first-row column ordering, no synthetic type inference, meta-less row caps, and unchanged meta-first typed behavior, plus representative type: '' regressions across the exhaustive result-column consumer audit (grid-render, results/openCellDetail, chart-data/autoChart, kpi, logs/detectLogsView, variable-options, panel-cfg/autoPanel, and the spec-completion adapter's existing spec-editor test) confirming every typed consumer fails closed/generic rather than crashing or fabricating a type. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz --- src/core/stream.ts | 27 ++++++++- tests/unit/chart-data.test.ts | 11 ++++ tests/unit/grid-render.test.ts | 43 ++++++++++++++ tests/unit/kpi.test.ts | 17 ++++++ tests/unit/logs.test.ts | 11 ++++ tests/unit/panel-cfg.test.ts | 9 +++ tests/unit/results.test.ts | 12 ++++ tests/unit/spec-editor.test.ts | 19 ++++++ tests/unit/stream.test.ts | 90 +++++++++++++++++++++++++++++ tests/unit/variable-options.test.ts | 10 ++++ 10 files changed, 247 insertions(+), 2 deletions(-) diff --git a/src/core/stream.ts b/src/core/stream.ts index a171d071..00f6a813 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,11 @@ 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; + + if (result.columns.length === 0) { + result.columns = Object.keys(row).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/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..d3da2b5f 100644 --- a/tests/unit/panel-cfg.test.ts +++ b/tests/unit/panel-cfg.test.ts @@ -169,6 +169,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..938c10ea 100644 --- a/tests/unit/stream.test.ts +++ b/tests/unit/stream.test.ts @@ -79,6 +79,96 @@ 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('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', () => { From 036760afe565db89a43f112fa37db3bf7f192257 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Wed, 12 Aug 2026 21:56:40 +0200 Subject: [PATCH 2/7] docs(#627): document ClickHouse 24.8 limited support and reconcile ADR-0005 README replaces the stale #627 "render silently empty" gap paragraph with a new ClickHouse server compatibility section: 24.8 is limited support (query execution + Table results; typed-result features may degrade). docs/ARCHITECTURE.md and .wiki/Architecture.md describe the meta-first vs meta-less normalization now owned by core/stream.ts. .wiki/Decisions-and- Roadmap.md records #627 as resolved without reopening ADR-0005. docs/ADR-0005-clickhouse-web-client.md adds a post-decision "#627 production compatibility follow-up" note and corrects its now-false "does not mean 24.8 is newly supported" sentence, while ADR-0005 itself remains Rejected. docs/evidence/585/README.md drops the dead check:client-spike:evidence instruction (retired in #630 Phase 8) and cross-references the new docs/evidence/627/ live evidence; the historical #585 matrix itself is untouched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz --- .wiki/Architecture.md | 11 ++++++++++- .wiki/Decisions-and-Roadmap.md | 9 +++++++++ CHANGELOG.md | 7 +++++++ README.md | 16 ++++++++++++---- docs/ADR-0005-clickhouse-web-client.md | 20 ++++++++++++++++---- docs/ARCHITECTURE.md | 14 ++++++++++++++ docs/evidence/585/README.md | 21 ++++++++++++++++++++- 7 files changed, 88 insertions(+), 10 deletions(-) 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..15794845 100644 --- a/docs/ADR-0005-clickhouse-web-client.md +++ b/docs/ADR-0005-clickhouse-web-client.md @@ -147,10 +147,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 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`. From 40bd39a7aaf82f6a9601ddb443bb6683f5239669 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Wed, 12 Aug 2026 22:05:12 +0200 Subject: [PATCH 3/7] test(#627): commit live ClickHouse 24.8 verification evidence Both pinned historical 24.8 digests (OSS 24.8.14.39, Altinity Stable 24.8.14.10547.altinitystable) verified against the real production decoder/accumulator (packages/clickhouse-http's streamLines() + src/core/stream.ts's applyStreamLine()): neither raw stream ever emits a meta record, and the normalized {columns,rows,error,capped} exactly match independently declared expected literals, including full lexical/ Decimal-precision preservation. Raw captured ndjson + normalized JSON for both rows committed under docs/evidence/627/; the temporary verifier script itself was not committed, per plan. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz --- docs/evidence/627/README.md | 135 ++++++++++++++++++ .../627/altinity-24.8.14.10547.ndjson | 5 + .../altinity-24.8.14.10547.normalized.json | 31 ++++ docs/evidence/627/oss-24.8.14.39.ndjson | 5 + .../627/oss-24.8.14.39.normalized.json | 31 ++++ 5 files changed, 207 insertions(+) create mode 100644 docs/evidence/627/README.md create mode 100644 docs/evidence/627/altinity-24.8.14.10547.ndjson create mode 100644 docs/evidence/627/altinity-24.8.14.10547.normalized.json create mode 100644 docs/evidence/627/oss-24.8.14.39.ndjson create mode 100644 docs/evidence/627/oss-24.8.14.39.normalized.json diff --git a/docs/evidence/627/README.md b/docs/evidence/627/README.md new file mode 100644 index 00000000..bfcac6a0 --- /dev/null +++ b/docs/evidence/627/README.md @@ -0,0 +1,135 @@ +# 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 + +`036760afe565db89a43f112fa37db3bf7f192257` (branch `wip/627-metaless-stream-columns`) — +`src/core/stream.ts`'s meta-less fallback was introduced in this branch's first commit, +`fe19ab7` (`fix(#627): preserve Table results from ClickHouse 24.8 meta-less streams`), +and is unchanged since. + +## 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. + +## 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 +} From edcb8ba7cdeed3f57427c01ab241d98dcc1c22fa Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Wed, 12 Aug 2026 22:28:58 +0200 Subject: [PATCH 4/7] fix(#627): restore row-width invariant for zero-key rows, pin explicit-Logs-cfg coverage, fix ADR-0005 tense MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - src/core/stream.ts: a zero-key `{"row":{}}` line no longer "establishes" empty columns before real columns exist. That left columns.length===0 ambiguous between "not yet established" and "established with zero columns," so a later real row would re-establish columns out from under an already-pushed zero-width row, breaking the invariant that 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 >=1 column). Added a regression test driving the exact {}-then-real-row sequence; sabotage-checked (reverting the guard fails the new test) and restored. - tests/unit/panel-cfg.test.ts: pin that an explicit Logs cfg resolves time/msg by column NAME even against meta-less columns (type: ''), unlike convention detection which fails closed on ''. This is pre-existing, intended name-based-path behavior permitted by #627's degraded-functionality contract, not a production change — the gap was that it was untested. - docs/ADR-0005-clickhouse-web-client.md: three passages still asserted applyStreamLine's meta-less gap in the present tense; shifted the code-state claims to the past and pointed at the existing "#627 production compatibility follow-up" note. Historical evidence/results are unchanged; ADR-0005 remains Rejected. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz --- docs/ADR-0005-clickhouse-web-client.md | 24 +++++++++++++++--------- src/core/stream.ts | 16 +++++++++++++++- tests/unit/panel-cfg.test.ts | 20 ++++++++++++++++++++ tests/unit/stream.test.ts | 15 +++++++++++++++ 4 files changed, 65 insertions(+), 10 deletions(-) diff --git a/docs/ADR-0005-clickhouse-web-client.md b/docs/ADR-0005-clickhouse-web-client.md index 15794845..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 @@ -278,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 @@ -870,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/src/core/stream.ts b/src/core/stream.ts index 00f6a813..3062da79 100644 --- a/src/core/stream.ts +++ b/src/core/stream.ts @@ -112,9 +112,23 @@ 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) { - result.columns = Object.keys(row).map((name) => ({ name, type: '' })); + // 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. diff --git a/tests/unit/panel-cfg.test.ts b/tests/unit/panel-cfg.test.ts index d3da2b5f..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', () => { diff --git a/tests/unit/stream.test.ts b/tests/unit/stream.test.ts index 938c10ea..a39d54e7 100644 --- a/tests/unit/stream.test.ts +++ b/tests/unit/stream.test.ts @@ -156,6 +156,21 @@ describe('applyStreamLine — meta-less streams (#627)', () => { 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); From e94b7c506a0ccd6c4e6b339a048b4265206cd6e6 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Wed, 12 Aug 2026 22:59:09 +0200 Subject: [PATCH 5/7] fix(#627): address review pass 1 findings docs/evidence/627/README.md pinned the "Tested commit" to the live-capture SHA (036760a) and claimed src/core/stream.ts's meta-less fallback "is unchanged since" -- true when written, but the branch's final commit (edcb8ba) later added a zero-key-row guard to the exact json.row fallback arm this evidence exercises, making that blanket claim stale relative to the head acceptance criterion 7's evidence is meant to attest to. Correct the provenance text to distinguish the live-capture SHA from the final head, and add tests/unit/evidence-627-replay.test.ts: a permanent regression test that replays both committed raw NDJSON captures through the real streamLines() -> applyStreamLine() production path at whatever commit npm test runs against, proving the guard is inert against this corpus (no committed row is ever zero-key) without needing to re-run the live Docker capture. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz --- docs/evidence/627/README.md | 48 ++++++++++++-- tests/unit/evidence-627-replay.test.ts | 88 ++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 4 deletions(-) create mode 100644 tests/unit/evidence-627-replay.test.ts diff --git a/docs/evidence/627/README.md b/docs/evidence/627/README.md index bfcac6a0..4b093b7f 100644 --- a/docs/evidence/627/README.md +++ b/docs/evidence/627/README.md @@ -8,10 +8,42 @@ these two rows are pinned, immutable digests, not a maintained-forever regressio ## Tested commit -`036760afe565db89a43f112fa37db3bf7f192257` (branch `wip/627-metaless-stream-columns`) — -`src/core/stream.ts`'s meta-less fallback was introduced in this branch's first commit, -`fe19ab7` (`fix(#627): preserve Table results from ClickHouse 24.8 meta-less streams`), -and is unchanged since. +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 final head. A later commit on this same branch, +`edcb8ba7` (`fix(#627): restore row-width invariant for zero-key rows...`), 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 the branch's final head, not only the commit +the live capture happened to run against — so the distinction matters: + +* The guard **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 `edcb8ba7`'s `stream.ts` reproduces the + committed `*.normalized.json` files byte-for-byte. +* 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 commit `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 currently passes at `edcb8ba7` and every + commit after it that leaves the corpus/guard behavior unchanged. + +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 at the +branch's final head, `edcb8ba7`, via the committed replay test above — not frozen +at the live-capture commit. ## Execution date @@ -117,6 +149,14 @@ 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` (added in PR review pass 1, see "Tested +commit" above) 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 again 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. diff --git a/tests/unit/evidence-627-replay.test.ts b/tests/unit/evidence-627-replay.test.ts new file mode 100644 index 00000000..ed67e9d9 --- /dev/null +++ b/tests/unit/evidence-627-replay.test.ts @@ -0,0 +1,88 @@ +// Issue #627 — PR-review finding (pass 1): `docs/evidence/627/README.md` +// pinned its "Tested commit" to `036760a` and claimed `src/core/stream.ts`'s +// meta-less fallback "is unchanged since". That was true when written, but +// this branch's own later commit `edcb8ba` (`fix(#627): restore row-width +// invariant for zero-key rows...`) added a zero-key-row guard to the exact +// `json.row` fallback arm the evidence exercises — making the README's +// blanket claim stale/false relative to the branch's final head. +// +// This spec is the promised replay: it feeds the two RAW committed NDJSON +// captures through the REAL production path at the CURRENT head — the +// package's `streamLines()` (unmodified protocol mechanics) followed by +// `src/core/stream.ts`'s `newResult()`/`applyStreamLine()` (the #627 +// result-policy fallback, now including the zero-key-row guard) — and +// asserts the result is byte-identical to the committed +// `*.normalized.json` files. Both committed captures carry only 3-key rows +// (id/precise/lexical), never a zero-key `{}` row, so the guard added in +// `edcb8ba` is provably inert against this corpus: this test is 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 at the current head (PR review pass 1)', () => { + 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']); + }); +}); From e9ca6788bb46e5f28c0e0060db856c44cc0cb7b2 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Wed, 12 Aug 2026 23:15:40 +0200 Subject: [PATCH 6/7] fix(#627): address review pass 2 findings - docs/evidence/627/README.md: stop pinning "final head" to edcb8ba7 (the commit that added this prose was itself one commit later, e94b7c5, proving the SHA-pinned framing goes stale immediately). Reword to a stable alias ("the zero-key-guard commit") and describe the replay test as validating whichever checkout/HEAD executes it, not a permanently final SHA. - Fix "byte-identical"/"byte-for-byte" overstatement in README.md and tests/unit/evidence-627-replay.test.ts's header comment: the replay test does deep/structural equality on parsed *.normalized.json objects, not a byte-for-byte file comparison. Reworded to "exactly/structurally equal". Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz --- docs/evidence/627/README.md | 49 ++++++++++++++------------ tests/unit/evidence-627-replay.test.ts | 5 +-- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/docs/evidence/627/README.md b/docs/evidence/627/README.md index 4b093b7f..aa2f57f0 100644 --- a/docs/evidence/627/README.md +++ b/docs/evidence/627/README.md @@ -16,34 +16,39 @@ diverged fork). At that commit, `src/core/stream.ts`'s meta-less fallback had be 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 final head. A later commit on this same branch, -`edcb8ba7` (`fix(#627): restore row-width invariant for zero-key rows...`), 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 the branch's final head, not only the commit -the live capture happened to run against — so the distinction matters: - -* The guard **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 `edcb8ba7`'s `stream.ts` reproduces the - committed `*.normalized.json` files byte-for-byte. +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 commit `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 currently passes at `edcb8ba7` and every - commit after it that leaves the corpus/guard behavior unchanged. + 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 at the -branch's final head, `edcb8ba7`, via the committed replay test above — not frozen -at the live-capture commit. +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 diff --git a/tests/unit/evidence-627-replay.test.ts b/tests/unit/evidence-627-replay.test.ts index ed67e9d9..a075f18e 100644 --- a/tests/unit/evidence-627-replay.test.ts +++ b/tests/unit/evidence-627-replay.test.ts @@ -11,8 +11,9 @@ // package's `streamLines()` (unmodified protocol mechanics) followed by // `src/core/stream.ts`'s `newResult()`/`applyStreamLine()` (the #627 // result-policy fallback, now including the zero-key-row guard) — and -// asserts the result is byte-identical to the committed -// `*.normalized.json` files. Both committed captures carry only 3-key rows +// asserts the result is exactly/structurally equal to the committed +// `*.normalized.json` files (deep value equality on the parsed objects, not +// a byte-for-byte file comparison). Both committed captures carry only 3-key rows // (id/precise/lexical), never a zero-key `{}` row, so the guard added in // `edcb8ba` is provably inert against this corpus: this test is the // evidence for that inertness claim, not just an assertion of it in prose. From 97d65a004cf84047071725c35749911e7eaddc1f Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Wed, 12 Aug 2026 23:20:14 +0200 Subject: [PATCH 7/7] docs(#627): state stable reasons in evidence replay spec and evidence note Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz --- docs/evidence/627/README.md | 13 ++++---- tests/unit/evidence-627-replay.test.ts | 42 ++++++++++++++------------ 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/docs/evidence/627/README.md b/docs/evidence/627/README.md index aa2f57f0..bf3304a1 100644 --- a/docs/evidence/627/README.md +++ b/docs/evidence/627/README.md @@ -154,13 +154,12 @@ 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` (added in PR review pass 1, see "Tested -commit" above) 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 again the next time `src/core/stream.ts`'s fallback changes. +`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 diff --git a/tests/unit/evidence-627-replay.test.ts b/tests/unit/evidence-627-replay.test.ts index a075f18e..1140a2a0 100644 --- a/tests/unit/evidence-627-replay.test.ts +++ b/tests/unit/evidence-627-replay.test.ts @@ -1,22 +1,26 @@ -// Issue #627 — PR-review finding (pass 1): `docs/evidence/627/README.md` -// pinned its "Tested commit" to `036760a` and claimed `src/core/stream.ts`'s -// meta-less fallback "is unchanged since". That was true when written, but -// this branch's own later commit `edcb8ba` (`fix(#627): restore row-width -// invariant for zero-key rows...`) added a zero-key-row guard to the exact -// `json.row` fallback arm the evidence exercises — making the README's -// blanket claim stale/false relative to the branch's final head. +// 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 the promised replay: it feeds the two RAW committed NDJSON -// captures through the REAL production path at the CURRENT head — the -// package's `streamLines()` (unmodified protocol mechanics) followed by -// `src/core/stream.ts`'s `newResult()`/`applyStreamLine()` (the #627 -// result-policy fallback, now including the zero-key-row guard) — and -// asserts the result is exactly/structurally equal to the committed -// `*.normalized.json` files (deep value equality on the parsed objects, not -// a byte-for-byte file comparison). Both committed captures carry only 3-key rows -// (id/precise/lexical), never a zero-key `{}` row, so the guard added in -// `edcb8ba` is provably inert against this corpus: this test is the -// evidence for that inertness claim, not just an assertion of it in prose. +// 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'; @@ -62,7 +66,7 @@ async function replay(ndjson: string): Promise<{ metaSeen: boolean; result: Stre return { metaSeen, result }; } -describe('#627 evidence replay — production path at the current head (PR review pass 1)', () => { +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'],