Skip to content

feat: query activity drawer - #612

Merged
emrberk merged 12 commits into
mainfrom
feat/query-activity-drawer
Sep 28, 2026
Merged

feat: query activity drawer#612
emrberk merged 12 commits into
mainfrom
feat/query-activity-drawer

Conversation

@emrberk

@emrberk emrberk commented Sep 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Query Activity drawer

A new Query Activity button in the right sidebar, directly below Table Details, opens a drawer that lists the queries currently running on the server. It is built on the server's query_activity() function, which now reports live memory usage per query.

What the drawer shows

Header. The title carries a badge with the number of running queries.

Summary tiles.

  • Running queries: how many queries the server is executing right now.
  • Memory in use: the total native memory currently held by those queries. Background work such as WAL apply and view refresh reports no memory and is not counted.

Search. The search box filters the list as you type. It matches the query text, the query id, and the username, case-insensitively. Pressing Escape clears the box. Pressing Escape on an empty box closes the drawer.

Sort. A dropdown offers six orders: Memory used, Start time, and Query ID, each ascending or descending. The default is Memory used descending, so the query holding the most memory is on top. Queries with no memory data always sort last under Memory used. There is no Duration order, because sorting by start time already answers "which query has run longest".

Auto refresh. The refresh toggle polls the server once a second while it is on. Turning it on fetches a fresh list at once. Turning it off stops polling and sends nothing further, so the list freezes. The choice is remembered across sessions.

Last updated. A line under the toolbar reports the age of the list, for example Last updated 3s ago. It is always visible, so a frozen or failing list never looks live. A spinner appears to its left while an automatic refresh is in flight, but only once the request outlasts 500 ms, and it then stays for at least 500 ms. A normal round trip therefore shows nothing at all. A manual refresh, from Retry or after a cancel, shows no spinner while auto refresh is on.

What a query row shows

  • State: an hourglass for Running, a cross for Cancelled, a check for Finished.
  • Memory: the memory the query holds right now. When the query has a memory limit it reads used / limit. Background work shows N/A.
  • Start time: how long ago the query started, for example Started 1m 12s ago, updated every second. Hover it to see the start time in ISO format, to the millisecond, with a copy button.
  • Query text: the SQL, formatted for reading. Very long statements show their first and last lines with the middle elided. Hover the block for Open in editor, which opens the full formatted query in a new tab, and Copy, which copies the full formatted query.
  • Footer: the user who ran the query, the worker pool and worker id, a WAL tag when the statement is being applied by the WAL job, the query id, and the Cancel button.

The row reports when a query started, never how long it ran. The server sends no stop time, and while auto refresh is off it sends nothing at all, so any runtime figure would be a guess that grows without limit. A start time stays true no matter when the list was last refreshed. The Last updated line tells you how old the Running label itself is.

Cancelling a query

Cancel asks for confirmation, then sends a cancel request to the server. The row usually switches to Finished, because the server drops a cancelled query within milliseconds and the next poll no longer lists it. A row shows Cancelled only when a poll catches the query while the server is still winding it down.

Cancel is not offered for WAL rows, because the server refuses to cancel them, nor for rows that are already cancelled or finished. On Enterprise, cancelling another user's query requires the SQL ENGINE ADMIN permission; the server's error is shown as a notification. The list refreshes after a cancel whether it succeeded or failed, so a row for a query that has already gone clears itself.

Memory warnings

Only memory produces a warning or error tone, and only for queries that run under a memory limit. The tone reflects how much of that limit the query holds. Queries without a limit, and background work that reports no memory, are never coloured. Elapsed time never colours a row, since what counts as long depends on the workload.

Memory used Tone Hover text on the state icon
under 50% of the limit none —
50% to 80% warning More than 50% of the available memory used
80% and above error More than 80% of the available memory used

Cancelled queries are never graded.

How a query disappears

The server only lists queries while they run. It does not report an outcome. When a poll no longer returns a query, the row switches to Finished with the hover text "The query left the registry. It finished, failed, or timed out." The row stays for five seconds, then fades out.

While your mouse is over the row, or keyboard focus is inside it, the row stays as long as you like. When you leave it, a fresh five seconds starts. A cancelled query that leaves the server lingers the same way but keeps its Cancelled state.

Other things to know

  • On Enterprise, users without SQL ENGINE ADMIN see only their own queries. Administrators see everything.
  • The drawer's own listing query is hidden from the list.
  • Query ids are assigned by the server and start again from zero after a restart.
  • If the server cannot be reached, the drawer shows an error. If it stops responding while the drawer is open, the last successful list stays visible with a banner until polling recovers.
  • The loading panel on open is delayed by 200 ms, so a fast listing never flashes it.
  • The list is virtualised and stays smooth with thousands of running queries.
  • CPU time and disk I/O per query are not shown because the server does not expose them yet. The drawer is designed so they can be added as further columns.
  • Finished queries are not kept beyond the grace period. Query history is a separate feature.

Changes outside the drawer

SQL formatter

Format Document now uses @questdb/sql-parser instead of sql-formatter. Output changes for most queries. The new formatter keeps every token across about 100 QuestDB-specific shapes, and it is much faster than the old one.

A new editor setting, Capitalize keywords on format, controls whether the formatter upper-cases keywords. It is off by default.

Static SQL blocks

Read-only SQL blocks elsewhere in the console, in the AI chat, the table details DDL, and the shared link confirmation, now render as static highlighted text instead of an embedded editor. Their text can be selected with the mouse, and their copy button now appears on hover, like the open-in-editor action already did. For a typical short statement this is lighter than mounting an editor.

