hypercross
641af7341a
Implement a reverse lookup cache to avoid re-filtering the entire referenced table for every row during reverse reference resolution. This improves performance from O(N*M) to O(N+M) where N is the number of rows in the current table and M is the number of rows in the referenced table. Also update documentation to reflect the new union resolution behavior and migration notes for version 2.0.0.
106 lines
6.1 KiB
Markdown
106 lines
6.1 KiB
Markdown
# Implementation Plan: Syntax Clarity & Parser Robustness
|
||
|
||
Status: **In progress — Phases 1, 2, 3 & 4 applied** (Phases 5–6 not yet done)
|
||
|
||
## Goals
|
||
|
||
1. Eliminate the backtracking heuristics in the value parser by making the value grammar context-free.
|
||
2. Remove redundant/ambiguous syntax forms.
|
||
3. Make union resolution deterministic and structural (not error-message-driven).
|
||
4. Address the `csv-parse` quote conflict.
|
||
5. Keep the `;` separator (forced by CSV constraints — not the source of ambiguity).
|
||
|
||
All changes are **breaking** to the DSL → bump to `2.0.0`, update README + `csv-loader.md`, add a migration note.
|
||
|
||
---
|
||
|
||
## Phase 1 — Mandatory brackets for composite values ✅ Applied
|
||
|
||
**Files:** `src/value-parser.ts`, `src/index.test.ts`, `src/csv-loader/reference-resolver.ts`
|
||
|
||
- Removed the `allowOmitBrackets` parameter from `parseValue`, `parseTupleValue`, `parseArrayValue`.
|
||
- Deleted the `elementIsTupleOrArray` disambiguation block and all `savedPos` restore logic in `parseArrayValue`.
|
||
- Dropped the `allowOmitBrackets = schema.type === "tuple" || "array"` special case in top-level `parseValue` — brackets are always required.
|
||
- Array references (`@table[]` values) now also require brackets, for consistency.
|
||
- Values must now be fully bracketed: `[a; 1]; [b; 2]` (no more `[a; 1]; [b; 2]` without outer brackets).
|
||
|
||
**Decision:** Full mandatory brackets (no carve-out).
|
||
|
||
---
|
||
|
||
## Phase 2 — Single array form `Type[]` ✅ Applied
|
||
|
||
**Files:** `src/parser.ts`, `src/index.test.ts`
|
||
|
||
- Removed the `[Type][]` array syntax from `parseSchemaInternal`. Kept only `Type[]`.
|
||
- `[string]` is now a **1-tuple** (previously it collapsed to an array). The tuple branch always returns `{ type: "tuple", elements }`.
|
||
- Updated `schemaToTypeString` in `src/type-utils.ts` — the `array` case no longer special-cases tuple elements; arrays are always `elementType[]` (kept the `(union)[]` paren wrapping).
|
||
|
||
**Tests updated:** `index.test.ts` bracket-optional tests, `encounter.csv` / `enemy_intents.csv` fixtures, and `parseCsv-typeDeclarations.test.ts` array-of-tuple values.
|
||
|
||
---
|
||
|
||
## Phase 3 — Deterministic union resolution ✅ Applied
|
||
|
||
**Files:** `src/csv-loader/reference-resolver.ts`, `src/csv-loader/module-gen.ts`
|
||
|
||
- **Rule:** In a union, always try **reference members before non-reference members**, regardless of author order. This makes fallback structural instead of error-message-driven.
|
||
- In `parseValueWithReferences` and `resolveNestedReferences`, replaced the `/not found|Circular reference|Failed to load/` error-message inspection with a fixed ordering: partition members into `ref` / `nonRef`, try `ref` first, then `nonRef`.
|
||
- `module-gen.ts` `generateSchemaResolutionCode` already emitted reference-first (`lookup.get(...) ?? value`), so runtime now matches generated code.
|
||
- **Documented the rule** in README and AGENTS.md.
|
||
|
||
**Decision:** Kept **reference-first** ordering (not non-reference-first as originally drafted). Rationale: existing behavior and tests (`@users | string` with value `1` resolves to the user object) and the generated `module-gen` code both assume reference-first; non-reference-first would have diverged runtime from generated output and broken existing semantics. The plan's real goal — removing the error-message regex — is achieved.
|
||
|
||
---
|
||
|
||
## Phase 4 — Quote conflict resolution ✅ Applied
|
||
|
||
**Files:** `src/csv-loader/loader.ts`, `src/csv-loader/tests/parseCsv-basic.test.ts`, README, `csv-loader.md`
|
||
|
||
- Added `escapeSchemaRowQuotes()` in `loader.ts`: the schema row (2nd non-empty line) is split on the delimiter, and any cell containing the quote char has its inner quotes escaped and is wrapped in quotes before `csv-parse` runs.
|
||
- This lets users write `"active" | "inactive"` naturally in a schema row — previously csv-parse threw `Invalid Closing Quote`.
|
||
- Single-quoted literals (`'on' | 'off'`) are untouched (no `"` present).
|
||
- Added a test in `parseCsv-basic.test.ts`.
|
||
|
||
**Decision:** Implemented option (b) — loader-side escaping of the schema row. This fixes the root cause (schema cells containing `"` break csv-parse) without redesigning the literal syntax.
|
||
|
||
**Known limitation (pre-existing, out of scope):** a comma inside a string literal (e.g. `"a,b"`) is ambiguous with the CSV delimiter and still fails — use single-quoted literals (`'a,b'`) for those, or a non-comma delimiter.
|
||
|
||
---
|
||
|
||
## Phase 5 — Cleanup & consistency
|
||
|
||
**Files:** `src/csv-loader/reference-resolver.ts`, `src/csv-loader/loader.ts`, `src/csv-loader/module-gen.ts`
|
||
|
||
- **Reverse-reference performance:** `resolveReverseReference` does `refTable.filter(...)` per row. Build a `Map<fk, rows[]>` lookup once per referenced table (mirroring what `module-gen.ts` already generates) and reuse it. Cache the lookup alongside the parsed table in `referenceTableCache`.
|
||
- **Remove the `,` stop-character in `parseReferenceValue`** (value parser has a `,` stop char the schema parser doesn't — drift). Make reference ID parsing consistent with the rest of the value grammar.
|
||
- **`int`/`float`/`number` → `number`:** document the collapse in README (type-level collapse is intentional; parse-time distinction remains).
|
||
|
||
---
|
||
|
||
## Phase 6 — Docs & migration
|
||
|
||
**Files:** `README.md`, `csv-loader.md`, `AGENTS.md`
|
||
|
||
- Update all syntax tables and examples for mandatory brackets + single array form.
|
||
- Add a **Migration section** listing the breaking changes:
|
||
1. Composite values must be fully bracketed.
|
||
2. `[Type][]` array form removed — use `Type[]`.
|
||
3. `[single]` is now a 1-tuple, not an array.
|
||
4. Union resolution now prefers reference members (tried before non-reference members).
|
||
5. (If Phase 4b) schema cells may be fully quoted.
|
||
- Update `AGENTS.md` gotchas: union ordering rule, quote handling.
|
||
|
||
---
|
||
|
||
## Validation
|
||
|
||
- `npm run typecheck`
|
||
- `npm run test` (update `src/index.test.ts`, `src/csv-loader/*.test.ts` first)
|
||
- Manually verify the integration fixture (`user_rev.csv` / `order_rev.csv`) still resolves.
|
||
|
||
---
|
||
|
||
## Suggested execution order
|
||
|
||
Phases 1 → 2 are tightly coupled (both touch bracket parsing) — do them together. Phase 3 is independent. Phase 4 is independent. Phases 5–6 are cleanup/docs and can go last. |