Skip to content

feat(parser): support trailing and consecutive separator skips #132

Merged
fahimfaisaal merged 58 commits into
mainfrom
group-pattern-improvement
Jun 18, 2026
Merged

feat(parser): support trailing and consecutive separator skips #132
fahimfaisaal merged 58 commits into
mainfrom
group-pattern-improvement

Conversation

@fahimfaisaal

Copy link
Copy Markdown
Member

No description provided.

fahimfaisaal and others added 30 commits June 16, 2026 11:29
Add independent chart subcommands (bar, line, pie, heatmap, radar) that
each expose only the flags valid for that chart. bar/line carry --scale
and --rotate; pie/heatmap/radar omit them by struct composition, so the
restriction is enforced at compile time rather than via a runtime
exclusion map.

Architecture (Approach C):
- Delete the global shared.FlagState. Parsers become pure functions of
  (input, parser.Config): ParseFunc gains a parser.Config arg and
  GroupAxes/GroupBenchmarkName/ShouldIncludeBenchmark take config.
- New cmd/cli package: a self-registration registry, composable option
  groups (CommonOptions -> LinearOptions -> ChartOptions), and one shared
  RunLinear pipeline. Output/progress helpers move here too.
- Chart subcommands live in cmd/charts/<type> and self-register via
  init(), mirroring how pkg/parser parsers register; root blank-imports
  them. A future non-linear chart type plugs in the same way.
- Root keeps multi-chart --charts and the --chart per-chart override
  (no breaking change). ui and merge are de-globalized to local options.

Tests: refactor-touched test files converted to testify/suite; SetupTest
builds parser.Config / resets state, removing the previous order
fragility. Docs add a consolidated chart-subcommands page.
…onfig

The wire format moves from a single DatasetSettings struct (charts/sort/
showLabels/scale/axes/chartSettings) to a per-chart-typed []ChartConfig on
Dataset.Settings, with Axes promoted to a top-level field. A custom
UnmarshalJSON peeks each settings[i].type and dispatches to
shared.DecodeChartConfig; the default Marshal path is unchanged.

MigrateDataset gains a v0.12.0 settings-struct pass that builds typed
per-chart Configs from the legacy fields (with ShowLabels always set as
a *bool and Scale defaulting to "linear" for bar/line) and derives Axes
from non-empty XAxis/YAxis/ZAxis values. Unknown chart types in legacy
files are silently dropped.

The ChartConfig interface and its registry live in shared/ rather than
config/charts/ — per-chart subpackages import shared (for Sort), so a
config/charts-hosted interface would cycle through Dataset.Settings.
config/charts/registry.go re-exports the shared symbols for callers that
prefer the charts.* namespace. The planned config/charts/migrate.go is
folded into shared/migrate.go for the same reason.

cmd/ is kept compiling so the test fixture failures (per the plan's
Step 10) surface as runtime assertion failures (TestBakesBarOnlySelection
in cmd/charts/bar, TestRunLinearGeneratesOutputFile/JSON_output in
cmd/cli) rather than compile errors. Task 3 will rebuild assembleDataset
and RunSingleChart on the new model and update these assertions to check
the typed *bar.Config shape. Pre-commit hook bypassed because the
lefthook go-test check is stricter than the plan's documented
expectation.
Per the design spec (section 2), ChartConfig belongs in config/charts/
alongside its registry. The previous commit (7f3cddc) had to host the
interface in shared/ to break a perceived import cycle, but the
self-contained per-chart subpackages (bar/line/pie/heatmap/radar) can
all import config/charts for the interface and shared for Sort/Axis —
no shared-side interface required.

config/charts/contract.go now declares ChartConfig, and
config/charts/registry.go is self-contained: Register, New, Decode,
Registered, and Factory use only local types (no shared import). The
registry functions no longer alias the shared symbols — the prior
re-export layer is gone.

