ADR-0002 complete: convert the remaining 87 modules + tests to strict TypeScript - #271
Merged
Merged
Conversation
All tier-1 leaves: core utils (cell, dot, dot-layout, explain, graph-layout, panzoom, quoted-span, sort, target, jwt, pkce), core data/params (field-config, filter-options, param-serialize, recent-values, relative-time, share, storage, stream, json-schema-validation, library-migrations, markdown-lite), spec/ editor/net leaves (spec-completion, sql-reference, auth-handoff, codemirror-base, editor-port, spec-json-context, oauth-config, oauth), and ui leaves (combobox, combo-footer, dnd-mime, icons, shortcuts, toast, var-field) — plus their tests. Exported types match the wrapper consts earlier conversions pinned, so those keep compiling until their cleanup. App.runReadInto and dashboard.ts now consume the real StreamResult instead of local aliases. One genuine coverage gap surfaced and closed (spec-completion rank tie-break was never exercised; v8 attribution on the .js had masked it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XP1KN94N969SVMNFPdVtGg
spec/sql core (sql-spans, sql-lex, sql-split, spec-migrations, spec-schema
with its recursive JsonSchemaNode + service contracts), the editor stack
(code-viewer, spec-completion-adapter, spec-editor against their existing
.types.ts ports), eight ui field modules (detached-view, filter-option-field,
enum-field, recent-field, placeholder, login, tabs, relative-time-field), and
the type/codec batch (clickhouse-type's real TypeNode AST union, library-codec,
spec-draft, filter-bar, filter-preview) — plus tests. QueryTab.
lastSuccessfulResultColumns now declares the real {name, type} descriptor
shape app.js assigns (was a string[] placeholder lie).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XP1KN94N969SVMNFPdVtGg
core: format, from-scope, schema-graph, param-comparison, kpi (readKpiFields now types its row param as the real unknown[] | Record union, closing the keyed-row gap), saved-io, export, query-source, script-result, type-display, chart-data (full axis/role/pivot contracts + ChartJs config result types). net: ch-client with the injected-fetch ChCtx contract and generic ChJsonResult<T> row typing per query helper. ui: chart-render (injected Chart.js seam typed as a narrow structural constructor), grid-render, splitters, saved-history, kpi-panel — plus all their tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XP1KN94N969SVMNFPdVtGg
core/schema-cards, core/logs + ui/logs (shared spec), core/completions (CompletionContext/CompletionItem contracts the CM6 adapter consumes), ui/schema (narrow SchemaApp interface), ui/file-menu, ui/schema-detail (null-tolerant app per its deliberate contract), and editor/ codemirror-adapter (implements EditorPort against real CM6 .d.ts; the info/hover closures keep their casts inside the deferred callbacks since app.refData can change between mount and invocation) — plus all tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XP1KN94N969SVMNFPdVtGg
explain-graph.ts types the dagre injected seam (DagreModule, mirroring dot-layout.ts), the pipeline/schema lineage graph shapes, and the two partial app contracts its callers actually satisfy; branch coverage verified 1:1 against the .js original (same 11 uncovered branches, zero new). results.ts now owns the canonical Result contract the workbench passes around: Result = ScriptResult | ScriptExportResult | QueryResult (StreamResult + post-run explainView/schemaGraph/source/colWidths/panelState), with ScriptEntry as a status-discriminated union. ResultsApp stays narrower than App (panels.test drives renderResults with a minimal fixture); the three registry calls that genuinely need the full App cast at that one seam. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XP1KN94N969SVMNFPdVtGg
…cript createApp's return value is declared against the App contract; app.types.ts is reconciled with reality along the way (SchemaFocus gains kind and optional db, loadIdps/openNodeDetail/activateInvalidSpecDraft signatures corrected, AppDom documents the full internal surface). The conversion surfaced and fixed two dropped-return regressions from an interrupted first pass (exportEntry and renderDashboard no longer swallow their promises — caught by the existing await-based tests). app.test.ts (4.6k lines) types its harness with the established withApp/qs/Indexed conventions; three tests added to hold the branch-coverage baseline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XP1KN94N969SVMNFPdVtGg
…b in fake-app.ts main.ts closes the src tree: bootstrap(app: BootstrapApp, env: BootstrapEnv) with the narrow app slice it actually reads; the #266 share-link view assignment carries its honest commented cast (fix tracked separately). tests/helpers/fake-app.ts now satisfies the full App contract with a generic makeApp(overrides) that preserves callers' vi.fn() mock types; the appDefaults/withApp/asApp copies in eight test files are deleted (~1,080 lines of duplicated scaffolding gone). tests/helpers/saved-query.ts exports the typed SavedQueryV2 builder. App.specCompletionSources/CreateAppEnv now declare the real DynamicSources shape (was unknown[]); app.ts drops the cast that lie forced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XP1KN94N969SVMNFPdVtGg
tests/setup.ts; build.mjs entry points at src/main.ts explicitly; the vitest mixed-tree shim documents its one remaining reason to exist (two node-tooling .js specs pending an @types/node decision); tsconfig documents why allowJs survives (the two generated Ajv artifacts). ADR-0002 status: migration COMPLETE. CLAUDE.md and CHANGELOG reconciled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XP1KN94N969SVMNFPdVtGg
…erage Semantic-diff review vs origin/main found four conversion-introduced runtime deltas, all restored: ch-client's isAbort/errMessage go back to duck-typed checks (the injected fetch seam may reject with plain objects — the /Unknown setting/ pre-25.8 compat path depends on reading .message off them); results.ts's schema-graph Expand gate and chart-data's tooltip label drop the defensive branches the originals didn't have (erased ! instead). The same review's coverage audit found seven files whose branch coverage slipped below their .js baseline — artificial conversion-added branches erased, eight targeted tests added; all seven now at or above baseline (four at 100%). ADR/CHANGELOG module count corrected (106 runtime + 6 type-only). app.ts's dedicated function-by-function semantic diff came back clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XP1KN94N969SVMNFPdVtGg
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completes the ADR-0002 strict-TypeScript migration. Closes #267.
After #265 landed the gate + phases 1–5 (23 modules), this PR converts the remaining 87 hand-written
.jsfiles — every module undersrc/and every unit test is now strict TypeScript, converted leaf-up in dependency-tier waves with a zero-behavior-change policy, wave-gated ontsc+ the full unit suite + build + Playwright e2e.What converted (9 commits, 6 waves)
spec-schema's recursiveJsonSchemaNode+ service contracts), the CM6 editor stack against its.types.tsports, eight ui field modules,clickhouse-type's realTypeNodeAST union,library-codec,spec-draft.format,from-scope,schema-graph,kpi(whosereadKpiFieldsnow types the keyed-row union, closing the known gap),chart-data(full chart contracts),ch-client(injected-fetchChCtx+ genericChJsonResult<T>row typing),chart-render/grid-render(Chart.js stays a narrow injected structural seam),splitters,saved-history.completions(the contracts the CM6 adapter consumes), schema browser UI,codemirror-adapter(implementsEditorPortagainst real CM6.d.ts, preserving the Column-name autocompletion: FROM-driven column loading + alias/scope awareness #84/Workbench: add separate SQL and Spec JSON editor modes #212 happy-dom re-entrancy quirks).explain-graph(dagre seam; branch coverage verified 1:1 vs the original) andresults, which now owns the canonicalResult = ScriptResult | ScriptExportResult | QueryResultunion.app.ts(createApp declared against theAppcontract;app.types.tsreconciled to reality —SchemaFocus.kind,specCompletionSources: DynamicSources, correctedloadIdps/openNodeDetail) andmain.ts.Test-helper debt paid (issue item)
tests/helpers/fake-app.tsnow satisfies the fullAppcontract with a genericmakeApp(overrides)that preserves callers'vi.fn()mock types — the triplicated per-test App stubs (and five more copies that had accumulated) are deleted: ~1,080 lines of scaffolding gone across eight specs.saved-query.tsis a typedSavedQueryV2builder.Verification & review
tsc --noEmitclean · 2866 unit tests (101 files, per-file coverage thresholds) · build byte-stable in kind · 80/80 e2e (chromium+webkit).origin/main: function-by-function semantic diffs of the five riskiest conversions (app, results, ch-client, codemirror-adapter, explain-graph) plus a coverage/conventions audit. Findings fixed in the last commit: two dropped-returnregressions inapp.ts(exportEntry/renderDashboard promises — caught mid-migration by the tests), duck-typing restored inch-client'sisAbort/errMessage(the injected fetch seam may reject with plain objects; the/Unknown setting/pre-25.8 compat path reads.messageoff them), two conversion-added defensive branches erased, and seven files' branch coverage restored to at/above their.jsbaseline (8 targeted tests added). The conversion also surfaced a genuinely never-exercised completion-ranking branch (now tested) and several staleapp.types.tscontract lies (now truthful).Deliberately still
.js(recorded in ADR-0002's status)src/generated/*.js— generated Ajv artifacts (standalone emits JS);allowJsstays for them, documented in tsconfig.tests/e2e/*.spec.js— run by Playwright's own loader, outside thetscgate.tests/unit/{schema-build,spec-examples}.test.js— node build-tooling specs; typing them needs a deliberate@types/nodedecision (globalsetTimeout/DOM-lib interaction), deferred. The vitest mixed-tree resolver shim survives solely for these two, with a comment saying exactly when to delete it.Build and artifact unchanged in kind: four bundled runtime deps, zero third-party requests,
package.json/lockfile byte-identical to main.🤖 Generated with Claude Code
https://claude.ai/code/session_01XP1KN94N969SVMNFPdVtGg