Merge pull request #51 from DDecoene/feature/50-test-hardening

test: strict CREATE TABLE, demo schema pins, grid message coverage (#50)

Author: DDecoene

.gitignore

@@ -11,3 +11,4 @@ webbase-screenshot.png
 playwright-report/
 test-results/
 out/
+coverage/

CHANGELOG.md

@@ -7,7 +7,7 @@ Versions follow [Semantic Versioning](https://semver.org/) — minor bump per su
 
 ---
 
-## [Unreleased] — v1.2.0 — TIME columns, WEEK(), grid validation, Overtime demo
+## [Unreleased] — v1.2.0 — TIME columns, WEEK(), grid validation, test hardening, Overtime demo
 
 ### Added
 - `TIME` column type — `CREATE TABLE ... (col TIME)` / `TIME(n)` for a minute-granularity
@@ -27,6 +27,14 @@ Versions follow [Semantic Versioning](https://semver.org/) — minor bump per su
   live in `src/shared/cellValidation.ts` and run on both the client (instant feedback) and
   the server (`grid-edit` is now validated authoritatively — previously it wrote straight
   to SQLite with no check at all). (#45)
+- `NUM(p,s)` is now a genuinely supported qualifier — the precision and scale are parsed,
+  recorded, and enforced on grid edits (`NUM(8,2)` accepts `123456.78`, rejects `1.234`).
+  Previously the scale silently corrupted the schema; see Fixed. (#45) The Assistant's
+  **New table** wizard accepts a width (`8`) or a precision,scale pair (`8,2`). (#50)
+- `LIST STRUCTURE` prints the **declared** type of every column (`CHAR(10)`, `NUM(8,2)`,
+  `DATE`, `TIME(15)`, `LOGICAL`, `INT`) rather than SQLite's storage class (`TEXT`/`REAL`/
+  `INTEGER`). Declared types are recorded per `(database, table, column)` in
+  `server/ColumnMetaStore.ts`. (#45)
 
 ### Fixed
 - `CREATE TABLE t (price NUM(8,2))` silently created a **phantom column named `2`** of
@@ -38,6 +46,28 @@ Versions follow [Semantic Versioning](https://semver.org/) — minor bump per su
   tables previously shared (and overwrote) one another's declared column types, so a
   `TIME(15)` column in one database could be validated against another database's
   `CHAR(20)` declaration of the same name. (#45)
+- `CREATE TABLE` now **rejects a malformed column list** instead of silently inventing
+  columns from tokens it doesn't understand. `CREATE TABLE t (a CHAR(10) b INT)` (missing
+  comma), `(a)` (no type), `(a NUM(8,2,9))` and an unclosed paren all now raise a parse
+  error naming the offending column, and create nothing. This permissiveness was the root
+  cause of the phantom-column bug above. (#50)
+- **Index metadata is now scoped per database.** `indexes`/`active_indexes` were keyed by
+  table name alone, so opening `PEOPLE` in one database silently activated an index defined
+  on a *different* database's `PEOPLE` — pointing the record order at a column that need not
+  even exist there, and breaking `BROWSE`/`LIST`. On first run, existing index definitions
+  are adopted into the one database that owns the table; definitions whose owner is ambiguous
+  (same table name in two databases) or missing are dropped and must be recreated with
+  `INDEX ON`. The underlying SQLite indexes are untouched. (#50)
+- A bare `INPUT "prompt" TO <var>` typed at the REPL silently discarded the value: the
+  submitted form was only applied when a continuation existed, which is never the case for
+  a single statement. Values a form collects are now always stored. (#50)
+
+### Changed
+- Removed the `input-request` / `input-response` WebSocket message types. They were declared
+  in the protocol but never sent or handled by anything — `INPUT` collects its value through
+  `form-open` / `form-submit`. (#50)
+- New `npm run coverage` (vitest + v8, reporting only, no thresholds), so modules no test ever
+  executes stop hiding. (#50)
 
 ---
 

CLAUDE.md

@@ -50,7 +50,7 @@ server/
   SessionManager.ts     Tracks all active sessions; broadcast() fans data-changed to peers viewing a mutated table
   ServerDatabaseBridge.ts  IDatabaseBridge impl wrapping better-sqlite3
   ProgramStore.ts       .prg program storage in data/system.sqlite3
-  IndexStore.ts         Index metadata + active index in data/system.sqlite3
+  IndexStore.ts         Index metadata + active index per (db, table) in data/system.sqlite3
   ColumnMetaStore.ts    Declared column types per (db, table, column) in data/system.sqlite3 — SQLite affinity can't distinguish TIME/DATE/CHAR, LOGICAL/INT, or recover NUM(p,s)
   ReportStore.ts        Report definition storage in data/system.sqlite3 (reports table)
   ReportRunner.ts       ASCII and HTML report rendering, group breaks, subtotals, grand totals
@@ -115,6 +115,10 @@ tests/
   ColumnMeta.test.ts    NUM(p,s) parsing, declared types in LIST STRUCTURE, grid-open columnTypes, server-side grid-edit validation
   ColumnMetaStore.test.ts  Per-(db,table,column) type metadata + legacy-schema migration
   CellValidation.test.ts   Shared per-type cell validation rules
+  CreateTableParse.test.ts Strict CREATE TABLE grammar — malformed column lists must throw
+  DemoSchemas.test.ts   Golden column lists for every table the demos create
+  GridMessages.test.ts  grid-edit / grid-delete / grid-new-row / grid-refresh + INPUT form round-trip
+  IndexStoreMigration.test.ts  Adopting pre-#50 unscoped index rows into their owning database
   Print.test.ts         `?` / `??` print command
   Aggregate.test.ts     `SUM` / `AVERAGE`
   Builtins.test.ts / BuiltinsParse.test.ts   built-in functions (direct + through the parser)
@@ -315,6 +319,8 @@ line.
   qualifier validated on write; declared types tracked in `server/ColumnMetaStore.ts` (#43) ✅
 - ~~`WEEK()` built-in~~ — ISO-8601 week number (#44) ✅
 - ~~BROWSE per-cell validation~~ — grid rejects invalid edits per column type, validated on both client and server via `src/shared/cellValidation.ts` (#45) ✅
+- ~~Test hardening~~ — strict `CREATE TABLE` grammar, golden demo schemas, coverage for every
+  grid WS message, per-database index/column metadata scoping, `npm run coverage` (#50) ✅
 - `demos/overtime.prg` — overtime tracker showcasing all three of the above (#46)
 
 ## Boolean literals
@@ -324,11 +330,37 @@ Both styles accepted: `TRUE`/`FALSE` and `.T.`/`.TRUE.`/`.F.`/`.FALSE.` (dBASE I
 ## Testing
 
 ```bash
-npm test                # Vitest unit + integration (316 tests)
+npm test                # Vitest unit + integration (358 tests)
+npm run coverage        # Vitest + v8 coverage report (reporting only, no thresholds)
 npx playwright test     # E2E browser tests — requires dev server on :5173/:3000
 ```
 
-Playwright suites (79 tests): `tests/assistant.spec.ts` (22 tests — sidebar, wizards, report designer, MODIFY STRUCTURE round-trip, `TIME(15)` column + REPLACE validation, Browse-action grid validation, program run, CSV/SORT/SUM-AVERAGE/REINDEX/PACK actions, demo launchers), `tests/integration.spec.ts` (20 tests — full REPL scenario), `tests/inventory.spec.ts` (8 tests — INVENTORY.prg menu + valuation/low-stock report/sort/CSV/JOIN), `tests/crm.spec.ts` (6 tests — CRM demo menu, pipeline summary, sort, report, CSV, JOIN), `tests/parity-commands.spec.ts` (5 tests — `?`/`??`, built-in functions, `WEEK()`, `SUM`/`AVERAGE`, `SORT ON … TO`), `tests/multiarea.spec.ts` (4 tests — multi-work-area, relations, alias.field), `tests/demos.spec.ts` (4 tests — demo program + report seeding), `tests/grid-validation.spec.ts` (3 tests — BROWSE per-cell validation: TIME(15), NUM(p,s)/DATE, Esc abandons), `tests/copycsv.spec.ts` (2 tests — COPY TO download + APPEND FROM upload), `tests/splash.spec.ts` (2 tests — version banner + demo discoverability), `tests/join.spec.ts` (1 test — JOIN materialization), `tests/propagation.spec.ts` (1 test — live multiuser refresh), `tests/program-side-effects.spec.ts` (1 test — CSV/report side-effects fire from inside a program block).
+Playwright suites (83 tests): `tests/assistant.spec.ts` (23 tests — sidebar, wizards, report designer, MODIFY STRUCTURE round-trip, `TIME(15)` column + REPLACE validation, `NUM(p,s)` wizard, Browse-action grid validation, program run, CSV/SORT/SUM-AVERAGE/REINDEX/PACK actions, demo launchers), `tests/integration.spec.ts` (20 tests — full REPL scenario), `tests/inventory.spec.ts` (8 tests — INVENTORY.prg menu + valuation/low-stock report/sort/CSV/JOIN), `tests/crm.spec.ts` (6 tests — CRM demo menu, pipeline summary, sort, report, CSV, JOIN), `tests/parity-commands.spec.ts` (5 tests — `?`/`??`, built-in functions, `WEEK()`, `SUM`/`AVERAGE`, `SORT ON … TO`), `tests/multiarea.spec.ts` (4 tests — multi-work-area, relations, alias.field), `tests/demos.spec.ts` (4 tests — demo program + report seeding), `tests/grid-validation.spec.ts` (3 tests — BROWSE per-cell validation: TIME(15), NUM(p,s)/DATE, Esc abandons), `tests/schema-errors.spec.ts` (3 tests — malformed CREATE TABLE errors, NUM(p,s) column count, bare INPUT stores its value), `tests/copycsv.spec.ts` (2 tests — COPY TO download + APPEND FROM upload), `tests/splash.spec.ts` (2 tests — version banner + demo discoverability), `tests/join.spec.ts` (1 test — JOIN materialization), `tests/propagation.spec.ts` (1 test — live multiuser refresh), `tests/program-side-effects.spec.ts` (1 test — CSV/report side-effects fire from inside a program block).
+
+## Test discipline
+
+Two bugs shipped through a 283-test suite (found in #45/#50). Both were structural blind
+spots, not bad luck. When adding tests, remember what the existing ones cannot see:
+
+- **`toContain` can only prove presence, never absence.** Almost every assertion in this
+  repo greps rendered text for a substring, so a *phantom extra column* (`NUM(8,2)` used to
+  create a column literally named `2`) sailed through every `LIST`/`LIST STRUCTURE` check.
+  Assert **exact** structure — column lists, record counts — with `toEqual`/`toHaveLength`
+  wherever you can. `tests/DemoSchemas.test.ts` pins the demo tables for exactly this reason.
+- **Test the surface, not the happy path through it.** Four of twelve `ClientMessage` types
+  had zero tests; `grid-edit` wrote straight to SQLite with no validation and nobody noticed,
+  because the grid tests only opened the grid and pressed Escape. Every WS message type
+  should have a test that drives it and asserts the database/UI effect
+  (`tests/GridMessages.test.ts`).
+- **Green CI does not mean correct.** The cross-database `ColumnMetaStore` leak shipped with
+  seven passing tests, because they all used a single database. When state is keyed by name,
+  write the test that uses two.
+- **Prefer failing loudly to guessing.** The parser used to absorb any token it didn't
+  understand and invent a column from it. `CREATE TABLE` is now strict; keep it that way.
+
+Run `npm run coverage` when touching an area you suspect is untested. **Never run `npm test`
+and `npx playwright test` concurrently** — both mutate `data/` and `data/system.sqlite3`, and
+a state-dependent e2e test will fail for reasons that have nothing to do with your change.
 
 ## Definition of done
 

README.md

@@ -229,6 +229,9 @@ WebBase-III supports **unlimited work areas** — each independently holding a t
 
 > Column ops that can invalidate an index (DROP, RENAME, ALTER type) drop all of the table's indexes and warn you to rebuild with `INDEX ON`.
 
+> `CREATE TABLE` rejects a malformed column list (missing comma, missing type, unclosed paren, a third
+> type argument) with a parse error naming the offending column, and creates nothing.
+
 **Column types**: `CHAR(n)` (aliases `CHARACTER`/`VARCHAR`/`STRING`/`MEMO`), `NUM`/`NUM(p,s)` (`NUMERIC`/`FLOAT`/`DOUBLE`/`DECIMAL`), `INT`/`INTEGER`, `LOGICAL`/`BOOLEAN`, `DATE`, and `TIME`/`TIME(n)`. `TIME` stores `HH:MM` (24-hour); the optional `TIME(n)` qualifier (e.g. `TIME(15)`) requires minutes to be a multiple of `n`. `REPLACE ... WITH` rejects a malformed or off-granularity `TIME` value instead of silently coercing it, and `LIST STRUCTURE` prints the declared type (`NUM(8,2)`, `TIME(15)`) rather than SQLite's storage class.
 
 > **CSV format (`COPY TO` / `APPEND FROM`):** Unlike dBASE III's headerless,