Two known limits, both tracked separately:

  • A very long unbroken token no longer wraps. It is clipped or scrolls sideways, where the editor used to wrap it.
  • A very large statement renders every line, so opening one costs more than the editor did, which windowed to the visible lines.

Escape in drawers

Escape now behaves the same way in every drawer. While focus is in a text field that holds content, Escape does not close the drawer. The Query Activity and chat history search boxes clear themselves on the first press, so the next press closes the drawer. Other fields keep their content, so the user must clear the field or move focus before Escape closes the drawer. This keeps drafts that were previously lost, for example in the AI chat and the CSV import forms.

Shared toolbar spinner

The notebook cell toolbar spinner moved from src/scenes/Editor/Notebook/cells/Spinner.tsx to src/components/Spinner, so the query activity toolbar can use the same neutral control. Its timing is now a shared useDelayedFlag hook, which turns a flag on only once work outlasts a delay and then holds it for a minimum, so a short wait never flashes an indicator.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Web Console deploy preview

Preview Commit Logs
https://pr-612--web-console.netlify.app daac0a3 build log

@emrberk

emrberk commented Sep 22, 2026

Copy link
Copy Markdown
Member Author

Reviewing PR #612 at level 3

Issues

Issue ID Issue name Category Severity Location Net impact Evidence Description Steps to reproduce Suggested fix
#1 Populated composer traps Escape Cross-context caller impact Moderate in-diff — src/components/Drawer/index.tsx:126-170,245-252 AI-chat users lose Escape dismissal whenever a draft is present. Cypress: drawer stayed open at adf1d4fe; identical test passed with the Drawer hunk reverted to 9b914793 behavior. The shared Drawer now assumes every populated native text field clears itself on Escape. The AI composer handles Enter only, so Escape neither clears the draft nor dismisses the drawer; repeated presses remain inert. This regresses the base keyboard contract.
  • Configure and open AI chat.
  • Type an unsent draft in the composer.
  • Press Escape one or more times.
  • Observe that the draft and drawer remain open.
 Keep Drawer’s default Escape-to-dismiss behavior. Make two-stage clear-then-close behavior an explicit consumer opt-in, or have only consumers that actually clear their field prevent the first Escape.
#2 Toggle-off skips final refresh Query execution & data integrity Moderate in-diff — src/scenes/QueryActivity/index.tsx:115-120 Operators can freeze Query Activity on a stale pre-toggle snapshot. Independent Cypress: no listing request within 700 ms at adf1d4fe; N/A — new surface. Turning auto-refresh off only persists the setting. It disables the adaptive loop without requesting the fresh snapshot promised for either toggle direction. If server activity changes after the last poll, the frozen list can omit that change indefinitely until the user re-enables refresh or reopens the drawer.
  • Open Query Activity with auto-refresh enabled.
  • After the current response, change the mocked/server query set.
  • Turn auto-refresh off before the next scheduled poll.
  • Observe that no immediate listing request occurs and the prior snapshot remains.
 Trigger an explicit fetchNow/refresh for both toggle directions. Ensure the off transition’s final request is not canceled by disabling the polling loop. Add the missing on-to-off Cypress case.
#3 Focused finished row persists React correctness & hooks Moderate in-diff — src/scenes/QueryActivity/useQueryActivity.ts:94-103,126-143; src/scenes/QueryActivity/QueryActivityRow.tsx:275-309 Keyboard users leave focused finished queries visible indefinitely. Independent Cypress at adf1d4fe: Finished row remained after 9 seconds; N/A — new surface. When a focused running query disappears, the current snapshot first unmounts the row and a passive effect then reconstructs it as Finished. Unmount loses focus without releasing the parent’s held ID. Every later expiry tick therefore treats the row as held, so the advertised five-second grace period never starts unless the user happens to hover and leave the replacement row.
  • Open Query Activity with a running query.
  • Focus the row’s elapsed-time button.
  • Return a subsequent activity snapshot without that query.
  • Observe the replacement Finished row after more than five seconds; it remains indefinitely without further pointer interaction.
 Preserve the row through the running-to-finished transition or release its held ID during unmount. Make the snapshot/finished transition atomic enough to avoid the intermediate removal, and add a focused-row expiry Cypress case.
#4 Launcher focus not restored Accessibility & UX Moderate in-diff — src/scenes/QueryActivity/index.tsx:142-158 Keyboard users lose their place whenever Query Activity closes. Independent Cypress at adf1d4fe: visible launcher was not focused after close; N/A — new surface. The visible QueryActivityButton opens the Redux-controlled drawer, but the Drawer registers an unrelated non-focusable <span /> as its Radix trigger. On close, Radix attempts to restore focus to that span, so focus does not return to the control that opened the drawer.
  • Focus the Query Activity sidebar button and open the drawer.
  • Close it with the drawer’s close button.
  • Observe that the Query Activity launcher is not focused.
 Wire the visible launcher to Radix as the actual trigger, or pass a launcher ref and restore it explicitly in onCloseAutoFocus. Add a keyboard focus-return assertion.

Adjacent findings (not blocking — file as issues)

None.