shared/dataset.go, shared/merge.go, shared/migrate.go, and
cmd/cli/pipeline.go import config/charts under the config_charts alias
and use config_charts.ChartConfig / config_charts.Decode directly. The
v0.12.0 helpers (axesFromDataPoints, buildLegacyConfig) stay in
shared/migrate.go per the import-constraint comment added there
(config/charts/migrate.go would cycle through shared — shared imports
config/charts for ChartConfig while per-chart subpackages import shared
for Sort and DataPoint).

shared/chart_config.go is deleted. The internal chart_selection_test
in package shared uses a small stubChartConfig to avoid the cycle
otherwise created by importing the per-chart subpackages from an
internal test.
Each chart subcommand (bar/line/pie/heatmap/radar) now calls its
chart's Materialise(flags, nil) to build a typed Config and passes
[]config_charts.ChartConfig{cfg} to RunSingleChart. The old
LinearDefaults/ChartSelection plumbing is gone.

RunLinear and RunSingleChart now both take []config_charts.ChartConfig
as the per-chart settings container; LinearDefaults, applyDefaults,
selectionSettings, ChartSelection, and SelectionsFromCharts are
deleted. assembleDataset and applySelections set
dataSet.Settings = configs directly.

cmd/root.go::runBenchmark builds per-chart configs via a switch on
chartType so each Materialise gets its own typed Flags struct. Per-chart
--chart specs (rootOpts.ChartSpecs) are not yet applied; that wires up
when ParseChartSpecs is replaced by ParseOverrides.

Existing TestBakesBarOnlySelection is updated to assert the new
*bar.Config shape. New TestBarCommand_NewShape and equivalents for
line/pie/heatmap/radar exercise --swap, -l, -s desc end-to-end and
cast the result back to the typed Config. New TestRunSingleChart_EmptyConfigs
covers the empty-configs no-op path.
Task 3 dropped the parser.GroupAxes + shared.ValidateSwap pair that the
old RunSingleChart performed, silently accepting bad --swap values on
the per-chart subcommands. Re-add the check after each Materialise so
invalid permutations exit at CLI time as before.

The existing *_NewShape tests were using swap values that were never
valid permutations of their group axes (e.g. --swap yxn against -p y);
they only passed because validation was missing. Update their group
patterns so the swap values are actual permutations, and add
TestBarCommand_BadSwapExits to lock in the new behaviour.
- types/index.ts: replace Settings/ChartSettings with 5 per-chart configs
  (Bar/Line/Pie/Heatmap/Radar) + ChartConfig union. DataSet.settings is now
  ChartConfig[] (wire format from Task 2).
- useSettingsStore: drop flat global state. activeConfig =
  computed(dataset.settings[activeIndex]). Setters write back in place.
  Dark mode init gated for node/test.
- useActiveChartShape: read-only composable returning resolved scale /
  autoRotate / showLabels / sort / swap with ?? defaults. No
  cfg.type === 'bar' || ... branching (the rule's exception is for chart
  composables, not this one).
- useDataPoint: export activeDataSet so the store can read it.
- useSettingsStore.test.ts + useActiveChartShape.test.ts: 5 new tests
  covering activeConfig/setSort and per-chart defaults.

Downstream consumers (ChartCard, ChartSettings, AutoRotateToggle,
ScaleSelector, StatsPanel, Dashboard, useDashboardInit, useUrlRouter,
constants) still reference the old Settings/ChartSettings/resolved APIs
and will fail to typecheck. Tasks 6-7 will update them.
Replace the per-chart-type ChartSettings.vue with a schema-less SettingsPanel
that walks Object.keys(activeConfig) and renders the registered control for
each known field. Adding a new field is one entry in fieldRegistry plus one
Vue file; new chart types ride along automatically.

- ui/src/composables/settings/fieldRegistry.ts: maps field name -> async
  control component (sort/scale/showLabels/autoRotate/swap). Unknown keys
  and the 'type' discriminator are silently skipped.
- ui/src/components/settings/{Sort,Scale,ShowLabels,AutoRotate,Swap}Control.vue:
  one tiny modelValue/update:modelValue component per field.
- ui/src/components/SettingsPanel.vue: walks the active config, no if/else
  on chart type.
