|
2 | 2 |
|
3 | 3 | > **Audience:** This file contains guidance for AI agents contributing to the `ClickHouse/clickhouse-js` repository itself. It is **not** intended for downstream projects that depend on `@clickhouse/client` or `@clickhouse/client-web` |
4 | 4 |
|
5 | | -1. When adding log messages, make sure to use eager log level checks to avoid unnecessary calculations for log messages that will not be emitted. For example: |
| 5 | +This root file holds repo-wide guidance. Folder-specific guidance lives in nested `AGENTS.md` files next to the code they describe — read the one closest to the files you are editing: |
6 | 6 |
|
7 | | - ```ts |
8 | | - if (log_level <= ClickHouseLogLevel.WARN) { |
9 | | - log_writer.warn({ |
10 | | - message: "Example log message", |
11 | | - }); |
12 | | - } |
13 | | - ``` |
14 | | - |
15 | | -2. When adding new log messages with suggestions for users, make sure to create a unique documentation page under the `docs/` directory (use `docs/howto/` for task-style guides; see `docs/socket_hang_up_econnreset.md` as a reference) with a detailed explanation of the issue and how to resolve it. Then, include a link to that documentation page in the log message. For example: |
16 | | - |
17 | | - ```ts |
18 | | - if (some_condition) { |
19 | | - log_writer.warn({ |
20 | | - message: |
21 | | - "Example log message with suggestions for users. For more information, see https://github.com/ClickHouse/clickhouse-js/blob/main/docs/socket_hang_up_econnreset.md", |
22 | | - }); |
23 | | - } |
24 | | - ``` |
25 | | - |
26 | | -## Package structure and code duplication |
27 | | - |
28 | | -The source packages under [`packages/`](packages) are: |
29 | | - |
30 | | -- `client-common` — platform-agnostic shared code (config, query-param formatting, multipart |
31 | | - assembly, URL handling, result sets, etc.). It must not depend on Node.js-only or Web-only APIs. |
32 | | - The published `@clickhouse/client-common` package is **deprecated**: `client-node` and `client-web` |
33 | | - no longer depend on it and instead bundle its sources via the `src/common` symlink |
34 | | - (`packages/client-node/src/common` and `packages/client-web/src/common` both point to |
35 | | - `packages/client-common/src`), importing from it with relative paths (e.g. `./common/index`). |
36 | | -- `client-node` (`@clickhouse/client`) — the Node.js client. |
37 | | -- `client-web` (`@clickhouse/client-web`) — the Web/edge client. |
38 | | - |
39 | | -`client-node` and `client-web` are slated to be **separated into fully independent packages**. Because |
40 | | -of that, some logic is **intentionally duplicated** between the two connection implementations |
41 | | -(`packages/client-node/src/connection/node_base_connection.ts` and |
42 | | -`packages/client-web/src/connection/web_connection.ts`) rather than hoisted into `client-common` — for |
43 | | -example the per-request `use_multipart_params` resolution and the `param_*` multipart-part assembly |
44 | | -loop. **Do not flag this node/web duplication as something to consolidate**, and prefer keeping each |
45 | | -client self-contained over adding shared helpers that only exist to remove the duplication. Genuinely |
46 | | -platform-agnostic primitives (like `buildMultipartBody`) still belong in `client-common`. |
| 7 | +- [`packages/AGENTS.md`](packages/AGENTS.md) — client source packages: log-message conventions, package structure, and intentional node/web duplication. |
| 8 | +- [`examples/AGENTS.md`](examples/AGENTS.md) — the example corpus layout and conventions. |
| 9 | +- [`skills/AGENTS.md`](skills/AGENTS.md) — shipped agent skills and how they are declared. |
| 10 | + - [`skills/clickhouse-js-node-rowbinary-parser/AGENTS.md`](skills/clickhouse-js-node-rowbinary-parser/AGENTS.md) — `@clickhouse/rowbinary` reader/writer conventions (tests, no defensive validation). |
| 11 | +- [`docs/AGENTS.md`](docs/AGENTS.md) — embedded troubleshooting / how-to pages. |
| 12 | +- [`tests/clickhouse-test-runner/AGENTS.md`](tests/clickhouse-test-runner/AGENTS.md) — the upstream SQL test harness and allowlist strategy. |
47 | 13 |
|
48 | 14 | ## Code intelligence (TypeScript LSP) |
49 | 15 |
|
50 | 16 | The repository ships `typescript-language-server` as a root devDependency, so after `npm install` you can start a TypeScript language server with `npx typescript-language-server --stdio` from the repo root for precise go-to-definition, find-references, hover (signatures and JSDoc, including `@deprecated`), workspace symbol search, completions, and type diagnostics. Prefer it over text search when resolving symbols or usages across the `packages/*` workspaces. See [`.claude/skills/typescript-lsp/SKILL.md`](.claude/skills/typescript-lsp/SKILL.md) for verified capabilities and protocol notes. |
51 | 17 |
|
52 | | -## Examples |
53 | | - |
54 | | -The repository contains an [`examples`](examples) directory that is being refactored to be AI-agent-friendly. |
55 | | -The goals of the refactor are: |
56 | | - |
57 | | -1. Examples should be runnable right away, with no manual edits required to get them working against a |
58 | | - local ClickHouse instance (use `docker-compose up` from the repo root for the default setup). |
59 | | -2. Examples are organized by client flavor and tailored to the corresponding runtime: |
60 | | - - [`examples/node`](examples/node) — examples for the Node.js client (`@clickhouse/client`). These |
61 | | - may freely use Node.js-only APIs (file streams, TLS, `http`, `node:*` built-ins, etc.) and import |
62 | | - Node built-ins using the `node:` prefix (e.g., `node:fs`, `node:path`, `node:stream`). |
63 | | - - [`examples/web`](examples/web) — examples for the Web client (`@clickhouse/client-web`). These |
64 | | - must only use Web-platform APIs (e.g., `globalThis.crypto.randomUUID()` instead of Node's |
65 | | - `crypto` module) and must not depend on Node.js-only modules. |
66 | | -3. `examples/node` and `examples/web` are independent npm packages, each with its own `package.json`, |
67 | | - `tsconfig.json`, and ESLint config. Keep dependencies and configuration scoped to the relevant |
68 | | - subpackage. |
69 | | -4. General-purpose scenarios (configuration, ping, inserts, selects, parameters, sessions, etc.) should |
70 | | - exist in both subdirectories where applicable, with the only differences being the `import` |
71 | | - statement and any platform-specific adjustments. Examples that rely on Node.js-only APIs live only |
72 | | - under `examples/node`. |
73 | | -5. Within each subpackage, examples are split into intent-driven **use-case folders** so each folder |
74 | | - can back a focused AI agent skill: |
75 | | - - `coding/` — day-to-day client API usage (configure, ping, basic insert/select, parameter |
76 | | - binding, sessions, data types, custom JSON). |
77 | | - - `performance/` — async inserts, streaming with backpressure, file/Parquet streams, progress |
78 | | - streaming, server-side bulk moves. Mostly Node-only; `examples/web/performance/` exists for the |
79 | | - few perf scenarios that work in the browser (e.g. streaming `JSONEachRow`). |
80 | | - - `troubleshooting/` — cancellation, timeouts, long-running query progress, server error surfaces, |
81 | | - number-precision pitfalls. |
82 | | - - `security/` — TLS, RBAC, SQL-injection-safe parameter binding. |
83 | | - - `schema-and-deployments/` — `CREATE TABLE` examples for each deployment shape and |
84 | | - deployment-shaped connection strings. |
85 | | -6. A small number of examples are **intentionally duplicated** across folders so each folder is a |
86 | | - self-contained skill corpus. Each duplicated example has one _primary_ location; the secondary |
87 | | - copies are excluded from the Vitest runner via the per-package `vitest.config.ts`. When you edit |
88 | | - a duplicated example, update **all** copies. The current duplicates and their primary locations |
89 | | - are listed in [`examples/README.md`](examples/README.md#editing-duplicated-examples). |
90 | | - |
91 | | -## Skills |
92 | | - |
93 | | -- Each shipped skill must also be listed in the `agents.skills` array of |
94 | | - [`packages/client-node/package.json`](packages/client-node/package.json) so downstream tooling can |
95 | | - discover it. The [`Skills E2E`](.github/workflows/e2e-skills.yml) workflow |
96 | | - (`tests/e2e/skills/check.js`) asserts that the packaged tarball contains the declared skills. |
97 | | - |
98 | | -## RowBinary skill (`@clickhouse/rowbinary`) tests |
99 | | - |
100 | | -The [`skills/clickhouse-js-node-rowbinary-parser`](skills/clickhouse-js-node-rowbinary-parser) package |
101 | | -has a symmetric reader/writer codebase, and its tests follow a few conventions worth preserving: |
102 | | - |
103 | | -- **Reader and writer tests are separate files.** Readers are tested in `tests/*.test.ts`; writers in |
104 | | - `tests/*.write.test.ts`. Keep the two independent: a writer test must **never** decode its bytes back |
105 | | - through a reader (and vice versa), so a bug on one side cannot mask a bug on the other. Writer tests |
106 | | - assert the encoded bytes against **live ClickHouse output** as the source of truth. |
107 | | -- **Each case is an isolated `it()` with a fully-inline body.** Write the assertion out per case, e.g. |
108 | | - `expect(encode(writer, value)).toEqual(await query("SELECT … FORMAT RowBinary"))`. Do **not** hide the |
109 | | - assertion behind a thunk-factory helper (`it("name", expectFoo(...))`), and do **not** wrap the query |
110 | | - in a per-file helper — embed the literal SQL inline, including any `SETTINGS` clause, so the full |
111 | | - query is visible in the test. The only shared helpers are the generic `query()` (`tests/clickhouse.ts`, |
112 | | - runs SQL → bytes) and `encode()` (`tests/encode.ts`, value → bytes). Repeating SQL across cases is |
113 | | - fine; reviewability beats DRY here. See [`tests/Integers.write.test.ts`](skills/clickhouse-js-node-rowbinary-parser/tests/Integers.write.test.ts) |
114 | | - as the canonical example. |
115 | | - |
116 | | -## Embedded docs |
117 | | - |
118 | | -The [`docs/`](docs) directory holds long-form troubleshooting / how-to pages that log messages and |
119 | | -skill references can link to (e.g. `docs/socket_hang_up_econnreset.md`, `docs/howto/`). Prefer |
120 | | -adding new pages here over linking out to external docs from log messages. |
121 | | - |
122 | | -## Upstream SQL test harness |
123 | | - |
124 | | -The [`tests/clickhouse-test-runner`](tests/clickhouse-test-runner) harness is a Node.js port of `clickhouse-client` that allows the official ClickHouse Python test runner (`tests/clickhouse-test`) to drive a subset of the upstream SQL test suite against `@clickhouse/client`. |
125 | | - |
126 | | -### What the harness does |
127 | | - |
128 | | -- Wraps `@clickhouse/client` in a tiny CLI (`bin/clickhouse` → `dist/main.js`) that mimics enough of the upstream `clickhouse-client` binary (same flags, `extract-from-config` shortcut, stdin/`--query` behavior) for the Python `tests/clickhouse-test` runner to drive it without modification. |
129 | | -- The runner is an npm workspace of the root `clickhouse-js` package, so `npm install` from the repo root links `@clickhouse/client` and `@clickhouse/client-common` from the local checkout instead of resolving them from the npm registry. Always install + build from the repo root (`npm install && npm run build`) so the harness exercises the code under review rather than the last published client. |
130 | | -- The CI matrix runs the harness against ClickHouse `latest` and `head` so that we exercise `@clickhouse/client` against both server versions and detect server regressions. The allowlist is also split into round-robin shards (`SHARD_INDEX` / `SHARD_TOTAL`) so each matrix job stays at roughly one minute; bump both the `shard` matrix values and the `SHARD_TOTAL` env value in the workflow together if per-shard runtime climbs back above ~1 minute. |
131 | | -- Reads the curated test list from [`upstream-allowlist.txt`](tests/clickhouse-test-runner/upstream-allowlist.txt) (one test name per line, `#` for comments) and forwards them as positional arguments to `tests/clickhouse-test`. |
132 | | -- The `SERVER_SETTINGS`/`CLIENT_ONLY_SETTINGS` allowlists in [`src/settings.ts`](tests/clickhouse-test-runner/src/settings.ts) are copied from the Java port and may need periodic resync as ClickHouse adds or reclassifies settings. |
133 | | - |
134 | | -See [`tests/clickhouse-test-runner/README.md`](tests/clickhouse-test-runner/README.md) for build, usage, and environment-variable documentation. When harness behavior changes (new wrapper flags, new short-circuited keys in `bin/clickhouse`, new entries in the settings allowlists), review the README and [`.github/workflows/upstream-sql-tests.yml`](.github/workflows/upstream-sql-tests.yml) to keep them in sync with the implementation. |
135 | | - |
136 | | -### Strategy for growing the allowlist |
137 | | - |
138 | | -The allowlist is grown in **batches of ~100 candidate tests at a time**, in upstream filename order, following this loop: |
139 | | - |
140 | | -1. **Pre-filter the candidate batch.** Skip non-SQL tests (`.sh`, `.py`, `.j2`) and tests tagged for unsupported infrastructure (`shard`, `distributed`, `replicated`, `zookeeper`, `kafka`, `s3`, `mysql`, `tls`, etc.). These will never pass through this harness as it stands today. |
141 | | -2. **Run each candidate through the harness** with `--no-stateful --no-long`. **Only keep tests that report `[ OK ]`**; drop failures and skips. |
142 | | -3. **Validate against the CI matrix before committing**, not just one local server version. The CI workflow runs `{ClickHouse latest, head} × {shard 1..N}` — a test that passes locally on `head` may fail on `latest` (or vice versa) and break CI. |
143 | | -4. **Beware substring/prefix expansion.** `tests/clickhouse-test` treats positional arguments as **substring/prefix matches** rather than exact names, so an allowlist entry like `00396_uuid` will silently pull in `00396_uuid_v7`, `00712_prewhere_with_alias` will pull in `00712_prewhere_with_alias_bug_2`, etc. When adding an entry whose name is a prefix of any other test in `0_stateless`, prefer the longest unambiguous form, or accept that the siblings come along and verify they all pass. |
144 | | -5. **Prune flakes promptly.** If a previously-passing test starts to flake on the nightly run, remove it (or its prefix-expanded siblings) from the allowlist rather than retrying — the allowlist exists to be a stable green signal, not a TODO list. |
145 | | - |
146 | 18 | ## When reviewing code changes |
147 | 19 |
|
148 | 20 | For every pull request review, make sure to provide an evaluation of the following aspects: |
|
0 commit comments