Summary

  • Verdict: approve with comments — address wip test for error range #1–add test for query run when cursor position is next to ending semicolon #4, but none is merge-blocking under the review rubric.
  • Correctness gate: passes; no admitted Critical issues.
  • Test gate: passes; 2 admitted Moderate coverage gaps and 0 admitted Critical coverage gaps.
  • Quality checks: yarn typecheck, yarn build, yarn lint, and yarn test:unit passed. Unit tests: 98 files, 2,158 tests.
  • Browser rung: e2e/tests/console/queryActivity.spec.js passed all 15 tests. Targeted falsification artifacts reproduced each issue above; the live query_activity() SQL also succeeded against the bundled authenticated QuestDB instance.
  • Location split: 4 in-diff, 0 out-of-diff. The shared-caller pass covered Drawer, LiteEditor/HighlightedSql, SQL formatting, adaptive polling, catalog sources, local-storage context, sidebar state, and the extracted Table Details utilities.
  • Severity distribution: 0 Critical, 4 Moderate, 0 Minor.
  • Tradeoffs: the drawer’s core listing, cancellation, sorting, error, and retry paths are well covered, and measured list recomputation remained below 8 ms at 10,000 rows. The remaining issues are bounded to toggle freshness and keyboard/focus contracts.

@emrberk
emrberk marked this pull request as ready for review September 22, 2026 12:52
@puzpuzpuz

puzpuzpuz commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Reviewing PR #612 at level 3 (full mission-critical pass), against head 5bd1cfc3 and base ec3b8bb6.

Verdict: request changes. The new SQL formatter silently changes or breaks some valid QuestDB queries. That is the one Critical finding. The other eleven findings are ten Moderate and one Minor.

PR title and description

  • Title: it follows Conventional Commits. The repo convention repeats the verb, so it should be feat: add query activity drawer.
  • Description omits user-visible changes:
    • The SQL formatter was swapped from sql-formatter to @questdb/sql-parser, so Format Document output changes for nearly every query.
    • There is a new "Capitalize keywords on format" editor setting.
    • Escape now behaves differently in every drawer.
  • Description claims the code doesn't meet:

Issues

ID Issue Category Severity Location Net impact Evidence Description Steps to reproduce Suggested fix
#1 Formatter splits QuestDB literals Query execution & data integrity Critical in-diff: src/utils/formatSql.ts:11. Reaches unchanged consumers: editor Format (questdb-sql/formatting.ts:9,27), AI apply (EditorProvider/index.tsx:829, aiAssistant.ts:562), Query Activity copy/open Users of hex/float/geohash literals get wrong results or broken DDL Cypress with a real Alt+Shift+F plus the live server, at 5bd1cfc3 and at ec3b8bb6 (base keeps every token intact) The new formatter inserts a space inside single tokens: 0x1F→0 x1F, 16777217.0f→16777217.0 f, geohash(5c)→geohash(5 c). Some results change silently: SELECT id, 0x1F FROM t runs green and returns INT 0 in a column named x1F instead of LONG256 0x1f. …0f returns a DOUBLE with a different value. Others fail: geohash DDL and casts (the documented precision syntax), WHERE h = 0x…, VALUES(…, 1.5f). AI suggestions go through normalizeSql unconditionally, after the model has already validated the raw SQL. The capitalize setting doesn't matter.
  1. Type SELECT id, 0x1F FROM t
  2. Alt+Shift+F
  3. Run; the grid shows 0
Or format CREATE TABLE g (h geohash(5c), ts timestamp) timestamp(ts) → "invalid GEOHASH size".		 Fix the sql-parser lexer so hex, f/F-suffixed numbers, Nc/Nb geohash precision and Nns stay single tokens, then bump the package. Add a formatSql corpus test that compares input and output with whitespace removed. For the AI path, fall back to the raw SQL if that check fails. Keep the new formatter's fixes for !~, <<= and 1.5E+3, which base broke.
#2 Saved notebook grid layouts orphaned Persistence & migrations Moderate out-of-diff: notebookColumnLayoutStore.ts:10 → ResultGridPanel.tsx:49, GridShimmer.tsx:217. Broken contract: the persisted key is derived from formatter output Notebook users on 2.0.0–2.0.3: one-time layout reset Round-trip vitest with the real base save and head load: base finds 18/18, head 4/18. Cypress with a seeded store: base applies the widths and pin, head doesn't Layout keys hash normalizeSql output. The formatter swap changes that output for almost every query, so saved widths, column order and pins no longer load after upgrade. There is no migration or fallback lookup, and the old entries stay orphaned until LRU eviction.
  1. On 2.0.3, resize or pin columns in a notebook cell
  2. Upgrade
  3. Reopen the notebook: default layout
 Key on formatter-independent text (e.g. whitespace-collapsed SQL) so future bumps can't orphan layouts again. Either look up the legacy key once, or state the reset in the release notes.
#3 SQL review hides bidi characters and line tails Browser compat & security Moderate out-of-diff: share-link dialog Monaco/index.tsx:2648. Broken contract: LiteEditor render changed from a Monaco view to a static <pre> (LiteEditor/index.tsx:396, HighlightedSql/index.tsx:17-19,54) Share-link recipients can't see disguised writes Cypress, head vs base: base shows red [U+202E] markers and wraps long tokens; head shows neither (scrollWidth 1402 vs 516) colorize runs with control-character rendering off, so bidi characters reorder the displayed text. A crafted link can make DROP TABLE x look like it sits after --, and confirming runs the DROP. Long runs with no spaces no longer wrap, so a statement glued after them is hidden behind a faint horizontal scroll. Mitigations: the generic write-statement warning, keyword colouring, and the editor behind the dialog all remain.
  1. Open /?query=<SQL with /*U+202E U+2066*/ DROP TABLE t; …>&executeQuery=true
  2. Compare the dialog text with the real SQL
 Render U+202A–202E, U+2066–2069, U+200E/F, U+061C and C0 characters as visible markers in HighlightedSql. Add overflow-wrap: anywhere to its <pre>.
