feat: BROWSE per-cell validation by declared column type (#45) - #49
Merged
Merged
Conversation
Grid edits are now checked against the column's declared type before they commit. An invalid edit keeps the cell in edit mode, outlined in red, showing why; the error clears as soon as the value becomes valid, and Esc abandons the edit leaving the original value intact. The rules live in src/shared/cellValidation.ts and run on both sides: Grid.ts for instant feedback, and Session's grid-edit handler as the authoritative check. grid-edit previously bypassed the Executor and wrote straight to SQLite with no validation at all, so a WS message could plant any value in any column. Serving that needed the declared type of every column, not just TIME: SQLite's affinity cannot tell TIME/DATE/CHAR apart (all TEXT), nor LOGICAL from INT (both INTEGER), nor recover a NUM(p,s) qualifier. CREATE TABLE now records all of them, grid-open ships them to the client, and LIST STRUCTURE prints them as declared. Two bugs surfaced and are fixed here: - CREATE TABLE t (price NUM(8,2)) created a phantom column named "2" of type ")" — the parser read the precision and then treated the scale as the next column definition. The PRODUCTS, DEALS and SALES demo tables all carried the stray column. - Column metadata was keyed by (table, column) with no database scope, so same-named tables in different databases overwrote each other's declared types. Now keyed by (db, table, column), with a migration for the pre-#45 schema.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #45.
Summary
BROWSEnow validates each cell edit against its column's declared type before committing. An invalid edit keeps the cell in edit mode, outlined red, with the reason shown (HH:MM,multiple of 15,at most 2 decimal place(s),not a real date); the message clears as soon as the value becomes valid, andEscabandons the edit leaving the original value intact.The rules live in one place —
src/shared/cellValidation.ts— and run on both sides:Grid.tschecks before commit, for instant feedback (the UX ask).Session'sgrid-edithandler re-checks authoritatively before writing.That second half turned out to be necessary, not just belt-and-braces:
grid-editbypassed the Executor entirely and wrote straight to SQLite with no validation whatsoever, so a WS message could plant any value in any column. TheTIMEcheck added in #43 only ever coveredREPLACE.DATEYYYY-MM-DD, a real calendar dateTIME/TIME(n)HH:MM; minutes a multiple ofnNUM(p,s)sdecimals, ≤p - sinteger digitsINTLOGICAL.T./.F./.TRUE./.FALSE./T/F/TRUE/FALSE/1/0CHAR/MEMOEmpty clears a cell and is always allowed. Columns with no recorded declared type stay unconstrained (so pre-existing tables behave exactly as before).
Supporting this meant recording the declared type of every column, not just
TIME: SQLite's affinity can't tellTIME/DATE/CHARapart (allTEXT), norLOGICALfromINT(bothINTEGER), nor recover aNUM(p,s)qualifier.CREATE TABLEnow records all of them,grid-openships acolumnTypesmap to the client, andLIST STRUCTUREprints the declared type (NUM(8,2),TIME(15)).Two bugs found and fixed along the way
NUM(p,s)created a phantom column.CREATE TABLE t (price NUM(8,2))produced a column literally named2of type)— the parser read the precision, then parsed the scale as the next column definition. This is live: the shippedPRODUCTS,DEALSandSALESdemo tables all carry the stray column (LIST STRUCTUREon the inventory demo shows it). Fixed inparseCreate/skipTypeSize. Tables created before this fix keep the stray column until recreated.Column metadata leaked across databases.
ColumnMetaStore(added in TIME column type #43) keyed on(table, column)with no database scope, so two databases with same-named tables overwrote each other's declared types — aTIME(15)column in one DB could be validated against another DB'sCHAR(20)of the same name. Now keyed by(db, table, column), with a migration for the pre-BROWSE per-cell validation hook #45 schema (covered by a test).IndexStorehas the same unscoped-key shape; that's pre-existing and left alone here.Scope notes
REPLACEstill enforces onlyTIME, exactly as TIME column type #43 shipped it. Widening it toDATE/NUM/LOGICALwould change the semantics of existing programs and demos, and the issue scopes the new validation to the grid. Noted inCLAUDE.md.ALTER TABLE ADD/ALTERstill drops the(n)/(p,s)qualifier (pre-existingskipTypeSize()behavior for every type); it now records the base type.skipTypeSizewas taught to consume the,sso it doesn't corrupt the token stream.Assistant parity
No new command — this is
BROWSEbehavior, and the Assistant's existing Browse action opens the same grid. Per the DoD's Assistant-parity rule, the Assistant path is exercised in a real browser: a newtests/assistant.spec.tscase clicks Browse, types an off-granularityTIME(15)value, asserts the inline rejection, then commits a valid one.Test plan
Suites run serially (they share
data/).npx tsc --noEmitclean.npm test— 316/316 vitest (was 283). New:CellValidation.test.ts(18, the shared rules),ColumnMeta.test.ts(10,NUM(p,s)parsing, no phantom column, declared types inLIST STRUCTURE,grid-open.columnTypes, server-sidegrid-editrejection, cross-database isolation),ColumnMetaStore.test.ts(5, per-(db,table,column)storage + legacy-schema migration).npx playwright test— 79/79 (was 75). Newtests/grid-validation.spec.ts(3): invalidTIME(15)rejected inline then valid one commits and lands in the DB; badNUM(8,2)and impossibleDATErejected whileCHARaccepts anything;Escabandons an invalid edit and restores the original. Plus the Assistant Browse-action case above.Docs
CHANGELOG.md(Added + Fixed),README.md(column types, BROWSE cell-validation note),CLAUDE.md(architecture tree incl.cellValidation.ts, cell-validation table, roadmap, test trees and counts).