feat(#630): extract progress-stream and late-exception primitives (phase 3)

.wiki/Decisions-and-Roadmap.md

@@ -179,14 +179,36 @@ Two roadmap tracks are current:
   own, exposing only its `.` export — and turns
   `src/net/clickhouse-http-transport.ts` into a temporary compatibility
   adapter whose `send()` delegates to the package's `request()`;
-  `streamLines()` stays local, deferred to a later phase. `ch-client.ts`'s
-  composition graph, auth/epoch/retry policy, and eager pre-credential
-  `chUrl` preflight are all unchanged. See
+  `streamLines()` stayed local at that point, deferred to Phase 3.
+  `ch-client.ts`'s composition graph, auth/epoch/retry policy, and eager
+  pre-credential `chUrl` preflight are all unchanged. **Phase 3** (merged)
+  moves the progress-bearing JSON-lines read loop (`streamLines`, plus the
+  canonical `StreamLine`/`StreamCallbacks`/`ProgressMetaColumn` wire types)
+  and the HTTP exception-text parser + byte-safe late-exception framer
+  (`parseExceptionText`, `findExceptionFrame`/`ExceptionFrame` — now
+  `Uint8Array`-in, no caller-side latin1 conversion) into the package too —
+  a real move+delete, not an additive compatibility layer: the transport
+  adapter and its type contract are now request/send-only, and
+  `core/stream.ts` no longer declares a second copy of the wire type.
+  `runQuery` (itself under `src/net/**`) calls the package's `streamLines`
+  directly rather than through the transport seam, since there is exactly
+  one production stream implementation now. SQL Browser keeps `StreamResult`,
+  row caps, percentages, raw/result presentation, editor-caret positioning,
+  and auth-expiry/denial UI policy exactly where they were —
+  `applyStreamLine` now narrows an open `Record<string, unknown>` parsed
+  record instead of re-declaring the package's wire type. Still deferred to
+  later phases: `queryJson`/`queryText`/`queryProgress` convenience APIs,
+  `ensureClickHouseSuccess`, `ClickHouseError`, package `KILL QUERY`, SQL
+  quoting/type-grammar extraction, an authentication-composition rewrite,
+  and `runQuery`/`exportQuery`/the remaining request transport seam's own
+  eventual migration/deletion (Phase 7). See
   [[Source-Map]] and [[Architecture]] for the file-level detail and
-  `build/check-boundaries.mjs`'s Rules A–D for the mechanical boundary
-  enforcement (package↔root-src ban, package zero-bare-specifier ban,
-  root↔package-deep-import ban, bare-import location restricted to
-  `src/net/**`).
+  `build/check-boundaries.mjs`'s Rules A–D plus the Phase 3 narrow
+  legacy-owner rule for the mechanical boundary enforcement (package↔root-src
+  ban, package zero-bare-specifier ban, root↔package-deep-import ban,
+  bare-import location restricted to `src/net/**`, and the former
+  transport/contract/`core/stream.ts` owners rejected from regaining any
+  moved identifier).
 
 Re-read GitHub before acting because issue state can change; a MERGED PR is
 not proof its code is on `main` (see the reset above).

.wiki/Source-Map.md

@@ -16,10 +16,10 @@ Back to [[Home]]. Related: [[Architecture]], [[Product-and-Features]].
 | `src/dashboard/application/dashboard-repaint-plan.js` | pure repaint-decision arbitration extracted from `ui/dashboard.js`'s `renderDashboard` effect (#589) |
 | `src/ui/dashboard-tile-gestures.js` | Dashboard corner-drag resize, Command/Ctrl-drag reorder, and modifier-cue controller, extracted from `ui/dashboard.js` behind an injected `TileGestureDeps` seam (#589) |
 | `src/state.js` | signals-backed state model and persistence operations |
-| `src/net/ch-client.js` | ClickHouse HTTP execution and schema calls; auth/epoch/retry policy, product operations, `ChCtx` (#585 Phase 1: generic request/stream mechanics delegate through the transport seam below; #630 Phase 2: `chUrl` re-exported from `@altinity/clickhouse-http`) |
-| `src/net/clickhouse-transport.types.js` | Type-only `ClickHouseTransport` contract (`send`/`streamLines`); `TransportDeps`/`TransportRequest` alias the package's own types (#585 Phase 1; #630 Phase 2) |
-| `src/net/clickhouse-http-transport.js` | `createHttpTransport` — temporary compatibility adapter: `send()` delegates to `@altinity/clickhouse-http`'s `request()`; `streamLines()` (progress-bearing JSON-lines loop) stays local (#585 Phase 1; #630 Phase 2) |
-| `packages/clickhouse-http/src/` | First-party npm workspace package (repo's first) — `url.ts` (`chUrl`, the ONE URL-serializer implementation), `client.ts` (`createClickHouseHttpClient`, the low-level request/Fetch invocation); public export only, zero runtime dependencies, zero bare-specifier imports, no SQL Browser `src/**` dependency (#630 Phase 2) |
+| `src/net/ch-client.js` | ClickHouse HTTP execution and schema calls; auth/epoch/retry policy, product operations, `ChCtx` (#585 Phase 1: generic request/stream mechanics delegate through the transport seam below; #630 Phase 2: `chUrl` re-exported from `@altinity/clickhouse-http`; #630 Phase 3: `streamLines` called directly, `parseExceptionText`/`findExceptionFrame`/`StreamLine`/`StreamCallbacks` re-exported) |
+| `src/net/clickhouse-transport.types.js` | Type-only `ClickHouseTransport` contract — `send()` ONLY since #630 Phase 3 (`streamLines`/`StreamCallbacks` moved to the package); `TransportDeps`/`TransportRequest` alias the package's own types (#585 Phase 1; #630 Phase 2) |
+| `src/net/clickhouse-http-transport.js` | `createHttpTransport` — temporary compatibility adapter, REQUEST/SEND-ONLY since #630 Phase 3: `send()` delegates to `@altinity/clickhouse-http`'s `request()`; no stream member at all (`ch-client.ts`'s `runQuery` calls the package's `streamLines` directly instead) (#585 Phase 1; #630 Phases 2-3) |
+| `packages/clickhouse-http/src/` | First-party npm workspace package (repo's first) — `url.ts` (`chUrl`, the ONE URL-serializer implementation), `client.ts` (`createClickHouseHttpClient`, the low-level request/Fetch invocation), `progress-stream.ts` (`streamLines`, the ONE progress-bearing JSON-lines read loop, plus the canonical `StreamLine`/`StreamCallbacks`/`ProgressMetaColumn` wire types), `exceptions.ts` (`parseExceptionText`, `findExceptionFrame`/`ExceptionFrame` — byte-oriented, no caller-side latin1 conversion) — public export only, zero runtime dependencies, zero bare-specifier imports, no SQL Browser `src/**` dependency (#630 Phase 2; progress-stream/exceptions since Phase 3) |
 | `src/net/oauth.js` | OAuth flow/token exchange |
 | `src/editor/editor-port.js` | SQL editor contract and safe no-op port |
 | `src/editor/codemirror-adapter.js` | SQL CodeMirror 6 adapter |

CHANGELOG.md

@@ -10,6 +10,43 @@ auto-generated per-PR notes; this file is the curated, human-readable history.
 ## [Unreleased]
 
 ### Added
+- **#630 Phase 3: move the progress-stream read loop and HTTP exception
+  parsing/late-exception framing into `@altinity/clickhouse-http`.**
+  `packages/clickhouse-http` now owns the progress-bearing JSON-lines read
+  loop (`streamLines`, plus the canonical `StreamLine`/`StreamCallbacks`/
+  `ProgressMetaColumn` wire types — `progress-stream.ts`) and the ClickHouse
+  HTTP exception-text parser + byte-safe late-exception framer
+  (`parseExceptionText`, `findExceptionFrame`/`ExceptionFrame` —
+  `exceptions.ts`); `findExceptionFrame` now takes the retained tail as a raw
+  `Uint8Array` directly rather than a caller-supplied latin1 surrogate
+  string. This is a real move+delete, not an additive compatibility layer:
+  `src/net/clickhouse-http-transport.ts` and `clickhouse-transport.types.ts`
+  are now request/send-only (no `streamLines`/`StreamCallbacks` member at
+  all); `src/core/stream.ts` no longer declares `StreamLine`, `splitBuffer`,
+  `parseExceptionText`, `ExceptionFrame`, or `findExceptionFrame`. `runQuery`
+  (`src/net/ch-client.ts`, itself under `src/net/**`) calls the package's
+  `streamLines` directly instead of through the transport seam;
+  `export-service.ts`'s `streamToFile` calls the package's byte-oriented
+  `findExceptionFrame` on its retained holdback directly, through
+  `ch-client.ts`'s zero-logic re-export gateway, deleting the caller-side
+  latin1 converter it used to carry. SQL Browser keeps everything downstream
+  of the wire exactly where it was: `StreamResult`, row caps, progress
+  percentages, in-band exception → `result.error` folding (`applyStreamLine`,
+  now narrowed to an open `Record<string, unknown>` parsed-record boundary
+  rather than a second declared copy of the wire type), raw/result
+  presentation, editor-caret positioning (`parseErrorPos`), and
+  auth-expiry/denial UI policy (`isAuthExpiredBody`, `authDeniedMessage`).
+  `build/check-boundaries.mjs` gains a narrow legacy-owner regression rule
+  rejecting the three former owners from regaining any moved identifier,
+  mirrored in `tests/unit/clickhouse-http-package-policy.test.js` with its
+  own sabotage probes and a checker-source drift binding. The real-browser
+  Chromium/WebKit transport fault harness
+  (`tests/e2e/clickhouse-http-transport.{html,spec.js}`) now streams through
+  the package's `streamLines` directly instead of a transport method. This is
+  phase 3 of 8 (issue #630); consuming query APIs, SQL quoting/type grammar,
+  auth composition, and final migration/deletion of `runQuery`/`exportQuery`/
+  the remaining request transport seam remain later phases.
+
 - **#630 Phase 2: create `@altinity/clickhouse-http`, the repository's first npm
   workspace package, and move low-level request/URL mechanics into it.**
   `packages/clickhouse-http` now owns `chUrl()`/URL serialization and a

CLAUDE.md

@@ -24,12 +24,14 @@ all bundled — see hard rule 4). Quality is held by tests.
    policy* (OAuth, `ChCtx`, auth/epoch/retry, product operations) goes in
    `src/net/`, with the fetch seam *injected*, never imported. Reusable,
    product-agnostic ClickHouse HTTP/Fetch mechanics (URL serialization, the
-   low-level request) may live in the first-party workspace package
-   `packages/clickhouse-http` (#630 Phase 2) instead — `src/net/**` is the
-   only place allowed to import it, by its exact public package name, never
-   a deep import into its `src/**`; the package itself may depend on
-   nothing under SQL Browser `src/**` and declares zero runtime
-   dependencies (mechanically enforced, `build/check-boundaries.mjs`).
+   low-level request, the progress-stream wire shape and its reader/decoder
+   loop, and HTTP exception-text/late-exception byte framing) may live in the
+   first-party workspace package `packages/clickhouse-http` (#630 Phase 2;
+   the progress-stream/exception primitives since Phase 3) instead —
+   `src/net/**` is the only place allowed to import it, by its exact public
+   package name, never a deep import into its `src/**`; the package itself
+   may depend on nothing under SQL Browser `src/**` and declares zero
+   runtime dependencies (mechanically enforced, `build/check-boundaries.mjs`).
    DOM rendering goes in `src/ui/` as functions that take the
    `app` controller — except the editor, which lives in `src/editor/` behind the
    injected editor seams (#143/#212): only `main.js` imports concrete adapters,

build/check-boundaries.mjs

@@ -13,13 +13,21 @@
 // compile time. Extend RULES below in later phases rather than growing a
 // second script.
 //
-// Hand-rolled regex scan, no AST parser: the codebase has no exotic import
-// syntax, so scanning for import/export specifiers is enough and keeps this
-// a zero-dependency, sub-second pretest step.
+// Hand-rolled regex scan for the import-specifier rules: the codebase has no
+// exotic import syntax, so scanning for import/export specifiers is enough
+// and keeps those rules a zero-dependency, sub-second pretest step. The one
+// exception is the Phase 3 legacy-owner rule at the bottom, which needs
+// identifier-level (not specifier-level) detection and therefore delegates to
+// a real TypeScript parse in `build/lib/check-legacy-owners.mjs` — see that
+// module for why textual matching was retired there.
 
 import fs from 'node:fs';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
+import {
+  findLegacyOwnerViolations,
+  PHASE3_LEGACY_OWNER_FILES,
+} from './lib/check-legacy-owners.mjs';
 
 const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
 const SOURCE_EXT = /\.(ts|tsx|js|mjs)$/;
@@ -333,6 +341,27 @@ for (const file of collectFiles(path.join(repoRoot, 'src'))) {
   }
 }
 
+// Issue #630 Phase 3 — narrow legacy-owner regression rule: the former
+// production owners of the moved progress-stream/exception-parsing
+// primitives must not regain them — not as a second implementation and not
+// as a forwarding wrapper. The detection is a real TypeScript parse (an AST
+// identifier/property walk), shared with the unit suite via
+// `build/lib/check-legacy-owners.mjs`; see that module for the owner/name
+// lists and for why the earlier hand-rolled comment/string/template/regex
+// scanner was retired. Deliberately narrower than a repository-wide
+// function-name ban (Phase 8 owns broader anti-regrowth hardening): exactly
+// the three former owners, exactly the names Phase 3 moved out of them.
+// `applyStreamLine` (SQL Browser result policy, never moved) stays allowed:
+// it is a different identifier, and the AST walk matches exact names only.
+for (const relFile of PHASE3_LEGACY_OWNER_FILES) {
+  const file = path.join(repoRoot, relFile);
+  if (!fs.existsSync(file)) continue;
+  checkedFiles += 1;
+  for (const name of findLegacyOwnerViolations(fs.readFileSync(file, 'utf8'), relFile)) {
+    violations.push(`${relFile} → regained ${name} (issue #630 Phase 3: the moved stream/exception primitives are owned by @altinity/clickhouse-http — a former owner must not redeclare, re-import, or forward them)`);
+  }
+}
+
 if (violations.length) {
   console.error('check-boundaries: architecture violations:');
   for (const line of violations) console.error(`  ${line}`);

build/lib/check-legacy-owners.mjs

@@ -0,0 +1,119 @@
+// Issue #630 Phase 3 — the narrow legacy-owner ownership check, shared by
+// `build/check-boundaries.mjs` (the `check:arch` gate) and
+// `tests/unit/clickhouse-http-package-policy.test.js` (the in-suite mirror,
+// which imports THIS module rather than maintaining a second copy of the
+// scanning algorithm — the two-implementations-that-drift convention was
+// retired for this rule after three review passes each found a real lexical
+// bypass in the hand-rolled comment/string/template/regex scanner it used).
+//
+// The check is a real TypeScript parse, not textual matching. typescript@7
+// (the native, Go-based compiler) ships no in-process JS parser — the classic
+// `ts.createSourceFile(fileName, text, …)` does not exist in its public
+// surface — but its `typescript/unstable/sync` API spawns the bundled native
+// `tsc` binary (`--api` mode, a synchronous MessagePack RPC channel over
+// stdio; see `dist/api/syncChannel.js`) and decodes real parser output into
+// walkable JS AST nodes. Fed through `typescript/unstable/fs`'s virtual file
+// system, that parses an arbitrary in-memory string deterministically and
+// synchronously — comments, strings, template literals, and regex-vs-division
+// are resolved by the actual full grammar, so none of the three historical
+// scanner bypasses (comment markers inside strings, `//` inside a regex
+// literal, a regex literal after a control-flow `)`) is even representable
+// here. `typescript` stays a devDependency of the root: this module is
+// build/test tooling only and must never be imported by `src/**` or
+// `packages/clickhouse-http/**` runtime code.
+//
+// Scope stays deliberately narrow (this is NOT a generic static-analysis
+// framework): exactly the three former production owners of the moved
+// progress-stream/exception-parsing primitives, and exactly the identifier/
+// property names Phase 3 moved into `@altinity/clickhouse-http`. An AST walk
+// flags any Identifier with a moved name — a declaration, an import/export
+// specifier, a member reference — and any string-literal property/member
+// name (`{ "streamLines": … }`), so a second implementation and a forwarding
+// wrapper both fail. Intentionally obfuscated constructs (computed strings,
+// dynamically built property names) are outside this check's threat model.
+// Comments and JSDoc are trivia to the parser, so prose narrating the move
+// can never false-positive.
+
+import path from 'node:path';
+import { API } from 'typescript/unstable/sync';
+import { createVirtualFileSystem } from 'typescript/unstable/fs';
+import { SyntaxKind } from 'typescript/unstable/ast';
+
+/** Every symbol Phase 3 moved out of the legacy owners into the package
+ *  (`splitBuffer` was absorbed into the package's stream loop rather than
+ *  re-exported, but regaining it would equally restore the old surface). */
+export const PHASE3_MOVED_NAMES = Object.freeze([
+  'streamLines',
+  'parseExceptionText',
+  'findExceptionFrame',
+  'splitBuffer',
+  'StreamLine',
+  'StreamCallbacks',
+  'ProgressMetaColumn',
+  'ExceptionFrame',
+]);
+
+/** The three former production owners. `src/net/ch-client.ts` is NOT one of
+ *  them: it is the sanctioned consumer/migration-plumbing surface and
+ *  legitimately imports the moved names from the package. */
+export const PHASE3_LEGACY_OWNER_FILES = Object.freeze([
+  'src/net/clickhouse-http-transport.ts',
+  'src/net/clickhouse-transport.types.ts',
+  'src/core/stream.ts',
+]);
+
+const MOVED = new Set(PHASE3_MOVED_NAMES);
+
+/**
+ * Parse `source` with the real TypeScript parser and return the moved names
+ * it declares or references (in `PHASE3_MOVED_NAMES` order, deduplicated).
+ * `filename` is the repo-relative path the source claims to be; files that
+ * are not one of the three legacy owners are out of scope and return `[]`.
+ *
+ * @param {string} source
+ * @param {string} filename repo-relative, forward-slash separated
+ * @returns {string[]} the forbidden names found (empty when clean)
+ */
+export function findLegacyOwnerViolations(source, filename) {
+  if (!PHASE3_LEGACY_OWNER_FILES.includes(filename)) return [];
+  // The virtual path keeps the real basename so the parser applies the right
+  // grammar for the file's extension (.ts here; never .tsx among the owners).
+  const virtualPath = `/legacy-owner-check/${path.posix.basename(filename)}`;
+  const api = new API({ fs: createVirtualFileSystem({ [virtualPath]: source }) });
+  try {
+    const snapshot = api.updateSnapshot({ openFiles: [virtualPath] });
+    const sourceFile = snapshot
+      .getDefaultProjectForFile(virtualPath)
+      ?.program.getSourceFile(virtualPath);
+    if (!sourceFile) {
+      // Fail loud, never silently-clean: an unparseable probe must not read
+      // as "no violations".
+      throw new Error(`check-legacy-owners: could not parse ${filename}`);
+    }
+    const found = new Set();
+    const walk = (node) => {
+      if (node.kind === SyntaxKind.Identifier && MOVED.has(node.text)) {
+        found.add(node.text);
+      }
+      // A quoted (non-computed) property/member name is an exact property
+      // declaration too: `{ "streamLines": impl }`, `"streamLines"() {}`,
+      // and the string module-export-name forms `import { "streamLines" as
+      // x }` / `export { x as "streamLines" }` (propertyName/name).
+      for (const nameNode of [node.name, node.propertyName]) {
+        if (
+          nameNode
+          && (nameNode.kind === SyntaxKind.StringLiteral
+            || nameNode.kind === SyntaxKind.NoSubstitutionTemplateLiteral)
+          && MOVED.has(nameNode.text)
+        ) {
+          found.add(nameNode.text);
+        }
+      }
+      node.forEachChild(walk);
+    };
+    walk(sourceFile);
+    return PHASE3_MOVED_NAMES.filter((name) => found.has(name));
+  } finally {
+    api.close(); // always reap the native child process
+  }
+}