#4 Large SQL freezes AI chat Performance & rendering at scale Moderate out-of-diff: AIChatWindow/index.tsx:1043, ChatMessages.tsx:705,791, AssistantMarkdown.tsx:151. Broken contract: LiteEditor now renders every line AI users with multi-thousand-line statements: 1–6 s freezes Cypress timing, head vs base: 5k/10k/20k lines → 1.0/2.2/6.2 s longest frame gap at head; base flat at ~0.12 s maxHeight is now just CSS. Every line is colorized synchronously (Monaco's colorizer is quadratic in lines) and inserted into the DOM, where base Monaco rendered about 11 lines. This repeats on every chat reopen and theme switch.
  1. Paste a 10k-line INSERT
  2. Click its AI glyph: the page stalls about 2 s
 When maxHeight is set, colorize only the lines that fit, or render plain text above a line threshold.
#5 Finished row sticks, focus lost React correctness & hooks Moderate in-diff: useQueryActivity.ts:126-137, QueryActivityRow.tsx:275-310, queryActivity.ts:232 Keyboard and click users: finished rows never expire Cypress at 5bd1cfc3 (MutationObserver plus commit probe): row node replaced, focus on <body>, row still present after 12 s. Hover-only control expires normally. finished is updated in a passive effect, so the row unmounts for one commit and remounts as Finished. Focus inside it drops to <body>, and React 17 drops the blur, so the hold stays set. Held ids never expire and are never pruned. The same leak happens when a filter or an Escape-close unmounts a hovered row.
  1. Click a running row's elapsed or copy button
  2. Move the mouse away
  3. The query ends
  4. The Finished row stays until the user hovers it
 Build the finished rows in the same render as the snapshot (derive them synchronously). Release the hold on row unmount, and prune heldIds to ids that are still listed.
#6 Stale durations presented as current Query execution & data integrity Moderate in-diff: useQueryActivity.ts:51,69, queryActivity.ts:231 Operators during outages see inflated or previous-session data Cypress against the live server at 5bd1cfc3: a 3 s query shown Finished "21s"/"34s". On a failing reopen, a previous-session row showed "Running 39s" (really 12.7 s) with Cancel (a) Reopening during an outage never shows "Unable to load". The previous session's rows come back as Running, with durations inflated by the whole closed period, a stale badge and summary tiles, and only a "from the last successful response" banner. (b) A Finished row's duration runs to the next successful poll, not to when it was last seen. After auto-refresh off→on, or backoff, it shows start + gap, and sorts accordingly.
  1. Open, then close the drawer
  2. Make the server fail
  3. Reopen: old rows show as Running
Or: auto-refresh off, wait 20 s, turn it on → a 2 s query shows ~22 s.		 Reset lastReadyData per drawer session so a failed reopen shows the error. Freeze the Finished duration at the last-seen snapshot.
#7 Table Details shows "Unavailable" after one blip Cross-context caller impact Moderate in-diff policy change TableDetailsDrawer/index.tsx:260,306-327 (MANUAL policy plus deadline re-evaluation) affecting the unchanged DetailsTab.tsx Table Details users hit ~2 s false "Unavailable" Cypress, head vs base: head 2.05 s "Unavailable" with Copy DDL disabled; base ~1 s "Loading…", or data immediately One failed SHOW COLUMNS or DDL request now makes the source unavailable at once (grace 0). A policy switch re-judges earlier failures, and POLLING then needs two successes to recover.
  1. The first SHOW COLUMNS fails on open (Monitoring tab)
  2. Open Details: "Unavailable", Copy DDL disabled for ~2 s
 Don't re-judge counted failures when the policy changes, or keep POLLING for columns/DDL. Fetch immediately when Details becomes active.
#8 Long single-line query row unbounded Performance & rendering at scale Moderate in-diff: queryPreview.ts (line-count elision only), QueryActivityRow.tsx:365 (no maxHeight) Rows can reach ~8,000 px Cypress plus the live server: a real 24 KB query rendered as one 8,491 px row; a no-space literal gave a 144,000 px horizontal scroll Elision counts lines only, and the server doesn't truncate query text. Inlined literals or long expression chains fill the drawer for ~19 screens.
  1. Run SELECT '<20 KB text>' …
  2. Open Query Activity
 Also cap the preview by characters per line, or give the row's LiteEditor a maxHeight.
#9 Escape swallowed by non-clearing fields Accessibility & UX Moderate out-of-diff: Drawer/index.tsx:126,165 affects TableSelector, the CSV import Settings and Schema dialogs, and the main editor with any side drawer open Keyboard users: Escape does nothing in several contexts Cypress, head vs base: base closes on the first Esc; head stays open after three The guard treats every non-empty input or textarea as one that "clears itself", which is false outside the two search boxes. It matches read-only inputs, untouched prefilled fields, and Monaco's hidden textarea. Mixed impact: at base, Esc in the AI composer and import forms also discarded the draft, so head protects those.
  1. Open Table Details
  2. Shift+Tab to the table-name field
  3. Press Esc repeatedly: nothing happens
 Make clear-then-close opt-in (e.g. a data-clears-on-escape attribute) and fix the comment.
