feat: rule engine — force-extract QuickSight sheets into per-category files (#15)#19
Merged
Merged
Conversation
… files Add a path-anchored rule layer to the split walk. The engine now threads the walked path (mapping keys / sequence steps) to each node and consults the resolved preset's rules before the size/depth gate. A ForceExtract rule extracts a node regardless of MaxLines/MaxDepth; StopRecurse writes it whole (one file, no further splitting). The quicksight preset uses this to give a dashboard/analysis a stable, readable layout: the Definition and Sheets list always split out, each sheet becomes its own folder + a details file, and each present subcomponent category (Visuals, FilterControls, ParameterControls, Layouts, SheetControlLayouts, TextBoxes) is written whole to one file inside that folder. Rules are root-anchored so they only match the dashboard/analysis shape and never fire on other documents. The depth gate moved from process() into extractChild() so a forced node deeper than MaxDepth (a sheet category sits at depth 4, under the default depth 3) still extracts; unforced nodes past the threshold stay inline exactly as before. Forced extraction reuses the existing include/anchor machinery, so it is lossless by construction — covered by a round-trip test. Closes #15. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review of the rule-engine slice: - Add an explicit regression test that an unforced node past MaxDepth stays inline after moving the depth gate from process() into extractChild(). - Add a two-sheet (shared Name) test covering slug dedup and the file/folder coexistence (overview.yml next to overview/), with a round-trip assertion. - Document that ForceExtract applies to complex nodes only (scalar sidecars come later) and that a StopRecurse node shadows any rule whose path lies inside it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a path-anchored rule engine to the split walker and wires QuickSight preset rules so dashboards/analyses are exploded into a predictable per-sheet folder layout (plus per-category files) regardless of node size, while keeping unforced size/depth behavior unchanged.
Changes:
- Introduce
preset.Rule/preset.Rulespath matching (literal,*,[]) withForceExtractandStopRecurse. - Thread a logical
[]preset.Steppath throughsplitrecursion and apply rules to bypass MaxLines/MaxDepth (and optionally stop recursion). - Add QuickSight preset rules + tests (unit tests + testscript) to ensure forced sheet/category layout and round-trip fidelity.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/split/split.go | Threads path through recursion and applies preset rules to force extraction / stop recursion. |
| internal/split/split_test.go | Adds regression + QuickSight rule behavior tests (forced layout, dedupe, round-trip). |
| internal/preset/rule.go | Implements rule/path matching primitives (Step, Rule, Rules.Match). |
| internal/preset/rule_test.go | Tests AnyKey/AnySeq matching, first-wins, and QuickSight rule presence. |
| internal/preset/preset.go | Adds Rules to presets and defines QuickSight rule set for sheets/categories. |
| internal/cli/testdata/script/preset-quicksight-dashboard.txtar | End-to-end script verifying QuickSight forced layout and join/verify behavior. |
| internal/cli/commands.go | Ensures resolved split options include preset rules (rules not overridden by config/flags). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+286
to
+288
| if _, err := os.Stat(filepath.Join(out, "definition/sheets/overview/filtercontrols.yml")); err == nil { | ||
| t.Error("filtercontrols.yml should not exist: the sheet has no FilterControls") | ||
| } |
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.
What & why
Second slice of preset-based semantic explosion (#15). Turns "explode by size" into "explode meaningfully" for QuickSight: each dashboard/analysis sheet gets its own folder + details file, and each subcomponent category becomes its own whole file — regardless of size. Built on the preset abstraction from #14.
How
internal/preset/rule.go): aRulematches the walked path segment-for-segment (literal key,*any-key,[]any-seq-element). Actions:ForceExtract(+StopRecurse). First match wins.internal/split/split.go):process/extractChildcarry a[]preset.Step. AForceExtractmatch bypasses both the size and depth gates;StopRecursewrites the node whole (no descent). The depth gate moved fromprocessintoextractChildso a forced node deeper thanMaxDepthstill extracts (a sheet category sits at depth 4, under the default depth 3), while unforced deep nodes stay inline exactly as before.Definition,Definition.Sheets,Definition.Sheets[]force-extract; the six sheet categories force-extract +StopRecurse. Root-anchored, so they only match the dashboard/analysis shape and are inert on datasets/other docs.Result (real Analysis fixture)
Forced extraction reuses the existing
!include/anchor-closure machinery, so it's lossless by construction — the quicksight-preset join is byte-identical to the ruleless join, and a round-trip test asserts it.Verification
go build ./...,go test ./...,golangci-lint runall green.AnyKey/AnySeq/length/first-wins), forced sheet layout on a synthetic small dashboard (proves force, not size), category-written-whole, absent-category-no-file, and a full round-trip. Newpreset-quicksight-dashboard.txtarend-to-end.MaxDepthStopsRecursionetc. still pass (unforced behavior unchanged).Notes
verifystill uses rulelessDefaultOptions— fine here since forced YAML extraction is lossless by construction; the sidecar slice (SQL sidecar (.sql) extraction with local fidelity guard #16) is where verify becomes the fidelity gate.Closes #15.
🤖 Generated with Claude Code