From 27f77224cf2457aa92516596a022d30b919d298e Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Fri, 17 Jul 2026 13:57:04 +0000 Subject: [PATCH 1/6] =?UTF-8?q?refactor(#276):=20extract=20WorkbenchParame?= =?UTF-8?q?terSession=20=E2=80=94=20phase=204B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit src/application/workbench-parameter-session.ts (app.params) owns the {name:Type} parameter policy: analysis/prepare/gate/exec-view (#173/ #165), input-vs-execute hardening (#170), schema-inferred enum options (#172 v2), recent-value recording/clearing (#171), and the varValues/ filterActive/varRecent persistence wrappers. Thinner cut per plan review: renderVarStrip stays in app.ts as the DOM view calling session methods; the full view-model API is deferred. Live-state accessors (never snapshots — filter-bar/dashboard/results write varValues directly); app.hardenedVars aliases the session's own Set (tests call .has() on it). Both consumers re-pointed in one change: the workbench session's param hooks and the export block's direct calls (workbench-session.ts itself: zero edits). sessionParams/needsSession/ sessionParamsFor stay app.ts-local through Phase 4 (plan ruling — their final home is a Phase-5 decision). saveVarRecent has two documented entry points (App delegate + internal hook reading the live app property) to preserve app.test.ts's property-mock semantics unchanged. app.test.ts unmodified. Gate: 108 files / 3135 tests, build, check:arch green. New spec: 31 tests, 100/100/100/100. Part of #276 (Phase-4 combined PR, commit 1 of 4). Claude-Session: ad39bf19-1690-43c7-abb4-f0084fca00a2 Co-authored-by: Claude Fable 5 --- .../workbench-parameter-session.ts | 326 ++++++++++++++ src/ui/app.ts | 262 ++++------- src/ui/app.types.ts | 18 +- tests/helpers/fake-app.ts | 40 +- .../unit/workbench-parameter-session.test.ts | 419 ++++++++++++++++++ 5 files changed, 882 insertions(+), 183 deletions(-) create mode 100644 src/application/workbench-parameter-session.ts create mode 100644 tests/unit/workbench-parameter-session.test.ts diff --git a/src/application/workbench-parameter-session.ts b/src/application/workbench-parameter-session.ts new file mode 100644 index 0000000..cf5220d --- /dev/null +++ b/src/application/workbench-parameter-session.ts @@ -0,0 +1,326 @@ +// #276 Phase 4B1's WorkbenchParameterSession — the `{name:Type}` query- +// variable POLICY extracted from app.ts (issue #276 §7, THINNER CUT per plan +// review): analyze/prepare/gate/execution-view, the #170 hardening bookkeeping, +// the #172 v2 schema-cache enum-suggestion inference, and the #171 recent- +// value + persistence policy over `varValues`/`filterActive`/`varRecent`/ +// `varRecentDisabled`. Constructible without App/AppState/DOM, like +// `workbench-session.ts`/`schema-catalog-service.ts` before it. +// +// Deliberately NOT included (plan-review rulings): `renderVarStrip` (the DOM +// view) stays in app.ts wholesale, calling this session's methods directly — +// the full `analyze() -> ParameterViewModel[]` view-model API the issue +// sketches is deferred, not built here. `setRunBtn` (DOM) also stays in +// app.ts. `sessionParams`/`needsSession`/`sessionParamsFor` stay app.ts-local +// (they're `tab.chSession`/transport material — Phase 4C's concern, not this +// session's). +// +// Every state field this session reads/writes (`varValues`/`filterActive`/ +// `varRecent`/`varRecentDisabled`) stays a LIVE `AppState` field, never a +// snapshot — the session receives accessor closures (mirrors +// `dashboard-session.ts`'s own `DashboardSessionDeps` convention), and +// `filter-bar.ts`/`dashboard.ts`/`results.ts` keep mutating `app.state.*` +// directly (through the SAME live objects these accessors read), so they +// keep observing this session's own reads without any consumer edit. +// +// `hardenedVars` (#170 review: names whose value hardened to invalid) is +// owned here as a private `Set`, exposed as a plain (never +// reassigned, only mutated in place) property — `app.hardenedVars` in app.ts +// is assigned this SAME instance (not a copy), so `app.hardenedVars.has(...)` +// (app.test.ts) keeps working unchanged. +// +// `saveVarRecent`'s two faces: `saveVarRecent()` (this session's own, real, +// `saveJSON`-calling implementation — what `app.saveVarRecent`'s one-line +// delegate calls) vs. `deps.hooks.saveVarRecent()` (used ONLY internally, by +// `recordBoundParams`/`clearVarRecent`/`clearAllVarRecent`, to trigger the +// persist). The two are kept distinct on purpose: `deps.hooks.saveVarRecent` +// is wired by app.ts to `() => app.saveVarRecent()` — a property read on the +// mutable `app` object, evaluated fresh on every call — so a caller that +// swaps `app.saveVarRecent` (app.test.ts's `app.saveVarRecent = +// vi.fn(app.saveVarRecent)` mock-substitution case) still observes every +// automatic persist this policy performs, byte-identical to the +// pre-extraction code's own `app.saveVarRecent()` property-style call inside +// `clearVarRecent`/`clearAllVarRecent`/`recordBoundParams`. Routing the real +// implementation through that same hook would recurse (`app.saveVarRecent` +// -> hook -> `app.saveVarRecent` -> ...), so the hook and the real +// implementation are deliberately two different entry points to the same +// effect. + +import { + analyzeParameterizedSources, prepareParameterizedBatch, executionView, +} from '../core/param-pipeline.js'; +import type { + ParameterAnalysis, PreparedSource, PreparedBatch, PreparedFieldState, ValidationMode, FieldControl, +} from '../core/param-pipeline.js'; +import { isRowReturning } from '../core/sql-split.js'; +import { effectiveFilterActive } from '../state.js'; +import type { QueryTab, SaveJSON } from '../state.js'; +import { KEYS } from '../state.js'; +import type { RecentMap } from '../core/recent-values.js'; +import { recordRecent, clearRecent, clearAllRecent } from '../core/recent-values.js'; +import { enumValues, parseParamType } from '../core/param-type.js'; +import type { ParamComparisonEntry } from '../core/param-comparison.js'; +import { resolveComparisonColumnType } from '../core/from-scope.js'; +import type { SchemaDb } from '../core/from-scope.js'; + +// ── Construction deps ──────────────────────────────────────────────────────── + +export interface WorkbenchParameterSessionHooks { + /** Fired when `varGateBlocked` finds a blocking condition — the shell's own + * toast (app.ts wires this to `flashToast(message, { document: doc })`); + * this session never imports `ui/toast.js`. */ + onGateBlocked(message: string): void; + /** See this module's header comment: routed back through the mutable, + * test-visible `app.saveVarRecent` property (app.ts wires this to + * `() => app.saveVarRecent()`) rather than calling this session's own + * `saveVarRecent()` directly, so every automatic persist + * `recordBoundParams`/`clearVarRecent`/`clearAllVarRecent` performs stays + * observable through that exact seam. */ + saveVarRecent(): void; +} + +/** Live accessors onto `app.state` — read fresh on every call (never a + * snapshot), mirroring `dashboard-session.ts`'s own `DashboardSessionDeps` + * convention. `varValues`/`filterActive` are mutated IN PLACE by their + * consumers (renderVarStrip's `onValueInput`, filter-bar.ts, dashboard.ts) — + * no setter needed. `varRecent` is reassigned wholesale (a new `RecentMap` + * replaces the old one), hence `setVarRecent`. */ +export interface WorkbenchParameterSessionDeps { + varValues(): Record; + filterActive(): Record; + varRecent(): RecentMap; + setVarRecent(map: RecentMap): void; + varRecentDisabled(): boolean; + /** The schema-cache accessor for `inferredEnumOptions`'s #172 v2 + * suggestion tier — `state.schema.value` cast the same way the + * pre-extraction code did (only ever read against the workbench's own + * already-loaded schema cache, never a new query). */ + schema(): SchemaDb[] | null; + activeTab(): QueryTab; + /** The #173 wave wall clock (epoch ms) — matches app.ts's own `wallNow`. */ + wallNow(): number; + /** `core/storage.js`'s `saveJSON` seam (state.ts's own `SaveJSON` shape) — + * this session's own `saveVarValues`/`saveFilterActive`/`saveVarRecent`/ + * `saveVarRecentDisabled` call it directly against the KEYS below. */ + saveJSON: SaveJSON; + hooks: WorkbenchParameterSessionHooks; +} + +// ── The session ────────────────────────────────────────────────────────────── + +export interface WorkbenchParameterSession { + /** Names of `{name:Type}` variables whose value has hardened to invalid + * (#170 review) — a single `Set` instance, mutated in place (never + * reassigned), so app.ts's `app.hardenedVars = session.hardenedVars` + * aliases the SAME object other modules/tests read/mutate. */ + readonly hardenedVars: Set; + tabAnalysis(sql: string): ParameterAnalysis; + prepareAnalyzedBatch(analysis: ParameterAnalysis, wallNowMs: number, validationMode?: ValidationMode): PreparedBatch; + prepareTabBatch(sql: string, wallNowMs: number, validationMode?: ValidationMode): PreparedBatch; + prepareTabSource(sql: string, wallNowMs: number, validationMode?: ValidationMode): PreparedSource; + execStatementSql(stmt: string): string; + varGateBlocked(wallNowMs?: number): boolean; + hardenVar(name: string, field?: PreparedFieldState): void; + inputGate(analysis: ParameterAnalysis): { missing: string[]; invalid: string[]; errors: string[] }; + inferredEnumOptions( + v: FieldControl, sql: string, comparisonColumns: Record, + ): string[] | null; + recordBoundParams(boundParams: Array<{ name: string; rawValue: unknown }>): void; + clearVarRecent(name: string): void; + clearAllVarRecent(): void; + saveVarValues(): void; + saveFilterActive(): void; + saveVarRecent(): void; + saveVarRecentDisabled(): void; +} + +/** Build a `WorkbenchParameterSession` bound to `deps`. Trivial constructor — + * no validation, no defaulting; the caller supplies every field exactly as + * it wants it used. */ +export function createWorkbenchParameterSession(deps: WorkbenchParameterSessionDeps): WorkbenchParameterSession { + // #170 review: names of `{name:Type}` variables whose value has hardened to + // invalid (blur/Enter/execute committed a strict verdict of invalid). + // setRunBtn's gate-less fallback (called from unrelated re-renders — + // renderVarStrip's tail call on every SQL-editor keystroke, and the + // hasSelection effect on every cursor/selection move) recomputes in + // lenient 'input' mode, which reads a still-incomplete prefix (e.g. a + // lone '-') as merely incomplete, not invalid — without this bookkeeping + // that recompute would silently re-enable Run while the field itself + // still paints red. Editing the field's value again (its own `oninput`) + // clears the name here, returning it to normal lenient behavior. + const hardenedVars = new Set(); + + // The optional-block activation map (#165): explicit filterActive entries + // win; params without one derive activation from their stored value. + const activeMap = (): Record => effectiveFilterActive(deps.varValues(), deps.filterActive()); + + // The workbench SQL as the pipeline's single parameterized source (#173). + function tabAnalysis(sql: string): ParameterAnalysis { + return analyzeParameterizedSources([ + { id: 'tab', label: 'editor tab', kind: 'tab', sql, bindPolicy: 'row-returning' }, + ]); + } + + // Analyze + prepare `sql` as the workbench's single parameterized source + // (#173): one call per SQL string per wave, drawing values from the shared + // varValues (+ the #165 activation map). Returns the full prepared batch + // ({fields, sources, diagnostics}) — `fields` is per-`{name:Type}` (#170's + // validated state, for the var-strip's inline affordance); `sources[0]` is + // this single source's `{statements, missing, invalid, errors, runnable}`. + // Args for a request come from a source's statements (or mergedSourceArgs + // when the SQL ships as one request); each statement's `sql` is its + // execution view (#165) — byte-identical for SQL without optional blocks. + function prepareAnalyzedBatch(analysis: ParameterAnalysis, wallNowMs: number, validationMode: ValidationMode = 'execute'): PreparedBatch { + return prepareParameterizedBatch(analysis, { + values: deps.varValues(), active: activeMap(), wallNowMs, validationMode, + }); + } + function prepareTabBatch(sql: string, wallNowMs: number, validationMode: ValidationMode = 'execute'): PreparedBatch { + return prepareAnalyzedBatch(tabAnalysis(sql), wallNowMs, validationMode); + } + function prepareTabSource(sql: string, wallNowMs: number, validationMode: ValidationMode = 'execute'): PreparedSource { + return prepareTabBatch(sql, wallNowMs, validationMode).sources[0]; + } + + // The execution text of one statement (#165): only active optional blocks + // retained, markers stripped — byte-identical for SQL without blocks. Follows + // the #134 bind gate: a non-row-returning statement passes through verbatim. + function execStatementSql(stmt: string): string { + return isRowReturning(stmt) ? executionView(stmt, activeMap()) : stmt; + } + + // Block execution while any {name:Type} variable in the active tab is unfilled + // or invalid, or while its value can't serialize (e.g. an array value against + // a scalar declaration) — toasting why (#134/#173). Gating on the whole + // tab.sqlDraft — the exact set the variable strip shows — keeps every execution + // path consistent: the Run button (setRunBtn), the Run/⌘↵ path, Explain, and + // Export all agree. `wallNowMs` is the caller's wave clock. + function varGateBlocked(wallNowMs: number = deps.wallNow()): boolean { + const tab = deps.activeTab(); + const src = tab ? prepareTabSource(tab.sqlDraft, wallNowMs) : null; + if (!src) return false; + const blockers = src.missing.concat(src.invalid); + if (blockers.length) { + deps.hooks.onGateBlocked('Enter a value for: ' + blockers.join(', ')); + return true; + } + if (src.errors.length) { + deps.hooks.onGateBlocked(src.errors[0]); + return true; + } + return false; + } + + // Keep `hardenedVars` (#170 review) in sync with a field's just-computed + // 'execute'-mode verdict: added when it's invalid, cleared otherwise — so a + // corrected-then-reharded value, or a variable that simply stopped being + // invalid, doesn't linger in the set. Shared by every place that commits a + // strict verdict for a field (blur, Enter, and the strip's initial/rebuild + // paint, which is itself an 'execute'-mode read of the persisted value). + function hardenVar(name: string, field?: PreparedFieldState): void { + if (field && field.state === 'invalid') hardenedVars.add(name); + else hardenedVars.delete(name); + } + + // The Run button's lenient ('input'-mode) gate for an already-computed + // analysis. #170 review: a field that hardened to invalid (blur/Enter/ + // execute committed a strict invalid verdict) must keep blocking Run even + // though this recompute is lenient — 'input' mode reads a still-incomplete + // prefix like '-' as merely incomplete, not invalid — so `hardenedVars` + // is folded in. Only names the batch actually declares are considered, so a + // hardened flag for a variable that dropped out of the tab's SQL (or + // belongs to a different tab) doesn't block Run forever; the + // `!src.invalid.includes` filter just avoids listing a name twice. Shared + // by setRunBtn's own fallback and renderVarStrip's tail (review F9: one + // analysis per strip repaint feeds both consumers). + function inputGate(analysis: ParameterAnalysis): { missing: string[]; invalid: string[]; errors: string[] } { + const gateBatch = prepareAnalyzedBatch(analysis, deps.wallNow(), 'input'); + const src = gateBatch.sources[0]; + const hardened = [...hardenedVars].filter((name) => name in gateBatch.fields && !src.invalid.includes(name)); + return { missing: src.missing, invalid: src.invalid.concat(hardened), errors: src.errors }; + } + + // #172 v2 (schema-cache inference — the SUGGESTION tier): the enum member + // list a plain `{name:String}` param's compared column implies, or null. + // The declared type's own Enum members (v1, authoritative and blocking — + // #170 validates those as a real Enum) are fieldControlKind's business; + // this helper only ever resolves the workbench's own SQL against the + // already-loaded schema cache (`paramComparisonColumns` + + // `resolveComparisonColumnType`), never a new query, and the declared type + // stays String, so #170 never blocks on a non-member. + function inferredEnumOptions( + v: FieldControl, sql: string, comparisonColumns: Record, + ): string[] | null { + // `.base` is value-transparent (#238): a `LowCardinality(String)` param + // reaches this suggestion tier too, same as plain `String` — intentional, + // since LowCardinality is just a storage encoding of String values. + if (parseParamType(v.type).base !== 'String') return null; + const cmp = comparisonColumns[v.name]; + if (!cmp) return null; + const colType = resolveComparisonColumnType(sql, cmp.pos, cmp, deps.schema()); + return colType ? enumValues(colType) : null; + } + + // --- #171 recent values + persistence ------------------------------------- + + function saveVarValues(): void { deps.saveJSON(KEYS.varValues, deps.varValues()); } + function saveFilterActive(): void { deps.saveJSON(KEYS.filterActive, deps.filterActive()); } + function saveVarRecent(): void { deps.saveJSON(KEYS.varRecent, deps.varRecent()); } + function saveVarRecentDisabled(): void { deps.saveJSON(KEYS.varRecentDisabled, deps.varRecentDisabled()); } + + // Record every successful statement's `boundParams` (#173's immutable + // per-statement snapshots) into the recent-value history — the single hook + // point every success path (run/runScript's single-statement + per-script- + // statement paths, and the dashboard's per-tile completion) calls. A no-op + // while the disable-history preference is on (existing history is left + // alone, only new recording stops) or when nothing was actually bound. + // Array-valued `rawValue` (an `Array(...)`-typed param) is skipped — v1 + // recents are a text-value affordance, like #172's (not yet built) enum + // controls; #160's curated-`filter:`-param opt-out hook has nothing to + // check yet (no curated param exists before #160 lands). + function recordBoundParams(boundParams: Array<{ name: string; rawValue: unknown }>): void { + if (deps.varRecentDisabled() || !boundParams || !boundParams.length) return; + let map = deps.varRecent(); + for (const p of boundParams) { + if (typeof p.rawValue !== 'string') continue; + map = recordRecent(map, p.name, p.rawValue); + } + if (map !== deps.varRecent()) { + deps.setVarRecent(map); + deps.hooks.saveVarRecent(); + } + } + // Per-field "Clear recent" (the dropdown footer) / "Clear all recent + // values" (the File menu) — both no-op (no re-persist) when there was + // nothing to clear, mirroring recordRecent's own same-reference no-op. + function clearVarRecent(name: string): void { + const next = clearRecent(deps.varRecent(), name); + if (next !== deps.varRecent()) { + deps.setVarRecent(next); + deps.hooks.saveVarRecent(); + } + } + function clearAllVarRecent(): void { + deps.setVarRecent(clearAllRecent()); + deps.hooks.saveVarRecent(); + } + + return { + hardenedVars, + tabAnalysis, + prepareAnalyzedBatch, + prepareTabBatch, + prepareTabSource, + execStatementSql, + varGateBlocked, + hardenVar, + inputGate, + inferredEnumOptions, + recordBoundParams, + clearVarRecent, + clearAllVarRecent, + saveVarValues, + saveFilterActive, + saveVarRecent, + saveVarRecentDisabled, + }; +} diff --git a/src/ui/app.ts b/src/ui/app.ts index 0e5027f..5ce8637 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -9,18 +9,16 @@ import { Icon } from './icons.js'; import { createState, activeTab, KEYS, recordHistory, recordScriptHistory, createSavedQuery, commitSavedQuery, savedForTab, tabPanel, - normalizeRowLimit, MOBILE_BREAKPOINT_PX, effectiveFilterActive, + normalizeRowLimit, MOBILE_BREAKPOINT_PX, } from '../state.js'; import type { QueryTab, AppState, SpecValidationService, HistoryResultSnapshot, QuerySpecDraft } from '../state.js'; import type { SavedQueryV2 } from '../generated/json-schema.types.js'; import { splitStatements, isRowReturning, leadingKeyword } from '../core/sql-split.js'; import { - analyzeParameterizedSources, prepareParameterizedBatch, mergedSourceArgs, mergedSourceSql, executionView, analysisView, + mergedSourceArgs, mergedSourceSql, analysisView, fieldControls, fieldControlKind, } from '../core/param-pipeline.js'; -import type { - ParameterAnalysis, PreparedSource, PreparedBatch, PreparedFieldState, ValidationMode, FieldControl, -} from '../core/param-pipeline.js'; +import type { PreparedSource } from '../core/param-pipeline.js'; import { hasOptionalBlocks } from '../core/optional-blocks.js'; import { saveJSON, saveStr } from '../core/storage.js'; import { sqlString, inferQueryName, shortVersion, userShortName, withStatementBreak, isSchemaMutatingSql, prepareExportSql, formatBytes, formatRows } from '../core/format.js'; @@ -66,11 +64,8 @@ import { buildEnumField } from './enum-field.js'; import type { EnumField } from './enum-field.js'; import { wireComboInput } from './combobox.js'; import type { ComboField } from './combobox.js'; -import { recordRecent, clearRecent, clearAllRecent, recentOptions } from '../core/recent-values.js'; -import { enumValues, parseParamType } from '../core/param-type.js'; +import { recentOptions } from '../core/recent-values.js'; import { paramComparisonColumns } from '../core/param-comparison.js'; -import type { ParamComparisonEntry } from '../core/param-comparison.js'; -import { resolveComparisonColumnType } from '../core/from-scope.js'; import type { SchemaDb } from '../core/from-scope.js'; import type { AssembledReference, CompletionItem } from '../core/completions.js'; import { libraryControls, renderLibraryTitle } from './file-menu.js'; @@ -86,6 +81,7 @@ import type { LineageFocus } from '../net/ch-client.js'; import { createQueryExecutionService } from '../application/query-execution-service.js'; import { createConnectionSession } from '../application/connection-session.js'; import { createSchemaCatalogService } from '../application/schema-catalog-service.js'; +import { createWorkbenchParameterSession } from '../application/workbench-parameter-session.js'; import { createWorkbenchSession } from './workbench/workbench-session.js'; /** Optional globals a plain browser page (or the CM6/Chart/dagre UMD bundles a @@ -164,15 +160,11 @@ export function createApp(env: CreateAppEnv = {}): App { state: createState(), dom: {}, // #170 review: names of `{name:Type}` variables whose value has hardened - // to invalid (blur/Enter/execute committed a strict verdict of invalid). - // setRunBtn's gate-less fallback (called from unrelated re-renders — - // renderVarStrip's tail call on every SQL-editor keystroke, and the - // hasSelection effect on every cursor/selection move) recomputes in - // lenient 'input' mode, which reads a still-incomplete prefix (e.g. a - // lone '-') as merely incomplete, not invalid — without this bookkeeping - // that recompute would silently re-enable Run while the field itself - // still paints red. Editing the field's value again (its own `oninput`) - // clears the name here, returning it to normal lenient behavior. + // to invalid — owned by the WorkbenchParameterSession (#276 Phase 4B1, + // application/workbench-parameter-session.ts), which the block below + // constructs; `app.hardenedVars` is aliased (not copied) to its `Set` + // once that session exists (see the `const params = …` block below), so + // this placeholder is only ever read before that assignment happens. hardenedVars: new Set(), root: env.root || doc.getElementById('root'), document: doc, @@ -227,50 +219,13 @@ export function createApp(env: CreateAppEnv = {}): App { app.saveJSON = saveJSON; app.saveStr = saveStr; app.savePref = (name, value) => saveStr(KEYS[name as keyof typeof KEYS], String(value)); - // Persist the shared query-variable values (#134) after each edit. - app.saveVarValues = () => saveJSON(KEYS.varValues, app.state.varValues); - // Persist the optional-block activation map (#165) alongside varValues. - app.saveFilterActive = () => saveJSON(KEYS.filterActive, app.state.filterActive); - // Persist the per-variable recent-value MRU history (#171) alongside - // varValues — same key convention, own key. - app.saveVarRecent = () => saveJSON(KEYS.varRecent, app.state.varRecent); - app.saveVarRecentDisabled = () => saveJSON(KEYS.varRecentDisabled, app.state.varRecentDisabled); - // Record every successful statement's `boundParams` (#173's immutable - // per-statement snapshots) into the recent-value history — the single hook - // point every success path (run/runScript's single-statement + per-script- - // statement paths, and the dashboard's per-tile completion) calls. A no-op - // while the disable-history preference is on (existing history is left - // alone, only new recording stops) or when nothing was actually bound. - // Array-valued `rawValue` (an `Array(...)`-typed param) is skipped — v1 - // recents are a text-value affordance, like #172's (not yet built) enum - // controls; #160's curated-`filter:`-param opt-out hook has nothing to - // check yet (no curated param exists before #160 lands). - app.recordBoundParams = (boundParams) => { - if (app.state.varRecentDisabled || !boundParams || !boundParams.length) return; - let map = app.state.varRecent; - for (const p of boundParams) { - if (typeof p.rawValue !== 'string') continue; - map = recordRecent(map, p.name, p.rawValue); - } - if (map !== app.state.varRecent) { - app.state.varRecent = map; - app.saveVarRecent(); - } - }; - // Per-field "Clear recent" (the dropdown footer) / "Clear all recent - // values" (the File menu) — both no-op (no re-persist) when there was - // nothing to clear, mirroring recordRecent's own same-reference no-op. - app.clearVarRecent = (name) => { - const next = clearRecent(app.state.varRecent, name); - if (next !== app.state.varRecent) { - app.state.varRecent = next; - app.saveVarRecent(); - } - }; - app.clearAllVarRecent = () => { - app.state.varRecent = clearAllRecent(); - app.saveVarRecent(); - }; + // The `{name:Type}` var-value/filter-active/recent-value persistence + // wrappers (saveVarValues/saveFilterActive/saveVarRecent/ + // saveVarRecentDisabled) + the recent-value policy that sits on top of them + // (recordBoundParams/clearVarRecent/clearAllVarRecent) now live in the + // WorkbenchParameterSession (#276 Phase 4B1) — see the `const params = …` + // block below, which wires `app.saveVarValues`/etc. as one-line delegates + // once that session exists. app.FileReader = (env.FileReader || win.FileReader) as typeof FileReader; // Exposed seam for the header File menu (file-menu.js): the file-download // helper (defined below). The library title (name + dirty dot) repaints via a @@ -588,62 +543,51 @@ export function createApp(env: CreateAppEnv = {}): App { function sessionParamsFor(tab: QueryTab, sqls: string[]): Record { return tab.chSession != null || needsSession(sqls) ? sessionParams(tab) : {}; } - // The workbench SQL as the pipeline's single parameterized source (#173). - function tabAnalysis(sql: string): ParameterAnalysis { - return analyzeParameterizedSources([ - { id: 'tab', label: 'editor tab', kind: 'tab', sql, bindPolicy: 'row-returning' }, - ]); - } - // The optional-block activation map (#165): explicit filterActive entries - // win; params without one derive activation from their stored value. - const activeMap = (): Record => effectiveFilterActive(app.state.varValues, app.state.filterActive); - // Analyze + prepare `sql` as the workbench's single parameterized source - // (#173): one call per SQL string per wave, drawing values from the shared - // varValues (+ the #165 activation map). Returns the full prepared batch - // ({fields, sources, diagnostics}) — `fields` is per-`{name:Type}` (#170's - // validated state, for the var-strip's inline affordance); `sources[0]` is - // this single source's `{statements, missing, invalid, errors, runnable}`. - // Args for a request come from a source's statements (or mergedSourceArgs - // when the SQL ships as one request); each statement's `sql` is its - // execution view (#165) — byte-identical for SQL without optional blocks. - function prepareAnalyzedBatch(analysis: ParameterAnalysis, wallNowMs: number, validationMode: ValidationMode = 'execute'): PreparedBatch { - return prepareParameterizedBatch(analysis, { - values: app.state.varValues, active: activeMap(), wallNowMs, validationMode, - }); - } - function prepareTabBatch(sql: string, wallNowMs: number, validationMode: ValidationMode = 'execute'): PreparedBatch { - return prepareAnalyzedBatch(tabAnalysis(sql), wallNowMs, validationMode); - } - function prepareTabSource(sql: string, wallNowMs: number, validationMode: ValidationMode = 'execute'): PreparedSource { - return prepareTabBatch(sql, wallNowMs, validationMode).sources[0]; - } - // The execution text of one statement (#165): only active optional blocks - // retained, markers stripped — byte-identical for SQL without blocks. Follows - // the #134 bind gate: a non-row-returning statement passes through verbatim. - function execStatementSql(stmt: string): string { - return isRowReturning(stmt) ? executionView(stmt, activeMap()) : stmt; - } - // Block execution while any {name:Type} variable in the active tab is unfilled - // or invalid, or while its value can't serialize (e.g. an array value against - // a scalar declaration) — toasting why (#134/#173). Gating on the whole - // tab.sqlDraft — the exact set the variable strip shows — keeps every execution - // path consistent: the Run button (setRunBtn), the Run/⌘↵ path, Explain, and - // Export all agree. `wallNowMs` is the caller's wave clock. - function varGateBlocked(wallNowMs: number = wallNow()): boolean { - const tab = app.activeTab(); - const src = tab ? prepareTabSource(tab.sqlDraft, wallNowMs) : null; - if (!src) return false; - const blockers = src.missing.concat(src.invalid); - if (blockers.length) { - flashToast('Enter a value for: ' + blockers.join(', '), { document: doc }); - return true; - } - if (src.errors.length) { - flashToast(src.errors[0], { document: doc }); - return true; - } - return false; - } + // The `{name:Type}` query-variable POLICY — analyze/prepare/gate/execution- + // view, the #170 hardening bookkeeping, the #172 v2 schema-cache enum- + // suggestion inference, and the #171 recent-value + persistence policy — + // now lives in `application/workbench-parameter-session.ts` (#276 Phase + // 4B1), constructible without App/AppState/DOM. `renderVarStrip` (the DOM + // view, below) and the workbench-session hooks + export block (further + // down) call its methods directly; `app.hardenedVars` is aliased (not + // copied) to its `Set` so `app.hardenedVars.has(...)` keeps working + // unchanged. `sessionParams`/`needsSession`/`sessionParamsFor` above stay + // HERE (they're `tab.chSession`/transport material, not parameter policy — + // Phase 4C's concern). + const params = createWorkbenchParameterSession({ + varValues: () => app.state.varValues, + filterActive: () => app.state.filterActive, + varRecent: () => app.state.varRecent, + setVarRecent: (map) => { app.state.varRecent = map; }, + varRecentDisabled: () => app.state.varRecentDisabled, + schema: () => app.state.schema.value as SchemaDb[] | null, + activeTab: () => app.activeTab(), + wallNow, + saveJSON, + hooks: { + onGateBlocked: (message) => flashToast(message, { document: doc }), + // Routed through the mutable, test-visible `app.saveVarRecent` + // property (a fresh property read every call) rather than + // `params.saveVarRecent()` directly — see + // workbench-parameter-session.ts's header comment: this keeps every + // automatic persist `recordBoundParams`/`clearVarRecent`/ + // `clearAllVarRecent` performs observable through the exact seam + // app.test.ts's `app.saveVarRecent = vi.fn(app.saveVarRecent)` + // mock-substitution case exercises, byte-identical to the + // pre-extraction code's own `app.saveVarRecent()` property call. + saveVarRecent: () => app.saveVarRecent(), + }, + }); + app.params = params; + // Alias (not copy) — see `params`'s own construction comment above. + app.hardenedVars = params.hardenedVars; + app.saveVarValues = () => params.saveVarValues(); + app.saveFilterActive = () => params.saveFilterActive(); + app.saveVarRecent = () => params.saveVarRecent(); + app.saveVarRecentDisabled = () => params.saveVarRecentDisabled(); + app.recordBoundParams = (boundParams) => params.recordBoundParams(boundParams); + app.clearVarRecent = (name) => params.clearVarRecent(name); + app.clearAllVarRecent = () => params.clearAllVarRecent(); // The run/runScript/runEntry/cancel orchestration (#276 Phase 3a) now lives // in ui/workbench/workbench-session.ts — a route-scoped session that owns @@ -665,7 +609,8 @@ export function createApp(env: CreateAppEnv = {}): App { loadSchema: () => { void app.loadSchema(); }, recordHistory: (tab, sql) => app.recordHistory(tab, sql), recordBoundParams: (bp) => app.recordBoundParams([...bp]), - prepareTabSource, varGateBlocked, execStatementSql, sessionParamsFor, + prepareTabSource: params.prepareTabSource, varGateBlocked: params.varGateBlocked, + execStatementSql: params.execStatementSql, sessionParamsFor, getSelectionText: () => app.sqlEditor.getSelection().text, tickElapsed, saveJSON, @@ -676,33 +621,9 @@ export function createApp(env: CreateAppEnv = {}): App { // Milliseconds since the running query started (0 when idle) — delegates to // the session's own private runT0 bookkeeping. app.elapsedMs = () => workbench.elapsedMs(); - // Keep `app.hardenedVars` (#170 review) in sync with a field's just-computed - // 'execute'-mode verdict: added when it's invalid, cleared otherwise — so a - // corrected-then-reharded value, or a variable that simply stopped being - // invalid, doesn't linger in the set. Shared by every place that commits a - // strict verdict for a field (blur, Enter, and the strip's initial/rebuild - // paint, which is itself an 'execute'-mode read of the persisted value). - function hardenVar(name: string, field?: PreparedFieldState): void { - if (field && field.state === 'invalid') app.hardenedVars.add(name); - else app.hardenedVars.delete(name); - } - // The Run button's lenient ('input'-mode) gate for an already-computed - // analysis. #170 review: a field that hardened to invalid (blur/Enter/ - // execute committed a strict invalid verdict) must keep blocking Run even - // though this recompute is lenient — 'input' mode reads a still-incomplete - // prefix like '-' as merely incomplete, not invalid — so `app.hardenedVars` - // is folded in. Only names the batch actually declares are considered, so a - // hardened flag for a variable that dropped out of the tab's SQL (or - // belongs to a different tab) doesn't block Run forever; the - // `!src.invalid.includes` filter just avoids listing a name twice. Shared - // by setRunBtn's own fallback and renderVarStrip's tail (review F9: one - // analysis per strip repaint feeds both consumers). - function inputGate(analysis: ParameterAnalysis): { missing: string[]; invalid: string[]; errors: string[] } { - const gateBatch = prepareAnalyzedBatch(analysis, wallNow(), 'input'); - const src = gateBatch.sources[0]; - const hardened = [...app.hardenedVars].filter((name) => name in gateBatch.fields && !src.invalid.includes(name)); - return { missing: src.missing, invalid: src.invalid.concat(hardened), errors: src.errors }; - } + // hardenVar/inputGate (#170 review bookkeeping) now live on `params` (see + // its construction above) — setRunBtn's fallback and renderVarStrip's tail + // call `params.inputGate`/`params.hardenVar` directly. function setRunBtn(running: boolean, gate?: { missing: string[]; invalid: string[]; errors: string[] }): void { if (!app.dom.runBtn) return; // Disabled while running, or while any detected {name:Type} query variable @@ -719,7 +640,7 @@ export function createApp(env: CreateAppEnv = {}): App { if (gate == null) { gate = running || !tab ? { missing: [], invalid: [], errors: [] } - : inputGate(tabAnalysis(tab.sqlDraft)); + : params.inputGate(params.tabAnalysis(tab.sqlDraft)); } const blockers = gate.missing.concat(gate.invalid); app.dom.runBtn!.disabled = running || blockers.length > 0 || gate.errors.length > 0; @@ -748,26 +669,9 @@ export function createApp(env: CreateAppEnv = {}): App { // same variables keeps the (already-correct, shared) values in place. Always // re-syncs the Run button's disabled/tooltip state. // - // #172 v2 (schema-cache inference — the SUGGESTION tier): the enum member - // list a plain `{name:String}` param's compared column implies, or null. - // The declared type's own Enum members (v1, authoritative and blocking — - // #170 validates those as a real Enum) are fieldControlKind's business; - // this helper only ever resolves the workbench's own SQL against the - // already-loaded schema cache (`paramComparisonColumns` + - // `resolveComparisonColumnType`), never a new query, and the declared type - // stays String, so #170 never blocks on a non-member. - function inferredEnumOptions( - v: FieldControl, sql: string, comparisonColumns: Record, - ): string[] | null { - // `.base` is value-transparent (#238): a `LowCardinality(String)` param - // reaches this suggestion tier too, same as plain `String` — intentional, - // since LowCardinality is just a storage encoding of String values. - if (parseParamType(v.type).base !== 'String') return null; - const cmp = comparisonColumns[v.name]; - if (!cmp) return null; - const colType = resolveComparisonColumnType(sql, cmp.pos, cmp, app.state.schema.value as SchemaDb[] | null); - return colType ? enumValues(colType) : null; - } + // #172 v2 (schema-cache inference — the SUGGESTION tier) now lives on + // `params.inferredEnumOptions` (see its construction above) — pure over + // schema + analysis, no DOM. function renderVarStrip(): void { const strip = app.dom.varStrip; if (!strip) return; @@ -776,7 +680,7 @@ export function createApp(env: CreateAppEnv = {}): App { // comparison scan, a rebuild's initial field paint, and the tail's Run- // button gate all feed off this single pass instead of re-analyzing the // same SQL a second time per editor keystroke. - const analysis = tab ? tabAnalysis(tab.sqlDraft) : null; + const analysis = tab ? params.tabAnalysis(tab.sqlDraft) : null; const vars = analysis ? fieldControls(analysis) : []; // #172 v2 scans the tab SQL's ANALYSIS materialization (review F2): in // the raw text a comparison inside a /*[ ]*/ optional block is one opaque @@ -787,7 +691,7 @@ export function createApp(env: CreateAppEnv = {}): App { const comparisonColumns = tab ? paramComparisonColumns(scanSql) : {}; // Each field's control kind + member list (shared enum > date-like > text // priority; a type-conflicted field degrades to text — fieldControlKind). - const controls = vars.map((v) => fieldControlKind(v, inferredEnumOptions(v, scanSql, comparisonColumns))); + const controls = vars.map((v) => fieldControlKind(v, params.inferredEnumOptions(v, scanSql, comparisonColumns))); // The signature folds in each var's control kind and resolved enum // options — not just name/type/optional — so a column landing on the // idle-tick loader (loadColumns calls renderVarStrip on completion) @@ -803,7 +707,7 @@ export function createApp(env: CreateAppEnv = {}): App { // gate-less fallback would re-analyze the identical SQL). Lazy so the // running / tab-less states (whose gate setRunBtn hard-empties anyway) // skip the prepare entirely. - const runGate = () => (analysis && !app.state.running.value ? inputGate(analysis) : undefined); + const runGate = () => (analysis && !app.state.running.value ? params.inputGate(analysis) : undefined); if (sig !== app.dom.varStripSig) { // A signature change while the user is focused INSIDE the strip would // replaceChildren() every field out from under them — a background @@ -842,7 +746,7 @@ export function createApp(env: CreateAppEnv = {}): App { // The freshly-(re)built strip paints each field's already-committed // state ('execute' mode — no field is mid-typing right after a // rebuild, e.g. a tab switch restoring a previously-invalid value). - const initialFields = prepareAnalyzedBatch(analysis!, wallNow(), 'execute').fields; + const initialFields = params.prepareAnalyzedBatch(analysis!, wallNow(), 'execute').fields; strip.replaceChildren(...vars.map((v, i) => { // controls[i] (fieldControlKind above) picks the field's control: // #172 enum members (v1 declared or v2 inferred) > #169 date-like @@ -877,14 +781,14 @@ export function createApp(env: CreateAppEnv = {}): App { // 'input' mode (#170): a plausible prefix stays neutral while // the field is focused — only a value that's already certainly // wrong shows the inline error here. - const inputBatch = prepareTabBatch(tab.sqlDraft, wallNow(), 'input'); + const inputBatch = params.prepareTabBatch(tab.sqlDraft, wallNow(), 'input'); applyFieldState(input, inputBatch.fields[v.name], baseTitle, combo?.previewEl); setRunBtn(app.state.running.value, inputBatch.sources[0]); }; const onCommitHard = (): void => { // Hardens 'incomplete' → 'invalid' on commit (#170). - const commitBatch = prepareTabBatch(tab.sqlDraft, wallNow(), 'execute'); - hardenVar(v.name, commitBatch.fields[v.name]); + const commitBatch = params.prepareTabBatch(tab.sqlDraft, wallNow(), 'execute'); + params.hardenVar(v.name, commitBatch.fields[v.name]); applyFieldState(input, commitBatch.fields[v.name], baseTitle, combo?.previewEl); setRunBtn(app.state.running.value, commitBatch.sources[0]); }; @@ -906,7 +810,7 @@ export function createApp(env: CreateAppEnv = {}): App { input = combo.input; wireComboInput(combo, { onValueInput, onCommit: onCommitHard }); if (conflictNote) input.classList.add('is-conflict'); - hardenVar(v.name, initialFields[v.name]); + params.hardenVar(v.name, initialFields[v.name]); applyFieldState(input, initialFields[v.name], baseTitle, combo?.previewEl); return h('label', { class: 'var-field' + (v.optional ? ' is-optional' : '') }, h('span', { class: 'var-name' }, v.name), combo.el); @@ -1362,7 +1266,7 @@ export function createApp(env: CreateAppEnv = {}): App { if (app.activeTab().editorMode !== 'sql') return undefined; if (app.state.exporting.value) return undefined; const waveMs = wallNow(); // one wall clock for this export wave (gate + args) - if (varGateBlocked(waveMs)) return undefined; // don't export with unfilled variables (#134) + if (params.varGateBlocked(waveMs)) return undefined; // don't export with unfilled variables (#134) const input = app.activeTab().sqlDraft; const statements = splitStatements(input); if (!statements.length) { flashToast('Nothing to export', { document: doc }); return undefined; } @@ -1376,7 +1280,7 @@ export function createApp(env: CreateAppEnv = {}): App { if (!app.canExport()) return; // aria-disabled button; defensive guard const tab = app.activeTab(); // Export streams the execution view (#165) — identical bytes without blocks. - const { sql, format } = prepareExportSql(execStatementSql(sqlInput)); + const { sql, format } = prepareExportSql(params.execStatementSql(sqlInput)); if (!sql) { flashToast('Nothing to export', { document: doc }); return; } const { ext, mime } = formatFileMeta(format); // Prepared args captured NOW — synchronously with exportEntry's gate @@ -1384,7 +1288,7 @@ export function createApp(env: CreateAppEnv = {}): App { // with run/runScript/exportScript): gate and args see the same varValues // snapshot; edits during those awaits apply to the next export. (Session // params stay live below — they don't read varValues.) - const paramArgs = mergedSourceArgs(prepareTabSource(sql, waveMs)); + const paramArgs = mergedSourceArgs(params.prepareTabSource(sql, waveMs)); // Flip the flag before the picker (not after, like the file handle) so a // second click while the native dialog is still open is blocked by the @@ -1535,7 +1439,7 @@ export function createApp(env: CreateAppEnv = {}): App { // exportDirect): gate and args see the same varValues snapshot; edits // during those awaits apply to the next export. `statements` came from // splitStatements(originalInput), so the batch aligns by index. - const paramSrc = prepareTabSource(originalInput, waveMs); + const paramSrc = params.prepareTabSource(originalInput, waveMs); // Flip the flag before the picker (mirrors exportDirect) so a second click // while the directory dialog / auth is still in flight is blocked by // exportEntry's guard — exportScript itself doesn't set this until after diff --git a/src/ui/app.types.ts b/src/ui/app.types.ts index eb48239..d4303a9 100644 --- a/src/ui/app.types.ts +++ b/src/ui/app.types.ts @@ -21,6 +21,7 @@ import type { SpecValidatorFn } from '../core/spec-draft.js'; import type { SavedQueryV2 } from '../generated/json-schema.types.js'; import type { DynamicSources } from '../core/spec-completion.js'; import type { WorkbenchSession } from './workbench/workbench-session.js'; +import type { WorkbenchParameterSession } from '../application/workbench-parameter-session.js'; export type { QueryTab as Tab, AppState as State } from '../state.js'; @@ -164,8 +165,9 @@ export interface App { state: State; dom: AppDom; /** Names of `{name:Type}` variables whose value has hardened to invalid - * (#170 review — see app.ts's construction-site comment). Read directly by - * tests; otherwise app.ts-internal bookkeeping. */ + * (#170 review — owned by `params`, see below; this is the SAME `Set` + * instance, aliased, not copied — app.ts's construction-site comment). + * Read directly by tests; otherwise app.ts-internal bookkeeping. */ hardenedVars: Set; root: Element | null; document: Document; @@ -298,6 +300,18 @@ export interface App { * Run/Cancel actions and the Explain/row-limit re-run paths delegate to it, * and `renderApp`'s `attachShell` call wires its 3 run-coupled effects. */ workbench: WorkbenchSession; + /** The `{name:Type}` query-variable POLICY (#276 Phase 4B1 — + * `src/application/workbench-parameter-session.ts`), constructible + * without App/AppState/DOM: analyze/prepare/gate/execution-view, the #170 + * hardening bookkeeping, the #172 v2 schema-cache enum-suggestion + * inference, and the #171 recent-value + persistence policy. + * `renderVarStrip`/`setRunBtn` (DOM) stay in app.ts, calling this + * session's methods directly; the workbench-session hooks + the export + * block's direct calls are re-pointed here too. `saveVarValues`/ + * `saveFilterActive`/`saveVarRecent`/`saveVarRecentDisabled`/ + * `recordBoundParams`/`clearVarRecent`/`clearAllVarRecent` (declared + * below, under Persistence) are one-line delegates onto this. */ + params: WorkbenchParameterSession; /** The shared request/stream/normalize + multiquery-script transport * service (#276 Phase 1) — `src/application/query-execution-service.ts`, * constructible without App/AppState/DOM. `src/ui/**` may depend on diff --git a/tests/helpers/fake-app.ts b/tests/helpers/fake-app.ts index ea4e64c..193f772 100644 --- a/tests/helpers/fake-app.ts +++ b/tests/helpers/fake-app.ts @@ -33,6 +33,7 @@ import type { QueryExecutionService } from '../../src/application/query-executio import type { ConnectionSession } from '../../src/application/connection-session.js'; import type { SchemaCatalogService } from '../../src/application/schema-catalog-service.js'; import type { WorkbenchSession } from '../../src/ui/workbench/workbench-session.js'; +import type { WorkbenchParameterSession } from '../../src/application/workbench-parameter-session.js'; import { assembleReferenceData, buildCompletions } from '../../src/core/completions.js'; import type { AssembledReference } from '../../src/core/completions.js'; @@ -140,6 +141,33 @@ const catalogDefaults: SchemaCatalogService = { invalidate: vi.fn(), }; +// A minimal `WorkbenchParameterSession` stub (#276 Phase 4B1) — no +// render-module fixture exercises the session directly (that's +// workbench-parameter-session.test.ts's job); this just satisfies the +// `App.params` contract. `hardenedVars` is the SAME `Set` instance as +// `appDefaults.hardenedVars` below (the production aliasing invariant — +// app.ts assigns `app.hardenedVars = params.hardenedVars`). +const paramsHardenedVarsDefault = new Set(); +const paramsDefaults: WorkbenchParameterSession = { + hardenedVars: paramsHardenedVarsDefault, + tabAnalysis: vi.fn(() => ({ fields: {}, sources: [], sourceErrors: {}, diagnostics: [] })), + prepareAnalyzedBatch: vi.fn(() => ({ fields: {}, sources: [], diagnostics: [] })), + prepareTabBatch: vi.fn(() => ({ fields: {}, sources: [], diagnostics: [] })), + prepareTabSource: vi.fn(() => ({ id: 'tab', statements: [], missing: [], invalid: [], errors: [], runnable: true })), + execStatementSql: vi.fn((stmt: string) => stmt), + varGateBlocked: vi.fn(() => false), + hardenVar: vi.fn(), + inputGate: vi.fn(() => ({ missing: [], invalid: [], errors: [] })), + inferredEnumOptions: vi.fn(() => null), + recordBoundParams: vi.fn(), + clearVarRecent: vi.fn(), + clearAllVarRecent: vi.fn(), + saveVarValues: vi.fn(), + saveFilterActive: vi.fn(), + saveVarRecent: vi.fn(), + saveVarRecentDisabled: vi.fn(), +}; + // Every `App` member this file's own concrete stubs (below) don't cover, // filled with an inert placeholder never read by a fixture that doesn't // override it — same convention as (and previously duplicated by) each of @@ -149,13 +177,16 @@ const catalogDefaults: SchemaCatalogService = { const appDefaults: App = { state: {} as AppState, dom: {}, - hardenedVars: new Set(), + // The SAME `Set` instance as `paramsDefaults.hardenedVars` — the production + // aliasing invariant (app.ts's `app.hardenedVars = params.hardenedVars`). + hardenedVars: paramsHardenedVarsDefault, matchMedia: null, build: 'v0.0.0-test', root: null, document, conn: connDefaults, catalog: catalogDefaults, + params: paramsDefaults, sqlEditor: {} as App['sqlEditor'], specEditor: {} as App['specEditor'], CodeViewer: () => ({ setText: () => {}, setLanguage: () => {}, setWrap: () => {}, focus: () => {}, destroy: () => {} }), @@ -247,7 +278,7 @@ const appDefaults: App = { * onSignedOut }` (a caller overriding one ChCtx member, not the whole * service) would otherwise fail the constraint even though the nested merge * below happily accepts a partial sub-object. */ -type AppOverrides = Partial> & { +type AppOverrides = Partial> & { dom?: Partial; chCtx?: Partial; actions?: Partial; @@ -264,6 +295,10 @@ type AppOverrides = Partial; + /** Partial like `workbench` above (#276 Phase 4B1) — most fixtures never + * touch the session directly; a test asserting e.g. `params.clearVarRecent` + * was called can override just that method. */ + params?: Partial; }; // `overrides` is generic so its properties keep their OWN precise call-site @@ -397,6 +432,7 @@ export function makeApp>(override exec: { ...appDefaults.exec, ...base.exec, ...(overrides.exec ?? {}) }, conn: { ...connDefaults, ...(overrides.conn ?? {}), chCtx }, workbench: { ...workbenchDefaults, ...(overrides.workbench ?? {}) }, + params: { ...paramsDefaults, ...(overrides.params ?? {}) }, }; // Assignability check only (a variable reference, not a fresh literal, so // this never trips an excess-property error) — `merged`'s own inferred type diff --git a/tests/unit/workbench-parameter-session.test.ts b/tests/unit/workbench-parameter-session.test.ts new file mode 100644 index 0000000..925f179 --- /dev/null +++ b/tests/unit/workbench-parameter-session.test.ts @@ -0,0 +1,419 @@ +import { describe, it, expect, vi } from 'vitest'; +import { createWorkbenchParameterSession } from '../../src/application/workbench-parameter-session.js'; +import type { + WorkbenchParameterSessionDeps, WorkbenchParameterSessionHooks, +} from '../../src/application/workbench-parameter-session.js'; +import { newTabObj, KEYS } from '../../src/state.js'; +import type { QueryTab, SaveJSON } from '../../src/state.js'; +import { emptyRecentMap, recordRecent } from '../../src/core/recent-values.js'; +import type { RecentMap } from '../../src/core/recent-values.js'; +import type { SchemaDb } from '../../src/core/from-scope.js'; +import type { PreparedFieldState, FieldControl } from '../../src/core/param-pipeline.js'; + +// WorkbenchParameterSession (#276 Phase 4B1) — the `{name:Type}` query- +// variable POLICY extracted from app.ts, unit-tested directly against fake +// deps/hooks (no App, no DOM). app.test.ts's own var-strip/export/persistence +// suites are the end-to-end safety net for the app.ts wiring; these tests are +// the session's own unit surface. + +// ── Fakes ──────────────────────────────────────────────────────────────────── + +function makeHooks(): WorkbenchParameterSessionHooks & { + onGateBlocked: ReturnType; + saveVarRecent: ReturnType; +} { + return { onGateBlocked: vi.fn(), saveVarRecent: vi.fn() }; +} + +/** Mutable backing store for the accessor closures below — mirrors a real + * `AppState` slice closely enough to exercise the live-state contract (a + * write through `state.varValues` is observed on the NEXT accessor read, + * never a snapshot). Exposed alongside `deps` so a test can both drive state + * changes and assert on them without going through the session. */ +function makeState(): { + varValues: Record; + filterActive: Record; + varRecent: RecentMap; + varRecentDisabled: boolean; + schema: SchemaDb[] | null; +} { + return { + varValues: {}, filterActive: {}, varRecent: emptyRecentMap(), varRecentDisabled: false, schema: null, + }; +} + +function makeDeps(over: { + state?: ReturnType; + tab?: QueryTab; + wallNow?: () => number; + saveJSON?: ReturnType; + hooks?: ReturnType; +} = {}): { + deps: WorkbenchParameterSessionDeps; + state: ReturnType; + tab: QueryTab; + saveJSON: ReturnType; + hooks: ReturnType; +} { + const state = over.state || makeState(); + // `'tab' in over` (not `over.tab || …`) — a test deliberately passing + // `tab: null` (the no-active-tab defensive case) must NOT fall back to a + // fresh tab, which `null || newTabObj(...)` would silently do. + const tab = 'tab' in over ? over.tab! : newTabObj('t1'); + const saveJSON = over.saveJSON || vi.fn(); + const hooks = over.hooks || makeHooks(); + const wallNow = over.wallNow || (() => 1700000000000); + const deps: WorkbenchParameterSessionDeps = { + varValues: () => state.varValues, + filterActive: () => state.filterActive, + varRecent: () => state.varRecent, + setVarRecent: (map) => { state.varRecent = map; }, + varRecentDisabled: () => state.varRecentDisabled, + schema: () => state.schema, + activeTab: () => tab, + wallNow, + saveJSON: saveJSON as unknown as SaveJSON, + hooks, + }; + return { deps, state, tab, saveJSON, hooks }; +} + +// ── tabAnalysis / prepareAnalyzedBatch / prepareTabBatch / prepareTabSource ── + +describe('tabAnalysis / prepare*', () => { + it('tabAnalysis analyzes the SQL as the single "tab" source (row-returning bind policy)', () => { + const { deps } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + const analysis = session.tabAnalysis('SELECT {year:UInt16}'); + expect(analysis.sources).toHaveLength(1); + expect(analysis.sources[0].id).toBe('tab'); + expect(Object.keys(analysis.fields)).toEqual(['year']); + }); + + it('prepareAnalyzedBatch prepares against the live varValues + #165 activation map', () => { + const { deps, state } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + state.varValues.year = '2024'; + const analysis = session.tabAnalysis('SELECT {year:UInt16}'); + const batch = session.prepareAnalyzedBatch(analysis, 1700000000000); + expect(batch.sources[0].statements[0].args).toEqual({ param_year: '2024' }); + expect(batch.fields.year.state).toBe('ok'); + }); + + it('prepareTabBatch / prepareTabSource compose tabAnalysis + prepareAnalyzedBatch for a raw SQL string', () => { + const { deps, state } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + state.varValues.year = '2024'; + const batch = session.prepareTabBatch('SELECT {year:UInt16}', 1700000000000); + expect(batch.sources[0].statements[0].args).toEqual({ param_year: '2024' }); + const src = session.prepareTabSource('SELECT {year:UInt16}', 1700000000000); + expect(src.id).toBe(batch.sources[0].id); // prepareTabSource === prepareTabBatch(...).sources[0] + expect(src.statements[0].args).toEqual({ param_year: '2024' }); + }); + + it('reads live state, not a snapshot: a value written after construction is seen on the next prepare', () => { + const { deps, state } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + expect(session.prepareTabSource('SELECT {year:UInt16}', 1700000000000).missing).toEqual(['year']); + state.varValues.year = '2024'; // filter-bar.ts/dashboard.ts/results.ts write app.state.varValues directly + expect(session.prepareTabSource('SELECT {year:UInt16}', 1700000000000).missing).toEqual([]); + }); + + it('#165 activation: an explicit filterActive entry overrides the derived-from-value default', () => { + const { deps, state } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + const sql = 'SELECT 1 /*[ AND a = {a:String} ]*/'; + state.varValues.a = 'x'; + state.filterActive.a = false; // explicit inactive wins over "has a value" + const src = session.prepareTabSource(sql, 1700000000000); + expect(src.statements[0].sql).toBe('SELECT 1 '); + }); +}); + +// ── execStatementSql ───────────────────────────────────────────────────────── + +describe('execStatementSql', () => { + it('materializes only active optional blocks for a row-returning statement (#165)', () => { + const { deps, state } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + const sql = 'SELECT 1 /*[ AND a = {a:String} ]*/'; + expect(session.execStatementSql(sql)).toBe('SELECT 1 '); // inactive by default (blank value) + state.varValues.a = 'x'; + expect(session.execStatementSql(sql)).toBe('SELECT 1 AND a = {a:String} '); + }); + + it('passes a non-row-returning statement through verbatim (#134 bind gate)', () => { + const { deps } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + const ddl = 'CREATE TABLE t (a String) ENGINE=Memory'; + expect(session.execStatementSql(ddl)).toBe(ddl); + }); +}); + +// ── varGateBlocked ─────────────────────────────────────────────────────────── + +describe('varGateBlocked', () => { + it('is false when there is no active tab (defensive)', () => { + const { deps, hooks } = makeDeps({ tab: null as unknown as QueryTab }); + const session = createWorkbenchParameterSession(deps); + expect(session.varGateBlocked(1700000000000)).toBe(false); + expect(hooks.onGateBlocked).not.toHaveBeenCalled(); + }); + + it('blocks + toasts "Enter a value for: …" for missing/invalid variables', () => { + const tab = newTabObj('t1'); + tab.sqlDraft = 'SELECT {year:UInt16}'; + const { deps, hooks } = makeDeps({ tab }); + const session = createWorkbenchParameterSession(deps); + expect(session.varGateBlocked(1700000000000)).toBe(true); + expect(hooks.onGateBlocked).toHaveBeenCalledWith('Enter a value for: year'); + }); + + it('blocks + toasts the first structural/serialization error when missing/invalid are empty', () => { + const tab = newTabObj('t1'); + tab.sqlDraft = 'SELECT {db:String}'; + const { deps, state, hooks } = makeDeps({ tab }); + // An Array value against a scalar String declaration is a structural + // serialization error (param-pipeline.ts), not missing/invalid — a real + // shape share-linked/curated values can take even though `varValues`'s + // static type says string. + (state.varValues as unknown as Record).db = ['not', 'scalar']; + const session = createWorkbenchParameterSession(deps); + expect(session.varGateBlocked(1700000000000)).toBe(true); + expect(hooks.onGateBlocked).toHaveBeenCalledTimes(1); + expect(hooks.onGateBlocked.mock.calls[0][0]).toContain('{db}'); + }); + + it('is false (no toast) when every variable resolves cleanly', () => { + const tab = newTabObj('t1'); + tab.sqlDraft = 'SELECT {year:UInt16}'; + const { deps, state, hooks } = makeDeps({ tab }); + state.varValues.year = '2024'; + const session = createWorkbenchParameterSession(deps); + expect(session.varGateBlocked(1700000000000)).toBe(false); + expect(hooks.onGateBlocked).not.toHaveBeenCalled(); + }); + + it('defaults wallNowMs to deps.wallNow() when omitted', () => { + const tab = newTabObj('t1'); + tab.sqlDraft = 'SELECT {d:DateTime}'; + const wallNow = vi.fn(() => 1700000000000); + const { deps, state } = makeDeps({ tab, wallNow }); + state.varValues.d = 'now-1h'; + const session = createWorkbenchParameterSession(deps); + session.varGateBlocked(); // no explicit wave clock + expect(wallNow).toHaveBeenCalled(); + }); +}); + +// ── hardenVar / inputGate ──────────────────────────────────────────────────── + +describe('hardenVar / inputGate', () => { + it('hardenVar adds a name on an invalid field verdict, clears it otherwise', () => { + const { deps } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + const invalid: PreparedFieldState = { state: 'invalid', reason: 'nope' }; + const ok: PreparedFieldState = { state: 'ok' }; + session.hardenVar('year', invalid); + expect(session.hardenedVars.has('year')).toBe(true); + session.hardenVar('year', ok); + expect(session.hardenedVars.has('year')).toBe(false); + // No field at all (e.g. the name dropped out of the batch) also clears. + session.hardenVar('year', invalid); + session.hardenVar('year', undefined); + expect(session.hardenedVars.has('year')).toBe(false); + }); + + it('inputGate folds in a hardened name still present in the batch (and not already invalid)', () => { + const { deps } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + session.hardenedVars.add('year'); + const analysis = session.tabAnalysis('SELECT {year:UInt16}'); + const gate = session.inputGate(analysis); + expect(gate.invalid).toContain('year'); // 'input' mode alone wouldn't flag a blank value as invalid + }); + + it('inputGate never double-lists a hardened name the lenient recompute already marked invalid', () => { + const { deps, state } = makeDeps(); + state.varValues.year = '999999'; // out of UInt16 range — invalid even in lenient 'input' mode + const session = createWorkbenchParameterSession(deps); + session.hardenedVars.add('year'); + const analysis = session.tabAnalysis('SELECT {year:UInt16}'); + const gate = session.inputGate(analysis); + expect(gate.invalid).toEqual(['year']); // not ['year','year'] + }); + + it('inputGate drops a hardened name that is not part of THIS batch (a different tab\'s field)', () => { + const { deps } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + session.hardenedVars.add('other'); // never appears in this analysis + const analysis = session.tabAnalysis('SELECT {year:UInt16}'); + const gate = session.inputGate(analysis); + expect(gate.invalid).not.toContain('other'); + }); +}); + +// ── inferredEnumOptions (#172 v2) ──────────────────────────────────────────── + +describe('inferredEnumOptions', () => { + const ENUM_STATUS = "Enum8('active' = 1, 'deleted' = 2)"; + const fc = (over: Partial = {}): FieldControl => ({ name: 's', type: 'String', optional: false, ...over }); + + it('returns null for a non-String declared type', () => { + const { deps } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + expect(session.inferredEnumOptions(fc({ type: 'UInt16' }), 'SELECT {s:UInt16}', {})).toBeNull(); + }); + + it('returns null when the param has no resolved comparison column', () => { + const { deps } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + expect(session.inferredEnumOptions(fc(), 'SELECT {s:String}', {})).toBeNull(); + }); + + it('returns null when the compared column type cannot be resolved (schema not loaded / ambiguous)', () => { + const { deps } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + const sql = 'SELECT * FROM events WHERE status = {s:String}'; + expect(session.inferredEnumOptions(fc(), sql, { s: { qualifier: null, column: 'status', pos: sql.indexOf('{s') } })).toBeNull(); + }); + + it('resolves the compared column\'s Enum members against the live schema cache', () => { + const { deps, state } = makeDeps(); + state.schema = [{ db: 'app', tables: [{ name: 'events', columns: [{ name: 'status', type: ENUM_STATUS }] }] }]; + const session = createWorkbenchParameterSession(deps); + const sql = 'SELECT * FROM events WHERE status = {s:String}'; + const comparisonColumns = { s: { qualifier: null, column: 'status', pos: sql.indexOf('{s') } }; + expect(session.inferredEnumOptions(fc(), sql, comparisonColumns)).toEqual(['active', 'deleted']); + }); + + it('is value-transparent for LowCardinality(String) (#238)', () => { + const { deps, state } = makeDeps(); + state.schema = [{ db: 'app', tables: [{ name: 'events', columns: [{ name: 'status', type: ENUM_STATUS }] }] }]; + const session = createWorkbenchParameterSession(deps); + const sql = 'SELECT * FROM events WHERE status = {s:LowCardinality(String)}'; + const comparisonColumns = { s: { qualifier: null, column: 'status', pos: sql.indexOf('{s') } }; + expect(session.inferredEnumOptions(fc({ type: 'LowCardinality(String)' }), sql, comparisonColumns)).toEqual(['active', 'deleted']); + }); + + it('returns null when the resolved column type has no Enum members', () => { + const { deps, state } = makeDeps(); + state.schema = [{ db: 'app', tables: [{ name: 'events', columns: [{ name: 'status', type: 'String' }] }] }]; + const session = createWorkbenchParameterSession(deps); + const sql = 'SELECT * FROM events WHERE status = {s:String}'; + const comparisonColumns = { s: { qualifier: null, column: 'status', pos: sql.indexOf('{s') } }; + expect(session.inferredEnumOptions(fc(), sql, comparisonColumns)).toBeNull(); + }); +}); + +// ── hardenedVars aliasing ──────────────────────────────────────────────────── + +describe('hardenedVars', () => { + it('is a single Set instance, mutated in place by hardenVar (the app.hardenedVars alias invariant)', () => { + const { deps } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + const ref = session.hardenedVars; + session.hardenVar('year', { state: 'invalid' }); + expect(ref.has('year')).toBe(true); // the SAME Set a caller aliased earlier observes the mutation + expect(session.hardenedVars).toBe(ref); // never reassigned + }); +}); + +// ── persistence wrappers ───────────────────────────────────────────────────── + +describe('saveVarValues / saveFilterActive / saveVarRecent / saveVarRecentDisabled', () => { + it('each persists its own field under its own KEYS entry', () => { + const { deps, state, saveJSON } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + state.varValues.a = '1'; + state.filterActive.a = true; + state.varRecent = recordRecent(emptyRecentMap(), 'a', '1'); + state.varRecentDisabled = true; + + session.saveVarValues(); + expect(saveJSON).toHaveBeenCalledWith(KEYS.varValues, state.varValues); + session.saveFilterActive(); + expect(saveJSON).toHaveBeenCalledWith(KEYS.filterActive, state.filterActive); + session.saveVarRecent(); + expect(saveJSON).toHaveBeenCalledWith(KEYS.varRecent, state.varRecent); + session.saveVarRecentDisabled(); + expect(saveJSON).toHaveBeenCalledWith(KEYS.varRecentDisabled, true); + }); +}); + +// ── recordBoundParams / clearVarRecent / clearAllVarRecent (#171) ────────── + +describe('recordBoundParams', () => { + it('is a no-op (no state write, no persist hook) while history recording is disabled', () => { + const { deps, state, hooks } = makeDeps(); + state.varRecentDisabled = true; + const before = state.varRecent; + const session = createWorkbenchParameterSession(deps); + session.recordBoundParams([{ name: 'a', rawValue: '1' }]); + expect(state.varRecent).toBe(before); + expect(hooks.saveVarRecent).not.toHaveBeenCalled(); + }); + + it('is a no-op for an empty/absent boundParams list', () => { + const { deps, state, hooks } = makeDeps(); + const before = state.varRecent; + const session = createWorkbenchParameterSession(deps); + session.recordBoundParams([]); + session.recordBoundParams(null as unknown as Array<{ name: string; rawValue: unknown }>); + expect(state.varRecent).toBe(before); + expect(hooks.saveVarRecent).not.toHaveBeenCalled(); + }); + + it('skips an Array-valued rawValue (#172 not-yet-built enum controls) and records only string values', () => { + const { deps, state, hooks } = makeDeps(); + const session = createWorkbenchParameterSession(deps); + session.recordBoundParams([ + { name: 'arr', rawValue: ['x', 'y'] }, + { name: 'a', rawValue: '1' }, + ]); + expect(state.varRecent.byName.arr).toBeUndefined(); + expect(state.varRecent.byName.a).toBeDefined(); + expect(hooks.saveVarRecent).toHaveBeenCalledTimes(1); + }); + + it('is a no-op (no persist hook) when nothing actually changed (an Array-only batch)', () => { + const { deps, state, hooks } = makeDeps(); + const before = state.varRecent; + const session = createWorkbenchParameterSession(deps); + session.recordBoundParams([{ name: 'arr', rawValue: ['x'] }]); + expect(state.varRecent).toBe(before); + expect(hooks.saveVarRecent).not.toHaveBeenCalled(); + }); +}); + +describe('clearVarRecent', () => { + it('is a no-op (no persist hook) for a name with no recorded history', () => { + const { deps, state, hooks } = makeDeps(); + state.varRecent = recordRecent(emptyRecentMap(), 'a', '1'); + const before = state.varRecent; + const session = createWorkbenchParameterSession(deps); + session.clearVarRecent('nope'); + expect(state.varRecent).toBe(before); + expect(hooks.saveVarRecent).not.toHaveBeenCalled(); + }); + + it('clears the name\'s history and persists via the hook (routed through the reassignable app seam)', () => { + const { deps, state, hooks } = makeDeps(); + state.varRecent = recordRecent(emptyRecentMap(), 'a', '1'); + const session = createWorkbenchParameterSession(deps); + session.clearVarRecent('a'); + expect(state.varRecent.byName.a).toBeUndefined(); + expect(hooks.saveVarRecent).toHaveBeenCalledTimes(1); + }); +}); + +describe('clearAllVarRecent', () => { + it('resets every name\'s history and always persists', () => { + const { deps, state, hooks } = makeDeps(); + state.varRecent = recordRecent(recordRecent(emptyRecentMap(), 'a', '1'), 'b', '2'); + const session = createWorkbenchParameterSession(deps); + session.clearAllVarRecent(); + expect(state.varRecent).toEqual(emptyRecentMap()); + expect(hooks.saveVarRecent).toHaveBeenCalledTimes(1); + }); +}); From f2942676509fe00ffb8e1406a8d58f7710d50366 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Fri, 17 Jul 2026 14:23:43 +0000 Subject: [PATCH 2/6] =?UTF-8?q?refactor(#276):=20extract=20ExportService?= =?UTF-8?q?=20=E2=80=94=20phase=204B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit src/application/export-service.ts (app.exports) owns direct + script export: picker-first ordering (transient user activation — now pinned by an explicit pickFile-before-ensureConfig test), hold-back-buffer streaming with mid-stream ClickHouse exception excision, .partial rename semantics, per-statement script files + directory naming, stop-on-first-failure, and BOTH formerly-module-scope cancellation state groups (now service-private — the last of the three duplicated cancellation copies from the Phase-0 map). Transport deps are ch-client fns + the live ctx accessor, NOT app.exec (raw-Response hold-back is incompatible with parsed results); ExportSink { pickFile, pickDirectory } is the injectable filesystem seam (issue §5); state.exporting stays an AppState signal with the service as sole writer (running's precedent); F6 waveMs handling preserved. showExportProgress (DOM banner), downloadFile (Blob+anchor), canExport checks stay app.ts. Test migration: the two ~700-line app.test.ts export describes (40 scenarios) moved to tests/unit/export-service.test.ts against an in-memory sink; app.test.ts keeps 8 thin integration checks (wiring, signal→button effect, one round trip per kind). Coverage: app.ts unchanged, export-service.ts 100/93.6/100/100. Known pre-existing wart preserved verbatim (out of scope): exportDirect names files with ambient Date.now() rather than the injected wallNow(). Gate: 109 files / 3142 tests, build, check:arch green. Part of #276 (Phase-4 combined PR, commit 2 of 4). Claude-Session: ad39bf19-1690-43c7-abb4-f0084fca00a2 Co-authored-by: Claude Fable 5 --- src/application/export-service.ts | 564 ++++++++++++++++++++++ src/ui/app.ts | 412 +++------------- src/ui/app.types.ts | 11 + tests/helpers/fake-app.ts | 12 + tests/unit/app.test.ts | 585 ++--------------------- tests/unit/export-service.test.ts | 766 ++++++++++++++++++++++++++++++ 6 files changed, 1448 insertions(+), 902 deletions(-) create mode 100644 src/application/export-service.ts create mode 100644 tests/unit/export-service.test.ts diff --git a/src/application/export-service.ts b/src/application/export-service.ts new file mode 100644 index 0000000..12acc22 --- /dev/null +++ b/src/application/export-service.ts @@ -0,0 +1,564 @@ +// #276 Phase 4B2's ExportService — the streaming single-file export (issue +// #87) and multi-statement script export (issue #99) POLICY extracted from +// app.ts (issue #276 §5), a PURE MOVE of the export bodies (already +// re-pointed onto `WorkbenchParameterSession`/`app.params` by Phase 4B1) +// wholesale: `exportEntry`'s statement-count dispatch, `exportDirect`'s +// picker-first/stream/hold-back-buffer path, `exportScriptEntry`/ +// `exportScript`'s directory-picker/per-statement transport loop, and both +// cancel paths — byte-identical to app.ts's own prior history for these +// functions. Constructible without App/AppState/DOM, like +// `workbench-parameter-session.ts` before it; no imports from `src/ui/**` or +// `src/editor/**` (check:arch enforces this). +// +// Deliberately NOT included (mirrors the issue's own carve-outs, same +// reasoning as workbench-parameter-session.ts's header comment): +// `canExport`/`canExportScript` (env capability checks — app.ts-owned, read +// here only through the injected `deps.canExport`/`deps.canExportScript` +// booleans), `showExportProgress` (the DOM progress banner — app.ts-owned, +// called through `deps.hooks.showExportProgress`, matching its existing call +// shape `(onCancel) => { update(bytes), remove() }`), and `downloadFile` (a +// generic Blob+anchor file-save helper with no export-specific policy — it +// builds a Blob, which is DOM/browser material, not export policy; stays in +// app.ts, used by the File menu, untouched by this move). +// +// `state.exporting` stays an `AppState` signal (mirrors `workbench- +// session.ts`'s own `state.running` precedent) — this service is its SOLE +// production writer; `deps.state` is the SAME live slice `app.state` backs +// (a structural `Pick`, not a snapshot), so `app.state.exporting.value` +// reads/the `setExportBtn` effect observe this service's writes directly, no +// re-mirroring needed. +// +// `WritableFileStreamLike`/`FileHandleLike`/`DirectoryHandleLike` (formerly +// app.ts-local seam types over the File System Access API, which this +// project's lib.dom build doesn't carry at all) move here, exported, since +// they're this service's own transport shapes now; `ExportSink` wraps +// `showSaveFilePicker`/`showDirectoryPicker` behind an injected seam (mirrors +// `app.Chart`/`app.Dagre`/the editor ports) so this module never references +// `window`/`env` directly. +// +// `ScriptExportEntry`/`ScriptExportResult` below mirror (never import) +// `src/ui/results.ts`'s identically-named, identically-shaped types: `tab.result` +// is deliberately opaque (`Record | null`, state.ts) at this +// boundary — ui/results.ts is the one place that owns the real shape a run +// (or a script export) actually produces, exactly the same "produced here, +// read back there through the opaque field" relationship `core/script- +// result.ts`'s own `ScriptEntry` has with the transport service (Phase 1) +// vs. results.ts's re-export of it. `src/application/**` may never import +// `src/ui/**` (check:arch), so a structural mirror — not an import — is the +// only option; the two are kept in sync by hand (small, stable shapes). + +import type { Signal } from '@preact/signals-core'; +import { splitStatements, isRowReturning } from '../core/sql-split.js'; +import { mergedSourceArgs } from '../core/param-pipeline.js'; +import type { PreparedSource } from '../core/param-pipeline.js'; +import { prepareExportSql, isSchemaMutatingSql } from '../core/format.js'; +import { formatFileMeta, exportFilename, scriptExportName } from '../core/export.js'; +import { findExceptionFrame } from '../core/stream.js'; +import type { QueryTab } from '../state.js'; +import type { ResultSort } from '../core/sort.js'; +import type { ChCtx, exportQuery, runQuery, killQuery } from '../net/ch-client.js'; +import type { WorkbenchParameterSession } from './workbench-parameter-session.js'; + +// ── File System Access seam (moved from app.ts) ───────────────────────────── + +/** A `FileSystemWritableFileStream`-shaped handle — narrower than the DOM + * lib's own (this project's lib.dom build doesn't carry the File System + * Access API at all, hence the sink below being the injected seam). */ +export interface WritableFileStreamLike { + write(chunk: Uint8Array): Promise; + close(): Promise; + abort(): Promise; +} +/** A `FileSystemFileHandle`-shaped handle — `ExportSink.pickFile`'s resolved + * value, and `DirectoryHandleLike.getFileHandle`'s. `move` (Chrome 110+) is + * optional — feature-detected at the one call site that uses it. */ +export interface FileHandleLike { + name: string; + createWritable(): Promise; + move?(name: string): Promise; +} +/** A `FileSystemDirectoryHandle`-shaped handle — `ExportSink.pickDirectory`'s + * resolved value. */ +export interface DirectoryHandleLike { + getFileHandle(name: string, opts?: { create?: boolean }): Promise; +} + +/** The single-file/directory picker seam — wraps `showSaveFilePicker`/ + * `showDirectoryPicker` (env-injected, feature-detected — see app.ts's own + * `canExport`/`canExportScript`) so this module never touches `window` + * directly. Production wires this to the two real pickers; tests inject an + * in-memory sink. Rejecting with an `Error` named `'AbortError'` (matching + * the real pickers' own dismissal signal) is how a caller reports the user + * closed the dialog — both export entry points treat that name specially. */ +export interface ExportSink { + pickFile(input: { + suggestedName: string; + types: { description: string; accept: Record }[]; + }): Promise; + pickDirectory(input: { mode: 'readwrite' }): Promise; +} + +// ── tab.result shapes this service produces (mirrors ui/results.ts — see the +// module doc above for why this is a structural mirror, not an import) ────── + +/** One statement's export outcome in a script-export run (#99) — metadata + * only, never the exported rows. Mirrors `ui/results.ts`'s identically-named + * `ScriptExportEntry`. */ +export interface ScriptExportEntry { + i: number; + sql?: string; + type: string; + status: string; + file: string | null; + bytes: number; + startedAt: number | null; + ms: number | null; + error: string | null; +} + +/** A script-export run's `tab.result` shape (#99). Mirrors `ui/results.ts`'s + * identically-named `ScriptExportResult`. */ +export interface ScriptExportResult { + scriptExport: ScriptExportEntry[]; + startedAt: number; + elapsedMs?: number; + colWidths?: Record; +} + +// ── Injected dependency seam ───────────────────────────────────────────────── + +/** The `state.exporting`/`state.resultSort` slice this service reads/writes — + * a structural `Pick`, not a snapshot: production passes `app.state` itself + * (satisfies this directly), so a write here is observed immediately by + * every consumer of the live signal/property (mirrors `workbench- + * session.ts`'s own `WorkbenchStateSlice` convention). This service is the + * SOLE production writer of `exporting` (mirrors `running`'s own + * precedent). */ +export interface ExportStateSlice { + exporting: Signal; + /** Reassigned wholesale (`state.resultSort = {...}`) at the start of an + * export-script wave — a plain settable property, not a signal (matches + * `WorkbenchStateSlice.resultSort`). */ + resultSort: ResultSort; +} + +/** DOM/render hooks this service calls into — the shell's job, injected + * (mirrors `workbench-session.ts`'s own `WorkbenchHooks` convention). */ +export interface ExportHooks { + /** Per-statement (script export) results-pane repaint. */ + renderResults(): void; + /** The inline "Exporting… · s" banner (app.ts-owned DOM + * builder) — called once per single-file export with the export's own + * Cancel callback; returns the same `{update(bytes), remove()}` handle the + * pre-extraction code built inline. */ + showExportProgress(onCancel: () => void): { update(bytes: number): void; remove(): void }; + /** A user-facing toast (app.ts wires this to `flashToast(message, { + * document: doc })`); this service never imports `ui/toast.js`. */ + toast(message: string): void; + /** Fire-and-forget schema reload after a schema-mutating script statement + * actually ran (mirrors `WorkbenchHooks.loadSchema`). */ + loadSchema(): void; +} + +/** Every side effect this service needs, injected as a narrow bag — mirrors + * `query-execution-service.ts`'s own `QueryExecutionDeps`/`workbench- + * session.ts`'s own `WorkbenchSessionDeps` conventions. Transport deps carry + * the exact `ch-client.js` functions this service's export paths use + * (`exportQuery` for both the single-file and per-statement-rows paths, + * `runQuery` for a script's non-row effect statements, `killQuery` for both + * cancel paths) plus a live `ctx` PROVIDER (not a snapshot — the caller may + * rebuild it after a token refresh, same as `QueryExecutionDeps.ctx`). Kept + * as the raw ch-client functions + `ctx()` rather than routed through + * `app.exec` — `app.exec`'s `executeRead`/`executeScript` return already- + * parsed results, but the export paths need the raw streaming `Response` + * itself (for `streamToFile`'s hold-back-buffer inspection), which + * `app.exec`'s surface doesn't expose. */ +export interface ExportServiceDeps { + exportQuery: typeof exportQuery; + runQuery: typeof runQuery; + killQuery: typeof killQuery; + ctx(): ChCtx; + ensureConfig(): Promise; + /** Resolves the live bearer/basic credential, or null when signed out / + * unrefreshable — both export entry points call `ctx().onSignedOut()` and + * return in that case (byte-equivalent to the ported app.ts's + * `chCtx.onSignedOut(); return;`). Unlike `workbench-session.ts`'s own + * `onAuthFailed` hook, this service already depends on `ctx()` directly + * (see above), so there's no separate hook to keep it ignorant of chCtx. */ + getToken(): Promise; + /** SQL-string-quoting function `killQuery` needs (matches `core/format.js`'s + * `sqlString`). */ + sqlString: (s: unknown) => string; + /** Perf clock — export/script-row elapsed ms, matches app.ts's `now`. */ + now(): number; + /** The #173 wave wall clock (epoch ms) — matches app.ts's `wallNow`; + * `exportEntry` resolves one snapshot per export wave (gate + args), same + * F6 invariant as run()/runScript(). */ + wallNow(): number; + uid(prefix: string): string; + /** Env capability checks (`showSaveFilePicker`/`showDirectoryPicker` + + * secure-context feature detection) — the boolean LOGIC stays app.ts-owned + * (`app.canExport`/`app.canExportScript`); this service only reads the + * result, mirroring the defensive re-check the pre-extraction code already + * performed inside `exportDirect`/`exportScriptEntry` themselves (in + * addition to the Export button's own aria-disabled gating). */ + canExport(): boolean; + canExportScript(): boolean; + sink: ExportSink; + state: ExportStateSlice; + activeTab(): QueryTab; + /** The `{name:Type}` query-variable POLICY (#276 Phase 4B1) — narrowed to + * exactly the three methods the export paths call, matching `workbench- + * session.ts`'s own narrow `Pick` convention + * for `exec`. */ + params: Pick; + /** `tab.chSession`/transport material — stays app.ts-local (Phase 4C's + * concern, not this service's), injected as a plain function dep. */ + sessionParamsFor(tab: QueryTab, sqls: string[]): Record; + hooks: ExportHooks; +} + +// ── The service ────────────────────────────────────────────────────────────── + +export interface ExportService { + exportEntry(): Promise | undefined; + exportDirect(sqlInput: string, waveMs: number): Promise; + cancelExport(): void; + cancelExportScript(): void; +} + +// A latin1 decode (1 char per byte) for byte-accurate exception-frame slicing +// — pure, no injected deps. +const latin1 = (bytes: Uint8Array): string => { + let s = ''; + for (const b of bytes) s += String.fromCharCode(b); + return s; +}; + +/** Build an `ExportService` bound to `deps`. Trivial constructor — no + * validation, no defaulting; the caller supplies every field exactly as it + * wants it used. */ +export function createExportService(deps: ExportServiceDeps): ExportService { + // Full, uncapped export of a query — never the loaded grid — streamed + // straight to a user-chosen file. Its own query_id + abort, kept separate + // from the workbench session's own private run bookkeeping so an export and + // a grid run never clobber each other's cancel state. + let exportAbort: AbortController | null = null; + let exportQueryId: string | null = null; + // Script-export state (issue #99) — its own abort/query-id, reassigned each + // iteration so Cancel reaches the in-flight statement, and kept distinct + // from both the workbench session's own run bookkeeping and the single- + // export state above. + let exportScriptAbort: AbortController | null = null; + let exportScriptQueryId: string | null = null; + let exportScriptCancelled = false; + let exportScriptTick: ReturnType | null = null; + + // The Export button dispatches by statement count: one statement keeps the + // rich single-file flow below; more than one opens the script-export flow + // (its own directory + per-statement log, since one file per script makes + // no sense). Mirrors runEntry's split/branch. + function exportEntry(): Promise | undefined { + if (deps.activeTab().editorMode !== 'sql') return undefined; + if (deps.state.exporting.value) return undefined; + const waveMs = deps.wallNow(); // one wall clock for this export wave (gate + args) + if (deps.params.varGateBlocked(waveMs)) return undefined; // don't export with unfilled variables (#134) + const input = deps.activeTab().sqlDraft; + const statements = splitStatements(input); + if (!statements.length) { deps.hooks.toast('Nothing to export'); return undefined; } + if (statements.length === 1) return exportDirect(statements[0], waveMs); + return exportScriptEntry(statements, input, waveMs); + } + + async function exportDirect(sqlInput: string, waveMs: number): Promise { + if (deps.activeTab().editorMode !== 'sql') return; + if (deps.state.exporting.value) return; + if (!deps.canExport()) return; // aria-disabled button; defensive guard + const tab = deps.activeTab(); + // Export streams the execution view (#165) — identical bytes without blocks. + const { sql, format } = prepareExportSql(deps.params.execStatementSql(sqlInput)); + if (!sql) { deps.hooks.toast('Nothing to export'); return; } + const { ext, mime } = formatFileMeta(format); + // Prepared args captured NOW — synchronously with exportEntry's gate + // check, BEFORE the picker/auth awaits below (review F6 invariant, shared + // with run/runScript/exportScript): gate and args see the same varValues + // snapshot; edits during those awaits apply to the next export. (Session + // params stay live below — they don't read varValues.) + const paramArgs = mergedSourceArgs(deps.params.prepareTabSource(sql, waveMs)); + + // Flip the flag before the picker (not after, like the file handle) so a + // second click while the native dialog is still open is blocked by the + // guard above — the button's own disabled state (setExportBtn) also + // reflects this via an effect, but the guard is the authority. + deps.state.exporting.value = true; + try { + // Picker FIRST, before any await: showSaveFilePicker requires the click's + // transient activation, which a prior await (e.g. a token refresh in + // ensureConfig/getToken can be a network round trip) would forfeit. + let handle: FileHandleLike; + try { + handle = await deps.sink.pickFile({ + suggestedName: exportFilename(tab.name, Date.now(), ext), + types: [{ description: format + ' data', accept: { [mime]: ['.' + ext] } }], + }); + } catch (e) { + if (e instanceof Error && e.name === 'AbortError') return; // user dismissed the picker + deps.hooks.toast('Save dialog failed: ' + String((e instanceof Error && e.message) || e)); + return; + } + + // Now the awaits are safe — we already hold the file handle. + await deps.ensureConfig(); + if (!(await deps.getToken())) { deps.ctx().onSignedOut(); return; } + + exportQueryId = 'export-' + deps.uid(''); + exportAbort = new AbortController(); + const progress = deps.hooks.showExportProgress(cancelExport); + try { + const resp = await deps.exportQuery(deps.ctx(), sql, { + queryId: exportQueryId, signal: exportAbort.signal, format, + // Native query-parameter substitution (#134/#173), same as run() — + // paramArgs is the wave-start snapshot captured above (review F6). + params: { ...deps.sessionParamsFor(tab, [sql]), ...paramArgs }, + }); + const tag = resp.headers.get('X-ClickHouse-Exception-Tag'); // null on servers < 24.11 + const err = await streamToFile(resp, handle, { + signal: exportAbort.signal, tag, onProgress: (bytes) => progress.update(bytes), + }); + if (err) deps.hooks.toast('Export incomplete — server error mid-stream: ' + err); + else deps.hooks.toast('Export complete'); + } catch (e) { + // AbortError (cancelled) and 'signed out' (chCtx.onSignedOut already + // rendered the login screen) both already have their own signal — an + // extra toast on top would just be a confusing second message. + const msg = String((e instanceof Error && e.message) || e); + if (!(e instanceof Error && e.name === 'AbortError') && msg !== 'signed out') { + deps.hooks.toast('Export failed: ' + msg); + } + } finally { + progress.remove(); + exportAbort = null; + exportQueryId = null; + } + } finally { + deps.state.exporting.value = false; + } + } + + // Stream `resp.body` to `handle` with a hold-back buffer: ClickHouse's + // mid-stream exception frame (findExceptionFrame) is at most 16 KiB and + // always trailing, so bytes are only committed to disk once they've aged + // out of a 32 KiB window — at EOF the retained tail is inspected and only + // the clean prefix is written, so a mid-stream exception is never written + // into the file. Memory stays flat (one HOLDBACK-sized buffer) regardless of + // result size. Reads the stream directly (not via a TransformStream) because + // the write is conditional (withhold, inspect, commit) — a passthrough + // transform can't un-write. Returns the CH error message, or null when clean. + async function streamToFile( + resp: Response, handle: FileHandleLike, + { signal, tag, onProgress }: { signal: AbortSignal; tag: string | null; onProgress: (bytes: number) => void }, + ): Promise { + const writable = await handle.createWritable(); + const HOLDBACK = 32 * 1024; // >= ClickHouse's MAX_EXCEPTION_SIZE (16 KiB) + margin + const reader = resp.body!.getReader(); + let held = new Uint8Array(0); + let written = 0; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + if (signal.aborted) throw new DOMException('aborted', 'AbortError'); + const merged = new Uint8Array(held.length + value.length); + merged.set(held); + merged.set(value, held.length); + const commit = Math.max(0, merged.length - HOLDBACK); + if (commit > 0) { + await writable.write(merged.subarray(0, commit)); + written += commit; + onProgress(written); + } + held = merged.subarray(commit); + } + // EOF: inspect the retained tail (latin1: 1 char per byte, for byte-accurate slicing). + const frame = findExceptionFrame(latin1(held), tag); + const clean = frame ? held.subarray(0, frame.cleanBytes) : held; + if (clean.length) { + await writable.write(clean); + written += clean.length; + onProgress(written); + } + await writable.close(); + return frame ? frame.message : null; + } catch (e) { + // writable.abort() would discard everything already committed: on + // Chrome/File System Access API it leaves a hidden, 0-byte + // `.crswap` swap file behind and never materializes the visible + // target at all — so a cancelled/failed export recovers nothing. + // close() instead finalizes the bytes already written under the + // target handle, then move() (Chrome 110+) renames it in place with + // a `.partial` suffix so it reads as an inspectable, clearly-labeled + // partial artifact rather than a clean export. Best-effort: on + // browsers without move() (or if it throws), the file is still + // recoverable under its original name, just without the suffix. + await writable.close().catch(() => {}); + if (typeof handle.move === 'function') await handle.move(handle.name + '.partial').catch(() => {}); + throw e; + } finally { + reader.releaseLock(); + } + } + + // Mirrors cancel() (the grid run) but on the export's own id/abort. + function cancelExport(): void { + if (exportAbort) exportAbort.abort(); + deps.killQuery(deps.ctx(), exportQueryId, deps.sqlString); + } + + // Directory picker first (transient-activation rule, same as exportDirect's + // save-file picker), and skip the prompt entirely when there's nothing to + // export — no point asking for a folder a script will never write into. + async function exportScriptEntry(statements: string[], originalInput: string, waveMs: number): Promise { + if (!deps.canExportScript()) { + deps.hooks.toast('Script export requires Chrome/Edge directory access over HTTPS'); + return; + } + if (!statements.some(isRowReturning)) { + deps.hooks.toast('Nothing to export — script has no result-producing statements.'); + return; + } + // One prepared batch for the whole export wave (#173), captured NOW — + // synchronously with exportEntry's gate check, BEFORE the directory-picker + // and auth awaits below (review F6 invariant, shared with run/runScript/ + // exportDirect): gate and args see the same varValues snapshot; edits + // during those awaits apply to the next export. `statements` came from + // splitStatements(originalInput), so the batch aligns by index. + const paramSrc = deps.params.prepareTabSource(originalInput, waveMs); + // Flip the flag before the picker (mirrors exportDirect) so a second click + // while the directory dialog / auth is still in flight is blocked by + // exportEntry's guard — exportScript itself doesn't set this until after + // those awaits, which would otherwise leave a re-entrancy window open. + deps.state.exporting.value = true; + try { + let dir: DirectoryHandleLike; + try { + dir = await deps.sink.pickDirectory({ mode: 'readwrite' }); + } catch (e) { + if (e instanceof Error && e.name === 'AbortError') return; // dismissed → silent no-op + deps.hooks.toast('Folder dialog failed: ' + String((e instanceof Error && e.message) || e)); + return; + } + await deps.ensureConfig(); + if (!(await deps.getToken())) { deps.ctx().onSignedOut(); return; } + await exportScript(statements, dir, paramSrc); + } finally { + // No-op if exportScript already reset it — covers every early-return + // path above that never reaches exportScript's own finally. + deps.state.exporting.value = false; + } + } + + // Run a script's statements sequentially into `dir`, one file per + // row-returning statement, for effect otherwise. A single shared session + // carries SET/TEMPORARY state across statements (sessionParamsFor). The log + // lives in tab.result.scriptExport — per-statement metadata only (status, + // file, bytes, time); the exported rows themselves are never held in + // memory/state, so a multi-million-row script export stays flat. Stop on + // first failure, mirroring runScript's script grid — but with no retry + // (statements run one-at-a-time in a single session, so SESSION_IS_LOCKED + // can't self-collide, and a partially-written file shouldn't be silently + // re-attempted). + async function exportScript(statements: string[], dir: DirectoryHandleLike, paramSrc: PreparedSource): Promise { + const tab = deps.activeTab(); + const t0 = deps.now(); + const sp = deps.sessionParamsFor(tab, statements); + // `paramSrc` is the wave's prepared batch (#173), captured by + // exportScriptEntry at wave start, before its awaits (review F6). + const entries: ScriptExportEntry[] = statements.map((sql, i) => ({ + i, sql, type: isRowReturning(sql) ? 'rows' : 'effect', + status: 'pending', file: null, bytes: 0, startedAt: null, ms: 0, error: null, + })); + const scriptExportResult: ScriptExportResult = { scriptExport: entries, startedAt: t0 }; + Object.assign(tab, { result: scriptExportResult }); + deps.state.resultSort = { col: null, dir: 'asc' }; + exportScriptCancelled = false; + deps.state.exporting.value = true; + const taken = new Set(); + try { + // Live elapsed for the running row (bytes tick via onProgress; this ticks + // time). Started inside the try so a throw here still clears it below — + // an interval set before the try would otherwise leak forever. + exportScriptTick = setInterval(() => deps.hooks.renderResults(), 200); + deps.hooks.renderResults(); + for (const e of entries) { + if (exportScriptCancelled) { e.status = 'skipped'; continue; } + // Wire text = the pipeline's per-statement execution view (#165); + // verbatim for effect/DDL statements and for block-free SQL. + const execStmt = paramSrc.statements[e.i].sql; + const { sql, format } = prepareExportSql(execStmt); + // Per-statement prepared args (#134/#173): the pipeline binds only + // row-returning statements, so an effect/DDL statement (incl. CREATE + // VIEW) is sent with its {name:Type} placeholders intact. + const params = { ...sp, ...paramSrc.statements[e.i].args }; + exportScriptQueryId = 'export-' + deps.uid(''); + exportScriptAbort = new AbortController(); + const signal = exportScriptAbort.signal; + e.startedAt = deps.now(); + e.status = e.type === 'rows' ? 'exporting' : 'running'; + deps.hooks.renderResults(); + try { + if (e.type !== 'rows') { + const out = await deps.runQuery(deps.ctx(), execStmt, + { format: 'TSV', signal, queryId: exportScriptQueryId, params }); + if (out.error != null) throw new Error(out.error); + e.status = 'ok'; + } else { + const { ext } = formatFileMeta(format); + const name = scriptExportName(e.i, e.sql || '', ext, taken); + taken.add(name); + e.file = name; + const fileHandle = await dir.getFileHandle(name, { create: true }); + const resp = await deps.exportQuery(deps.ctx(), sql, + { queryId: exportScriptQueryId, signal, format, params }); + const tag = resp.headers.get('X-ClickHouse-Exception-Tag'); + const midErr = await streamToFile(resp, fileHandle, + { signal, tag, onProgress: (b) => { e.bytes = b; } }); + if (midErr) { + e.status = 'failed'; + e.error = 'File may be incomplete; server failed after streaming started. ' + midErr; + e.ms = deps.now() - e.startedAt!; + break; // stop-on-first-failure + } + e.status = 'ok'; + } + e.ms = deps.now() - e.startedAt!; + deps.hooks.renderResults(); + } catch (ex) { + e.ms = deps.now() - e.startedAt!; + if (ex instanceof Error && ex.name === 'AbortError') { e.status = 'cancelled'; exportScriptCancelled = true; } + else { e.status = 'failed'; e.error = String((ex instanceof Error && ex.message) || ex); } + break; // stop-on-first-failure + } + } + for (const e of entries) if (e.status === 'pending') e.status = 'skipped'; + } finally { + clearInterval(exportScriptTick as ReturnType); exportScriptTick = null; + exportScriptAbort = null; + exportScriptQueryId = null; + deps.state.exporting.value = false; + scriptExportResult.elapsedMs = deps.now() - t0; + // A schema-mutating effect statement that actually ran refreshes the tree + // (mirrors runScript) even though this export ran outside runScript. + if (entries.some((e) => e.status === 'ok' && isSchemaMutatingSql(e.sql))) deps.hooks.loadSchema(); + deps.hooks.renderResults(); + } + } + + // Mirrors cancelExport but on the script's own active id/abort. + function cancelExportScript(): void { + exportScriptCancelled = true; // stops the loop from starting the next statement + if (exportScriptAbort) exportScriptAbort.abort(); + deps.killQuery(deps.ctx(), exportScriptQueryId, deps.sqlString); + } + + return { exportEntry, exportDirect, cancelExport, cancelExportScript }; +} diff --git a/src/ui/app.ts b/src/ui/app.ts index 5ce8637..b87bbc3 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -13,20 +13,16 @@ import { } from '../state.js'; import type { QueryTab, AppState, SpecValidationService, HistoryResultSnapshot, QuerySpecDraft } from '../state.js'; import type { SavedQueryV2 } from '../generated/json-schema.types.js'; -import { splitStatements, isRowReturning, leadingKeyword } from '../core/sql-split.js'; -import { - mergedSourceArgs, mergedSourceSql, analysisView, - fieldControls, fieldControlKind, -} from '../core/param-pipeline.js'; -import type { PreparedSource } from '../core/param-pipeline.js'; +import { splitStatements, leadingKeyword } from '../core/sql-split.js'; +import { mergedSourceSql, analysisView, fieldControls, fieldControlKind } from '../core/param-pipeline.js'; import { hasOptionalBlocks } from '../core/optional-blocks.js'; import { saveJSON, saveStr } from '../core/storage.js'; -import { sqlString, inferQueryName, shortVersion, userShortName, withStatementBreak, isSchemaMutatingSql, prepareExportSql, formatBytes, formatRows } from '../core/format.js'; +import { sqlString, inferQueryName, shortVersion, userShortName, withStatementBreak, formatBytes, formatRows } from '../core/format.js'; import { buildSchemaGraph, expandLineage } from '../core/schema-graph.js'; import { buildCardGraph } from '../core/schema-cards.js'; import type { SchemaCardColumnRow } from '../core/schema-cards.js'; -import { toTSV, formatFileMeta, exportFilename, scriptExportName } from '../core/export.js'; -import { newResult, parseErrorPos, findExceptionFrame } from '../core/stream.js'; +import { toTSV } from '../core/export.js'; +import { newResult, parseErrorPos } from '../core/stream.js'; import { encodeShare } from '../core/share.js'; import { queryName, queryPanel, withQuerySpec } from '../core/saved-query.js'; import { effectiveDashboardRole } from '../core/result-choice.js'; @@ -48,7 +44,7 @@ import type { QueryOrName } from './tabs.js'; import { effect, batch } from '@preact/signals-core'; import { renderSchema } from './schema.js'; import { renderResults } from './results.js'; -import type { Result, QueryResult, ScriptResult, ScriptEntry, ScriptExportEntry, ScriptExportResult, ResultSchemaGraph } from './results.js'; +import type { Result, QueryResult, ScriptResult, ScriptEntry, ResultSchemaGraph } from './results.js'; import { renderDashboard } from './dashboard.js'; import { openSchemaView } from './explain-graph.js'; import type { SchemaLineageGraph, SchemaLineageNode, DetachedGraphApp } from './explain-graph.js'; @@ -82,6 +78,8 @@ import { createQueryExecutionService } from '../application/query-execution-serv import { createConnectionSession } from '../application/connection-session.js'; import { createSchemaCatalogService } from '../application/schema-catalog-service.js'; import { createWorkbenchParameterSession } from '../application/workbench-parameter-session.js'; +import { createExportService } from '../application/export-service.js'; +import type { ExportSink, FileHandleLike, DirectoryHandleLike } from '../application/export-service.js'; import { createWorkbenchSession } from './workbench/workbench-session.js'; /** Optional globals a plain browser page (or the CM6/Chart/dagre UMD bundles a @@ -98,29 +96,6 @@ import { createWorkbenchSession } from './workbench/workbench-session.js'; * control kinds, which never populate it. */ type VarStripCombo = (EnumField | RecentField | RelativeTimeField) & { previewEl?: HTMLElement }; -/** A `FileSystemWritableFileStream`-shaped handle — narrower than the DOM - * lib's own (this project's lib.dom build doesn't carry the File System - * Access API at all, hence `env.showSaveFilePicker`/`showDirectoryPicker` - * being typed `unknown` seams in the first place). */ -interface WritableFileStreamLike { - write(chunk: Uint8Array): Promise; - close(): Promise; - abort(): Promise; -} -/** A `FileSystemFileHandle`-shaped handle — `showSaveFilePicker`'s resolved - * value, and `getFileHandle`'s. `move` (Chrome 110+) is optional — feature- - * detected at the one call site that uses it. */ -interface FileHandleLike { - name: string; - createWritable(): Promise; - move?(name: string): Promise; -} -/** A `FileSystemDirectoryHandle`-shaped handle — `showDirectoryPicker`'s - * resolved value. */ -interface DirectoryHandleLike { - getFileHandle(name: string, opts?: { create?: boolean }): Promise; -} - interface WindowExtras { Chart?: unknown; dagre?: unknown; @@ -589,6 +564,40 @@ export function createApp(env: CreateAppEnv = {}): App { app.clearVarRecent = (name) => params.clearVarRecent(name); app.clearAllVarRecent = () => params.clearAllVarRecent(); + // The streaming single-file export (issue #87) + multi-statement script + // export (issue #99) POLICY (#276 Phase 4B2) now lives in + // `application/export-service.ts`, constructible without App/AppState/DOM + // — a pure move of the export bodies (already re-pointed onto `params`'s + // methods by Phase 4B1) wholesale. `exportSink` wraps the two File System + // Access pickers (feature-detected as `app.showSaveFilePicker`/ + // `app.showDirectoryPicker` above — only ever called once + // `canExport`/`canExportScript` has already gated true); `canExport`/ + // `canExportScript` themselves and `showExportProgress` (the DOM progress + // banner, defined further below) stay app.ts-owned, injected into the + // service. `state.exporting` stays an `AppState` signal this service is + // the sole writer of (mirrors `workbench`'s own `running` precedent). + const exportSink: ExportSink = { + pickFile: (input) => app.showSaveFilePicker!(input) as Promise, + pickDirectory: (input) => app.showDirectoryPicker!(input) as Promise, + }; + const exportService = createExportService({ + exportQuery: ch.exportQuery, runQuery: ch.runQuery, killQuery: ch.killQuery, + ctx: () => chCtx, ensureConfig, getToken, sqlString, now, wallNow, uid, + canExport: () => app.canExport(), canExportScript: () => app.canExportScript(), + sink: exportSink, + state: app.state, // AppState structurally satisfies ExportStateSlice + activeTab: () => app.activeTab(), + params: { prepareTabSource: params.prepareTabSource, varGateBlocked: params.varGateBlocked, execStatementSql: params.execStatementSql }, + sessionParamsFor, + hooks: { + renderResults: () => renderResults(app), + showExportProgress: (onCancel) => showExportProgress(onCancel), + toast: (message) => flashToast(message, { document: doc }), + loadSchema: () => { void app.loadSchema(); }, + }, + }); + app.exports = exportService; + // The run/runScript/runEntry/cancel orchestration (#276 Phase 3a) now lives // in ui/workbench/workbench-session.ts — a route-scoped session that owns // the run bookkeeping (runT0/runQueryId/runTick) and the in-flight @@ -1243,331 +1252,18 @@ export function createApp(env: CreateAppEnv = {}): App { } function copyResult(): void { copySnapshot(exportableResult(), doc); } // --- streaming export (issue #87 single-file / #99 script) -------------- - // Full, uncapped export of a query — never the loaded grid — streamed - // straight to a user-chosen file. Its own query_id + abort, kept separate - // from the workbench session's own private run bookkeeping so an export and - // a grid run never clobber each other's cancel state. - let exportAbort: AbortController | null = null; - let exportQueryId: string | null = null; - // Script-export state (issue #99) — its own abort/query-id, reassigned each - // iteration so Cancel reaches the in-flight statement, and kept distinct - // from both the workbench session's own run bookkeeping and the single- - // export state above. - let exportScriptAbort: AbortController | null = null; - let exportScriptQueryId: string | null = null; - let exportScriptCancelled = false; - let exportScriptTick: ReturnType | null = null; - - // The Export button dispatches by statement count: one statement keeps the - // rich single-file flow below; more than one opens the script-export flow - // (its own directory + per-statement log, since one file per script makes - // no sense). Mirrors runEntry's split/branch. - function exportEntry(): Promise | undefined { - if (app.activeTab().editorMode !== 'sql') return undefined; - if (app.state.exporting.value) return undefined; - const waveMs = wallNow(); // one wall clock for this export wave (gate + args) - if (params.varGateBlocked(waveMs)) return undefined; // don't export with unfilled variables (#134) - const input = app.activeTab().sqlDraft; - const statements = splitStatements(input); - if (!statements.length) { flashToast('Nothing to export', { document: doc }); return undefined; } - if (statements.length === 1) return exportDirect(statements[0], waveMs); - return exportScriptEntry(statements, input, waveMs); - } - - async function exportDirect(sqlInput: string, waveMs: number): Promise { - if (app.activeTab().editorMode !== 'sql') return; - if (app.state.exporting.value) return; - if (!app.canExport()) return; // aria-disabled button; defensive guard - const tab = app.activeTab(); - // Export streams the execution view (#165) — identical bytes without blocks. - const { sql, format } = prepareExportSql(params.execStatementSql(sqlInput)); - if (!sql) { flashToast('Nothing to export', { document: doc }); return; } - const { ext, mime } = formatFileMeta(format); - // Prepared args captured NOW — synchronously with exportEntry's gate - // check, BEFORE the picker/auth awaits below (review F6 invariant, shared - // with run/runScript/exportScript): gate and args see the same varValues - // snapshot; edits during those awaits apply to the next export. (Session - // params stay live below — they don't read varValues.) - const paramArgs = mergedSourceArgs(params.prepareTabSource(sql, waveMs)); - - // Flip the flag before the picker (not after, like the file handle) so a - // second click while the native dialog is still open is blocked by the - // guard above — the button's own disabled state (setExportBtn) also - // reflects this via an effect, but the guard is the authority. - app.state.exporting.value = true; - try { - // Picker FIRST, before any await: showSaveFilePicker requires the click's - // transient activation, which a prior await (e.g. a token refresh in - // ensureConfig/getToken can be a network round trip) would forfeit. - // `!`: app.canExport() above already gated on this being non-null. - const pickFile = app.showSaveFilePicker!; - let handle: FileHandleLike; - try { - handle = (await pickFile({ - suggestedName: exportFilename(tab.name, Date.now(), ext), - types: [{ description: format + ' data', accept: { [mime]: ['.' + ext] } }], - })) as FileHandleLike; - } catch (e) { - if (e instanceof Error && e.name === 'AbortError') return; // user dismissed the picker - flashToast('Save dialog failed: ' + String((e instanceof Error && e.message) || e), { document: doc }); - return; - } - - // Now the awaits are safe — we already hold the file handle. - await ensureConfig(); - if (!(await getToken())) { chCtx.onSignedOut(); return; } - - exportQueryId = 'export-' + uid(''); - exportAbort = new AbortController(); - const progress = showExportProgress(cancelExport); - try { - const resp = await ch.exportQuery(chCtx, sql, { - queryId: exportQueryId, signal: exportAbort.signal, format, - // Native query-parameter substitution (#134/#173), same as run() — - // paramArgs is the wave-start snapshot captured above (review F6). - params: { ...sessionParamsFor(tab, [sql]), ...paramArgs }, - }); - const tag = resp.headers.get('X-ClickHouse-Exception-Tag'); // null on servers < 24.11 - const err = await streamToFile(resp, handle, { - signal: exportAbort.signal, tag, onProgress: (bytes) => progress.update(bytes), - }); - if (err) flashToast('Export incomplete — server error mid-stream: ' + err, { document: doc }); - else flashToast('Export complete', { document: doc }); - } catch (e) { - // AbortError (cancelled) and 'signed out' (chCtx.onSignedOut already - // rendered the login screen) both already have their own signal — an - // extra toast on top would just be a confusing second message. - const msg = String((e instanceof Error && e.message) || e); - if (!(e instanceof Error && e.name === 'AbortError') && msg !== 'signed out') { - flashToast('Export failed: ' + msg, { document: doc }); - } - } finally { - progress.remove(); - exportAbort = null; - exportQueryId = null; - } - } finally { - app.state.exporting.value = false; - } - } - - // Stream `resp.body` to `handle` with a hold-back buffer: ClickHouse's - // mid-stream exception frame (findExceptionFrame) is at most 16 KiB and - // always trailing, so bytes are only committed to disk once they've aged - // out of a 32 KiB window — at EOF the retained tail is inspected and only - // the clean prefix is written, so a mid-stream exception is never written - // into the file. Memory stays flat (one HOLDBACK-sized buffer) regardless of - // result size. Reads the stream directly (not via a TransformStream) because - // the write is conditional (withhold, inspect, commit) — a passthrough - // transform can't un-write. Returns the CH error message, or null when clean. - async function streamToFile( - resp: Response, handle: FileHandleLike, - { signal, tag, onProgress }: { signal: AbortSignal; tag: string | null; onProgress: (bytes: number) => void }, - ): Promise { - const writable = await handle.createWritable(); - const HOLDBACK = 32 * 1024; // >= ClickHouse's MAX_EXCEPTION_SIZE (16 KiB) + margin - const reader = resp.body!.getReader(); - let held = new Uint8Array(0); - let written = 0; - try { - for (;;) { - const { done, value } = await reader.read(); - if (done) break; - if (signal.aborted) throw new DOMException('aborted', 'AbortError'); - const merged = new Uint8Array(held.length + value.length); - merged.set(held); - merged.set(value, held.length); - const commit = Math.max(0, merged.length - HOLDBACK); - if (commit > 0) { - await writable.write(merged.subarray(0, commit)); - written += commit; - onProgress(written); - } - held = merged.subarray(commit); - } - // EOF: inspect the retained tail (latin1: 1 char per byte, for byte-accurate slicing). - const frame = findExceptionFrame(latin1(held), tag); - const clean = frame ? held.subarray(0, frame.cleanBytes) : held; - if (clean.length) { - await writable.write(clean); - written += clean.length; - onProgress(written); - } - await writable.close(); - return frame ? frame.message : null; - } catch (e) { - // writable.abort() would discard everything already committed: on - // Chrome/File System Access API it leaves a hidden, 0-byte - // `.crswap` swap file behind and never materializes the visible - // target at all — so a cancelled/failed export recovers nothing. - // close() instead finalizes the bytes already written under the - // target handle, then move() (Chrome 110+) renames it in place with - // a `.partial` suffix so it reads as an inspectable, clearly-labeled - // partial artifact rather than a clean export. Best-effort: on - // browsers without move() (or if it throws), the file is still - // recoverable under its original name, just without the suffix. - await writable.close().catch(() => {}); - if (typeof handle.move === 'function') await handle.move(handle.name + '.partial').catch(() => {}); - throw e; - } finally { - reader.releaseLock(); - } - } - const latin1 = (bytes: Uint8Array): string => { let s = ''; for (const b of bytes) s += String.fromCharCode(b); return s; }; - - // Mirrors cancel() (the grid run) but on the export's own id/abort. - function cancelExport(): void { - if (exportAbort) exportAbort.abort(); - ch.killQuery(chCtx, exportQueryId, sqlString); - } - - // Directory picker first (transient-activation rule, same as exportDirect's - // save-file picker), and skip the prompt entirely when there's nothing to - // export — no point asking for a folder a script will never write into. - async function exportScriptEntry(statements: string[], originalInput: string, waveMs: number): Promise { - if (!app.canExportScript()) { - flashToast('Script export requires Chrome/Edge directory access over HTTPS', { document: doc }); - return; - } - if (!statements.some(isRowReturning)) { - flashToast('Nothing to export — script has no result-producing statements.', { document: doc }); - return; - } - // One prepared batch for the whole export wave (#173), captured NOW — - // synchronously with exportEntry's gate check, BEFORE the directory-picker - // and auth awaits below (review F6 invariant, shared with run/runScript/ - // exportDirect): gate and args see the same varValues snapshot; edits - // during those awaits apply to the next export. `statements` came from - // splitStatements(originalInput), so the batch aligns by index. - const paramSrc = params.prepareTabSource(originalInput, waveMs); - // Flip the flag before the picker (mirrors exportDirect) so a second click - // while the directory dialog / auth is still in flight is blocked by - // exportEntry's guard — exportScript itself doesn't set this until after - // those awaits, which would otherwise leave a re-entrancy window open. - app.state.exporting.value = true; - try { - // `!`: app.canExportScript() above already gated on this being non-null. - const pickDir = app.showDirectoryPicker!; - let dir: DirectoryHandleLike; - try { - dir = (await pickDir({ mode: 'readwrite' })) as DirectoryHandleLike; - } catch (e) { - if (e instanceof Error && e.name === 'AbortError') return; // dismissed → silent no-op - flashToast('Folder dialog failed: ' + String((e instanceof Error && e.message) || e), { document: doc }); - return; - } - await ensureConfig(); - if (!(await getToken())) { chCtx.onSignedOut(); return; } - await exportScript(statements, dir, paramSrc); - } finally { - // No-op if exportScript already reset it — covers every early-return - // path above that never reaches exportScript's own finally. - app.state.exporting.value = false; - } - } - - // Run a script's statements sequentially into `dir`, one file per - // row-returning statement, for effect otherwise. A single shared session - // carries SET/TEMPORARY state across statements (sessionParamsFor). The log - // lives in tab.result.scriptExport — per-statement metadata only (status, - // file, bytes, time); the exported rows themselves are never held in - // memory/state, so a multi-million-row script export stays flat. Stop on - // first failure, mirroring runScript's script grid — but with no retry - // (statements run one-at-a-time in a single session, so SESSION_IS_LOCKED - // can't self-collide, and a partially-written file shouldn't be silently - // re-attempted). - async function exportScript(statements: string[], dir: DirectoryHandleLike, paramSrc: PreparedSource): Promise { - const tab = app.activeTab(); - const t0 = now(); - const sp = sessionParamsFor(tab, statements); - // `paramSrc` is the wave's prepared batch (#173), captured by - // exportScriptEntry at wave start, before its awaits (review F6). - const entries: ScriptExportEntry[] = statements.map((sql, i) => ({ - i, sql, type: isRowReturning(sql) ? 'rows' : 'effect', - status: 'pending', file: null, bytes: 0, startedAt: null, ms: 0, error: null, - })); - const scriptExportResult: ScriptExportResult = { scriptExport: entries, startedAt: t0 }; - Object.assign(tab, { result: scriptExportResult }); - app.state.resultSort = { col: null, dir: 'asc' }; - exportScriptCancelled = false; - app.state.exporting.value = true; - const taken = new Set(); - try { - // Live elapsed for the running row (bytes tick via onProgress; this ticks - // time). Started inside the try so a throw here still clears it below — - // an interval set before the try would otherwise leak forever. - exportScriptTick = setInterval(() => renderResults(app), 200); - renderResults(app); - for (const e of entries) { - if (exportScriptCancelled) { e.status = 'skipped'; continue; } - // Wire text = the pipeline's per-statement execution view (#165); - // verbatim for effect/DDL statements and for block-free SQL. - const execStmt = paramSrc.statements[e.i].sql; - const { sql, format } = prepareExportSql(execStmt); - // Per-statement prepared args (#134/#173): the pipeline binds only - // row-returning statements, so an effect/DDL statement (incl. CREATE - // VIEW) is sent with its {name:Type} placeholders intact. - const params = { ...sp, ...paramSrc.statements[e.i].args }; - exportScriptQueryId = 'export-' + uid(''); - exportScriptAbort = new AbortController(); - const signal = exportScriptAbort.signal; - e.startedAt = now(); - e.status = e.type === 'rows' ? 'exporting' : 'running'; - renderResults(app); - try { - if (e.type !== 'rows') { - const out = await ch.runQuery(chCtx, execStmt, - { format: 'TSV', signal, queryId: exportScriptQueryId, params }); - if (out.error != null) throw new Error(out.error); - e.status = 'ok'; - } else { - const { ext } = formatFileMeta(format); - const name = scriptExportName(e.i, e.sql || '', ext, taken); - taken.add(name); - e.file = name; - const fileHandle = await dir.getFileHandle(name, { create: true }); - const resp = await ch.exportQuery(chCtx, sql, - { queryId: exportScriptQueryId, signal, format, params }); - const tag = resp.headers.get('X-ClickHouse-Exception-Tag'); - const midErr = await streamToFile(resp, fileHandle, - { signal, tag, onProgress: (b) => { e.bytes = b; } }); - if (midErr) { - e.status = 'failed'; - e.error = 'File may be incomplete; server failed after streaming started. ' + midErr; - e.ms = now() - e.startedAt!; - break; // stop-on-first-failure - } - e.status = 'ok'; - } - e.ms = now() - e.startedAt!; - renderResults(app); - } catch (ex) { - e.ms = now() - e.startedAt!; - if (ex instanceof Error && ex.name === 'AbortError') { e.status = 'cancelled'; exportScriptCancelled = true; } - else { e.status = 'failed'; e.error = String((ex instanceof Error && ex.message) || ex); } - break; // stop-on-first-failure - } - } - for (const e of entries) if (e.status === 'pending') e.status = 'skipped'; - } finally { - clearInterval(exportScriptTick as ReturnType); exportScriptTick = null; - exportScriptAbort = null; - exportScriptQueryId = null; - app.state.exporting.value = false; - scriptExportResult.elapsedMs = now() - t0; - // A schema-mutating effect statement that actually ran refreshes the tree - // (mirrors runScript) even though this export ran outside runScript. - if (entries.some((e) => e.status === 'ok' && isSchemaMutatingSql(e.sql))) app.loadSchema(); - renderResults(app); - } - } - - // Mirrors cancelExport but on the script's own active id/abort. - function cancelExportScript(): void { - exportScriptCancelled = true; // stops the loop from starting the next statement - if (exportScriptAbort) exportScriptAbort.abort(); - ch.killQuery(chCtx, exportScriptQueryId, sqlString); - } + // The export POLICY (statement-count dispatch, the picker-first/stream/ + // hold-back-buffer path, the script-export transport loop, both cancel + // paths) now lives in `application/export-service.ts` (#276 Phase 4B2 — + // `exportService`, constructed above alongside `params`). `exportEntry`/ + // `exportDirect`/`cancelExport`/`cancelExportScript` below are one-line + // delegates onto it, kept as named locals (rather than inlining + // `exportService.*` at the actions-registry call sites) so those registry + // entries stay untouched. + const exportEntry = (): Promise | undefined => exportService.exportEntry(); + const exportDirect = (sqlInput: string, waveMs: number): Promise => exportService.exportDirect(sqlInput, waveMs); + const cancelExport = (): void => exportService.cancelExport(); + const cancelExportScript = (): void => exportService.cancelExportScript(); // Inline progress banner (bytes written + elapsed, with Cancel) — no extra // tab/window; see the issue's "Why inline, not a child tab" rationale. diff --git a/src/ui/app.types.ts b/src/ui/app.types.ts index d4303a9..8fe9f51 100644 --- a/src/ui/app.types.ts +++ b/src/ui/app.types.ts @@ -22,6 +22,7 @@ import type { SavedQueryV2 } from '../generated/json-schema.types.js'; import type { DynamicSources } from '../core/spec-completion.js'; import type { WorkbenchSession } from './workbench/workbench-session.js'; import type { WorkbenchParameterSession } from '../application/workbench-parameter-session.js'; +import type { ExportService } from '../application/export-service.js'; export type { QueryTab as Tab, AppState as State } from '../state.js'; @@ -312,6 +313,16 @@ export interface App { * `recordBoundParams`/`clearVarRecent`/`clearAllVarRecent` (declared * below, under Persistence) are one-line delegates onto this. */ params: WorkbenchParameterSession; + /** The streaming single-file export (issue #87) + multi-statement script + * export (issue #99) POLICY (#276 Phase 4B2 — + * `src/application/export-service.ts`), constructible without + * App/AppState/DOM. `actions.exportEntry`/`.exportDirect`/`.cancelExport`/ + * `.cancelExportScript` are one-line delegates onto this; `state.exporting` + * stays an `AppState` signal this service is the sole writer of. + * `canExport`/`canExportScript` (env capability checks) and + * `showExportProgress` (the DOM progress banner) stay app.ts-owned, + * injected into this service. */ + exports: ExportService; /** The shared request/stream/normalize + multiquery-script transport * service (#276 Phase 1) — `src/application/query-execution-service.ts`, * constructible without App/AppState/DOM. `src/ui/**` may depend on diff --git a/tests/helpers/fake-app.ts b/tests/helpers/fake-app.ts index 193f772..182dcc4 100644 --- a/tests/helpers/fake-app.ts +++ b/tests/helpers/fake-app.ts @@ -34,6 +34,7 @@ import type { ConnectionSession } from '../../src/application/connection-session import type { SchemaCatalogService } from '../../src/application/schema-catalog-service.js'; import type { WorkbenchSession } from '../../src/ui/workbench/workbench-session.js'; import type { WorkbenchParameterSession } from '../../src/application/workbench-parameter-session.js'; +import type { ExportService } from '../../src/application/export-service.js'; import { assembleReferenceData, buildCompletions } from '../../src/core/completions.js'; import type { AssembledReference } from '../../src/core/completions.js'; @@ -168,6 +169,16 @@ const paramsDefaults: WorkbenchParameterSession = { saveVarRecentDisabled: vi.fn(), }; +// A minimal `ExportService` stub (#276 Phase 4B2) — no render-module fixture +// exercises the service directly (that's export-service.test.ts's job); this +// just satisfies the `App.exports` contract. +const exportsDefaults: ExportService = { + exportEntry: vi.fn(), + exportDirect: vi.fn(async () => {}), + cancelExport: vi.fn(), + cancelExportScript: vi.fn(), +}; + // Every `App` member this file's own concrete stubs (below) don't cover, // filled with an inert placeholder never read by a fixture that doesn't // override it — same convention as (and previously duplicated by) each of @@ -248,6 +259,7 @@ const appDefaults: App = { elapsedMs: () => 0, tickElapsed: () => {}, workbench: workbenchDefaults, + exports: exportsDefaults, exec: { executeRead: async (result) => result, executeScript: async () => ({ entries: [], aborted: false }), diff --git a/tests/unit/app.test.ts b/tests/unit/app.test.ts index 0526a3f..614c34a 100644 --- a/tests/unit/app.test.ts +++ b/tests/unit/app.test.ts @@ -3454,8 +3454,23 @@ describe('exhaustive controller coverage', () => { }); }); +// The streaming single-file export (issue #87) and multi-statement script +// export (issue #99) POLICY now lives in `application/export-service.ts` +// (#276 Phase 4B2 — `createExportService`, exercised directly and +// exhaustively by export-service.test.ts: statement-count dispatch, the +// picker-first/stream/hold-back-buffer path, cancellation, the +// directory-picker/per-statement transport loop, file naming, and every +// error/edge-case branch). What's left here is THIN integration coverage — +// the pieces export-service.test.ts's own fake sink/ch-fns/hooks can't +// exercise: `app.canExport`/`app.canExportScript` resolving from the real +// injected env seams, the `setExportBtn` DOM effect reacting to +// `state.exporting`, `app.actions.*` delegating to `app.exports`, and one +// real round trip through each of `app.ts`'s own wiring lines (the +// `ExportSink` wrapper around `showSaveFilePicker`/`showDirectoryPicker`, and +// the `hooks` object's `showExportProgress`/`toast`/`renderResults`/ +// `loadSchema` bodies) so those lines stay covered end-to-end, not just at +// the service layer. describe('streaming export (issue #87)', () => { - const TAG = 'abcdef0123456789'; const fakeWin = (): Window => asWindow({ history: { replaceState: vi.fn() }, navigator: {} }); it('canExport resolves from the injected seams; the toolbar button reflects it', () => { @@ -3473,23 +3488,6 @@ describe('streaming export (issue #87)', () => { expect(enabled.dom.exportBtn!.getAttribute('aria-disabled')).toBeNull(); }); - it('is a no-op when export is unavailable, or when one is already running', async () => { - const showSaveFilePicker1 = vi.fn(); - const unavailable = createApp(env({ window: fakeWin(), showSaveFilePicker: null, isSecureContext: false })); - unavailable.renderApp(); - unavailable.activeTab().sqlDraft = 'SELECT 1'; - await unavailable.actions.exportEntry(); - expect(showSaveFilePicker1).not.toHaveBeenCalled(); - - const showSaveFilePicker2 = vi.fn(); - const busy = createApp(env({ window: fakeWin(), showSaveFilePicker: showSaveFilePicker2, isSecureContext: true })); - busy.renderApp(); - busy.activeTab().sqlDraft = 'SELECT 1'; - busy.state.exporting.value = true; - await busy.actions.exportEntry(); - expect(showSaveFilePicker2).not.toHaveBeenCalled(); - }); - it('"Nothing to export" toast when the editor is empty', async () => { const app = createApp(env({ window: fakeWin(), showSaveFilePicker: vi.fn(), isSecureContext: true })); app.renderApp(); @@ -3498,65 +3496,7 @@ describe('streaming export (issue #87)', () => { expect(qs(document, '.share-toast').textContent).toBe('Nothing to export'); }); - it('query variables (#134): export is blocked when a variable is unfilled', async () => { - const showSaveFilePicker = vi.fn(); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT {database:String}'; - await app.actions.exportEntry(); - expect(showSaveFilePicker).not.toHaveBeenCalled(); - expect(qs(document, '.share-toast').textContent).toContain('database'); - }); - - it('query variables (#134): export sends native param_ args for a filled query', async () => { - const { handle } = fakeFileHandle(); - const showSaveFilePicker = vi.fn(async () => handle); - const EXPORT_SQL = 'SELECT {database:String}\nFORMAT TabSeparatedWithNames'; - const fetch = makeFetch([[(u, sql) => sql === EXPORT_SQL, () => resp({ body: streamBody(['x']) })]]); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT {database:String}'; - app.state.varValues = { database: 'default' }; - await app.actions.exportEntry(); - const exportCall = asMock(fetch).mock.calls.find((c) => c[1] && c[1].body === EXPORT_SQL)!; - expect(exportCall[0]).toMatch(/param_database=default/); - }); - - it('picker AbortError (user dismissed the dialog) is a silent no-op', async () => { - const showSaveFilePicker = vi.fn(async () => { throw Object.assign(new Error('x'), { name: 'AbortError' }); }); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1'; - await app.actions.exportEntry(); - expect(qs(document, '.share-toast')).toBeNull(); - expect(app.state.exporting.value).toBe(false); - }); - - it('a non-abort picker failure toasts "Save dialog failed"', async () => { - const showSaveFilePicker = vi.fn(async () => { throw new Error('disk full'); }); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1'; - await app.actions.exportEntry(); - expect(qs(document, '.share-toast').textContent).toBe('Save dialog failed: disk full'); - }); - - it('signed out (no token): the picker still opens (transient activation preserved), but no query runs', async () => { - const { handle } = fakeFileHandle(); - const showSaveFilePicker = vi.fn(async () => handle); - const fetch = asFetch(vi.fn(async () => resp({ json: { data: [] } }))); // only the config-doc load, if anything - const app = createApp(env({ - window: fakeWin(), showSaveFilePicker, isSecureContext: true, fetch, sessionStorage: memSession({}), - })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1'; - await app.actions.exportEntry(); - expect(showSaveFilePicker).toHaveBeenCalledTimes(1); - expect(handle.createWritable).not.toHaveBeenCalled(); // never reached the streaming step - expect(app.state.exporting.value).toBe(false); - }); - - it('streams a clean result to disk (default TSV) and reports completion', async () => { + it('streams a clean result to disk (default TSV) and reports completion — a real round trip through app.ts\'s own ExportSink/hooks wiring', async () => { const { handle, writable, chunks } = fakeFileHandle(); let pickerOpts: SaveFilePickerOpts | undefined; const showSaveFilePicker = vi.fn(async (opts: unknown) => { pickerOpts = opts as SaveFilePickerOpts; return handle; }); @@ -3577,204 +3517,6 @@ describe('streaming export (issue #87)', () => { expect(exportCall[0]).toContain('default_format=TabSeparatedWithNames'); }); - it('honors an explicit FORMAT in the query for the picker + the request', async () => { - const { handle } = fakeFileHandle(); - let pickerOpts: SaveFilePickerOpts | undefined; - const showSaveFilePicker = vi.fn(async (opts: unknown) => { pickerOpts = opts as SaveFilePickerOpts; return handle; }); - const EXPORT_SQL = 'SELECT 1 FORMAT JSON'; - const fetch = makeFetch([[(u, sql) => sql === EXPORT_SQL, () => resp({ body: streamBody(['[]']) })]]); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = EXPORT_SQL; - await app.actions.exportEntry(); - expect(pickerOpts!.suggestedName).toMatch(/\.json$/); - expect(pickerOpts!.types[0].accept).toEqual({ 'application/json': ['.json'] }); - expect(asMock(fetch).mock.calls.some((c) => c[1] && c[1].body === EXPORT_SQL)).toBe(true); - }); - - it('holds back the trailing 32 KiB and streams the rest incrementally (no full buffering)', async () => { - const { handle, writable, chunks } = fakeFileHandle(); - const showSaveFilePicker = vi.fn(async () => handle); - const big = 'a'.repeat(40960); // > HOLDBACK (32 KiB) in a single chunk - const EXPORT_SQL = 'SELECT 1\nFORMAT TabSeparatedWithNames'; - const fetch = makeFetch([[(u, sql) => sql === EXPORT_SQL, () => resp({ body: streamBody([big]) })]]); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1'; - await app.actions.exportEntry(); - // mid-loop commit (8192 = 40960 - 32768 HOLDBACK) then the EOF flush of the held-back tail. - expect(asAnyMock(writable.write).mock.calls.map((c) => c[0].length)).toEqual([8192, 32768]); - expect(writtenText(chunks)).toBe(big); - expect(writable.close).toHaveBeenCalledTimes(1); - }); - - it('excises a mid-stream exception frame — only clean bytes reach the file; reports "incomplete"', async () => { - const { handle, writable, chunks } = fakeFileHandle(); - const showSaveFilePicker = vi.fn(async () => handle); - const clean = 'x'.repeat(40); - const frame = exceptionFrame(TAG, 'DB::Exception: Memory limit (total) exceeded'); - const EXPORT_SQL = 'SELECT 1\nFORMAT TabSeparatedWithNames'; - const fetch = makeFetch([[(u, sql) => sql === EXPORT_SQL, - () => resp({ body: streamBody([clean, frame]), headers: { 'X-ClickHouse-Exception-Tag': TAG } })]]); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1'; - await app.actions.exportEntry(); - expect(writtenText(chunks)).toBe(clean); // the exception frame never reaches the file - expect(writable.close).toHaveBeenCalledTimes(1); - expect(writable.abort).not.toHaveBeenCalled(); - expect(qs(document, '.share-toast').textContent) - .toBe('Export incomplete — server error mid-stream: DB::Exception: Memory limit (total) exceeded'); - }); - - it('a stream read failure mid-export closes (not aborts) the writable and renames it .partial', async () => { - const { handle, writable } = fakeFileHandle('My_Query.tsv'); - const showSaveFilePicker = vi.fn(async () => handle); - const EXPORT_SQL = 'SELECT 1\nFORMAT TabSeparatedWithNames'; - let reads = 0; - const body = { - getReader: () => ({ - read: async () => { - reads += 1; - if (reads === 1) return { done: false, value: new TextEncoder().encode('partial') }; - throw new Error('network drop'); - }, - releaseLock: () => {}, - }), - }; - const fetch = makeFetch([[(u, sql) => sql === EXPORT_SQL, () => resp({ body })]]); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1'; - await app.actions.exportEntry(); - // close (not abort), so the already-committed bytes materialize under the - // target handle instead of a hidden 0-byte .crswap orphan being left behind. - expect(writable.abort).not.toHaveBeenCalled(); - expect(writable.close).toHaveBeenCalledTimes(1); - expect(handle.move).toHaveBeenCalledWith('My_Query.tsv.partial'); - expect(qs(document, '.share-toast').textContent).toBe('Export failed: network drop'); - expect(app.state.exporting.value).toBe(false); - }); - - it('falls back to leaving the plain (non-renamed) file when the handle has no move() (no File System Access API move support)', async () => { - const { handle, writable } = fakeFileHandle(); - delete handle.move; - const showSaveFilePicker = vi.fn(async () => handle); - const EXPORT_SQL = 'SELECT 1\nFORMAT TabSeparatedWithNames'; - const fetch = makeFetch([[(u, sql) => sql === EXPORT_SQL, () => resp({ body: throwingBody('network drop') })]]); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1'; - await app.actions.exportEntry(); - expect(writable.abort).not.toHaveBeenCalled(); - expect(writable.close).toHaveBeenCalledTimes(1); - // No TypeError from calling a missing move() — the guard held, and the - // original "network drop" error (not a broken-guard error) is what surfaces. - expect(qs(document, '.share-toast').textContent).toBe('Export failed: network drop'); - }); - - it('a failed move() (e.g. name collision) is swallowed — the plain file is still recoverable', async () => { - const { handle, writable } = fakeFileHandle(); - handle.move = vi.fn(async () => { throw new Error('collision'); }); - const showSaveFilePicker = vi.fn(async () => handle); - const EXPORT_SQL = 'SELECT 1\nFORMAT TabSeparatedWithNames'; - const fetch = makeFetch([[(u, sql) => sql === EXPORT_SQL, () => resp({ body: throwingBody('network drop') })]]); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1'; - await app.actions.exportEntry(); - expect(writable.abort).not.toHaveBeenCalled(); - expect(handle.move).toHaveBeenCalledTimes(1); - // move()'s rejection is swallowed, not propagated in place of the original error. - expect(qs(document, '.share-toast').textContent).toBe('Export failed: network drop'); - }); - - it('a pre-header (non-OK) failure toasts "Export failed" without ever opening the writable', async () => { - const { handle } = fakeFileHandle(); - const showSaveFilePicker = vi.fn(async () => handle); - const EXPORT_SQL = 'SELECT 1\nFORMAT TabSeparatedWithNames'; - const fetch = makeFetch([[(u, sql) => sql === EXPORT_SQL, - () => resp({ ok: false, status: 500, text: '{"exception":"DB::Exception: nope"}' })]]); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1'; - await app.actions.exportEntry(); - expect(qs(document, '.share-toast').textContent).toBe('Export failed: DB::Exception: nope'); - expect(handle.createWritable).not.toHaveBeenCalled(); - expect(app.state.exporting.value).toBe(false); - }); - - it('exporting.value is true for the duration of the run; cancel aborts the export\'s own signal + issues its own KILL QUERY', async () => { - const EXPORT_BODY = 'SELECT 1\nFORMAT TabSeparatedWithNames'; - let resolveExportFetch!: (value: FakeResponse | Promise) => void; - const fetch = asFetch(vi.fn((url: string, init?: { body?: string; signal?: AbortSignal }) => (init && init.body === EXPORT_BODY - ? new Promise((res) => { resolveExportFetch = res; }) - : Promise.resolve(resp({ json: { data: [] } }))))); - const { handle } = fakeFileHandle(); - const showSaveFilePicker = vi.fn(async () => handle); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1'; - const pending = app.actions.exportEntry(); - await new Promise((r) => setTimeout(r)); // let the picker + export request kick off - expect(app.state.exporting.value).toBe(true); - const exportCall = asMock(fetch).mock.calls.find((c) => c[1] && c[1].body === EXPORT_BODY)!; - expect(exportCall[1].signal.aborted).toBe(false); - - app.actions.cancelExport(); // the workbench session's own run bookkeeping is untouched — this is the export's own - expect(exportCall[1].signal.aborted).toBe(true); - resolveExportFetch(Promise.reject(Object.assign(new Error('aborted'), { name: 'AbortError' }))); - await pending; - - expect(app.state.exporting.value).toBe(false); - expect(qs(document, '.share-toast')).toBeNull(); // AbortError → silent - expect(asMock(fetch).mock.calls.some((c) => c[1] && /KILL QUERY WHERE query_id = 'export-/.test(c[1].body))).toBe(true); - }); - - it('attaches the tab session_id when the tab already has an open session (e.g. after a TEMPORARY TABLE run)', async () => { - const { handle } = fakeFileHandle(); - const showSaveFilePicker = vi.fn(async () => handle); - const EXPORT_SQL = 'SELECT * FROM t\nFORMAT TabSeparatedWithNames'; - const fetch = makeFetch([[(u, sql) => sql === EXPORT_SQL, () => resp({ body: streamBody(['x']) })]]); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true, fetch })); - app.renderApp(); - const tab = app.activeTab(); - tab.chSession = 'sess-abc'; - tab.sqlDraft = 'SELECT * FROM t'; - await app.actions.exportEntry(); - const exportCall = asMock(fetch).mock.calls.find((c) => c[1] && c[1].body === EXPORT_SQL)!; - expect(exportCall[0]).toContain('session_id=sess-abc'); - }); - - it('suppresses the "Export failed" toast when the underlying error is "signed out" (onSignedOut already showed the login screen)', async () => { - const { handle } = fakeFileHandle(); - const showSaveFilePicker = vi.fn(async () => handle); - const EXPORT_SQL = 'SELECT 1\nFORMAT TabSeparatedWithNames'; - const fetch = makeFetch([[(u, sql) => sql === EXPORT_SQL, () => { throw new Error('signed out'); }]]); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1'; - await app.actions.exportEntry(); - expect(qs(document, '.share-toast')).toBeNull(); - expect(app.state.exporting.value).toBe(false); - }); - - it('a second click while the save-file picker is still open is blocked (exporting flips true before the picker await)', async () => { - let rejectPicker!: (reason: unknown) => void; - const showSaveFilePicker = vi.fn(() => new Promise((_res, rej) => { rejectPicker = rej; })); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, isSecureContext: true })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1'; - const first = app.actions.exportEntry(); - await new Promise((r) => setTimeout(r)); // let the first call reach the picker await - expect(app.state.exporting.value).toBe(true); - await app.actions.exportEntry(); // second click: blocked by the re-entrance guard - expect(showSaveFilePicker).toHaveBeenCalledTimes(1); // only the first call opened a picker - rejectPicker(Object.assign(new Error('x'), { name: 'AbortError' })); - await first; - expect(app.state.exporting.value).toBe(false); - }); - it('setExportBtn reflects the exporting state on the toolbar button, blocking a second click visually too', () => { const app = createApp(env({ window: fakeWin(), showSaveFilePicker: vi.fn(), isSecureContext: true })); app.renderApp(); @@ -3788,10 +3530,26 @@ describe('streaming export (issue #87)', () => { expect(app.dom.exportBtn!.classList.contains('is-disabled')).toBe(false); expect(app.dom.exportBtn!.getAttribute('aria-disabled')).toBeNull(); }); + + it('actions.exportEntry/exportDirect/cancelExport/cancelExportScript delegate to app.exports (#276 Phase 4B2)', async () => { + const app = createApp(env({ window: fakeWin(), showSaveFilePicker: vi.fn(), isSecureContext: true })); + app.renderApp(); + const exportEntry = vi.spyOn(app.exports, 'exportEntry').mockResolvedValue(undefined); + const exportDirect = vi.spyOn(app.exports, 'exportDirect').mockResolvedValue(undefined); + const cancelExport = vi.spyOn(app.exports, 'cancelExport').mockImplementation(() => {}); + const cancelExportScript = vi.spyOn(app.exports, 'cancelExportScript').mockImplementation(() => {}); + await app.actions.exportEntry(); + await app.actions.exportDirect('SELECT 1', 7); + app.actions.cancelExport(); + app.actions.cancelExportScript(); + expect(exportEntry).toHaveBeenCalledTimes(1); + expect(exportDirect).toHaveBeenCalledWith('SELECT 1', 7); + expect(cancelExport).toHaveBeenCalledTimes(1); + expect(cancelExportScript).toHaveBeenCalledTimes(1); + }); }); describe('script export (issue #99)', () => { - const TAG = 'abcdef0123456789'; const fakeWin = (): Window => asWindow({ history: { replaceState: vi.fn() }, navigator: {} }); // A fake FileSystemDirectoryHandle: getFileHandle(name) hands back a fresh @@ -3808,45 +3566,6 @@ describe('script export (issue #99)', () => { return { dir, written }; } - it('exportEntry dispatches by statement count: 1 → the single-file picker, N → the directory picker', async () => { - const showSaveFilePicker = vi.fn(async () => { throw Object.assign(new Error('x'), { name: 'AbortError' }); }); - const showDirectoryPicker = vi.fn(async () => { throw Object.assign(new Error('x'), { name: 'AbortError' }); }); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, showDirectoryPicker, isSecureContext: true })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1'; - await app.actions.exportEntry(); - expect(showSaveFilePicker).toHaveBeenCalledTimes(1); - expect(showDirectoryPicker).not.toHaveBeenCalled(); - - app.activeTab().sqlDraft = 'SELECT 1;\nSELECT 2;'; - await app.actions.exportEntry(); - expect(showDirectoryPicker).toHaveBeenCalledTimes(1); - }); - - it('exportEntry is unavailable in Spec mode and exports sqlDraft after switching to SQL', async () => { - const showSaveFilePicker = vi.fn(async () => { throw Object.assign(new Error('x'), { name: 'AbortError' }); }); - const showDirectoryPicker = vi.fn(async () => { throw Object.assign(new Error('x'), { name: 'AbortError' }); }); - const app = createApp(env({ window: fakeWin(), showSaveFilePicker, showDirectoryPicker, isSecureContext: true })); - app.renderApp(); - app.state.savedQueries = [savedQueryFixture({ id: 's9', name: 'Fav', sql: 'SELECT 1; SELECT 2' })]; - app.actions.loadIntoNewTab(asQueryOrName(app.state.savedQueries[0])); - app.actions.setEditorMode('spec'); - app.dom.specEditorView!.dispatch({ selection: { anchor: 0, head: 8 } }); - await app.actions.exportEntry(); - expect(showSaveFilePicker).not.toHaveBeenCalled(); - expect(showDirectoryPicker).not.toHaveBeenCalled(); - app.actions.setEditorMode('sql'); - await app.actions.exportEntry(); - expect(showDirectoryPicker).toHaveBeenCalledTimes(1); - }); - - it('exportDirect itself guards against empty input (defensive — exportEntry never sends it empty)', async () => { - const app = createApp(env({ window: fakeWin(), showSaveFilePicker: vi.fn(), isSecureContext: true })); - app.renderApp(); - await app.actions.exportDirect(' ', 0); - expect(qs(document, '.share-toast').textContent).toBe('Nothing to export'); - }); - it('canExportScript resolves from the showDirectoryPicker seam + secure context', () => { const withPicker = createApp(env({ window: fakeWin(), showDirectoryPicker: vi.fn(), isSecureContext: true })); expect(withPicker.canExportScript()).toBe(true); @@ -3856,74 +3575,19 @@ describe('script export (issue #99)', () => { expect(insecure.canExportScript()).toBe(false); }); - it('toasts and never opens the directory picker when canExportScript is false', async () => { - const app = createApp(env({ window: fakeWin(), showDirectoryPicker: null, isSecureContext: true })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1;\nSELECT 2;'; - await app.actions.exportEntry(); - expect(qs(document, '.share-toast').textContent) - .toBe('Script export requires Chrome/Edge directory access over HTTPS'); - expect(app.state.exporting.value).toBe(false); - }); - - it('a script with no row-returning statements toasts and never prompts for a directory', async () => { - const showDirectoryPicker = vi.fn(); - const app = createApp(env({ window: fakeWin(), showDirectoryPicker, isSecureContext: true })); - app.renderApp(); - app.activeTab().sqlDraft = 'CREATE TABLE t (a Int8);\nINSERT INTO t VALUES (1);'; - await app.actions.exportEntry(); - expect(showDirectoryPicker).not.toHaveBeenCalled(); - expect(qs(document, '.share-toast').textContent) - .toBe('Nothing to export — script has no result-producing statements.'); - }); - - it('dismissing the directory picker (AbortError) is a silent no-op', async () => { + it('exportEntry dispatches by statement count: 1 → the single-file picker, N → the directory picker', async () => { + const showSaveFilePicker = vi.fn(async () => { throw Object.assign(new Error('x'), { name: 'AbortError' }); }); const showDirectoryPicker = vi.fn(async () => { throw Object.assign(new Error('x'), { name: 'AbortError' }); }); - const app = createApp(env({ window: fakeWin(), showDirectoryPicker, isSecureContext: true })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1;\nSELECT 2;'; - await app.actions.exportEntry(); - expect(qs(document, '.share-toast')).toBeNull(); - expect(app.state.exporting.value).toBe(false); - }); - - it('a non-abort directory picker failure toasts "Folder dialog failed"', async () => { - const showDirectoryPicker = vi.fn(async () => { throw new Error('denied'); }); - const app = createApp(env({ window: fakeWin(), showDirectoryPicker, isSecureContext: true })); + const app = createApp(env({ window: fakeWin(), showSaveFilePicker, showDirectoryPicker, isSecureContext: true })); app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1;\nSELECT 2;'; + app.activeTab().sqlDraft = 'SELECT 1'; await app.actions.exportEntry(); - expect(qs(document, '.share-toast').textContent).toBe('Folder dialog failed: denied'); - }); + expect(showSaveFilePicker).toHaveBeenCalledTimes(1); + expect(showDirectoryPicker).not.toHaveBeenCalled(); - it('the directory picker opens before ensureConfig/getToken; a signed-out tab never runs the script', async () => { - const { dir } = fakeDirHandle(); - const showDirectoryPicker = vi.fn(async () => dir); - const app = createApp(env({ - window: fakeWin(), showDirectoryPicker, isSecureContext: true, sessionStorage: memSession({}), - })); - app.renderApp(); app.activeTab().sqlDraft = 'SELECT 1;\nSELECT 2;'; await app.actions.exportEntry(); - expect(showDirectoryPicker).toHaveBeenCalledTimes(1); // opened despite no token - expect(dir.getFileHandle).not.toHaveBeenCalled(); // never reached the run loop - expect(app.state.exporting.value).toBe(false); - }); - - it('a second click while the directory picker is still open is blocked (exporting flips true before the picker await, like exportDirect)', async () => { - let rejectPicker!: (reason: unknown) => void; - const showDirectoryPicker = vi.fn(() => new Promise((_res, rej) => { rejectPicker = rej; })); - const app = createApp(env({ window: fakeWin(), showDirectoryPicker, isSecureContext: true })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1;\nSELECT 2;'; - const first = app.actions.exportEntry(); - await new Promise((r) => setTimeout(r)); // let the first call reach the picker await - expect(app.state.exporting.value).toBe(true); - await app.actions.exportEntry(); // second click: blocked by the re-entrance guard - expect(showDirectoryPicker).toHaveBeenCalledTimes(1); // only the first call opened a picker - rejectPicker(Object.assign(new Error('x'), { name: 'AbortError' })); - await first; - expect(app.state.exporting.value).toBe(false); + expect(showDirectoryPicker).toHaveBeenCalledTimes(1); }); it('runs statements sequentially in one shared session, effect statements logged ok with no file, rows streamed to their own file', async () => { @@ -3961,157 +3625,6 @@ describe('script export (issue #99)', () => { expect(app.state.exporting.value).toBe(false); }); - it('row-returning statements get distinct, deterministic file names', async () => { - const { dir } = fakeDirHandle(); - const showDirectoryPicker = vi.fn(async () => dir); - const fetch = makeFetch([ - [(u, sql) => sql === 'SELECT 1\nFORMAT TabSeparatedWithNames', () => resp({ body: streamBody(['a']) })], - [(u, sql) => sql === 'SELECT 2\nFORMAT TabSeparatedWithNames', () => resp({ body: streamBody(['b']) })], - ]); - const app = createApp(env({ window: fakeWin(), showDirectoryPicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1;\nSELECT 2;'; - await app.actions.exportEntry(); - const entries = scriptExportOf(app.activeTab()); - expect(entries[0].file).toBe('001-select-1.tsv'); - expect(entries[1].file).toBe('002-select-2.tsv'); - }); - - it('respects an explicit trailing FORMAT per statement', async () => { - const { dir } = fakeDirHandle(); - const showDirectoryPicker = vi.fn(async () => dir); - const fetch = makeFetch([ - [(u, sql) => sql === 'SELECT 1 FORMAT JSON', () => resp({ body: streamBody(['[]']) })], - [(u, sql) => sql === 'SELECT 2\nFORMAT TabSeparatedWithNames', () => resp({ body: streamBody(['x']) })], - ]); - const app = createApp(env({ window: fakeWin(), showDirectoryPicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1 FORMAT JSON;\nSELECT 2;'; - await app.actions.exportEntry(); - const entries = scriptExportOf(app.activeTab()); - expect(entries[0].file).toBe('001-select-1-format-json.json'); - expect(entries[1].file).toBe('002-select-2.tsv'); - }); - - it('a non-row statement error marks it failed with no file and stops the script; the rest are skipped', async () => { - const { dir } = fakeDirHandle(); - const showDirectoryPicker = vi.fn(async () => dir); - const fetch = makeFetch([ - [(u, sql) => sql === 'CREATE TABLE bad', () => resp({ ok: false, status: 500, text: '{"exception":"DB::Exception: table exists"}' })], - ]); - const app = createApp(env({ window: fakeWin(), showDirectoryPicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'CREATE TABLE bad;\nSELECT 1;'; - await app.actions.exportEntry(); - const entries = scriptExportOf(app.activeTab()); - expect(entries[0].status).toBe('failed'); - expect(entries[0].error).toBe('DB::Exception: table exists'); - expect(entries[0].file).toBeNull(); - expect(entries[1].status).toBe('skipped'); - expect(dir.getFileHandle).not.toHaveBeenCalled(); - }); - - it('a pre-header (non-OK) export failure marks the row failed and stops; the rest are skipped', async () => { - const { dir } = fakeDirHandle(); - const showDirectoryPicker = vi.fn(async () => dir); - const fetch = makeFetch([ - [(u, sql) => sql === 'SELECT 1\nFORMAT TabSeparatedWithNames', - () => resp({ ok: false, status: 500, text: '{"exception":"DB::Exception: nope"}' })], - ]); - const app = createApp(env({ window: fakeWin(), showDirectoryPicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1;\nSELECT 2;'; - await app.actions.exportEntry(); - const entries = scriptExportOf(app.activeTab()); - expect(entries[0].status).toBe('failed'); - expect(entries[0].error).toBe('DB::Exception: nope'); - expect(entries[1].status).toBe('skipped'); - }); - - it('a mid-stream exception marks the row failed/incomplete and stops the script (regression: streamToFile\'s return must not be ignored)', async () => { - const { dir, written } = fakeDirHandle(); - const showDirectoryPicker = vi.fn(async () => dir); - const clean = 'x'.repeat(10); - const frame = exceptionFrame(TAG, 'DB::Exception: Memory limit (total) exceeded'); - const fetch = makeFetch([ - [(u, sql) => sql === 'SELECT 1\nFORMAT TabSeparatedWithNames', - () => resp({ body: streamBody([clean, frame]), headers: { 'X-ClickHouse-Exception-Tag': TAG } })], - ]); - const app = createApp(env({ window: fakeWin(), showDirectoryPicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1;\nSELECT 2;'; - await app.actions.exportEntry(); - const entries = scriptExportOf(app.activeTab()); - expect(entries[0].status).toBe('failed'); - expect(entries[0].error).toBe('File may be incomplete; server failed after streaming started. DB::Exception: Memory limit (total) exceeded'); - expect(writtenText(written.get('001-select-1.tsv')!.chunks)).toBe(clean); // the exception frame never reaches the file - expect(entries[1].status).toBe('skipped'); - }); - - it('never retries — a transient SESSION_IS_LOCKED failure is reported like any other error', async () => { - const { dir } = fakeDirHandle(); - const showDirectoryPicker = vi.fn(async () => dir); - const fetch = makeFetch([ - [(u, sql) => sql === 'INSERT INTO t VALUES (1)', - () => resp({ ok: false, status: 500, text: '{"exception":"Code: 373. DB::Exception: SESSION_IS_LOCKED"}' })], - ]); - const app = createApp(env({ window: fakeWin(), showDirectoryPicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'INSERT INTO t VALUES (1);\nSELECT 1;'; - await app.actions.exportEntry(); - const insertCalls = asMock(fetch).mock.calls.filter((c) => c[1] && c[1].body === 'INSERT INTO t VALUES (1)'); - expect(insertCalls).toHaveLength(1); // no retry - expect(scriptExportOf(app.activeTab())[0].status).toBe('failed'); - }); - - it('cancel aborts the active row, marks it cancelled, skips the rest, kills the active query, and keeps completed files', async () => { - const { dir, written } = fakeDirHandle(); - const showDirectoryPicker = vi.fn(async () => dir); - let resolveSecond!: (value: FakeResponse | Promise) => void; - const fetch = makeFetch([ - [(u, sql) => sql === 'SELECT 1\nFORMAT TabSeparatedWithNames', () => resp({ body: streamBody(['a']) })], - [(u, sql) => sql === 'SELECT 2\nFORMAT TabSeparatedWithNames', () => new Promise((res) => { resolveSecond = res; })], - ]); - const app = createApp(env({ window: fakeWin(), showDirectoryPicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'SELECT 1;\nSELECT 2;\nSELECT 3;'; - const pending = app.actions.exportEntry(); - await new Promise((r) => setTimeout(r)); // let stmt1 finish and stmt2's request kick off - const entries = scriptExportOf(app.activeTab()); - expect(entries[0].status).toBe('ok'); - expect(entries[1].status).toBe('exporting'); - - app.actions.cancelExportScript(); - resolveSecond(Promise.reject(Object.assign(new Error('aborted'), { name: 'AbortError' }))); - await pending; - - expect(entries[1].status).toBe('cancelled'); - expect(entries[2].status).toBe('skipped'); - expect(asMock(fetch).mock.calls.some((c) => c[1] && /KILL QUERY WHERE query_id = 'export-/.test(c[1].body))).toBe(true); - expect(written.get('001-select-1.tsv')!.writable.close).toHaveBeenCalledTimes(1); // completed file kept - expect(app.state.exporting.value).toBe(false); - }); - - it('a cancel that arrives just after a statement completed cleanly still skips the remaining statements', async () => { - const { dir } = fakeDirHandle(); - const showDirectoryPicker = vi.fn(async () => dir); - let resolveFirst!: (value: FakeResponse | Promise) => void; - const fetch = makeFetch([ - [(u, sql) => sql === 'CREATE TABLE t (a Int8)', () => new Promise((res) => { resolveFirst = res; })], - ]); - const app = createApp(env({ window: fakeWin(), showDirectoryPicker, isSecureContext: true, fetch })); - app.renderApp(); - app.activeTab().sqlDraft = 'CREATE TABLE t (a Int8);\nSELECT 1;'; - const pending = app.actions.exportEntry(); - await new Promise((r) => setTimeout(r)); // let it reach the pending fetch for stmt1 - app.actions.cancelExportScript(); // cancel arrives while stmt1 is still in flight... - resolveFirst(resp({ text: '' })); // ...but the request completes cleanly anyway - await pending; - const entries = scriptExportOf(app.activeTab()); - expect(entries[0].status).toBe('ok'); // completed before the cancel could affect it - expect(entries[1].status).toBe('skipped'); // caught at the top of stmt2's iteration - }); - it('refreshes the schema when an effect statement that actually ran is schema-mutating', async () => { const { dir } = fakeDirHandle(); const showDirectoryPicker = vi.fn(async () => dir); @@ -4127,22 +3640,6 @@ describe('script export (issue #99)', () => { await app.actions.exportEntry(); expect(spy).toHaveBeenCalledTimes(1); }); - - it('does not refresh the schema when no statement that ran was schema-mutating', async () => { - const { dir } = fakeDirHandle(); - const showDirectoryPicker = vi.fn(async () => dir); - const fetch = makeFetch([ - [(u, sql) => sql === 'SELECT 1\nFORMAT TabSeparatedWithNames', () => resp({ body: streamBody(['x']) })], - [(u, sql) => sql === 'SELECT 2\nFORMAT TabSeparatedWithNames', () => resp({ body: streamBody(['y']) })], - ]); - const app = createApp(env({ window: fakeWin(), showDirectoryPicker, isSecureContext: true, fetch })); - app.renderApp(); - await new Promise((r) => setTimeout(r)); - const spy = vi.spyOn(app, 'loadSchema'); - app.activeTab().sqlDraft = 'SELECT 1;\nSELECT 2;'; - await app.actions.exportEntry(); - expect(spy).not.toHaveBeenCalled(); - }); }); describe('schema lineage graph (drag a db/table onto the results pane)', () => { diff --git a/tests/unit/export-service.test.ts b/tests/unit/export-service.test.ts new file mode 100644 index 0000000..834dc37 --- /dev/null +++ b/tests/unit/export-service.test.ts @@ -0,0 +1,766 @@ +import { describe, it, expect, vi } from 'vitest'; +import type { Mock } from 'vitest'; +import { signal } from '@preact/signals-core'; +import { sqlString } from '../../src/core/format.js'; +import { splitStatements } from '../../src/core/sql-split.js'; +import { createExportService } from '../../src/application/export-service.js'; +import type { + ExportServiceDeps, ExportStateSlice, ExportHooks, ExportSink, + FileHandleLike, DirectoryHandleLike, WritableFileStreamLike, +} from '../../src/application/export-service.js'; +import { newTabObj } from '../../src/state.js'; +import type { QueryTab } from '../../src/state.js'; +import type { ChCtx, RunQueryResult } from '../../src/net/ch-client.js'; +import type { PreparedSource, PreparedStatement } from '../../src/core/param-pipeline.js'; +import type { WorkbenchParameterSession } from '../../src/application/workbench-parameter-session.js'; + +// ── Small deferred/flush helpers (mirrors workbench-session.test.ts's own +// convention for scripting async picker/fetch behaviors on a test's own +// schedule). ────────────────────────────────────────────────────────────── + +function deferred(): { promise: Promise; resolve: (v: T) => void; reject: (e: unknown) => void } { + let resolve!: (v: T) => void; + let reject!: (e: unknown) => void; + const promise = new Promise((res, rej) => { resolve = res; reject = rej; }); + return { promise, resolve, reject }; +} +function flush(): Promise { + return new Promise((r) => setTimeout(r, 0)); +} +function abortError(): Error { + return Object.assign(new Error('x'), { name: 'AbortError' }); +} + +function preparedStatement(over: Partial = {}): PreparedStatement { + return { sql: 'SELECT 1', args: {}, boundParams: [], ...over }; +} +function preparedSource(over: Partial = {}): PreparedSource { + return { + id: 'tab', statements: [preparedStatement()], missing: [], invalid: [], errors: [], runnable: true, ...over, + }; +} + +// ── Streaming-response / File System Access fakes (ported from +// app.test.ts's own identically-named helpers — see that file's header +// comment on why these aren't a shared tests/helpers/ module: this service's +// tests mock `exportQuery`/`runQuery` directly rather than a `fetch` seam, so +// only the Response/file-handle SHAPES are shared, not the fetch-routing +// machinery). ────────────────────────────────────────────────────────────── + +interface FakeBody { getReader(): { read(): Promise<{ done: boolean; value?: Uint8Array }>; releaseLock(): void } } +function streamBody(lines: string[]): FakeBody { + let i = 0; + return { + getReader: () => ({ + read: async () => (i < lines.length ? { done: false, value: new TextEncoder().encode(lines[i++]) } : { done: true }), + releaseLock: () => {}, + }), + }; +} +function throwingBody(message: string): FakeBody { + return { getReader: () => ({ read: async () => { throw new Error(message); }, releaseLock: () => {} }) }; +} +interface FakeExportResponse { headers: { get(name: string): string | null }; body?: FakeBody | null } +function fakeExportResponse(opts: { body?: FakeBody | null; headers?: Record } = {}): FakeExportResponse { + return { body: opts.body, headers: { get: (name) => (opts.headers && opts.headers[name]) ?? null } }; +} +// `ExportServiceDeps.exportQuery`'s real signature returns a genuine DOM +// `Response`; a `{headers,body}`-only fake doesn't overlap enough of the real +// interface for a direct `as Response` (same "object"-parameter bridge as +// app.test.ts's own `asFetch`/`asWindow`). +const asResponse = (v: object): Response => v as Response; + +// Build a ClickHouse mid-stream exception frame's raw text (issue #87): +// \r\n__exception__\r\n\r\n\n \r\n__exception__\r\n +function exceptionFrame(tag: string, message: string): string { + const len = new TextEncoder().encode(message).length; + return '\r\n__exception__\r\n' + tag + '\r\n' + message + '\n' + len + ' ' + tag + '\r\n__exception__\r\n'; +} + +interface FakeWritable { write(chunk: Uint8Array): Promise; close(): Promise; abort(): Promise } +interface FakeFileHandle { name: string; createWritable(): Promise; move?(name: string): Promise } +function fakeFileHandle(name = 'export.tsv'): { handle: FakeFileHandle; writable: FakeWritable; chunks: Uint8Array[] } { + const chunks: Uint8Array[] = []; + const writable: FakeWritable = { + write: vi.fn(async (chunk: Uint8Array) => { chunks.push(Uint8Array.from(chunk)); }), + close: vi.fn(async () => {}), + abort: vi.fn(async () => {}), + }; + const handle: FakeFileHandle = { name, createWritable: vi.fn(async () => writable), move: vi.fn(async () => {}) }; + return { handle, writable, chunks }; +} +function writtenText(chunks: Uint8Array[]): string { + const total = chunks.reduce((n, c) => n + c.length, 0); + const merged = new Uint8Array(total); + let o = 0; + for (const c of chunks) { merged.set(c, o); o += c.length; } + return new TextDecoder().decode(merged); +} +// A fake FileSystemDirectoryHandle: getFileHandle(name) hands back a fresh +// fakeFileHandle() and remembers it (keyed by name) for write assertions. +function fakeDirHandle(): { dir: DirectoryHandleLike; written: Map> } { + const written = new Map>(); + const dir: DirectoryHandleLike = { + getFileHandle: vi.fn(async (name: string) => { + const f = fakeFileHandle(); + written.set(name, f); + return f.handle as unknown as FileHandleLike; + }), + }; + return { dir, written }; +} +const asFileHandleLike = (v: FakeFileHandle): FileHandleLike => v as unknown as FileHandleLike; +const asWritableLike = (v: FakeWritable): WritableFileStreamLike => v as unknown as WritableFileStreamLike; +void asWritableLike; + +// ── Fakes for the service's own injected deps ─────────────────────────────── + +function makeCh(): { exportQuery: Mock; runQuery: Mock; killQuery: Mock } { + const exportQuery = vi.fn(async () => asResponse(fakeExportResponse({ body: streamBody([]) }))); + const runQuery = vi.fn(async (): Promise => ({})); + const killQuery = vi.fn(async () => {}); + return { exportQuery, runQuery, killQuery }; +} + +function makeState(over: Partial = {}): ExportStateSlice { + return { exporting: signal(false), resultSort: { col: null, dir: 'asc' }, ...over }; +} + +type ExportParamsDeps = Pick; +function makeParams(over: Partial = {}): ExportParamsDeps { + return { + // Splits `sql` the same way `exportEntry`'s own `splitStatements` call + // does, so `paramSrc.statements[i]` aligns with the script's own + // per-statement array by default — a test overrides this only when it + // cares about specific per-statement args/sql. + prepareTabSource: vi.fn((sql: string) => preparedSource({ statements: splitStatements(sql).map((s) => preparedStatement({ sql: s })) })), + varGateBlocked: vi.fn(() => false), + execStatementSql: vi.fn((stmt: string) => stmt), + ...over, + }; +} + +function makeHooks(over: Partial = {}): ExportHooks { + return { + renderResults: vi.fn(), + showExportProgress: vi.fn(() => ({ update: vi.fn(), remove: vi.fn() })), + toast: vi.fn(), + loadSchema: vi.fn(), + ...over, + }; +} + +function makeSink(over: Partial = {}): ExportSink { + return { + pickFile: vi.fn(async () => asFileHandleLike(fakeFileHandle().handle)), + pickDirectory: vi.fn(async () => fakeDirHandle().dir), + ...over, + }; +} + +interface Harness { + deps: ExportServiceDeps; + state: ExportStateSlice; + hooks: ExportHooks; + sink: ExportSink; + ch: ReturnType; + ctx: ChCtx; + tab: QueryTab; + params: ExportParamsDeps; +} + +function makeHarness(opts: { + state?: Partial; + hooks?: Partial; + sink?: Partial; + tab?: Partial; + params?: Partial; + canExport?: () => boolean; + canExportScript?: () => boolean; + ensureConfig?: () => Promise; + getToken?: () => Promise; + sessionParamsFor?: (tab: QueryTab, sqls: string[]) => Record; +} = {}): Harness { + const state = makeState(opts.state); + const hooks = makeHooks(opts.hooks); + const sink = makeSink(opts.sink); + const ch = makeCh(); + const tab: QueryTab = { ...newTabObj('t1'), ...opts.tab }; + const params = makeParams(opts.params); + const ctx: ChCtx = { + fetch: (undefined as unknown) as typeof fetch, origin: 'https://ch.example', + getToken: async () => null, refresh: async () => false, onSignedOut: vi.fn(), + }; + const uidSeq = { n: 0 }; + const deps: ExportServiceDeps = { + exportQuery: ch.exportQuery, runQuery: ch.runQuery, killQuery: ch.killQuery, + ctx: () => ctx, + ensureConfig: opts.ensureConfig || vi.fn(async () => undefined), + getToken: opts.getToken || vi.fn(async () => 'tok'), + sqlString, + now: () => { uidSeq.n += 10; return uidSeq.n; }, + wallNow: () => 1_700_000_000_000, + uid: (prefix: string) => `${prefix}${++uidSeq.n}`, + canExport: opts.canExport || vi.fn(() => true), + canExportScript: opts.canExportScript || vi.fn(() => true), + sink, + state, + activeTab: () => tab, + params, + sessionParamsFor: opts.sessionParamsFor || vi.fn(() => ({})), + hooks, + }; + return { deps, state, hooks, sink, ch, ctx, tab, params }; +} + +// ── exportEntry (dispatch) ────────────────────────────────────────────────── + +describe('createExportService: exportEntry (dispatch)', () => { + it('is a no-op when the active tab is not in SQL mode', async () => { + const h = makeHarness({ tab: { editorMode: 'spec' } }); + const service = createExportService(h.deps); + await service.exportEntry(); + expect(h.sink.pickFile).not.toHaveBeenCalled(); + expect(h.sink.pickDirectory).not.toHaveBeenCalled(); + }); + + it('is a no-op while an export is already running', async () => { + const h = makeHarness({ state: { exporting: signal(true) } }); + const service = createExportService(h.deps); + await service.exportEntry(); + expect(h.sink.pickFile).not.toHaveBeenCalled(); + }); + + it('is blocked (no picker) when the {name:Type} gate is blocked (#134)', async () => { + const h = makeHarness({ params: { varGateBlocked: vi.fn(() => true) } }); + const service = createExportService(h.deps); + await service.exportEntry(); + expect(h.sink.pickFile).not.toHaveBeenCalled(); + expect(h.deps.ensureConfig).not.toHaveBeenCalled(); + }); + + it('toasts "Nothing to export" for blank/whitespace-only SQL', async () => { + const h = makeHarness({ tab: { sqlDraft: ' ' } }); + const service = createExportService(h.deps); + await service.exportEntry(); + expect(h.hooks.toast).toHaveBeenCalledWith('Nothing to export'); + expect(h.sink.pickFile).not.toHaveBeenCalled(); + }); + + it('one statement -> the single-file picker; more than one -> the directory picker', async () => { + const h = makeHarness({ tab: { sqlDraft: 'SELECT 1' } }); + const service = createExportService(h.deps); + await service.exportEntry(); + expect(h.sink.pickFile).toHaveBeenCalledTimes(1); + expect(h.sink.pickDirectory).not.toHaveBeenCalled(); + + h.tab.sqlDraft = 'SELECT 1;\nSELECT 2;'; + await service.exportEntry(); + expect(h.sink.pickDirectory).toHaveBeenCalledTimes(1); + }); +}); + +// ── exportDirect (single-file, issue #87) ─────────────────────────────────── + +describe('createExportService: exportDirect (issue #87)', () => { + it('guards against non-SQL mode / already-running / canExport() false / empty input, all defensively', async () => { + const notSql = makeHarness({ tab: { editorMode: 'spec' } }); + await createExportService(notSql.deps).exportDirect('SELECT 1', 0); + expect(notSql.sink.pickFile).not.toHaveBeenCalled(); + + const busy = makeHarness({ state: { exporting: signal(true) } }); + await createExportService(busy.deps).exportDirect('SELECT 1', 0); + expect(busy.sink.pickFile).not.toHaveBeenCalled(); + + const unavailable = makeHarness({ canExport: () => false }); + await createExportService(unavailable.deps).exportDirect('SELECT 1', 0); + expect(unavailable.sink.pickFile).not.toHaveBeenCalled(); + + const empty = makeHarness(); + await createExportService(empty.deps).exportDirect(' ', 0); + expect(empty.hooks.toast).toHaveBeenCalledWith('Nothing to export'); + expect(empty.sink.pickFile).not.toHaveBeenCalled(); + }); + + it('picker AbortError (user dismissed the dialog) is a silent no-op', async () => { + const h = makeHarness({ sink: { pickFile: vi.fn(async () => { throw abortError(); }) } }); + await createExportService(h.deps).exportDirect('SELECT 1', 0); + expect(h.hooks.toast).not.toHaveBeenCalled(); + expect(h.state.exporting.value).toBe(false); + }); + + it('a non-abort picker failure toasts "Save dialog failed"', async () => { + const h = makeHarness({ sink: { pickFile: vi.fn(async () => { throw new Error('disk full'); }) } }); + await createExportService(h.deps).exportDirect('SELECT 1', 0); + expect(h.hooks.toast).toHaveBeenCalledWith('Save dialog failed: disk full'); + expect(h.state.exporting.value).toBe(false); + }); + + it('picker opens BEFORE ensureConfig/getToken (transient-activation ordering, review F6)', async () => { + const order: string[] = []; + const { handle } = fakeFileHandle(); + const h = makeHarness({ + sink: { pickFile: vi.fn(async () => { order.push('pickFile'); return asFileHandleLike(handle); }) }, + ensureConfig: vi.fn(async () => { order.push('ensureConfig'); }), + getToken: vi.fn(async () => { order.push('getToken'); return 'tok'; }), + }); + await createExportService(h.deps).exportDirect('SELECT 1', 0); + expect(order).toEqual(['pickFile', 'ensureConfig', 'getToken']); + }); + + it('signed out (no token): the picker still opens, but no query runs', async () => { + const { handle } = fakeFileHandle(); + const h = makeHarness({ sink: { pickFile: vi.fn(async () => asFileHandleLike(handle)) }, getToken: vi.fn(async () => null) }); + await createExportService(h.deps).exportDirect('SELECT 1', 0); + expect(h.sink.pickFile).toHaveBeenCalledTimes(1); + expect(h.ctx.onSignedOut).toHaveBeenCalledTimes(1); + expect(h.ch.exportQuery).not.toHaveBeenCalled(); + expect(h.state.exporting.value).toBe(false); + }); + + it('streams a clean result to disk (default TSV) and reports completion', async () => { + const { handle, writable, chunks } = fakeFileHandle(); + let pickerOpts: { suggestedName: string; types: { accept: Record }[] } | undefined; + const h = makeHarness({ + sink: { + pickFile: vi.fn(async (opts) => { pickerOpts = opts; return asFileHandleLike(handle); }), + }, + tab: { name: 'My Query!' }, + }); + h.ch.exportQuery.mockResolvedValue(asResponse(fakeExportResponse({ body: streamBody(['a'.repeat(100)]) }))); + await createExportService(h.deps).exportDirect('SELECT 1', 0); + expect(pickerOpts!.suggestedName).toBe('My_Query.tsv'); + expect(pickerOpts!.types[0].accept).toEqual({ 'text/tab-separated-values': ['.tsv'] }); + expect(writtenText(chunks)).toBe('a'.repeat(100)); + expect(writable.close).toHaveBeenCalledTimes(1); + expect(writable.abort).not.toHaveBeenCalled(); + expect(h.hooks.toast).toHaveBeenCalledWith('Export complete'); + expect(h.state.exporting.value).toBe(false); + const call = h.ch.exportQuery.mock.calls[0]; + expect(call[1]).toBe('SELECT 1\nFORMAT TabSeparatedWithNames'); + expect(call[2].format).toBe('TabSeparatedWithNames'); + }); + + it('honors an explicit FORMAT in the query for the picker + the request', async () => { + const { handle } = fakeFileHandle(); + let pickerOpts: { suggestedName: string; types: { accept: Record }[] } | undefined; + const h = makeHarness({ + sink: { pickFile: vi.fn(async (opts) => { pickerOpts = opts; return asFileHandleLike(handle); }) }, + params: { execStatementSql: vi.fn((s: string) => s) }, + }); + h.ch.exportQuery.mockResolvedValue(asResponse(fakeExportResponse({ body: streamBody(['[]']) }))); + await createExportService(h.deps).exportDirect('SELECT 1 FORMAT JSON', 0); + expect(pickerOpts!.suggestedName).toMatch(/\.json$/); + expect(pickerOpts!.types[0].accept).toEqual({ 'application/json': ['.json'] }); + const call = h.ch.exportQuery.mock.calls[0]; + expect(call[2].format).toBe('JSON'); + }); + + it('query variables (#134/#173): sends the wave-captured params merged with sessionParamsFor', async () => { + const { handle } = fakeFileHandle(); + const h = makeHarness({ + sink: { pickFile: vi.fn(async () => asFileHandleLike(handle)) }, + params: { + prepareTabSource: vi.fn(() => preparedSource({ statements: [preparedStatement({ args: { param_database: 'default' } })] })), + }, + sessionParamsFor: vi.fn(() => ({ session_id: 'sess-1' })), + }); + h.ch.exportQuery.mockResolvedValue(asResponse(fakeExportResponse({ body: streamBody(['x']) }))); + await createExportService(h.deps).exportDirect('SELECT {database:String}', 42); + expect(h.params.prepareTabSource).toHaveBeenCalledWith('SELECT {database:String}\nFORMAT TabSeparatedWithNames', 42); + const call = h.ch.exportQuery.mock.calls[0]; + expect(call[2].params).toEqual({ session_id: 'sess-1', param_database: 'default' }); + }); + + it('a pre-header (non-OK) export failure toasts "Export failed" without ever opening the writable', async () => { + const { handle } = fakeFileHandle(); + const h = makeHarness({ sink: { pickFile: vi.fn(async () => asFileHandleLike(handle)) } }); + h.ch.exportQuery.mockRejectedValue(new Error('DB::Exception: nope')); + await createExportService(h.deps).exportDirect('SELECT 1', 0); + expect(h.hooks.toast).toHaveBeenCalledWith('Export failed: DB::Exception: nope'); + expect(handle.createWritable).not.toHaveBeenCalled(); + expect(h.state.exporting.value).toBe(false); + }); + + it('suppresses the "Export failed" toast when the underlying error is "signed out"', async () => { + const { handle } = fakeFileHandle(); + const h = makeHarness({ sink: { pickFile: vi.fn(async () => asFileHandleLike(handle)) } }); + h.ch.exportQuery.mockRejectedValue(new Error('signed out')); + await createExportService(h.deps).exportDirect('SELECT 1', 0); + expect(h.hooks.toast).not.toHaveBeenCalled(); + expect(h.state.exporting.value).toBe(false); + }); + + it('holds back the trailing 32 KiB and streams the rest incrementally (no full buffering)', async () => { + const { handle, writable, chunks } = fakeFileHandle(); + const big = 'a'.repeat(40960); // > HOLDBACK (32 KiB) in a single chunk + const h = makeHarness({ sink: { pickFile: vi.fn(async () => asFileHandleLike(handle)) } }); + h.ch.exportQuery.mockResolvedValue(asResponse(fakeExportResponse({ body: streamBody([big]) }))); + await createExportService(h.deps).exportDirect('SELECT 1', 0); + // mid-loop commit (8192 = 40960 - 32768 HOLDBACK) then the EOF flush of the held-back tail. + expect((writable.write as Mock).mock.calls.map((c) => (c[0] as Uint8Array).length)).toEqual([8192, 32768]); + expect(writtenText(chunks)).toBe(big); + expect(writable.close).toHaveBeenCalledTimes(1); + }); + + it('excises a mid-stream exception frame — only clean bytes reach the file; reports "incomplete"', async () => { + const TAG = 'abcdef0123456789'; + const { handle, writable, chunks } = fakeFileHandle(); + const clean = 'x'.repeat(40); + const frame = exceptionFrame(TAG, 'DB::Exception: Memory limit (total) exceeded'); + const h = makeHarness({ sink: { pickFile: vi.fn(async () => asFileHandleLike(handle)) } }); + h.ch.exportQuery.mockResolvedValue(asResponse(fakeExportResponse({ body: streamBody([clean, frame]), headers: { 'X-ClickHouse-Exception-Tag': TAG } }))); + await createExportService(h.deps).exportDirect('SELECT 1', 0); + expect(writtenText(chunks)).toBe(clean); + expect(writable.close).toHaveBeenCalledTimes(1); + expect(writable.abort).not.toHaveBeenCalled(); + expect(h.hooks.toast).toHaveBeenCalledWith('Export incomplete — server error mid-stream: DB::Exception: Memory limit (total) exceeded'); + }); + + it('a stream read failure mid-export closes (not aborts) the writable and renames it .partial', async () => { + const { handle, writable } = fakeFileHandle('My_Query.tsv'); + let reads = 0; + const body: FakeBody = { + getReader: () => ({ + read: async () => { + reads += 1; + if (reads === 1) return { done: false, value: new TextEncoder().encode('partial') }; + throw new Error('network drop'); + }, + releaseLock: () => {}, + }), + }; + const h = makeHarness({ sink: { pickFile: vi.fn(async () => asFileHandleLike(handle)) } }); + h.ch.exportQuery.mockResolvedValue(asResponse(fakeExportResponse({ body }))); + await createExportService(h.deps).exportDirect('SELECT 1', 0); + expect(writable.abort).not.toHaveBeenCalled(); + expect(writable.close).toHaveBeenCalledTimes(1); + expect(handle.move).toHaveBeenCalledWith('My_Query.tsv.partial'); + expect(h.hooks.toast).toHaveBeenCalledWith('Export failed: network drop'); + expect(h.state.exporting.value).toBe(false); + }); + + it('falls back to leaving the plain (non-renamed) file when the handle has no move()', async () => { + const { handle, writable } = fakeFileHandle(); + delete handle.move; + const h = makeHarness({ sink: { pickFile: vi.fn(async () => asFileHandleLike(handle)) } }); + h.ch.exportQuery.mockResolvedValue(asResponse(fakeExportResponse({ body: throwingBody('network drop') }))); + await createExportService(h.deps).exportDirect('SELECT 1', 0); + expect(writable.abort).not.toHaveBeenCalled(); + expect(writable.close).toHaveBeenCalledTimes(1); + expect(h.hooks.toast).toHaveBeenCalledWith('Export failed: network drop'); + }); + + it('a failed move() (e.g. name collision) is swallowed — the plain file is still recoverable', async () => { + const { handle, writable } = fakeFileHandle(); + handle.move = vi.fn(async () => { throw new Error('collision'); }); + const h = makeHarness({ sink: { pickFile: vi.fn(async () => asFileHandleLike(handle)) } }); + h.ch.exportQuery.mockResolvedValue(asResponse(fakeExportResponse({ body: throwingBody('network drop') }))); + await createExportService(h.deps).exportDirect('SELECT 1', 0); + expect(writable.abort).not.toHaveBeenCalled(); + expect(handle.move).toHaveBeenCalledTimes(1); + expect(h.hooks.toast).toHaveBeenCalledWith('Export failed: network drop'); + }); + + it('exporting.value is true for the duration of the run; cancelExport aborts the signal + issues its own KILL QUERY', async () => { + const { handle } = fakeFileHandle(); + const pending = deferred(); + const h = makeHarness({ sink: { pickFile: vi.fn(async () => asFileHandleLike(handle)) } }); + h.ch.exportQuery.mockImplementation(async () => pending.promise); + const service = createExportService(h.deps); + const run = service.exportDirect('SELECT 1', 0); + await flush(); + expect(h.state.exporting.value).toBe(true); + const signalArg = h.ch.exportQuery.mock.calls[0][2].signal as AbortSignal; + expect(signalArg.aborted).toBe(false); + + service.cancelExport(); + expect(signalArg.aborted).toBe(true); + pending.reject(abortError()); + await run; + + expect(h.state.exporting.value).toBe(false); + expect(h.hooks.toast).not.toHaveBeenCalled(); // AbortError → silent + expect(h.ch.killQuery).toHaveBeenCalledWith(h.ctx, expect.stringMatching(/^export-/), sqlString); + }); + + it('a second click while the picker is still open is blocked (exporting flips true before the picker await)', async () => { + const pending = deferred(); + const h = makeHarness({ sink: { pickFile: vi.fn(() => pending.promise) } }); + const service = createExportService(h.deps); + const first = service.exportDirect('SELECT 1', 0); + await flush(); + expect(h.state.exporting.value).toBe(true); + await service.exportDirect('SELECT 1', 0); // second click: blocked by the re-entrance guard + expect(h.sink.pickFile).toHaveBeenCalledTimes(1); + pending.reject(abortError()); + await first; + expect(h.state.exporting.value).toBe(false); + }); + + it('shows + tears down the progress banner around the streamed request', async () => { + const { handle } = fakeFileHandle(); + const progress = { update: vi.fn(), remove: vi.fn() }; + const h = makeHarness({ + sink: { pickFile: vi.fn(async () => asFileHandleLike(handle)) }, + hooks: { showExportProgress: vi.fn(() => progress) }, + }); + h.ch.exportQuery.mockResolvedValue(asResponse(fakeExportResponse({ body: streamBody(['a'.repeat(50)]) }))); + await createExportService(h.deps).exportDirect('SELECT 1', 0); + expect(h.hooks.showExportProgress).toHaveBeenCalledTimes(1); + expect(progress.update).toHaveBeenCalled(); + expect(progress.remove).toHaveBeenCalledTimes(1); + }); +}); + +// ── exportScriptEntry / exportScript (issue #99) ──────────────────────────── + +describe('createExportService: exportScriptEntry / exportScript (issue #99)', () => { + it('canExportScript() gates the directory picker; a script with no result-producing statements toasts instead', async () => { + const unavailable = makeHarness({ canExportScript: () => false, tab: { sqlDraft: 'SELECT 1;\nSELECT 2;' } }); + await createExportService(unavailable.deps).exportEntry(); + expect(unavailable.hooks.toast).toHaveBeenCalledWith('Script export requires Chrome/Edge directory access over HTTPS'); + expect(unavailable.sink.pickDirectory).not.toHaveBeenCalled(); + expect(unavailable.state.exporting.value).toBe(false); + + const noRows = makeHarness({ tab: { sqlDraft: 'CREATE TABLE t (a Int8);\nINSERT INTO t VALUES (1);' } }); + await createExportService(noRows.deps).exportEntry(); + expect(noRows.sink.pickDirectory).not.toHaveBeenCalled(); + expect(noRows.hooks.toast).toHaveBeenCalledWith('Nothing to export — script has no result-producing statements.'); + }); + + it('dismissing the directory picker (AbortError) is a silent no-op', async () => { + const h = makeHarness({ sink: { pickDirectory: vi.fn(async () => { throw abortError(); }) }, tab: { sqlDraft: 'SELECT 1;\nSELECT 2;' } }); + await createExportService(h.deps).exportEntry(); + expect(h.hooks.toast).not.toHaveBeenCalled(); + expect(h.state.exporting.value).toBe(false); + }); + + it('a non-abort directory-picker failure toasts "Folder dialog failed"', async () => { + const h = makeHarness({ sink: { pickDirectory: vi.fn(async () => { throw new Error('denied'); }) }, tab: { sqlDraft: 'SELECT 1;\nSELECT 2;' } }); + await createExportService(h.deps).exportEntry(); + expect(h.hooks.toast).toHaveBeenCalledWith('Folder dialog failed: denied'); + }); + + it('the directory picker opens BEFORE ensureConfig/getToken; a signed-out tab never runs the script', async () => { + const { dir } = fakeDirHandle(); + const order: string[] = []; + const h = makeHarness({ + sink: { pickDirectory: vi.fn(async () => { order.push('pickDirectory'); return dir; }) }, + ensureConfig: vi.fn(async () => { order.push('ensureConfig'); }), + getToken: vi.fn(async () => { order.push('getToken'); return null; }), + tab: { sqlDraft: 'SELECT 1;\nSELECT 2;' }, + }); + await createExportService(h.deps).exportEntry(); + expect(order).toEqual(['pickDirectory', 'ensureConfig', 'getToken']); + expect(h.ctx.onSignedOut).toHaveBeenCalledTimes(1); + expect((dir.getFileHandle as Mock)).not.toHaveBeenCalled(); + expect(h.state.exporting.value).toBe(false); + }); + + it('a second click while the directory picker is still open is blocked', async () => { + const pending = deferred(); + const h = makeHarness({ sink: { pickDirectory: vi.fn(() => pending.promise) }, tab: { sqlDraft: 'SELECT 1;\nSELECT 2;' } }); + const service = createExportService(h.deps); + const first = service.exportEntry(); + await flush(); + expect(h.state.exporting.value).toBe(true); + await service.exportEntry(); + expect(h.sink.pickDirectory).toHaveBeenCalledTimes(1); + pending.reject(abortError()); + await first; + expect(h.state.exporting.value).toBe(false); + }); + + it('runs statements sequentially in one shared session; effect statements log ok with no file, rows stream to their own file', async () => { + const { dir, written } = fakeDirHandle(); + const SCRIPT = 'CREATE TEMPORARY TABLE t (a Int8);\nINSERT INTO t VALUES (1);\nSELECT * FROM t'; + const h = makeHarness({ + sink: { pickDirectory: vi.fn(async () => dir) }, + tab: { sqlDraft: SCRIPT }, + params: { + prepareTabSource: vi.fn(() => preparedSource({ + statements: [ + preparedStatement({ sql: 'CREATE TEMPORARY TABLE t (a Int8)' }), + preparedStatement({ sql: 'INSERT INTO t VALUES (1)' }), + preparedStatement({ sql: 'SELECT * FROM t' }), + ], + })), + execStatementSql: vi.fn((s: string) => s), + }, + sessionParamsFor: vi.fn(() => ({ session_id: 'sess-xyz' })), + }); + h.ch.exportQuery.mockResolvedValue(asResponse(fakeExportResponse({ body: streamBody(['1\n']) }))); + await createExportService(h.deps).exportEntry(); + + // Effect statements (non-'rows') go through runQuery with format TSV. + expect(h.ch.runQuery).toHaveBeenCalledTimes(2); + const runCalls = h.ch.runQuery.mock.calls; + expect(runCalls[0][1]).toBe('CREATE TEMPORARY TABLE t (a Int8)'); + expect(runCalls[1][1]).toBe('INSERT INTO t VALUES (1)'); + runCalls.forEach((c) => expect(c[2].params).toMatchObject({ session_id: 'sess-xyz' })); + // Row-returning statement streams via exportQuery, one file. + expect(h.ch.exportQuery).toHaveBeenCalledTimes(1); + expect((dir.getFileHandle as Mock)).toHaveBeenCalledTimes(1); + const [name] = (dir.getFileHandle as Mock).mock.calls[0]; + expect(name).toBe('003-t.tsv'); + expect(written.get('003-t.tsv')!.writable.close).toHaveBeenCalledTimes(1); + expect(h.state.exporting.value).toBe(false); + }); + + it('row-returning statements get distinct, deterministic file names', async () => { + const { dir } = fakeDirHandle(); + const h = makeHarness({ + sink: { pickDirectory: vi.fn(async () => dir) }, + tab: { sqlDraft: 'SELECT 1;\nSELECT 2;' }, + }); + h.ch.exportQuery + .mockResolvedValueOnce(asResponse(fakeExportResponse({ body: streamBody(['a']) }))) + .mockResolvedValueOnce(asResponse(fakeExportResponse({ body: streamBody(['b']) }))); + await createExportService(h.deps).exportEntry(); + const names = (dir.getFileHandle as Mock).mock.calls.map((c) => c[0]); + expect(names).toEqual(['001-select-1.tsv', '002-select-2.tsv']); + }); + + it('respects an explicit trailing FORMAT per statement', async () => { + const { dir } = fakeDirHandle(); + const h = makeHarness({ + sink: { pickDirectory: vi.fn(async () => dir) }, + tab: { sqlDraft: 'SELECT 1 FORMAT JSON;\nSELECT 2;' }, + params: { execStatementSql: vi.fn((s: string) => s) }, + }); + h.ch.exportQuery + .mockResolvedValueOnce(asResponse(fakeExportResponse({ body: streamBody(['[]']) }))) + .mockResolvedValueOnce(asResponse(fakeExportResponse({ body: streamBody(['x']) }))); + await createExportService(h.deps).exportEntry(); + const names = (dir.getFileHandle as Mock).mock.calls.map((c) => c[0]); + expect(names).toEqual(['001-select-1-format-json.json', '002-select-2.tsv']); + }); + + it('a non-row statement error marks it failed with no file and stops the script; the rest are skipped', async () => { + const { dir } = fakeDirHandle(); + const h = makeHarness({ + sink: { pickDirectory: vi.fn(async () => dir) }, + tab: { sqlDraft: 'CREATE TABLE bad;\nSELECT 1;' }, + }); + h.ch.runQuery.mockResolvedValue({ error: 'DB::Exception: table exists' }); + await createExportService(h.deps).exportEntry(); + expect((dir.getFileHandle as Mock)).not.toHaveBeenCalled(); + expect(h.hooks.loadSchema).not.toHaveBeenCalled(); + }); + + it('a pre-header (non-OK) export failure marks the row failed and stops; the rest are skipped', async () => { + const { dir } = fakeDirHandle(); + const h = makeHarness({ + sink: { pickDirectory: vi.fn(async () => dir) }, + tab: { sqlDraft: 'SELECT 1;\nSELECT 2;' }, + }); + h.ch.exportQuery.mockRejectedValue(new Error('DB::Exception: nope')); + await createExportService(h.deps).exportEntry(); + expect(h.ch.exportQuery).toHaveBeenCalledTimes(1); // stopped before statement 2 + }); + + it('a mid-stream exception marks the row failed/incomplete and stops the script', async () => { + const TAG = 'abcdef0123456789'; + const { dir } = fakeDirHandle(); + const clean = 'x'.repeat(10); + const frame = exceptionFrame(TAG, 'DB::Exception: Memory limit (total) exceeded'); + const h = makeHarness({ + sink: { pickDirectory: vi.fn(async () => dir) }, + tab: { sqlDraft: 'SELECT 1;\nSELECT 2;' }, + }); + h.ch.exportQuery.mockResolvedValue(asResponse(fakeExportResponse({ body: streamBody([clean, frame]), headers: { 'X-ClickHouse-Exception-Tag': TAG } }))); + await createExportService(h.deps).exportEntry(); + expect(h.ch.exportQuery).toHaveBeenCalledTimes(1); // stopped before statement 2 + }); + + it('never retries — a transient SESSION_IS_LOCKED failure is reported like any other error', async () => { + const { dir } = fakeDirHandle(); + const h = makeHarness({ + sink: { pickDirectory: vi.fn(async () => dir) }, + tab: { sqlDraft: 'INSERT INTO t VALUES (1);\nSELECT 1;' }, + }); + h.ch.runQuery.mockResolvedValue({ error: 'Code: 373. DB::Exception: SESSION_IS_LOCKED' }); + await createExportService(h.deps).exportEntry(); + expect(h.ch.runQuery).toHaveBeenCalledTimes(1); // no retry + expect(h.ch.exportQuery).not.toHaveBeenCalled(); // stopped before the SELECT + }); + + it('cancelExportScript aborts the active row, marks it cancelled, skips the rest, kills the active query, keeps completed files', async () => { + const { dir, written } = fakeDirHandle(); + const pending = deferred(); + const h = makeHarness({ + sink: { pickDirectory: vi.fn(async () => dir) }, + tab: { sqlDraft: 'SELECT 1;\nSELECT 2;\nSELECT 3;' }, + }); + h.ch.exportQuery + .mockResolvedValueOnce(asResponse(fakeExportResponse({ body: streamBody(['a']) }))) + .mockImplementationOnce(async () => pending.promise); + const service = createExportService(h.deps); + const run = service.exportEntry(); + await flush(); + await flush(); // let stmt1 finish and stmt2's request kick off + + service.cancelExportScript(); + pending.reject(abortError()); + await run; + + expect(written.get('001-select-1.tsv')!.writable.close).toHaveBeenCalledTimes(1); // completed file kept + expect(h.ch.killQuery).toHaveBeenCalledWith(h.ctx, expect.stringMatching(/^export-/), sqlString); + expect(h.state.exporting.value).toBe(false); + }); + + it('a cancel that arrives just after a statement completed cleanly still skips the remaining statements', async () => { + const { dir } = fakeDirHandle(); + const pending = deferred(); + const h = makeHarness({ + sink: { pickDirectory: vi.fn(async () => dir) }, + tab: { sqlDraft: 'CREATE TABLE t (a Int8);\nSELECT 1;' }, + }); + h.ch.runQuery.mockImplementationOnce(async () => pending.promise); + const service = createExportService(h.deps); + const run = service.exportEntry(); + await flush(); + service.cancelExportScript(); // cancel arrives while stmt1 is still in flight... + pending.resolve({}); // ...but the request completes cleanly anyway + await run; + expect(h.ch.exportQuery).not.toHaveBeenCalled(); // stmt2 was skipped, not run + }); + + it('refreshes the schema when an effect statement that actually ran is schema-mutating', async () => { + const { dir } = fakeDirHandle(); + const h = makeHarness({ + sink: { pickDirectory: vi.fn(async () => dir) }, + tab: { sqlDraft: 'CREATE TABLE t (a Int8);\nSELECT 1;' }, + }); + h.ch.exportQuery.mockResolvedValue(asResponse(fakeExportResponse({ body: streamBody(['x']) }))); + await createExportService(h.deps).exportEntry(); + expect(h.hooks.loadSchema).toHaveBeenCalledTimes(1); + }); + + it('does not refresh the schema when no statement that ran was schema-mutating', async () => { + const { dir } = fakeDirHandle(); + const h = makeHarness({ + sink: { pickDirectory: vi.fn(async () => dir) }, + tab: { sqlDraft: 'SELECT 1;\nSELECT 2;' }, + }); + h.ch.exportQuery + .mockResolvedValueOnce(asResponse(fakeExportResponse({ body: streamBody(['x']) }))) + .mockResolvedValueOnce(asResponse(fakeExportResponse({ body: streamBody(['y']) }))); + await createExportService(h.deps).exportEntry(); + expect(h.hooks.loadSchema).not.toHaveBeenCalled(); + }); + + it('repaints via hooks.renderResults on the interval tick + per statement', async () => { + const { dir } = fakeDirHandle(); + const h = makeHarness({ + sink: { pickDirectory: vi.fn(async () => dir) }, + tab: { sqlDraft: 'SELECT 1;\nSELECT 2;' }, + }); + h.ch.exportQuery + .mockResolvedValueOnce(asResponse(fakeExportResponse({ body: streamBody(['x']) }))) + .mockResolvedValueOnce(asResponse(fakeExportResponse({ body: streamBody(['y']) }))); + await createExportService(h.deps).exportEntry(); + expect((h.hooks.renderResults as Mock).mock.calls.length).toBeGreaterThan(0); + }); +}); From 797e396d47d6c2d356e9f63c083d2d8af71da449 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Fri, 17 Jul 2026 14:54:24 +0000 Subject: [PATCH 3/6] =?UTF-8?q?refactor(#276):=20extract=20QueryDocumentSe?= =?UTF-8?q?ssion=20+=20SavedQueryService=20=E2=80=94=20phase=204C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit src/application/query-document-session.ts (app.queryDoc) owns the Spec evaluation/document lifecycle over QueryTab objects: apply/evaluate draft, revalidate-all, reveal-first-error, validator registration, and the editor-mode policy gate — diagnostics typed with core/spec-draft's SpecValidationDiagnostic (never an editor import; check:arch enforces). src/application/saved-query-service.ts (app.saved) owns create/commit (validation-before-persist), history recording, and pure share-URL building — all returning discriminated-union results; the shell keeps the exact toast messages and the post-commit DOM cascade (which is a hand-repeated convention across ≥5 call sites, deliberately NOT service-owned per plan review). No service-to-service dependency: the shell sequences evaluate → commit. Prerequisite type-narrowing in state.ts (createSavedQuery/commitSavedQuery/patchSavedSpec/ tabsForSaved/invalidSpecTabForSaved/recordHistory → exact Picks, type-only). sessionParams/needsSession/sessionParamsFor untouched (Phase-5 decision). activateInvalidSpecDraft stays shell (>80% DOM). app.test.ts: zero edits (all 276 tests pass unmodified). Gate: 111 files / 3184 tests, build, check:arch green. New specs: 20 + 22 tests, both files 100/100/100/100. Part of #276 (Phase-4 combined PR, commit 3 of 4). Claude-Session: ad39bf19-1690-43c7-abb4-f0084fca00a2 Co-authored-by: Claude Fable 5 --- src/application/query-document-session.ts | 207 ++++++++++++++ src/application/saved-query-service.ts | 185 +++++++++++++ src/state.ts | 40 ++- src/ui/app.ts | 186 ++++++------- src/ui/app.types.ts | 23 +- tests/helpers/fake-app.ts | 43 ++- tests/unit/query-document-session.test.ts | 306 +++++++++++++++++++++ tests/unit/saved-query-service.test.ts | 317 ++++++++++++++++++++++ 8 files changed, 1203 insertions(+), 104 deletions(-) create mode 100644 src/application/query-document-session.ts create mode 100644 src/application/saved-query-service.ts create mode 100644 tests/unit/query-document-session.test.ts create mode 100644 tests/unit/saved-query-service.test.ts diff --git a/src/application/query-document-session.ts b/src/application/query-document-session.ts new file mode 100644 index 0000000..3e267d1 --- /dev/null +++ b/src/application/query-document-session.ts @@ -0,0 +1,207 @@ +// #276 Phase 4C's QueryDocumentSession — the Spec-evaluation/document +// lifecycle extracted from app.ts (issue #276 §8, plan-review rulings): +// applySpecEvaluation/evaluateSpecDraft/revalidateSpecDrafts/ +// revealFirstSpecError/registerSpecValidator, plus the editor-mode POLICY +// half of `setEditorMode` (`resolveEditorMode` below — whether a mode switch +// is allowed, and why not; the DOM/focus toggles that use the answer stay in +// app.ts's own `setEditorMode`). Constructible without App/AppState/DOM, like +// `workbench-parameter-session.ts`/`schema-catalog-service.ts` before it. +// +// Deliberately NOT included (plan-review ruling): `sessionParams`/ +// `needsSession`/`sessionParamsFor` stay app.ts-local (Phase 5 decides their +// home) — they're `tab.chSession`/transport material, unrelated to Spec +// evaluation. `activateInvalidSpecDraft` (>80% shell: toast/batch/focus) +// stays app.ts too. +// +// Diagnostics are typed as core/spec-draft.js's own `SpecValidationDiagnostic` +// — NOT `src/editor/spec-editor.types.js`'s `SpecDiagnostic` — so this module +// never imports `src/editor/**` (check:arch enforces `src/application/**` +// never importing `src/ui/**`/`src/editor/**`). `SpecValidationDiagnostic` is +// a strict superset of every field `SpecDiagnostic` optionally declares (see +// its own doc comment in core/spec-draft.ts), so it is directly assignable +// wherever a real `SpecDiagnostic` is expected — app.ts's own +// `hooks.setDiagnostics`/`hooks.revealDiagnostic` wiring passes these +// straight to `app.specEditor.setDiagnostics`/`.revealDiagnostic` without a +// cast. +// +// Every DOM/editor touch this session used to perform directly (repainting +// the Spec editor's diagnostics gutter, revealing a diagnostic, repainting +// tabs, the Save button, the editor-mode chrome) is now a `hooks` callback — +// app.ts supplies real closures over `app.specEditor`/`app.actions`/ +// `app.updateSaveBtn`/`app.updateEditorModeUi` (some of those app.ts members +// are only assigned later in `createApp`, so app.ts's own hook closures keep +// the exact same `if (app.actions) …`-style guards the pre-extraction inline +// code used — this session calls every hook unconditionally, unaware of that +// construction-order concern). + +import { evaluateSpecText } from '../core/spec-draft.js'; +import type { SpecValidationDiagnostic, SpecValidatorFn, QuerySpecValidationService } from '../core/spec-draft.js'; +import { savedForTab } from '../state.js'; +import type { QueryTab, AppState } from '../state.js'; + +// ── Construction deps ──────────────────────────────────────────────────────── + +export interface QueryDocumentSessionHooks { + /** Repaint the Spec editor's diagnostics gutter for the tab this call cares + * about — `evaluateSpecDraft`/`revalidateSpecDrafts` only ever call this + * for the CURRENTLY active tab (mirrors the pre-extraction inline code: + * `applySpecEvaluation` alone never repaints anything). */ + setDiagnostics(diagnostics: SpecValidationDiagnostic[]): void; + /** Jump the Spec editor's caret/scroll to diagnostic `index` (of the active + * tab's current `specDiagnostics`). */ + revealDiagnostic(index: number): void; + rerenderTabs(): void; + updateSaveBtn(): void; + updateEditorModeUi(): void; +} + +/** Live accessors — read fresh on every call, mirroring every other #276 + * session's own `*SessionDeps` convention (never a snapshot). `state` is + * narrowed to exactly the fields this session reads: `tabs` (iterating every + * open document in `revalidateSpecDrafts`) and `savedQueries` + * (`resolveEditorMode`'s `savedForTab` check) — production passes `app.state` + * itself, which satisfies this directly. */ +export interface QueryDocumentSessionDeps { + state: Pick; + activeTab(): QueryTab; + /** The app-owned Spec validation service (core/spec-draft.js) — canonical + * schema validation plus registered feature rules, with `.register` for + * `registerSpecValidator`. Production passes `app.specValidators`. */ + specValidators: QuerySpecValidationService; + hooks: QueryDocumentSessionHooks; +} + +/** One Spec-evaluation outcome: the parsed draft (`null` when the textual + * Spec isn't valid JSON — see `QueryTab.specParsed`'s own invariant) plus + * every diagnostic the canonical schema + registered validators produced. */ +export interface SpecEvaluationResult { + parsed: unknown; + diagnostics: SpecValidationDiagnostic[]; +} + +/** `resolveEditorMode`'s verdict — `message` is present only when there's a + * user-facing reason to explain the block (the "spec mode needs a saved + * query" case); the defensive not-sql-not-spec case carries none, matching + * the pre-extraction inline code's own silent early return there. */ +export interface EditorModeGate { + ok: boolean; + message?: string; +} + +// ── The session ────────────────────────────────────────────────────────────── + +export interface QueryDocumentSession { + /** Parse + validate `text` against `specValidators` and write the result + * onto `tab` (`specText`/`specParsed`/`specDiagnostics`/`dirtySpec`) — no + * DOM/hooks side effect. The low-level op every other method builds on; + * app.ts's own `sqlEditor.onDocChange` filter-role gate calls this + * directly (not `evaluateSpecDraft`) so it can pair it with its own, + * different, tail of UI updates. */ + applySpecEvaluation(tab: QueryTab, text: string, opts?: { dirty?: boolean }): SpecEvaluationResult; + /** `applySpecEvaluation` plus the full UI-refresh tail: repaints the Spec + * editor's diagnostics ONLY when `tab` is the currently active one, then + * unconditionally repaints tabs / the Save button / the editor-mode + * chrome. */ + evaluateSpecDraft(tab: QueryTab, text: string, opts?: { dirty?: boolean }): SpecEvaluationResult; + /** Re-run `applySpecEvaluation` for every open tab (e.g. after a validator + * registers/unregisters, or a Save that could change what "blocking" + * means elsewhere). `refreshUi: false` skips the repaint tail entirely + * (still re-evaluates every tab's diagnostics). */ + revalidateSpecDrafts(opts?: { refreshUi?: boolean }): void; + /** Jump to the first error-severity diagnostic on `tab` (defaults to the + * active tab) via `hooks.revealDiagnostic`; a no-op if there is none. */ + revealFirstSpecError(tab?: QueryTab): void; + /** Register a validator at `path`; every open Spec draft is revalidated + * both on register and on the returned unregister. */ + registerSpecValidator(path: (string | number)[], validate: SpecValidatorFn): () => void; + /** The editor-mode POLICY half of app.ts's `setEditorMode`: whether + * switching `tab` into `mode` is allowed right now. Entering 'spec' mode + * requires `tab` to already be linked to a saved query (an unsaved tab has + * no persisted Spec to edit as JSON) — the `message` explains that case to + * the user. app.ts's own `setEditorMode` performs the DOM/focus side of a + * successful switch (assigning `tab.editorMode`, repainting the editor-mode + * chrome, focusing the target editor). */ + resolveEditorMode(tab: QueryTab, mode: 'sql' | 'spec'): EditorModeGate; +} + +/** Build a `QueryDocumentSession` bound to `deps`. Trivial constructor — no + * validation, no defaulting; the caller supplies every field exactly as it + * wants it used. */ +export function createQueryDocumentSession(deps: QueryDocumentSessionDeps): QueryDocumentSession { + function applySpecEvaluation( + tab: QueryTab, text: string, { dirty = true }: { dirty?: boolean } = {}, + ): SpecEvaluationResult { + const evaluated = evaluateSpecText(text, deps.specValidators, { sql: tab.sqlDraft, tab }); + tab.specText = text; + tab.specParsed = evaluated.parsed as QueryTab['specParsed']; + tab.specDiagnostics = evaluated.diagnostics; + tab.dirtySpec = dirty; + return evaluated; + } + + function evaluateSpecDraft( + tab: QueryTab, text: string, { dirty = true }: { dirty?: boolean } = {}, + ): SpecEvaluationResult { + const evaluated = applySpecEvaluation(tab, text, { dirty }); + // `evaluated.diagnostics` (not `tab.specDiagnostics`, though + // `applySpecEvaluation` just assigned the very same array onto it) — the + // tab's own field is declared `SpecDiagnostic[]` (state.ts, the editor's + // looser app-wide contract), which a real `SpecValidationDiagnostic[]` + // assigns INTO fine but doesn't read back OUT of without narrowing; using + // the freshly-returned value sidesteps that one-directional relationship + // entirely instead of asserting it back down. + if (tab === deps.activeTab()) deps.hooks.setDiagnostics(evaluated.diagnostics); + deps.hooks.rerenderTabs(); + deps.hooks.updateSaveBtn(); + deps.hooks.updateEditorModeUi(); + return evaluated; + } + + function revalidateSpecDrafts({ refreshUi = true }: { refreshUi?: boolean } = {}): void { + for (const tab of deps.state.tabs.value) { + applySpecEvaluation(tab, tab.specText, { dirty: tab.dirtySpec }); + } + if (!refreshUi) return; + const tab = deps.activeTab(); + // `tab.specDiagnostics` is declared `SpecDiagnostic[]` (state.ts, the + // editor's looser app-wide contract) but was just written, in the loop + // above, by `applySpecEvaluation` — this module's only writer of that + // field — as a real `SpecValidationDiagnostic[]`. The one-directional + // relationship (see this module's header comment) makes this a safe, + // narrowing-only assertion, not a new runtime assumption; unconditional, + // matching the pre-extraction inline code exactly (it never guarded on + // whether the active tab happened to be freshly re-evaluated either). + deps.hooks.setDiagnostics(tab.specDiagnostics as SpecValidationDiagnostic[]); + deps.hooks.rerenderTabs(); + deps.hooks.updateSaveBtn(); + deps.hooks.updateEditorModeUi(); + } + + function revealFirstSpecError(tab: QueryTab = deps.activeTab()): void { + const index = tab.specDiagnostics?.findIndex((diagnostic) => diagnostic.severity === 'error') ?? -1; + if (index >= 0) deps.hooks.revealDiagnostic(index); + } + + function registerSpecValidator(path: (string | number)[], validate: SpecValidatorFn): () => void { + const unregister = deps.specValidators.register(path, validate); + revalidateSpecDrafts(); + return () => { unregister(); revalidateSpecDrafts(); }; + } + + function resolveEditorMode(tab: QueryTab, mode: 'sql' | 'spec'): EditorModeGate { + if (mode === 'spec' && !savedForTab(deps.state, tab)) { + return { ok: false, message: 'Save this query to create an editable Spec.' }; + } + if (mode !== 'sql' && mode !== 'spec') return { ok: false }; + return { ok: true }; + } + + return { + applySpecEvaluation, + evaluateSpecDraft, + revalidateSpecDrafts, + revealFirstSpecError, + registerSpecValidator, + resolveEditorMode, + }; +} diff --git a/src/application/saved-query-service.ts b/src/application/saved-query-service.ts new file mode 100644 index 0000000..35559da --- /dev/null +++ b/src/application/saved-query-service.ts @@ -0,0 +1,185 @@ +// #276 Phase 4C's SavedQueryService — the saved-query create/commit policy, +// history recording, and share-URL building extracted from app.ts (issue +// #276 §8, plan-review rulings): wraps state.ts's own `createSavedQuery`/ +// `commitSavedQuery`/`recordHistory` (already narrowed to exactly the +// `AppState` fields each reads — see state.ts) plus `core/share.js`'s +// `encodeShare`. Constructible without App/AppState/DOM, like +// `query-document-session.ts` alongside it. +// +// Deliberately NOT included (plan-review rulings): +// - This service never calls `QueryDocumentSession` — `commit`/ +// `buildShareUrl` both take an ALREADY-EVALUATED Spec (`{parsed, +// diagnostics}`, the exact shape `evaluateSpecText`/ +// `QueryDocumentSession.evaluateSpecDraft` produce) as an input +// parameter. The SHELL (app.ts) sequences evaluate → commit/share itself; +// no service-to-service dependency exists in either direction. +// - The post-commit cascade (`revalidateSpecDrafts` → +// `specEditor.syncFromState` → `updateSaveBtn` → `rerenderTabs` → +// `renderSavedHistory` → [`renderResults`] → `updateEditorModeUi`) stays a +// SHELL convention — app.ts's `commitLinkedQuery`/`openSavePopover`'s +// commit closure keep their exact call sequences, driven off this +// service's typed result. This service never imports `ui/results.js`, +// `ui/saved-history.js`, or any editor port. +// - Clipboard/location writes stay shell — `buildShareUrl` only builds the +// URL string; app.ts's own `share()` performs `history.replaceState` and +// the clipboard write off the returned value. +// - `savedForTab`/`tabPanel`-based gates that don't touch persistence +// (`saveActiveQuery`'s linked-tab check, `openSavePopover`'s own +// "Nothing to save" pre-check before a popover even opens) stay app.ts — +// they never call into this service. + +import { + createSavedQuery, commitSavedQuery, recordHistory as stateRecordHistory, +} from '../state.js'; +import type { + QueryTab, AppState, SaveJSON, SpecValidationService, HistoryResultSnapshot, QuerySpecDraft, +} from '../state.js'; +import { hasBlockingSpecErrors } from '../core/spec-draft.js'; +import type { SpecValidationDiagnostic } from '../core/spec-draft.js'; +import { queryPanel, withQuerySpec } from '../core/saved-query.js'; +import { isQuerylessPanel } from '../core/panel-cfg.js'; +import { encodeShare } from '../core/share.js'; +import type { SavedQueryV2 } from '../generated/json-schema.types.js'; + +// ── Construction deps ──────────────────────────────────────────────────────── + +/** Live accessors onto `app.state` — a structural `Pick`, not a snapshot: + * production passes `app.state` itself (satisfies this directly), mirroring + * every other #276 session's own deps convention. Exactly the fields + * `createSavedQuery`/`commitSavedQuery`/`recordHistory` (state.ts, all three + * narrowed to their own exact reads) need between them. */ +export interface SavedQueryServiceDeps { + state: Pick; + saveJSON: SaveJSON; + /** `createSavedQuery`'s minting timestamp — a genuine wall-clock read + * (production wires this to `() => Date.now()`, called at the exact + * moment a query is created), NOT app.ts's own `now`/`wallNow` seams + * (unrelated clocks — see app.ts's own construction comments): this keeps + * `createSavedQuery`'s minted ids byte-identical to the pre-extraction + * inline `Date.now()` call while still giving tests an injectable seam. */ + now(): number; + /** The app-owned Spec validation service (core/spec-draft.js's `.validate` + * surface — state.ts's own narrower `SpecValidationService`, not the + * fuller `QuerySpecValidationService` `QueryDocumentSession` needs; + * `createSavedQuery`/`commitSavedQuery` only ever call `.validate`). + * Production passes `app.specValidators` (structurally assignable). */ + specValidators: SpecValidationService; +} + +// ── Result types ───────────────────────────────────────────────────────────── + +export type CreateSavedResult = + | { ok: true; entry: SavedQueryV2 } + /** `createSavedQuery` itself returned null (already-linked tab, blank SQL + * on a non-text panel, blank name, or a blocking validation diagnostic) — + * the pre-extraction inline code never distinguished a reason here either + * (silent no-op), so neither does this result. */ + | { ok: false }; + +export type CommitLinkedResult = + | { ok: true; entry: SavedQueryV2 } + | { + ok: false; + reason: + /** The evaluated Spec has no parsed draft, or a blocking diagnostic. */ + | 'invalid-spec' + /** Blank SQL on a panel type that isn't SQL-optional (#166's text panel). */ + | 'empty' + /** `commitSavedQuery` itself returned null (tab no longer linked, or its + * own re-validation against the normalized Spec rejected it) — a + * defensive case the pre-extraction inline code never toasted either. */ + | 'rejected'; + }; + +export type ShareResult = + | { ok: true; url: string } + | { ok: false; reason: 'invalid-spec' | 'empty' }; + +/** `buildShareUrl`'s input: the exact fields it reads off the tab, the + * already-evaluated Spec (see this module's header comment on why this + * service never evaluates one itself), and the three `location` parts the + * encoded fragment is appended to (app.ts supplies `loc.origin`/ + * `loc.pathname`/`loc.search` — this service never touches `location` + * itself, matching the "clipboard/location writes stay shell" ruling). */ +export interface ShareUrlInput { + tab: Pick; + evaluated: { parsed: unknown; diagnostics: SpecValidationDiagnostic[] }; + origin: string; + pathname: string; + search: string; +} + +// ── The service ────────────────────────────────────────────────────────────── + +export interface SavedQueryService { + /** Creation-only path (app.ts's Save-popover commit): mint a brand-new + * saved query from an unsaved tab's current `sqlDraft`/`specParsed` plus + * `name`/`description`. Rejects (silently — see `CreateSavedResult`) an + * already-linked tab, per `createSavedQuery`'s own guard. */ + create(tab: QueryTab, name: unknown, description: unknown): CreateSavedResult; + /** Update-in-place path (app.ts's `commitLinkedQuery`, the "Save" button on + * an already-linked tab): persist `evaluated` as the linked saved query's + * new Spec. Takes the Spec evaluation as an input (see this module's + * header comment) rather than evaluating it itself. */ + commit(tab: QueryTab, evaluated: { parsed: unknown; diagnostics: SpecValidationDiagnostic[] }): CommitLinkedResult; + /** Record a successful run in history (state.ts's own `recordHistory`) — + * never touches rendering; app.ts's own `app.recordHistory` delegate + * conditionally repaints the History side panel itself after calling + * this. */ + recordHistory(tab: QueryTab, sqlText?: string): void; + /** Build the shareable URL for an already-evaluated Spec, or a typed + * rejection reason — never writes `location`/clipboard itself. */ + buildShareUrl(input: ShareUrlInput): ShareResult; +} + +/** Build a `SavedQueryService` bound to `deps`. Trivial constructor — no + * validation, no defaulting; the caller supplies every field exactly as it + * wants it used. */ +export function createSavedQueryService(deps: SavedQueryServiceDeps): SavedQueryService { + function create(tab: QueryTab, name: unknown, description: unknown): CreateSavedResult { + const entry = createSavedQuery(deps.state, tab, name, description, deps.saveJSON, deps.now(), deps.specValidators); + return entry ? { ok: true, entry } : { ok: false }; + } + + function commit( + tab: QueryTab, evaluated: { parsed: unknown; diagnostics: SpecValidationDiagnostic[] }, + ): CommitLinkedResult { + if (!evaluated.parsed || hasBlockingSpecErrors(evaluated.diagnostics)) { + return { ok: false, reason: 'invalid-spec' }; + } + const panel = queryPanel({ spec: evaluated.parsed }); + if (!String(tab.sqlDraft || '').trim() && !isQuerylessPanel(panel)) { + return { ok: false, reason: 'empty' }; + } + const entry = commitSavedQuery(deps.state, tab, evaluated.parsed as QuerySpecDraft | null, deps.saveJSON, deps.specValidators); + return entry ? { ok: true, entry } : { ok: false, reason: 'rejected' }; + } + + function recordHistoryFn(tab: QueryTab, sqlText?: string): void { + // `tab.result` is state.ts's deliberately opaque `Record | + // null` — by the time this is ever called (only after a successful run), + // it already holds a real `QueryResult`-shaped value (rawText/rows/ + // progress.elapsed_ns), the exact fields `HistoryResultSnapshot` pins. + const result = (tab.result as HistoryResultSnapshot | null)!; + // `now` deliberately omitted (`undefined`) — matches the pre-extraction + // inline call exactly: `recordHistory`'s own default (`Date.now()`) mints + // the entry's timestamp, independent of this service's own injected + // `deps.now` (which only feeds `createSavedQuery`'s id). + stateRecordHistory(deps.state, { sqlDraft: tab.sqlDraft, result }, deps.saveJSON, undefined, sqlText); + } + + function buildShareUrl(input: ShareUrlInput): ShareResult { + const { tab, evaluated } = input; + if (!evaluated.parsed || hasBlockingSpecErrors(evaluated.diagnostics)) { + return { ok: false, reason: 'invalid-spec' }; + } + const sql = String(tab.sqlDraft || ''); + const panel = queryPanel({ spec: evaluated.parsed }); + if (!sql.trim() && !isQuerylessPanel(panel)) return { ok: false, reason: 'empty' }; + const query = withQuerySpec({ id: tab.savedId, sql }, evaluated.parsed); + const url = input.origin + input.pathname + input.search + '#' + encodeShare(query); + return { ok: true, url }; + } + + return { create, commit, recordHistory: recordHistoryFn, buildShareUrl }; +} diff --git a/src/state.ts b/src/state.ts index af09ddb..ddab8e9 100644 --- a/src/state.ts +++ b/src/state.ts @@ -526,11 +526,17 @@ export function allocTabId(state: AppState): string { const rnd = () => Math.random().toString(36).slice(2, 6); const makeId = (prefix: string, now: number) => prefix + now + rnd(); -export const tabsForSaved = (state: AppState, id: string): QueryTab[] => +// Narrowed to `Pick` (#276 Phase 4C) — the only field read +// — so `patchSavedSpec`'s own narrowed `state` param (below) can pass it +// through unchanged; every real caller already passes a full `AppState`, +// which satisfies this directly. +export const tabsForSaved = (state: Pick, id: string): QueryTab[] => state.tabs.value.filter((t) => t.savedId === id); -/** First linked tab whose textual Spec is not currently parseable JSON. */ -export const invalidSpecTabForSaved = (state: AppState, id: string): QueryTab | null => +/** First linked tab whose textual Spec is not currently parseable JSON. + * Narrowed to `Pick` for the same reason as + * `tabsForSaved` above (which this delegates to). */ +export const invalidSpecTabForSaved = (state: Pick, id: string): QueryTab | null => tabsForSaved(state, id).find((tab) => tab.specDiagnostics?.some((diagnostic) => diagnostic.code === 'invalid-json')) || null; @@ -575,9 +581,15 @@ export function savedForTab( /** * Create a saved query from an unsaved tab. Linked tabs use commitSavedQuery() * instead, so popover metadata can never compete with the textual Spec draft. + * Narrowed to `Pick` + * (#276 Phase 4C — the exact fields read/written) instead of full `AppState`, + * same convention as `savedForTab`/`recordScriptHistory` — every real caller + * (app.ts's own `SavedQueryService`) already has a full `AppState` to pass, + * which satisfies this directly. */ export function createSavedQuery( - state: AppState, tab: QueryTab | null | undefined, name: unknown, description?: unknown, + state: Pick, + tab: QueryTab | null | undefined, name: unknown, description?: unknown, save: SaveJSON = saveJSON, now: number = Date.now(), validationService: SpecValidationService = defaultSpecValidationService, ): SavedQueryV2 | null { @@ -617,9 +629,11 @@ export function createSavedQuery( return entry; } -/** Atomically persist both documents of a linked tab in one Library write. */ +/** Atomically persist both documents of a linked tab in one Library write. + * Narrowed to `Pick` (#276 Phase + * 4C), same convention as `createSavedQuery` above. */ export function commitSavedQuery( - state: AppState, tab: QueryTab, spec: QuerySpecDraft | null | undefined, + state: Pick, tab: QueryTab, spec: QuerySpecDraft | null | undefined, save: SaveJSON = saveJSON, validationService: SpecValidationService = defaultSpecValidationService, ): SavedQueryV2 | null { @@ -647,9 +661,14 @@ export function commitSavedQuery( * Generic committed-Spec writer for pencil/star/future controls. The patch is * applied independently to the persisted entry and every linked valid draft, * preserving unrelated unsaved fields. Invalid JSON blocks the whole write. + * Narrowed to `Pick` + * (#276 Phase 4C — `tabs` via `invalidSpecTabForSaved`/`tabsForSaved`, both + * narrowed the same way above), same convention as `createSavedQuery`/ + * `commitSavedQuery`. `renameSaved`/`toggleFavorite` below keep passing a + * full `AppState` through unchanged (it satisfies this directly). */ export function patchSavedSpec( - state: AppState, id: string, patch: SpecPatch, + state: Pick, id: string, patch: SpecPatch, save: SaveJSON = saveJSON, validationService: SpecValidationService = defaultSpecValidationService, ): PatchSavedResult { @@ -877,9 +896,14 @@ export interface HistoryResultSnapshot { /** * Record a successful run in history. `sqlText` overrides the recorded SQL (used * when a selection — not the whole tab — was run); it defaults to `tab.sqlDraft`. + * Narrowed to `Pick` (#276 Phase 4C) — the only field + * read/written (via `pushHistory`, already narrowed this way) — same + * convention as `recordScriptHistory`; app.ts's own `SavedQueryService` + * passes a `Pick` that also carries `savedQueries`/`resultView`/ + * `libraryDirty` for its other methods, which satisfies this directly. */ export function recordHistory( - state: AppState, + state: Pick, tab: { sqlDraft: string | null; result: HistoryResultSnapshot }, save: SaveJSON = saveJSON, now: number = Date.now(), sqlText?: string | null, ): void { diff --git a/src/ui/app.ts b/src/ui/app.ts index b87bbc3..178deb9 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -7,11 +7,11 @@ import { h, fixedAnchor } from './dom.js'; import { Icon } from './icons.js'; import { - createState, activeTab, KEYS, recordHistory, recordScriptHistory, - createSavedQuery, commitSavedQuery, savedForTab, tabPanel, + createState, activeTab, KEYS, recordScriptHistory, + savedForTab, tabPanel, normalizeRowLimit, MOBILE_BREAKPOINT_PX, } from '../state.js'; -import type { QueryTab, AppState, SpecValidationService, HistoryResultSnapshot, QuerySpecDraft } from '../state.js'; +import type { QueryTab, AppState, SpecValidationService } from '../state.js'; import type { SavedQueryV2 } from '../generated/json-schema.types.js'; import { splitStatements, leadingKeyword } from '../core/sql-split.js'; import { mergedSourceSql, analysisView, fieldControls, fieldControlKind } from '../core/param-pipeline.js'; @@ -23,11 +23,10 @@ import { buildCardGraph } from '../core/schema-cards.js'; import type { SchemaCardColumnRow } from '../core/schema-cards.js'; import { toTSV } from '../core/export.js'; import { newResult, parseErrorPos } from '../core/stream.js'; -import { encodeShare } from '../core/share.js'; -import { queryName, queryPanel, withQuerySpec } from '../core/saved-query.js'; +import { queryName } from '../core/saved-query.js'; import { effectiveDashboardRole } from '../core/result-choice.js'; import { - CORE_SPEC_VALIDATORS, createSpecValidatorRegistry, evaluateSpecText, formatSpecText, + CORE_SPEC_VALIDATORS, createSpecValidatorRegistry, formatSpecText, hasBlockingSpecErrors, } from '../core/spec-draft.js'; import type { SpecValidatorEntry, QuerySpecValidationService } from '../core/spec-draft.js'; @@ -81,6 +80,8 @@ import { createWorkbenchParameterSession } from '../application/workbench-parame import { createExportService } from '../application/export-service.js'; import type { ExportSink, FileHandleLike, DirectoryHandleLike } from '../application/export-service.js'; import { createWorkbenchSession } from './workbench/workbench-session.js'; +import { createQueryDocumentSession } from '../application/query-document-session.js'; +import { createSavedQueryService } from '../application/saved-query-service.js'; /** Optional globals a plain browser page (or the CM6/Chart/dagre UMD bundles a * `