feat(applications): visual schema.graphql editor#1419
Conversation
Add a visual, table-oriented editor for Harper `schema.graphql` files, opening by default with an "Edit as text" escape hatch to Monaco — the same preview/source toggle pattern as the SVG editor. This replaces the hidden File → New Table modal, which only ever appended a table. - Pure, tested parse/serialize library (`lib/schema/`) that round-trips a schema losslessly: unedited tables and any content the GUI doesn't model (comments, scalars/enums, non-@table types, unknown directives/args) are preserved verbatim; only edited/new tables are regenerated canonically. Output mimics the file's indentation (tabs vs spaces) and orders tables alphabetically. - Full directive coverage (`@table`/`@export`/`@sealed` and every field directive incl. vector `@indexed` params), driven by the existing harper-language metadata (reconciled to add `randomAccessFields`). - Collapsible table cards (existing tables default collapsed); add/edit/remove tables and fields; GraphQL-name validation highlights invalid names. - Unnamed, in-progress tables are not written to the file until named. - Fold the New-Table modal into the editor: the File-menu item now switches to the visual editor and starts a new table (`AddSchemaTable`). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a visual-first schema editor (SchemaEditorView) for Harper schema.graphql files, replacing the previous modal-based table creation form. It includes parsing and serialization logic to seamlessly toggle between the visual editor and a raw text editor while preserving comments, formatting, and unmodeled schema definitions. Feedback on the changes highlights a few key improvements: using stable keys instead of array indices for the dynamic list of fields to prevent UI state leakage, only sorting tables alphabetically during serialization when edits are present to avoid marking files as dirty immediately upon opening, and defensively skipping empty-named fields during serialization to prevent invalid GraphQL syntax.
- Stable React keys for the field list: add FieldModel.key (populated on parse and on createField/createTable) so a field's transient UI state (its "Advanced" disclosure) follows the field across add/remove/reorder instead of shifting by array index. - Only reorder tables alphabetically once something is edited. Sorting on open would reorder verbatim blocks and mark an unsorted file dirty the moment it's viewed; deferring keeps "just looking" a no-op while still normalizing order in generated output. - Skip an added-but-unnamed field during serialization (it would emit invalid `\t: String`), mirroring the existing unnamed-table handling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kriszyp
left a comment
There was a problem hiding this comment.
Reviewed via Claude review-queue. Genuinely well-architected feature — the parse → segment model → serialize approach that keeps everything unmodeled as byte-for-byte raw text is the right design, and the visual/text stale-overwrite race I was watching for is correctly handled (bufferRef captures live edits before the reset effect runs).
Two real data-fidelity bugs in GraphQL description ("""...""") handling, both empirically confirmed by running the actual PR-head parse/serialize round trip, and neither covered by the test suite:
Blocker: multi-line description gets corrupted with duplicate indentation the first time its table is edited. parseFields captures a field's """...""" description with its original embedded indentation, then generateTable blindly re-prepends the indent to every line on regeneration. Confirmed: editing any field on a table with a multi-line description doubles the indent on every interior line, and doing it twice doubles it again.
Blocker: a table's leading """...""" description doesn't travel with it on alphabetical sort. splitAttachedLeading only recognizes # comment lines as "attached" trivia that moves with a table when tables get re-sorted — a preceding docstring isn't matched, so it stays anchored at the old position and ends up describing whatever table sorts into that slot instead. Confirmed with a two-table repro.
Since lossless round-tripping is the PR's own stated #1 ask, I'd like these fixed (plus regression tests covering multi-line descriptions at both table and field level, through an edit + sort) before merging. The pre-existing silent save-error-surfacing gap is worth a follow-up ticket but not a blocker for this PR.
Two data-fidelity bugs in `"""…"""` handling, both surfacing only on edit and now covered by regression tests: - Multi-line field descriptions no longer compound their indentation when the table is edited. `generateTable` re-prepended the canonical indent to lines that already carried their own, doubling it each edit; it now strips existing leading whitespace per line first (blank lines stay blank), making it idempotent. - A table's leading `"""…"""` description now travels with it when tables are sorted. `splitAttachedLeading` only treated `#` comment lines as attached trivia, so a preceding docstring stayed anchored and re-labeled whatever table sorted into its slot; it now recognizes `"""…"""` blocks too. Adds round-trip coverage for single- and multi-line descriptions at table and field level, through an edit and an alphabetical sort. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the thorough round-trip testing — both description bugs are fixed in a0a36de, with regression tests through edit + sort:
New tests: verbatim round-trip of single-/multi-line descriptions at table and field level ( Filing the pre-existing silent save-error-surfacing gap as a separate follow-up as suggested. 🤖 Addressed by Claude Code |
kriszyp
left a comment
There was a problem hiding this comment.
Reviewed via Claude review-queue — both prior blockers (double-indent on multi-line descriptions, docstring not traveling with table on resort) fixed and test-verified (36/36 vitest incl. 2 new regression tests). No new bugs. LGTM.
Closes #1417.
A real GUI for editing Harper
schema.graphqlfiles. Opening a.graphqlfile now shows a visual, table-oriented editor by default, with an "Edit as text" escape hatch to Monaco — the same preview/source toggle pattern as the SVG editor. This subsumes the hidden File → New Table modal (which only ever appended a table).What it does
.graphqlfiles open in the table editor; toggle to raw text anytime.harper-languagemetadata:@table(incl.database,expiration,audit,eviction,scanInterval,replicate,randomAccessFields),@export(name/rest/mqtt),@sealed, and every field directive —@primaryKey,@indexed(incl. vector/HNSW params),@computed,@relationship,@allow, timestamps,@expiresAt,@enumerable. (Reconciled the metadata mirror to add the missingrandomAccessFields.)name · N fields · REST); newly-added tables open expanded.Safety / fidelity (issue's #1 ask)
The core is a small, dependency-free, well-tested parse/serialize library (
lib/schema/) that round-trips a schema losslessly:@tabletypes, unknown directives/args — are preserved verbatim. Only edited/new tables are regenerated canonically.type @tableorphan; doesn't even mark the file dirty).Stability (issue's #3 ask)
Generated output mimics the file's indentation (tabs vs N spaces) and orders tables alphabetically; regeneration is idempotent.
Testing
lib/schema/unit tests: round-trip fidelity, unknown-directive/arg + comment preservation, tabs-vs-spaces, alphabetical ordering, list/non-null, the mutation→serialize pipeline, skip-unnamed-table, and GraphQL-name validation.TableCardcomponent tests (jsdom): render/edit/toggle/collapse/read-only.🤖 Generated with Claude Code