refactor: relocate example generators - #239
Merged
Merged
Conversation
Add role-aware workbench previews and strict one-row option bundles, then run Filter sources ahead of Dashboard panels with validation, reconciliation, cancellation, and retry. Co-Authored-By: OpenAI Codex <noreply@openai.com> Claude-Session: Codex
…owCardinality parsing, object-tuple Table rendering Follow-up fixes from a UI/UX + correctness review of the Dashboard Filter sources feature before merge: - filter-option-field.js: the clear (×) button reported the stale pre-clear value to onValueChange/onCommit instead of '' (app.state.varValues kept the old selection after "clearing"), and had no aria-label naming the field it clears — the same bug the e2e suite's "Clear origin" assertions were actually catching (root cause of the 3-browser e2e failure on this PR). Clear now uses the shared Icon.close() glyph. - styles.css: `.filter-option-clear` had no position/size rule so it rendered in normal flow below the input instead of overlaid inside it; the workbench Filter preview's type/diagnostic text referenced undefined `--muted`/ `--danger` CSS variables (now `--fg-mute`/`--error-fg`/`--warn-fg`); the Dashboard's `.dash-config-diagnostic` role/Filter banners had zero styling and rendered as unframed body text; the tab-strip/Library "Filter" role badge shared the bordered `.qtab` row with no styling of its own, reading as a second open tab. - clickhouse-type.js: Enum8/Enum16 columns failed to parse at all (the quoted `'member' = n` argument list hit the non-Tuple type-arg branch, which rejects quoted tokens) despite isSupportedOptionScalar advertising them as supported; LowCardinality(...) was never unwrapped alongside Nullable. - grid-render.js: a named tuple/map serialized as a plain object (KPI and Filter's owned execution profile both request output_format_json_named_tuples_as_objects) rendered as "[object Object]" in the Table view; now JSON.stringify's, matching the existing logs.js precedent for the same object-vs-String(v) problem. - filter-bar.js: a curated field never got the shared is-invalid/conflict affordance (var-field.js) a plain filter field gets for the same prepared- batch state. - tabs.js/saved-history.js: extracted the duplicated "Filter" role badge button into one shared `filterRoleBadge` helper. npm test: 2616/2616 passing, coverage gate holds on every touched file. Verified live on otel (kubectl cp + sha256sum match): clear button icon/ position/aria-label, value actually resets, styled diagnostic banner, "Filter" badge no longer reads as a tab, Table/JSON views show real data instead of [object Object]. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQbDSVDUTUk1bA8891cX9y
CI caught a real regression my previous fix introduced: a genuine mousedown on the clear button blurs the still-focused input FIRST, and the blur handler's strictCommit() re-commits whatever text is still showing (it still matches the just-selected option at that point) before the button's own click handler runs — the clear ends up firing two commits instead of one. Same fix combobox.js already applies to an option's own mousedown-commit (#174 §1, "commit BEFORE blur"): preventDefault on the clear button's mousedown stops the input from blurring at all, so only the click handler's single commit fires. Added a unit test asserting exactly one onCommit/ onValueChange call for the clear interaction — the prior test only did a programmatic .click(), which doesn't reproduce the browser's real mousedown-then-blur ordering. npm test: 2617/2617 passing. Verified live on otel: real pointer click (mousedown+mouseup+click) on Clear now cleanly resets the field. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQbDSVDUTUk1bA8891cX9y
Found while building a demo dashboard exercising every curated field shape:
buildFilterBar's curated branch built its wrapping <label> with a fixed
'var-field is-curated' class, never appending 'is-optional' the way the
plain-field branch two lines below does — so every curated field showed the
required-field asterisk (.var-field:not(.is-optional) .var-name::after)
even when its underlying param was genuinely optional (wrapped in a
/*[ ... ]*/ block in every consuming Panel). The field's title tooltip
already said "optional: blank leaves its filter block out" — only the visual
marker and CSS class were wrong.
Added a test asserting a required and an optional curated field render with
the correct is-optional class, mirroring the existing plain-field coverage.
npm test: 2618/2618 passing, filter-bar.js coverage gate holds.
Verified live on otel: an optional curated field ({user}/{kind}/{errcode} in
the query-log-explorer demo library) no longer shows the required asterisk.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQbDSVDUTUk1bA8891cX9y
validate-my-library.mjs was a throwaway script used to check a demo Library JSON against parseImportDoc + defaultSpecValidationService while building the query-log-explorer example dashboard — not meant for this PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQbDSVDUTUk1bA8891cX9y
…urces demo A complete Library example against system.query_log (works on any cluster, no synthetic dataset needed) exercising every Filter variant this PR adds: - three Filter-role sources, one per curated option shape — Filter · Users (Array(Tuple(value, label))), Filter · Query kind (Map(String, String)), Filter · Exception codes (plain Array(T)) - plain auto-detected fields alongside them (hours/minDurationMs/namePattern), all optional, several shared across multiple Panels - a KPI panel, four analytical Panels adapted from the Altinity KB's "Handy queries for system.query_log" (slowest queries, most-selected columns, most-used functions, errors over time), a Logs panel, and a Text panel explaining the demo — matching the requested KPI-top/text-after-KPI/ logs-bottom layout Validated against the real parseImportDoc + defaultSpecValidationService (the same path the app's own Append/Open uses) and picked up automatically by the existing tests/unit/spec-examples.test.js example-discovery test. Verified end-to-end live on the otel demo cluster: imported via File ▾ → Append, all three curated dropdowns populate with real data, the KPI/bar/ area/table/log panels render real query_log activity, and selecting a curated exception code live-narrows the "Errors over time" chart. README gets one paragraph linking the example from the existing "Dashboard Filter sources" section. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQbDSVDUTUk1bA8891cX9y
/#236/#237 A second pre-merge cleanup pass on the Dashboard Filter sources work (#232), removing invented primitives / duplication and fixing bugs found alongside them. - Rewrite filter-option-field.js to reuse the shared var-combo combobox (combobox.js createCombobox/wireComboInput, .var-combo/.var-input/ .var-combo-list) in BOTH the Dashboard filter bar and the drawer Filter preview. It previously hand-rolled its own listbox with CSS classes that do not exist, so the curated dropdown rendered as an unstyled inline bulleted list and pushed the clear (×) button out of place. - #236: extract core/diagnostics.js as the one shared diagnostic factory; dashboard-filters/filter-options/filter-execution + dashboard.js use it. - #237: Filter sources reuse the tile wave's generation/abort guard (supersedeSlot / slot.gen) instead of a parallel re-implementation. - #234: seed curated fields from a persisted asb:filterCurated bundle so they paint as the searchable dropdown immediately (no plain-text flash). - Fix a result-picker regression: a table-typed panel mapped to a panel:table value that matched no option (blank select); table/unknown now map to (auto). - Limit per-keystroke full Spec re-evaluation to Filter-role tabs. #235 (Filter-wave/Panel-wave serialization) deferred — needs a new declared-helper-names contract. #238 filed for the coexisting ClickHouse type parsers (param-type.js vs clickhouse-type.js LowCardinality divergence). Closes #234 Closes #236 Closes #237 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQbDSVDUTUk1bA8891cX9y
…xplorer filters - Add a first-class "Filter" view tab to the workbench results drawer for a Filter-role query (alongside Table/JSON), rendering the option-bundle preview as it appears on the Dashboard. Previously the preview was only reachable by re-selecting "Filter" in the role <select>, which fires no change event when it is already the selected role — so the view never switched and the drawer kept showing the raw table/JSON (reported bug). - examples/query-log-explorer.json: replace the `hours` lookback with a real DateTime range — `from` (required) + `to` (optional) on event_time — across every system.query_log panel; replace `namePattern` with a universal optional `search` that substring-matches the query text on all panels (and the exception message in the log panel). from/to render as relative-time fields (relative expressions resolve to epoch for binding). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQbDSVDUTUk1bA8891cX9y
…ilter demo source
Revises two UX decisions from the prior round and simplifies the demo Library.
- Remove the separate "Filter" view tab from the results drawer. Instead the
result-presentation <select> now shows a "Preview…" placeholder while on
Table/JSON, so picking ANY entry (chart / Logs / KPI / Text / Filter role) —
including the query's current one — is a real change event that switches to
that preview. (A native <select> fires no change when the current option is
re-picked, which is why re-selecting "Filter" did nothing before.)
- Filter drawer preview is now a result-grid consistent with Table/JSON:
columns name · options · type · example, with the interactive per-helper
combobox living in the example cell (× suppressed via the field's preview
flag). Reuses the .res-table grid CSS; buildFilterOptionField unchanged.
- examples/query-log-explorer.json: consolidate the three per-shape Filter
favorites into a single "Filter" source returning user + query_kind; remove
the exception-code Filter and the "Errors over time" panel; rename the
columns panel's {kind:String} → {query_kind:String} to match the new helper.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQbDSVDUTUk1bA8891cX9y
… shapes in tests The single Filter source now returns `user` as a named value/label bundle (Array(Tuple(value, label)) — the #160-supported shape): value is the full user that binds to a Panel's {user:String}, label is the name before '@' for display. An unnamed Array(Tuple(String, String)) is (correctly) rejected as an unsupported element type — that was the reported error, not a bug. - examples/query-log-explorer.json: `user` filter → arraySort(x -> x.label, groupUniqArray((user AS value, splitByChar('@', user)[1] AS label))). - tests/unit/filter-options.test.js: explicit cases for the two #160 shapes (named value/label tuple array, and Map), incl. the example's user bundle. - tests/unit/spec-examples.test.js: guard that every Filter-role example query passes the Filter SQL contract (single row-returning statement, no params/FORMAT). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQbDSVDUTUk1bA8891cX9y
`WHERE user != ''` → `WHERE query_log.user != ''` in the query-log-explorer Filter source (avoids ambiguity with the `user AS value` tuple aliasing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQbDSVDUTUk1bA8891cX9y
The workbench Filter preview is now a result-grid (# · name · options · type · example) rather than the old .filter-preview-helper/.filter-preview-type sections, so the first e2e assertion targeted removed selectors. Assert the grid header + cells and that the example-cell combobox has no clear × (the other two e2e tests exercise the dashboard field and were unaffected). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQbDSVDUTUk1bA8891cX9y
…-sources-160 # Conflicts: # examples/query-log-explorer.json
Co-Authored-By: Claude Code <noreply@openai.com> Claude-Session: codex
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
1 participant
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.
What & why
.mjsfiles intoexamples/mjs/and add a local index README for that subdir.examples/directory instead of the repo root.Checklist
npm testpasses (the per-file coverage gate is non-negotiable)npm run buildsucceeds (single-filedist/sql.html)src/core/, network insrc/net/(injected fetch), DOM insrc/ui/CHANGELOG.md([Unreleased]) updated if behavior or the deployed surface changed