From ded530487f66069b78c67dcc8f2c40563f458aae Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Thu, 25 Jun 2026 10:48:17 +0200 Subject: [PATCH 1/2] fix(editor): show Format errors in the results panel + jump the caret to them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A formatQuery() syntax error was shown in a brief, easily-missed toast. Now it renders persistently in the results panel (summarized — dropping the "Code: N. DB::Exception:" preamble and the huge "Expected one of: …" tail) and the editor caret jumps to, and scrolls to, the offending position. ClickHouse reports a 1-based offset into the query (newlines counted), so the raw (untrimmed) SQL is sent and the caret lands at N-1. A later successful format clears that error; real run results are never cleared on success (a `formatError` flag distinguishes them). Adds pure parseErrorPos/summarizeError (core/stream.js) and app.dom.editorRevealCaret (editor.js). 753 tests pass. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01QGBS74oUsXarGkCRQKEFLu --- src/core/stream.js | 25 +++++++++++++++++++++++++ src/ui/app.js | 25 +++++++++++++++++++------ src/ui/editor.js | 15 ++++++++++++++- tests/unit/app.test.js | 29 +++++++++++++++++++++++------ tests/unit/editor.test.js | 23 +++++++++++++++++++++++ tests/unit/stream.test.js | 25 ++++++++++++++++++++++++- 6 files changed, 128 insertions(+), 14 deletions(-) diff --git a/src/core/stream.js b/src/core/stream.js index b8a77e7..8b16664 100644 --- a/src/core/stream.js +++ b/src/core/stream.js @@ -72,6 +72,31 @@ export function parseExceptionText(text) { return text; } +/** + * The 0-based caret offset a ClickHouse error points at, or null. CH syntax + * errors carry "failed at position N (token): …" where N is 1-based and relative + * to the query string (newlines counted as one char), so it maps straight onto + * the editor text. Used to jump the caret to a format/parse error. Pure. + */ +export function parseErrorPos(msg) { + const m = /\bposition (\d+)/i.exec(String(msg || '')); + return m ? Math.max(0, parseInt(m[1], 10) - 1) : null; +} + +/** + * Condense a ClickHouse error for in-panel display: start at "Syntax error" when + * present (dropping the "Code: N. DB::Exception: …" preamble) and cut the noisy + * "Expected one of: " tail. Non-syntax messages pass through. + * Pure. + */ +export function summarizeError(msg) { + let s = String(msg || ''); + const i = s.search(/Syntax error/i); + if (i >= 0) s = s.slice(i); + s = s.split(/\.?\s*Expected one of:/i)[0]; + return s.trim(); +} + /** * True when a non-OK response body indicates an expired/invalid JWT. CH * returns HTTP 500 with `token_verification_exception` for a bad token, which diff --git a/src/ui/app.js b/src/ui/app.js index de41cc7..cc70f53 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -14,7 +14,7 @@ import { decodeJwtPayload, isTokenExpired } from '../core/jwt.js'; import { sqlString, inferQueryName, shortVersion, userShortName, withStatementBreak, detectSqlFormat, isExplain } from '../core/format.js'; import { resolveTarget } from '../core/target.js'; import { toTSV, toCSV } from '../core/export.js'; -import { newResult, applyStreamLine } from '../core/stream.js'; +import { newResult, applyStreamLine, parseErrorPos, summarizeError } from '../core/stream.js'; import { encodeShare } from '../core/share.js'; import { assembleReferenceData, buildCompletions } from '../core/completions.js'; import { generatePKCE, randomState } from '../core/pkce.js'; @@ -445,20 +445,33 @@ export function createApp(env = {}) { } app.setRunBtn = setRunBtn; - // Pretty-print the editor's SQL via ClickHouse's formatQuery(), in place. + // Pretty-print the editor's SQL via ClickHouse's formatQuery(), in place. The + // raw (untrimmed) SQL is sent so a syntax error's reported position maps 1:1 + // onto the editor text. On error we show it persistently in the results panel + // and jump the caret to the offending token; a later successful format clears + // that error. Success never touches real run results. async function formatQuery() { - const sql = (app.activeTab().sql || '').trim(); - if (!sql) return; + const raw = app.activeTab().sql || ''; + if (!raw.trim()) return; await ensureConfig(); if (!(await getToken())) { chCtx.onSignedOut(); return; } + const tab = app.activeTab(); try { - const json = await ch.queryJson(chCtx, 'SELECT formatQuery(' + sqlString(sql) + ') AS q FORMAT JSON'); + const json = await ch.queryJson(chCtx, 'SELECT formatQuery(' + sqlString(raw) + ') AS q FORMAT JSON'); const q = (json.data && json.data[0] && json.data[0].q) || ''; // Terminate so the caret lands past the last token — otherwise the input // event from the replace re-opens autocomplete on the trailing word. if (q) replaceEditor(app, withStatementBreak(q)); + if (tab.result && tab.result.formatError) { tab.result = null; renderResults(app); } // clear a prior format error } catch (e) { - flashToast('Format failed: ' + String((e && e.message) || e), { document: doc }); + const msg = String((e && e.message) || e); + tab.result = newResult('Table'); + tab.result.error = summarizeError(msg); + tab.result.formatError = true; // a format error, not a run result (so success can clear just this) + app.state.resultView = 'table'; + renderResults(app); + const pos = parseErrorPos(msg); + if (pos != null) app.dom.editorRevealCaret(pos); } } diff --git a/src/ui/editor.js b/src/ui/editor.js index 0f3983b..b1cffe9 100644 --- a/src/ui/editor.js +++ b/src/ui/editor.js @@ -5,7 +5,7 @@ import { h, zoomScale } from './dom.js'; import { tokenize, maskFromTokens } from '../core/sql-highlight.js'; import { buildMarkSegments } from '../core/editor-marks.js'; import { matchBracketAt, bracketEdit } from '../core/editor-brackets.js'; -import { caretXY, offsetFromXY } from '../core/editor-geometry.js'; +import { caretXY, caretLineCol, offsetFromXY } from '../core/editor-geometry.js'; import { createSearch } from './editor-search.js'; import { createComplete } from './editor-complete.js'; import { createIntel } from './editor-intel.js'; @@ -324,6 +324,19 @@ export function mountEditor(app, container) { app.dom.editorComplete = complete; app.dom.editorIntel = intel; app.dom.editorSync = sync; + // Move the caret to a character offset and scroll its line into view — used to + // jump to a format/parse error position (#format-error). + app.dom.editorRevealCaret = (pos) => { + const p = Math.max(0, Math.min(pos | 0, ta.value.length)); + ta.focus(); + ta.selectionStart = ta.selectionEnd = p; + complete.hide(); + const top = PAD_Y + caretLineCol(ta.value, p).line * LINE_HEIGHT_PX; + if (top < ta.scrollTop) ta.scrollTop = Math.max(0, top - LINE_HEIGHT_PX); + else if (top + LINE_HEIGHT_PX > ta.scrollTop + ta.clientHeight) ta.scrollTop = top + LINE_HEIGHT_PX - ta.clientHeight; + syncScroll(); + paintMarks(); + }; sync(); } diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index b310bc7..c7c8d84 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -399,15 +399,32 @@ describe('formatQuery', () => { await app.actions.formatQuery(); expect(app.root.querySelector('.login-screen')).not.toBeNull(); }); - it('surfaces a format failure without changing the editor', async () => { + it('shows a format error persistently in the results panel and moves the caret to it', async () => { const { app } = appFor([ - [(u, sql) => /formatQuery/.test(sql), resp({ ok: false, status: 500, text: '{"exception":"DB::Exception: syntax"}' })], + [(u, sql) => /formatQuery/.test(sql), resp({ ok: false, status: 500, text: '{"exception":"Code: 62. DB::Exception: Syntax error: failed at position 8 (BEWEEN): BEWEEN 2. Expected one of: BETWEEN, …. (SYNTAX_ERROR)"}' })], ]); - app.activeTab().sql = 'select 1'; - app.dom.editorTextarea.value = 'select 1'; + app.activeTab().sql = 'select x BEWEEN 2'; + app.dom.editorTextarea.value = 'select x BEWEEN 2'; await app.actions.formatQuery(); - expect(app.dom.editorTextarea.value).toBe('select 1'); // unchanged - expect(document.body.querySelector('.share-toast')).not.toBeNull(); + expect(app.dom.editorTextarea.value).toBe('select x BEWEEN 2'); // editor unchanged + const err = app.root.querySelector('.results-error'); + expect(err).not.toBeNull(); + expect(err.textContent).toContain('Syntax error: failed at position 8 (BEWEEN): BEWEEN 2'); // summarized (no Code prefix / Expected-of tail) + expect(app.dom.editorTextarea.selectionStart).toBe(7); // caret jumped to the offending token (pos 8 → offset 7) + expect(app.activeTab().result.formatError).toBe(true); + }); + it('a later successful format clears a prior format error', async () => { + const { app } = appFor([ + [(u, sql) => /BEWEEN/.test(sql), resp({ ok: false, status: 500, text: '{"exception":"Syntax error: failed at position 8 (BEWEEN): x. Expected one of: foo"}' })], + [(u, sql) => /formatQuery/.test(sql), resp({ json: { data: [{ q: 'SELECT 1' }] } })], + ]); + app.activeTab().sql = 'select x BEWEEN 2'; + await app.actions.formatQuery(); + expect(app.root.querySelector('.results-error')).not.toBeNull(); + app.activeTab().sql = 'select 1'; // fixed + await app.actions.formatQuery(); + expect(app.root.querySelector('.results-error')).toBeNull(); // error cleared + expect(app.activeTab().result).toBeNull(); }); }); diff --git a/tests/unit/editor.test.js b/tests/unit/editor.test.js index b6020a0..c21a87f 100644 --- a/tests/unit/editor.test.js +++ b/tests/unit/editor.test.js @@ -873,3 +873,26 @@ describe('signature help + hover docs (#27)', () => { }); }); }); + +describe('editorRevealCaret (jump to a format-error position)', () => { + it('sets the caret to the offset, clamps out-of-range, and scrolls the line into view', () => { + const app = makeApp(); + mountEditor(app, document.createElement('div')); + const ta = app.dom.editorTextarea; + ta.value = 'line0\nline1\nline2\nline3'; + // a caret below the viewport scrolls down to reveal it + ta.scrollTop = 0; + app.dom.editorRevealCaret(20); // on the last line + expect(ta.selectionStart).toBe(20); + expect(ta.selectionEnd).toBe(20); + expect(ta.scrollTop).toBeGreaterThan(0); + // a caret above the viewport scrolls back up + ta.scrollTop = 1000; + app.dom.editorRevealCaret(0); + expect(ta.selectionStart).toBe(0); + expect(ta.scrollTop).toBe(0); + // an out-of-range offset clamps to the end + app.dom.editorRevealCaret(99999); + expect(ta.selectionStart).toBe(ta.value.length); + }); +}); diff --git a/tests/unit/stream.test.js b/tests/unit/stream.test.js index ce3f349..0cd014b 100644 --- a/tests/unit/stream.test.js +++ b/tests/unit/stream.test.js @@ -1,7 +1,7 @@ import { describe, it, expect } from 'vitest'; import { newResult, applyStreamLine, splitBuffer, parseExceptionText, isAuthExpiredBody, - authDeniedMessage, + authDeniedMessage, parseErrorPos, summarizeError, } from '../../src/core/stream.js'; describe('newResult', () => { @@ -81,6 +81,29 @@ describe('parseExceptionText', () => { }); }); +describe('parseErrorPos', () => { + it('returns the 0-based caret offset from "position N" (1-based in the message)', () => { + expect(parseErrorPos('Syntax error: failed at position 18 (BEWEEN): …')).toBe(17); + expect(parseErrorPos('failed at position 1 (x)')).toBe(0); + }); + it('returns null when no position is present', () => { + expect(parseErrorPos('Some other DB::Exception')).toBeNull(); + expect(parseErrorPos('')).toBeNull(); + expect(parseErrorPos(null)).toBeNull(); + }); +}); + +describe('summarizeError', () => { + it('starts at "Syntax error" and drops the Expected-one-of tail', () => { + const raw = 'Code: 62. DB::Exception: Syntax error: failed at position 8 (BEWEEN): BEWEEN 2. Expected one of: BETWEEN, AND, OR. (SYNTAX_ERROR)'; + expect(summarizeError(raw)).toBe('Syntax error: failed at position 8 (BEWEEN): BEWEEN 2'); + }); + it('passes non-syntax messages through (trimmed)', () => { + expect(summarizeError(' DB::Exception: Unknown function foo ')).toBe('DB::Exception: Unknown function foo'); + expect(summarizeError(null)).toBe(''); + }); +}); + describe('isAuthExpiredBody', () => { it('detects token verification failures', () => { expect(isAuthExpiredBody('... token_verification_exception ...')).toBe(true); From 41af62b383bc546c986e9437b7c72d90dd6ea2c8 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Thu, 25 Jun 2026 11:00:00 +0200 Subject: [PATCH 2/2] fix(format): show the full ClickHouse error in the panel, not a summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop summarizeError — the format-error path now puts the original exception text (Code/DB prefix + "Expected one of: …" tail) into the results panel verbatim. Caret-to-position and clear-on-success unchanged. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01QGBS74oUsXarGkCRQKEFLu --- src/core/stream.js | 14 -------------- src/ui/app.js | 4 ++-- tests/unit/app.test.js | 2 +- tests/unit/stream.test.js | 13 +------------ 4 files changed, 4 insertions(+), 29 deletions(-) diff --git a/src/core/stream.js b/src/core/stream.js index 8b16664..92d8664 100644 --- a/src/core/stream.js +++ b/src/core/stream.js @@ -83,20 +83,6 @@ export function parseErrorPos(msg) { return m ? Math.max(0, parseInt(m[1], 10) - 1) : null; } -/** - * Condense a ClickHouse error for in-panel display: start at "Syntax error" when - * present (dropping the "Code: N. DB::Exception: …" preamble) and cut the noisy - * "Expected one of: " tail. Non-syntax messages pass through. - * Pure. - */ -export function summarizeError(msg) { - let s = String(msg || ''); - const i = s.search(/Syntax error/i); - if (i >= 0) s = s.slice(i); - s = s.split(/\.?\s*Expected one of:/i)[0]; - return s.trim(); -} - /** * True when a non-OK response body indicates an expired/invalid JWT. CH * returns HTTP 500 with `token_verification_exception` for a bad token, which diff --git a/src/ui/app.js b/src/ui/app.js index cc70f53..0dda99f 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -14,7 +14,7 @@ import { decodeJwtPayload, isTokenExpired } from '../core/jwt.js'; import { sqlString, inferQueryName, shortVersion, userShortName, withStatementBreak, detectSqlFormat, isExplain } from '../core/format.js'; import { resolveTarget } from '../core/target.js'; import { toTSV, toCSV } from '../core/export.js'; -import { newResult, applyStreamLine, parseErrorPos, summarizeError } from '../core/stream.js'; +import { newResult, applyStreamLine, parseErrorPos } from '../core/stream.js'; import { encodeShare } from '../core/share.js'; import { assembleReferenceData, buildCompletions } from '../core/completions.js'; import { generatePKCE, randomState } from '../core/pkce.js'; @@ -466,7 +466,7 @@ export function createApp(env = {}) { } catch (e) { const msg = String((e && e.message) || e); tab.result = newResult('Table'); - tab.result.error = summarizeError(msg); + tab.result.error = msg; tab.result.formatError = true; // a format error, not a run result (so success can clear just this) app.state.resultView = 'table'; renderResults(app); diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index c7c8d84..057d644 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -409,7 +409,7 @@ describe('formatQuery', () => { expect(app.dom.editorTextarea.value).toBe('select x BEWEEN 2'); // editor unchanged const err = app.root.querySelector('.results-error'); expect(err).not.toBeNull(); - expect(err.textContent).toContain('Syntax error: failed at position 8 (BEWEEN): BEWEEN 2'); // summarized (no Code prefix / Expected-of tail) + expect(err.textContent).toContain('Code: 62. DB::Exception: Syntax error: failed at position 8 (BEWEEN): BEWEEN 2. Expected one of: BETWEEN, …. (SYNTAX_ERROR)'); // full original message, untruncated expect(app.dom.editorTextarea.selectionStart).toBe(7); // caret jumped to the offending token (pos 8 → offset 7) expect(app.activeTab().result.formatError).toBe(true); }); diff --git a/tests/unit/stream.test.js b/tests/unit/stream.test.js index 0cd014b..d135512 100644 --- a/tests/unit/stream.test.js +++ b/tests/unit/stream.test.js @@ -1,7 +1,7 @@ import { describe, it, expect } from 'vitest'; import { newResult, applyStreamLine, splitBuffer, parseExceptionText, isAuthExpiredBody, - authDeniedMessage, parseErrorPos, summarizeError, + authDeniedMessage, parseErrorPos, } from '../../src/core/stream.js'; describe('newResult', () => { @@ -93,17 +93,6 @@ describe('parseErrorPos', () => { }); }); -describe('summarizeError', () => { - it('starts at "Syntax error" and drops the Expected-one-of tail', () => { - const raw = 'Code: 62. DB::Exception: Syntax error: failed at position 8 (BEWEEN): BEWEEN 2. Expected one of: BETWEEN, AND, OR. (SYNTAX_ERROR)'; - expect(summarizeError(raw)).toBe('Syntax error: failed at position 8 (BEWEEN): BEWEEN 2'); - }); - it('passes non-syntax messages through (trimmed)', () => { - expect(summarizeError(' DB::Exception: Unknown function foo ')).toBe('DB::Exception: Unknown function foo'); - expect(summarizeError(null)).toBe(''); - }); -}); - describe('isAuthExpiredBody', () => { it('detects token verification failures', () => { expect(isAuthExpiredBody('... token_verification_exception ...')).toBe(true);