mirror of
https://github.com/olehomelchenko/astrolabe.git
synced 2026-08-08 02:02:33 +00:00
Editor: Vega-Lite expression intelligence; single-home render errors
This commit is contained in:
@@ -236,6 +236,13 @@ feature owns — which modal is open, the runtime theme, transient render flags.
|
||||
> that's the signal to extract a feature store. A bloated central store is the
|
||||
> thing this split exists to prevent.
|
||||
|
||||
> Rule: a store earns its place by **decoupling** producers from consumers — a fact
|
||||
> belongs in one when more than one component reads it, or when many sites produce it for
|
||||
> one surface to consume (the imperative `notify()` / `confirm()` overlay stores). When a
|
||||
> single component is both the only producer and the only consumer, the fact is that
|
||||
> component's **local `useState`**, not a store — a store there decouples nothing, and is
|
||||
> the shape to fold back.
|
||||
|
||||
---
|
||||
|
||||
## 4. Actions: Mutations Live in the Store, Not Components
|
||||
|
||||
@@ -538,11 +538,12 @@ blank mid-edit.
|
||||
### A second preview surface: the Chart Builder
|
||||
|
||||
The editor's `LivePreview` is **bound to the snippet editor** — it reads `SnippetStore`
|
||||
(shown spec), `AppStore` (fit mode/theme), and `PreviewStore` (shared error). The
|
||||
**Chart Builder modal** needs a preview of a _different_ spec source (its config), so it
|
||||
does **not** reuse `LivePreview`; it runs its own small debounced render over the same
|
||||
`chart-renderer.renderSpec` + `prepareSpecForRender`, with **local** error state (never the
|
||||
shared `PreviewStore`, which would cross-talk with the editor). Two preview surfaces, one
|
||||
(shown spec) and `AppStore` (fit mode/theme), and keeps its render status (`error`/`busy`) in
|
||||
its own local state. The **Chart Builder modal** needs a preview of a _different_ spec source
|
||||
(its config), so it does **not** reuse `LivePreview`; it runs its own small debounced render
|
||||
over the same `chart-renderer.renderSpec` + `prepareSpecForRender`, with its own **local**
|
||||
error state. Each preview surface owns its render status locally — no shared store to
|
||||
cross-talk. Two preview surfaces, one
|
||||
renderer service. Builder flow: `chart-builder.ts` (pure spec assembler) → `ChartBuilderStore`
|
||||
(config + create) → `ChartBuilderModal`'s `BuilderPreview`. Reach for a reusable preview
|
||||
component only if a _third_ surface appears.
|
||||
@@ -603,13 +604,13 @@ the existing view is re-measured via a `ResizeObserver`-driven event — see §8
|
||||
|
||||
A spec that cannot be rendered must produce a **readable** message in the preview
|
||||
area and recover on its own once the spec is valid again. Errors arise at three
|
||||
stages, all funneled to one error field the preview reads:
|
||||
stages, all funneled to the one error state the preview owns:
|
||||
|
||||
| Stage | Failure | Surfaced as |
|
||||
| -------------------------------- | -------------------------------------- | ---------------------- |
|
||||
| Parse | Invalid JSON | "Invalid JSON: …" |
|
||||
| Prepare (`prepareSpecForRender`) | Referenced dataset missing/unfetchable | "Dataset not found: …" |
|
||||
| Embed (`vega-embed`) | Vega-Lite compile / data error | "Rendering error: …" |
|
||||
| Stage | Failure | Surfaced as |
|
||||
| -------------------------------- | -------------------------------------- | --------------------------------- |
|
||||
| Parse | Invalid JSON | `Invalid JSON · …` |
|
||||
| Prepare (`prepareSpecForRender`) | Referenced dataset missing/unfetchable | `Dataset "x" not found · …` |
|
||||
| Embed (`vega-embed`) | Vega-Lite compile, or a bad expression | `Line N · …` / `Render error · …` |
|
||||
|
||||
```ts
|
||||
// inside render(), driven by the debounced renderer
|
||||
@@ -620,7 +621,7 @@ async function render(): Promise<void> {
|
||||
if (!text) {
|
||||
current?.destroy();
|
||||
current = null;
|
||||
usePreviewStore.getState().setError(null);
|
||||
setError(null); // `error`/`busy` are the pane's local state, not a store
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -628,7 +629,7 @@ async function render(): Promise<void> {
|
||||
try {
|
||||
parsed = JSON.parse(text);
|
||||
} catch (e) {
|
||||
usePreviewStore.getState().setError(`Invalid JSON: ${(e as Error).message}`);
|
||||
setError(`Invalid JSON · ${(e as Error).message}`);
|
||||
return; // keep the last good chart underneath the error, or show the message
|
||||
}
|
||||
|
||||
@@ -638,14 +639,9 @@ async function render(): Promise<void> {
|
||||
const config = chartConfigFor(uiTheme);
|
||||
current?.destroy();
|
||||
current = await renderSpec(node, prepared, config);
|
||||
usePreviewStore.getState().setError(null); // success clears any prior error
|
||||
setError(null); // success clears any prior error
|
||||
} catch (e) {
|
||||
usePreviewStore
|
||||
.getState()
|
||||
.setError(
|
||||
`Rendering error: ${(e as Error).message}. ` +
|
||||
`Check your JSON syntax and that the spec is valid Vega-Lite.`,
|
||||
);
|
||||
setError(`Render error · ${(e as Error).message}`);
|
||||
}
|
||||
}
|
||||
```
|
||||
@@ -660,10 +656,10 @@ manual retry, no reload.
|
||||
- **Do** treat empty/blank spec text as "render nothing" — finalize the current
|
||||
view, clear the error, show a clean empty pane.
|
||||
- **Do** clear the error state on every successful render.
|
||||
- **Do** make messages legible and actionable (the underlying reason plus a hint
|
||||
to check JSON/Vega-Lite validity), never a raw stack trace dump.
|
||||
- **Do** distinguish the failing stage in the message (Invalid JSON vs Dataset
|
||||
not found vs Rendering error).
|
||||
- **Do** make messages legible and actionable — lead with the location or the failing
|
||||
stage, then the underlying reason — never a raw stack trace dump.
|
||||
- **Do** distinguish the failing stage in the message (invalid JSON vs missing dataset
|
||||
vs a bad expression or render error).
|
||||
- **Don't** show a broken/partial chart — replace the chart area with the
|
||||
message.
|
||||
- **Don't** require a manual "retry"; validity restores the chart on its own.
|
||||
@@ -729,5 +725,5 @@ bookkeeping. Gate the observer to responsive modes (Original needs no re-fit).
|
||||
| Field names | `escapeVegaField` on every data-derived `field:` | `src/core/rendering.ts` |
|
||||
| Debounce | Inline timer; `0` on buffer-load/view-switch, `renderDebounce` on keystroke (§5) | `LivePreview.tsx` (service not yet extracted) |
|
||||
| Spec prep | `prepareSpecForRender` (pure, on a copy) | `src/core/rendering.ts` (see _Live Preview_) |
|
||||
| Errors | One error field, cleared on success, empty = nothing | `PreviewStore.error` |
|
||||
| Errors | One error field, cleared on success, empty = nothing | `LivePreview` local state (`error`/`busy`) |
|
||||
| Container fit | Inner host + frame (out-specify `.vega-embed`); resize via synthetic `window:resize` | §8 (`LivePreview` + `chart-renderer`) |
|
||||
|
||||
@@ -277,10 +277,10 @@ defaults-spread" discipline is worth keeping.
|
||||
|
||||
## 5 · Editor augmentation (our layer over the borrowed base)
|
||||
|
||||
Beyond schema validation/completion (§1), the spec editor adds structural refactors and
|
||||
data-aware hints — the edits that are awkward in raw JSON and out of reach of the
|
||||
single-view visual builder. All transform logic is pure `src/core/`; the Monaco glue is
|
||||
thin app-layer services.
|
||||
Beyond schema validation/completion (§1), the spec editor adds structural refactors,
|
||||
data-aware hints, and expression intelligence — the edits and feedback that are awkward in
|
||||
raw JSON and out of reach of the single-view visual builder. All transform and analysis
|
||||
logic is pure `src/core/`; the Monaco glue is thin app-layer services.
|
||||
|
||||
**Core (pure, portable):**
|
||||
|
||||
@@ -321,11 +321,25 @@ thin app-layer services.
|
||||
flatten/collapse cleanup runs after. `simplifyStructure` collapses redundant single-child
|
||||
compositions recursively (a `{hconcat:[v]}` is just `v`; `facet`/`repeat` hold one child by design
|
||||
and are left alone) — the wireframe's Simplify, returning null when nothing is redundant.
|
||||
- `expr-validate` — one Vega expression, parsed with Vega's own `parseExpression` (no divergent
|
||||
grammar): `validateExpression` (valid + parser message), `referencedFields` (its `datum.<field>`
|
||||
references), and `activeCall` (the enclosing call + which argument the cursor is in, for signature
|
||||
help).
|
||||
- `spec-expressions` — the expressions embedded in a spec's JSON strings (`EXPRESSION_KEYS` =
|
||||
`calculate`/`filter`/`expr`/`test`; only string values, so object predicates are skipped).
|
||||
`expressionStringsIn` locates each (byte span + key) to drive markers; `firstExpressionError`
|
||||
names the first malformed one (key + parser message + 1-based line) so a failed render can
|
||||
attribute itself.
|
||||
- `vega-expr-catalog` — the expression language's function/constant **names derived from
|
||||
`vega-expression`'s own registry** (zero drift; a test asserts the curated set ⊆ derived), plus
|
||||
curated parameter signatures for the commonly-typed functions (what the registry can't supply).
|
||||
|
||||
**Services (app, store-aware via `getState`):** `spec-transform-actions` (the
|
||||
wrap/simplify/add-view operations and their surfaces), `spec-dataset-hints` (completion,
|
||||
hover, inlay providers), `active-dataset` (`dataInfoAt(text, offset)` — the columns/types/stats
|
||||
plus derived fields the draft sees at the cursor). `SpecEditor` does the wiring.
|
||||
wrap/simplify/add-view operations and their surfaces), `spec-dataset-hints` (data-column
|
||||
completion, hover, inlay providers), `spec-expression-hints` (expression completion,
|
||||
signature help, hover, and the diagnostic markers), `active-dataset` (`dataInfoAt(text,
|
||||
offset)` — the columns/types/stats plus derived fields the draft sees at the cursor).
|
||||
`SpecEditor` does the wiring.
|
||||
|
||||
Decision rules:
|
||||
|
||||
@@ -381,6 +395,19 @@ Decision rules:
|
||||
- **No unknown-field diagnostic.** Hints are additive and forgiving, so over- or
|
||||
under-listing costs nothing; a "field not in data" squiggle would false-positive on every
|
||||
derived or data-dependent field, so there is deliberately none.
|
||||
- **Expression intelligence is one service; its markers are per-editor.** `spec-expression-hints`
|
||||
registers the expression completion/signature-help/hover **once for `json`** (like the other
|
||||
providers), but the marker pass — validating every expression string and squiggling the invalid
|
||||
ones with `setModelMarkers` (the app's only editor markers besides the JSON worker's, under the
|
||||
`vega-expr` owner) — is **per editor**, since it writes to one model, and recomputes debounced on
|
||||
edit and on a draft↔published toggle. All expression concerns (completion, hover, markers) live
|
||||
here; `spec-dataset-hints` owns only data-column hints, so neither is a grab-bag.
|
||||
- **Completion replace-ranges come from a self-parsed partial, never `getWordUntilPosition`.**
|
||||
Monaco's JSON `wordPattern` counts `.` and `(` as word characters, so the model's "word" after
|
||||
`datum.` or `fn(` spans the whole `datum.`/`fn(` token; used as a completion item's range it both
|
||||
mis-targets the edit and filters every suggestion out (none start with `datum.`). A provider
|
||||
completing inside a string must build the replace range from the partial it parses itself — a rule
|
||||
any future in-string completion (transform/param scaffolding) inherits.
|
||||
- **Code-action menu icons are kind-derived** (a wrench for the `refactor.*` kinds) — Monaco's
|
||||
`CodeAction` carries no icon field. Custom iconography lives only where it is supported:
|
||||
CodeLens titles (`$(codicon)`), completion-item kinds, and glyph-margin decorations.
|
||||
|
||||
@@ -34,12 +34,12 @@ Astrolabe has four distinct ways to tell the user something. They are **not**
|
||||
interchangeable; picking the wrong one is the most common interaction bug. Choose by the
|
||||
nature of the message, not by convenience.
|
||||
|
||||
| Channel | Use when | Blocks? | Dismissal | Implemented by |
|
||||
| -------------------- | --------------------------------------------------------------------------------------------------- | ----------- | ------------------------------------------------------------------------- | -------------------------------------------------------- |
|
||||
| **Confirm dialog** | A **destructive or irreversible** action needs explicit consent (delete, revert, reset) | Yes — modal | User must choose; Escape/Cancel = no; backdrop click does **not** dismiss | `ConfirmStore` + `ConfirmDialog` |
|
||||
| **Toast** | A **non-blocking outcome** happened the user should know about (save failed, published, imported) | No | Auto for success/info; persists for error/warning; always a close button | `NotificationStore` + `Toaster` |
|
||||
| **Inline error** | A problem is **tied to a specific surface** and recovers in place (invalid spec → editor + preview) | No | Clears automatically when the cause is fixed | `PreviewStore`, surfaced in `SpecEditor` + `LivePreview` |
|
||||
| **Status indicator** | **Passive, ambient** state worth glancing at (draft vs. published, storage usage) | No | N/A — it just reflects state | library draft dot; storage monitor (later) |
|
||||
| Channel | Use when | Blocks? | Dismissal | Implemented by |
|
||||
| -------------------- | ------------------------------------------------------------------------------------------------------------------------- | ----------- | ------------------------------------------------------------------------- | ----------------------------------------------------- |
|
||||
| **Confirm dialog** | A **destructive or irreversible** action needs explicit consent (delete, revert, reset) | Yes — modal | User must choose; Escape/Cancel = no; backdrop click does **not** dismiss | `ConfirmStore` + `ConfirmDialog` |
|
||||
| **Toast** | A **non-blocking outcome** happened the user should know about (save failed, published, imported) | No | Auto for success/info; persists for error/warning; always a close button | `NotificationStore` + `Toaster` |
|
||||
| **Inline error** | A problem is **tied to a specific surface** and recovers in place (invalid spec → preview, cause squiggled in the editor) | No | Clears automatically when the cause is fixed | `LivePreview` local state; `vega-expr` editor markers |
|
||||
| **Status indicator** | **Passive, ambient** state worth glancing at (draft vs. published, storage usage) | No | N/A — it just reflects state | library draft dot; storage monitor (later) |
|
||||
|
||||
**Rules.**
|
||||
|
||||
@@ -80,9 +80,12 @@ nature of the message, not by convenience.
|
||||
the other publishes silently (the exact inconsistency this rule prevents). The shortcut is
|
||||
owned globally by the EventRouter (arch 04), so the helper is the only place the outcome is
|
||||
confirmed.
|
||||
- **The same failure can light up two channels.** An unrenderable spec shows the _same_
|
||||
message inline in both the editor (§03E) and the preview (§04) — one producer
|
||||
(`PreviewStore`), two subscribers. That's intentional, not duplication.
|
||||
- **A render failure has one message home: the preview.** An unrenderable spec shows its
|
||||
message in the preview pane (§04), where the chart would be — error _xor_ chart, since the
|
||||
preview's `error` state is non-null only when the render failed (success/empty clear it; export
|
||||
failures report through the export UI, never here). The editor marks the offending spot with
|
||||
an inline squiggle (§03E) rather than repeating the text. Render status is `LivePreview`'s own
|
||||
local state (`error`/`busy`) — one producer, one surface, so it needs no store.
|
||||
|
||||
## 2. Latency & feedback budgets
|
||||
|
||||
@@ -437,11 +440,13 @@ data-first door ("Build a chart from your data") beside its primary. _(Consulted
|
||||
docs/exploration/chart-builder-enhancement-scope.md §3 · 3D. This bullet is the contract; cite it, not
|
||||
the source.)_
|
||||
|
||||
**Resolved — one live region per shared message.** When the same error feeds two surfaces
|
||||
(the §1 "one producer, two subscribers" case — render errors via `PreviewStore`), exactly
|
||||
**one** subscriber is the live region (`role="alert"` on the editor, where focus is); the
|
||||
other shows the text visually with no live role. Two live regions would announce the same
|
||||
message twice.
|
||||
**Resolved — a render error lives in one place, the preview.** The render-error message
|
||||
(`LivePreview`'s local `error` state) shows only in the preview pane, where the chart would be
|
||||
(error _xor_ chart), and that single surface is the `role="alert"` live region — assertive, since the user
|
||||
just caused it. It is announced regardless of where focus sits, so it needs no duplicate near
|
||||
the editor; the editor instead pinpoints the cause with an inline squiggle. Messages share a
|
||||
terse line-led / noun-led shape (`Line 14 · Unexpected end of input`, `Dataset "x" not found ·
|
||||
…`, `Invalid JSON · …`) — the location or the problem noun first, then the parser detail.
|
||||
|
||||
**Resolved — inline _live_ validation feedback is polite, glyphed, and field-linked.** A
|
||||
validator that re-checks on **every keystroke** (the Chart Builder expression inputs — a
|
||||
|
||||
Reference in New Issue
Block a user