Phase 7 — filters & variables: shared param pipeline, optional SQL blocks, typed validation, relative time, recents, enum dropdowns (#68) - #176
Merged
Conversation
) Phase 7.0 foundation. New pure core modules, each 100%-covered: - param-scan.js: scanParamDeclarations — every {name:Type} occurrence, no dedup (the primitive conflict detection needs); detectParams becomes a first-wins compatibility wrapper over it. - param-type.js: parseParamType (Array/Nullable/args), normalizeParamType, typeLexKind, conflictingTypes. - param-serialize.js: typed serializer — scalar strings byte-identical to today; Array(T) → ClickHouse array literals with quote/backslash escaping ([] for empty); big integers stay strings end-to-end, never through a JS Number; NULL elements and nested arrays rejected with clear errors; an array value against a scalar declaration is a structural error. - param-pipeline.js: analyzeParameterizedSources (per-field declarations, per-source requiredIn/optionalIn + *Anywhere rollups, sourceErrors, type-conflict diagnostics) and prepareParameterizedBatch (fixed stage order: split → execution view → classify → resolve → validate → serialize → snapshot; per-source missing/invalid/errors/runnable; immutable boundParams snapshots; per-param field states). #165/#169/#170 stages ship as pluggable identity/unknown seams. bindPolicy is per-source: 'row-returning' keeps #134's rule, 'all' binds every statement (#175). Serialization is per-statement by the local declaration; wallNowMs is threaded to the resolve stage — one wall clock per wave, coalescing stays in callers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
fixes #155) Every gate/exec call site in ui/app.js and ui/dashboard.js now consumes one prepared batch per execution wave — no direct paramArgs/unfilledParams pairs remain at call sites: - app.js: new env.wallNow seam (epoch wall clock, injected separately from the performance.now-based duration clock); prepareTabSource() helper; the var gate (missing + invalid + serialization errors), run(), runScript() (per-statement args by index), setRunBtn, exportDirect/exportScript (the original input is threaded so the export batch aligns per statement, one wave clock from exportEntry), and runTile (accepts the dashboard wave's prepared args; self-prepares when called standalone — fixing #155's multi-statement favorite case). - dashboard.js: the favorites snapshot is analyzed once per render; runAll and runAffected prepare one batch (one wall-clock read) per wave; tile gating/args come from that batch, so a value that cannot serialize for one tile's declaration errors only that tile and never blocks siblings; affected-tile detection reads analysis.fields (ready for #165's inactive-block params). Behavior for existing scalar-string queries is byte-identical (regression sweep in tests); Array(T) values now bind as ClickHouse array literals. Closes #155. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
…#165) One narrow template construct: a comment-wrapped optional block /*[ AND d = {d:String} ]*/ is included (markers stripped, content byte-identical) only when every parameter inside it is active; otherwise the whole block is removed before the SQL is sent. Values are never interpolated — ClickHouse still performs the typed {name:Type} substitution — and the raw template is SQL-transparent: to any tool that doesn't know the convention each block is a plain comment, so it runs anywhere with all filters inactive. - new src/core/optional-blocks.js (pure, 100%): code-context scanning on the shared sql-spans scanner, v1 validation (no nesting, no */ or code-context ';' in content, no parameterless or whole-statement blocks, unbalanced → clear error), materializeOptionalBlocks + ALL_ACTIVE mode. - param-pipeline (#173) stages become real: the all-active analysis view feeds analyzeParameterizedSources (optionalIn/optionalAnywhere now populated; required-outside-a-block wins per source; template errors are per-source), the execution view is produced inside prepareParameterizedBatch — params of omitted blocks are never bound (absent from args and boundParams, field state 'inactive'); an explicitly active empty value binds a real empty string. Materialization is per-statement behind the #134 isRowReturning gate: DDL / parameterized views pass through verbatim. New mergedSourceSql + fieldControls helpers. - state.filterActive: own storage key (asb:filterActive), persisted alongside varValues, never in share links; effectiveFilterActive derives activation from value non-emptiness when no entry exists, so pre-#165 persisted values keep working on first load. - workbench: variables strip lists block-only params from the analysis view with an "optional" affordance (blank ⇒ inactive, typed ⇒ active); Run gates only on materialized-statement missing params; run/runScript/ Explain/export all send the execution view (byte-identical for SQL without blocks); Format skips a statement containing blocks with a notice instead of round-tripping a template through formatQuery(). - dashboard: filter-bar discovery via fieldControls over the all-active analysis; a blank text filter deactivates optional predicates instead of blocking the tile (required params still gate with the placeholder); activation flips re-run affected tiles through the same debounce + generation guard; tiles run the materialized wire text (block-free favorites keep their exact bytes). dashboardParams is absorbed. - regression guards: SELECT [[1, 2], [3, 4]] and SELECT [[{a:UInt8}, 2], [3, 4]] pass through untouched; scalar params outside blocks remain required; every existing query's behavior is byte-identical. Closes #165 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
…redness (#165 review) Review finding 1 (critical): a string literal containing ]*/ inside block content ended the SQL comment early, and the truncated candidate still looked well-formed (it ends with ]*/) — validation passed and silently mangled SQL went to the server. scanBlocks now detects a candidate whose content ends inside an unterminated string / quoted-identifier span (replaying the scanner's backslash + doubled-quote escape rules) and rejects it with a dedicated error, for both the ]*/-terminated and the plain */-terminated in-string forms — rule 3's "no */ in any form" now actually holds, and the README wording says so. Review finding 2 (major): prepareParameterizedBatch's active-empty-string bypass ignored per-statement requiredness — values {d:''} with active {d:true} silently bound param_d='' for a d *required outside any block*. The bypass now applies only to block-confined params: each bound statement re-derives its required set from the raw scan (blocks are comments to it — the same derivation phase 1 uses), a required occurrence gates as missing on a blank value regardless of the active map, and the field-state rollup reports 'missing' whenever any source gated, so a cross-source mix (required in A, block-confined in B) gates A while B binds the explicit empty string. Also adds the review's edge tests: adjacent blocks with no separator, CRLF byte preservation, space-free markers /*[{d:String}]*/, empty-content /*[]*/, and escape-rule coverage for the in-string detector. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
New src/core/param-validate.js validates {name:Type} variable values
against ClickHouse's actual param-value grammar (verified live against a
26.3.13 server, not guessed from SQL literal syntax): range-checked
Int8..Int256/UInt8..UInt256 via BigInt, Float32/64 syntax (incl.
inf/nan), a narrow never-invalid Bool accept-set, and UUID (hyphenated
or 32-hex compact). Everything else stays 'unknown' (pass-through),
permissive by construction. A tri-state (valid/invalid/incomplete)
lets a plausible mid-typing prefix ('-', '1e', a half UUID) stay
neutral while focused and harden into the inline error only on
blur/Enter/execute.
Wires into #173's pipeline as its validation stage (param-pipeline.js),
and introduces the shared invalid-field affordance (src/ui/var-field.js)
used by both the workbench var-strip and the Dashboard's global filter
bar. Also fixes two dormant #173-review gaps this stage exposed: the
Run button's disabled state now reflects invalid/source-error variables
too (previously only 'missing'), and a field whose value fails
serialization no longer rolls up as 'ok' in the pipeline's per-field
state.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
…ped reasons (#170 review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
Date/Date32/DateTime/DateTime64 variables now accept Grafana-grammar relative expressions (-1h, now-7d, now/d) alongside absolute values, with a preset dropdown + live preview. The stored value is the expression, so it re-resolves against "now" on every Run/Refresh/filter wave instead of freezing a timestamp. - src/core/relative-time.js (100%): grammar parse + resolution, DST-safe calendar arithmetic, per-type formatting (verified against ClickHouse 26.3.13's param_* path). - src/ui/combobox.js (100%): accessible type-to-filter combobox primitive (#174 §1) — full keyboard map, ARIA roles, IME-safe, mousedown-before-blur. - src/ui/relative-time-field.js (100%): composes the above for the workbench var-strip and the Dashboard filter bar. - src/core/param-pipeline.js: wires the real resolveRelativeValue stage (previously identity), gating a near-miss expression as invalid via #170's existing affordance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
…conds, aria-describedby (#169 review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
Every {name:Type} field remembers its 10 most recently used values,
recorded from a successful statement's #173 boundParams (never a
keystroke, never a failed statement, never an omitted-block or
empty-string param) and offered in a dropdown on focus that composes
with #169's preset dropdown for date-like fields. New pure
src/core/recent-values.js (100%) owns MRU/dedupe/caps and the
render-time type-filtering helper; src/ui/recent-field.js and a
combo-footer.js "Clear recent" affordance reuse the existing
combobox.js primitive rather than building a second control. The
header File menu gains a "Variable history" section (disable-history
preference + "Clear all recent values").
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
…e (v2) (#172) - v1: enumMembers/enumValues (param-type.js) parse Enum8/16 members straight out of the declaration; param-validate.js enforces membership (blocking), live-verified to also accept a bare numeric code. Works in the workbench var-strip and the Dashboard filter bar. - v2 (workbench only, suggestions never blocking): paramComparisonColumns (new src/core/param-comparison.js) finds a param's direct-equality column reference; resolveComparisonColumnType (from-scope.js) resolves it against the FROM scope + loaded schema cache. Zero new network requests — upgrades automatically once the existing idle-tick column loader lands. - Third combobox consumer: src/ui/enum-field.js (values-then-recents composition, filter-then-cap for large enums with a "type to narrow" hint). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
…e deferred strip rebuild, code-prefix incomplete (#172) - MAJOR 1: enumMembers now implements ClickHouse's auto-numbering (implicit member = previous code + 1, starting at 1; explicit codes reset the counter) instead of dropping implicit members — Enum8('a' = 1, 'b') no longer falsely rejects 'b', and Enum8('hello', 'world') enforces real membership. enumValues returns null (never []) for an empty/unparseable member list so every truthiness-checking consumer falls back to the plain input instead of an empty dropdown. - MAJOR 2: a background loadColumns completing while the user is focused inside the variables strip no longer rebuilds it out from under them — renderVarStrip defers the rebuild (one pending rerender, applied on the strip's focusout once focus actually leaves it; intra-strip focus moves keep deferring via relatedTarget). Focus, in-progress text, and any open dropdown survive; the v2 upgrade applies on blur. - MINOR 3: digits that are a strict prefix of some declared code's string form ('1' toward code 12, '-1' toward -12) validate as 'incomplete' while typing, mirroring the member-name prefix rule; a full number no code can extend stays immediately invalid (matches live-verified server rejection of unknown codes and case-mismatched members). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
…ved-identity comparison matching, shared element-token grammar Whole-branch review findings on the #173 pipeline core: - F1 (spec deviation, #173 acceptance): fields[name].conflict was computed and consumed by nothing. fieldControls now carries `conflict` (the distinct normalized types) per field, and a new pure `fieldControlKind` helper owns the enum > date-like > text control priority both rendering surfaces previously duplicated (F8c) — a conflicted field always degrades to text. - F3 (false CONFLICT on qualifier text): paramComparisonColumns no longer conflicts `e.status = {s}` with `status = {s}` on raw qualifier-string inequality. Same-column refs with different qualifier spellings are all returned (`refs`), and resolveComparisonColumnType decides on RESOLVED identity — every ref must resolve to the same table (single-table alias+bare ⇒ match; JOIN sides / ambiguous ⇒ null). Column-NAME disagreement stays a syntactic conflict. - F7 (serializer/validator grammar divergence): the array-element serializer now uses param-validate.js's live-verified token grammars (exported INT_TOKEN / isValidFloatToken) instead of its own third copy — '007' is rejected like the scalar path rejects it; 'inf'/'nan' Float elements are accepted. No range checks added (server wraps; unchanged). - analyzeParameterizedSources now defaults its analysis stage to the exported `analysisView`, symmetric with prepareParameterizedBatch's executionView. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
…licate member/preset recents, shared combobox wiring helpers
- F4: "Clear recent" only hid the footer — the open listbox kept the cleared
Recent options rendered and clickable. createCombobox now exposes
`refresh()` (re-pull options for the current text, re-render + aria-live
count), and all three field modules call it from onClear.
- F5: enum-field and relative-time-field no longer list a recorded value
twice when it duplicates a rendered enum member / preset — the recent is
excluded; the primary group's row already offers it.
- F8a/b: the six copy-pasted focus/input/keydown/blur/composition wiring
blocks (3× app.js var-strip, 3× dashboard.js filter bar) get one shared
`wireComboInput(field, {onValueInput, onCommit})`, and the triplicated
`idSafe` one-liner is one export — both in combobox.js. (Consumed by the
surfaces in the follow-up commit.)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
…sees optional blocks, wave-start arg snapshots, one analysis per keystroke - F1 (#173 acceptance): a type-conflicted variable now renders as a PLAIN text input on both the workbench var-strip and the dashboard filter bar (fieldControlKind; enum/date controls disabled), with a visible amber `.is-conflict` warning — distinct from is-invalid — whose tooltip lists the disagreeing declarations. New --warn-* theme variables, both themes. - F2 (#172 v2 blind to optional blocks): the comparison scan + FROM-scope resolution now run on the tab SQL's ANALYSIS materialization instead of the raw text, so `col = {p}` inside a /*[ … ]*/ block — one opaque comment span to the raw scan — gets its schema-cache enum dropdown too. - F6 (inconsistent batch capture vs auth awaits): all four execution paths (run, runScript, exportDirect, exportScript) capture their prepared source ONCE at wave start, synchronously with the gate check and before the picker/auth awaits — gate and args see the same varValues snapshot; edits during a token refresh apply to the next run. Invariant commented at each site. - F8: both surfaces consume the shared wireComboInput + fieldControlKind helpers (6 wiring copies and 2 control-priority copies deleted). - F9: renderVarStrip computes one analysis per repaint and feeds fieldControls, the v2 scan, the rebuild's initial paint, and the Run-button gate (new shared inputGate; setRunBtn's fallback uses it too) — previously the same SQL was re-analyzed by the setRunBtn tail on every editor keystroke (~0.7ms wasted on 100-statement scripts). The #172 deferred focus-guard rebuild is untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
…-Enum docs (phase7 user feedback) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK
Closed
32 tasks
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.
Phase 7 — filters & variables (roadmap #68)
One integration branch shipping the whole phase: every surface now shares the
{name:Type}/state.varValuesmachinery through a single two-phase parameter pipeline.Closes #173
Closes #155
Closes #165
Closes #170
Closes #169
Closes #171
Closes #172
Part of #174
analyzeParameterizedSources/prepareParameterizedBatch),scanParamDeclarations,parseParamType+ conflict detection, typed serializer (arrays, big ints as strings), per-source gating, immutableboundParamssnapshots, injectedwallNowwave clockbindPolicy); all app/dashboard gate/exec call sites migrated; scalar-string behavior byte-identical (regression sweep)/*[ … ]*/optional SQL blocks +state.filterActive]*/truncation now a clear error; activation never bypasses per-statement requirednessInt*/UInt*range-checked via BigInt,Float*, Bool, UUID), incomplete-while-typing model, inline invalid affordance, Run/tile gating+5/007/whitespace rejected; Bool never hard-rejects (server acceptsenable); ranges are deliberately stricter than the server, which silently wraps (256→0). Review fix: hardened invalid persists across unrelated re-renders-1h,now-7d,now/d, Grafana grammar) for date-like params, preset dropdown (first combobox consumer, #174 §1 contract), live human-readable preview, one pinnednowper rerun waveTZ=America/New_Yorkon npm test). Review fixes: readable preview, near-miss neutral while typing, floor seconds,aria-describedbyasb:varRecent), recorded fromboundParamsper successful statement, type-filtered via #170, presets+recents composition, Clear/disable affordancessrc/ui/combobox.jsprimitive and its three consumers + unit specWhole-branch high review (8-angle) — applied
fieldControlsexposesconflict.Verification
-1h,now-7d,now/d+ preset dropdown #169/Typed validation for variable inputs — range-checked Int/UInt, Float syntax, inline error + run gating #170/Enum variables → dropdown: values from the declared type (v1) or the autocomplete schema cache (v2, workbench) #172 were built (epoch binding, int/float/Bool/UUID width, enum numeric codes, array literal shapes).param_patabsent → present),-1hre-resolving per wave, out-of-range inline error, enum dropdown with implicit member, dashboard filter deactivation re-running tiles unfiltered, recents recorded after success.Manual-testing feedback (applied post-review, verified live on otel)
{o:Enum8/16}documented: ClickHouse rejects the type itself ("Enum data type cannot be empty", verified on 26.3.13) — README now points to{o:String}(inferred member dropdown, no retyping) or the full pasted enum type.Deferred (noted, not blocking)
#174§2–§5 (with Dashboard: multi-filter option bundles, shared preview, and role-aware result selector #160); per-field Clear keyboard path (above).sourceErrors-level UI beyond field warnings; localStorage write batching on keystroke; dashboard per-keystroke full-batch prepare (fine at current tile counts) — candidates for a Phase 7 polish follow-up.🤖 Generated with Claude Code
https://claude.ai/code/session_01N347SBQGxtASiGTmB3JBtK