feat(parser): support trailing and consecutive separator skips #132
Merged
Conversation
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 Report❌ Patch coverage is
📢 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.
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
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.
No description provided.