1.23 beta8 - #920
Merged
Merged
Conversation
## Problem CI install steps fail intermittently with transient registry errors, e.g. the `ECONNRESET` seen on #900: ``` npm error code ECONNRESET npm error network aborted npm error network This is a problem related to network connectivity. ``` npm's default is only **2** network retries, so a single connection reset mid-download fails the entire `npm ci` / `npm install` step and the job. ## Fix Raise npm's fetch retry count and timeouts via top-level workflow `env` in every workflow that installs dependencies: ```yaml NPM_CONFIG_FETCH_RETRIES: "5" NPM_CONFIG_FETCH_RETRY_MINTIMEOUT: "10000" NPM_CONFIG_FETCH_RETRY_MAXTIMEOUT: "60000" NPM_CONFIG_FETCH_TIMEOUT: "600000" ``` Applied as workflow env vars rather than a committed `.npmrc`, because `.npmrc` is gitignored as a token-leak guard (kept that convention intact). ### Scope 10 workflows that run `npm ci`/`npm install`: `publish.yml`, `tests-node.yml`, `tests-web.yml`, `tests-oss-dependents.yml`, `tests-skill-rowbinary-parser.yml`, `publish-datatype-parser.yml`, `publish-skill-rowbinary-parser.yml`, `e2e-skills.yml`, `examples.yml`, `upstream-sql-tests.yml`. `tests-bun.yml` is intentionally excluded — `bun install` does not read `NPM_CONFIG_*` env vars. ## Verification - Confirmed the env-var mapping locally: `NPM_CONFIG_FETCH_RETRIES=7 npm config get fetch-retries` → `fetch-retries=7` (and the same for the timeout keys). - All 10 edited workflows parse as valid YAML with exactly the 4 `NPM_CONFIG_*` keys under top-level `env`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…de 20) (#905) ## Summary Fixes the `e2e (node 20)` failure in the `publish` workflow ([run](https://github.com/ClickHouse/clickhouse-js/actions/runs/28302399799/job/83852950182)): ``` tests/e2e/install/src/integration.ts:15 SyntaxError: Unexpected token ':' ``` `tests/e2e/install/src/integration.ts` (added in #899) is run via `node src/integration.ts` across Node 20/22/24. **Node 20 does not strip TypeScript types by default** (that became default only in 22.18 / 23.6), so the `client: any` and `caught: unknown` annotations were a hard syntax error on the Node 20 leg. `src/index.ts` survives because it's plain JS in a `.ts` file. ### Fix Make `integration.ts` TS-syntax-free, like `src/index.ts`: - Inline the ping-wait loop into `main()`, removing the annotated `waitForClickHouse(client: any)` parameter (this also keeps the strict `tsc --noEmit` step happy — no implicit `any`, since there's no longer a parameter to annotate). - Drop the `let caught: unknown` annotation. Behaviour is unchanged. ## Verification - `grep` confirms no TS-only syntax remains. - `tsc --noEmit` (strict) passes in `tests/e2e/install`. - Ran `node src/integration.ts` against the real published `@clickhouse/client` + a local ClickHouse — exit 0, all assertions pass. - The plain-JS-in-`.ts` pattern matches `src/index.ts`, which already runs on the Node 20 leg. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…unner (#907) ## Summary The `upstream-sql-tests` workflow has been failing on every nightly run (and on `main` pushes / PRs) for over a week — a single test, `03380_accurate_cast_or_null_qbit`, on one shard, failing identically on `head` and `latest`. **Root cause:** upstream added a statement annotated with `-- { serverError SIZES_OF_ARRAYS_DONT_MATCH }` — i.e. the query is *expected to throw*. The native `clickhouse-client` parses these trailing hints (`src/Client/TestHint.cpp`) and treats a matching error as a pass; our Node shim had no such handling, so the propagated `ClickHouseError` aborted the script and `clickhouse-test` recorded a FAIL. Not a flake, not a client regression — a harness gap. ## Changes - **`src/test-hint.ts`** (new) — parses `-- { serverError ... }` / `-- { clientError ... }` / `-- { error ... }` hints (numeric and named codes, comma lists, block comments), attaches each trailing hint to the statement it annotates (the splitter cuts on `;`, so a hint becomes the *next* element's leading comment → attached to the previous statement, matching upstream's trailing-comment semantics), and matches against `ClickHouseError.code` / `.type`. - **`backends/client.ts`** — per statement: a matched expected error is suppressed (no output, continue); a wrong error or an unexpectedly-successful query fails. Also set `log.level = OFF` — `clickhouse-test` fails any test that writes to **stderr**, so the client's default error logger must stay quiet on suppressed errors. - **`main.ts`** — builds `Statement[]` and threads expectations through. - **`__tests__/test-hint.test.ts`** (new) — unit tests for parsing, hint association, and error matching. ## Verification - Lint, typecheck, and unit tests (**53 passing**) all green. - End-to-end against a live server: a matching serverError is suppressed → exit 0, correct stdout, **0 bytes stderr**; a mismatched code and an unexpected success both fail correctly; normal queries and genuine errors unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nt (#887) ## Summary The rowbinary skill/library README documented the decoder API but never showed how to feed it bytes from the official client. Adds a **"Using it with the ClickHouse JS client"** section to `skills/clickhouse-js-node-rowbinary-parser/README.md`. - **Key wiring point:** `RowBinary` isn't a client-decoded format, so `client.query({ format })` doesn't apply — use `client.exec({ query: "... FORMAT RowBinary" })` to get the raw byte stream and pass it to `streamRowBatches(chunks, readRow)`. - **Scope is Node.js (`@clickhouse/client`):** `exec`'s `Stream.Readable` is already an `AsyncIterable<Uint8Array>` whose `Buffer`/`Uint8Array` chunks `streamRowBatches` normalizes, so it's fed straight in. Web is intentionally excluded — `@clickhouse/rowbinary` decodes into Node `Buffer`s and is Node-only per SKILL.md. ```ts const { stream } = await client.exec({ query: "SELECT id, uid, price, status FROM orders FORMAT RowBinary", }); for await (const rows of streamRowBatches(stream, readOrderRow)) { for (const row of rows) console.log(row); } ``` ## Checklist - [ ] For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com>
## Summary Refresh the supported Node.js policy: add Node 26 and drop the EOL Node 18. Node 20 stays fully supported (CI + engines). ### Node 26 - Added `26` to every Node.js test/e2e matrix (`tests-node` ×5, `publish`, `tests-oss-dependents`, `tests-skill-rowbinary-parser`, `publish-skill-rowbinary-parser`, `publish-datatype-parser`) → `[20, 22, 24, 26]`. ### Drop Node 18 - README support table: removed the `18.x` row (20.x remains ✔). - Raised the `engines.node` floor of the published **`@clickhouse/client`** (`>=16` → `>=20`) and **`@clickhouse/datatype-parser`** (`>=18.0.0` → `>=20`), with matching `package-lock.json` updates (workspace entries only — transitive deps untouched). Other manifests already required `>=20`. - Updated CONTRIBUTING, the setup skill, the example/demo prerequisites, and added a CHANGELOG migration note. > [!NOTE] > Raising `engines.node` on the published `@clickhouse/client` (was `>=16`) and `@clickhouse/datatype-parser` (was `>=18.0.0`) is user-facing — installs on Node <20 will now emit an `EBADENGINE` warning. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…908) ## What Adds a second backend to the `tests/clickhouse-test-runner` harness that runs the upstream ClickHouse SQL test suite **through the RowBinary parser** (`@clickhouse/rowbinary`), turning the suite into a broad, real-world conformance test for the dynamic header→reader decode path. Today the harness is a pure passthrough: it asks ClickHouse for `TabSeparated` and streams the bytes out, so it only exercises transport/session/settings. The new `rowbinary` backend instead requests `RowBinaryWithNamesAndTypes` for each result-returning statement, decodes it with `compileRowBinaryWithNamesAndTypes`, and re-renders the rows as `TabSeparated` — so the existing upstream `.reference` diff is still the oracle. **ClickHouse is the byte oracle; `.reference` is the value oracle.** A faithful round-trip (decode → render == server's text) proves the parser decoded every column correctly. Selected via `TEST_RUNNER_BACKEND=rowbinary`; the default stays passthrough, so the existing CI signal is untouched. ## How it works - **`src/tsv-serialize.ts`** — a type-AST-directed serializer that renders decoded values into ClickHouse's exact `TabSeparated` text: top-level vs nested (`Array`/`Tuple`/`Map`) quoting, `NULL` as `\N` / `NULL`, enum integers mapped to names, dates/datetimes, decimals, UUID/IP, big integers. Reuses the parser's own `format*` helpers. Unsupported types (`Variant`, `JSON`, `Dynamic`, geo, `Nested`, `Float32` precision) throw, so such tests stay off the allowlist rather than silently passing an unexercised path. - **`src/backends/rowbinary.ts`** — statement routing: bare result-queries take the decode path; DDL / `INSERT` / `SET` / explicit-`FORMAT` statements fall through to passthrough. Decodes **positionally** (via `columnReaders`) to survive integer-like and duplicate column names. Honors `-- { serverError ... }` hints via the shared `settleExpectedError`, exactly like the passthrough backend. - **`src/backends/shared.ts`** — extracted the shared client/session/settings/error-hint scaffolding so both backends behave identically. - **`rowbinary-allowlist.txt`** — 2239 tests, generated by a differential sweep: kept only where the RowBinary decode reproduces passthrough output byte-for-byte (193 differing + 413 erroring excluded). It is a subset of `upstream-allowlist.txt`. - **CI** — `upstream-sql-tests.yml` gains a `backend: [passthrough, rowbinary]` matrix dimension, a skill-build step (the `@clickhouse/rowbinary` `file:` dependency's `dist/` is gitignored), and rowbinary allowlist wiring. README documents both backends. ## Testing - New unit tests for the serializer and statement routing; full runner suite green. - Serializer validated byte-for-byte against a live server across a 30+ type battery (scalars, decimals, dates, enums, UUID/IP, arrays/tuples/maps, deep nesting). - Spot-checked dozens of real upstream `.sql` tests: decode-and-render matches `.reference`. ## Notes - The 2239-test allowlist is a validated starter set; prune anything that flakes (e.g. coincidental non-determinism matches), grow it as the renderer learns more types. - The serializer formats `DateTime` in UTC (the server tz); columns with an explicit non-UTC tz are excluded by the allowlist for now. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## What `readEnum8` / `readEnum16` previously returned the raw underlying `Int8`/`Int16`; the name had to be resolved by the caller. But people reach for enums precisely for the *label*, so this makes name resolution the default. They are now **factories** that take the enum's `value -> name` map and return a `Reader<string>` yielding the name: ```ts // before readEnum8(state: Cursor): number // after readEnum8(valueToName: EnumNameMap): Reader<string> // EnumNameMap = ReadonlyMap<number, string> ``` Both decode paths build the map from the `'name' = value` pairs the type already carries and pass it in: - `compile.ts` (textual `RowBinaryWithNamesAndTypes` header) — from `EnumDataType.values`. - `dynamic.ts` (binary type encoding, tags `0x17`/`0x18`) — from the name/value pairs in the header (previously read and discarded). An unmapped value falls back to its stringified integer, so a decode never throws on an unexpected wire value. The **raw integer** is still a first-class path via `readInt8`/`readInt16` — the example readers (`orders`, `observability`) use those to keep their monomorphized hot paths allocation-free. ## Breaking change `readEnum8`/`readEnum16` changed signature, and decoded enum values are now `string` (name) rather than `number`. Pre-1.0 package, so a minor bump should carry it; flagged with a `BREAKING CHANGE:` footer. ## Tests - Rewrote `Enum8`/`Enum16` unit tests to assert name resolution + the unmapped-fallback, and updated the `framing`, `rowBinaryWithNamesAndTypes` (header path), and `Dynamic` (binary path) enum assertions. - Full skill suite green: **564 passing** (against a live CH 26.1), `tsc --noEmit` clean, prettier clean. ## Heads-up for the test-runner harness (separate package, not in this PR) `tests/clickhouse-test-runner`'s TSV serializer maps enum int→name itself (`enumName`) and currently expects a `number`. It consumes the **published** `@clickhouse/rowbinary`, so it's unaffected until this is released and the dep is bumped — at which point that serializer's enum case must adapt to receive the name directly. Context: split out from #909 review; complements the Variant discriminant discussion in #910. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
…n) (#913) ## Problem PR #912 (`feat(rowbinary)!: resolve Enum8/Enum16 to their names`) landed a `SKILL.md` edit that is not Prettier-clean. The repo-wide `prettier --check .` in the `code-quality` job now fails on `main`, which means **every open PR** (whose CI checks out PR-merged-with-main) also fails `code-quality`. ``` [warn] skills/clickhouse-js-node-rowbinary-parser/SKILL.md [error] File is not formatted with Prettier. ``` ## Fix `prettier --write` on the file (a one-line reflow). No content change. Spotted while triaging red CI on #909. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#909) ## What Re-ran the differential sweep over the full passthrough `upstream-allowlist.txt` with the Float32 / float-text / DateTime64 fixes from #908 merged, and expanded `rowbinary-allowlist.txt` with the tests that now decode-match passthrough byte-for-byte: **2239 → 2264**. Union with the prior list, so it can only add tests — no regressions. ## Scope note (honest expectation) The realized gain is **modest** (~+25). Most of the ~600 passthrough tests the rowbinary backend doesn't yet cover are **not** Float32 mismatches — they need **renderer features that don't exist yet** (`Variant`, `JSON`, `Dynamic`, geo, `Nested`) or are **non-deterministic** (can't be validated against a static `.reference`). Closing that remainder is a separate, larger effort (new type renderers), not this PR. ## On validation A local standalone sweep is **not** a reliable oracle here: some tests produce different (or empty) output outside the upstream Python `clickhouse-test` runner, so `rowbinary == passthrough` can hold trivially standalone yet say nothing about the `.reference` diff. The authoritative check is the **CI `rowbinary` matrix leg** (Python runner vs the static `.reference`) — which is green on this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…skill (#914) - Split the repository-wide CHANGELOG into per-package CHANGELOG.md files (root frozen, copied as-is into the client packages; standalone packages seeded from recent entries); ship them via each package's `files`. Rewire AGENTS.md, the review instructions, and the PR template to the per-package policy. - Split publish.yml into per-package publish workflows (publish-client / -web / -common), each with a path-scoped automatic `head` publish and a manual `latest` publish. The clients share the common sources via the src/common symlink, so packages/client-common/** is an input to both client workflows. - Add a real-browser post-publish e2e for @clickhouse/client-web (vitest browser mode + Playwright, chromium & firefox) against a live ClickHouse. - Add the in-repo `release` skill as the source of truth for the release process (RELEASING.md now points to it).
## Summary The `@clickhouse/rowbinary` package only had the decode (reader) side. This adds the encode (writer) side, mirroring the reader module-for-module: every `readX` gains a `writeX` counterpart in a sibling `*_writer.ts` module (the reader files are left unchanged), plus a `writer.ts` barrel. The dynamic AST-based compile path and the skill docs are intentionally left out. ### Foundation (`core_writer.ts`) - `Sink` — a **fixed-length** output buffer wrapping a caller-supplied `Buffer`, the encode-side analog of the reader's `Cursor`. - `Writer<T> = (sink, value) => void` and `reserve(sink, n)` — the counterpart to the reader's `advance`. `reserve` treats the buffer as a constant-length window: when a write would overflow it, it throws the `NeedMoreSpace` sentinel (mirroring the reader's `NeedMoreData`) without advancing the position, so the caller can flush the buffer down the connection and continue. ### Writers (one type per module) - **Leaf types**: varint, integers (`UInt8`…`Int256`), bool, enums, floats (incl. `BFloat16`), decimals, strings/fixed-strings, uuid, ip, datetime (`Date`/`Date32`/`DateTime`/`DateTime64`), time, interval. 64-bit words are written directly via the Node `Buffer` bigint methods (`writeBigUInt64LE`/`writeBigInt64LE`), with `MASK64` masking for 128/256-bit words. - **Combinators** (`composite_writer.ts`): `writeNullable`, `writeArray`, `writeQBit`, `writeTuple`, `writeTupleNamed`, `writeMap`, `writeVariant`; `writeRows` driver. - **Geo**: `writePoint`/`Ring`/`LineString`/`Polygon`/`MultiLineString`/`MultiPolygon`/`Geometry`. - **Transparent wrappers**: `writeLowCardinality` and `writeSimpleAggregateFunction` are identity combinators (no extra wire layer); `writeNested` = `writeArray(writeTupleNamed(...))`. - **Non-encodable**: `writeNothing` and `writeAggregateFunction` throw, mirroring their readers (zero-width / opaque unframed state). ### Notable asymmetries - `writeVariant`/`writeGeometry` take a tagged `[discriminant, value]` because the reader doesn't surface which alternative was decoded (value shapes overlap), so the writer can't infer it. - Added `parse*` inverses of the reader's `format*` helpers: `parseDecimal`, `parseUUID`, `parseIPv4`/`parseIPv6`, `parseTime`/`parseTime64`. ### Tests - Writer tests are independent from the reader tests: a shared `encode` helper feeds a JS value through the writer and the resulting bytes are asserted against live ClickHouse output (or hard-coded wire bytes), never decoded via a reader. Each scenario is an isolated `it()` case. ### Out of scope - Dynamic/JSON writers and the `compile`/`header`/`rowBinaryWithNamesAndTypes` AST path (require type inference) — deferred. - The skill documentation — left to the maintainer. `WRITER_TODO.md` captures the design notes and the full per-type checklist. ## Checklist - [x] Unit and integration tests covering the common scenarios were added --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Peter Leonov <peter.leonov@clickhouse.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…fer (#915) writeRows was a Writer<readonly T[]> that wrote a whole array into one fixed-size Sink; a row overflowing the buffer let BufferFull escape mid-row, so the caller couldn't tell how many rows landed and a result larger than the buffer had no path through. writeRows now returns a generator that owns the sink: it catches BufferFull, rewinds to the last complete row boundary (never a half-written row), yields that batch, and starts a fresh buffer for the rest. It takes an Iterable<T> (so an unbounded row source works) and a bufferSize (validated as a positive integer). An oversized row grows the buffer (doubling, warns once) instead of throwing, and each flushed buffer's fill is published on the @clickhouse/rowbinary:writeRows.flush node:diagnostics_channel for buffer-utilization metrics. Adds a 0.2.0 CHANGELOG entry documenting the new writer surface. Addresses the review comment on #911.
## Summary Follow-up PR for the RowBinary **writer** added in #911. Peter's own review comments on #911 were all resolved during that PR; the remaining items were the Copilot bot's review findings, which Peter left for a separate PR ("Looks good so far, gonna continue in another PR"). This addresses the three that were still live in `main`: - **`datetime_writer.ts` — round → floor.** `writeDate`, `writeDate32` and `writeDateTime` used `Math.round`, which shifts a non-midnight `Date` into the next/previous day (`12:00` → next day) and a sub-second `DateTime` up to the next second (`00:00:00.600` → `+1s`). They now `Math.floor`, truncating to the calendar day / whole second — matching `readDate*`/`readDateTime` and the existing `writeDateTime64` (`Math.floor(getTime()/1000)`). `writeDate32` floors toward `-inf`, so pre-1970 instants land on the correct earlier day rather than rounding toward the epoch. - **`ip_writer.ts` — strict IPv6 group parsing.** `parseIPv6` did `parseInt(part, 16) & 0xffff`, which silently encoded malformed groups as `0` (`NaN & 0xffff === 0`) and wrapped negatives like `"-1"` to `0xffff`. It now validates each group is 1–4 hex digits and throws `RangeError` otherwise. - **`geo_writer.ts` — validate before writing.** `writeGeometry` wrote the 1-byte discriminant *before* the `switch`, so an out-of-range discriminant threw *after* advancing the sink, leaving a partial payload. The discriminant byte is now written only inside an accepted `case`; an unknown value throws from `default` before any bytes are written (mirrors `writeVariant`). Two other Copilot findings on #911 are already resolved and need no change here: the `WRITER_TODO.md` `NeedMoreSpace`/`BufferFull` mismatch (the file was removed) and the missing CHANGELOG entry (added for #911/#915). The `writeRows` design comment was handled in #915. ## Tests Each fix gets a ClickHouse-independent test (`encode`-vs-`encode` for the flooring, throw-assertions for the validation), so they run without a live server and can't be masked by a reader. Full package suite: **724 passing**, typecheck and prettier clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## Summary The root [`AGENTS.md`](https://github.com/ClickHouse/clickhouse-js/blob/main/AGENTS.md) had grown to cover packages, examples, skills, docs and the upstream test harness all in one file. This splits each section into a nested `AGENTS.md` next to the code it describes, so an agent (or human) reads the guidance closest to the files being edited: | File | Content | | ---- | ------- | | `packages/AGENTS.md` | Log-message conventions, package structure, intentional node/web duplication | | `examples/AGENTS.md` | Example corpus layout and conventions | | `skills/AGENTS.md` | Skill declaration requirement (`agents.skills` array) | | `skills/clickhouse-js-node-rowbinary-parser/AGENTS.md` | RowBinary reader/writer test conventions **+ a new no-defensive-validation rule** for the codecs | | `docs/AGENTS.md` | Embedded docs guidance | | `tests/clickhouse-test-runner/AGENTS.md` | Upstream SQL harness + allowlist-growth strategy | The **root `AGENTS.md`** keeps only cross-cutting guidance — the audience note, TypeScript LSP, and the code-review section (security / API stability / changelog rules) — plus an **index** linking to each nested file. ## Notes - **Relative links rewritten** for each file's new location; all link targets verified to resolve on disk. - The "API quality and stability" section stays at root because [`.github/instructions/review.instructions.md`](https://github.com/ClickHouse/clickhouse-js/blob/main/.github/instructions/review.instructions.md) links to it by name. - The new **no-defensive-validation rule** for the RowBinary codecs (don't runtime-validate input in hot-path `readX`/`writeX`; document the precondition in JSDoc and let the server reject bad bytes — with narrow exceptions for wire-framing checks and zero-cost parse helpers) lands in the rowbinary skill's `AGENTS.md`. It codifies the convention applied in #911/#916. Docs-only; no code or test changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the release/testing/tooling surface around the 1.23 line by (1) splitting changelogs per package and updating packaging metadata accordingly, (2) extending the upstream SQL test harness with ClickHouse-test error-hint semantics plus a new RowBinary-backed execution mode, and (3) adding a post-publish, real-browser e2e suite for @clickhouse/client-web. It also updates CI/workflows (Node 26 coverage, npm network-retry hardening) and introduces per-package publish workflows for the web/common clients.
Changes:
- Add a real-browser, post-publish e2e suite for
@clickhouse/client-web(Vitest browser + Playwright) and wire it into a new publish workflow. - Extend
tests/clickhouse-test-runnerwith upstream{ serverError ... }hint support and aTEST_RUNNER_BACKEND=rowbinarymode that decodesRowBinaryWithNamesAndTypesvia@clickhouse/rowbinary. - Add a RowBinary writer implementation to
@clickhouse/rowbinary, bump it to0.2.0, and introduce per-package changelogs (with the rootCHANGELOG.mdmarked frozen).
Reviewed changes
Copilot reviewed 107 out of 110 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/web-browser/vitest.config.ts | New Vitest browser config (Playwright provider) for real-browser e2e. |
| tests/e2e/web-browser/test/web.browser.test.ts | New browser e2e validating import/ping/query/streaming/error surfacing. |
| tests/e2e/web-browser/package.json | New isolated npm project used by publish workflow e2e. |
| tests/e2e/web-browser/.gitignore | Ignore local install artifacts for the browser e2e project. |
| tests/e2e/install/src/integration.ts | Make install e2e script Node20-compatible (no TS-only syntax). |
| tests/clickhouse-test-runner/upstream-allowlist.txt | Document + disable one flaky upstream test on latest. |
| tests/clickhouse-test-runner/src/test-hint.ts | Implement parsing/attaching upstream clickhouse-test error hints. |
| tests/clickhouse-test-runner/src/main.ts | Switch to statement model, honor hints, add backend selector env. |
| tests/clickhouse-test-runner/src/backends/shared.ts | Shared settings/session client + expected-error reconciliation helpers. |
| tests/clickhouse-test-runner/src/backends/rowbinary.ts | New rowbinary backend: decode RBWNT via @clickhouse/rowbinary, re-render TSV. |
| tests/clickhouse-test-runner/src/backends/client.ts | Refactor passthrough backend to use shared helpers + expected-error logic. |
| tests/clickhouse-test-runner/README.md | Document passthrough vs rowbinary backends + env var. |
| tests/clickhouse-test-runner/package.json | Add @clickhouse/rowbinary dependency for rowbinary backend. |
| tests/clickhouse-test-runner/AGENTS.md | Add harness-specific agent guidance. |
| tests/clickhouse-test-runner/tests/test-hint.test.ts | Unit tests for hint parsing/statement building/error matching. |
| tests/clickhouse-test-runner/tests/should-decode.test.ts | Unit tests for rowbinary backend routing logic. |
| skills/clickhouse-js-node-rowbinary-parser/tests/UVarint.write.test.ts | New writer tests (varint + sink reserve behavior). |
| skills/clickhouse-js-node-rowbinary-parser/tests/UUID.write.test.ts | New writer tests for UUID encode helpers. |
| skills/clickhouse-js-node-rowbinary-parser/tests/TimeInterval.write.test.ts | New writer tests for Time/Time64/Interval. |
| skills/clickhouse-js-node-rowbinary-parser/tests/String.write.test.ts | New writer tests for String/FixedString (string + raw bytes). |
| skills/clickhouse-js-node-rowbinary-parser/tests/Rows.write.test.ts | New writer tests for writeRows generator (flush/grow/metrics). |
| skills/clickhouse-js-node-rowbinary-parser/tests/Nothing.write.test.ts | New writer tests for Nothing (must not be directly writable). |
| skills/clickhouse-js-node-rowbinary-parser/tests/IP.write.test.ts | New writer+parser tests for IPv4/IPv6. |
| skills/clickhouse-js-node-rowbinary-parser/tests/Integers.write.test.ts | New writer tests for integer widths/signedness. |
| skills/clickhouse-js-node-rowbinary-parser/tests/Geo.write.test.ts | New writer tests for geo types + Geometry discriminant handling. |
| skills/clickhouse-js-node-rowbinary-parser/tests/Decimal.write.test.ts | New writer tests + parseDecimal coverage. |
| skills/clickhouse-js-node-rowbinary-parser/tests/DateTime.write.test.ts | New writer tests for Date/Date32/DateTime/DateTime64 (incl flooring). |
| skills/clickhouse-js-node-rowbinary-parser/tests/Composite.write.test.ts | New writer tests for Nullable/Array/Tuple/Map/Variant. |
| skills/clickhouse-js-node-rowbinary-parser/tests/BoolEnumFloat.write.test.ts | New writer tests for Bool/Enum underlying ints/Floats/BFloat16. |
| skills/clickhouse-js-node-rowbinary-parser/tests/AggregateFunction.write.test.ts | Writer throws for opaque AggregateFunction state. |
| skills/clickhouse-js-node-rowbinary-parser/tests/parseIP.bench.ts | Add benchmark coverage for IP parsing helpers. |
| skills/clickhouse-js-node-rowbinary-parser/tests/rowBinaryWithNamesAndTypes.test.ts | Update enum expectations to resolve to names. |
| skills/clickhouse-js-node-rowbinary-parser/tests/framing.test.ts | Update enum framing tests for new enum reader API. |
| skills/clickhouse-js-node-rowbinary-parser/tests/Enum8.test.ts | Update Enum8 reader tests to new map-based API + fallback behavior. |
| skills/clickhouse-js-node-rowbinary-parser/tests/Enum16.test.ts | Update Enum16 reader tests to new map-based API + fallback behavior. |
| skills/clickhouse-js-node-rowbinary-parser/tests/encode.ts | Shared helper for writer tests (value -> bytes), reader-independent. |
| skills/clickhouse-js-node-rowbinary-parser/src/writer.ts | New writer barrel export (@clickhouse/rowbinary/writer). |
| skills/clickhouse-js-node-rowbinary-parser/src/core_writer.ts | Add Sink/reserve/BufferFull writer core primitives. |
| skills/clickhouse-js-node-rowbinary-parser/src/varint_writer.ts | Add writeUVarint implementation. |
| skills/clickhouse-js-node-rowbinary-parser/src/integers_writer.ts | Add integer writers (8..256-bit, signed/unsigned). |
| skills/clickhouse-js-node-rowbinary-parser/src/bool_writer.ts | Add Bool writer. |
| skills/clickhouse-js-node-rowbinary-parser/src/enums_writer.ts | Add Enum8/Enum16 writers (underlying ints). |
| skills/clickhouse-js-node-rowbinary-parser/src/floats_writer.ts | Add Float32/64 and BFloat16 writer. |
| skills/clickhouse-js-node-rowbinary-parser/src/decimals_writer.ts | Add decimal writers + parseDecimal helper. |
| skills/clickhouse-js-node-rowbinary-parser/src/strings_writer.ts | Add String/FixedString writers (string + raw bytes). |
| skills/clickhouse-js-node-rowbinary-parser/src/uuid_writer.ts | Add UUID writers + parseUUID helper. |
| skills/clickhouse-js-node-rowbinary-parser/src/ip_writer.ts | Add IP writers + parseIPv4/parseIPv6 helpers (strict). |
| skills/clickhouse-js-node-rowbinary-parser/src/datetime_writer.ts | Add Date/Date32/DateTime/DateTime64 writers (flooring semantics). |
| skills/clickhouse-js-node-rowbinary-parser/src/time_writer.ts | Add Time/Time64 writers + parse helpers. |
| skills/clickhouse-js-node-rowbinary-parser/src/interval_writer.ts | Add Interval writer alias. |
| skills/clickhouse-js-node-rowbinary-parser/src/composite_writer.ts | Add composite writers (Nullable/Array/Tuple/Map/Variant/QBit). |
| skills/clickhouse-js-node-rowbinary-parser/src/rows_writer.ts | Add streaming writeRows generator + diagnostics_channel flush events. |
| skills/clickhouse-js-node-rowbinary-parser/src/geo_writer.ts | Add geo writers + Geometry tagged encoding. |
| skills/clickhouse-js-node-rowbinary-parser/src/lowCardinality_writer.ts | Add LowCardinality writer identity wrapper. |
| skills/clickhouse-js-node-rowbinary-parser/src/simpleAggregateFunction_writer.ts | Add SimpleAggregateFunction writer identity wrapper. |
| skills/clickhouse-js-node-rowbinary-parser/src/nested_writer.ts | Add Nested writer wrapper (Array(TupleNamed(...))). |
| skills/clickhouse-js-node-rowbinary-parser/src/nothing_writer.ts | Add Nothing writer that always throws (zero-width). |
| skills/clickhouse-js-node-rowbinary-parser/src/aggregateFunction_writer.ts | Add AggregateFunction writer that throws (opaque). |
| skills/clickhouse-js-node-rowbinary-parser/src/enums.ts | Change enum readers to resolve underlying ints to names via supplied map. |
| skills/clickhouse-js-node-rowbinary-parser/src/dynamic.ts | Update dynamic type decoding to build enum maps and resolve to names. |
| skills/clickhouse-js-node-rowbinary-parser/src/compile.ts | Update type-string compilation to build enum name maps; fallback for bare Enum8/16. |
| skills/clickhouse-js-node-rowbinary-parser/src/examples/orders.ts | Update example to use raw int enum reader (readInt8) post enum API change. |
| skills/clickhouse-js-node-rowbinary-parser/src/examples/observability.ts | Same: switch enum field read to raw underlying int. |
| skills/clickhouse-js-node-rowbinary-parser/SKILL.md | Update docs to reflect enum resolution behavior. |
| skills/clickhouse-js-node-rowbinary-parser/README.md | Add guidance for using RowBinary decoding with @clickhouse/client. |
| skills/clickhouse-js-node-rowbinary-parser/package.json | Bump @clickhouse/rowbinary to 0.2.0 and ship standalone CHANGELOG. |
| skills/clickhouse-js-node-rowbinary-parser/package-lock.json | Version bump reflected in the skill package lockfile. |
| skills/clickhouse-js-node-rowbinary-parser/CHANGELOG.md | New standalone package changelog for @clickhouse/rowbinary. |
| skills/clickhouse-js-node-rowbinary-parser/AGENTS.md | Add skill-specific conventions (writer tests separation, validation rules). |
| skills/AGENTS.md | Add top-level guidance for skills/ directory. |
| RELEASING.md | Replace legacy release docs with pointer to in-repo release skill. |
| README.md | Update supported Node matrix + document per-package changelogs. |
| packages/datatype-parser/package.json | Ship CHANGELOG and raise engines floor to Node >=20. |
| packages/datatype-parser/CHANGELOG.md | New standalone changelog for datatype-parser. |
| packages/client-web/package.json | Ship package CHANGELOG in published files list. |
| packages/client-node/package.json | Raise Node engines floor to >=20 and ship package CHANGELOG. |
| packages/client-common/package.json | Ship package CHANGELOG in published files list. |
| packages/AGENTS.md | Add package-level agent guidance (logging + duplication rules). |
| package-lock.json | Add @clickhouse/rowbinary dependency and raise engines floors in lock metadata. |
| examples/README.md | Update example prerequisites to Node 20+. |
| examples/AGENTS.md | Add agent guidance for examples structure/conventions. |
| docs/AGENTS.md | Add agent guidance for embedded docs. |
| demo/logs/README.md | Update demo prerequisites to Node 20+. |
| CONTRIBUTING.md | Update supported Node versions list. |
| CHANGELOG.md | Mark root changelog frozen and add redirect to per-package changelogs. |
| .github/workflows/upstream-sql-tests.yml | Pin upstream ref, drop head server image, add backend matrix (passthrough/rowbinary), npm retry hardening. |
| .github/workflows/tests-web.yml | Add npm retry hardening env. |
| .github/workflows/tests-skill-rowbinary-parser.yml | Add npm retry hardening and Node 26 to matrix. |
| .github/workflows/tests-oss-dependents.yml | Add npm retry hardening and Node 26 to matrix. |
| .github/workflows/tests-node.yml | Add npm retry hardening and Node 26 to matrices. |
| .github/workflows/publish-skill-rowbinary-parser.yml | Add npm retry hardening and Node 26 e2e matrix; update workflow description. |
| .github/workflows/publish-datatype-parser.yml | Add npm retry hardening and Node 26 e2e matrix; update workflow description. |
| .github/workflows/publish-client-web.yml | New per-package publish workflow for @clickhouse/client-web + browser e2e gate. |
| .github/workflows/publish-client-common.yml | New per-package publish workflow for deprecated @clickhouse/client-common. |
| .github/workflows/examples.yml | Add npm retry hardening env. |
| .github/workflows/e2e-skills.yml | Add npm retry hardening env. |
| .github/pull_request_template.md | Update checklist to require per-package changelog updates. |
| .github/instructions/review.instructions.md | Update review rules to per-package changelog model (root frozen). |
| .claude/skills/setup/SKILL.md | Update setup skill to reflect Node 26 CI coverage. |
Files not reviewed (1)
- skills/clickhouse-js-node-rowbinary-parser/package-lock.json: Generated file
…ource by direction (#921) The `@clickhouse/rowbinary` skill/package gained a writer (and now uses the new type parser), so it's no longer a parser-only skill. This reshapes it into a read+write **codec** and reorganizes both the source tree and the skill docs by direction. ## What changed - **Renamed the skill** `clickhouse-js-node-rowbinary-parser` → `clickhouse-js-node-rowbinary` (dir, SKILL.md frontmatter, package metadata, the two workflow files, the `release` skill, and root/skills docs). The npm package name stays `@clickhouse/rowbinary`. - **Split `src/` by direction**: readers → `src/readers/`, writers → `src/writers/`, and dropped the now-redundant `_writer` suffix (`integers_writer.ts` → `writers/integers.ts`). Subpath exports now reflect the folders (`@clickhouse/rowbinary/readers/<type>`, `/writers/<type>`); the root export and the new `./writer` barrel are unchanged. - **Split the skill doc** `SKILL.md` (which had grown large) into a lean router plus two self-contained sides — `reader.md` and `writer.md` — since a task normally needs only one direction. SKILL.md keeps the format gate, the pick-your-side router, and the cross-cutting principles; each side holds its own decisions, guidance, and per-type reference table. - **Tightened `writer.md` to the intended layering** `SKILL.md → writer.md → source file with JSDoc`: each paragraph keeps the judgment (when/why) and points at the exact `src/writers/<file>.ts`, with mechanics living in (descriptive) JSDoc. A per-paragraph verification pass corrected a few claims (Sink is caller-supplied state; hoisted offsets must sync back to `sink.pos`; all date writers floor; `DateTime64` floors on the JS ms number). ## Verification - Typecheck clean; **724/724 tests pass**; `npm run build` emits the new `dist/readers` / `dist/writers` layout and both barrels resolve. - The first commit is a **pure rename** (193 files, 0 content changes) so the relocation reviews at a glance; subsequent commits carry the content. ## Notes / follow-ups - Subpath imports are a **breaking change** for anyone importing `@clickhouse/rowbinary/<type>` (now `/readers/<type>`), but this rides along in the already-pending, unreleased `0.2.0` (npm latest is `0.1.2`) that also introduces the whole writer surface — so **no extra version bump**. - Not yet done: a CHANGELOG `0.2.0` note documenting the rename + moved subpaths. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.