#10 Weak and missing Query Activity tests Test review & coverage Moderate in-diff: queryActivity.test.ts:183,258, queryActivity.spec.js Regressions in cancelled grading and error banners go unnoticed Mutation run: with the state === "cancelled" branch removed, 27/27 unit tests and the e2e logic still pass. Grep: zero tests for query-activity-stale and the cancel-error toast. Every cancelled fixture has no memory limit, so it grades "none" regardless of state. Cancelled rows with a limit are a real server state. The stale banner and the cancel-error toast (e.g. the Enterprise permission error) are untested. Remove the cancelled branch in classifyQuery; all tests stay green. Add a cancelled fixture with a 1 GiB limit and 900 MiB used. Add e2e tests for the stale banner (success, then 500) and for a cancel returning 400.
#11 Cancel dialog drops keyboard focus Accessibility & UX Moderate in-diff: CancelQueryDialog.tsx:18 Keyboard and screen-reader users lose their place Cypress (real keys) at 5bd1cfc3: activeElement is BODY after Keep or Escape The AlertDialog has no trigger, so Radix restores focus to nothing. The same pattern exists in older dialogs, but this instance is new.
  1. Tab to a row's Cancel button, press Enter
  2. Press Esc: focus is on <body>
 Save the opener when the dialog opens and focus it in onCloseAutoFocus.
#12 Dead fade constant, wrong comment Code structure, readability & types Minor in-diff: queryActivity.ts:54-58 Developer-facing; rows vanish ~0.7 s early N/A: static, grep shows no uses FINISHED_FADE_DELAY_MS is unused, and the comment describes a transition delay that QueryActivityRow.tsx:79 doesn't apply. — Apply it as transition-delay or delete it, and fix the comment.

Adjacent findings

None met the evidence bar.

Summary

