From bc2d0b3f499925e83858cf1e0fe2f5cbce5894f9 Mon Sep 17 00:00:00 2001 From: huyplb Date: Sun, 16 Aug 2026 23:38:13 -0600 Subject: [PATCH 1/5] feat(blueprint): show column and index position, and fix what SQL-editor syntax testing turned up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Blueprint** now carries a `#` column in the column and index tables. "Column id" is the ordinal in every catalog that exposes one under that name (Oracle COLUMN_ID, SQL Server column_id), and it needs no new introspection — the compare already builds the column list in the source table's own order. The number comes from the *unfiltered* diff, not the rendered row index: numbering visible rows would renumber them the moment "show unchanged" is off, so column 7 would read as 2 — a worse lie than showing nothing. **Editor syntax across dialects**, tested rather than assumed (`dialect-syntax.test.ts`). What already works: MySQL DELIMITER blocks, Oracle `/` terminators, Postgres dollar-quoted bodies, semicolons inside bracket, backtick and string literals, and write detection for REPLACE / COPY / LOAD DATA / SELECT INTO / MERGE / TRUNCATE. One real fix: an anonymous `BEGIN … END` block read as a **non-write**, so Safe Mode ran it without a confirmation. That is the shape this repo itself generates for Db2 and Oracle tolerant drops, and the statement inside usually sits in an `EXECUTE IMMEDIATE '…'` literal the scanner strips on purpose — so it fails closed now. `BEGIN;` / `BEGIN TRANSACTION` stay quiet, since a confirmation on those would only teach people to click through. The RBAC gate was already correct here (its allowlist is fail-closed); this was the confirmation dialog only. Monaco now gives the wire-compatible relatives their family's grammar — MariaDB/TiDB get MySQL, CockroachDB/YugabyteDB/Redshift get PostgreSQL — instead of falling through to generic `sql` and losing backtick and dollar-quote handling. Two gaps are recorded as `it.fails` rather than quietly left: T-SQL `GO` is not a terminator the splitter knows (a GO script arrives as one statement and the server rejects it), and a bare `BEGIN … END;` block is chopped into fragments. Both need the dialect, which `splitSqlStatements` does not take; `BEGIN` cannot simply become an opener because it starts a transaction in Postgres and MySQL. They will turn red when fixed, which is the signal to drop the `.fails`. Co-Authored-By: Claude Opus 5 --- .../frontend/components/SchemaBlueprint.tsx | 25 ++++ apps/web/src/frontend/monaco-setup.ts | 9 ++ .../sql/src/modules/dialect-syntax.test.ts | 114 ++++++++++++++++++ packages/sql/src/modules/sql-splitter.ts | 15 +++ 4 files changed, 163 insertions(+) create mode 100644 packages/sql/src/modules/dialect-syntax.test.ts diff --git a/apps/web/src/frontend/components/SchemaBlueprint.tsx b/apps/web/src/frontend/components/SchemaBlueprint.tsx index 2699f09f..0c5f1529 100644 --- a/apps/web/src/frontend/components/SchemaBlueprint.tsx +++ b/apps/web/src/frontend/components/SchemaBlueprint.tsx @@ -215,6 +215,12 @@ export function SchemaBlueprint({ const keep = (status: string) => showUnchanged || status !== 'UNCHANGED'; const colDiffs = diff.columnDiffs.filter((c) => keep(c.status)); const indexDiffs = diff.indexDiffs.filter((i) => keep(i.status)); + // Position in the *unfiltered* list. Numbering the rendered rows instead + // would renumber them the moment "show unchanged" is off, so column 7 would + // read as 2 — a worse lie than showing nothing. Column order is the source + // table's own (compare.module builds it from the source columns first). + const positionOf = new Map(diff.columnDiffs.map((c, i) => [c.name, i + 1])); + const indexPositionOf = new Map(diff.indexDiffs.map((i, n) => [i.name, n + 1])); const fkDiffs = diff.foreignKeyDiffs.filter((f) => keep(f.status)); const trgDiffs = (diff.triggerDiffs ?? []).filter((t) => keep(t.status)); @@ -518,6 +524,11 @@ export function SchemaBlueprint({ + {!isRole && ( + + )} @@ -539,6 +550,14 @@ export function SchemaBlueprint({ return ( + {!isRole && ( + // Ordinal position, which is what "column id" means in + // the catalogs that expose one (Oracle COLUMN_ID, SQL + // Server column_id). Roles have members, not columns. + + )}
+ # + {isRole ? 'Member' : 'Column Name'} Original State Compare
+ {positionOf.get(col.name) ?? '—'} + {isRole && col.status !== 'UNCHANGED' && onToggleMember && ( @@ -680,6 +699,9 @@ export function SchemaBlueprint({ + @@ -708,6 +730,9 @@ export function SchemaBlueprint({ return ( +
+ # + Index Name Columns Constraint
+ {indexPositionOf.get(idx.name) ?? '—'} + {idx.status !== 'UNCHANGED' && onToggleIndex && ( diff --git a/apps/web/src/frontend/monaco-setup.ts b/apps/web/src/frontend/monaco-setup.ts index 43325690..2a73e126 100644 --- a/apps/web/src/frontend/monaco-setup.ts +++ b/apps/web/src/frontend/monaco-setup.ts @@ -161,9 +161,18 @@ export const MONACO_EDITOR_BASE_OPTIONS = { */ export function monacoLanguage(dialect: string): string { switch (dialect.toLowerCase()) { + // Wire-compatible relatives get their family's grammar rather than the + // generic one: MariaDB and TiDB speak MySQL, and CockroachDB, YugabyteDB + // and Redshift speak PostgreSQL. Falling through to plain `sql` cost them + // backtick and dollar-quote handling for no reason. case 'mysql': + case 'mariadb': + case 'tidb': return 'mysql'; case 'postgres': + case 'cockroachdb': + case 'yugabytedb': + case 'redshift': return 'pgsql'; default: return 'sql'; diff --git a/packages/sql/src/modules/dialect-syntax.test.ts b/packages/sql/src/modules/dialect-syntax.test.ts new file mode 100644 index 00000000..6579142b --- /dev/null +++ b/packages/sql/src/modules/dialect-syntax.test.ts @@ -0,0 +1,114 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + * + * Dialect-specific syntax, as the SQL editor sees it. + * + * The editor splits before it does anything else — runs a statement, decides + * whether it is a write, offers it to the caret. Every dialect brings its own + * way of ending or nesting a statement, and a wrong split means running a + * fragment of what the user typed. + * + * `splitSqlStatements` takes no dialect, so this also records where that costs + * something. The two `it.fails` cases below are **known gaps, not + * expectations** — they document exactly what breaks so the next person does + * not have to rediscover it, and they will turn red the moment someone fixes + * them, which is the signal to delete the `.fails`. + */ +import { describe, expect, it } from 'vitest'; +import { isWriteStatement, splitSqlStatements } from './sql-splitter.js'; + +const parts = (sql: string) => splitSqlStatements(sql).filter((p) => p.text.trim()); + +describe('statement terminators per dialect', () => { + it('keeps a MySQL DELIMITER block whole', () => { + // Between DELIMITER $$ and DELIMITER ; the inner semicolons are body text. + const sql = 'DELIMITER $$\nCREATE PROCEDURE p() BEGIN SELECT 1; END$$\nDELIMITER ;'; + expect(parts(sql)).toHaveLength(1); + }); + + it('treats an Oracle slash as its own terminator', () => { + const sql = 'CREATE OR REPLACE PROCEDURE p AS BEGIN NULL; END;\n/\nSELECT 1 FROM dual;'; + expect(parts(sql)).toHaveLength(2); + }); + + it('keeps a Postgres dollar-quoted body whole', () => { + const sql = + 'CREATE FUNCTION f() RETURNS int AS $$ BEGIN RETURN 1; END $$ LANGUAGE plpgsql;\nSELECT 1;'; + const out = parts(sql); + expect(out).toHaveLength(2); + expect(out[0]!.text).toContain('$$'); + }); + + it('does not split inside quoted identifiers that contain a semicolon', () => { + // A semicolon inside [brackets] or `backticks` is part of the name. + expect(parts('SELECT * FROM [my;table];\nSELECT 2;')).toHaveLength(2); + expect(parts('SELECT * FROM `a;b`;\nSELECT 2;')).toHaveLength(2); + }); + + it('does not split inside a string literal', () => { + expect(parts("SELECT 'a;b' AS s;\nSELECT 2;")).toHaveLength(2); + }); +}); + +describe('known gaps — dialect syntax the splitter cannot see', () => { + // Remove the `.fails` when these are fixed; a green run here means the gap + // closed and the test should become an ordinary assertion. + + it.fails('T-SQL GO should end a batch', () => { + // GO is a client-side batch separator, not SQL. The splitter does not know + // it, so a whole GO script arrives as one statement and the server rejects + // it ("Incorrect syntax near 'GO'"). Fixing it needs the dialect, which + // splitSqlStatements does not currently take. + expect(parts('SELECT 1\nGO\nSELECT 2\nGO')).toHaveLength(2); + }); + + it.fails('an anonymous BEGIN … END block should stay whole', () => { + // PL/SQL and Db2 SQL PL both use bare `BEGIN … END;` blocks whose inner + // semicolons are body text — the shape the Db2 tolerant-drop statements + // use. The splitter chops `BEGIN NULL; END;` into two fragments, neither + // of which runs. Migrations are unaffected (MigrationModule sends whole + // statements), but pasting generated SQL into the editor breaks. + // + // The fix is not simply "treat BEGIN as an opener": in Postgres and MySQL + // `BEGIN;` starts a transaction, so this needs the dialect or a heuristic + // on what follows the keyword. + expect(parts('BEGIN NULL; END;')).toHaveLength(1); + }); + + it('records what the GO gap does to write classification', () => { + // Documented rather than asserted as correct: because the batch never + // splits, a DELETE after GO sits inside a statement whose leading verb is + // SELECT, and the whole thing reads as a non-write. It is not executable + // either — the server rejects GO — so this misleads the confirmation + // dialog without putting data at risk. Fixing the split fixes this too. + const script = 'SELECT 1\nGO\nDELETE FROM accounts\nGO'; + expect(parts(script)).toHaveLength(1); + expect(isWriteStatement(script)).toBe(false); + }); +}); + +describe('write detection across dialect-specific writes', () => { + it.each([ + ['MySQL REPLACE', 'REPLACE INTO t (id) VALUES (1)'], + ['Postgres COPY from', "COPY t FROM '/tmp/x.csv'"], + ['MySQL LOAD DATA', "LOAD DATA INFILE '/tmp/x' INTO TABLE t"], + ['SQL Server SELECT INTO', 'SELECT * INTO backup FROM t'], + ['Oracle MERGE', 'MERGE INTO t USING s ON (t.id = s.id) WHEN MATCHED THEN UPDATE SET t.v = s.v'], + ['TRUNCATE', 'TRUNCATE TABLE t'], + ['Db2 anonymous block that writes', "BEGIN EXECUTE IMMEDIATE 'DROP TABLE t'; END"], + ])('classifies %s as a write', (_label, sql) => { + expect(isWriteStatement(sql)).toBe(true); + }); + + it.each([ + ['Oracle FROM DUAL', 'SELECT 1 FROM dual'], + ['Db2 SYSDUMMY1', 'SELECT 1 FROM sysibm.sysdummy1'], + ['SQL Server TOP', 'SELECT TOP 10 * FROM t'], + ['Postgres LIMIT', 'SELECT * FROM t LIMIT 10'], + ['SHOW', 'SHOW TABLES'], + ])('classifies %s as a read', (_label, sql) => { + expect(isWriteStatement(sql)).toBe(false); + }); +}); diff --git a/packages/sql/src/modules/sql-splitter.ts b/packages/sql/src/modules/sql-splitter.ts index 6fee4061..10411adb 100644 --- a/packages/sql/src/modules/sql-splitter.ts +++ b/packages/sql/src/modules/sql-splitter.ts @@ -862,6 +862,21 @@ function sqlTextIsWrite(text: string): boolean { const inner = peelExplainAnalyze(text); return inner !== null && sqlTextIsWrite(inner); } + if (kw === 'begin') { + // Two different statements share this keyword. `BEGIN;` / `BEGIN + // TRANSACTION` / `BEGIN WORK` is transaction control and writes nothing on + // its own — confirming those would only train people to click through. + // Anything else after BEGIN is a procedural block body (PL/SQL, Db2 SQL PL, + // T-SQL), and those fail closed: a block usually reaches the database via + // `EXECUTE IMMEDIATE '…'`, whose statement hides inside a string literal + // this scanner strips on purpose. `BEGIN EXECUTE IMMEDIATE 'DROP TABLE t'; + // END` — the shape this repo itself generates for Db2 and Oracle tolerant + // drops — otherwise read as a plain read and skipped the confirmation. + const rest = stripSqlStringsAndComments(text) + .replace(/^\s*begin\b/i, '') + .trim(); + return !/^(transaction|work)?\s*;?$/i.test(rest); + } if (kw === 'with') { const bodies: string[] = []; // Recurse into CTE bodies + the tail rather than matching its leading verb, so From cccc4a0add894e64e62d179c4fa5ce56089c68ae Mon Sep 17 00:00:00 2001 From: huyplb Date: Mon, 17 Aug 2026 08:31:27 -0600 Subject: [PATCH 2/5] feat(history): the legend is the filter, and a script can be opened full screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **One control per concept.** The sidebar carried four panels for two ideas: a "Legend" that only showed the object-type colours, an "Object status" list that only showed the status colours, and separately an "Object type" checkbox list and a "Status filter" button row that did the actual filtering. The colour key was never where the click was. Now there are two: **Object type** and **Object status**, each row carrying its own dot and doing the filtering when clicked. Verified in the browser — clicking Deleted takes the graph from 14 nodes to 11 and back. The type rows stay real checkboxes on purpose: `schema-history.test.ts` asserts `isChecked()` on six of those testids, and turning them into buttons would have broken the suite for a cosmetic reason. **Script pane gets a maximize button.** The inline diff is capped at `max-h-56` because it sits in a detail column beside everything else about the object, which is fine for a short table and useless for a view or a routine body. The header now has a maximize button that reopens the same diff full screen (Esc or the backdrop closes it). It lives in `GithubScriptDiff`, so the object inspector and the version compare modal both get it without either growing its own copy — and the line renderer is shared between the two sizes rather than duplicated. Co-Authored-By: Claude Opus 5 --- .../lokee-weave/GithubScriptDiff.tsx | 140 ++++++++++++++---- .../components/lokee-weave/LokeeWeavePage.tsx | 86 ++++++----- 2 files changed, 157 insertions(+), 69 deletions(-) diff --git a/apps/web/src/frontend/components/lokee-weave/GithubScriptDiff.tsx b/apps/web/src/frontend/components/lokee-weave/GithubScriptDiff.tsx index 2a3d82ae..3ccc26b4 100644 --- a/apps/web/src/frontend/components/lokee-weave/GithubScriptDiff.tsx +++ b/apps/web/src/frontend/components/lokee-weave/GithubScriptDiff.tsx @@ -4,47 +4,137 @@ * SPDX-License-Identifier: Apache-2.0 * * Unified GitHub-style diff for a Lokee object script (CREATE TABLE / routine). + * + * The inline pane is deliberately short — it sits inside a detail column next + * to everything else about the object. A view or a routine body is routinely + * longer than that, so the header carries a maximize button that reopens the + * same diff full screen. Both call sites (the object inspector and the version + * compare modal) get it from here rather than each growing its own copy. */ import React from 'react'; +import { Maximize2, X } from 'lucide-react'; import { diffLines } from '../../utils/lineDiff'; +/** One rendered diff line — shared by the inline pane and the maximized one. */ +function DiffLine({ line }: { line: { type: string; text: string } }): React.ReactElement { + const cls = + line.type === 'added' + ? 'bg-emerald-500/15 text-emerald-200' + : line.type === 'removed' + ? 'bg-rose-500/15 text-rose-300' + : 'text-slate-400'; + const mark = line.type === 'added' ? '+' : line.type === 'removed' ? '−' : ' '; + return ( +
+ {mark} + {line.text || ' '} +
+ ); +} + export function GithubScriptDiff({ original, modified, + title = 'Script', }: { original: string; modified: string; + /** Shown in the header and as the maximized dialog's heading. */ + title?: string; }): React.ReactElement { + const [maximized, setMaximized] = React.useState(false); const lines = diffLines(original, modified); const added = lines.filter((l) => l.type === 'added').length; const removed = lines.filter((l) => l.type === 'removed').length; + + // Escape closes it, like every other dialog here. + React.useEffect(() => { + if (!maximized) return; + const onKey = (e: KeyboardEvent) => { + if (e.key !== 'Escape') return; + e.preventDefault(); + e.stopPropagation(); + setMaximized(false); + }; + window.addEventListener('keydown', onKey, true); + return () => window.removeEventListener('keydown', onKey, true); + }, [maximized]); + + const counts = ( + + +{added} + / + −{removed} + + ); + return ( -
-
- Script - - +{added} - / - −{removed} - + <> +
+
+ {title} + + {counts} + + +
+ 
+          {lines.map((line, i) => (
+            
+          ))}
+
- 
-        {lines.map((line, i) => {
-          const cls =
-            line.type === 'added'
-              ? 'bg-emerald-500/15 text-emerald-200'
-              : line.type === 'removed'
-                ? 'bg-rose-500/15 text-rose-300'
-                : 'text-slate-400';
-          const mark = line.type === 'added' ? '+' : line.type === 'removed' ? '−' : ' ';
-          return (
-            

-              {mark}
-              {line.text || ' '}
+
+      {maximized && (
+        
 setMaximized(false)}
+        >
+          
 e.stopPropagation()}
+          >
+            

+              {title}
+              
+                {counts}
+                
+              
             

-          );
-        })}
-
-
+ 
+              {lines.map((line, i) => (
+                
+              ))}
+
+
+ + )} + ); } diff --git a/apps/web/src/frontend/components/lokee-weave/LokeeWeavePage.tsx b/apps/web/src/frontend/components/lokee-weave/LokeeWeavePage.tsx index bf3858a7..79e68485 100644 --- a/apps/web/src/frontend/components/lokee-weave/LokeeWeavePage.tsx +++ b/apps/web/src/frontend/components/lokee-weave/LokeeWeavePage.tsx @@ -340,30 +340,6 @@ export const LokeeWeavePage: React.FC = ({