From d0e3cb26bde63a533cadd68000c7dfd87908693d Mon Sep 17 00:00:00 2001 From: AaronW Date: Thu, 26 Mar 2026 21:23:00 +0000 Subject: [PATCH 01/11] Add execution snapshots and stale query warning - Implemented `addRunInputSnapshot` and `useExecutionSnapshots` for managing query execution inputs. - Added `shouldComputeStaleWarning` and `useStaleQueryWarning` to handle stale query notifications. - Integrated snapshot recording in `DataDocQueryCell` and `QueryComposer` components. - Displayed stale query warnings in `DataDocQueryExecutions` and `QueryComposerExecution` components. Made-with: Cursor --- .../queryEditor/useExecutionSnapshots.test.ts | 37 +++++++ .../queryEditor/useStaleQueryWarning.test.ts | 96 +++++++++++++++++++ .../DataDocQueryCell/DataDocQueryCell.tsx | 33 ++++++- .../DataDocQueryExecutions.tsx | 19 ++++ .../StaleQueryWarning.tsx | 13 +++ .../QueryComposer/QueryComposer.tsx | 25 ++++- .../QueryComposer/QueryComposerExecution.tsx | 16 ++++ .../queryEditor/useExecutionSnapshots.ts | 24 +++++ .../hooks/queryEditor/useStaleQueryWarning.ts | 59 ++++++++++++ 9 files changed, 316 insertions(+), 6 deletions(-) create mode 100644 querybook/webapp/__tests__/hooks/queryEditor/useExecutionSnapshots.test.ts create mode 100644 querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts create mode 100644 querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx create mode 100644 querybook/webapp/hooks/queryEditor/useExecutionSnapshots.ts create mode 100644 querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts diff --git a/querybook/webapp/__tests__/hooks/queryEditor/useExecutionSnapshots.test.ts b/querybook/webapp/__tests__/hooks/queryEditor/useExecutionSnapshots.test.ts new file mode 100644 index 000000000..e07ab7f3e --- /dev/null +++ b/querybook/webapp/__tests__/hooks/queryEditor/useExecutionSnapshots.test.ts @@ -0,0 +1,37 @@ +import { addRunInputSnapshot } from 'hooks/queryEditor/useExecutionSnapshots'; + +describe('addRunInputSnapshot', () => { + test('adds a snapshot to an empty record', () => { + const result = addRunInputSnapshot({}, 1, 'SELECT 1'); + expect(result).toEqual({ 1: 'SELECT 1' }); + }); + + test('adds a new execution to existing snapshots', () => { + const prev = { 1: 'SELECT 1' }; + const result = addRunInputSnapshot(prev, 2, 'SELECT 2'); + expect(result).toEqual({ 1: 'SELECT 1', 2: 'SELECT 2' }); + }); + + test('overwrites an existing execution snapshot', () => { + const prev = { 1: 'SELECT old' }; + const result = addRunInputSnapshot(prev, 1, 'SELECT new'); + expect(result).toEqual({ 1: 'SELECT new' }); + }); + + test('does not mutate the original record', () => { + const prev = { 1: 'SELECT 1' }; + const result = addRunInputSnapshot(prev, 2, 'SELECT 2'); + expect(prev).toEqual({ 1: 'SELECT 1' }); + expect(result).not.toBe(prev); + }); + + test('handles many snapshots without pruning', () => { + let record: Record = {}; + for (let i = 0; i < 50; i++) { + record = addRunInputSnapshot(record, i, `SELECT ${i}`); + } + expect(Object.keys(record)).toHaveLength(50); + expect(record[0]).toBe('SELECT 0'); + expect(record[49]).toBe('SELECT 49'); + }); +}); diff --git a/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts b/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts new file mode 100644 index 000000000..cfb28fe13 --- /dev/null +++ b/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts @@ -0,0 +1,96 @@ +import { shouldComputeStaleWarning } from 'hooks/queryEditor/useStaleQueryWarning'; + +describe('shouldComputeStaleWarning', () => { + const snapshots: Record = { + 1: 'SELECT * FROM t1', + 2: 'SELECT * FROM t2', + }; + + test('returns false when selectedExecutionId is null', () => { + expect( + shouldComputeStaleWarning(null, snapshots, 'SELECT * FROM t1') + ).toBe(false); + }); + + test('returns false when selectedExecutionId is undefined', () => { + expect( + shouldComputeStaleWarning(undefined, snapshots, 'SELECT * FROM t1') + ).toBe(false); + }); + + test('returns false when snapshot matches current input', () => { + expect( + shouldComputeStaleWarning(1, snapshots, 'SELECT * FROM t1') + ).toBe(false); + }); + + test('returns true when snapshot differs from current input', () => { + expect( + shouldComputeStaleWarning(1, snapshots, 'SELECT * FROM changed') + ).toBe(true); + }); + + test('returns false when execution has no snapshot and no initialQuery', () => { + expect( + shouldComputeStaleWarning(99, snapshots, 'SELECT * FROM t1') + ).toBe(false); + }); + + describe('initialQuery fallback', () => { + const initialQuery = 'SELECT 1'; + + test('uses initialQuery when no snapshot exists for the execution', () => { + expect( + shouldComputeStaleWarning( + 99, + snapshots, + 'SELECT 2', + initialQuery + ) + ).toBe(true); + }); + + test('returns false when current input matches initialQuery (no snapshot)', () => { + expect( + shouldComputeStaleWarning( + 99, + snapshots, + 'SELECT 1', + initialQuery + ) + ).toBe(false); + }); + + test('prefers in-memory snapshot over initialQuery', () => { + expect( + shouldComputeStaleWarning( + 1, + snapshots, + 'SELECT * FROM t1', + 'something else' + ) + ).toBe(false); + }); + + test('warns when input differs from snapshot even if initialQuery matches', () => { + expect( + shouldComputeStaleWarning( + 1, + snapshots, + 'SELECT * FROM changed', + 'SELECT * FROM changed' + ) + ).toBe(true); + }); + }); + + test('works with empty snapshots and no initialQuery', () => { + expect(shouldComputeStaleWarning(1, {}, 'any query')).toBe(false); + }); + + test('works with empty snapshots and initialQuery provided', () => { + expect( + shouldComputeStaleWarning(1, {}, 'changed', 'original') + ).toBe(true); + }); +}); diff --git a/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx b/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx index 81193ae60..4abd985ec 100644 --- a/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx +++ b/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx @@ -13,6 +13,7 @@ import { DataDocQueryExecutions } from 'components/DataDocQueryExecutions/DataDo import { DataDocTableSamplingInfo } from 'components/DataDocTableSamplingInfo/DataDocTableSamplingInfo'; import { QueryCellTitle } from 'components/QueryCellTitle/QueryCellTitle'; import { runQuery, transformQuery } from 'components/QueryComposer/RunQuery'; +import { addRunInputSnapshot } from 'hooks/queryEditor/useExecutionSnapshots'; import { BoundQueryEditor } from 'components/QueryEditor/BoundQueryEditor'; import { IQueryEditorHandles } from 'components/QueryEditor/QueryEditor'; import { QueryPeerReviewModal } from 'components/QueryPeerReviewModal/QueryPeerReviewModal'; @@ -120,6 +121,8 @@ interface IState { showTableSamplingInfoModal: boolean; showPeerReviewModal: boolean; + executionRunInputById: Record; + transpilerConfig?: { toEngine: IQueryEngine; transpilerName: string; @@ -130,9 +133,11 @@ class DataDocQueryCellComponent extends React.PureComponent { private queryEditorRef = React.createRef(); private runButtonRef = React.createRef(); private commandInputRef = React.createRef(); + private readonly initialQuery: string; public constructor(props) { super(props); + this.initialQuery = props.query; this.state = { query: props.query, @@ -149,6 +154,7 @@ class DataDocQueryCellComponent extends React.PureComponent { samplingTables: {}, showTableSamplingInfoModal: false, showPeerReviewModal: false, + executionRunInputById: {}, }; } @@ -443,11 +449,15 @@ class DataDocQueryCellComponent extends React.PureComponent { return this.handleMetaChange('sample_rate', sampleRate); } + @bind + public getRunInputString(): string { + return this.queryEditorRef.current?.getSelection?.() ?? this.state.query; + } + @bind public async getTransformedQuery() { const { templatedVariables = [] } = this.props; - const { query } = this.state; - const rawQuery = this.queryEditorRef.current?.getSelection?.() ?? query; + const rawQuery = this.getRunInputString(); return transformQuery( rawQuery, @@ -469,6 +479,17 @@ class DataDocQueryCellComponent extends React.PureComponent { return Object.keys(metadata).length === 0 ? null : metadata; } + @bind + public recordRunInputSnapshot(executionId: number, runInput: string) { + this.setState((prev) => ({ + executionRunInputById: addRunInputSnapshot( + prev.executionRunInputById, + executionId, + runInput + ), + })); + } + @bind public async executeQuery(options: { element: ElementType; @@ -502,6 +523,7 @@ class DataDocQueryCellComponent extends React.PureComponent { executionMetadata, peerReviewParams ); + this.recordRunInputSnapshot(queryExecution.id, this.state.query); return queryExecution.id; } ); @@ -596,12 +618,14 @@ class DataDocQueryCellComponent extends React.PureComponent { const executionMetadata = this.sampleRate > 0 ? { sample_rate: this.sampleRate } : null; - return this.props.createQueryExecution( + const queryExecution = await this.props.createQueryExecution( renderedQuery, this.engineId, this.props.cellId, executionMetadata ); + this.recordRunInputSnapshot(queryExecution.id, this.state.query); + return queryExecution; } } @@ -1077,6 +1101,9 @@ class DataDocQueryCellComponent extends React.PureComponent { onSamplingInfoClick={this.toggleShowTableSamplingInfoModal} hasSamplingTables={this.hasSamplingTables} sampleRate={this.sampleRate} + currentRunInput={this.state.query} + executionRunInputSnapshots={this.state.executionRunInputById} + initialQuery={this.initialQuery} /> ); } diff --git a/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx b/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx index 8badabfbe..115558a0d 100644 --- a/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx +++ b/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx @@ -13,11 +13,14 @@ import { QueryExecutionDuration } from 'components/QueryExecution/QueryExecution import { QueryExecutionBar } from 'components/QueryExecutionBar/QueryExecutionBar'; import { DataDocContext } from 'context/DataDoc'; import { useMakeSelector } from 'hooks/redux/useMakeSelector'; +import { useStaleQueryWarning } from 'hooks/queryEditor/useStaleQueryWarning'; import { getQueryString } from 'lib/utils/query-string'; import * as queryExecutionsActions from 'redux/queryExecutions/action'; import * as queryExecutionsSelectors from 'redux/queryExecutions/selector'; import { StyledText } from 'ui/StyledText/StyledText'; +import { StaleQueryWarning } from './StaleQueryWarning'; + interface IProps { docId: number; cellId: number; @@ -28,6 +31,10 @@ interface IProps { onSamplingInfoClick?: () => void; hasSamplingTables?: boolean; sampleRate: number; + + currentRunInput?: string; + executionRunInputSnapshots?: Readonly>; + initialQuery?: string; } export const DataDocQueryExecutions: React.FunctionComponent = @@ -40,6 +47,9 @@ export const DataDocQueryExecutions: React.FunctionComponent = onSamplingInfoClick, hasSamplingTables, sampleRate, + currentRunInput, + executionRunInputSnapshots, + initialQuery, }) => { const { cellIdToExecutionId, onQueryCellSelectExecution } = useContext(DataDocContext); @@ -149,6 +159,14 @@ export const DataDocQueryExecutions: React.FunctionComponent = }; const selectedExecution = queryExecutions[selectedExecutionIndex]; + + const { showWarning: showStaleWarning } = useStaleQueryWarning({ + selectedExecutionId: selectedExecution?.id ?? null, + snapshots: executionRunInputSnapshots ?? {}, + currentRunInput: currentRunInput ?? '', + initialQuery, + }); + const queryExecutionDOM = selectedExecution && ( = return (
+ {showStaleWarning && } {generateExecutionsPickerDOM()} {queryExecutionDOM} diff --git a/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx b/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx new file mode 100644 index 000000000..66dee00de --- /dev/null +++ b/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx @@ -0,0 +1,13 @@ +import React from 'react'; + +import { Message } from 'ui/Message/Message'; + +export const StaleQueryWarning: React.FC = () => ( + +); diff --git a/querybook/webapp/components/QueryComposer/QueryComposer.tsx b/querybook/webapp/components/QueryComposer/QueryComposer.tsx index 2ba92bdc7..f091dd6a1 100644 --- a/querybook/webapp/components/QueryComposer/QueryComposer.tsx +++ b/querybook/webapp/components/QueryComposer/QueryComposer.tsx @@ -38,6 +38,7 @@ import { IQueryEngine } from 'const/queryEngine'; import { ISearchOptions, ISearchResult } from 'const/searchAndReplace'; import { SurveySurfaceType } from 'const/survey'; import { useDebounceState } from 'hooks/redux/useDebounceState'; +import { useExecutionSnapshots } from 'hooks/queryEditor/useExecutionSnapshots'; import { useSurveyTrigger } from 'hooks/ui/useSurveyTrigger'; import { useBrowserTitle } from 'hooks/useBrowserTitle'; import { useTrackView } from 'hooks/useTrackView'; @@ -424,6 +425,9 @@ const QueryComposer: React.FC = () => { ); const dispatch: Dispatch = useDispatch(); const { query, setQuery } = useQuery(dispatch, environmentId); + const reduxQuery = useSelector( + (state: IStoreState) => state.adhocQuery[environmentId]?.query ?? '' + ); const { engine, setEngineId, queryEngines, queryEngineById } = useEngine( dispatch, environmentId @@ -466,6 +470,14 @@ const QueryComposer: React.FC = () => { const [showPeerReviewModal, setShowPeerReviewModal] = useState(false); const hasPeerReviewFeature = engine?.feature_params?.peer_review; + const { snapshots: executionRunInputById, recordSnapshot: recordRunInputSnapshot } = + useExecutionSnapshots(); + + const initialQueryRef = useRef(reduxQuery); + if (!initialQueryRef.current && reduxQuery) { + initialQueryRef.current = reduxQuery; + } + const runButtonRef = useRef(null); const clickOnRunButton = useCallback(() => { if (runButtonRef.current) { @@ -558,8 +570,9 @@ const QueryComposer: React.FC = () => { // Throttle to prevent double run await sleep(250); + const selectedQuery = getCurrentSelectedQuery(); const transformedQuery = await transformQuery( - getCurrentSelectedQuery(), + selectedQuery, engine.language, templatedVariables, engine, @@ -571,16 +584,17 @@ const QueryComposer: React.FC = () => { const queryId = await runQuery( transformedQuery, engine.id, - async (query, engineId) => { + async (q, engineId) => { const data = await dispatch( queryExecutionsAction.createQueryExecution( - query, + q, engineId, null, queryExecutionMetadata, peerReviewParams ) ); + recordRunInputSnapshot(data.id, query); return data.id; } ); @@ -602,12 +616,14 @@ const QueryComposer: React.FC = () => { getQueryExecutionMetadata, hasLintErrors, getCurrentSelectedQuery, + query, engine, templatedVariables, rowLimit, triggerSurvey, dispatch, setExecutionId, + recordRunInputSnapshot, ] ); @@ -750,6 +766,9 @@ const QueryComposer: React.FC = () => { } sampleRate={getSampleRate()} onUpdateQuery={updateAndRunQuery} + currentRunInput={query} + executionRunInputSnapshots={executionRunInputById} + initialQuery={initialQueryRef.current} />
diff --git a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx index 241be0e39..bc1842daf 100644 --- a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx +++ b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { useSelector } from 'react-redux'; +import { StaleQueryWarning } from 'components/DataDocQueryExecutions/StaleQueryWarning'; import { QueryExecution } from 'components/QueryExecution/QueryExecution'; import { QueryExecutionDuration } from 'components/QueryExecution/QueryExecutionDuration'; import { QueryExecutionBar } from 'components/QueryExecutionBar/QueryExecutionBar'; @@ -8,6 +9,7 @@ import { queryStatusToStatusIcon, STATUS_TO_TEXT_MAPPING, } from 'const/queryStatus'; +import { useStaleQueryWarning } from 'hooks/queryEditor/useStaleQueryWarning'; import { IStoreState } from 'redux/store/types'; import { Level } from 'ui/Level/Level'; import { StatusIcon } from 'ui/StatusIcon/StatusIcon'; @@ -19,6 +21,9 @@ interface IProps { hasSamplingTables?: boolean; sampleRate: number; onUpdateQuery?: (query: string, run?: boolean) => any; + currentRunInput?: string; + executionRunInputSnapshots?: Readonly>; + initialQuery?: string; } export const QueryComposerExecution: React.FunctionComponent = ({ @@ -27,11 +32,21 @@ export const QueryComposerExecution: React.FunctionComponent = ({ hasSamplingTables, sampleRate, onUpdateQuery, + currentRunInput, + executionRunInputSnapshots, + initialQuery, }) => { const execution = useSelector( (state: IStoreState) => state.queryExecutions.queryExecutionById[id] ); + const { showWarning: showStaleWarning } = useStaleQueryWarning({ + selectedExecutionId: id, + snapshots: executionRunInputSnapshots ?? {}, + currentRunInput: currentRunInput ?? '', + initialQuery, + }); + if (!execution) { return null; } @@ -47,6 +62,7 @@ export const QueryComposerExecution: React.FunctionComponent = ({ return (
+ {showStaleWarning && }
diff --git a/querybook/webapp/hooks/queryEditor/useExecutionSnapshots.ts b/querybook/webapp/hooks/queryEditor/useExecutionSnapshots.ts new file mode 100644 index 000000000..9bcc9e7bd --- /dev/null +++ b/querybook/webapp/hooks/queryEditor/useExecutionSnapshots.ts @@ -0,0 +1,24 @@ +import { useCallback, useState } from 'react'; + +export function addRunInputSnapshot( + prev: Record, + executionId: number, + runInput: string +): Record { + return { ...prev, [executionId]: runInput }; +} + +export function useExecutionSnapshots() { + const [snapshots, setSnapshots] = useState>({}); + + const recordSnapshot = useCallback( + (executionId: number, runInput: string) => { + setSnapshots((prev) => + addRunInputSnapshot(prev, executionId, runInput) + ); + }, + [] + ); + + return { snapshots, recordSnapshot }; +} diff --git a/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts b/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts new file mode 100644 index 000000000..3fa1c9396 --- /dev/null +++ b/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts @@ -0,0 +1,59 @@ +import { useDebounce } from 'hooks/useDebounce'; + +const DEFAULT_DEBOUNCE_MS = 300; + +export function shouldComputeStaleWarning( + selectedExecutionId: number | null | undefined, + snapshots: Readonly>, + currentInput: string, + initialQuery?: string +): boolean { + if (selectedExecutionId == null) { + return false; + } + + const snapshot = snapshots[selectedExecutionId] ?? initialQuery; + if (snapshot === undefined) { + return false; + } + + return currentInput !== snapshot; +} + +export function useStaleQueryWarning(options: { + selectedExecutionId: number | null | undefined; + snapshots: Readonly>; + currentRunInput: string; + initialQuery?: string; + debounceMs?: number; +}): { showWarning: boolean } { + const { + selectedExecutionId, + snapshots, + currentRunInput, + initialQuery, + debounceMs = DEFAULT_DEBOUNCE_MS + } = options; + + const debouncedInput = useDebounce(currentRunInput, debounceMs); + + // Debounced check stays stable during typing to prevent flickering. + // Real-time check instantly suppresses false positives on initial page load + // (when currentRunInput briefly differs from the snapshot before the editor populates). + const isStaleDebounced = shouldComputeStaleWarning( + selectedExecutionId, + snapshots, + debouncedInput, + initialQuery + ); + const isStaleRealtime = shouldComputeStaleWarning( + selectedExecutionId, + snapshots, + currentRunInput, + initialQuery + ); + + return { + showWarning: isStaleDebounced && isStaleRealtime, + }; +} From 3e51487e6958731903bf89f546777d71ad97e079 Mon Sep 17 00:00:00 2001 From: AaronW Date: Thu, 26 Mar 2026 21:23:05 +0000 Subject: [PATCH 02/11] Consolidate on agent rules - Created AGENTS.md to provide an overview of Querybook, including its tech stack, directory layout, plugin system, and local development instructions. - Added CLAUDE.md as a reference to AGENTS.md for internal documentation purposes. Made-with: Cursor --- AGENTS.md | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ CLAUDE.md | 1 + 2 files changed, 92 insertions(+) create mode 100644 AGENTS.md create mode 100644 CLAUDE.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..077f52769 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,91 @@ +# Querybook + +Querybook is Pinterest's open-source Big Data IDE for discovering, creating, and sharing data analyses. It combines a rich-text editor, SQL query engine, charting, scheduling, and table documentation in a single web app. + +## Tech Stack + +- **Backend:** Python 3.10, Flask, SQLAlchemy (MySQL), Celery (Redis broker), Elasticsearch/OpenSearch, gevent + Flask-SocketIO (WebSockets), uWSGI (production) +- **Frontend:** React 17, TypeScript, Redux, Webpack 5, CodeMirror (SQL editor), Draft.js (rich text), Chart.js/D3/ReactFlow + +## Directory Layout + +- `querybook/server/` — Flask backend + - `app/` — app setup + - `datasources/` — REST API endpoints + - `logic/` — business logic + - `models/` — SQLAlchemy models + - `tasks/` — Celery tasks + - `lib/` — utilities, executors, metastores + - `env.py` — `QuerybookSettings` configuration +- `querybook/webapp/` — React/TypeScript frontend + - `components/` — React components + - `hooks/` — custom React hooks + - `redux/` — Redux store, actions, reducers + - `lib/` — frontend utilities + - `ui/` — reusable UI primitives + - `resource/` — API client layer +- `querybook/config/` — YAML config files +- `plugins/` — plugin stubs (extension point for custom behavior) +- `requirements/` — pip requirements (`base.txt`, `prod.txt`, `engine/*.txt`, `auth/*.txt`) +- `containers/` — Docker Compose files (dev, prod, test) +- `docs_website/` — Docusaurus documentation site +- `helm/` / `k8s/` — Kubernetes deployment manifests + +## Plugin System + +Querybook is extended via plugins without forking. The env var `QUERYBOOK_PLUGIN` (default `./plugins`) points to a directory where plugin modules are discovered by `lib.utils.import_helper.import_module_with_default()`. + +Each plugin module exports a well-known variable (e.g. `ALL_PLUGIN_EXECUTORS`) that the server merges with built-in defaults. + +Key plugin types: `executor_plugin`, `metastore_plugin`, `auth_plugin`, `api_plugin`, `exporter_plugin`, `result_store_plugin`, `notifier_plugin`, `event_logger_plugin`, `stats_logger_plugin`, `job_plugin`, `tasks_plugin`, `dag_exporter_plugin`, `ai_assistant_plugin`, `vector_store_plugin`, `webpage_plugin`, `monkey_patch_plugin`, `query_validation_plugin`, `query_transpilation_plugin`, `engine_status_checker_plugin`, `table_uploader_plugin`. + +## Configuration + +Priority: **env vars > `querybook_config.yaml` > `querybook_default_config.yaml`**. + +Key settings live in `querybook/server/env.py` (`QuerybookSettings`). + +## Running Locally + +```bash +make # docker-compose up (full stack) → http://localhost:10001 +make web # web server only +make worker # celery worker +make scheduler # celery beat +``` + +## Making Commits + +When preparing a PR, run the relevant checks: + +**Backend changes** (anything under `querybook/server/`): +- `make test` — run backend tests +- Pre-commit hooks handle linting (configured in `.pre-commit-config.yaml`) + +**Frontend changes** (anything under `querybook/webapp/`): +- `pnpm test` or `yarn test` — run Jest tests +- `pnpm run lint` or `yarn lint` — ESLint with auto-fix +- `pnpm run tsc-check` or `yarn tsc-check` — TypeScript type checking + +## Pinterest Internal Deployment + +Pinterest deploys Querybook internally via a separate repo (`datahub-pinterest`) that uses this repo's Docker image as a base and layers Pinterest-specific plugins on top. Changes to core Querybook go here; Pinterest-specific features belong in that plugin repo. Do not add Pinterest-internal details to this file. + +## Maintaining This File + +**Include:** +- Repo purpose, tech stack, and high-level architecture +- Directory layout (key paths only) +- How to run, test, and lint locally +- Commit and PR workflow expectations +- Plugin system overview and extension points + +**Do not include:** +- Detailed API docs or function-level documentation +- Inline code examples longer than 5 lines +- Deployment runbooks or operational procedures (keep in README or docs/) +- Credentials, secrets, or internal URLs +- Information that changes frequently (version numbers, dependency lists) +- Content already covered in README.md +- Content that can be easily derived by AI agents (e.g. reading file trees, package.json) +- References to internal/proprietary repos — this is an open-source project diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..43c994c2d --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md From 45166d2b6d3a72e9dfc001b0cc1cf562fc93f56b Mon Sep 17 00:00:00 2001 From: AaronW Date: Thu, 26 Mar 2026 23:39:55 +0000 Subject: [PATCH 03/11] Add revert button to stale query warning - Added `getSnapshotForExecution` utility to retrieve execution snapshots based on execution ID. - Updated `useStaleQueryWarning` to include `onRevert` functionality for reverting to previous query states. - Modified `StaleQueryWarning` component to display a revert button when applicable. - Integrated snapshot handling in `DataDocQueryExecutions` and `QueryComposerExecution` components to improve user experience with stale queries. --- .../queryEditor/useStaleQueryWarning.test.ts | 42 +++++++++++++++++- .../DataDocQueryExecutions.tsx | 18 +++++--- .../StaleQueryWarning.tsx | 21 ++++++++- .../QueryComposer/QueryComposerExecution.tsx | 18 +++++--- .../hooks/queryEditor/useStaleQueryWarning.ts | 44 +++++++++++++++---- 5 files changed, 118 insertions(+), 25 deletions(-) diff --git a/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts b/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts index cfb28fe13..77fbadb51 100644 --- a/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts +++ b/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts @@ -1,4 +1,44 @@ -import { shouldComputeStaleWarning } from 'hooks/queryEditor/useStaleQueryWarning'; +import { + getSnapshotForExecution, + shouldComputeStaleWarning, +} from 'hooks/queryEditor/useStaleQueryWarning'; + +describe('getSnapshotForExecution', () => { + const snapshots: Record = { + 1: 'SELECT * FROM t1', + 2: 'SELECT * FROM t2', + }; + + test('returns undefined when executionId is null', () => { + expect(getSnapshotForExecution(null, snapshots)).toBeUndefined(); + }); + + test('returns undefined when executionId is undefined', () => { + expect(getSnapshotForExecution(undefined, snapshots)).toBeUndefined(); + }); + + test('returns snapshot when it exists', () => { + expect(getSnapshotForExecution(1, snapshots)).toBe( + 'SELECT * FROM t1' + ); + }); + + test('falls back to initialQuery when no snapshot exists', () => { + expect(getSnapshotForExecution(99, snapshots, 'SELECT 1')).toBe( + 'SELECT 1' + ); + }); + + test('returns undefined when no snapshot and no initialQuery', () => { + expect(getSnapshotForExecution(99, snapshots)).toBeUndefined(); + }); + + test('prefers snapshot over initialQuery', () => { + expect( + getSnapshotForExecution(1, snapshots, 'something else') + ).toBe('SELECT * FROM t1'); + }); +}); describe('shouldComputeStaleWarning', () => { const snapshots: Record = { diff --git a/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx b/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx index 115558a0d..ab17f290d 100644 --- a/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx +++ b/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx @@ -160,12 +160,14 @@ export const DataDocQueryExecutions: React.FunctionComponent = const selectedExecution = queryExecutions[selectedExecutionIndex]; - const { showWarning: showStaleWarning } = useStaleQueryWarning({ - selectedExecutionId: selectedExecution?.id ?? null, - snapshots: executionRunInputSnapshots ?? {}, - currentRunInput: currentRunInput ?? '', - initialQuery, - }); + const { showWarning: showStaleWarning, onRevert } = + useStaleQueryWarning({ + selectedExecutionId: selectedExecution?.id ?? null, + snapshots: executionRunInputSnapshots ?? {}, + currentRunInput: currentRunInput ?? '', + initialQuery, + onUpdateQuery, + }); const queryExecutionDOM = selectedExecution && ( = return (
- {showStaleWarning && } + {showStaleWarning && ( + + )} {generateExecutionsPickerDOM()} {queryExecutionDOM} diff --git a/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx b/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx index 66dee00de..aa0831432 100644 --- a/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx +++ b/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx @@ -1,13 +1,30 @@ import React from 'react'; +import { TextButton } from 'ui/Button/Button'; import { Message } from 'ui/Message/Message'; -export const StaleQueryWarning: React.FC = () => ( +interface IProps { + onRevert?: () => void; +} + +export const StaleQueryWarning: React.FC = ({ onRevert }) => ( + Query has been edited. + {onRevert && ( + + )} + + } /> ); diff --git a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx index bc1842daf..b8d741897 100644 --- a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx +++ b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx @@ -40,12 +40,14 @@ export const QueryComposerExecution: React.FunctionComponent = ({ (state: IStoreState) => state.queryExecutions.queryExecutionById[id] ); - const { showWarning: showStaleWarning } = useStaleQueryWarning({ - selectedExecutionId: id, - snapshots: executionRunInputSnapshots ?? {}, - currentRunInput: currentRunInput ?? '', - initialQuery, - }); + const { showWarning: showStaleWarning, onRevert } = + useStaleQueryWarning({ + selectedExecutionId: id, + snapshots: executionRunInputSnapshots ?? {}, + currentRunInput: currentRunInput ?? '', + initialQuery, + onUpdateQuery, + }); if (!execution) { return null; @@ -62,7 +64,9 @@ export const QueryComposerExecution: React.FunctionComponent = ({ return (
- {showStaleWarning && } + {showStaleWarning && ( + + )}
diff --git a/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts b/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts index 3fa1c9396..46cdbfc02 100644 --- a/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts +++ b/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts @@ -1,22 +1,34 @@ +import { useCallback } from 'react'; + import { useDebounce } from 'hooks/useDebounce'; const DEFAULT_DEBOUNCE_MS = 300; +export function getSnapshotForExecution( + executionId: number | null | undefined, + snapshots: Readonly>, + initialQuery?: string +): string | undefined { + if (executionId == null) { + return undefined; + } + return snapshots[executionId] ?? initialQuery; +} + export function shouldComputeStaleWarning( selectedExecutionId: number | null | undefined, snapshots: Readonly>, currentInput: string, initialQuery?: string ): boolean { - if (selectedExecutionId == null) { - return false; - } - - const snapshot = snapshots[selectedExecutionId] ?? initialQuery; + const snapshot = getSnapshotForExecution( + selectedExecutionId, + snapshots, + initialQuery + ); if (snapshot === undefined) { return false; } - return currentInput !== snapshot; } @@ -26,13 +38,15 @@ export function useStaleQueryWarning(options: { currentRunInput: string; initialQuery?: string; debounceMs?: number; -}): { showWarning: boolean } { + onUpdateQuery?: (query: string, run?: boolean) => any; +}): { showWarning: boolean; snapshotQuery: string | undefined; onRevert?: () => void } { const { selectedExecutionId, snapshots, currentRunInput, initialQuery, - debounceMs = DEFAULT_DEBOUNCE_MS + debounceMs = DEFAULT_DEBOUNCE_MS, + onUpdateQuery, } = options; const debouncedInput = useDebounce(currentRunInput, debounceMs); @@ -53,7 +67,21 @@ export function useStaleQueryWarning(options: { initialQuery ); + const snapshotQuery = getSnapshotForExecution( + selectedExecutionId, + snapshots, + initialQuery + ); + + const onRevert = useCallback(() => { + if (snapshotQuery !== undefined) { + onUpdateQuery?.(snapshotQuery); + } + }, [snapshotQuery, onUpdateQuery]); + return { showWarning: isStaleDebounced && isStaleRealtime, + snapshotQuery, + onRevert: onUpdateQuery ? onRevert : undefined, }; } From bb5a17389ca91a5a230632c98f4e5c2067d91a56 Mon Sep 17 00:00:00 2001 From: AaronW Date: Thu, 26 Mar 2026 23:57:09 +0000 Subject: [PATCH 04/11] Enhance contribution guidelines and testing instructions - Updated AGENTS.md to clarify CI checks for backend and frontend changes, including Docker and non-Docker testing commands. - Added details on formatting checks and pre-commit hook setup to prevent common CI failures. - Revised README.md to link to the full contribution guide, emphasizing local test execution. - Expanded contributing.mdx with comprehensive testing instructions for both backend and frontend, including CI-equivalent commands and linting setup. --- AGENTS.md | 25 +++++++++---- README.md | 2 +- .../docs/developer_guide/contributing.mdx | 35 +++++++++++++++++++ 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 077f52769..0ae7e5ddb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -56,16 +56,29 @@ make scheduler # celery beat ## Making Commits -When preparing a PR, run the relevant checks: +When preparing a PR, run the relevant checks. CI runs all of the following automatically on every PR via GitHub Actions (`.github/workflows/`). **Backend changes** (anything under `querybook/server/`): -- `make test` — run backend tests -- Pre-commit hooks handle linting (configured in `.pre-commit-config.yaml`) +- `make test` — builds the `querybook-test` Docker image and runs pytest inside it +- CI-equivalent without Docker: `PYTHONPATH=querybook/server:plugins ./querybook/scripts/run_test --python` **Frontend changes** (anything under `querybook/webapp/`): -- `pnpm test` or `yarn test` — run Jest tests -- `pnpm run lint` or `yarn lint` — ESLint with auto-fix -- `pnpm run tsc-check` or `yarn tsc-check` — TypeScript type checking +- `./querybook/scripts/run_test --node` — CI-equivalent; runs all four checks below in parallel: + - `yarn tsc-check` — TypeScript type checking + - `yarn test` — Jest unit tests + - `yarn lint` — ESLint with auto-fix + - `webpack --mode=production` — production build verification + +**Formatting (all changes) — common CI failure:** + +`./querybook/scripts/run_test --node` does **not** run Prettier. CI runs Prettier separately via `pre-commit`, so formatting issues are a frequent cause of CI failures. Always run Prettier on changed files before pushing: + +- `yarn prettier --write ` — format specific files +- `pre-commit run --all-files` — run the full pre-commit suite (Black, Prettier, flake8) + +To catch formatting automatically on every `git commit`, install the hooks once: + +- `pip install pre-commit && pre-commit install` ## Pinterest Internal Deployment diff --git a/README.md b/README.md index 55e613010..3f6373464 100644 --- a/README.md +++ b/README.md @@ -106,4 +106,4 @@ Lineage & Analytics # Contributing Back -See [CONTRIBUTING](CONTRIBUTING.md). +See [CONTRIBUTING](CONTRIBUTING.md) for the full guide, including [how to run tests locally](docs_website/docs/developer_guide/contributing.mdx#testing). diff --git a/docs_website/docs/developer_guide/contributing.mdx b/docs_website/docs/developer_guide/contributing.mdx index cd8db994e..0fadff567 100644 --- a/docs_website/docs/developer_guide/contributing.mdx +++ b/docs_website/docs/developer_guide/contributing.mdx @@ -65,6 +65,41 @@ To increase the chances that your pull request will be accepted: - Write tests for your changes - Write a good commit message +### Testing + +CI runs all of the checks below automatically on every pull request via GitHub Actions. You should reproduce them locally before pushing. + +**Backend** (changes under `querybook/server/`): + +```sh +# Docker-based (recommended) — builds the test image and runs pytest +make test + +# Without Docker (mirrors CI) +PYTHONPATH=querybook/server:plugins ./querybook/scripts/run_test --python +``` + +**Frontend** (changes under `querybook/webapp/`): + +```sh +# CI-equivalent — runs TypeScript check, Jest, ESLint, and a production webpack build in parallel +./querybook/scripts/run_test --node + +# Or run individual checks manually +yarn tsc-check # TypeScript type checking +yarn test # Jest unit tests +yarn lint # ESLint with auto-fix +``` + +**Linting (all changes):** + +Pre-commit hooks (Black, Prettier, flake8) run automatically on `git commit` if installed, and in CI on every PR. To set them up locally: + +```sh +pip install pre-commit +pre-commit install +``` + ### Pull Request Format When you create a new pull request, please make sure it's title follows the specifications in https://www.conventionalcommits.org/en/v1.0.0/. This is to ensure automatic versioning. From a1486122343c94552245fa7b3cdb8c168d0c361e Mon Sep 17 00:00:00 2001 From: AaronW Date: Thu, 26 Mar 2026 23:57:14 +0000 Subject: [PATCH 05/11] Fix lint errors --- .../queryEditor/useStaleQueryWarning.test.ts | 16 +++++++--------- .../DataDocQueryCell/DataDocQueryCell.tsx | 9 +++++++-- .../components/QueryComposer/QueryComposer.tsx | 6 ++++-- .../hooks/queryEditor/useStaleQueryWarning.ts | 6 +++++- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts b/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts index 77fbadb51..92c18884f 100644 --- a/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts +++ b/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts @@ -18,9 +18,7 @@ describe('getSnapshotForExecution', () => { }); test('returns snapshot when it exists', () => { - expect(getSnapshotForExecution(1, snapshots)).toBe( - 'SELECT * FROM t1' - ); + expect(getSnapshotForExecution(1, snapshots)).toBe('SELECT * FROM t1'); }); test('falls back to initialQuery when no snapshot exists', () => { @@ -34,9 +32,9 @@ describe('getSnapshotForExecution', () => { }); test('prefers snapshot over initialQuery', () => { - expect( - getSnapshotForExecution(1, snapshots, 'something else') - ).toBe('SELECT * FROM t1'); + expect(getSnapshotForExecution(1, snapshots, 'something else')).toBe( + 'SELECT * FROM t1' + ); }); }); @@ -129,8 +127,8 @@ describe('shouldComputeStaleWarning', () => { }); test('works with empty snapshots and initialQuery provided', () => { - expect( - shouldComputeStaleWarning(1, {}, 'changed', 'original') - ).toBe(true); + expect(shouldComputeStaleWarning(1, {}, 'changed', 'original')).toBe( + true + ); }); }); diff --git a/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx b/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx index 4abd985ec..b8d2be0cb 100644 --- a/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx +++ b/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx @@ -451,7 +451,9 @@ class DataDocQueryCellComponent extends React.PureComponent { @bind public getRunInputString(): string { - return this.queryEditorRef.current?.getSelection?.() ?? this.state.query; + return ( + this.queryEditorRef.current?.getSelection?.() ?? this.state.query + ); } @bind @@ -523,7 +525,10 @@ class DataDocQueryCellComponent extends React.PureComponent { executionMetadata, peerReviewParams ); - this.recordRunInputSnapshot(queryExecution.id, this.state.query); + this.recordRunInputSnapshot( + queryExecution.id, + this.state.query + ); return queryExecution.id; } ); diff --git a/querybook/webapp/components/QueryComposer/QueryComposer.tsx b/querybook/webapp/components/QueryComposer/QueryComposer.tsx index f091dd6a1..fca27e9b4 100644 --- a/querybook/webapp/components/QueryComposer/QueryComposer.tsx +++ b/querybook/webapp/components/QueryComposer/QueryComposer.tsx @@ -470,8 +470,10 @@ const QueryComposer: React.FC = () => { const [showPeerReviewModal, setShowPeerReviewModal] = useState(false); const hasPeerReviewFeature = engine?.feature_params?.peer_review; - const { snapshots: executionRunInputById, recordSnapshot: recordRunInputSnapshot } = - useExecutionSnapshots(); + const { + snapshots: executionRunInputById, + recordSnapshot: recordRunInputSnapshot, + } = useExecutionSnapshots(); const initialQueryRef = useRef(reduxQuery); if (!initialQueryRef.current && reduxQuery) { diff --git a/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts b/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts index 46cdbfc02..01760be31 100644 --- a/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts +++ b/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts @@ -39,7 +39,11 @@ export function useStaleQueryWarning(options: { initialQuery?: string; debounceMs?: number; onUpdateQuery?: (query: string, run?: boolean) => any; -}): { showWarning: boolean; snapshotQuery: string | undefined; onRevert?: () => void } { +}): { + showWarning: boolean; + snapshotQuery: string | undefined; + onRevert?: () => void; +} { const { selectedExecutionId, snapshots, From 5e6da88ba1d569fa5607be100a0c259b941c2887 Mon Sep 17 00:00:00 2001 From: AaronW Date: Fri, 27 Mar 2026 00:04:01 +0000 Subject: [PATCH 06/11] Fix Prettier formatting --- .../QueryComposer/QueryComposerExecution.tsx | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx index b8d741897..1297b7d94 100644 --- a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx +++ b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx @@ -40,14 +40,13 @@ export const QueryComposerExecution: React.FunctionComponent = ({ (state: IStoreState) => state.queryExecutions.queryExecutionById[id] ); - const { showWarning: showStaleWarning, onRevert } = - useStaleQueryWarning({ - selectedExecutionId: id, - snapshots: executionRunInputSnapshots ?? {}, - currentRunInput: currentRunInput ?? '', - initialQuery, - onUpdateQuery, - }); + const { showWarning: showStaleWarning, onRevert } = useStaleQueryWarning({ + selectedExecutionId: id, + snapshots: executionRunInputSnapshots ?? {}, + currentRunInput: currentRunInput ?? '', + initialQuery, + onUpdateQuery, + }); if (!execution) { return null; @@ -64,9 +63,7 @@ export const QueryComposerExecution: React.FunctionComponent = ({ return (
- {showStaleWarning && ( - - )} + {showStaleWarning && }
From 5ffa7f96d526970fcf8d23161a0862ce267fecd1 Mon Sep 17 00:00:00 2001 From: AaronW Date: Fri, 27 Mar 2026 22:09:41 +0000 Subject: [PATCH 07/11] Update contribution guidelines with enhanced testing instructions --- .../docs/developer_guide/contributing.mdx | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/docs_website/docs/developer_guide/contributing.mdx b/docs_website/docs/developer_guide/contributing.mdx index 0fadff567..69882a6e1 100644 --- a/docs_website/docs/developer_guide/contributing.mdx +++ b/docs_website/docs/developer_guide/contributing.mdx @@ -67,28 +67,24 @@ To increase the chances that your pull request will be accepted: ### Testing -CI runs all of the checks below automatically on every pull request via GitHub Actions. You should reproduce them locally before pushing. +CI runs these checks on every pull request via GitHub Actions (a maintainer may need to approve the workflow run for external contributors). You should reproduce them locally before pushing. -**Backend** (changes under `querybook/server/`): +The simplest way to run the full test suite locally is: ```sh -# Docker-based (recommended) — builds the test image and runs pytest make test - -# Without Docker (mirrors CI) -PYTHONPATH=querybook/server:plugins ./querybook/scripts/run_test --python ``` -**Frontend** (changes under `querybook/webapp/`): +This builds a Docker test image and runs both Python and Node tests in parallel, mirroring CI. + +If you only changed one side and want a faster feedback loop: ```sh -# CI-equivalent — runs TypeScript check, Jest, ESLint, and a production webpack build in parallel -./querybook/scripts/run_test --node +# Backend only (changes under querybook/server/) +PYTHONPATH=querybook/server:plugins ./querybook/scripts/run_test --python -# Or run individual checks manually -yarn tsc-check # TypeScript type checking -yarn test # Jest unit tests -yarn lint # ESLint with auto-fix +# Frontend only (changes under querybook/webapp/) +./querybook/scripts/run_test --node ``` **Linting (all changes):** From f5c96886f9055f1e44b407f53129935bd3246e3c Mon Sep 17 00:00:00 2001 From: AaronW Date: Fri, 27 Mar 2026 22:10:23 +0000 Subject: [PATCH 08/11] Remove Revert in stale query warning --- .../queryEditor/useStaleQueryWarning.test.ts | 40 +---------------- .../DataDocQueryExecutions.tsx | 18 +++----- .../StaleQueryWarning.tsx | 21 +-------- .../QueryComposer/QueryComposerExecution.tsx | 5 +-- .../hooks/queryEditor/useStaleQueryWarning.ts | 44 +++---------------- 5 files changed, 17 insertions(+), 111 deletions(-) diff --git a/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts b/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts index 92c18884f..d5da97326 100644 --- a/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts +++ b/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts @@ -1,42 +1,4 @@ -import { - getSnapshotForExecution, - shouldComputeStaleWarning, -} from 'hooks/queryEditor/useStaleQueryWarning'; - -describe('getSnapshotForExecution', () => { - const snapshots: Record = { - 1: 'SELECT * FROM t1', - 2: 'SELECT * FROM t2', - }; - - test('returns undefined when executionId is null', () => { - expect(getSnapshotForExecution(null, snapshots)).toBeUndefined(); - }); - - test('returns undefined when executionId is undefined', () => { - expect(getSnapshotForExecution(undefined, snapshots)).toBeUndefined(); - }); - - test('returns snapshot when it exists', () => { - expect(getSnapshotForExecution(1, snapshots)).toBe('SELECT * FROM t1'); - }); - - test('falls back to initialQuery when no snapshot exists', () => { - expect(getSnapshotForExecution(99, snapshots, 'SELECT 1')).toBe( - 'SELECT 1' - ); - }); - - test('returns undefined when no snapshot and no initialQuery', () => { - expect(getSnapshotForExecution(99, snapshots)).toBeUndefined(); - }); - - test('prefers snapshot over initialQuery', () => { - expect(getSnapshotForExecution(1, snapshots, 'something else')).toBe( - 'SELECT * FROM t1' - ); - }); -}); +import { shouldComputeStaleWarning } from 'hooks/queryEditor/useStaleQueryWarning'; describe('shouldComputeStaleWarning', () => { const snapshots: Record = { diff --git a/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx b/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx index ab17f290d..115558a0d 100644 --- a/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx +++ b/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx @@ -160,14 +160,12 @@ export const DataDocQueryExecutions: React.FunctionComponent = const selectedExecution = queryExecutions[selectedExecutionIndex]; - const { showWarning: showStaleWarning, onRevert } = - useStaleQueryWarning({ - selectedExecutionId: selectedExecution?.id ?? null, - snapshots: executionRunInputSnapshots ?? {}, - currentRunInput: currentRunInput ?? '', - initialQuery, - onUpdateQuery, - }); + const { showWarning: showStaleWarning } = useStaleQueryWarning({ + selectedExecutionId: selectedExecution?.id ?? null, + snapshots: executionRunInputSnapshots ?? {}, + currentRunInput: currentRunInput ?? '', + initialQuery, + }); const queryExecutionDOM = selectedExecution && ( = return (
- {showStaleWarning && ( - - )} + {showStaleWarning && } {generateExecutionsPickerDOM()} {queryExecutionDOM} diff --git a/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx b/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx index aa0831432..76f7583a4 100644 --- a/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx +++ b/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx @@ -1,30 +1,13 @@ import React from 'react'; -import { TextButton } from 'ui/Button/Button'; import { Message } from 'ui/Message/Message'; -interface IProps { - onRevert?: () => void; -} - -export const StaleQueryWarning: React.FC = ({ onRevert }) => ( +export const StaleQueryWarning: React.FC = () => ( - Query has been edited. - {onRevert && ( - - )} - - } + message="Query has been edited." /> ); diff --git a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx index 1297b7d94..bc1842daf 100644 --- a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx +++ b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx @@ -40,12 +40,11 @@ export const QueryComposerExecution: React.FunctionComponent = ({ (state: IStoreState) => state.queryExecutions.queryExecutionById[id] ); - const { showWarning: showStaleWarning, onRevert } = useStaleQueryWarning({ + const { showWarning: showStaleWarning } = useStaleQueryWarning({ selectedExecutionId: id, snapshots: executionRunInputSnapshots ?? {}, currentRunInput: currentRunInput ?? '', initialQuery, - onUpdateQuery, }); if (!execution) { @@ -63,7 +62,7 @@ export const QueryComposerExecution: React.FunctionComponent = ({ return (
- {showStaleWarning && } + {showStaleWarning && }
diff --git a/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts b/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts index 01760be31..8d8c1ab05 100644 --- a/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts +++ b/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts @@ -1,31 +1,17 @@ -import { useCallback } from 'react'; - import { useDebounce } from 'hooks/useDebounce'; const DEFAULT_DEBOUNCE_MS = 300; -export function getSnapshotForExecution( - executionId: number | null | undefined, - snapshots: Readonly>, - initialQuery?: string -): string | undefined { - if (executionId == null) { - return undefined; - } - return snapshots[executionId] ?? initialQuery; -} - export function shouldComputeStaleWarning( selectedExecutionId: number | null | undefined, snapshots: Readonly>, currentInput: string, initialQuery?: string ): boolean { - const snapshot = getSnapshotForExecution( - selectedExecutionId, - snapshots, - initialQuery - ); + if (selectedExecutionId == null) { + return false; + } + const snapshot = snapshots[selectedExecutionId] ?? initialQuery; if (snapshot === undefined) { return false; } @@ -38,19 +24,13 @@ export function useStaleQueryWarning(options: { currentRunInput: string; initialQuery?: string; debounceMs?: number; - onUpdateQuery?: (query: string, run?: boolean) => any; -}): { - showWarning: boolean; - snapshotQuery: string | undefined; - onRevert?: () => void; -} { +}): { showWarning: boolean } { const { selectedExecutionId, snapshots, currentRunInput, initialQuery, debounceMs = DEFAULT_DEBOUNCE_MS, - onUpdateQuery, } = options; const debouncedInput = useDebounce(currentRunInput, debounceMs); @@ -71,21 +51,7 @@ export function useStaleQueryWarning(options: { initialQuery ); - const snapshotQuery = getSnapshotForExecution( - selectedExecutionId, - snapshots, - initialQuery - ); - - const onRevert = useCallback(() => { - if (snapshotQuery !== undefined) { - onUpdateQuery?.(snapshotQuery); - } - }, [snapshotQuery, onUpdateQuery]); - return { showWarning: isStaleDebounced && isStaleRealtime, - snapshotQuery, - onRevert: onUpdateQuery ? onRevert : undefined, }; } From 756a40d47e07b6d106631419a247103b409f3097 Mon Sep 17 00:00:00 2001 From: AaronW Date: Tue, 31 Mar 2026 18:25:49 +0000 Subject: [PATCH 09/11] Resolve comments --- AGENTS.md | 39 +++++++++++-------- .../DataDocQueryExecutions.tsx | 21 +++++----- .../StaleQueryWarning.scss | 14 +++++++ .../StaleQueryWarning.tsx | 18 +++++---- .../QueryComposer/QueryComposerExecution.tsx | 4 +- 5 files changed, 59 insertions(+), 37 deletions(-) create mode 100644 querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.scss diff --git a/AGENTS.md b/AGENTS.md index 0ae7e5ddb..0c6b8e3a8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,8 +47,17 @@ Key settings live in `querybook/server/env.py` (`QuerybookSettings`). ## Running Locally +Start the full stack (web server, worker, scheduler, and all dependencies) with Docker Compose: + +```bash +make +``` + +This brings up everything and serves the app at http://localhost:10001. This is the primary command for local development. + +To restart individual services without bouncing the full stack: + ```bash -make # docker-compose up (full stack) → http://localhost:10001 make web # web server only make worker # celery worker make scheduler # celery beat @@ -56,29 +65,27 @@ make scheduler # celery beat ## Making Commits -When preparing a PR, run the relevant checks. CI runs all of the following automatically on every PR via GitHub Actions (`.github/workflows/`). +When preparing a PR, run the relevant checks. CI runs all of the following via GitHub Actions (`.github/workflows/`), but must be manually triggered by a maintainer. -**Backend changes** (anything under `querybook/server/`): -- `make test` — builds the `querybook-test` Docker image and runs pytest inside it -- CI-equivalent without Docker: `PYTHONPATH=querybook/server:plugins ./querybook/scripts/run_test --python` +Always run tests via `make test`, which builds a `querybook-test` Docker image and runs checks inside it. This ensures an isolated, reproducible environment. Do not run test commands (pytest, yarn, webpack) directly on the host. -**Frontend changes** (anything under `querybook/webapp/`): -- `./querybook/scripts/run_test --node` — CI-equivalent; runs all four checks below in parallel: - - `yarn tsc-check` — TypeScript type checking - - `yarn test` — Jest unit tests - - `yarn lint` — ESLint with auto-fix - - `webpack --mode=production` — production build verification +`make test` runs both backend and frontend checks: +- **Backend** (anything under `querybook/server/`): pytest +- **Frontend** (anything under `querybook/webapp/`): TypeScript type checking, Jest unit tests, ESLint, and production build verification **Formatting (all changes) — common CI failure:** -`./querybook/scripts/run_test --node` does **not** run Prettier. CI runs Prettier separately via `pre-commit`, so formatting issues are a frequent cause of CI failures. Always run Prettier on changed files before pushing: +`make test` does **not** run Prettier. CI runs Prettier separately via `pre-commit`, so formatting issues are a frequent cause of CI failures. After running `make test`, also run Prettier on changed files before pushing: -- `yarn prettier --write ` — format specific files -- `pre-commit run --all-files` — run the full pre-commit suite (Black, Prettier, flake8) +```bash +npx prettier --write +``` -To catch formatting automatically on every `git commit`, install the hooks once: +For a full formatting pass (Black for Python, Prettier for JS/TS, flake8): -- `pip install pre-commit && pre-commit install` +```bash +pre-commit run --all-files +``` ## Pinterest Internal Deployment diff --git a/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx b/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx index 115558a0d..d7cef359d 100644 --- a/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx +++ b/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx @@ -18,7 +18,6 @@ import { getQueryString } from 'lib/utils/query-string'; import * as queryExecutionsActions from 'redux/queryExecutions/action'; import * as queryExecutionsSelectors from 'redux/queryExecutions/selector'; import { StyledText } from 'ui/StyledText/StyledText'; - import { StaleQueryWarning } from './StaleQueryWarning'; interface IProps { @@ -128,6 +127,15 @@ export const DataDocQueryExecutions: React.FunctionComponent = [queryExecutions] ); + const selectedExecution = queryExecutions[selectedExecutionIndex]; + + const { showWarning: showStaleWarning } = useStaleQueryWarning({ + selectedExecutionId: selectedExecution?.id ?? null, + snapshots: executionRunInputSnapshots ?? {}, + currentRunInput: currentRunInput ?? '', + initialQuery, + }); + const generateExecutionsPickerDOM = () => { const currentExecution = queryExecutions?.[selectedExecutionIndex]; @@ -138,6 +146,7 @@ export const DataDocQueryExecutions: React.FunctionComponent = return (
+ {showStaleWarning && } = ); }; - const selectedExecution = queryExecutions[selectedExecutionIndex]; - - const { showWarning: showStaleWarning } = useStaleQueryWarning({ - selectedExecutionId: selectedExecution?.id ?? null, - snapshots: executionRunInputSnapshots ?? {}, - currentRunInput: currentRunInput ?? '', - initialQuery, - }); - const queryExecutionDOM = selectedExecution && ( = return (
- {showStaleWarning && } {generateExecutionsPickerDOM()} {queryExecutionDOM} diff --git a/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.scss b/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.scss new file mode 100644 index 000000000..3113e2da2 --- /dev/null +++ b/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.scss @@ -0,0 +1,14 @@ +.stale-query-indicator[data-balloon-pos] { + cursor: default; +} + +.stale-query-indicator { + align-items: center; + padding: 2px 8px; + border-radius: var(--border-radius-sm); + background-color: var(--color-warning-lightest-0); + color: var(--color-warning-dark); + font-size: var(--xsmall-text-size); + font-weight: var(--bold-font); + white-space: nowrap; +} diff --git a/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx b/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx index 76f7583a4..23e63f6f6 100644 --- a/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx +++ b/querybook/webapp/components/DataDocQueryExecutions/StaleQueryWarning.tsx @@ -1,13 +1,15 @@ import React from 'react'; +import { Icon } from 'ui/Icon/Icon'; -import { Message } from 'ui/Message/Message'; +import './StaleQueryWarning.scss'; export const StaleQueryWarning: React.FC = () => ( - + + + Edited + ); diff --git a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx index bc1842daf..5f17a4aa4 100644 --- a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx +++ b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx @@ -1,7 +1,6 @@ import React from 'react'; import { useSelector } from 'react-redux'; -import { StaleQueryWarning } from 'components/DataDocQueryExecutions/StaleQueryWarning'; import { QueryExecution } from 'components/QueryExecution/QueryExecution'; import { QueryExecutionDuration } from 'components/QueryExecution/QueryExecutionDuration'; import { QueryExecutionBar } from 'components/QueryExecutionBar/QueryExecutionBar'; @@ -14,6 +13,7 @@ import { IStoreState } from 'redux/store/types'; import { Level } from 'ui/Level/Level'; import { StatusIcon } from 'ui/StatusIcon/StatusIcon'; import { AccentText } from 'ui/StyledText/StyledText'; +import { StaleQueryWarning } from 'components/DataDocQueryExecutions/StaleQueryWarning'; interface IProps { id: number; @@ -62,7 +62,6 @@ export const QueryComposerExecution: React.FunctionComponent = ({ return (
- {showStaleWarning && }
@@ -71,6 +70,7 @@ export const QueryComposerExecution: React.FunctionComponent = ({ Execution {execution.id} + {showStaleWarning && }
From 898946858378a46c15cc7c48590024ea145be089 Mon Sep 17 00:00:00 2001 From: AaronW Date: Thu, 2 Apr 2026 20:06:31 +0000 Subject: [PATCH 10/11] Enhance stale query warning to determine saved vs. edited --- .../queryEditor/useStaleQueryWarning.test.ts | 116 +++++++++++++++++- .../DataDocQueryCell/DataDocQueryCell.tsx | 3 +- .../DataDocQueryExecutions.tsx | 27 +++- .../StaleQueryWarning.tsx | 15 ++- .../QueryComposer/QueryComposerExecution.tsx | 9 +- .../hooks/queryEditor/useStaleQueryWarning.ts | 88 ++++++++++--- 6 files changed, 229 insertions(+), 29 deletions(-) diff --git a/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts b/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts index d5da97326..2740a72f9 100644 --- a/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts +++ b/querybook/webapp/__tests__/hooks/queryEditor/useStaleQueryWarning.test.ts @@ -1,4 +1,7 @@ -import { shouldComputeStaleWarning } from 'hooks/queryEditor/useStaleQueryWarning'; +import { + shouldComputeStaleWarning, + computeStaleWarningState, +} from 'hooks/queryEditor/useStaleQueryWarning'; describe('shouldComputeStaleWarning', () => { const snapshots: Record = { @@ -94,3 +97,114 @@ describe('shouldComputeStaleWarning', () => { ); }); }); + +describe('computeStaleWarningState', () => { + const snapshots: Record = { + 1: 'SELECT * FROM t1', + 2: 'SELECT * FROM t2', + }; + + const base = { + selectedExecutionId: 1 as number | null, + snapshots, + initialQuery: undefined as string | undefined, + }; + + test('returns null when live input matches snapshot', () => { + expect( + computeStaleWarningState({ + ...base, + savedInput: 'SELECT * FROM t1', + liveInput: 'SELECT * FROM t1', + }) + ).toBeNull(); + }); + + test('returns null when no execution is selected', () => { + expect( + computeStaleWarningState({ + ...base, + selectedExecutionId: null, + savedInput: 'SELECT * FROM t1', + liveInput: 'SELECT * FROM edited', + }) + ).toBeNull(); + }); + + test('returns "edited" when both live and saved differ from snapshot and not saving', () => { + expect( + computeStaleWarningState({ + ...base, + savedInput: 'SELECT * FROM edited', + liveInput: 'SELECT * FROM edited', + }) + ).toBe('edited'); + }); + + test('returns "unsaved" when live differs but saved still matches snapshot', () => { + expect( + computeStaleWarningState({ + ...base, + savedInput: 'SELECT * FROM t1', + liveInput: 'SELECT * FROM edited', + }) + ).toBe('unsaved'); + }); + + test('returns "unsaved" when both differ from snapshot but save is in-flight', () => { + expect( + computeStaleWarningState({ + ...base, + savedInput: 'SELECT * FROM edited', + liveInput: 'SELECT * FROM edited', + isSaving: true, + }) + ).toBe('unsaved'); + }); + + test('returns "unsaved" when live differs and saved matches snapshot while saving', () => { + expect( + computeStaleWarningState({ + ...base, + savedInput: 'SELECT * FROM t1', + liveInput: 'SELECT * FROM edited', + isSaving: true, + }) + ).toBe('unsaved'); + }); + + test('returns null when live matches snapshot even if saving', () => { + expect( + computeStaleWarningState({ + ...base, + savedInput: 'SELECT * FROM t1', + liveInput: 'SELECT * FROM t1', + isSaving: true, + }) + ).toBeNull(); + }); + + test('uses initialQuery fallback when no snapshot exists', () => { + expect( + computeStaleWarningState({ + ...base, + selectedExecutionId: 99, + savedInput: 'SELECT 1', + liveInput: 'SELECT 2', + initialQuery: 'SELECT 1', + }) + ).toBe('unsaved'); + }); + + test('returns "edited" with initialQuery when both differ', () => { + expect( + computeStaleWarningState({ + ...base, + selectedExecutionId: 99, + savedInput: 'SELECT 2', + liveInput: 'SELECT 2', + initialQuery: 'SELECT 1', + }) + ).toBe('edited'); + }); +}); diff --git a/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx b/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx index b8d2be0cb..cb5606bb0 100644 --- a/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx +++ b/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx @@ -1106,7 +1106,8 @@ class DataDocQueryCellComponent extends React.PureComponent { onSamplingInfoClick={this.toggleShowTableSamplingInfoModal} hasSamplingTables={this.hasSamplingTables} sampleRate={this.sampleRate} - currentRunInput={this.state.query} + savedRunInput={this.props.query} + liveRunInput={this.state.query} executionRunInputSnapshots={this.state.executionRunInputById} initialQuery={this.initialQuery} /> diff --git a/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx b/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx index d7cef359d..d1c4b02b4 100644 --- a/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx +++ b/querybook/webapp/components/DataDocQueryExecutions/DataDocQueryExecutions.tsx @@ -5,7 +5,7 @@ import React, { useMemo, useState, } from 'react'; -import { useDispatch } from 'react-redux'; +import { useDispatch, useSelector } from 'react-redux'; import { QueryExecutionPicker } from 'components/ExecutionPicker/QueryExecutionPicker'; import { QueryExecution } from 'components/QueryExecution/QueryExecution'; @@ -17,6 +17,7 @@ import { useStaleQueryWarning } from 'hooks/queryEditor/useStaleQueryWarning'; import { getQueryString } from 'lib/utils/query-string'; import * as queryExecutionsActions from 'redux/queryExecutions/action'; import * as queryExecutionsSelectors from 'redux/queryExecutions/selector'; +import { IStoreState } from 'redux/store/types'; import { StyledText } from 'ui/StyledText/StyledText'; import { StaleQueryWarning } from './StaleQueryWarning'; @@ -31,7 +32,8 @@ interface IProps { hasSamplingTables?: boolean; sampleRate: number; - currentRunInput?: string; + savedRunInput?: string; + liveRunInput?: string; executionRunInputSnapshots?: Readonly>; initialQuery?: string; } @@ -46,7 +48,8 @@ export const DataDocQueryExecutions: React.FunctionComponent = onSamplingInfoClick, hasSamplingTables, sampleRate, - currentRunInput, + savedRunInput, + liveRunInput, executionRunInputSnapshots, initialQuery, }) => { @@ -129,11 +132,21 @@ export const DataDocQueryExecutions: React.FunctionComponent = const selectedExecution = queryExecutions[selectedExecutionIndex]; - const { showWarning: showStaleWarning } = useStaleQueryWarning({ + const isCellSaving = useSelector((state: IStoreState) => { + const savePromise = state.dataDoc.dataDocSavePromiseById[docId]; + if (!savePromise) { + return false; + } + return `cell-${cellId}` in savePromise.itemToSave; + }); + + const { warningState } = useStaleQueryWarning({ selectedExecutionId: selectedExecution?.id ?? null, snapshots: executionRunInputSnapshots ?? {}, - currentRunInput: currentRunInput ?? '', + savedRunInput: savedRunInput ?? '', + liveRunInput: liveRunInput ?? '', initialQuery, + isSaving: isCellSaving, }); const generateExecutionsPickerDOM = () => { @@ -146,7 +159,9 @@ export const DataDocQueryExecutions: React.FunctionComponent = return (
- {showStaleWarning && } + {warningState && ( + + )} ( +interface IProps { + variant: NonNullable; +} + +const TOOLTIP: Record, string> = { + edited: 'Changes are saved but not executed', + unsaved: 'Changes are unsaved and not executed', +}; + +export const StaleQueryWarning: React.FC = ({ variant }) => ( diff --git a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx index 5f17a4aa4..f6071deb4 100644 --- a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx +++ b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx @@ -40,10 +40,11 @@ export const QueryComposerExecution: React.FunctionComponent = ({ (state: IStoreState) => state.queryExecutions.queryExecutionById[id] ); - const { showWarning: showStaleWarning } = useStaleQueryWarning({ + const { warningState } = useStaleQueryWarning({ selectedExecutionId: id, snapshots: executionRunInputSnapshots ?? {}, - currentRunInput: currentRunInput ?? '', + savedRunInput: currentRunInput ?? '', + liveRunInput: currentRunInput ?? '', initialQuery, }); @@ -70,7 +71,9 @@ export const QueryComposerExecution: React.FunctionComponent = ({ Execution {execution.id} - {showStaleWarning && } + {warningState && ( + + )}
diff --git a/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts b/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts index 8d8c1ab05..4e17c31d7 100644 --- a/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts +++ b/querybook/webapp/hooks/queryEditor/useStaleQueryWarning.ts @@ -2,6 +2,8 @@ import { useDebounce } from 'hooks/useDebounce'; const DEFAULT_DEBOUNCE_MS = 300; +export type StaleWarningState = 'edited' | 'unsaved' | null; + export function shouldComputeStaleWarning( selectedExecutionId: number | null | undefined, snapshots: Readonly>, @@ -18,40 +20,94 @@ export function shouldComputeStaleWarning( return currentInput !== snapshot; } +export function computeStaleWarningState(options: { + selectedExecutionId: number | null | undefined; + snapshots: Readonly>; + savedInput: string; + liveInput: string; + initialQuery?: string; + isSaving?: boolean; +}): StaleWarningState { + const { + selectedExecutionId, + snapshots, + savedInput, + liveInput, + initialQuery, + isSaving = false, + } = options; + + const isLiveStale = shouldComputeStaleWarning( + selectedExecutionId, + snapshots, + liveInput, + initialQuery + ); + + if (!isLiveStale) { + return null; + } + + const isSavedStale = shouldComputeStaleWarning( + selectedExecutionId, + snapshots, + savedInput, + initialQuery + ); + + if (isSavedStale && !isSaving) { + return 'edited'; + } + + return 'unsaved'; +} + export function useStaleQueryWarning(options: { selectedExecutionId: number | null | undefined; snapshots: Readonly>; - currentRunInput: string; + savedRunInput: string; + liveRunInput: string; initialQuery?: string; debounceMs?: number; -}): { showWarning: boolean } { + isSaving?: boolean; +}): { warningState: StaleWarningState } { const { selectedExecutionId, snapshots, - currentRunInput, + savedRunInput, + liveRunInput, initialQuery, debounceMs = DEFAULT_DEBOUNCE_MS, + isSaving = false, } = options; - const debouncedInput = useDebounce(currentRunInput, debounceMs); + const debouncedLiveInput = useDebounce(liveRunInput, debounceMs); // Debounced check stays stable during typing to prevent flickering. // Real-time check instantly suppresses false positives on initial page load - // (when currentRunInput briefly differs from the snapshot before the editor populates). - const isStaleDebounced = shouldComputeStaleWarning( + // (when liveRunInput briefly differs from the snapshot before the editor populates). + const debouncedState = computeStaleWarningState({ selectedExecutionId, snapshots, - debouncedInput, - initialQuery - ); - const isStaleRealtime = shouldComputeStaleWarning( + savedInput: savedRunInput, + liveInput: debouncedLiveInput, + initialQuery, + isSaving, + }); + const realtimeState = computeStaleWarningState({ selectedExecutionId, snapshots, - currentRunInput, - initialQuery - ); + savedInput: savedRunInput, + liveInput: liveRunInput, + initialQuery, + isSaving, + }); + + // Both must agree on a non-null state; if either is null, suppress. + const warningState = + debouncedState !== null && realtimeState !== null + ? debouncedState + : null; - return { - showWarning: isStaleDebounced && isStaleRealtime, - }; + return { warningState }; } From 7de31dffd8819b9d4dac51266bbd289c8a7280da Mon Sep 17 00:00:00 2001 From: AaronW Date: Sat, 11 Apr 2026 00:31:12 +0000 Subject: [PATCH 11/11] Resolve comments --- AGENTS.md | 4 ---- .../components/QueryComposer/QueryComposerExecution.tsx | 6 +++--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0c6b8e3a8..5869a0eec 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -87,10 +87,6 @@ For a full formatting pass (Black for Python, Prettier for JS/TS, flake8): pre-commit run --all-files ``` -## Pinterest Internal Deployment - -Pinterest deploys Querybook internally via a separate repo (`datahub-pinterest`) that uses this repo's Docker image as a base and layers Pinterest-specific plugins on top. Changes to core Querybook go here; Pinterest-specific features belong in that plugin repo. Do not add Pinterest-internal details to this file. - ## Maintaining This File **Include:** diff --git a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx index f6071deb4..b9349d798 100644 --- a/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx +++ b/querybook/webapp/components/QueryComposer/QueryComposerExecution.tsx @@ -65,15 +65,15 @@ export const QueryComposerExecution: React.FunctionComponent = ({
+ {warningState && ( + + )} {statusIcon} Execution {execution.id} - {warningState && ( - - )}