emrberk and others added 2 commits September 24, 2026 17:08
@emrberk @claude
Resolve review findings #1, #3, #5 and #6
3decaea 
Bump @questdb/sql-parser to 0.1.19. The formatter no longer inserts
a space inside hex, float-suffixed, geohash-precision and nanosecond
literals, so Format Document, AI apply and Query Activity keep the
query intact (#1).

Render bidi and C0 control characters as visible markers in
HighlightedSql, and let the share-link review dialog scroll long
lines instead of hiding them (#3).

Derive Query Activity's finished rows in the same render as the
snapshot, so a row keeps its node and focus when the query ends, and
release a row's hold on unmount so held rows expire (#5).

Reset the Query Activity source on every open and close, so a failed
reopen shows the error state instead of the previous session, and
freeze a finished duration at the last poll that listed the query (#6).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@emrberk
show start time, not duration, show last updated indicator
42c3baa
@emrberk

emrberk commented Sep 24, 2026

Copy link
Copy Markdown
Member Author

Reviewing PR #612 at level 3 (head 42c3baad, base ec3b8bb6).

PR title and description

  • Title: repeat the verb: feat: add query activity drawer.
  • Claims the code does not meet:
    • "The drawer closes on the next [Escape]": false for fields that do not clear themselves (wip test for error range #1).
    • "A spinner appears … while a refresh is in flight": it never shows for Retry or the refresh after a cancel (run web console #8).
    • "The exact start time with a copy button": the time is cut to milliseconds (clean web console #7).

Issues

ID Issue Category Severity Location Net impact Evidence Description Steps to reproduce Suggested fix
#1 Escape swallowed in drawers Accessibility & UX Moderate out-of-diff: Drawer/index.tsx:126-171 affects AI chat, Import settings, Table Details, and the main editor Keyboard users: Escape never closes the drawer from a filled field Playwright, 3 presses per context: head stays open in all 4 contexts; base closes on the 1st press Any focused input or textarea with a value blocks Escape. The value never clears, so the next press does not close the drawer either. This includes Monaco's hidden textarea, a read-only title input, and prefilled import fields. The AI chat draft kept at head is the only real gain. 1) Open Table Details. 2) Click into a non-empty SQL editor. 3) Press Esc 3 times: the drawer stays open. Make clear-then-close opt-in, for example a data-clears-on-escape attribute on the two search inputs.
#2 Share-link dialog hides SQL Browser compatibility & security Moderate out-of-diff: Monaco/index.tsx:2651 (scrollable), HighlightedSql/utils.ts:1-10 Share-link recipients can confirm SQL they cannot see Playwright: at head, "DROP" is at x=3516 and the visible area is x=554–1046, with no scrollbar. Base wraps it into view. Zero-width characters are marked at base and not at head white-space: pre keeps text after a long run of spaces off-screen. U+200B/C/D, U+2060–2064 and U+00AD are no longer marked. The write warning and the plural title still show. 1) Open ?query=SELECT 1; + 400 spaces + DROP TABLE t;&executeQuery=true. 2) The dialog shows only SELECT 1;. Drop scrollable and use pre-wrap with overflow-wrap: anywhere. Add zero-width and format characters to CONTROL_CHARACTER_RANGES.
#3 Table Details "unavailable" after one blip Cross-context caller impact Moderate out-of-diff: TableDetailsDrawer/index.tsx:310-325 (MANUAL policy) affects the unchanged DetailsTab Table Details users: about 2 s of false "unavailable", Copy DDL disabled Playwright plus a state-machine unit test: head shows "unavailable" for 2.0 s; base shows "loading" for 1.0–1.2 s, then the data While Monitoring is active, 1 failed SHOW COLUMNS or DDL fetch makes the source unavailable at once. On Details, the source needs 2 successful polls to recover. 1) The first SHOW COLUMNS fetch fails. 2) Click the Details tab: "unavailable" for about 2 s. Keep POLLING for columns and DDL, or fetch at once when Details becomes active.
#4 Saved notebook grid layouts lost Persistence & migrations Moderate out-of-diff: notebookColumnLayoutStore.ts:10 → ResultGridPanel.tsx:49, GridShimmer.tsx:217 (the key comes from formatter output) Notebook users upgrading from 2.0.x: one-time layout reset Vitest round-trip with the real base and head stores: 10 of 12 keys differ; a base save gives null on a head load The formatter swap changes the key for almost every query. Saved widths, order and pins no longer load. No fallback exists. 1) On 2.0.3, resize a notebook grid column. 2) Upgrade: the default layout returns. Use formatter-independent text as the key (whitespace collapsed), or look up the legacy key once. At the least, note it in the release notes.
#5 Finished row never expires React correctness & hooks Moderate in-diff: QueryActivityRow.tsx:307-312, 343-348 Mouse users who copy the start time Playwright: the row stays Finished at +5/15/30/45 s; focus is on BODY Copy in the portaled tooltip sets focused=true. The tooltip then unmounts, and React 17 drops the blur, so the hold is never released. 1) Hover "Started … ago", then click copy. 2) Move the mouse away. 3) The query ends: the row stays. Release the hold on the row's focusout with a native listener, or track focus with :focus-within when the pointer leaves.
#6 Long single-line query row Performance & rendering at scale Moderate in-diff: queryPreview.ts:20, QueryActivityRow.tsx:365 Rows about 6 screens tall for queries with big literals Playwright: a real 17 KB query gives a 6,371 px row; a literal with no spaces gives a 144,520 px scroll The preview counts lines only, and the server does not truncate query. 1) Run a query with a 17 KB string literal. 2) Open Query Activity. Also cap characters per line in buildQueryPreview, or give the row's LiteEditor a maxHeight.
#7 Start-time tooltip loses data and access Accessibility & UX Moderate in-diff: QueryActivityRow.tsx:268, 343-361 All rows: time cut to ms; copy only with a mouse Playwright: server …58.409078Z, clipboard …58.409Z. Enter/Space/tap closes the tooltip; Tab never reaches copy new Date().toISOString() drops microseconds, although formatUtcTimestamp keeps them. The plain Tooltip closes on activation. MonitoringTab already uses useToggletip for the same pattern. 1) Hover the start time and copy it. 2) Or press Enter on it: the tooltip closes. Use formatUtcTimestamp and useToggletip.
#8 Manual refresh shows no spinner Async, timers & cancellation Moderate in-diff: useQueryActivity.ts:139-169 Users who click Retry or cancel on slow servers get no feedback Playwright, 2 runs: a slow poll shows aria-busy=true; a slow Retry and a slow post-cancel refresh stay false for 2.8 s isFetching is one boolean. The restarted poll returns "skipped", or fetchNow aborts it, and its finally clears the flag. 1) Auto refresh on, and the listing is slow. 2) Click Retry: no spinner. Use a counter, or track only fetchNow. Do not bump pollGeneration in refresh.
#9 Cancel dialog drops focus Accessibility & UX Moderate in-diff: CancelQueryDialog.tsx:17-28 Keyboard users lose their place Playwright with real keys: after Keep or Esc, focus is on BODY; the next Tab leaves the drawer The dialog has no Trigger, so Radix returns focus to nothing. 1) Tab to Cancel, press Enter. 2) Press Esc. Store the opener and focus it in onCloseAutoFocus.
#10 Cancel, then reopen: endless loading Async, timers & cancellation Moderate in-diff: index.tsx:149-150, useCatalogSource.ts:132-136 Auto refresh off and slow server: rare, but no recovery path Playwright with a delayed CANCEL: loading from +1 s to +20 s. Control run without the delay loads normally The refresh from the earlier render aborts the reopened drawer's request. Its own response is then skipped. With auto refresh off, the loading state has no Retry button. 1) Auto refresh off, then confirm Cancel. 2) Close and reopen the drawer before CANCEL returns. Read refresh through a ref, or skip it when the session generation changed.
#11 Capitalize setting stale across tabs State & context architecture Moderate in-diff: formatSql.ts:7-12 Users with several tabs: the toggle does not match the format result Playwright, 2 tabs: B formats in uppercase while its switch shows off; saving B turns the setting off everywhere formatSql reads localStorage directly, while the provider reads the value once. At base, settings took effect through provider state, so a stale tab still matched its own UI. 1) Tab A: turn capitalize on. 2) Tab B: format, then save another setting. Pass capitalize from useLocalStorage to the formatter callers, or listen for storage events.
#12 Weak and missing tests Test review & coverage Moderate in-diff: queryActivity.test.ts, HighlightedSql/utils.test.ts, queryActivity.spec.js, editor.spec.js Regressions in grading, security markers and error states pass CI Mutation runs on scratch copies: removing the cancelled branch passes 26/26; removing U+202A–D, U+2067–8, U+200F or C1 passes 4/4. Grep finds no test for query-activity-stale and no check on the dialog SQL text Cancelled fixtures have no memory limit. Bidi fixtures test only range endpoints. The stale banner and the share-link SQL text have no test. Remove the cancelled branch in classifyQuery; the tests stay green. Add a cancelled row that has a limit, a test for each range, e2e for success then 500, and a check on the dialog SQL text.
#13 Small code cleanups Code structure, readability & types Minor in-diff Developer-facing only N/A, static: grep and source lines FINISHED_FADE_DELAY_MS is unused, and its comment is wrong. A second escapeHtml repeats utils/escapeHtml.ts. describeMemoryStatus does not use row. Inline style at QueryActivityRow.tsx:346. 1.2rem instead of theme.fontSize.xs. The PR adds 7 comments. The search input has no aria-label. — Delete or apply the constant. Reuse the existing escapeHtml. Remove the unused parameter. Use theme tokens. Remove the non-critical comments.

Adjacent findings

None met the evidence bar.

Summary

@puzpuzpuz

puzpuzpuz commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Reviewing PR #612 at level 3 (head 42c3baad, base ec3b8bb6).

Verdict: approve with comments. No finding reaches Critical, and typecheck, lint, unit tests (99 files, 2,161 tests) and build all pass. There are 10 Moderate findings and 1 Minor. Please fix #1–#4 before merge: they change existing behaviour for users who never open the new drawer. The rest can follow.

Every behavioural finding below was reproduced by a separate agent that first tried to disprove it, with the same steps run at base.

PR title and description

  • Title: repeat the verb: feat: add query activity drawer.
  • Claims the code doesn't meet:

Issues

ID Issue Category Severity Location Net impact Evidence Description Steps to reproduce Suggested fix
#1 Formatter breaks some valid SQL Query execution & data integrity Moderate in-diff src/utils/formatSql.ts:1-14; reaches unchanged Format Document (questdb-sql/formatting.ts) and AI apply (EditorProvider:829, aiAssistant.ts:562) Backtick or geohash-plus-comment SQL breaks after Format Document Cypress Format Document plus live server, both revisions. Scratch vitest fails at head, passes at base The new formatter doesn't treat backtick-quoted identifiers as single tokens. `my col` becomes `my col` ("Invalid column"). A backtick-quoted keyword like `from` gets a newline inserted into it. If both `a b` and `a b` exist, it silently reads the other column. Separately, #3c4nv/* c */ becomes #3c4nv/ * c */ ("missing bits size for GEOHASH constant"). Base kept both. Undo recovers 1) select `my col` from t 2) Shift+Alt+F 3) Run: error Fix the @questdb/sql-parser lexer: make backtick identifiers atomic, and stop the geohash pattern from swallowing the / of /*. Add a formatSql corpus test asserting quoted spans and comments survive
#2 Escape stops closing side drawers Accessibility & UX Moderate out-of-diff: Drawer/index.tsx:126-171 affects side drawers used next to the editor, Table Details, and the Search panel. Broken contract: Escape dismisses a side drawer Anyone editing SQL with a side drawer open Cypress, real keys: 4× Escape keeps it open at head; base closes on the first press. An empty editor still closes The guard treats any input or textarea with a value as "owns Escape". That includes Monaco's hidden textarea, which holds the editor text, so Escape in a non-empty editor or notebook cell never closes Table Details, Query Activity, News or AI chat. It also blocks on the read-only table-name input and on native checkboxes (value "on"), even outside the drawer. None of these hold anything that needs protecting. Gains to keep: the AI draft, import forms, and Escape to dismiss autocomplete no longer closes the drawer 1) Open Table Details 2) Click into a non-empty editor 3) Press Escape repeatedly: the drawer stays open Guard only editable text fields: textarea or input type text/search, not readOnly, not inside .monaco-editor, and preferably only inside the drawer
#3 Table Details shows "Unavailable" after one failure Cross-context caller impact Moderate out-of-diff: TableDetailsDrawer/index.tsx:311-325 (MANUAL policy, and the deadline effect re-runs when the policy changes) affects unchanged DetailsTab ~2 s of false "Unavailable" and Copy DDL disabled Cypress with one 500 injected: ~2.0 s "Unavailable" at head vs ~1 s "Loading…" or 0 s at base While Monitoring is active, one failed SHOW COLUMNS or DDL request marks the source unavailable at once. Switching tabs re-judges an earlier failure under the stricter policy. Details then needs 2 successful polls to recover. Timeouts behave the same at base 1) The first SHOW COLUMNS fails on open 2) Click Details: "Unavailable" and disabled Copy DDL for ~2 s Keep POLLING_RETRY_POLICY for columns and DDL on both tabs (Monitoring never renders them), or fetch immediately when Details becomes active
#4 Saved notebook grid layouts lost on upgrade Persistence & migrations Moderate out-of-diff: notebookColumnLayoutStore.ts:10 affects ResultGridPanel.tsx:49, GridShimmer.tsx:217. Broken contract: saved key is stable across upgrade Users of 2.0.0–2.0.3: one-time layout reset Vitest: base save, head load gives 4 hits and 48 misses of 52. Cypress: base-key layout applied at base, ignored at head The key hashes the formatter's output, which changed for almost every query. Saved widths, column order and pins stop applying. There is no fallback, and the orphaned entries hold slots in each cell's 20-entry cache 1) On 2.0.3, resize or pin notebook grid columns 2) Upgrade 3) Default layout Key on formatter-independent text (e.g. whitespace-collapsed SQL) so this can't recur, and mention the one-time reset in the release notes
#5 Share-link dialog can hide a statement Browser compatibility & security Moderate in-diff: Monaco/index.tsx:2651 (scrollable) plus the HighlightedSql pre mode Crafted share links can hide a DROP sideways Cypress: SELECT 1; + 200 spaces + DROP …. DROP is off-screen at head; wrapped into view at base With white-space: pre, padding or a long line pushes a later write statement out of view. The only cue is a thin horizontal scrollbar under a normal-looking line. Base wrapped the same SQL in full. Hiding it below the fold with newlines was already possible at base. The warning and plural title are unchanged Open /?query=SELECT 1;<200 spaces>DROP TABLE x;&executeQuery=true Drop scrollable. Add overflow-wrap: anywhere to the wrapping mode, which covers the long-token case scrollable was added for
#6 Long one-line query fills the drawer Performance & rendering at scale Moderate in-diff: queryPreview.ts:19-20 (counts lines only), QueryActivityRow.tsx:365 (no maxHeight) One row can be ~9 drawer heights tall Live server: an 18 KB literal query rendered one row 6,551 px tall; a multi-line IN-list was elided to 311 px The server returns the full query text, and the formatter keeps literals, ARRAY[...] and argument lists on one line. The preview never elides these, so one query pushes every other row many screens down Run a long query containing a ~20 KB string literal, then open Query Activity Pass maxHeight to the row's LiteEditor (it already clips and shows the toolbar), or cap characters per line
#7 Finished row never expires React correctness & hooks Moderate in-diff: QueryActivityRow.tsx:275-311 Keyboard users and users who copy the start time Cypress: row still "Finished" after 16 s; hovering again doesn't clear it; the no-focus control expires The hold is cleared only by onBlur. React 17 drops the blur when the focused element (the tooltip's Copy button, or Cancel when the query ends) is removed, so the hold stays set and the grace period never starts 1) Hover "Started … ago" and click Copy 2) Move away 3) The query ends: the row stays On mouse leave and on phase change, recompute focus as rowRef.current?.contains(document.activeElement) and release the hold if false
#8 Reopened drawer stuck loading after a cancel Async, timers & cancellation Moderate in-diff: index.tsx:144-151 stale refresh, useCatalogSource.ts:132-136 Auto-refresh-off users on slow or overloaded servers Cypress with a delayed CANCEL: "Loading…" for 15 s with no Retry. With auto refresh on it recovers in ~1 s. A saturated server delayed CANCEL by 5.6 s refresh() after the await is from the old render. Its fetchNow aborts the reopened session's request and then sends one under the old key, which is discarded. The machine never leaves loading. Closing and reopening again recovers 1) Auto refresh off; confirm Cancel 2) Close and reopen before CANCEL returns Call the latest refresh through a ref, or skip it when the drawer session changed during the await
#9 Cancel dialog reappears stale and drops focus Accessibility & UX Moderate in-diff: index.tsx:66-68,258, CancelQueryDialog.tsx:17 Keyboard users; rare stale re-prompt after a resize Cypress: after a resize-close, reopening shows "Cancel query 62179?" over an empty list. Keep, Escape and Confirm all leave focus on <body> cancelTarget survives a close that doesn't go through the dialog, so the confirm returns later for an old id. The dialog has no trigger, so focus falls to <body> on every close. The chat-history delete dialog already behaves this way at base 1) Click Cancel 2) Shrink the window below 1280 px 3) Reopen: dialog is back Clear cancelTarget when the drawer closes, and focus the opener in onCloseAutoFocus
#10 Weak and missing tests Test review & coverage Moderate in-diff: queryActivity.test.ts, HighlightedSql/utils.test.ts, formatSql.test.ts, queryActivity.spec.js Regressions in grading, bidi markers and formatter pass CI Mutation runs at 42c3baa: removing the cancelled branch passes 26/26; dropping U+202A–D and U+2067–8 passes 4/4. A grep finds 0 tests for query-activity-stale and the cancel error toast Cancelled fixtures have no memory limit. Control-character tests sample only range ends, yet these markers are the share-link dialog's safety net. Formatter tests cover case and semicolons only, which is why #1 slipped through Remove row.state === "cancelled" in classifyQuery: tests stay green Add a cancelled row with a limit, a per-range code-point test, e2e for success-then-500 and a failed cancel, and a formatter token corpus
#11 Dead code and wrong comments Code structure, readability & types Minor in-diff: queryActivity.ts:53-57,274, HighlightedSql/utils.ts:22, index.tsx:174-182 Developer-facing; small timing and flash differences N/A, static: grep plus source FINISHED_FADE_DELAY_MS is unused, and its comment describes a delay that isn't applied, so rows vanish at ~4.3 s. Under prefers-reduced-motion there is no transitionend, so the row goes invisible but keeps its space until expiry. There is a second escapeHtml (utils/escapeHtml.ts exists), an unused row parameter, and unused label maps. The loading panel ignores useDelayedFlag's 500 ms minimum — Delete or apply the constant, handle reduced motion, reuse utils/escapeHtml, remove the unused code, and honour showLoading

Adjacent findings (not blocking; pre-existing at base)

  • "Create materialized view" fails for tables with keyword-named columns.
    • Net impact: tables with columns like sample; the feature fails with a toast.
    • Location: src/utils/generateMatViewDDL.ts:543 parsing the SHOW CREATE TABLE output.
    • Symptom: "Failed to generate materialized view DDL: Parse error … 'sample'".
    • Reachability: SHOW CREATE TABLE returns keyword column names unquoted, and the parser rejects them. Proven at base with a "sample" column. subsample joins this set once the server reserves it (QuestDB #7013).
    • Fix: quote keyword identifiers before parsing, or have SHOW CREATE TABLE quote them.
    • Severity if filed standalone: Moderate.
  • Saving Editor Settings in a stale tab reverts other tabs' changes.
    • Net impact: multi-tab users; a setting silently reverts.
    • Location: EditorSettingsModal/index.tsx (Save writes every draft) plus LocalStorageProvider (no storage listener).
    • Symptom: change "run with selection" in tab A, save anything in tab B, and A's change is undone.
    • Reachability: executed at base.
    • Fix: listen for storage events in the provider, or write only fields that changed.
    • Severity if filed standalone: Minor.

Summary

@puzpuzpuz
puzpuzpuz self-requested a review September 28, 2026 10:08
@puzpuzpuz puzpuzpuz added the enhancement New feature or request label Sep 28, 2026
@emrberk
emrberk merged commit ef328c2 into main Sep 28, 2026
5 of 6 checks passed
@emrberk
emrberk deleted the feat/query-activity-drawer branch September 28, 2026 11:06
@emrberk emrberk mentioned this pull request Sep 30, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@puzpuzpuz puzpuzpuz puzpuzpuz approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@emrberk @puzpuzpuz