`tokenizeSync` decoded each token with a default `TextDecoder`, which strips a
leading `U+FEFF`. The token lengths then no longer matched the original text,
so the statement offsets used by the multi-statement "Format" button were off
by one and a BOM-prefixed `select 1;` was sent as `select `. Decode with
`ignoreBOM: true` and add a regression case to the harness.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- `splitAllQueries`: the lexer stops at the first error (e.g. an unterminated
string literal or comment), so the unlexed remainder was dropped and Format
would edit only the valid prefix. The remainder now belongs to the last
statement, so the server reports the error and the editor is unchanged.
- Format: re-check the connection, tab and text after awaiting the splitter
(which loads the lexer on first use), so the text is never sent with the
credentials of a connection selected after the click.
- Update the Format fixture of `test_play_reconcile_startup` for the new `f<N>`
aliases and cover multiple statements, all-or-nothing errors, lexer errors and
a connection change during the lexer load.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Split the editor's text into statements with `splitAllQueries`, format all of
them in one request (`SELECT formatQuery({q0:String}) AS f0, ...`), and replace
each statement in place. The `;` delimiters, whitespace, and comments between
statements are kept as is.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The `Format` button sent `password_elem.value` without `dropAutofilledStalePassword`, so a stale
autofilled password could still be sent by it. It now goes through the guard like the other
requests built from the live connection fields.
`rememberAuthenticationOutcome` ignored every `403`, but `ACCESS_DENIED` (a missing privilege) is
also answered with `403` and is thrown only after a successful authentication. The
`X-ClickHouse-Exception-Code` header tells it apart, so a `403` with `ACCESS_DENIED` now updates
the passwordless entry like any other error of the query. The harness pins both cases.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`buildTabsMarkdown` wrote `cell.params` verbatim, while `loadTabsFromMarkdown` stamps the
loaded pair as coherent (`paramsSyncedQuery = query`). A cell whose bindings were committed
for an older text therefore had them saved, and made durable on Load, under the new text.
Prune them against the text with `pruneParamsToText` unless `paramsSyncedQuery` matches,
as `writeHistoryEntry` does, and cover it with the `save-drops-stale-parameters` scenario.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflict in the `Format` button handler of `programs/server/play.html`: master added the
`connectionGeneration` check; the branch checks that the editor is still docked in the same
notebook cell. Both checks are kept.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`rememberLoginPassword` was only called from `checkCredentials` and from a 2xx
response in `postImpl`. On deployments where `checkURL` never reaches
`checkCredentials` (the ping probes the origin, not the configured path), the
database panel, `auxiliaryQuery` or the completions can be the first request
to authenticate with a newly typed password, and a query that fails after
authenticating (a syntax error, an unknown table) also proves the credentials.
The new `rememberAuthenticationOutcome` is called from `auxiliaryQuery`,
`loadCompletions` and `postImpl` (on the response the framing retries settled
on). `401`/`403` never update the entry; another error updates it when the
server itself produced it (`X-ClickHouse-Exception-Code`); without that proof
a non-empty password still forgets the entry, but an empty one is not
remembered. Adds the `test_play_passwordless_logins` harness test.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Two follow-ups to review comments on https://github.com/ClickHouse/ClickHouse/pull/121652:
- The click handler did not call `markBootstrapDirty` before its server round-trip, so a
click during startup reconciliation let `reconcileStartup` activate the saved tab or restore
the saved editor contents, and the formatted text was then discarded as stale.
- `auxiliaryQuery` still returns the payload after a connection change, and the handler did not
re-check `connectionGeneration`, so a `formatQuery` result from the previous server or user
could replace the editor text.
Both are covered by new scenarios in `test_play_reconcile_startup` (each one fails without its fix).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`prettyPrintJSON` rendered the value after a round trip through `JSON.parse`,
so the selected view could show a different document than the compact cell:
numbers became IEEE-754 doubles (`9007199254740993` shown as
`9007199254740992`, `1e309` as the non-JSON `Infinity`), strings were
re-escaped, a repeated key was dropped and integer-like keys moved first.
It now walks the original text with a small recursive-descent parser that
copies every scalar lexeme verbatim and only changes the whitespace; text that
is not JSON throws a `SyntaxError`, as before.
The new integration test `test_play_json_pretty_print` executes the real
`prettyPrintJSON` from the served page in Node.js (the same setup as
`test_play_result_shaping`) and checks the layout, the verbatim value, the
highlighting spans and the rejection of invalid text; against the previous
implementation 7 of its checks fail.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
If the tab title is not the default generated one (`Query A`, `Query B`, ...),
the window title is exactly the tab title, without the `ClickHouse Query: `
prefix. Tabs with default titles keep the previous behavior.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Adding, editing or deleting a Markdown cell, moving a cell, or deleting an
inactive query cell only armed the 400 ms `scheduleSave` debounce. No cell
activation or run follows such a change, so nothing else recorded it: a reload
inside that window found the pre-change notebook both in `IndexedDB` and in
the current history entry (whose `cells` recreate the tab on Back/Forward).
- `commitNotebookChange(tab)` refreshes the active tab's history entry in place
and saves at once (`saveNow`, which takes over a pending debounce); used by
`addCell` (Markdown cell), `moveCell`, `deleteCell` and the Markdown editor's
`blur`. Keystrokes stay debounced, like the query editor's.
- `addCell` for a query cell saves once the activation has loaded the new cell.
- A save still pending when the page goes away is started on `pagehide`.
New `notebook_harness.js` scenarios: `notebook-change-survives-immediate-reload`
(each change snapshotted 30 ms after it and rebooted; all 8 checks fail on the
previous `play.html`), and `save-load-round-trips-notebook` for the notebook-aware
Save / Load from the master merge. `waitForNextPersist` now waits until no save
is pending, since a notebook change no longer leaves a debounced one behind.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Conflicts in `programs/server/play.html`, all resolved in favour of master's
single layout while keeping the notebook structure:
- markup: the connection `#inputs` now live in the drop-down (master); the
editor keeps its `#editor-home` wrapper and gains master's `Format` button;
- `renderTabBar`: `syncConnectionUI` is gone (master), `focusEditor` kept;
- `saveHistory`: `cell.result` kept with master's comment;
- startup blank-tab pruning comment merged.
Reconciled with master's new code:
- master dropped the `controls_elem` declaration that the notebook run row
still needs; re-declared as `controls_form_elem` (so `05234`, which pins that
the connection inputs never move, keeps its meaning);
- Save / Load of tabs as Markdown now covers every cell of a notebook: each
query cell as before (query, parameters, result tables), each Markdown cell
as a `markdown` code block; Load turns every block under a tab heading into
a cell. A single-query tab saves exactly as before. `05233` updated;
- the `Format` button also drops its result when the editor moved to another
cell while the request was in flight.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The button is in the bottom right corner of the editor. It sends the query
to the server as a parameter of `SELECT formatQuery({q:String})` in the
request body and replaces the editor contents with the result (undoable).
A formatting error is shown in a balloon above the button.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The two dialog textareas carried only `spellcheck="false"`. They hold
SQL, Markdown and JSON, so give them the same opt-out the query editor
already has: `autocorrect`, `autocomplete` and `data-gramm` too.
Grammarly in particular adopts a textarea and injects its own overlay
into it, littering the monospace text with underlines.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The page had two layouts. A fresh visit - one tab that had never run -
hid the tab bar and showed the url/user/password inputs inline above the
editor; anything else showed the bar and relocated that same `#inputs`
element into the drop-down behind the `🔑` button. The bar appeared when
a second tab was opened, when a run started, and when a run left a
result, and collapsed back when the last of those went away.
The consequence was that everything living in the bar - now including
loading a saved workspace, which is exactly what one wants on a fresh
page - was unreachable until a query had been run.
Keep only the second layout. The bar is always shown, and the connection
inputs sit in the drop-down from the start: the markup puts them there,
so there is no relocation and no first-paint shift. With one layout,
`syncConnectionUI` and the focus hand-offs it needed across the
transition are gone, as is the bar's visibility rule and the two
re-renders (`beginFlight`, `endFlight`) that existed to drive it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The hint spelled out the whole document layout and which parts of it are
read back. That belongs in the code comments, where it already is; the
dialog only needs to say what the button does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two icons were drawn to different vertical extents on their shared
24x24 grid - the floppy's body is square (y from 3 to 21) while the open
folder is a unit shorter (y from 3 to 20) - so side by side their tops
and bottoms did not line up.
Shorten the floppy's body to 17 units so both span y from 3 to 20. It is
the floppy that gives way, not the folder: the folder's height is set by
two arcs tangent to its box, and moving those would reshape its corners.
Measured in the browser: both `getBBox` extents are now y 3.00 to 20.00,
and at 4x device scale the rendered ink of both spans exactly the same
rows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two buttons carried the `💾` and `📂` emoji. Replace them with
inline SVG line art in the same weight and 24x24 grid as the toolbar's
copy and download icons, stroked in `currentColor`, so they are muted
in the tabs' own `--misc-text-color` when idle and lit on hover - a
colour glyph takes no text color, so it could be neither.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two buttons at the right end of the tab bar, left of the `🔑` connection
button, save the whole workspace - the list of tabs, the query of every
tab and its query parameters - as a Markdown document, and load it back.
Each opens a modal. Save shows the document in a textarea with `Copy`
and `Download`; the textarea is editable and both actions read it back,
so the document can be trimmed before it leaves the page. Load takes a
document pasted into its textarea, or uploaded from a file into that
same textarea, so a file is reviewed on the same screen as a paste
before it replaces the workspace.
The document has one top-level heading per tab (its title), the tab's
editor content in a fenced code block, its parameter values as JSON
under a `## Parameters` sub-heading, and - when the tab has one - its
result as a Markdown table:
# Query A
```
SELECT {n:UInt8}
```
## Parameters
```json
{
"n": "1"
}
```
| 1 |
|---|
| 1 |
Loading reads back the titles, the queries and the parameters. The
parameters are written as JSON rather than as a table so that they
round-trip exactly: they are restored as data, and a table would mangle
a value holding a `|` or a newline. The result tables, in contrast, are
deliberately not read back - a table is a lossy rendering of a result
(no column types, cells escaped for the table syntax, rows capped), so
restoring one as data would fabricate a result the server never
returned. A loaded tab is therefore a query that has not been run yet.
The tables are built by replaying each tab's stored result snapshot into
a duck-typed stand-in for `query-result` that collects the header and
the rows instead of rendering them, so the framing and the compact row
shapes are parsed by the very code that rebuilds a table on screen
(`makeEventStreamHandler`) rather than by a second parser.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two findings of the AI review on `programs/server/play.html`.
1. `serializeCellsForHistory` counted only the result snapshots against
`HISTORY_NOTEBOOK_BUDGET`, so a text-heavy notebook could exceed the
browser's per-entry history-state limit through its Markdown alone and
`writeHistoryEntry` fell through to the `cells: []` fallback, after which
`Back` could recover only the active query cell of a closed tab. The
budget now covers the whole serialized cell (query, parameters, text,
result): result snapshots are stripped first, largest first, then
Markdown text is trimmed by no more than the remaining excess. Queries
are never cut, because a truncated statement can be a different, still
valid statement. `writeHistoryEntry` gained a rung between "results
stripped" and "no notebook copy" that drops the Markdown text but keeps
the structure and every query.
2. `deleteCell` stopped a run in the deleted cell with an unconditional
`progressEl.clear()` after `endFlight` had already repainted the shared
row from the surviving active cell, wiping that cell's stats. The delete
path now goes through `cancelTabRun`, whose own trailing clear is made
conditional on `cellOwnsChrome(cell)`: the widget is cleared only while
it still describes the stopped cell.
Tests: `notebook_harness.js` gains `delete-running-cell-keeps-active-chrome`
and `history-budget-covers-text`, and `stop-after-editor-moved-repaints-chrome`
now records the calls made on the shared widget. All three fail against
the previous page. `04548_play_ui_framing_fallbacks` expects five
`cancelTabRun(tab);` call sites.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A result header can carry the same output name more than once, and both the
row lookup and the per-column type flags are keyed by that name, so a `JSON`
namesake of a non-`JSON` column tagged the latter's cells with `_jsonText`;
selecting one of them then sent plain text through `JSON.parse` in
`_expandJSON`, which threw. Withhold the tag for a duplicated name, the way
sort and filter are already withheld from it (`duplicateColumnNames`), so the
compact form simply stays. The test pins the guarded line.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`dropAutofilledStalePassword` now also runs before `loadDatabases` (opening the databases
panel), before the download rerun, before `storeCredentials` and in `sendCredentialsToTerminal`,
so a stale autofilled password for a login remembered as passwordless is dropped on every path
that sends or stores it, not only on credential checks, query submission and completions.
`rememberLoginPassword` and `loadPasswordlessLogins` swallow `localStorage` failures
(`QuotaExceededError`, storage policy exceptions), so a successful authentication check or
query is never turned into a client-side error by the bookkeeping.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A long number in the result table was grouped by underlining one digit at
each group-of-three boundary, mirroring `clickhouse-client`. In a proportional
page - unlike a terminal - a gap reads better, and the classic typographic
grouping is a thin space: `1 234 567`.
A real separator character would be copied along with the number, so the
groups are separated by an EMPTY element instead - `<span class="num-sep">`,
an inline-block a quarter of a digit wide (`0.25ch`, which in the table's
monospace font is exactly a quarter of a digit). The gap is painted, but the
text of the cell is still nothing but the digits: selecting the number, or
copying the cell, yields `1234567`.
The same grouping is applied to the row-number column, whose numbers pass
five digits on a later page of a large result.
The digit groups in the query editor are left as they are: its backdrop must
match the metrics of the textarea underneath it character for character, so
the highlight cannot change the advance width of anything.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The offset a failed paginated run's rows were numbered from (`skipped_rows`)
was only put back into the result element in the plain `!result.ok` branch of
`materializeResult`. A default-format run of a current server is a framed
`EventStream`, and its failed snapshot is replayed by the `event_stream`
branch before that, so on a reload, Back/Forward, or a tab reopen the rows of
a failed page 2 were still renumbered from 1. Set `_skipped_rows` before the
branches, so every replay of a failed snapshot numbers its rows from the offset.
The `/play` startup harness (`tests/integration/test_play_reconcile_startup`)
stubs the result element, and `postSingle` now reads `_rowNumberOffset` from
it before dropping the page of a failed run: give the stub the method (the
stub never paginates), which fixes the `TypeError: el._rowNumberOffset is not
a function` failures of `test_play_reconcile_startup` in CI:
https://s3.amazonaws.com/clickhouse-test-reports/praktika.html?PR=116614&sha=20587bdb4074a205308cbce86b6328f7ab35856e&name_0=PR&name_1=Integration%20tests%20%28amd_tsan%2C%203%2F6%29https://github.com/ClickHouse/ClickHouse/pull/116614
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`deleteCell` handed the editor to a neighbouring query cell but left the current
history entry and the URL hash describing the cell it had just deleted. Both are
read back as authoritative for the tab, so the deleted query came back:
- `reconcileStartup` substitutes the URL query into the saved tab when it equals
neither its `query` nor its `lastSavedQuery`, and nulls the result with it;
- `window.onpopstate` writes the entry's query into whatever cell is active.
Reproduced in `notebook_harness.js`: with cells `A` (URL-synced) and `B`, making
`A` active and deleting it left the hash at `A`'s query, and the following reload
replaced `B`'s query with `A`'s and dropped `B`'s result. A Back to that entry
did the same in-session.
`deleteCell` now re-owns the entry from the post-delete notebook once the editor
handover completes, via `refreshCurrentHistoryEntry` - in place, because a
deletion is a structural change and must not push a Back target - exactly as
`setActiveCell` stamps history after its own activation.
Deleting the LAST query cell is now refused outright. That cell is where the
shared editor lives and everything the editor, history, URL and persistence code
reads through `tab.<field>` forwards to it, so a notebook of text cells alone has
no query to describe: `tab.query` was `undefined` and `renderTabBar` threw out of
`queryToColor`. The cell header already disables `✕` for that cell, so the rule
is now enforced in `deleteCell` for every other caller too, instead of papering
over the resulting state downstream.
New scenario `delete-active-cell-reowns-history` covers the entry, the URL hash,
the absence of a pushed entry, the reload, the Back, and the refusal. Verified
load-bearing: all five restore checks fail against the previous `play.html`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Clicking a cell of a `JSON` column in `play.html` now shows the value pretty-printed with
two-space indentation, one member per line, with keys, strings, numbers and the literals
`true`, `false`, `null` highlighted. The compact one-line form is restored when the
selection leaves the cell, and stays the value used for copying and filtering.
The pretty form is built lazily for the selected cell only (`_expandJSON`), from the compact
text the cell already holds, which is the same on the page's default `JSONCompactStrings`
format (the value arrives as JSON text) and on the `JSON` format (a parsed object). The
expanded overlay of a selected cell is color-inverted, so the pretty-printed block inverts
itself back and takes its palette from new `--json-*` variables defined per theme.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The page offers the login to the browser's password manager after each
Run, but the Credential Management API cannot express an empty password
(`PasswordCredential` rejects it with a `TypeError`) and there is no API
to delete a saved login. So after a server/user was saved with a password
and later authenticates with an empty one, the browser keeps the stale
entry and autofills it on every visit, and the page sends the stale secret
with every request.
Remember in `localStorage` which server/user logins last succeeded with an
empty password (recorded from the credential probe and from a successful
query run, forgotten again when a non-empty password succeeds), and drop
a browser-autofilled password (`:autofill`) for such a login when it shows
up: on the `input` event autofill raises, and at the points that read the
field to build a request. A typed password is always kept.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Address review: `postSingle` drops the tab's shape (`clearResultShape`) when a run fails, so a
paginated run that streamed rows before an in-band failure was snapshotted with `{ page: 0, size: 0 }`,
and `renderFailedSnapshot` replayed those rows through `appendRow` numbered from 1 - while live they
were numbered from `size * (page - 1)`. Capture the offset (`_rowNumberOffset`) before the shape is
dropped, persist it with the failure snapshot as `skipped_rows`, and let the restored element count
from it (`_skipped_rows`) while no page is selected. Successful runs keep counting from the restored
`pagination`; legacy entries read as a zero offset.
The stateless test checks the persistence and replay lines in the served `/play` source.