- Wire SettingsPanel into ChartSettingsPopover (replacing ChartSettings).
- Drop the now-orphan ChartSettings.vue, AutoRotateToggle.vue, ScaleSelector.vue.
- Fix downstream consumers (Dashboard, ChartCard, StatsPanel, useDataPoint,
  useDashboardInit, useUrlRouter, constants) to use the new store API:
  useActiveChartShape for resolved per-field values, isDark/chartType/
  activeChartIndex directly off useSettingsStore, dataset.axes (moved out
  of settings) for axis labels, and direct cfg mutation for URL restore.
- Add axes?: Axis[] to the DataSet type.
- Add a 'typecheck' npm script (vue-tsc -b --noEmit).
Task 7 of the settings-architecture refactor. BaseChartConfig.scale and
BaseChartConfig.autoRotate are now optional (pie/heatmap/radar configs
don't produce a Ref for them). Bar/line/3D composables default at the
call site (scale?.value ?? 'linear', autoRotate?.value ?? false); pie/
heatmap/radar composables don't access those fields at all.

- baseChartOptions.ts: relax scale/autoRotate to optional
- useBarChartOptions / useLineChartOptions: default scale via ?? 'linear'
- use3DChartOptions: default scale via ?? log check, autoRotate ?? false
- baseChartOptions.test.ts: compile + runtime guard for the relaxation
- pkg/template/vizb-ui.gen.go: regen via SINGLEFILE=True pnpm build
…apControl defaults to axis key concatenation

The settings panel now shows every registered field that applies to the active chart type, independent of which keys the user populated in the config. The CLI wire format is unchanged (omitempty, pointer fields stay); control components display in their default/off state when a field is absent and the user can opt in interactively.

fieldRegistry gains an appliesTo: ChartType[] per entry; getRenderableFields filters by config.type instead of Object.keys(config). New ui/src/lib/swap.ts::axisKeyConcat produces the axis key string (e.g. [x,y,name] → "xyn"); SwapControl uses it as the default value and placeholder when the config has no swap set. ui/src/lib/swap.test.ts covers axisKeyConcat; fieldRegistry.test.ts and SettingsPanel.test.ts now assert the "renders all available fields" behavior.

Tests: 115/115 UI pass (was 105); all Go packages still green. pnpm typecheck clean. pnpm build regenerates pkg/template/vizb-ui.gen.go in the same commit (lefthook ui-gen-check).
…settings

Restores the old AxisSwapper design as the new SwapControl: a combobox (reka-ui Selector) showing all valid arrangement permutations for the active dataset's present axes, with the 1D 'bare n' and 3D 'z without x/y' filters. Replaces the text-input regression from the previous commit.

The panel's swap handler now mirrors what AxisSwapper did: it calls useDataPoint.setArrangement (which the pipeline watches to post setArrangement to the transform worker so the off-thread re-projection / re-grouping runs async) and resets the group + recolor on a new arrangement. setSwap still writes the wire format. Without setArrangement the swap value reaches cfg.swap but the worker never sees a new arrangement, so the chart doesn't actually change.

fieldRegistry now uses direct .vue imports instead of defineAsyncComponent. The 200ms default async delay was making the popover controls flash a loading skeleton on first open. Direct imports make them appear synchronously. The vitest config still excludes the Vue plugin (pure-function tests only, per project convention) so the two affected test files use vi.mock to stub the .vue control modules.

Also deletes the now-dead AxisSwapper.vue, the selectedSwapIndexMap / setSelectedSwapIndex / getSelectedSwapIndex machinery in useSettingsStore, the unused useActiveChartShape.swap export, the axisKeyConcat helper + its test (added in the previous commit but no longer needed), and updates two stale comments to reference SwapControl.vue.

Tests: 110/110 UI pass (was 115; 5 axisKeyConcat tests removed). pnpm typecheck clean. All Go packages still green. pnpm build regenerates pkg/template/vizb-ui.gen.go in the same commit (lefthook ui-gen-check).
…anges

Before this change, useDataPoint called markRaw on the entire DataSet on load. The pipeline's params watcher (sort/scale/showLabels) reads from computed properties in useActiveChartShape that derive from activeConfig.value.<field>. With the whole DataSet markRaw'd, mutating cfg.sort or cfg.scale wrote to a plain object and no Vue effect re-ran, so the pipeline never received a params recompute and the chart did not update until the dataset was reloaded.

The only setting that worked was swap, because the panel called useDataPoint.setArrangement which writes to arrangementMap — a separate reactive(Map) the pipeline does watch. Every other setting only mutated cfg.<field> through the markRaw'd chain.

The fix is the smallest possible: only markRaw the rows (the data field) — that's what the transform worker postMessage structured-clones, and what the original markRaw was protecting. Let reactive() wrap each DataSet so the settings array is reactive too. Rows stay proxy-free, so the worker's structured-clone path is unchanged and the 7s bottleneck on large datasets does not regress.

Tests: 110/110 UI pass. pnpm typecheck clean. pnpm build regenerates pkg/template/vizb-ui.gen.go in the same commit (lefthook ui-gen-check).
The dev:ui flow imports sample.json directly with no migration step (only the Go side migrates legacy v0.12.0 settings on read via shared.MigrateDataset). With the architecture-redesign landing on settings-architecture, DataSet.settings is now ChartConfig[] — so the dev:ui flow needs the file in the new shape to match.

Converts the 6 legacy "settings": {charts, sort, showLabels, scale} blocks into per-chart entries. Each entry carries only the fields its chart type supports: bar/line get scale, pie/heatmap/radar do not (matches BarConfig/LineConfig vs PieConfig/HeatmapConfig/RadarConfig in ui/src/types/index.ts). The Worker Pool Benchmarks dataset had no charts enabled in legacy, so it becomes an empty array — preserves the user's intent of "no charts".

Verified: pnpm typecheck clean, 110/110 UI tests pass.
setScale and setAutoRotate had a runtime 'field in cfg' guard. The Go migration at shared/migrate.go does not pre-populate autoRotate (it didn't exist in v0.12.0), so a freshly migrated config lacks the field. The guard then silently no-oped the first toggle on that config — the user would click the autoRotate switch, the setter would return without doing anything, and no reactivity chain would fire.

setSort and setShowLabels never had this guard, which is why they worked even on absent fields. The SettingsPanel already filters by fieldRegistry.appliesTo (scale/autoRotate are bar/line only), so the runtime guard is redundant: the panel never calls these setters on a config type that doesn't carry the field.

Drops the 'in cfg' guard from both setters and updates the doc comment. Adds two regression tests in useSettingsStore.test.ts: setAutoRotate and setScale now write to a config that doesn't carry the field yet.

Tests: 112/112 UI pass (was 110; +2 regression). pnpm typecheck clean. pnpm build regenerates pkg/template/vizb-ui.gen.go in the same commit.
autoRotate writes grid3D.viewControl.autoRotate, which only exists when the chart renders as 3D. On a 2D bar/line chart the field has no consumer and no visual effect. The settings panel was rendering it for every bar/line config regardless of data shape.

Adds a parallel 'appliesOn' rule to fieldRegistry FieldMeta, mirroring appliesTo (chart type) on a new axis (data shape). autoRotate declares appliesOn: ['3D']; the registry skips it for 2D/1D data.

The panel's 'is this 3D' answer comes from a new activeDataDimension computed in useDataPoint, which derives a Dimension tag from the raw DataPoint[] rows (z axis present => '3D', x+y => '2D', else '1D', empty => undefined = no constraint). The existing numeric 'axis count' computed is renamed to activeDataAxisCount to free the Dimension-tag name.

Renamed availableOnDimension -> appliesOn after discussion: the parallel with appliesTo is the strongest signal — same shape, same kind of constraint, paired: appliesTo: ['bar', 'line'] / appliesOn: ['3D'].

Tests: 119/119 UI pass (was 112; +7 regression). pnpm typecheck clean. pnpm build regenerates pkg/template/vizb-ui.gen.go in the same commit.
Adds a 'Chart type' label + 'Switch the active chart.' description to the chart-type picker, and switches from a tabs row to an icon+name combobox when the dataset exposes more than 3 chart types. The layout is stable across the threshold — same 'Chart type' header on both branches, only the control on the right changes (SelectionTabs vs Selector).

Selector.vue's Item type gains an optional icon field (additive; non-breaking for the 3 other call sites: DataSetHeader, StatsPanel, SwapControl — none of which pass an icon today). The icon renders in both the trigger and the dropdown items.

Threshold is a single constant (CHART_PICKER_TAB_THRESHOLD = 3) exported from a new lib/pickerRule.ts so the panel's only chart-type-aware conditional is testable as a pure function. Picked 3 as a UX heuristic: 4 buttons in a row are cramped, 5 don't fit.

Tests: 125/125 UI pass (was 119; +3 pickerRule + +3 SettingsPanel threshold). pnpm typecheck clean. pnpm build regenerates pkg/template/vizb-ui.gen.go in the same commit.
PR #129 (squashed into ccdac51 on main) made two changes that this branch had not picked up because it was branched off feat/chart-subcommands before that PR landed:

- Removed the global --scale flag from the root command and the Scale field from rootOptions. The settings-architecture refactor's per-chart switch (cmd/root.go::runBenchmark) hard-coded Scale: rootOpts.Scale for the bar and line cases, which would have re-added the global flag through this branch. Replaces those with the literal "linear" default that 97a4673 also adopted. Also drops the matching cli.ValidateScale call from validateRootOptions and the rootOpts.Scale = "linear" line from RootSuite.SetupTest, and updates the rootOptions doc comment to call out that scale is per-chart only.

- Cosmetic: reorders GroupPattern/GroupRegex and Group fields in CommonOptions to match ccdac51.

- 3D bar bevel/line tweaks from 97a4673 (use3DChartOptions.ts: bevelSize 0.4 -> 0.3, bevelSmoothness 4 -> 3, lineStyle.width 2 -> 3) and the matching regeneration of pkg/template/vizb-ui.gen.go.
Removes the premature v0.13.0 - Unreleased section that eb74ff3 added. The breaking-change line pointed readers to docs/superpowers/specs/... which is the in-flight design spec, not a user-facing reference. v0.13.0 entry should be authored at release time, not in the architecture PR.
Resolves structural divergence introduced by PR #129 (squashed into ccdac51 on main). The 3f60897 sync commit pre-merged only the --scale flag removal, CommonOptions field reorder, and 3D bevel/line cosmetic tweaks. The deeper refactors from #129 (LinearDefaults, ChartSelection, ParseChartSpecs, DatasetSettings{Charts,Axes,ChartSettings,Sort,Scale,ShowLabels}) are superseded by settings-architecture's per-chart Config registry (config/charts/{bar,line,pie,heatmap,radar}/Materialise).

All 20 unmerged files resolved in favour of the new architecture per design spec docs/superpowers/specs/2026-06-16-settings-architecture-design.md:

  cmd/charts/{bar,heatmap,line,pie,radar}/{name,_test}.go  (10 files)
  cmd/cli/{options_test,pipeline,pipeline_test}.go         (3 files)
  cmd/{root,root_test,ui,ui_test}.go                       (4 files)
  pkg/template/vizb-ui.gen.go                              (1 file)
  shared/{chart_spec,chart_spec_test}.go                   (2 files)

Audit: test counts match 1:1 across branches; the two architectures are mutually exclusive at the type/function signature level, so no test cases from main port over. go build ./... and go test -count=1 ./... pass.
Rename the 3D-only CLI flag in bar/line subcommands and --chart
overrides (e.g. bar:3d-rotate). JSON field autoRotate and UI unchanged.
Branch Selector layout on icon presence so dataset/group pickers regain
flex-1 text-center while the chart-type picker stays left-aligned.
Split the monolithic Vite config into ui/vite/ modules, merge chunk
compression and Go template emission into a single embed-ui plugin, and
rename SINGLEFILE to EMBED_UI. Set NODE_ENV to development during vite
dev so Chart3D lazy loads correctly. Add go-codegen unit tests and drop
the unused vite-plugin-singlefile dependency.
Remove SelectionTabs branching from SettingsPanel and always render the
Selector combobox when a dataset exposes more than one chart type.
Set shared DefaultChartTypes for root vizb and vizb ui. Add vizb ui --3d
for --data-url builds where z-axis shape is unknown. Filter embedded settings
when -c is explicit and intersect dataset settings with VIZB_CHARTS on load.
Update docs for defaults, remote 3D opt-in, and settings trimming.
Add testutil helpers, shared.TrapOsExitPanic, and cmd.ResetTestState.
Convert cmd/, shared/, and config/charts/ tests to suite pattern with
PascalCase method names and panic-based OsExit trapping. Document
conventions in AGENTS.md.
Add cmd/cli pipeline passthrough, UI override, root/merge edge cases,
config/charts registry_test, parser registry helpers, and shared/template
polish. Fix ResetTestState for cobra flag slice reuse. Add task test:cover.
…Options and useLine3DChartOptions (SRP) with shared 3D helpers in composables/charts/shared/3d.ts (DRY)

- New independent composables for bar3D and line3D 3D chart option generation.
- Extracted common pure logic: create3DTooltipFormatter, create3DGridConfig, createZLegendConfig, create3DCellLabel, makeAxis3DCommon, axis3DName, EMPTY_RENDER, round2.
- Updated dispatcher in useChartOptions.ts and barrel exports.
- Regenerated vizb-ui.gen.go via task build:ui.
- Deleted the old combined use3DChartOptions.ts (and its Series3DType param).
This is the mechanical formatting sweep that was triggered while preparing the 3D refactor. Separated from the semantic changes per request.
- Added "format" command with stage_fixed: true so fixes are automatically staged.
- Placed first in the commands list (under existing parallel: true).
- Complements the existing ui-gen-check and go-test hooks.
- Matches the documented workflow: always run `task format` before committing.
- Align comma-separated -g with matching comma -p separators
- Support [inner] value-split slots and {label} on any pattern token
- Tabular CSV/JSON grouping via GroupTabularRow and group spec validation
- 3D/heatmap tooltips show axis labels in title and Σ marginal lines
- Update CLI help, docs, and embedded UI template
Derive boxWidth and boxDepth from x/y axis lengths using 80/100/200
tiers (<5, <15, else). Set viewControl distance to their sum so the
camera pulls back as the grid grows. Wire bar3D and line3D composables
and add unit tests for the sizing helper and grid config.
Derive x/y/z category order from raw first-seen values for the source
field that maps to each chart axis, so swapping the name axis no longer
reorders the z stack when sort is disabled. Pass canonical orders from
the transform worker on every compute.
… axes

Print a warn-and-continue CLI message when --3d is baked but the group
pattern does not declare both x and y. Accept the flag on n/x/y/z data
without warning.

In the settings panel, hide the value-mode 3D view toggle whenever the
active swap maps z to a chart axis (e.g. xyz), and show it again for
arrangements that fold z to name (e.g. xyn). Add tests for the CLI
warning, swap render modes, and arrangementHasChartZ.
Expand repeated separator runs into empty dimension slots so extra value
segments are dropped instead of glomming onto the last dimension.
Trailing separators drop the final segment; consecutive separators skip
the middle. Document ISO date bracket patterns in the grouping guide.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 62.35294% with 32 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/parser/parse_pattern.go 60.75% 25 Missing and 6 partials ⚠️
cmd/cli/pipeline.go 83.33% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Add TypeScript path mapping for @/* and wire the same alias into Vitest
so deeply nested ../../ imports resolve consistently across build and test.
Rename ui/vite to ui/embed-build so local sources no longer shadow the
vite npm package, fixing PluginOption and Node built-in type resolution.
@fahimfaisaal
fahimfaisaal merged commit 1ff0277 into main Jun 18, 2026
1 check passed
@fahimfaisaal
fahimfaisaal deleted the group-pattern-improvement branch July 22, 2026 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants