From 30f552d4585a2159cf7616a8818b0aa191f6af7d Mon Sep 17 00:00:00 2001 From: emrberk Date: Mon, 6 Jul 2026 12:04:42 +0300 Subject: [PATCH 1/5] feat: editor settings, remove readonly from single query run, smarter grid column widths --- e2e/questdb | 2 +- e2e/tests/console/editor.spec.js | 305 ++++++++++++ e2e/tests/console/grid.spec.js | 119 ++++- src/components/EditorSettingsModal/index.tsx | 238 +++++++++ src/components/FixQueryButton/index.tsx | 20 +- src/components/ResultGrid/ResultGrid.tsx | 42 +- src/components/ResultGrid/dimensions.ts | 19 + .../ResultGrid/inlineGridUtils.test.ts | 156 +++++- src/components/ResultGrid/inlineGridUtils.ts | 285 +++++++++-- src/components/ResultGrid/styles.ts | 29 +- src/components/ResultGrid/types.ts | 2 + .../ResultGrid/useContainerWidth.ts | 25 +- src/components/ResultGrid/useFontsReady.ts | 26 + src/components/Switch/index.tsx | 6 + src/consts/shared-definitions.sync.test.ts | 25 - src/providers/LocalStorageProvider/index.tsx | 155 ++++-- src/providers/LocalStorageProvider/types.ts | 4 + .../LocalStorageProvider/utils.test.ts | 55 +++ src/providers/LocalStorageProvider/utils.ts | 21 + src/scenes/Editor/Monaco/index.tsx | 366 +++++++++----- src/scenes/Editor/Monaco/tabs.tsx | 12 + src/scenes/Editor/Monaco/utils.test.ts | 454 ++++++++++++++++++ src/scenes/Editor/Monaco/utils.ts | 89 +++- src/scenes/Editor/Notebook/cells/Cell.tsx | 12 +- .../Notebook/result-table/ResultGridPanel.tsx | 3 + src/scenes/Editor/utils.test.ts | 107 +++++ src/scenes/Result/ResultGridAdapter.tsx | 3 + src/store/Query/reducers.test.ts | 150 ++++++ src/store/Query/reducers.ts | 15 +- src/utils/localStorage/types.ts | 2 + .../questdb/queryExecutionManager.test.ts | 68 +++ src/utils/questdb/queryExecutionManager.ts | 11 + 32 files changed, 2523 insertions(+), 303 deletions(-) create mode 100644 src/components/EditorSettingsModal/index.tsx create mode 100644 src/components/ResultGrid/useFontsReady.ts delete mode 100644 src/consts/shared-definitions.sync.test.ts create mode 100644 src/providers/LocalStorageProvider/utils.test.ts create mode 100644 src/scenes/Editor/utils.test.ts create mode 100644 src/store/Query/reducers.test.ts diff --git a/e2e/questdb b/e2e/questdb index b2806e98e..b7473a8b6 160000 --- a/e2e/questdb +++ b/e2e/questdb @@ -1 +1 @@ -Subproject commit b2806e98ee8df0f07fb5e7c6ea7b6b6ba860aca7 +Subproject commit b7473a8b639cbd024821abf12e98d54f29aa0476 diff --git a/e2e/tests/console/editor.spec.js b/e2e/tests/console/editor.spec.js index b49ef326a..3526b3bce 100644 --- a/e2e/tests/console/editor.spec.js +++ b/e2e/tests/console/editor.spec.js @@ -2086,3 +2086,308 @@ describe("import/export tabs", () => { cy.getByDataHook("import-summary-close").click() }) }) + +describe("editor settings", () => { + const runWithSelectionTable = "e2e_run_with_selection" + + beforeEach(() => { + cy.loadConsoleWithAuth() + cy.getEditorContent().should("be.visible") + cy.clearEditor() + cy.execQuery(`DROP TABLE IF EXISTS ${runWithSelectionTable}`) + cy.execQuery( + `CREATE TABLE ${runWithSelectionTable} AS (SELECT x a, x b, x c FROM long_sequence(1))`, + ) + }) + + afterEach(() => { + cy.execQuery(`DROP TABLE IF EXISTS ${runWithSelectionTable}`) + }) + + const openEditorSettings = () => { + cy.getByDataHook("editor-tabs-menu-button").click() + cy.getByDataHook("editor-tabs-menu-settings").click() + cy.getByDataHook("editor-settings-modal").should("be.visible") + } + + it("toggles 'Run with selection' from the modal and persists it", () => { + // Given the setting defaults to on + openEditorSettings() + cy.getByDataHook("editor-settings-run-with-selection").should( + "have.attr", + "aria-checked", + "true", + ) + + // When it is switched off and saved + cy.getByDataHook("editor-settings-run-with-selection").click() + cy.getByDataHook("editor-settings-save").click() + cy.getByDataHook("editor-settings-modal").should("not.exist") + + // Then the choice is persisted to local storage + cy.window() + .its("localStorage") + .invoke("getItem", "editor.runWithSelection") + .should("eq", "false") + + // And reopening the modal shows it still off + openEditorSettings() + cy.getByDataHook("editor-settings-run-with-selection").should( + "have.attr", + "aria-checked", + "false", + ) + cy.getByDataHook("editor-settings-cancel").click() + cy.getByDataHook("editor-settings-modal").should("not.exist") + }) + + it("runs the selected fragment when enabled and the whole query once disabled", () => { + const table = runWithSelectionTable + const query = `select a from ${table}` + const startColumn = query.indexOf(table) + 1 + const endColumn = startColumn + table.length + + // Given a query selecting one column, with the table name highlighted + cy.typeQueryDirectly(query) + cy.selectRange( + { lineNumber: 1, column: startColumn }, + { lineNumber: 1, column: endColumn }, + ) + + // When 'Run with selection' is enabled (default), the selection runs + cy.getByDataHook("button-run-query").should("contain", "Run selected query") + cy.clickRunQuery() + // Then the bare table name returns every column + cy.get("[data-hook='grid-header-name']").should("have.length", 3) + + // When 'Run with selection' is turned off + openEditorSettings() + cy.getByDataHook("editor-settings-run-with-selection").click() + cy.getByDataHook("editor-settings-save").click() + + // Then the run button reflects the new setting without any further + // editor interaction + cy.getByDataHook("button-run-query").should("contain", "Run query") + + // And the same fragment is selected and run again + cy.selectRange( + { lineNumber: 1, column: startColumn }, + { lineNumber: 1, column: endColumn }, + ) + cy.getByDataHook("button-run-query").should("contain", "Run query") + cy.clickRunQuery() + // Then the whole cursor query runs, returning only the single column + cy.get("[data-hook='grid-header-name']").should("have.length", 1) + }) + + it("runs the share-link selection even when 'Run with selection' is off", () => { + // Given the setting is turned off + openEditorSettings() + cy.getByDataHook("editor-settings-run-with-selection").click() + cy.getByDataHook("editor-settings-save").click() + + // And a buffer whose statement contains the shared fragment + cy.typeQueryDirectly("select 1 union all select 2;") + + // When a share link for the fragment auto-runs + cy.visit( + `${baseUrl}?query=${encodeURIComponent("select 2")}&executeQuery=true`, + ) + cy.getEditorContent().should("be.visible") + + // Then only the selected fragment runs, not the containing statement + cy.getGridRows().should("have.length", 1) + cy.getGridRow(0).should("contain", "2") + }) + + it("caps column width at the configured maximum and returns to auto", () => { + const cell = () => + cy.get("[data-hook='result-grid-tanstack']:visible #cell-0-0") + + // Given a result with a very wide column under automatic sizing + cy.typeQueryDirectly( + "select string_agg(x::string, ',') s from long_sequence(500)", + ) + cy.clickRunQuery() + cell().should("be.visible") + + // When a width below the minimum is saved + openEditorSettings() + cy.getByDataHook("editor-settings-max-column-width").clear().type("10") + cy.getByDataHook("editor-settings-save").click() + + // Then the modal stays open and shows a validation error + cy.getByDataHook("editor-settings-modal").should("be.visible") + cy.getByDataHook("editor-settings-max-column-width-error").should( + "contain", + "between 60 and 4000", + ) + + // When a valid maximum column width is saved + cy.getByDataHook("editor-settings-max-column-width").clear().type("250") + cy.getByDataHook("editor-settings-max-column-width-error").should( + "not.exist", + ) + cy.getByDataHook("editor-settings-save").click() + cy.getByDataHook("editor-settings-modal").should("not.exist") + + // Then the open result reflows and the column settles at the cap + cell().should(($cell) => { + expect($cell[0].getBoundingClientRect().width).to.be.closeTo(250, 3) + }) + + // And reopening the modal shows the stored value + openEditorSettings() + cy.getByDataHook("editor-settings-max-column-width").should( + "have.value", + "250", + ) + + // When the field is cleared back to auto and saved + cy.getByDataHook("editor-settings-max-column-width").clear() + cy.getByDataHook("editor-settings-save").click() + + // Then the column fills the grid again + let gridWidth + cy.get("[data-hook='result-grid-tanstack']:visible").then(($grid) => { + gridWidth = $grid[0].getBoundingClientRect().width + }) + cell().should(($cell) => { + expect($cell[0].getBoundingClientRect().width).to.be.closeTo( + gridWidth, + 20, + ) + }) + }) +}) + +describe("editing while a query is running", () => { + beforeEach(() => { + cy.loadConsoleWithAuth() + cy.getEditorContent().should("be.visible") + cy.clearEditor() + cy.intercept("/exec*", (req) => { + req.on("response", (res) => { + res.setDelay(2000) + }) + }) + }) + + it("keeps the editor editable in flight and still lands the result", () => { + // Given a running query + cy.typeQuery("select 1") + cy.clickRunIconInLine(1) + cy.getCancelIconInLine(1).should("be.visible") + + // Then the editor is not read-only and can be shifted mid-flight + cy.window().then((win) => { + const editor = win.monaco.editor.getEditors()[0] + expect(editor.getOption(win.monaco.editor.EditorOption.readOnly)).to.eq( + false, + ) + + const before = editor.getValue() + editor.setPosition({ lineNumber: 1, column: 1 }) + editor.trigger("test", "type", { text: "-- shifted while running\n" }) + expect(editor.getValue()).to.not.eq(before) + expect(editor.getValue()).to.contain("-- shifted while running") + }) + + // And the running glyph follows the query to its new line + cy.getCancelIconInLine(2).should("be.visible") + + // When the query completes, its result lands in the query log + cy.getByDataHook("success-notification").should("contain", "select 1") + cy.expandNotifications() + cy.getExpandedNotifications().should("contain", "select 1") + + // And the success glyph sits on the query's shifted line + cy.get(".glyph-widget-2 .glyph-run-icon.success").should("be.visible") + }) + + it("drops the glyphs but still lands the result when the running query is edited", () => { + // Given a running query + cy.typeQuery("select 1") + cy.clickRunIconInLine(1) + cy.getCancelIconInLine(1).should("be.visible") + + // When text is typed inside the running query, dislodging it + cy.window().then((win) => { + const editor = win.monaco.editor.getEditors()[0] + editor.setPosition({ lineNumber: 1, column: 8 }) + editor.trigger("test", "type", { text: "x" }) + expect(editor.getValue()).to.contain("select x1") + }) + + // Then no line claims the running glyph anymore + cy.get(".glyph-run-icon.cancel").should("not.exist") + + // When the query completes, its result still lands in the query log + cy.getByDataHook("success-notification").should("contain", "select 1") + + // And no success glyph is drawn for the edited query + cy.get(".glyph-run-icon.success").should("not.exist") + }) + + it("keeps the cursor where the user types when the query fails after an edit", () => { + // Given a running query that will fail + cy.typeQuery("select * from non_existent_table") + cy.clickRunIconInLine(1) + cy.getCancelIconInLine(1).should("be.visible") + + // When a comment line is typed above it while it runs + cy.window().then((win) => { + const editor = win.monaco.editor.getEditors()[0] + editor.setPosition({ lineNumber: 1, column: 1 }) + editor.trigger("test", "type", { text: "-- typing during the run\n" }) + editor.setPosition({ lineNumber: 1, column: 10 }) + }) + + // Then the error lands in the query log + cy.getByDataHook("error-notification").should( + "contain", + "table does not exist", + ) + + // And the error glyph sits on the query's shifted line + cy.get(".glyph-widget-2 .glyph-run-icon.error").should("be.visible") + + // And the cursor stays where the user was typing instead of jumping + // into the failed query + cy.window().then((win) => { + const position = win.monaco.editor.getEditors()[0].getPosition() + expect(position.lineNumber).to.eq(1) + expect(position.column).to.eq(10) + }) + }) + + it("supersedes the running query when a second run is confirmed, landing only the new result", () => { + // Given a first query that is still running + cy.typeQuery("select 1 a;{enter}select 2 b;") + cy.clickRunIconInLine(1) + cy.getCancelIconInLine(1).should("be.visible") + + // When a second query is run while the first is in flight + cy.clickRunIconInLine(2) + + // Then the console asks to cancel the running query, and confirming + // supersedes it + cy.getByDataHook("abort-confirmation-dialog").should("be.visible") + cy.getByDataHook("abort-confirmation-dialog-confirm").click() + + // Then only the second query's result lands in the query log + cy.get('[data-hook="success-notification"]', { timeout: 10000 }).should( + "contain", + "select 2 b", + ) + + // And the superseded first query never lands a result of its own + cy.get('[data-hook="success-notification"]').should( + "not.contain", + "select 1 a", + ) + + // And no query is left running + cy.get(".glyph-run-icon.cancel").should("not.exist") + }) +}) diff --git a/e2e/tests/console/grid.spec.js b/e2e/tests/console/grid.spec.js index 6bd26d798..f32ea0944 100644 --- a/e2e/tests/console/grid.spec.js +++ b/e2e/tests/console/grid.spec.js @@ -398,8 +398,10 @@ describe("questdb grid", () => { it("keeps the frozen column pinned while scrolling horizontally", () => { // Given a result wide enough to scroll horizontally, left column frozen - const columns = Array.from({ length: 20 }, (_, i) => `x c${i}`).join(", ") - cy.typeQuery(`select ${columns} from long_sequence(10)`) + const columns = Array.from({ length: 100 }, (_, i) => `x c${i}`).join( + ", ", + ) + cy.typeQueryDirectly(`select ${columns} from long_sequence(10)`) cy.runLine() cy.gridToolbar("freeze").click() cy.getGridCellAt(0, 0).should("have.attr", "data-frozen", "true") @@ -652,4 +654,117 @@ describe("questdb grid", () => { cy.getGridRows().should("have.length.greaterThan", 0) }) }) + + describe("column width allocation", () => { + beforeEach(() => { + cy.getEditorContent().should("be.visible") + cy.clearEditor() + }) + + const longArray = (n) => + `ARRAY[${Array.from({ length: n }, (_, i) => `${i + 1}.0`).join(",")}]` + + const gridArea = () => cy.get('[data-hook="result-grid-tanstack"]:visible') + + const cellIn = (col) => gridArea().find(`#cell-0-${col}`) + + const runQuery = (query) => { + cy.typeQueryDirectly(query) + cy.clickRunQuery() + cellIn(0).should("be.visible") + } + + it("S1: a single wide column fills the grid width", () => { + // Given a result with one very long column + runQuery("select string_agg(x::string, ',') s from long_sequence(500)") + + // Then it stretches to the full grid width + let gridWidth + gridArea().then(($grid) => { + gridWidth = $grid[0].getBoundingClientRect().width + }) + cellIn(0).then(($cell) => { + expect($cell[0].getBoundingClientRect().width).to.be.closeTo( + gridWidth, + 20, + ) + }) + }) + + it("S2: two wide columns split the grid width evenly and the array is truncated", () => { + // Given a result with two very wide columns + runQuery( + `select string_agg(x::string, ',') s, ${longArray(300)} arr from long_sequence(500)`, + ) + + // Then each column takes half of the grid width + let gridWidth + gridArea().then(($grid) => { + gridWidth = $grid[0].getBoundingClientRect().width + }) + let firstWidth + cellIn(0).then(($cell) => { + firstWidth = $cell[0].getBoundingClientRect().width + }) + cellIn(1).then(($cell) => { + const secondWidth = $cell[0].getBoundingClientRect().width + expect(firstWidth).to.be.closeTo(secondWidth, 8) + expect(firstWidth).to.be.closeTo(gridWidth / 2, 20) + }) + + // And the clipped array ends with an ellipsis before its closing bracket + cellIn(1) + .invoke("text") + .then((text) => { + expect(text.trim()).to.match(/\.\.\.]$/) + }) + }) + + it("S3: narrow columns keep their width and the array takes the remaining space", () => { + // Given a few narrow columns and one wide array column + runQuery(`select 1, 2, 3, ${longArray(300)} arr`) + + let gridWidth + gridArea().then(($grid) => { + gridWidth = $grid[0].getBoundingClientRect().width + }) + + // Then the narrow columns settle at one small, shared width + let firstDigitWidth + let digitsTotal = 0 + for (let col = 0; col < 3; col++) { + cellIn(col).then(($cell) => { + const width = $cell[0].getBoundingClientRect().width + if (col === 0) firstDigitWidth = width + else expect(width).to.be.closeTo(firstDigitWidth, 8) + digitsTotal += width + }) + } + + // And the array column takes exactly what the narrow columns leave behind + cellIn(3).then(($cell) => { + expect($cell[0].getBoundingClientRect().width).to.be.closeTo( + gridWidth - digitsTotal, + 20, + ) + }) + }) + + it("S4: the array column stops at the 400px cap when space is tight", () => { + const digits = Array.from({ length: 100 }, (_, i) => i + 1).join(",") + + // Given more narrow columns than the width can hold, plus a wide array + runQuery(`select ${digits}, ${longArray(300)} arr`) + + // When the far-right array column is scrolled into view + gridArea().find('[data-hook="grid-viewport"]').scrollTo("right") + + // Then it settles at the maximum column width + cellIn(100) + .should("be.visible") + .then(($cell) => { + expect($cell[0].getBoundingClientRect().width).to.be.closeTo(400, 3) + }) + }) + }) }) diff --git a/src/components/EditorSettingsModal/index.tsx b/src/components/EditorSettingsModal/index.tsx new file mode 100644 index 000000000..5435779b4 --- /dev/null +++ b/src/components/EditorSettingsModal/index.tsx @@ -0,0 +1,238 @@ +import React, { ReactNode, useRef, useState } from "react" +import styled from "styled-components" +import { Dialog } from "../Dialog" +import { Overlay } from "../Overlay" +import { ForwardRef } from "../ForwardRef" +import { Text } from "../Text" +import { Button } from "../Button" +import { Switch } from "../Switch" +import { Input } from "../Input" +import { useLocalStorage } from "../../providers/LocalStorageProvider" +import { + isMaxColumnWidthDraftValid, + parseMaxColumnWidth, +} from "../../providers/LocalStorageProvider/utils" +import { MAX_COLUMN_WIDTH_BOUNDS } from "../ResultGrid/dimensions" +import { StoreKey } from "../../utils/localStorage/types" + +type Props = { + open: boolean + onOpenChange: (open: boolean) => void +} + +const StyledContent = styled(Dialog.Content).attrs({ + maxwidth: "48rem", +})` + display: flex; + flex-direction: column; +` + +const Form = styled.form` + display: contents; +` + +const Body = styled.div` + display: flex; + flex-direction: column; + gap: 2rem; + padding: 2rem; +` + +const ItemRow = styled.div` + display: flex; + align-items: flex-start; + justify-content: space-between; + gap: 2rem; +` + +const ItemText = styled.div` + display: flex; + flex-direction: column; + gap: 0.4rem; +` + +const ItemControl = styled.div` + display: flex; + align-items: center; + gap: 0.6rem; + flex-shrink: 0; +` + +const WidthField = styled.div` + display: flex; + flex-direction: column; + align-items: flex-end; + gap: 0.4rem; +` + +const WidthInputRow = styled.div` + display: flex; + align-items: center; + gap: 0.6rem; +` + +const WidthInput = styled(Input)` + width: 9rem; + flex: 0 0 auto; + text-align: right; +` + +type SettingRowProps = { + label: string + description: string + controlId: string + children: ReactNode +} + +const settingDescriptionId = (controlId: string) => `${controlId}-description` + +const SettingRow = ({ + label, + description, + controlId, + children, +}: SettingRowProps) => ( + + + + {label} + +
+ + {description} + +
+ + {children} + +) + +const RUN_WITH_SELECTION_ID = "editor-settings-run-with-selection" +const MAX_COLUMN_WIDTH_ID = "editor-settings-max-column-width" +const MAX_COLUMN_WIDTH_ERROR_ID = `${MAX_COLUMN_WIDTH_ID}-error` + +const EditorSettingsForm = ({ onClose }: { onClose: () => void }) => { + const { runWithSelection, maxColumnWidth, updateSettings } = useLocalStorage() + const [runWithSelectionDraft, setRunWithSelectionDraft] = + useState(runWithSelection) + const [maxColumnWidthDraft, setMaxColumnWidthDraft] = useState( + maxColumnWidth === "auto" ? "" : String(maxColumnWidth), + ) + const [maxColumnWidthError, setMaxColumnWidthError] = useState(false) + const widthInputRef = useRef(null) + + const handleSave = () => { + if (!isMaxColumnWidthDraftValid(maxColumnWidthDraft)) { + setMaxColumnWidthError(true) + widthInputRef.current?.focus() + return + } + updateSettings(StoreKey.RUN_WITH_SELECTION, runWithSelectionDraft) + updateSettings( + StoreKey.MAX_COLUMN_WIDTH, + parseMaxColumnWidth(maxColumnWidthDraft), + ) + onClose() + } + + return ( +
{ + event.preventDefault() + handleSave() + }} + > + + + + + + + + { + setMaxColumnWidthDraft(event.target.value) + setMaxColumnWidthError(false) + }} + placeholder="Auto" + inputMode="numeric" + variant={maxColumnWidthError ? "error" : undefined} + aria-invalid={maxColumnWidthError} + aria-describedby={ + maxColumnWidthError + ? `${settingDescriptionId( + MAX_COLUMN_WIDTH_ID, + )} ${MAX_COLUMN_WIDTH_ERROR_ID}` + : settingDescriptionId(MAX_COLUMN_WIDTH_ID) + } + data-hook={MAX_COLUMN_WIDTH_ID} + /> + + px + + + {maxColumnWidthError && ( + + )} + + + + + + + +
+ ) +} + +export const EditorSettingsModal = ({ open, onOpenChange }: Props) => ( + + + + + + + Editor settings + onOpenChange(false)} /> + + + +) diff --git a/src/components/FixQueryButton/index.tsx b/src/components/FixQueryButton/index.tsx index bec3bd3c9..4ec3d86a2 100644 --- a/src/components/FixQueryButton/index.tsx +++ b/src/components/FixQueryButton/index.tsx @@ -1,5 +1,4 @@ import React, { useContext } from "react" -import type { MutableRefObject } from "react" import styled from "styled-components" import { Button } from ".." import { AISparkle } from "../AISparkle" @@ -10,7 +9,6 @@ import { selectors } from "../../store" import { useAIStatus } from "../../providers/AIStatusProvider" import { useAIConversation } from "../../providers/AIConversationProvider" import { extractErrorByQueryKey } from "../../scenes/Editor/utils" -import type { ExecutionRefs } from "../../scenes/Editor/index" import { executeAIFlow, createFixFlowConfig } from "../../utils/executeAIFlow" import { trackEvent } from "../../modules/ConsoleEventTracker" import { ConsoleEvent } from "../../modules/ConsoleEventTracker/events" @@ -42,16 +40,18 @@ export const FixQueryButton = () => { } = useAIConversation() const handleFixQuery = () => { - void trackEvent(ConsoleEvent.AI_FIX_QUERY) - const conversationId = chatWindowState.activeConversationId! - const conversation = getConversationMeta(conversationId)! - + const conversationId = chatWindowState.activeConversationId + if (!conversationId) return + const conversation = getConversationMeta(conversationId) + if (!conversation?.queryKey || conversation.bufferId == null) return const errorInfo = extractErrorByQueryKey( - conversation.queryKey!, - conversation.bufferId!, - executionRefs as MutableRefObject | undefined, + conversation.queryKey, + conversation.bufferId, + executionRefs, editorRef, - )! + ) + if (!errorInfo) return + void trackEvent(ConsoleEvent.AI_FIX_QUERY) const { errorMessage, queryText, word } = errorInfo diff --git a/src/components/ResultGrid/ResultGrid.tsx b/src/components/ResultGrid/ResultGrid.tsx index be129adae..ded85840c 100644 --- a/src/components/ResultGrid/ResultGrid.tsx +++ b/src/components/ResultGrid/ResultGrid.tsx @@ -20,8 +20,14 @@ import { import type { ColumnDefinition } from "../../utils/questdb/types" import { unescapeHtml } from "../../utils/escapeHtml" -import type { CellValue, ResultGridDataSource, ResultGridRow } from "./types" +import type { + CellValue, + MaxColumnWidth, + ResultGridDataSource, + ResultGridRow, +} from "./types" import { + applyMaxColumnWidth, clampColumnWidths, sampleColumnWidths, isLeftAligned, @@ -54,7 +60,9 @@ import { toAbsoluteIndex, toVisibleAbsoluteRange, } from "./virtualRowMapping" +import { MIN_COLUMN_WIDTH } from "./dimensions" import { useContainerWidth } from "./useContainerWidth" +import { useFontsReady } from "./useFontsReady" import { useScrollShadows } from "./useScrollShadows" import { useColumnSizing } from "./useColumnSizing" import { useFreezeDrag } from "./useFreezeDrag" @@ -140,6 +148,7 @@ const GridCell = React.memo(function GridCell({ type Props = { dataSource: ResultGridDataSource + maxColumnWidth: MaxColumnWidth runToken?: number // changes per run to reset focus/selection on the grid isFocused?: boolean initialColumnSizing?: Record @@ -202,6 +211,7 @@ export const ResultGrid = forwardRef( ( { dataSource, + maxColumnWidth, runToken, isFocused = true, initialColumnSizing, @@ -231,29 +241,44 @@ export const ResultGrid = forwardRef( const scrollRef = useRef(null) const containerWidth = useContainerWidth(gridRef) + const fontsReady = useFontsReady() const { scrolledDown, shadowLeft, handleScroll } = useScrollShadows(scrollRef) const virtualRowCount = Math.min(rowCount, MAX_VIRTUAL_ROWS) // Sampling text lengths over 1000 rows is the expensive part, so it runs - // once per result; the per-resize work is only the container clamp. + // once per result — and once more if the webfont finishes loading after a + // fallback-font measurement. const sampledWidths = useMemo( () => sampleColumnWidths(columns, sampleRows.slice(0, WIDTH_SAMPLE_ROWS)), - [columns, sampleRows], + [columns, sampleRows, fontsReady], + ) + + const cappedWidths = useMemo( + () => applyMaxColumnWidth(sampledWidths, maxColumnWidth), + [sampledWidths, maxColumnWidth], + ) + + const clampedWidths = useMemo( + () => + containerWidth === null + ? null + : clampColumnWidths(cappedWidths, containerWidth), + [cappedWidths, containerWidth], ) const columnDefs = useMemo[]>(() => { - const widths = clampColumnWidths(sampledWidths, containerWidth) + const widths = clampedWidths ?? cappedWidths return columns.map((col, i) => ({ id: columnId(i), accessorFn: (row: ResultGridRow) => row[i], header: col.name, size: widths[i], - minSize: 60, + minSize: MIN_COLUMN_WIDTH, meta: { col }, })) - }, [columns, sampledWidths, containerWidth]) + }, [columns, clampedWidths, cappedWidths]) const [columnOrder, setColumnOrder] = useState([]) const [columnPinning, setColumnPinning] = useState({ @@ -564,6 +589,7 @@ export const ResultGrid = forwardRef( const totalHeight = rowVirtualizer.getTotalSize() const virtualRows = rowVirtualizer.getVirtualItems() const virtualColumns = columnVirtualizer.getVirtualItems() + const rowsToRender = containerWidth === null ? [] : virtualRows const firstVirtual = virtualRows[0]?.index ?? 0 const lastVirtual = virtualRows[virtualRows.length - 1]?.index ?? 0 @@ -717,7 +743,7 @@ export const ResultGrid = forwardRef( } } - const viewportWidth = scrollRef.current?.clientWidth ?? containerWidth + const viewportWidth = scrollRef.current?.clientWidth ?? containerWidth ?? 0 const freezeHandleLeft = Math.min( frozenWidth, Math.max(0, viewportWidth - FREEZE_HANDLE_EDGE_INSET), @@ -763,7 +789,7 @@ export const ResultGrid = forwardRef( position: "relative", }} > - {virtualRows.map((virtualRow) => { + {rowsToRender.map((virtualRow) => { const virtualIndex = virtualRow.index const absoluteIndex = toAbsoluteIndex(virtualIndex, rowCount) const rowData = getRow(absoluteIndex) diff --git a/src/components/ResultGrid/dimensions.ts b/src/components/ResultGrid/dimensions.ts index d5a632ac3..d59bdbb25 100644 --- a/src/components/ResultGrid/dimensions.ts +++ b/src/components/ResultGrid/dimensions.ts @@ -2,3 +2,22 @@ // without pulling React into a unit-test environment. export const ROW_HEIGHT = 30 export const HEADER_HEIGHT = 44 + +export const MIN_COLUMN_WIDTH = 60 +export const MAX_SAMPLED_WIDTH_PX = 4000 + +export const MAX_COLUMN_WIDTH_BOUNDS = { + min: MIN_COLUMN_WIDTH, + max: MAX_SAMPLED_WIDTH_PX, +} +export const CELL_FONT_SIZE_PX = 13 +export const HEADER_NAME_FONT_SIZE_PX = 14 +export const HEADER_TYPE_FONT_SIZE_PX = 10 + +export const CELL_PADDING_PX = 12 +export const CELL_BORDER_PX = 1 + +export const HEADER_PADDING_PX = 20 +export const HEADER_GAP_PX = 6 +export const HEADER_COPY_BUTTON_PX = 14 +export const HEADER_BORDER_PX = 1 diff --git a/src/components/ResultGrid/inlineGridUtils.test.ts b/src/components/ResultGrid/inlineGridUtils.test.ts index a4c064891..dad0345a1 100644 --- a/src/components/ResultGrid/inlineGridUtils.test.ts +++ b/src/components/ResultGrid/inlineGridUtils.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from "vitest" import { + applyMaxColumnWidth, clampColumnWidths, sampleColumnWidths, formatCellValue, @@ -116,9 +117,6 @@ describe("formatCellValue", () => { }) it("truncates array content when columnWidth is tight", () => { - // columnWidth=200 → maxArrayTextLength = ceil(200/8.3) = 25, minus 7 - // overhead = 18 chars of content, which is less than a 100-element - // integer array stringified. const longArray = Array.from({ length: 100 }, (_, i) => i) const out = formatCellValue( longArray as unknown as number, @@ -131,8 +129,6 @@ describe("formatCellValue", () => { }) it("leaves array untruncated when columnWidth leaves ≤3 chars of content", () => { - // columnWidth=80 → maxContentLength drops to 3; the truncation branch is - // skipped (guard: maxContentLength > 3) and the full array is returned. const longArray = Array.from({ length: 100 }, (_, i) => i) const out = formatCellValue( longArray as unknown as number, @@ -231,18 +227,99 @@ describe("sampleColumnWidths", () => { expect(widths).toHaveLength(1) expect(widths[0]).toBeGreaterThanOrEqual(60) }) + + it("samples a budgeted row count when the column set is very wide", () => { + // Given 1000 columns, which drops the row budget to 50 rows + const columns = Array.from({ length: 1000 }, (_, i) => + col(`c${i}`, "STRING"), + ) + const shortRow = () => Array.from({ length: 1000 }, () => "x") + const baseline = sampleColumnWidths(columns, [shortRow()])[0] + + // When a wide value sits beyond the budgeted rows + const rowsWithLateWideValue = Array.from({ length: 55 }, shortRow) + rowsWithLateWideValue[54][0] = "w".repeat(100) + + // Then it does not widen the column + expect(sampleColumnWidths(columns, rowsWithLateWideValue)[0]).toBe(baseline) + + // And the same value within the budgeted rows does + const rowsWithEarlyWideValue = Array.from({ length: 55 }, shortRow) + rowsWithEarlyWideValue[0][0] = "w".repeat(100) + expect( + sampleColumnWidths(columns, rowsWithEarlyWideValue)[0], + ).toBeGreaterThan(baseline) + }) }) describe("clampColumnWidths", () => { - it("caps each width at containerWidth * 0.8", () => { - expect(clampColumnWidths([1000, 100], 1000)).toEqual([800, 100]) + it("leaves columns untouched when their total fits the container", () => { + // Given columns whose total width is within the container + // When clamped + const widths = clampColumnWidths([200, 300], 1000) + + // Then nothing is shrunk + expect(widths).toEqual([200, 300]) + }) + + it("keeps overflowing columns that are all within the max", () => { + // Given columns that overflow the container but none exceeds the max width + // When clamped + const widths = clampColumnWidths([300, 300, 300, 300], 1000) + + // Then every column keeps its width and the grid scrolls horizontally + expect(widths).toEqual([300, 300, 300, 300]) + }) + + it("gives a lone wide column the space the narrow ones leave", () => { + // Given one wide column alongside a narrow one, overflowing the container + // When clamped + const widths = clampColumnWidths([1000, 100], 1000) + + // Then the wide column takes the remaining space minus the scrollbar + // allowance, and the narrow one is kept + expect(widths).toEqual([886, 100]) + }) + + it("splits the leftover space among multiple wide columns", () => { + // Given several wide columns overflowing an otherwise empty container + // When clamped + const widths = clampColumnWidths([1000, 1000], 1000) + + // Then they share the container minus the scrollbar allowance equally + expect(widths).toEqual([493, 493]) + }) + + it("floors wide columns at the max width when space is tight", () => { + // Given more wide columns than the leftover space can fairly fit + // When clamped + const widths = clampColumnWidths([500, 500, 500], 1000) + + // Then each settles at the max width and the grid scrolls horizontally + expect(widths).toEqual([400, 400, 400]) + }) + + it("keeps the narrow columns and floors the lone wide one when they crowd it out", () => { + // Given narrow columns that consume most of the space beside one wide column + // When clamped + const widths = clampColumnWidths([300, 300, 300, 1000], 1000) + + // Then the narrow columns are untouched and the wide one floors at the max + expect(widths).toEqual([300, 300, 300, 400]) }) - it("keeps widths below the cap untouched", () => { - expect(clampColumnWidths([200, 300], 1000)).toEqual([200, 300]) + it("floors the wide column at the max when the narrow columns already overrun the container", () => { + // Given narrow columns whose total alone exceeds the available width, + // leaving a negative budget for the one wide column + // When clamped + const widths = clampColumnWidths([300, 300, 300, 300, 1000], 500) + + // Then the narrow columns are kept and the wide one still floors at the max + // rather than collapsing to a negative width + expect(widths).toEqual([300, 300, 300, 300, 400]) }) - it("caps a long sampled value at the container limit", () => { + it("fills the container with a single overflowing column", () => { // Given a column whose sampled value is far wider than the container const sampled = sampleColumnWidths( [col("long", "STRING")], @@ -252,7 +329,62 @@ describe("clampColumnWidths", () => { // When the container clamp is applied const widths = clampColumnWidths(sampled, 1000) - // Then the column never exceeds 80% of the container - expect(widths[0]).toBeLessThanOrEqual(800) + // Then it fills the container width minus the scrollbar allowance + expect(widths[0]).toBe(986) + }) +}) + +describe("applyMaxColumnWidth", () => { + it("returns the same array when the setting is auto", () => { + // Given widths and the automatic setting + const widths = [200, 1000] + + // When the cap is applied + const capped = applyMaxColumnWidth(widths, "auto") + + // Then the input array is returned untouched + expect(capped).toBe(widths) + }) + + it("returns the same empty array when there are no columns", () => { + // Given no columns + const widths: number[] = [] + + // When the cap is applied + const capped = applyMaxColumnWidth(widths, 200) + + // Then the input array is returned untouched + expect(capped).toBe(widths) + }) + + it("returns the same array when no width exceeds the cap", () => { + // Given widths that all fit under the cap + const widths = [200, 300] + + // When the cap is applied + const capped = applyMaxColumnWidth(widths, 400) + + // Then the input array is returned untouched + expect(capped).toBe(widths) + }) + + it("caps only the widths above the cap", () => { + // Given one width above and one below the cap + // When the cap is applied + const capped = applyMaxColumnWidth([1000, 100], 250) + + // Then only the wide column shrinks + expect(capped).toEqual([250, 100]) + }) + + it("keeps capped columns through the container clamp", () => { + // Given widths capped below the wide-column threshold + const capped = applyMaxColumnWidth([1000, 1000, 1000], 200) + + // When the container clamp runs on a container they overflow + const clamped = clampColumnWidths(capped, 500) + + // Then the capped widths are kept and the grid scrolls horizontally + expect(clamped).toEqual([200, 200, 200]) }) }) diff --git a/src/components/ResultGrid/inlineGridUtils.ts b/src/components/ResultGrid/inlineGridUtils.ts index 7ed7e4ebe..eee9b396d 100644 --- a/src/components/ResultGrid/inlineGridUtils.ts +++ b/src/components/ResultGrid/inlineGridUtils.ts @@ -1,19 +1,138 @@ import type { ColumnDefinition } from "../../utils/questdb/types" import { unescapeHtml } from "../../utils/escapeHtml" -import type { CellValue, ResultGridRow } from "./types" - -const CELL_WIDTH_MULTIPLIER = 9.6 -const ARRAY_CELL_WIDTH_MULTIPLIER = 8.3 -const MIN_COLUMN_WIDTH = 60 -const MAX_WIDTH_RATIO = 0.8 - -// Non-text horizontal space a header cell reserves but a data cell doesn't: cell -// padding + flex gap + the always-rendered (visibility:hidden) sm copy button. -const HEADER_PADDING_PX = 32 -const HEADER_GAP_PX = 6 -const HEADER_COPY_BUTTON_PX = 32 +import type { CellValue, MaxColumnWidth, ResultGridRow } from "./types" +import { theme } from "../../theme" +import { + CELL_FONT_SIZE_PX, + HEADER_NAME_FONT_SIZE_PX, + HEADER_TYPE_FONT_SIZE_PX, + CELL_PADDING_PX, + CELL_BORDER_PX, + HEADER_PADDING_PX, + HEADER_GAP_PX, + HEADER_COPY_BUTTON_PX, + HEADER_BORDER_PX, + MIN_COLUMN_WIDTH, + MAX_SAMPLED_WIDTH_PX, +} from "./dimensions" + +const WIDE_COLUMN_THRESHOLD_PX = 400 + +// Rows scroll inside a viewport that is narrower than the measured container +// when the vertical scrollbar renders, so fitting to the full container width +// would leave a permanent horizontal scrollbar. +const SCROLLBAR_ALLOWANCE_PX = 14 + const HEADER_CHROME_PX = - HEADER_PADDING_PX + HEADER_GAP_PX + HEADER_COPY_BUTTON_PX + HEADER_PADDING_PX + HEADER_GAP_PX + HEADER_COPY_BUTTON_PX + HEADER_BORDER_PX + +const CELL_CHROME_PX = CELL_PADDING_PX + CELL_BORDER_PX + +const CELL_FONT = `${CELL_FONT_SIZE_PX}px ${theme.font}` +const HEADER_NAME_FONT = `${HEADER_NAME_FONT_SIZE_PX}px ${theme.font}` +const HEADER_TYPE_FONT = `${HEADER_TYPE_FONT_SIZE_PX}px ${theme.font}` + +// No glyph in the cell font is narrower than punctuation, so available width +// over this advance bounds how many chars can fit. +const MIN_CHAR_ADVANCE_PX = 3 + +// The canvas 2D context is null outside the DOM (the node test runner); there a +// fixed per-char advance keeps sampling deterministic. +const FALLBACK_CHAR_WIDTH_PX = 9.6 +const measureContext = + typeof document === "undefined" + ? null + : document.createElement("canvas").getContext("2d") + +let lastMeasuredFont = "" +let cellCharAdvances: Float64Array | null | undefined +let asciiCellAdvances: Float64Array | undefined +let truncatedArrayCache = new WeakMap< + object, + { columnWidth: number; formatted: string } +>() + +// Safari resolves the canvas font at assignment time, so a face that finishes +// loading afterwards is only picked up once the font is reassigned. +export const invalidateMeasuredFont = (): void => { + lastMeasuredFont = "" + cellCharAdvances = undefined + asciiCellAdvances = undefined + truncatedArrayCache = new WeakMap() +} + +const measureTextWidth = (text: string, font: string): number => { + if (!measureContext) return text.length * FALLBACK_CHAR_WIDTH_PX + if (font !== lastMeasuredFont) { + measureContext.font = font + lastMeasuredFont = font + } + return measureContext.measureText(text).width +} + +// Numeric and timestamp values are made of these glyphs, which fonts don't +// kern, so their width is the sum of per-char advances — no canvas call per +// value. Verified per font against a real measurement; a font that fails the +// check keeps measuring every value. +const FAST_MEASURE_CHARS = "0123456789 .,:;+-eETZ" +const FAST_MEASURE_VALIDATION = "2026-01-02T03:44:55.667788Z, -1234.5e+16" + +const buildCellCharAdvances = (): Float64Array | null => { + if (!measureContext) return null + const advances = new Float64Array(128) + for (const char of FAST_MEASURE_CHARS) { + advances[char.charCodeAt(0)] = measureTextWidth(char, CELL_FONT) + } + let sum = 0 + for (let i = 0; i < FAST_MEASURE_VALIDATION.length; i++) { + sum += advances[FAST_MEASURE_VALIDATION.charCodeAt(i)] + } + const real = measureTextWidth(FAST_MEASURE_VALIDATION, CELL_FONT) + return Math.abs(sum - real) <= 0.5 ? advances : null +} + +// Kerning and ligatures only ever narrow Latin text in this font stack, so the +// per-glyph sum plus a small margin is an upper bound on the rendered width — +// tight enough that same-length values skip the canvas call entirely. +const UPPER_BOUND_MARGIN = 1.02 +const FIRST_PRINTABLE_ASCII = 32 +const LAST_PRINTABLE_ASCII = 126 + +const buildAsciiCellAdvances = (): Float64Array => { + const advances = new Float64Array(128) + for (let code = FIRST_PRINTABLE_ASCII; code <= LAST_PRINTABLE_ASCII; code++) { + advances[code] = measureTextWidth(String.fromCharCode(code), CELL_FONT) + } + return advances +} + +const cellWidthUpperBound = (text: string): number | null => { + if (asciiCellAdvances === undefined) { + asciiCellAdvances = buildAsciiCellAdvances() + } + let sum = 0 + for (let i = 0; i < text.length; i++) { + const advance = asciiCellAdvances[text.charCodeAt(i)] + if (!advance) return null + sum += advance + } + return sum * UPPER_BOUND_MARGIN + CELL_CHROME_PX +} + +const fastCellTextWidth = (text: string): number | null => { + if (cellCharAdvances === undefined) { + cellCharAdvances = buildCellCharAdvances() + } + if (cellCharAdvances === null) return null + let sum = 0 + for (let i = 0; i < text.length; i++) { + const code = text.charCodeAt(i) + const advance = code < 128 ? cellCharAdvances[code] : 0 + if (advance === 0) return null + sum += advance + } + return sum +} const LEFT_ALIGNED_TYPES = new Set(["STRING", "SYMBOL", "VARCHAR", "ARRAY"]) @@ -21,11 +140,9 @@ const FLOAT_TYPES = new Set(["FLOAT", "DOUBLE"]) const isArrayColumn = (col: ColumnDefinition): boolean => col.type === "ARRAY" -const getCellWidth = (textLength: number, isArray = false): number => { - const multiplier = isArray - ? ARRAY_CELL_WIDTH_MULTIPLIER - : CELL_WIDTH_MULTIPLIER - return Math.max(MIN_COLUMN_WIDTH, Math.ceil(textLength * multiplier)) +const cellWidth = (text: string): number => { + const width = fastCellTextWidth(text) ?? measureTextWidth(text, CELL_FONT) + return Math.max(MIN_COLUMN_WIDTH, Math.ceil(width) + CELL_CHROME_PX) } const getArrayString = (value: unknown): string => { @@ -50,6 +167,35 @@ const formatArrayFull = (value: unknown, col: ColumnDefinition): string => { return wrapArray(arrayContent(value, dim), dim) } +const fitArrayToWidth = ( + content: string, + full: string, + dim: number, + available: number, +): string => { + const maxFittingChars = Math.ceil(available / MIN_CHAR_ADVANCE_PX) + if ( + full.length <= maxFittingChars && + measureTextWidth(full, CELL_FONT) <= available + ) { + return full + } + + let lo = 0 + let hi = Math.min(content.length, maxFittingChars) + while (lo < hi) { + const mid = Math.ceil((lo + hi) / 2) + const candidate = wrapArray(`${content.slice(0, mid)}...`, dim) + if (measureTextWidth(candidate, CELL_FONT) <= available) { + lo = mid + } else { + hi = mid - 1 + } + } + if (lo <= 3) return full + return wrapArray(`${content.slice(0, lo)}...`, dim) +} + const formatArrayValue = ( value: unknown, col: ColumnDefinition, @@ -57,56 +203,72 @@ const formatArrayValue = ( ): string => { if (value === null) return "null" const dim = col.dim ?? 1 - const content = arrayContent(value, dim) - const full = wrapArray(content, dim) - if (!columnWidth) return full + if (!columnWidth) return wrapArray(arrayContent(value, dim), dim) - const maxArrayTextLength = Math.ceil( - columnWidth / ARRAY_CELL_WIDTH_MULTIPLIER, - ) - const maxContentLength = maxArrayTextLength - (dim * 2 + "ARRAY".length) - - if (content.length > maxContentLength && maxContentLength > 3) { - return wrapArray(`${content.slice(0, maxContentLength)}...`, dim) + const cacheable = typeof value === "object" + if (cacheable) { + const cached = truncatedArrayCache.get(value) + if (cached && cached.columnWidth === columnWidth) return cached.formatted } - return full + const content = arrayContent(value, dim) + const full = wrapArray(content, dim) + const formatted = fitArrayToWidth( + content, + full, + dim, + columnWidth - CELL_CHROME_PX, + ) + if (cacheable) { + truncatedArrayCache.set(value, { columnWidth, formatted }) + } + return formatted } -// Sampling is container-independent so it runs once per result; the -// container-driven cap is applied separately by clampColumnWidths. The -// constant ceiling keeps the sampling loop bounded for very long values. -const MAX_SAMPLED_WIDTH_PX = 4000 +const MAX_MEASURE_CHARS = 2000 + +// Wide results sample fewer rows so the sampled-cell count stays flat up to +// ~1000 columns, then holds at MIN_WIDTH_SAMPLE_ROWS rows per column. +const WIDTH_SAMPLE_CELL_BUDGET = 50000 +const MIN_WIDTH_SAMPLE_ROWS = 50 +// Sampling is container-independent so it runs once per result; the +// container-driven cap is applied separately by clampColumnWidths. export const sampleColumnWidths = ( columns: ColumnDefinition[], dataset: ResultGridRow[], ): number[] => { - const maxTextLenRegular = Math.ceil( - MAX_SAMPLED_WIDTH_PX / CELL_WIDTH_MULTIPLIER, - ) - const maxTextLenArray = Math.ceil( - MAX_SAMPLED_WIDTH_PX / ARRAY_CELL_WIDTH_MULTIPLIER, + const rowBudget = Math.max( + MIN_WIDTH_SAMPLE_ROWS, + Math.floor(WIDTH_SAMPLE_CELL_BUDGET / Math.max(columns.length, 1)), ) + const rows = + dataset.length > rowBudget ? dataset.slice(0, rowBudget) : dataset return columns.map((col, colIdx) => { const isArray = isArrayColumn(col) - const maxTextLen = isArray ? maxTextLenArray : maxTextLenRegular - const headerTextLen = Math.max( - col.name.length, - formatColumnType(col).length, + const headerTextPx = Math.max( + measureTextWidth(col.name, HEADER_NAME_FONT), + measureTextWidth(formatColumnType(col), HEADER_TYPE_FONT), + ) + let width = Math.max( + MIN_COLUMN_WIDTH, + Math.ceil(headerTextPx) + HEADER_CHROME_PX, ) - const headerTextPx = Math.ceil(headerTextLen * CELL_WIDTH_MULTIPLIER) - let width = Math.max(MIN_COLUMN_WIDTH, headerTextPx + HEADER_CHROME_PX) - for (const row of dataset) { + for (const row of rows) { const val = row[colIdx] const formatted = isArray ? formatArrayValue(val, col) : formatCellValue(val, col) - const displayLen = Math.min(formatted.length, maxTextLen) - width = Math.max(width, getCellWidth(displayLen, isArray)) + const measured = + formatted.length > MAX_MEASURE_CHARS + ? formatted.slice(0, MAX_MEASURE_CHARS) + : formatted + const upperBound = cellWidthUpperBound(measured) + if (upperBound !== null && upperBound <= width) continue + width = Math.max(width, cellWidth(measured)) if (width >= MAX_SAMPLED_WIDTH_PX) { width = MAX_SAMPLED_WIDTH_PX break @@ -116,12 +278,37 @@ export const sampleColumnWidths = ( }) } +export const applyMaxColumnWidth = ( + widths: number[], + maxColumnWidth: MaxColumnWidth, +): number[] => { + if (maxColumnWidth === "auto") return widths + if (widths.every((width) => width <= maxColumnWidth)) return widths + return widths.map((width) => Math.min(width, maxColumnWidth)) +} + export const clampColumnWidths = ( widths: number[], containerWidth: number, ): number[] => { - const maxWidth = containerWidth * MAX_WIDTH_RATIO - return widths.map((width) => Math.min(width, maxWidth)) + const available = containerWidth - SCROLLBAR_ALLOWANCE_PX + const total = widths.reduce((sum, width) => sum + width, 0) + if (total <= available) return widths + + const wideCount = widths.filter( + (width) => width > WIDE_COLUMN_THRESHOLD_PX, + ).length + if (wideCount === 0) return widths + + const narrowTotal = widths.reduce( + (sum, width) => (width <= WIDE_COLUMN_THRESHOLD_PX ? sum + width : sum), + 0, + ) + const wideCap = Math.max( + WIDE_COLUMN_THRESHOLD_PX, + (available - narrowTotal) / wideCount, + ) + return widths.map((width) => Math.min(width, wideCap)) } export const isLeftAligned = (type: string): boolean => diff --git a/src/components/ResultGrid/styles.ts b/src/components/ResultGrid/styles.ts index 5bb5dd79d..26c697378 100644 --- a/src/components/ResultGrid/styles.ts +++ b/src/components/ResultGrid/styles.ts @@ -2,7 +2,18 @@ import styled, { css, keyframes } from "styled-components" import { color } from "../../utils" import { theme } from "../../theme" import { CopyButton } from "../CopyButton" -import { HEADER_HEIGHT, ROW_HEIGHT } from "./dimensions" +import { + CELL_BORDER_PX, + CELL_FONT_SIZE_PX, + CELL_PADDING_PX, + HEADER_BORDER_PX, + HEADER_GAP_PX, + HEADER_HEIGHT, + HEADER_NAME_FONT_SIZE_PX, + HEADER_PADDING_PX, + HEADER_TYPE_FONT_SIZE_PX, + ROW_HEIGHT, +} from "./dimensions" export { HEADER_HEIGHT, ROW_HEIGHT } @@ -36,13 +47,13 @@ export const HeaderRow = styled.div<{ $shadowBottom: boolean }>` export const HeaderCell = styled.div<{ $align: string; $frozen?: boolean }>` position: relative; flex-shrink: 0; - padding: 0.5rem 1rem; + padding: 0.5rem ${HEADER_PADDING_PX / 2}px; display: flex; flex-direction: column; justify-content: center; user-select: none; text-align: ${({ $align }) => $align}; - border-right: 1px solid ${color("selection")}; + border-right: ${HEADER_BORDER_PX}px solid ${color("selection")}; /* Sticky-left: opaque background so scrolled-under headers don't show through. */ ${({ $frozen }) => $frozen && @@ -63,7 +74,7 @@ export const HeaderNameRow = styled.div<{ $align: string }>` flex-direction: ${({ $align }) => $align === "right" ? "row-reverse" : "row"}; justify-content: flex-start; - gap: 6px; + gap: ${HEADER_GAP_PX}px; ` export const HeaderName = styled.span` @@ -72,12 +83,12 @@ export const HeaderName = styled.span` overflow: hidden; text-overflow: ellipsis; min-width: 0; - font-size: ${({ theme }) => theme.fontSize.md}; + font-size: ${HEADER_NAME_FONT_SIZE_PX}px; ` export const HeaderType = styled.span` color: ${color("gray2")}; - font-size: ${({ theme }) => theme.fontSize.ms}; + font-size: ${HEADER_TYPE_FONT_SIZE_PX}px; white-space: nowrap; text-transform: lowercase; ` @@ -183,16 +194,16 @@ export const Cell = styled.div<{ height: ${ROW_HEIGHT}px; display: flex; align-items: center; - padding: 0 0.6rem; + padding: 0 ${CELL_PADDING_PX / 2}px; overflow: hidden; - font-size: ${({ theme }) => theme.fontSize.sm}; + font-size: ${CELL_FONT_SIZE_PX}px; color: ${({ $isNull, $isTimestamp }) => $isNull ? color("mutedLabel") : $isTimestamp ? color("green") : color("foreground")}; - border-right: 1px solid ${color("selection")}; + border-right: ${CELL_BORDER_PX}px solid ${color("selection")}; border-bottom: 1px solid ${color("selection")}; box-sizing: border-box; /* contain: layout, not paint — paint would clip the copy-pulse glow. */ diff --git a/src/components/ResultGrid/types.ts b/src/components/ResultGrid/types.ts index c691ad34c..0334a878d 100644 --- a/src/components/ResultGrid/types.ts +++ b/src/components/ResultGrid/types.ts @@ -8,6 +8,8 @@ export type CellValue = boolean | string | number | null export type ResultGridRow = CellValue[] +export type MaxColumnWidth = number | "auto" + export type ColumnLayout = { columnSizing?: Record columnOrder?: string[] diff --git a/src/components/ResultGrid/useContainerWidth.ts b/src/components/ResultGrid/useContainerWidth.ts index 6c6f0d580..37c8cb4ae 100644 --- a/src/components/ResultGrid/useContainerWidth.ts +++ b/src/components/ResultGrid/useContainerWidth.ts @@ -1,16 +1,11 @@ -import { - useEffect, - useLayoutEffect, - useRef, - useState, - type RefObject, -} from "react" +import { useEffect, useLayoutEffect, useState, type RefObject } from "react" const RESIZE_DEBOUNCE_MS = 100 -export const useContainerWidth = (ref: RefObject): number => { - const [width, setWidth] = useState(800) - const timerRef = useRef | null>(null) +export const useContainerWidth = ( + ref: RefObject, +): number | null => { + const [width, setWidth] = useState(null) useLayoutEffect(() => { const measured = ref.current?.getBoundingClientRect().width @@ -20,19 +15,17 @@ export const useContainerWidth = (ref: RefObject): number => { useEffect(() => { if (!ref.current) return + let timer: number | null = null const observer = new ResizeObserver(([entry]) => { const measured = entry.contentRect.width if (!measured) return - if (timerRef.current) clearTimeout(timerRef.current) - timerRef.current = setTimeout( - () => setWidth(measured), - RESIZE_DEBOUNCE_MS, - ) + if (timer !== null) window.clearTimeout(timer) + timer = window.setTimeout(() => setWidth(measured), RESIZE_DEBOUNCE_MS) }) observer.observe(ref.current) return () => { observer.disconnect() - if (timerRef.current) clearTimeout(timerRef.current) + if (timer !== null) window.clearTimeout(timer) } }, [ref]) diff --git a/src/components/ResultGrid/useFontsReady.ts b/src/components/ResultGrid/useFontsReady.ts new file mode 100644 index 000000000..3408a88fc --- /dev/null +++ b/src/components/ResultGrid/useFontsReady.ts @@ -0,0 +1,26 @@ +import { useEffect, useState } from "react" +import { invalidateMeasuredFont } from "./inlineGridUtils" + +const fontsLoaded = (): boolean => + typeof document === "undefined" || + !("fonts" in document) || + document.fonts.status === "loaded" + +export const useFontsReady = (): boolean => { + const [ready, setReady] = useState(fontsLoaded) + + useEffect(() => { + if (ready) return + let cancelled = false + void document.fonts.ready.then(() => { + invalidateMeasuredFont() + if (cancelled) return + setReady(true) + }) + return () => { + cancelled = true + } + }, [ready]) + + return ready +} diff --git a/src/components/Switch/index.tsx b/src/components/Switch/index.tsx index 79daac156..3149d1e7a 100644 --- a/src/components/Switch/index.tsx +++ b/src/components/Switch/index.tsx @@ -8,6 +8,8 @@ type Props = { onChange: (checked: boolean) => void dataHook?: string checked?: boolean + id?: string + ariaDescribedBy?: string } const Root = styled(SwitchPrimitive.Root)` @@ -69,6 +71,8 @@ export const Switch = ({ disabled, onChange, dataHook, + id, + ariaDescribedBy, }: Props) => ( diff --git a/src/consts/shared-definitions.sync.test.ts b/src/consts/shared-definitions.sync.test.ts deleted file mode 100644 index f8fb3c548..000000000 --- a/src/consts/shared-definitions.sync.test.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { existsSync, readFileSync } from "fs" -import { dirname, resolve } from "path" -import { fileURLToPath } from "url" - -// The MCP tool schema is duplicated: the UI loads its own copy, and the -// mcp-bridge package loads its own. They MUST stay byte-identical — drift -// silently breaks the bridge's view of the tools. `yarn sync-tools` would -// overwrite the UI copy from `main`, so when editing on a feature branch, -// edit BOTH src copies by hand and rely on this test to catch a missed one. -const here = dirname(fileURLToPath(import.meta.url)) -const uiCopy = resolve(here, "shared-definitions.json") -const bridgeCopy = resolve( - here, - "../../../mcp-bridge/src/consts/shared-definitions.json", -) - -describe("shared-definitions.json", () => { - it("UI and mcp-bridge copies are byte-identical", () => { - const ui = readFileSync(uiCopy, "utf8") - // The mcp-bridge package is not always checked out alongside the UI - // (e.g. UI-only CI). Skip rather than fail when the sibling is absent. - if (!existsSync(bridgeCopy)) return - expect(readFileSync(bridgeCopy, "utf8")).toBe(ui) - }) -}) diff --git a/src/providers/LocalStorageProvider/index.tsx b/src/providers/LocalStorageProvider/index.tsx index 2fe245ca7..fa39f2e2d 100644 --- a/src/providers/LocalStorageProvider/index.tsx +++ b/src/providers/LocalStorageProvider/index.tsx @@ -28,10 +28,12 @@ import React, { useContext, useCallback, useEffect, + useMemo, } from "react" import { getValue, setValue } from "../../utils/localStorage" import { StoreKey } from "../../utils/localStorage/types" -import { parseInteger, parseBoolean } from "./utils" +import { parseInteger, parseBoolean, parseMaxColumnWidth } from "./utils" +import type { MaxColumnWidth } from "../../components/ResultGrid/types" import { AiAssistantSettings, LocalConfig, @@ -53,6 +55,8 @@ const defaultConfig: LocalConfig = { exampleQueriesVisited: false, autoRefreshTables: true, useNewGrid: true, + runWithSelection: true, + maxColumnWidth: "auto", aiAssistantSettings: DEFAULT_AI_ASSISTANT_SETTINGS, leftPanelState: { type: LeftPanelType.DATASOURCES, @@ -70,6 +74,8 @@ type ContextProps = { exampleQueriesVisited: boolean autoRefreshTables: boolean useNewGrid: boolean + runWithSelection: boolean + maxColumnWidth: MaxColumnWidth leftPanelState: LeftPanelState updateLeftPanelState: (state: LeftPanelState) => void aiAssistantSettings: AiAssistantSettings @@ -77,6 +83,29 @@ type ContextProps = { updateAiChatPanelWidth: (width: number) => void } +const getAiAssistantSettings = (): AiAssistantSettings => { + const stored = getValue(StoreKey.AI_ASSISTANT_SETTINGS) + if (stored) { + try { + const parsed = JSON.parse(stored) as AiAssistantSettings + const reconciled = reconcileSettings({ + selectedModel: parsed.selectedModel, + providers: parsed.providers || {}, + ...(parsed.customProviders && { + customProviders: parsed.customProviders, + }), + }) + if (JSON.stringify(reconciled) !== stored) { + setValue(StoreKey.AI_ASSISTANT_SETTINGS, JSON.stringify(reconciled)) + } + return reconciled + } catch (e) { + return defaultConfig.aiAssistantSettings + } + } + return defaultConfig.aiAssistantSettings +} + const defaultValues: ContextProps = { editorCol: 1, editorLine: 1, @@ -86,6 +115,8 @@ const defaultValues: ContextProps = { exampleQueriesVisited: false, autoRefreshTables: true, useNewGrid: true, + runWithSelection: true, + maxColumnWidth: "auto", leftPanelState: defaultConfig.leftPanelState, updateLeftPanelState: (_state: LeftPanelState) => undefined, aiAssistantSettings: defaultConfig.aiAssistantSettings, @@ -147,6 +178,17 @@ export const LocalStorageProvider = ({ const [useNewGrid, setUseNewGrid] = useState(getInitialNewGrid) + const [runWithSelection, setRunWithSelection] = useState( + parseBoolean( + getValue(StoreKey.RUN_WITH_SELECTION), + defaultConfig.runWithSelection, + ), + ) + + const [maxColumnWidth, setMaxColumnWidth] = useState( + parseMaxColumnWidth(getValue(StoreKey.MAX_COLUMN_WIDTH)), + ) + useEffect(() => { const override = readNewGridOverride() if (override === null) return @@ -178,29 +220,6 @@ export const LocalStorageProvider = ({ const [leftPanelState, setLeftPanelState] = useState(getLeftPanelState()) - const getAiAssistantSettings = (): AiAssistantSettings => { - const stored = getValue(StoreKey.AI_ASSISTANT_SETTINGS) - if (stored) { - try { - const parsed = JSON.parse(stored) as AiAssistantSettings - const reconciled = reconcileSettings({ - selectedModel: parsed.selectedModel, - providers: parsed.providers || {}, - ...(parsed.customProviders && { - customProviders: parsed.customProviders, - }), - }) - if (JSON.stringify(reconciled) !== stored) { - setValue(StoreKey.AI_ASSISTANT_SETTINGS, JSON.stringify(reconciled)) - } - return reconciled - } catch (e) { - return defaultConfig.aiAssistantSettings - } - } - return defaultConfig.aiAssistantSettings - } - const [aiAssistantSettings, setAiAssistantSettings] = useState(getAiAssistantSettings()) @@ -216,22 +235,12 @@ export const LocalStorageProvider = ({ setAiChatPanelWidth(width) }, []) - const updateSettings = (key: StoreKey, value: SettingsType) => { - if (key === StoreKey.AI_ASSISTANT_SETTINGS) { - setValue(key, JSON.stringify(value)) - } else { - const typedValue = value as string | boolean | number - setValue(key, typedValue as string) - } - refreshSettings(key) - } - const updateLeftPanelState = useCallback((state: LeftPanelState) => { setValue(StoreKey.LEFT_PANEL_STATE, JSON.stringify(state)) setLeftPanelState(state) }, []) - const refreshSettings = (key: StoreKey) => { + const refreshSettings = useCallback((key: StoreKey) => { const value = getValue(key) switch (key) { case StoreKey.EDITOR_COL: @@ -259,30 +268,70 @@ export const LocalStorageProvider = ({ case StoreKey.USE_NEW_GRID: setUseNewGrid(value === "true") break + case StoreKey.RUN_WITH_SELECTION: + setRunWithSelection(value === "true") + break + case StoreKey.MAX_COLUMN_WIDTH: + setMaxColumnWidth(parseMaxColumnWidth(value)) + break case StoreKey.AI_ASSISTANT_SETTINGS: setAiAssistantSettings(getAiAssistantSettings()) break } - } + }, []) + + const updateSettings = useCallback( + (key: StoreKey, value: SettingsType) => { + if (key === StoreKey.AI_ASSISTANT_SETTINGS) { + setValue(key, JSON.stringify(value)) + } else { + const typedValue = value as string | boolean | number + setValue(key, typedValue as string) + } + refreshSettings(key) + }, + [refreshSettings], + ) + + const value = useMemo( + () => ({ + editorCol, + editorLine, + editorSplitterBasis, + resultsSplitterBasis, + updateSettings, + exampleQueriesVisited, + autoRefreshTables, + useNewGrid, + runWithSelection, + maxColumnWidth, + leftPanelState, + updateLeftPanelState, + aiAssistantSettings, + aiChatPanelWidth, + updateAiChatPanelWidth, + }), + [ + editorCol, + editorLine, + editorSplitterBasis, + resultsSplitterBasis, + updateSettings, + exampleQueriesVisited, + autoRefreshTables, + useNewGrid, + runWithSelection, + maxColumnWidth, + leftPanelState, + updateLeftPanelState, + aiAssistantSettings, + aiChatPanelWidth, + updateAiChatPanelWidth, + ], + ) return ( - + {children} ) diff --git a/src/providers/LocalStorageProvider/types.ts b/src/providers/LocalStorageProvider/types.ts index 3c7ca8b1e..bbc04be33 100644 --- a/src/providers/LocalStorageProvider/types.ts +++ b/src/providers/LocalStorageProvider/types.ts @@ -1,3 +1,5 @@ +import type { MaxColumnWidth } from "../../components/ResultGrid/types" + export type ProviderSettings = { apiKey: string enabledModels: string[] @@ -45,6 +47,8 @@ export type LocalConfig = { exampleQueriesVisited: boolean autoRefreshTables: boolean useNewGrid: boolean + runWithSelection: boolean + maxColumnWidth: MaxColumnWidth leftPanelState: LeftPanelState aiAssistantSettings: AiAssistantSettings aiChatPanelWidth: number diff --git a/src/providers/LocalStorageProvider/utils.test.ts b/src/providers/LocalStorageProvider/utils.test.ts new file mode 100644 index 000000000..c32141145 --- /dev/null +++ b/src/providers/LocalStorageProvider/utils.test.ts @@ -0,0 +1,55 @@ +import { describe, it, expect } from "vitest" +import { isMaxColumnWidthDraftValid, parseMaxColumnWidth } from "./utils" + +describe("parseMaxColumnWidth", () => { + it("parses a stored number", () => { + expect(parseMaxColumnWidth("550")).toBe(550) + }) + + it("falls back to auto for a missing value", () => { + expect(parseMaxColumnWidth("")).toBe("auto") + }) + + it("falls back to auto for the stored auto keyword", () => { + expect(parseMaxColumnWidth("auto")).toBe("auto") + }) + + it("falls back to auto for garbage", () => { + expect(parseMaxColumnWidth("wide")).toBe("auto") + }) + + it("clamps values below the minimum", () => { + expect(parseMaxColumnWidth("10")).toBe(60) + }) + + it("clamps values above the maximum", () => { + expect(parseMaxColumnWidth("99999")).toBe(4000) + }) +}) + +describe("isMaxColumnWidthDraftValid", () => { + it("accepts an empty draft as auto", () => { + expect(isMaxColumnWidthDraftValid("")).toBe(true) + }) + + it("accepts a whole number within the bounds", () => { + expect(isMaxColumnWidthDraftValid("250")).toBe(true) + }) + + it("rejects numbers outside the bounds", () => { + expect(isMaxColumnWidthDraftValid("10")).toBe(false) + expect(isMaxColumnWidthDraftValid("99999")).toBe(false) + }) + + it("rejects locale-formatted and decimal numbers", () => { + expect(isMaxColumnWidthDraftValid("1,500")).toBe(false) + expect(isMaxColumnWidthDraftValid("1.500")).toBe(false) + expect(isMaxColumnWidthDraftValid("250.5")).toBe(false) + }) + + it("rejects non-numeric input", () => { + expect(isMaxColumnWidthDraftValid("wide")).toBe(false) + expect(isMaxColumnWidthDraftValid("-250")).toBe(false) + expect(isMaxColumnWidthDraftValid("1e3")).toBe(false) + }) +}) diff --git a/src/providers/LocalStorageProvider/utils.ts b/src/providers/LocalStorageProvider/utils.ts index eccf087dc..1793d18c5 100644 --- a/src/providers/LocalStorageProvider/utils.ts +++ b/src/providers/LocalStorageProvider/utils.ts @@ -22,8 +22,29 @@ * ******************************************************************************/ +import { MAX_COLUMN_WIDTH_BOUNDS } from "../../components/ResultGrid/dimensions" +import type { MaxColumnWidth } from "../../components/ResultGrid/types" + export const parseBoolean = (value: string, defaultValue: boolean): boolean => value ? value === "true" : defaultValue export const parseInteger = (value: string, defaultValue: number): number => isNaN(parseInt(value)) ? defaultValue : parseInt(value) + +export const parseMaxColumnWidth = (value: string): MaxColumnWidth => { + const parsed = parseInt(value) + if (isNaN(parsed)) return "auto" + return Math.min( + Math.max(parsed, MAX_COLUMN_WIDTH_BOUNDS.min), + MAX_COLUMN_WIDTH_BOUNDS.max, + ) +} + +export const isMaxColumnWidthDraftValid = (draft: string): boolean => { + if (draft === "") return true + if (!/^\d+$/.test(draft)) return false + const width = parseInt(draft) + return ( + width >= MAX_COLUMN_WIDTH_BOUNDS.min && width <= MAX_COLUMN_WIDTH_BOUNDS.max + ) +} diff --git a/src/scenes/Editor/Monaco/index.tsx b/src/scenes/Editor/Monaco/index.tsx index da12740ea..fd5ed55be 100644 --- a/src/scenes/Editor/Monaco/index.tsx +++ b/src/scenes/Editor/Monaco/index.tsx @@ -29,6 +29,7 @@ import { isBlockingAIStatus, } from "../../../providers/AIStatusProvider" import { useAIConversationActions } from "../../../providers/AIConversationProvider" +import { useLocalStorage } from "../../../providers/LocalStorageProvider" import { actions, selectors } from "../../../store" import { RunningType } from "../../../store/Query/types" import { MAX_CELL_LINES } from "../../../store/notebook" @@ -62,6 +63,11 @@ import { createQueryKey, parseQueryKey, createQueryKeyFromRequest, + InflightQuery, + createInflightQuery, + shiftInflightQuery, + shiftSelection, + isInflightQueryStillInPlace, validateQueryJIT, setErrorMarkerForQuery, getQueryStartOffset, @@ -277,6 +283,7 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { } = editorContext const { quest, questExecution } = useContext(QuestContext) const { canUse: canUseAI, status: aiStatus } = useAIStatus() + const { runWithSelection } = useLocalStorage() const { handleGlyphClick, hasConversationForQuery, @@ -317,9 +324,13 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { const runningValueRef = useRef(running) const activeBufferRef = useRef(activeBuffer) const requestRef = useRef(request) + const inflightQueryRef = useRef(null) + const runSeqRef = useRef(0) const queryNotificationsRef = useRef(queryNotifications) const activeNotificationRef = useRef(activeNotification) const canUseAIRef = useRef(canUseAI) + const runWithSelectionRef = useRef(runWithSelection) + const shareLinkSelectionRunRef = useRef(false) const hasConversationForQueryRef = useRef(hasConversationForQuery) const shiftQueryKeysForBufferRef = useRef(shiftQueryKeysForBuffer) const findQueryByConversationIdRef = useRef(findQueryByConversationId) @@ -540,11 +551,31 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { buildAndCopyShareLink([query]) } + const syncQueriesToRun = ( + editor: editor.IStandaloneCodeEditor, + runWithSelection: boolean, + ): Request[] => { + const queriesToRun = getQueriesToRun( + editor, + queryOffsetsRef.current ?? [], + runWithSelection, + ) + queriesToRunRef.current = queriesToRun + dispatch(actions.query.setQueriesToRun(queriesToRun)) + return queriesToRun + } + const handleCopyLinkSelection = () => { void trackEvent(ConsoleEvent.EDITOR_COPY_QUERY_LINK, { from: "shortcut", }) - buildAndCopyShareLink(queriesToRunRef.current ?? []) + const editor = editorRef.current + if (!editor) return + // Link sharing always honors the selection, independent of the + // run-with-selection setting. + buildAndCopyShareLink( + getQueriesToRun(editor, queryOffsetsRef.current ?? [], true), + ) } const handleCopyLinkAllQueries = (shortcut?: boolean) => { @@ -750,6 +781,15 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { const activeBufferId = activeBufferRef.current.id as number + const inflightQuery = inflightQueryRef.current + const runningQueryLineNumber = + runningValueRef.current !== RunningType.NONE && + inflightQuery !== null && + !inflightQuery.dislodged && + inflightQuery.bufferId === activeBufferId + ? model.getPositionAt(inflightQuery.startOffset).lineNumber + : null + const allQueryOffsets: { startOffset: number; endOffset: number }[] = [] const newGlyphWidgetIds = new Map() const newGlyphWidgetLineNumbers = new Set() @@ -784,10 +824,7 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { ? hasConversationForQueryRef.current(activeBufferId, queryKey) : false - const isRunningQuery = - runningValueRef.current !== RunningType.NONE && - requestRef.current?.row !== undefined && - requestRef.current?.row + 1 === startLineNumber + const isRunningQuery = startLineNumber === runningQueryLineNumber const handleRunClick = () => { void trackEvent(ConsoleEvent.EDITOR_GLYPH_RUN) @@ -955,12 +992,7 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { } cursorChangeTimeoutRef.current = window.setTimeout(() => { - const queriesToRun = getQueriesToRun( - editor, - queryOffsetsRef.current ?? [], - ) - queriesToRunRef.current = queriesToRun - dispatch(actions.query.setQueriesToRun(queriesToRun)) + syncQueriesToRun(editor, runWithSelectionRef.current) if (monacoRef.current && editorRef.current) { applyLineMarkings(monaco, editor, e.source) @@ -997,6 +1029,39 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { const notificationUpdates: Array<() => void> = [] + const inflightQueryBeforeChanges = + inflightQueryRef.current && + inflightQueryRef.current.bufferId === activeBufferId + ? inflightQueryRef.current + : null + let inflightQueryAfterChanges: InflightQuery | null = null + if (inflightQueryBeforeChanges) { + const shiftedInflightQuery = shiftInflightQuery( + inflightQueryBeforeChanges, + e.changes, + ) + inflightQueryRef.current = shiftedInflightQuery + inflightQueryAfterChanges = shiftedInflightQuery + if ( + shiftedInflightQuery.queryKey !== inflightQueryBeforeChanges.queryKey + ) { + questExecution.rekeyActive( + inflightQueryBeforeChanges.queryKey, + shiftedInflightQuery.queryKey, + ) + // Dispatched before this handler's await below: the settle handlers + // read the live inflight key, so a query completing during the await + // must already find its notification under the shifted key. + dispatch( + actions.query.updateNotificationKey( + inflightQueryBeforeChanges.queryKey, + shiftedInflightQuery.queryKey, + activeBufferId, + ), + ) + } + } + if (bufferExecutions) { const keysToUpdate: Array<{ oldKey: QueryKey @@ -1059,6 +1124,13 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { Object.keys(currentNotifications).forEach((key) => { const queryKey = key as QueryKey + if ( + queryKey === inflightQueryBeforeChanges?.queryKey || + queryKey === inflightQueryAfterChanges?.queryKey + ) { + return + } + const { queryText, startOffset, endOffset } = parseQueryKey(queryKey) const effectiveOffsetDelta = e.changes .filter((change) => change.rangeOffset < endOffset) @@ -1111,12 +1183,7 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { applyGlyphsAndLineMarkings(monaco, editor) } - const queriesToRun = getQueriesToRun( - editor, - queryOffsetsRef.current ?? [], - ) - queriesToRunRef.current = queriesToRun - dispatch(actions.query.setQueriesToRun(queriesToRun)) + syncQueriesToRun(editor, runWithSelectionRef.current) contentJustChangedRef.current = false notificationUpdates.forEach((update) => update()) @@ -1239,12 +1306,13 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { // Initial decoration setup applyGlyphsAndLineMarkings(monaco, editor) - const queriesToRun = getQueriesToRun( + // A ?query link selects its statements to run them all; that selection + // must be honored even when the run-with-selection setting is off. + const runsShareLinkSelection = Boolean(query && executeQuery) + const queriesToRun = syncQueriesToRun( editor, - queryOffsetsRef.current ?? [], + runsShareLinkSelection || runWithSelectionRef.current, ) - queriesToRunRef.current = queriesToRun - dispatch(actions.query.setQueriesToRun(queriesToRun)) if (!query || !executeQuery) { triggerJitValidation() @@ -1253,6 +1321,7 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { if (queriesToRun.length > 1) { handleTriggerRunScript() } else { + shareLinkSelectionRunRef.current = true toggleRunning() } } @@ -1706,6 +1775,13 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { } } + useEffect(() => { + runWithSelectionRef.current = runWithSelection + const editor = editorRef.current + if (!editor) return + syncQueriesToRun(editor, runWithSelection) + }, [runWithSelection]) + useEffect(() => { canUseAIRef.current = canUseAI const lineCount = editorRef.current?.getModel()?.getLineCount() @@ -1778,25 +1854,49 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { if (![RunningType.NONE, RunningType.SCRIPT].includes(running)) { applyGlyphsAndLineMarkings(monaco, editor) + // Consumed once: only the share-link mount path sets it, right before + // dispatching this run. + const honorShareLinkSelection = shareLinkSelectionRunRef.current + shareLinkSelectionRunRef.current = false const request = running === RunningType.REFRESH ? getQueryRequestFromLastExecutedQuery(lastExecutedQuery) - : getQueryRequestFromEditor(editor) + : getQueryRequestFromEditor( + editor, + honorShareLinkSelection || runWithSelectionRef.current, + ) const isRunningExplain = running === RunningType.EXPLAIN const targetBufferId = activeBufferRef.current.id as number if (request?.query) { - editor.updateOptions({ readOnly: true }) const parentQuery = request.query - const parentQueryKey = createQueryKeyFromRequest(editor, request) + const runStartOffset = getQueryStartOffset(editor, request) + const runModelVersionId = editor.getModel()?.getVersionId() ?? null + const parentQueryKey = createQueryKey(parentQuery, runStartOffset) + const runSeq = ++runSeqRef.current + const isCurrentRun = () => runSeqRef.current === runSeq + inflightQueryRef.current = createInflightQuery( + targetBufferId, + parentQuery, + runStartOffset, + ) questExecution.markActive(targetBufferId, parentQueryKey, () => { if (editorQueryIdRef.current !== null) { quest.abort(editorQueryIdRef.current) editorQueryIdRef.current = null } }) + + const staleExecutions = executionRefs.current[targetBufferId.toString()] + if (staleExecutions && staleExecutions[parentQueryKey]) { + delete staleExecutions[parentQueryKey] + if (Object.keys(staleExecutions).length === 0) { + cleanupExecutionRefs(targetBufferId) + } + } + const originalQueryText = request.selection ? request.selection.queryText : request.query @@ -1807,12 +1907,17 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { // give the notification a slight delay to prevent flashing for fast queries notificationTimeoutRef.current = window.setTimeout(() => { - if (runningValueRef.current && requestRef.current && editor) { + if ( + isCurrentRun() && + runningValueRef.current && + requestRef.current && + editor + ) { dispatch( actions.query.addNotification( { type: NotificationType.LOADING, - query: parentQueryKey, + query: inflightQueryRef.current?.queryKey ?? parentQueryKey, isExplain: isRunningExplain, content: ( @@ -1834,6 +1939,35 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { notificationTimeoutRef.current = null }, 1000) + const settleExecution = () => { + // A run superseded by a newer one settles under its own key and must + // not touch the shared running state the newer run now owns. + if (!isCurrentRun()) { + return { inflightQuery: null, currentQueryKey: parentQueryKey } + } + if (notificationTimeoutRef.current) { + window.clearTimeout(notificationTimeoutRef.current) + notificationTimeoutRef.current = null + } + setRequest(undefined) + dispatch(actions.query.stopRunning()) + const inflightQuery = inflightQueryRef.current + const currentQueryKey = inflightQuery?.queryKey ?? parentQueryKey + questExecution.releaseExecution(currentQueryKey) + return { inflightQuery, currentQueryKey } + } + + const getAnchoredInflightQuery = ( + inflightQuery: InflightQuery | null, + model: editor.ITextModel | null, + ): InflightQuery | null => + inflightQuery !== null && + model !== null && + activeBufferRef.current.id === targetBufferId && + isInflightQueryStillInPlace(model.getValue(), inflightQuery) + ? inflightQuery + : null + const { promise: queryPromise, queryId } = quest.queryRaw( normalizeQueryText(queryToRun), { @@ -1846,19 +1980,12 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { void queryPromise .then((result) => { - if (notificationTimeoutRef.current) { - window.clearTimeout(notificationTimeoutRef.current) - notificationTimeoutRef.current = null - } - - setRequest(undefined) - dispatch(actions.query.stopRunning()) - questExecution.releaseExecution(parentQueryKey) + const { inflightQuery, currentQueryKey } = settleExecution() if (!editorRef.current) return const targetBufferIdStr = targetBufferId.toString() - if (executionRefs.current[targetBufferIdStr] && editorRef.current) { - delete executionRefs.current[targetBufferIdStr][parentQueryKey] + if (executionRefs.current[targetBufferIdStr]) { + delete executionRefs.current[targetBufferIdStr][currentQueryKey] if ( Object.keys(executionRefs.current[targetBufferIdStr]).length === 0 @@ -1867,26 +1994,20 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { } } - if (request.selection) { - const model = editorRef.current.getModel() - if (model) { - const targetBufferIdStr = targetBufferId.toString() - if (!executionRefs.current[targetBufferIdStr]) { - executionRefs.current[targetBufferIdStr] = {} - } + const model = editorRef.current.getModel() + const anchoredQuery = getAnchoredInflightQuery(inflightQuery, model) + if (request.selection && anchoredQuery) { + if (!executionRefs.current[targetBufferIdStr]) { + executionRefs.current[targetBufferIdStr] = {} + } - const queryStartOffset = getQueryStartOffset( - editorRef.current, - request, - ) - executionRefs.current[targetBufferIdStr][parentQueryKey] = { - success: true, - selection: request.selection, - queryText: parentQuery, - startOffset: queryStartOffset, - endOffset: - queryStartOffset + normalizeQueryText(parentQuery).length, - } + const offsetDelta = anchoredQuery.startOffset - runStartOffset + executionRefs.current[targetBufferIdStr][currentQueryKey] = { + success: true, + selection: shiftSelection(request.selection, offsetDelta), + queryText: parentQuery, + startOffset: anchoredQuery.startOffset, + endOffset: anchoredQuery.endOffset, } } @@ -1899,7 +2020,7 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { dispatch( actions.query.addNotification( { - query: parentQueryKey, + query: currentQueryKey, isExplain: isRunningExplain, content: , }, @@ -1913,7 +2034,7 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { dispatch( actions.query.addNotification( { - query: parentQueryKey, + query: currentQueryKey, isExplain: isRunningExplain, content: ( @@ -1937,7 +2058,7 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { dispatch( actions.query.addNotification( { - query: parentQueryKey, + query: currentQueryKey, isExplain: isRunningExplain, jitCompiled: result.explain?.jitCompiled ?? false, content: ( @@ -1955,75 +2076,94 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { } }) .catch((error: ErrorResult) => { - if (notificationTimeoutRef.current) { - window.clearTimeout(notificationTimeoutRef.current) - notificationTimeoutRef.current = null - } - - setRequest(undefined) - dispatch(actions.query.stopRunning()) - questExecution.releaseExecution(parentQueryKey) + const { inflightQuery, currentQueryKey } = settleExecution() if (editorRef?.current && monacoRef?.current) { - // For error positioning, we need to use the original request (without EXPLAIN prefix) - // but adjust the error position if it was an EXPLAIN query - let adjustedErrorPosition = error.position - if (isRunningExplain) { - // Adjust error position to account for removed "EXPLAIN " prefix - adjustedErrorPosition = Math.max(0, error.position - 8) - } - if (request.selection) { - adjustedErrorPosition += parentQuery.indexOf( - request.selection.queryText, - ) - } - - const errorRange = getErrorRange( - editorRef.current, - request, - adjustedErrorPosition, + const model = editorRef.current.getModel() + const anchoredQuery = getAnchoredInflightQuery( + inflightQuery, + model, ) + if (anchoredQuery && model) { + // For error positioning, we need to use the original request (without EXPLAIN prefix) + // but adjust the error position if it was an EXPLAIN query + let adjustedErrorPosition = error.position + if (isRunningExplain) { + // Adjust error position to account for removed "EXPLAIN " prefix + adjustedErrorPosition = Math.max(0, error.position - 8) + } + if (request.selection) { + adjustedErrorPosition += parentQuery.indexOf( + request.selection.queryText, + ) + } - const errorToStore = { ...error, position: adjustedErrorPosition } + const offsetDelta = anchoredQuery.startOffset - runStartOffset + const currentQueryPosition = model.getPositionAt( + anchoredQuery.startOffset, + ) + const currentRequest: Request = { + ...request, + row: currentQueryPosition.lineNumber - 1, + column: currentQueryPosition.column, + selection: + request.selection && + shiftSelection(request.selection, offsetDelta), + } - // Use the already-defined parentQueryKey instead of recalculating it here - const targetBufferIdStr = targetBufferId.toString() - if (!executionRefs.current[targetBufferIdStr]) { - executionRefs.current[targetBufferIdStr] = {} - } + const errorRange = getErrorRange( + editorRef.current, + currentRequest, + adjustedErrorPosition, + ) - const startOffset = getQueryStartOffset( - editorRef.current, - request, - ) - executionRefs.current[targetBufferIdStr][parentQueryKey] = { - error: errorToStore, - selection: request.selection, - queryText: parentQuery, - startOffset, - endOffset: startOffset + normalizeQueryText(parentQuery).length, - } + const errorToStore = { + ...error, + position: adjustedErrorPosition, + } - if (errorRange) { - editorRef?.current.focus() + const targetBufferIdStr = targetBufferId.toString() + if (!executionRefs.current[targetBufferIdStr]) { + executionRefs.current[targetBufferIdStr] = {} + } - if (!request.selection) { - editorRef?.current.setPosition({ + executionRefs.current[targetBufferIdStr][currentQueryKey] = { + error: errorToStore, + selection: currentRequest.selection, + queryText: parentQuery, + startOffset: anchoredQuery.startOffset, + endOffset: anchoredQuery.endOffset, + } + + const bufferUntouchedSinceRun = + model.getVersionId() === runModelVersionId + const isCancelledByUser = + String(error.error) === "Cancelled by user" + if ( + errorRange && + bufferUntouchedSinceRun && + !isCancelledByUser + ) { + editorRef?.current.focus() + + if (!request.selection) { + editorRef?.current.setPosition({ + lineNumber: errorRange.startLineNumber, + column: errorRange.startColumn, + }) + } + + editorRef?.current.revealPosition({ lineNumber: errorRange.startLineNumber, - column: errorRange.startColumn, + column: errorRange.endColumn, }) } - - editorRef?.current.revealPosition({ - lineNumber: errorRange.startLineNumber, - column: errorRange.endColumn, - }) } dispatch( actions.query.addNotification( { - query: parentQueryKey, + query: currentQueryKey, isExplain: isRunningExplain, content: {error.error}, sideContent: , @@ -2035,6 +2175,9 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { } }) .finally(() => { + if (isCurrentRun()) { + inflightQueryRef.current = null + } if (!scriptConfirmationOpenRef.current) { executePendingScriptRun() } @@ -2082,7 +2225,6 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { if (monacoRef?.current && editorRef?.current) { applyGlyphsAndLineMarkings(monacoRef.current, editorRef.current) } - editorRef.current?.updateOptions({ readOnly: !!request }) }, [request]) useEffect(() => { diff --git a/src/scenes/Editor/Monaco/tabs.tsx b/src/scenes/Editor/Monaco/tabs.tsx index 901498a7f..4e1d3893b 100644 --- a/src/scenes/Editor/Monaco/tabs.tsx +++ b/src/scenes/Editor/Monaco/tabs.tsx @@ -9,6 +9,7 @@ import { UploadSimpleIcon, NotebookIcon, FileTextIcon, + GearSixIcon, } from "@phosphor-icons/react" import { createDefaultNotebookViewState } from "../../../store/notebook" import { toast } from "../../../components/Toast" @@ -22,6 +23,7 @@ import { migrateBuffer, getCurrentDbVersion } from "../../../store/migrations" import { importInto, peakImportFile } from "dexie-export-import" import { exportBuffers } from "./exportTabs" import { ImportSummaryDialog, SkippedTab } from "./ImportSummaryDialog" +import { EditorSettingsModal } from "../../../components/EditorSettingsModal" import { Box, Button, @@ -119,6 +121,7 @@ export const Tabs = () => { x: number y: number } | null>(null) + const [settingsOpen, setSettingsOpen] = useState(false) const [importSummaryOpen, setImportSummaryOpen] = useState(false) const [importedCount, setImportedCount] = useState(0) const [skippedTabs, setSkippedTabs] = useState([]) @@ -665,9 +668,18 @@ export const Tabs = () => { Export tabs + + setSettingsOpen(true)} + data-hook="editor-tabs-menu-settings" + > + + Editor settings + + { + let currentSelection = selection + + const toSelection = (sel: SingleLineSelection) => ({ + startLineNumber: 1, + startColumn: sel.startColumn, + endLineNumber: 1, + endColumn: sel.endColumn, + getStartPosition: () => ({ lineNumber: 1, column: sel.startColumn }), + getEndPosition: () => ({ lineNumber: 1, column: sel.endColumn }), + isEmpty: () => sel.startColumn === sel.endColumn, + }) + + const model = { + getValueInRange: (range: { startColumn: number; endColumn: number }) => + text.substring(range.startColumn - 1, range.endColumn - 1), + getOffsetAt: (position: { column: number }) => position.column - 1, + getPositionAt: (offset: number) => ({ lineNumber: 1, column: offset + 1 }), + } + + return { + getModel: () => model, + getValue: () => text, + getPosition: () => ({ lineNumber: 1, column: cursorColumn }), + getSelection: () => + currentSelection ? toSelection(currentSelection) : null, + setSelection: (range: { startColumn: number; endColumn: number }) => { + currentSelection = { + startColumn: range.startColumn, + endColumn: range.endColumn, + } + }, + } as unknown as editor.IStandaloneCodeEditor +} + describe("getQueriesFromText", () => { it("splits two simple statements", () => { expect(getQueriesFromText("SELECT 1; SELECT 2;")).toEqual([ @@ -327,3 +377,407 @@ describe("isCursorInQuotedIdentifier", () => { expect(isCursorInQuotedIdentifier(text, 0, cursor)).toBe(text.indexOf('"')) }) }) + +describe("run with selection gating", () => { + // "SELECT 11" spans columns 1..10 on the single line; the cursor sits inside it. + const TEXT = "SELECT 11; SELECT 22" + const FIRST_STATEMENT_SELECTION = { startColumn: 1, endColumn: 10 } + const QUERY_OFFSETS = [ + { startOffset: 0, endOffset: 9 }, + { startOffset: 11, endOffset: 20 }, + ] + + describe("getQueriesToRun", () => { + it("runs only the selected text when enabled", () => { + // Given the first statement is selected and running with selection is enabled + const editor = makeSingleLineEditor(TEXT, 3, FIRST_STATEMENT_SELECTION) + + // When resolving the queries to run + const result = getQueriesToRun(editor, QUERY_OFFSETS, true) + + // Then the run carries the selection + expect(result.length).toBeGreaterThan(0) + expect(result.some((request) => request.selection)).toBe(true) + }) + + it("ignores the selection and runs the cursor query when disabled", () => { + // Given the first statement is selected but running with selection is disabled + const editor = makeSingleLineEditor(TEXT, 3, FIRST_STATEMENT_SELECTION) + + // When resolving the queries to run + const result = getQueriesToRun(editor, QUERY_OFFSETS, false) + + // Then it falls back to the cursor query with no selection attached + expect(result).toEqual([getQueryFromCursor(editor)]) + expect(result.every((request) => !request.selection)).toBe(true) + }) + }) + + describe("getQueryRequestFromEditor", () => { + it("builds a selection request when enabled", () => { + // Given the first statement is selected and running with selection is enabled + const editor = makeSingleLineEditor(TEXT, 3, FIRST_STATEMENT_SELECTION) + + // When building the request from the editor + const request = getQueryRequestFromEditor(editor, true) + + // Then the request carries the selection + expect(request?.selection).toBeDefined() + }) + + it("ignores the selection and uses the cursor query when disabled", () => { + // Given the first statement is selected but running with selection is disabled + const editor = makeSingleLineEditor(TEXT, 3, FIRST_STATEMENT_SELECTION) + + // When building the request from the editor + const request = getQueryRequestFromEditor(editor, false) + + // Then the request has no selection + expect(request?.selection).toBeUndefined() + }) + }) +}) + +describe("isQueryTextAtOffset", () => { + it("matches when the query is still at its offset", () => { + // Given a buffer holding the query at a known offset + const text = "SELECT a FROM t;\nSELECT b FROM u;" + + // When checking the second query at its start offset + const result = isQueryTextAtOffset(text, 17, "SELECT b FROM u") + + // Then it reports the query is still in place + expect(result).toBe(true) + }) + + it("does not match when text was inserted so the offset points elsewhere", () => { + // Given text inserted above shifts the query past its original offset + const text = "-- note\nSELECT a FROM t;" + + // When checking at the offset the query used to start at + const result = isQueryTextAtOffset(text, 0, "SELECT a FROM t") + + // Then it reports the query is no longer in place + expect(result).toBe(false) + }) + + it("does not match when the text at the offset was edited", () => { + // Given the query text at the offset was changed + const text = "SELECT z FROM t;" + + // When checking against the original query text + const result = isQueryTextAtOffset(text, 0, "SELECT a FROM t") + + // Then it reports the query is no longer in place + expect(result).toBe(false) + }) + + it("ignores a trailing semicolon difference", () => { + // Given the buffer keeps a trailing semicolon the request text omits + const text = "SELECT a FROM t;" + + // When checking against the normalized query without the semicolon + const result = isQueryTextAtOffset(text, 0, "SELECT a FROM t") + + // Then it still reports the query is in place + expect(result).toBe(true) + }) +}) + +describe("shiftInflightQuery", () => { + // "SELECT a FROM t" is 15 chars, so the query spans offsets 20-35 + const runningQuery = () => createInflightQuery(1, "SELECT a FROM t", 20) + + it("shifts the query forward when text is inserted above it", () => { + // Given a query running at offset 20 + const inflightQuery = runningQuery() + + // When 5 characters are inserted above the query + const shifted = shiftInflightQuery(inflightQuery, [ + { rangeOffset: 3, rangeLength: 0, text: "hello" }, + ]) + + // Then the offsets and the key move by the inserted length + expect(shifted).toEqual({ + bufferId: 1, + queryKey: "SELECT a FROM t@25-40", + startOffset: 25, + endOffset: 40, + dislodged: false, + }) + }) + + it("shifts the query backward when text is deleted above it", () => { + // Given a query running at offset 20 + const inflightQuery = runningQuery() + + // When 4 characters are deleted above the query + const shifted = shiftInflightQuery(inflightQuery, [ + { rangeOffset: 0, rangeLength: 4, text: "" }, + ]) + + // Then the offsets and the key move back by the deleted length + expect(shifted.queryKey).toBe("SELECT a FROM t@16-31") + expect(shifted.startOffset).toBe(16) + expect(shifted.endOffset).toBe(31) + }) + + it("shifts by the net delta when a change above replaces text", () => { + // Given a query running at offset 20 + const inflightQuery = runningQuery() + + // When 4 characters above are replaced with 2 + const shifted = shiftInflightQuery(inflightQuery, [ + { rangeOffset: 5, rangeLength: 4, text: "ab" }, + ]) + + // Then the query moves back by the net difference + expect(shifted.startOffset).toBe(18) + expect(shifted.endOffset).toBe(33) + }) + + it("shifts when text is inserted exactly at the query start", () => { + // Given a query running at offset 20 + const inflightQuery = runningQuery() + + // When a character is inserted at offset 20 + const shifted = shiftInflightQuery(inflightQuery, [ + { rangeOffset: 20, rangeLength: 0, text: "x" }, + ]) + + // Then the query text is pushed forward intact + expect(shifted.startOffset).toBe(21) + expect(shifted.dislodged).toBe(false) + }) + + it("shifts when a deletion ends exactly at the query start", () => { + // Given a query running at offsets 20-35 + const inflightQuery = runningQuery() + + // When a deletion whose range ends exactly at the query start is applied + const shifted = shiftInflightQuery(inflightQuery, [ + { rangeOffset: 15, rangeLength: 5, text: "" }, + ]) + + // Then the query shifts back rather than being dislodged + expect(shifted.dislodged).toBe(false) + expect(shifted.startOffset).toBe(15) + expect(shifted.endOffset).toBe(30) + }) + + it("stays untouched when the edit is after the query end", () => { + // Given a query running at offsets 20-35 + const inflightQuery = runningQuery() + + // When text is inserted past the query end + const shifted = shiftInflightQuery(inflightQuery, [ + { rangeOffset: 36, rangeLength: 0, text: "x" }, + ]) + + // Then nothing changes + expect(shifted).toBe(inflightQuery) + }) + + it("stays untouched when text is deleted right at the query end", () => { + // Given a query running at offsets 20-35 + const inflightQuery = runningQuery() + + // When text after the query is deleted starting at its end + const shifted = shiftInflightQuery(inflightQuery, [ + { rangeOffset: 35, rangeLength: 3, text: "" }, + ]) + + // Then nothing changes + expect(shifted).toBe(inflightQuery) + }) + + it("dislodges the query when text is inserted exactly at its end", () => { + // Given a query running at offsets 20-35 + const inflightQuery = runningQuery() + + // When text is appended right at the query end, extending the statement + const shifted = shiftInflightQuery(inflightQuery, [ + { rangeOffset: 35, rangeLength: 0, text: " where x = 1" }, + ]) + + // Then the query is dislodged + expect(shifted.dislodged).toBe(true) + }) + + it("dislodges the query when an edit lands inside it", () => { + // Given a query running at offsets 20-35 + const inflightQuery = runningQuery() + + // When a character is inserted in the middle of the query + const shifted = shiftInflightQuery(inflightQuery, [ + { rangeOffset: 25, rangeLength: 0, text: "x" }, + ]) + + // Then the query is dislodged and keeps its last known key + expect(shifted.dislodged).toBe(true) + expect(shifted.queryKey).toBe(inflightQuery.queryKey) + }) + + it("dislodges the query when a deletion straddles its start", () => { + // Given a query running at offsets 20-35 + const inflightQuery = runningQuery() + + // When a deletion covers text above and inside the query + const shifted = shiftInflightQuery(inflightQuery, [ + { rangeOffset: 18, rangeLength: 5, text: "" }, + ]) + + // Then the query is dislodged + expect(shifted.dislodged).toBe(true) + }) + + it("applies only the changes above the query when an event carries several", () => { + // Given a query running at offset 20 + const inflightQuery = runningQuery() + + // When one change lands above the query and one after it + const shifted = shiftInflightQuery(inflightQuery, [ + { rangeOffset: 0, rangeLength: 0, text: "abc" }, + { rangeOffset: 40, rangeLength: 2, text: "" }, + ]) + + // Then only the change above contributes to the shift + expect(shifted.startOffset).toBe(23) + expect(shifted.endOffset).toBe(38) + }) + + it("accumulates the deltas of several changes above the query", () => { + // Given a query running at offset 20 + const inflightQuery = runningQuery() + + // When two changes land above the query: +2 chars and -1 char + const shifted = shiftInflightQuery(inflightQuery, [ + { rangeOffset: 0, rangeLength: 0, text: "ab" }, + { rangeOffset: 5, rangeLength: 1, text: "" }, + ]) + + // Then the shift is the sum of both deltas + expect(shifted.startOffset).toBe(21) + expect(shifted.endOffset).toBe(36) + }) + + it("never shifts again once dislodged", () => { + // Given a query that was dislodged by an inner edit + const dislodgedQuery = shiftInflightQuery(runningQuery(), [ + { rangeOffset: 25, rangeLength: 0, text: "x" }, + ]) + + // When more text is inserted above it + const shifted = shiftInflightQuery(dislodgedQuery, [ + { rangeOffset: 0, rangeLength: 0, text: "abc" }, + ]) + + // Then it stays dislodged at its last known offsets + expect(shifted).toBe(dislodgedQuery) + }) +}) + +describe("isInflightQueryStillInPlace", () => { + it("reports in place while the query text sits at the tracked offset", () => { + // Given a query tracked at the start of the buffer + const inflightQuery = createInflightQuery(1, "SELECT a FROM t", 0) + + // When the buffer still holds the query there + const result = isInflightQueryStillInPlace( + "SELECT a FROM t;", + inflightQuery, + ) + + // Then the query is still in place + expect(result).toBe(true) + }) + + it("reports in place at the shifted offset after edits above", () => { + // Given a running query shifted by an insertion above it + const inflightQuery = shiftInflightQuery( + createInflightQuery(1, "SELECT a FROM t", 0), + [{ rangeOffset: 0, rangeLength: 0, text: "-- note\n" }], + ) + + // When the buffer holds the query at the shifted offset + const result = isInflightQueryStillInPlace( + "-- note\nSELECT a FROM t;", + inflightQuery, + ) + + // Then the query is still in place + expect(result).toBe(true) + }) + + it("reports out of place once the query is dislodged", () => { + // Given a query dislodged by an inner edit + const inflightQuery = shiftInflightQuery( + createInflightQuery(1, "SELECT a FROM t", 0), + [{ rangeOffset: 5, rangeLength: 0, text: "x" }], + ) + + // When checking against a buffer that still matches the tracked offset + const result = isInflightQueryStillInPlace( + "SELECT a FROM t;", + inflightQuery, + ) + + // Then the query is no longer trusted to be in place + expect(result).toBe(false) + }) + + it("reports out of place when the buffer text at the offset differs", () => { + // Given a query tracked at the start of the buffer + const inflightQuery = createInflightQuery(1, "SELECT a FROM t", 0) + + // When the buffer content changed underneath it + const result = isInflightQueryStillInPlace( + "SELECT z FROM t;", + inflightQuery, + ) + + // Then the query is no longer in place + expect(result).toBe(false) + }) +}) + +describe("shiftSelection", () => { + const selection = { + startOffset: 20, + endOffset: 35, + queryText: "SELECT a FROM t", + } + + it("moves both offsets forward by a positive delta and keeps the query text", () => { + // Given a stored selection + // When it is shifted forward by inserted text above it + const shifted = shiftSelection(selection, 5) + + // Then both offsets move by the delta and the query text is preserved + expect(shifted).toEqual({ + startOffset: 25, + endOffset: 40, + queryText: "SELECT a FROM t", + }) + }) + + it("moves both offsets backward by a negative delta", () => { + // Given a stored selection + // When it is shifted back by deleted text above it + const shifted = shiftSelection(selection, -4) + + // Then both offsets move back by the delta + expect(shifted.startOffset).toBe(16) + expect(shifted.endOffset).toBe(31) + }) + + it("leaves the offsets unchanged for a zero delta", () => { + // Given a stored selection + // When it is shifted by nothing + const shifted = shiftSelection(selection, 0) + + // Then the offsets are unchanged + expect(shifted.startOffset).toBe(20) + expect(shifted.endOffset).toBe(35) + }) +}) diff --git a/src/scenes/Editor/Monaco/utils.ts b/src/scenes/Editor/Monaco/utils.ts index 671f9e5ef..e176e6b9c 100644 --- a/src/scenes/Editor/Monaco/utils.ts +++ b/src/scenes/Editor/Monaco/utils.ts @@ -138,13 +138,14 @@ export const getSelectedText = ( export const getQueriesToRun = ( editor: IStandaloneCodeEditor, queryOffsets: { startOffset: number; endOffset: number }[], + runWithSelection: boolean, ): Request[] => { const model = editor.getModel() if (!model) return [] const selection = editor.getSelection() const selectedText = selection ? model.getValueInRange(selection) : undefined - if (!selection || !selectedText) { + if (!runWithSelection || !selection || !selectedText) { const queryInCursor = getQueryFromCursor(editor) if (queryInCursor) { return [queryInCursor] @@ -725,6 +726,7 @@ export const getQueryFromSelection = ( export const getQueryRequestFromEditor = ( editor: IStandaloneCodeEditor, + runWithSelection: boolean, ): Request | undefined => { let request: Request | undefined const selectedText = getSelectedText(editor) @@ -732,7 +734,7 @@ export const getQueryRequestFromEditor = ( ? stripSQLComments(normalizeQueryText(selectedText)) : undefined - if (strippedNormalizedSelectedText) { + if (runWithSelection && strippedNormalizedSelectedText) { request = getQueryFromSelection(editor) } else { request = getQueryFromCursor(editor) @@ -1004,6 +1006,15 @@ export const getQueryStartOffset = ( }) } +export const isQueryTextAtOffset = ( + fullText: string, + startOffset: number, + expectedQueryText: string, +): boolean => { + const expected = normalizeQueryText(expectedQueryText) + return fullText.slice(startOffset, startOffset + expected.length) === expected +} + export const createQueryKey = ( queryText: string, startOffset: number, @@ -1072,6 +1083,80 @@ export const createQueryKeyFromRequest = ( return createQueryKey(request.query, startOffset) } +export type InflightQuery = { + bufferId: number + queryKey: QueryKey + startOffset: number + endOffset: number + dislodged: boolean +} + +type ContentChange = Pick< + editor.IModelContentChange, + "rangeOffset" | "rangeLength" | "text" +> + +export const createInflightQuery = ( + bufferId: number, + queryText: string, + startOffset: number, +): InflightQuery => ({ + bufferId, + queryKey: createQueryKey(queryText, startOffset), + startOffset, + endOffset: startOffset + normalizeQueryText(queryText).length, + dislodged: false, +}) + +export const shiftInflightQuery = ( + inflightQuery: InflightQuery, + changes: readonly ContentChange[], +): InflightQuery => { + if (inflightQuery.dislodged) return inflightQuery + + const { startOffset, endOffset } = inflightQuery + const touchesQuery = changes.some( + (change) => + change.rangeOffset + change.rangeLength > startOffset && + (change.rangeOffset < endOffset || + (change.rangeOffset === endOffset && change.text.length > 0)), + ) + if (touchesQuery) { + return { ...inflightQuery, dislodged: true } + } + + const offsetDelta = changes + .filter((change) => change.rangeOffset + change.rangeLength <= startOffset) + .reduce((acc, change) => acc + change.text.length - change.rangeLength, 0) + if (offsetDelta === 0) return inflightQuery + + const { queryText } = parseQueryKey(inflightQuery.queryKey) + return { + ...inflightQuery, + queryKey: createQueryKey(queryText, startOffset + offsetDelta), + startOffset: startOffset + offsetDelta, + endOffset: endOffset + offsetDelta, + } +} + +export const isInflightQueryStillInPlace = ( + fullText: string, + inflightQuery: InflightQuery, +): boolean => { + if (inflightQuery.dislodged) return false + const { queryText } = parseQueryKey(inflightQuery.queryKey) + return isQueryTextAtOffset(fullText, inflightQuery.startOffset, queryText) +} + +export const shiftSelection = ( + selection: NonNullable, + offsetDelta: number, +): NonNullable => ({ + ...selection, + startOffset: selection.startOffset + offsetDelta, + endOffset: selection.endOffset + offsetDelta, +}) + export const setErrorMarkerForQuery = ( monaco: Monaco, editor: IStandaloneCodeEditor, diff --git a/src/scenes/Editor/Notebook/cells/Cell.tsx b/src/scenes/Editor/Notebook/cells/Cell.tsx index 597949f70..3d64f62cb 100644 --- a/src/scenes/Editor/Notebook/cells/Cell.tsx +++ b/src/scenes/Editor/Notebook/cells/Cell.tsx @@ -26,6 +26,7 @@ import { useCellSelectionDecoration } from "./useCellSelectionDecoration" import { useMonacoCellEditor } from "./useMonacoCellEditor" import { useCellReveal } from "./useCellReveal" import { useEditor } from "../../../../providers/EditorProvider" +import { useLocalStorage } from "../../../../providers/LocalStorageProvider" import { emitUserAction, signalUserEdit, @@ -190,6 +191,7 @@ const CellInner: React.FC = ({ const theme = useTheme() const { quest } = React.useContext(QuestContext) const { activeBuffer } = useEditor() + const { runWithSelection } = useLocalStorage() const bufferIdForEvents = typeof activeBuffer.id === "number" ? activeBuffer.id : undefined const isDrawMode = cell.mode === "draw" @@ -411,6 +413,7 @@ const CellInner: React.FC = ({ }, [editorRef]) const tryRunSelection = useCallback(async (): Promise => { + if (!runWithSelection) return false const ed = editorRef.current if (!ed) return false const selection = ed.getSelection() @@ -425,7 +428,14 @@ const CellInner: React.FC = ({ const success = await runCell(cell.id, normalized) applyHighlight(success) return true - }, [cell.id, runCell, editorRef, applyHighlight, clearHighlight]) + }, [ + runWithSelection, + cell.id, + runCell, + editorRef, + applyHighlight, + clearHighlight, + ]) const emitRanEvent = useCallback( (status: RanStatus) => { diff --git a/src/scenes/Editor/Notebook/result-table/ResultGridPanel.tsx b/src/scenes/Editor/Notebook/result-table/ResultGridPanel.tsx index 93841fb78..cae522fb4 100644 --- a/src/scenes/Editor/Notebook/result-table/ResultGridPanel.tsx +++ b/src/scenes/Editor/Notebook/result-table/ResultGridPanel.tsx @@ -15,6 +15,7 @@ import { removeNotebookColumnLayout, } from "../notebookColumnLayoutStore" import { ResultActionsBar } from "./ResultActionsBar" +import { useLocalStorage } from "../../../../providers/LocalStorageProvider" type Props = { data: DqlQueryResult @@ -41,6 +42,7 @@ export const ResultGridPanel: React.FC = ({ onYieldFocus, }) => { const gridRef = useRef(null) + const { maxColumnWidth } = useLocalStorage() const dataSource = useMemo( () => inMemoryDataSource(data.columns, data.dataset, data.timestamp ?? -1), @@ -76,6 +78,7 @@ export const ResultGridPanel: React.FC = ({ + ({ + current: { + getModel: () => ({ + getPositionAt: (offset: number) => ({ + lineNumber: 1, + column: offset + 1, + }), + getWordAtPosition: () => ({ word: "foo" }), + getValueInRange: () => "select foo", + }), + }, + }) as unknown as MutableRefObject + +const refsWith = (execution: unknown): MutableRefObject => + ({ + current: { [BUFFER_ID]: { [QUERY_KEY]: execution } }, + }) as unknown as MutableRefObject + +describe("extractErrorByQueryKey", () => { + it("returns the error details when an error execution is stored for the key", () => { + // Given a stored execution that failed for the query key + const executionRefs = refsWith({ + error: { error: "boom", position: 7 }, + queryText: "select foo", + startOffset: 0, + endOffset: 10, + }) + + // When the error is extracted + const result = extractErrorByQueryKey( + QUERY_KEY, + BUFFER_ID, + executionRefs, + editorWithModel(), + ) + + // Then the error message and offending word come back + expect(result?.errorMessage).toBe("boom") + expect(result?.word).toBe("foo") + expect(result?.queryText).toBe("select foo") + }) + + it("returns null when no execution is stored for the key", () => { + // Given a query that was edited mid-run so its offsets were never anchored, + // leaving no execution ref under its key + const executionRefs = { + current: { [BUFFER_ID]: {} }, + } as unknown as MutableRefObject + + // When the error is extracted + const result = extractErrorByQueryKey( + QUERY_KEY, + BUFFER_ID, + executionRefs, + editorWithModel(), + ) + + // Then there is nothing to fix, so the Fix button stays hidden + expect(result).toBeNull() + }) + + it("returns null when the stored execution succeeded", () => { + // Given a stored execution that carries no error + const executionRefs = refsWith({ + success: true, + queryText: "select foo", + startOffset: 0, + endOffset: 10, + }) + + // When the error is extracted + const result = extractErrorByQueryKey( + QUERY_KEY, + BUFFER_ID, + executionRefs, + editorWithModel(), + ) + + // Then there is no error to surface + expect(result).toBeNull() + }) + + it("returns null when the execution refs are absent", () => { + // Given no execution refs at all + // When the error is extracted + const result = extractErrorByQueryKey( + QUERY_KEY, + BUFFER_ID, + undefined, + editorWithModel(), + ) + + // Then it safely reports nothing to fix + expect(result).toBeNull() + }) +}) diff --git a/src/scenes/Result/ResultGridAdapter.tsx b/src/scenes/Result/ResultGridAdapter.tsx index df27597dc..a205b679a 100644 --- a/src/scenes/Result/ResultGridAdapter.tsx +++ b/src/scenes/Result/ResultGridAdapter.tsx @@ -15,6 +15,7 @@ import { import type { ColumnDefinition } from "../../utils/questdb/types" import type { IQuestDBGrid } from "../../js/console/grid" import { usePagedDataSource, type PaginationFn } from "./usePagedDataSource" +import { useLocalStorage } from "../../providers/LocalStorageProvider" import { trackEvent } from "../../modules/ConsoleEventTracker" import { ConsoleEvent } from "../../modules/ConsoleEventTracker/events" import { @@ -53,6 +54,7 @@ export const ResultGridAdapter = forwardRef< >(({ isFocused = true, paginationFn }, ref) => { const { dataSource, setResult, getSQL, getCurrentPageRows, hasData } = usePagedDataSource(paginationFn) + const { maxColumnWidth } = useLocalStorage() const [visible, setVisible] = useState(true) const [runToken, setRunToken] = useState(0) @@ -169,6 +171,7 @@ export const ResultGridAdapter = forwardRef< ({ + latest: { + query: queryKey, + createdAt, + content: null, + type: NotificationType.SUCCESS, + }, +}) + +const stateWithGroups = ( + groups: Record, +): QueryStateShape => ({ + ...initialState, + notifications: Object.values(groups).map((group) => group.latest), + queryNotifications: { [BUFFER_ID]: groups }, +}) + +describe("query reducer — UPDATE_NOTIFICATION_KEY", () => { + const oldKey = "select 1@10-18" as QueryKey + const newKey = "select 1@20-28" as QueryKey + + it("moves the notification group to the new key", () => { + // Given a notification group stored under the old key + const moved = notificationGroup(oldKey, new Date(1000)) + const state = stateWithGroups({ [oldKey]: moved }) + + // When the key is updated + const next = query( + state, + actions.updateNotificationKey(oldKey, newKey, BUFFER_ID), + ) + + // Then the group lives under the new key and the old key is gone + expect(next.queryNotifications[BUFFER_ID][newKey]).toBe(moved) + expect(next.queryNotifications[BUFFER_ID][oldKey]).toBeUndefined() + + // And the flat notification log follows the rename + expect(next.notifications[0].query).toBe(newKey) + }) + + it("keeps the old key alongside the new one when preserveOldKey is set", () => { + // Given a notification group stored under the old key + const moved = notificationGroup(oldKey, new Date(1000)) + const state = stateWithGroups({ [oldKey]: moved }) + + // When the key is updated with preserveOldKey + const next = query( + state, + actions.updateNotificationKey(oldKey, newKey, BUFFER_ID, true), + ) + + // Then the group is reachable under both keys + expect(next.queryNotifications[BUFFER_ID][newKey]).toBe(moved) + expect(next.queryNotifications[BUFFER_ID][oldKey]).toBe(moved) + }) + + it("overwrites an older group already sitting at the new key", () => { + // Given the new key holds a group older than the moved one + const moved = notificationGroup(oldKey, new Date(2000)) + const existing = notificationGroup(newKey, new Date(1000)) + const state = stateWithGroups({ [oldKey]: moved, [newKey]: existing }) + + // When the key is updated + const next = query( + state, + actions.updateNotificationKey(oldKey, newKey, BUFFER_ID), + ) + + // Then the newer moved group wins the collision + expect(next.queryNotifications[BUFFER_ID][newKey]).toBe(moved) + expect(next.queryNotifications[BUFFER_ID][oldKey]).toBeUndefined() + }) + + it("leaves the state untouched when the new key holds a newer group", () => { + // Given the new key holds a group newer than the moved one + const moved = notificationGroup(oldKey, new Date(1000)) + const existing = notificationGroup(newKey, new Date(2000)) + const state = stateWithGroups({ [oldKey]: moved, [newKey]: existing }) + + // When the key is updated + const next = query( + state, + actions.updateNotificationKey(oldKey, newKey, BUFFER_ID), + ) + + // Then the existing newer group survives and the moved one keeps its key + expect(next.queryNotifications[BUFFER_ID][newKey]).toBe(existing) + expect(next.queryNotifications[BUFFER_ID][oldKey]).toBe(moved) + + // And the flat notification log keeps the old key alongside the new one + expect(next.notifications.filter((n) => n.query === oldKey)).toHaveLength(1) + expect(next.notifications.filter((n) => n.query === newKey)).toHaveLength(1) + + // And the refusal is a referential no-op so no notification consumer rerenders + expect(next).toBe(state) + }) + + it("lets the moved group win when both groups share a timestamp", () => { + // Given the new key holds a group with the same createdAt as the moved one + const moved = notificationGroup(oldKey, new Date(1000)) + const existing = notificationGroup(newKey, new Date(1000)) + const state = stateWithGroups({ [oldKey]: moved, [newKey]: existing }) + + // When the key is updated + const next = query( + state, + actions.updateNotificationKey(oldKey, newKey, BUFFER_ID), + ) + + // Then the tie breaks toward the moved group, replacing the existing one + expect(next.queryNotifications[BUFFER_ID][newKey]).toBe(moved) + expect(next.queryNotifications[BUFFER_ID][oldKey]).toBeUndefined() + }) + + it("refuses the whole move when a newer group blocks the new key even with preserveOldKey", () => { + // Given a group to preserve at the old key and a newer group at the new key + const moved = notificationGroup(oldKey, new Date(1000)) + const existing = notificationGroup(newKey, new Date(2000)) + const state = stateWithGroups({ [oldKey]: moved, [newKey]: existing }) + + // When the key is updated with preserveOldKey + const next = query( + state, + actions.updateNotificationKey(oldKey, newKey, BUFFER_ID, true), + ) + + // Then the newer group is not overwritten and the old key is left as-is + expect(next.queryNotifications[BUFFER_ID][newKey]).toBe(existing) + expect(next.queryNotifications[BUFFER_ID][oldKey]).toBe(moved) + + // And the flat notification log keeps both keys, so the detached notification + // stays under the old key instead of clobbering the newer one + expect(next.notifications.filter((n) => n.query === oldKey)).toHaveLength(1) + expect(next.notifications.filter((n) => n.query === newKey)).toHaveLength(1) + + // And the refusal is a referential no-op so no notification consumer rerenders + expect(next).toBe(state) + }) +}) diff --git a/src/store/Query/reducers.ts b/src/store/Query/reducers.ts index 60e60319b..4353a2bb9 100644 --- a/src/store/Query/reducers.ts +++ b/src/store/Query/reducers.ts @@ -167,9 +167,18 @@ const query = (state = initialState, action: QueryAction): QueryStateShape => { const bufferNotifications = state.queryNotifications[bufferId] || {} const updatedBufferNotifications = { ...bufferNotifications } - if (updatedBufferNotifications[oldKey]) { - updatedBufferNotifications[newKey] = - updatedBufferNotifications[oldKey] + const moved = updatedBufferNotifications[oldKey] + const existing = updatedBufferNotifications[newKey] + if ( + moved && + existing && + existing.latest.createdAt > moved.latest.createdAt + ) { + return state + } + + if (moved) { + updatedBufferNotifications[newKey] = moved if (!preserveOldKey) { delete updatedBufferNotifications[oldKey] } diff --git a/src/utils/localStorage/types.ts b/src/utils/localStorage/types.ts index c24b54efe..ce23f445e 100644 --- a/src/utils/localStorage/types.ts +++ b/src/utils/localStorage/types.ts @@ -45,4 +45,6 @@ export enum StoreKey { AI_CHAT_PANEL_WIDTH = "ai.chat.panel.width", CLIENT_ID = "client.id", USE_NEW_GRID = "feature.new.grid", + RUN_WITH_SELECTION = "editor.runWithSelection", + MAX_COLUMN_WIDTH = "grid.maxColumnWidth", } diff --git a/src/utils/questdb/queryExecutionManager.test.ts b/src/utils/questdb/queryExecutionManager.test.ts index e26bd92f1..01cadaa48 100644 --- a/src/utils/questdb/queryExecutionManager.test.ts +++ b/src/utils/questdb/queryExecutionManager.test.ts @@ -287,6 +287,74 @@ describe("QueryExecutionManager", () => { ) }) + it("rekeys the active execution so release works with the new key", () => { + const manager = createManager() + const oldKey = "select 1@10-18" as QueryKey + const newKey = "select 1@20-28" as QueryKey + + manager.markActive(1, oldKey, vi.fn()) + manager.rekeyActive(oldKey, newKey) + + expect(manager.getActive()?.queryKey).toBe(newKey) + + manager.releaseExecution(oldKey) + expect(manager.getActive()?.queryKey).toBe(newKey) + + manager.releaseExecution(newKey) + expect(manager.getActive()).toBeNull() + }) + + it("ignores rekey when the old key does not match the active execution", () => { + const manager = createManager() + const activeKey = "select 1@10-18" as QueryKey + + manager.markActive(1, activeKey, vi.fn()) + manager.rekeyActive("other@0-5" as QueryKey, "other@5-10" as QueryKey) + + expect(manager.getActive()?.queryKey).toBe(activeKey) + }) + + it("keeps the abort callback across a rekey so cancel aborts the original run", () => { + // Given an active execution that was rekeyed after an edit above it + const manager = createManager() + const onAbort = vi.fn() + manager.markActive(1, "select 1@10-18" as QueryKey, onAbort) + manager.rekeyActive( + "select 1@10-18" as QueryKey, + "select 1@20-28" as QueryKey, + ) + + // When the user cancels the running query + manager.cancelActive() + + // Then the original run is aborted + expect(onAbort).toHaveBeenCalledTimes(1) + expect(manager.getActive()).toBeNull() + }) + + it("rekeys the active execution within a non-default scope", () => { + // Given an active execution under a notebook cell scope + const manager = createManager() + const scopeKey = "notebook-cell-7" + const oldKey = "select 1@10-18" as QueryKey + const newKey = "select 1@20-28" as QueryKey + manager.markActive(1, oldKey, vi.fn(), scopeKey) + + // When it is rekeyed within that scope + manager.rekeyActive(oldKey, newKey, scopeKey) + + // Then the scoped active execution moves to the new key + expect(manager.getActive(scopeKey)?.queryKey).toBe(newKey) + // And the default scope is untouched + expect(manager.getActive()).toBeNull() + + // And release still works with either key on that scope + manager.releaseExecution(oldKey, scopeKey) + expect(manager.getActive(scopeKey)?.queryKey).toBe(newKey) + manager.releaseExecution(newKey, scopeKey) + expect(manager.getActive(scopeKey)).toBeNull() + }) + it("notifies subscribers when manager state changes", () => { const manager = createManager() const activeKey = "active@0-6" as QueryKey diff --git a/src/utils/questdb/queryExecutionManager.ts b/src/utils/questdb/queryExecutionManager.ts index 82aa0fa48..3d59b624b 100644 --- a/src/utils/questdb/queryExecutionManager.ts +++ b/src/utils/questdb/queryExecutionManager.ts @@ -131,6 +131,17 @@ export class QueryExecutionManager { this.refreshSnapshot() } + rekeyActive = ( + oldKey: QueryKey, + newKey: QueryKey, + scopeKey: ScopeKey = DEFAULT_QUERY_EXECUTION_SCOPE, + ): void => { + const active = this._activeByScope.get(scopeKey) + if (!active || active.queryKey !== oldKey) return + this._activeByScope.set(scopeKey, { ...active, queryKey: newKey }) + this.refreshSnapshot() + } + private abortActive(scopeKey: ScopeKey): void { const active = this._activeByScope.get(scopeKey) if (!active) return From 2f8a77de2ec9a74afd5e23affe42250176eb433f Mon Sep 17 00:00:00 2001 From: emrberk Date: Mon, 6 Jul 2026 13:37:06 +0300 Subject: [PATCH 2/5] test fix --- e2e/tests/console/aiAssistant.spec.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/e2e/tests/console/aiAssistant.spec.js b/e2e/tests/console/aiAssistant.spec.js index 5bc3533f1..75d82ba10 100644 --- a/e2e/tests/console/aiAssistant.spec.js +++ b/e2e/tests/console/aiAssistant.spec.js @@ -3141,14 +3141,14 @@ Syntax: \`avg(column)\` // Then cy.getByDataHook("success-notification").should("be.visible") - // When - cy.clickLine(1) - - // Then - cy.getByDataHook("error-notification").should( - "contain", - "Cancelled by user", - ) + // When the notifications log is expanded + cy.expandNotifications() + + // Then the superseded editor query is recorded as cancelled, without the + // editor having to steal focus to surface it + cy.getExpandedNotifications() + .find('[data-hook="error-notification"]') + .should("contain", "Cancelled by user") }) it("should show abort dialog when running editor query while chat query is in flight", () => { From 6be3bf54a79f2fd11cb7ab639853323a022cf7cf Mon Sep 17 00:00:00 2001 From: emrberk Date: Mon, 13 Jul 2026 17:10:09 +0300 Subject: [PATCH 3/5] bulk-shift notification keys --- src/scenes/Editor/Monaco/index.tsx | 40 +++++++++----------- src/scenes/Editor/Monaco/utils.test.ts | 25 +++++++++++++ src/scenes/Editor/Monaco/utils.ts | 16 ++++++++ src/store/Query/actions.ts | 13 +++++++ src/store/Query/reducers.test.ts | 33 +++++++++++++++++ src/store/Query/reducers.ts | 51 ++++++++++++++++++++++++++ src/store/Query/types.ts | 15 ++++++++ 7 files changed, 170 insertions(+), 23 deletions(-) diff --git a/src/scenes/Editor/Monaco/index.tsx b/src/scenes/Editor/Monaco/index.tsx index fd5ed55be..d31b01cb3 100644 --- a/src/scenes/Editor/Monaco/index.tsx +++ b/src/scenes/Editor/Monaco/index.tsx @@ -67,6 +67,7 @@ import { createInflightQuery, shiftInflightQuery, shiftSelection, + applyQueryKeyUpdates, isInflightQueryStillInPlace, validateQueryJIT, setErrorMarkerForQuery, @@ -1027,7 +1028,7 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { const activeBufferId = activeBufferRef.current.id as number const bufferExecutions = executionRefs.current[activeBufferId.toString()] - const notificationUpdates: Array<() => void> = [] + const notificationKeyUpdates = new Map() const inflightQueryBeforeChanges = inflightQueryRef.current && @@ -1104,18 +1105,9 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { }) }) - keysToUpdate.forEach(({ oldKey, newKey, data }) => { - delete bufferExecutions[oldKey] - bufferExecutions[newKey] = data - notificationUpdates.push(() => - dispatch( - actions.query.updateNotificationKey( - oldKey, - newKey, - activeBufferId, - ), - ), - ) + applyQueryKeyUpdates(bufferExecutions, keysToUpdate) + keysToUpdate.forEach(({ oldKey, newKey }) => { + notificationKeyUpdates.set(oldKey, newKey) }) } @@ -1145,15 +1137,7 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { const newOffset = startOffset + effectiveOffsetDelta const newKey = createQueryKey(queryText, newOffset) - notificationUpdates.push(() => - dispatch( - actions.query.updateNotificationKey( - queryKey, - newKey, - activeBufferId, - ), - ), - ) + notificationKeyUpdates.set(queryKey, newKey) }) if (bufferExecutions && Object.keys(bufferExecutions).length === 0) { @@ -1186,7 +1170,17 @@ const MonacoEditor = ({ hidden = false }: { hidden?: boolean }) => { syncQueriesToRun(editor, runWithSelectionRef.current) contentJustChangedRef.current = false - notificationUpdates.forEach((update) => update()) + if (notificationKeyUpdates.size > 0) { + dispatch( + actions.query.updateNotificationKeys( + Array.from(notificationKeyUpdates, ([oldKey, newKey]) => ({ + oldKey, + newKey, + })), + activeBufferId, + ), + ) + } // JIT validation (debounced) if (validationTimeoutRef.current) { diff --git a/src/scenes/Editor/Monaco/utils.test.ts b/src/scenes/Editor/Monaco/utils.test.ts index 603f70e96..97ebbaa08 100644 --- a/src/scenes/Editor/Monaco/utils.test.ts +++ b/src/scenes/Editor/Monaco/utils.test.ts @@ -12,6 +12,7 @@ import { shiftInflightQuery, isInflightQueryStillInPlace, shiftSelection, + applyQueryKeyUpdates, } from "./utils" type SingleLineSelection = { startColumn: number; endColumn: number } @@ -781,3 +782,27 @@ describe("shiftSelection", () => { expect(shifted.endOffset).toBe(35) }) }) + +describe("applyQueryKeyUpdates", () => { + it("moves chained keys without overwriting a destination source", () => { + const firstKey = "select 1@0-8" + const secondKey = "select 1@10-18" + const thirdKey = "select 1@20-28" + const first = { id: "first" } + const second = { id: "second" } + const record = { + [firstKey]: first, + [secondKey]: second, + } + + applyQueryKeyUpdates(record, [ + { oldKey: firstKey, newKey: secondKey, data: first }, + { oldKey: secondKey, newKey: thirdKey, data: second }, + ]) + + expect(record).toEqual({ + [secondKey]: first, + [thirdKey]: second, + }) + }) +}) diff --git a/src/scenes/Editor/Monaco/utils.ts b/src/scenes/Editor/Monaco/utils.ts index e176e6b9c..5a5ca1dea 100644 --- a/src/scenes/Editor/Monaco/utils.ts +++ b/src/scenes/Editor/Monaco/utils.ts @@ -1157,6 +1157,22 @@ export const shiftSelection = ( endOffset: selection.endOffset + offsetDelta, }) +export const applyQueryKeyUpdates = ( + record: Record, + updates: readonly { + oldKey: QueryKey + newKey: QueryKey + data: T + }[], +): void => { + updates.forEach(({ oldKey }) => { + delete record[oldKey] + }) + updates.forEach(({ newKey, data }) => { + record[newKey] = data + }) +} + export const setErrorMarkerForQuery = ( monaco: Monaco, editor: IStandaloneCodeEditor, diff --git a/src/store/Query/actions.ts b/src/store/Query/actions.ts index fb318d24a..95f7a1a96 100644 --- a/src/store/Query/actions.ts +++ b/src/store/Query/actions.ts @@ -37,6 +37,7 @@ import { QueryAction, QueryAT, QueryKey, + QueryKeyUpdate, RunningType, QueriesToRun, } from "../../types" @@ -126,6 +127,17 @@ const updateNotificationKey = ( }, }) +const updateNotificationKeys = ( + updates: QueryKeyUpdate[], + bufferId: NotificationNamespaceKey, +): QueryAction => ({ + type: QueryAT.UPDATE_NOTIFICATION_KEYS, + payload: { + updates, + bufferId, + }, +}) + const moveNotificationNamespace = ( fromBufferId: NotificationNamespaceKey, toBufferId: NotificationNamespaceKey, @@ -150,6 +162,7 @@ export default { cleanupBufferNotifications, removeNotification, updateNotificationKey, + updateNotificationKeys, moveNotificationNamespace, setResult, stopRunning, diff --git a/src/store/Query/reducers.test.ts b/src/store/Query/reducers.test.ts index 698084f8b..c1f24866d 100644 --- a/src/store/Query/reducers.test.ts +++ b/src/store/Query/reducers.test.ts @@ -148,3 +148,36 @@ describe("query reducer — UPDATE_NOTIFICATION_KEY", () => { expect(next).toBe(state) }) }) + +describe("query reducer — UPDATE_NOTIFICATION_KEYS", () => { + it("moves chained keys atomically after vacating every source", () => { + const firstKey = "select 1@0-8" as QueryKey + const secondKey = "select 1@10-18" as QueryKey + const thirdKey = "select 1@20-28" as QueryKey + const first = notificationGroup(firstKey, new Date(1000)) + const second = notificationGroup(secondKey, new Date(2000)) + const state = { + ...stateWithGroups({ [firstKey]: first, [secondKey]: second }), + activeNotification: second.latest, + } + + const next = query( + state, + actions.updateNotificationKeys( + [ + { oldKey: firstKey, newKey: secondKey }, + { oldKey: secondKey, newKey: thirdKey }, + ], + BUFFER_ID, + ), + ) + + expect(next.queryNotifications[BUFFER_ID][firstKey]).toBeUndefined() + expect(next.queryNotifications[BUFFER_ID][secondKey]).toBe(first) + expect(next.queryNotifications[BUFFER_ID][thirdKey]).toBe(second) + expect( + next.notifications.map((notification) => notification.query), + ).toEqual([secondKey, thirdKey]) + expect(next.activeNotification?.query).toBe(thirdKey) + }) +}) diff --git a/src/store/Query/reducers.ts b/src/store/Query/reducers.ts index 4353a2bb9..fa9c2ce46 100644 --- a/src/store/Query/reducers.ts +++ b/src/store/Query/reducers.ts @@ -224,6 +224,57 @@ const query = (state = initialState, action: QueryAction): QueryStateShape => { } } + case QueryAT.UPDATE_NOTIFICATION_KEYS: { + const { updates, bufferId } = action.payload + const effectiveUpdates = updates.filter( + ({ oldKey, newKey }) => oldKey !== newKey, + ) + if (effectiveUpdates.length === 0) return state + + const bufferNotifications = state.queryNotifications[bufferId] || {} + const updatedBufferNotifications = { ...bufferNotifications } + const keyMap = new Map( + effectiveUpdates.map(({ oldKey, newKey }) => [oldKey, newKey]), + ) + + // Vacate every source before writing any destination so chained moves + // such as 0 -> 10 and 10 -> 20 cannot collide with transient keys. + effectiveUpdates.forEach(({ oldKey }) => { + delete updatedBufferNotifications[oldKey] + }) + + effectiveUpdates.forEach(({ oldKey, newKey }) => { + const moved = bufferNotifications[oldKey] + if (!moved) return + const existing = updatedBufferNotifications[newKey] + if (!existing || existing.latest.createdAt <= moved.latest.createdAt) { + updatedBufferNotifications[newKey] = moved + } + }) + + const updatedNotifications = state.notifications.map((notification) => { + const newKey = keyMap.get(notification.query) + return newKey ? { ...notification, query: newKey } : notification + }) + + const activeNotificationKey = state.activeNotification + ? keyMap.get(state.activeNotification.query) + : undefined + + return { + ...state, + notifications: updatedNotifications, + queryNotifications: { + ...state.queryNotifications, + [bufferId]: updatedBufferNotifications, + }, + activeNotification: + state.activeNotification && activeNotificationKey + ? { ...state.activeNotification, query: activeNotificationKey } + : state.activeNotification, + } + } + case QueryAT.MOVE_NOTIFICATION_NAMESPACE: { const { fromBufferId, toBufferId, newQueryKey } = action.payload if (fromBufferId === toBufferId) { diff --git a/src/store/Query/types.ts b/src/store/Query/types.ts index e7df2e74d..b2f35c94c 100644 --- a/src/store/Query/types.ts +++ b/src/store/Query/types.ts @@ -10,6 +10,11 @@ import type { Request } from "../../scenes/Editor/Monaco/utils" export type QueryKey = `${string}@${number}-${number}` export type NotificationNamespaceKey = string | number +export type QueryKeyUpdate = Readonly<{ + oldKey: QueryKey + newKey: QueryKey +}> + export enum NotificationType { ERROR = "error", INFO = "info", @@ -69,6 +74,7 @@ export enum QueryAT { CLEANUP_BUFFER_NOTIFICATIONS = "QUERY/CLEANUP_BUFFER_NOTIFICATIONS", REMOVE_NOTIFICATION = "QUERY/REMOVE_NOTIFICATION", UPDATE_NOTIFICATION_KEY = "QUERY/UPDATE_NOTIFICATION_KEY", + UPDATE_NOTIFICATION_KEYS = "QUERY/UPDATE_NOTIFICATION_KEYS", MOVE_NOTIFICATION_NAMESPACE = "QUERY/MOVE_NOTIFICATION_NAMESPACE", SET_RESULT = "QUERY/SET_RESULT", STOP_RUNNING = "QUERY/STOP_RUNNING", @@ -144,6 +150,14 @@ type UpdateNotificationKeyAction = Readonly<{ } }> +type UpdateNotificationKeysAction = Readonly<{ + type: QueryAT.UPDATE_NOTIFICATION_KEYS + payload: { + updates: QueryKeyUpdate[] + bufferId: NotificationNamespaceKey + } +}> + type MoveNotificationNamespaceAction = Readonly<{ type: QueryAT.MOVE_NOTIFICATION_NAMESPACE payload: { @@ -164,6 +178,7 @@ export type QueryAction = | CleanupBufferNotificationsAction | RemoveNotificationAction | UpdateNotificationKeyAction + | UpdateNotificationKeysAction | MoveNotificationNamespaceAction | SetResultAction | StopRunningAction From 5489b49bd5a2a3d0e6c1fb6dd6992c9bdc591f23 Mon Sep 17 00:00:00 2001 From: emrberk Date: Mon, 13 Jul 2026 17:16:42 +0300 Subject: [PATCH 4/5] submodule --- e2e/questdb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/questdb b/e2e/questdb index b7473a8b6..b0b8944bf 160000 --- a/e2e/questdb +++ b/e2e/questdb @@ -1 +1 @@ -Subproject commit b7473a8b639cbd024821abf12e98d54f29aa0476 +Subproject commit b0b8944bf202bc5007ffd2e3549e19fe58c89d3e From 940949777cba72e608929c72c9bfcd87cef88fe7 Mon Sep 17 00:00:00 2001 From: emrberk Date: Mon, 13 Jul 2026 18:05:15 +0300 Subject: [PATCH 5/5] column alias fix --- e2e/tests/console/result_charts.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/tests/console/result_charts.spec.js b/e2e/tests/console/result_charts.spec.js index 59a1e5b09..5382100b7 100644 --- a/e2e/tests/console/result_charts.spec.js +++ b/e2e/tests/console/result_charts.spec.js @@ -14,7 +14,7 @@ describe("questdb charts", () => { cy.get(".quick-vis-canvas").click() cy.getByDataHook("chart-panel-labels-select").should( "have.value", - `"rnd_timestamp(to_timestamp('2024-07-19:00:00:00.000000', 'yyy"`, + `rnd_timestamp(to_timestamp('2024-07-19:00:00:00.000000', 'yyy`, ) cy.getByDataHook("chart-panel-series-select").then(($element) => { expect($element[0].options[$element[0].selectedIndex].text).to.equal("x")