fix(analyze-changes): improve Step 1 metric confirmation and fix typos - #31
Closed
vyhuynh-product wants to merge 18 commits into
Closed
fix(analyze-changes): improve Step 1 metric confirmation and fix typos#31vyhuynh-product wants to merge 18 commits into
vyhuynh-product wants to merge 18 commits into
Conversation
build-dashboard's propose-and-confirm step now draws the proposed dashboard as a mermaid block-beta wireframe on the same 12-column grid as the canvas, so the user approves the real arrangement rather than a prose description of it. Column spans are the build widths (1/4 3, 1/3 4, 1/2 6, full 12), step 3 builds to the confirmed wireframe, and verify step 5 checks the canvas matches it. The text plan loses its per-block bullets and its Layout line, both of which the diagram now carries. Box colour encodes the kind of block -- data, control, section heading -- using design-system values, and everything sits inside a frame so the diagram reads as separate from the message text. The diagram declares htmlLabels:false, without which long labels are truncated in chat. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
build-custom-chart and build-dashboard-theme each prescribe where a new file goes. Both now point at .holistics/library/ instead of a bare library/ at the project root: .holistics/library/custom_charts/<chart_name>.chart.aml .holistics/library/themes/<slug>.theme.aml This covers the frontmatter descriptions, the workflow steps that write the file, and the "look for something to start from" steps that read the folder. Applied to both the native-skills copy and the plugins copy of each skill so the linked directories stay in sync. The two prose references to the public chart library are left alone; they do not name a folder. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…gin source Follows the shared-content model in DEVELOPMENT.md: a skill is defined once in a source plugin, and every consumer holds a full copy with a .link file pointing back at it. build-custom-chart sat at native-skills/system/build-custom-chart -- the only skill directory not under a plugin group folder, so it was not reachable as part of a plugin. Its native copy moves to native-skills/system/holistics-development/ and now links to the existing plugins/holistics-development/skills/build-custom-chart. The two directories were previously independent copies with nothing enforcing that they matched. build-dashboard had no plugin copy at all. Its content becomes the source at plugins/holistics-development/skills/build-dashboard, and the native copy keeps its path at native-skills/system/ holistics-common/build-dashboard, now carrying a .link. build-dashboard-theme was already correct and is untouched. Copies were produced with `pnpm sync-links`, so edits from here go to the source under plugins/ and propagate outward. Side effect: build-dashboard and build-custom-chart are now covered by `pnpm validate-frontmatter`, which globs plugins/**/SKILL.md only -- 31 files validated, up from 30. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…trols Two skills that complete the dashboard-building family alongside build-dashboard, splitting the `DashboardBlock` union by owner: - build-visualizations owns every VizBlock — built-in visualizations and dynamic content blocks (MarkdownViz). Covers picking a viz type by data job, authoring via generate_viz rather than by hand, and stripping the decoration and filters that the theme and the dashboard's controls own. - build-dashboard-controls owns FilterBlock / DateDrillBlock / PopBlock and the interactions array. Covers deriving the control set from a dashboard's job, wiring each control to every block it can affect, and disabling parent-child and cross-tab edges that would otherwise fall to platform defaults. Both are model-invocable only (user-invocable: false); build-dashboard stays the user-facing entry point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses @datbth's comments on #27 (the `never` on filter defaults, and the redundant "never force one field onto every block"), and corrects several things that were wrong or over-prescribed. Corrections: - Filters carry an explicit *neutral* default, not no default. Real dashboards write `default { operator: 'matches' value: '$H_NIL$' }`; the old rule said a FilterBlock must never have one, which no workspace dashboard matches. - DateDrillBlock / PopBlock defaults are optional (`default(optional)` in the type defs; product fixtures omit them). The schema showed `default: 'month'` and a year-over-year PoP inline, which quietly made both the house default — undoing the rule two sections up that neither is added by default. - Field vs manual filters are now distinguished. A field filter auto-maps to same-tab blocks on its dataset; a manual one carries no field and filters nothing until mapped per block. Only the field form was documented before. - Drill and pop are dead until mapped, like manual filters, so their cross-tab `disabled: true` scaffolding is gone — unmapped already excludes. The three live-by-default edges still need it. - A viz block takes at most one drill and one pop; the verify step now checks this, since the map-or-disable matrix passes a block mapped by two drills. Rework: - Interactions reorganised into the six edges grouped by default state (live-until-disabled vs dead-until-mapped) — that split decides what to write, and the schema example now mirrors it. - Input section: 5 items to 4, in source -> target order, plain names instead of coined ones ("Slicing needs", "Grain switching"). Dropped the item that restated which controls exist. - Output section: the 1-3 filter count replaced by the test that produces a count (breaks down a block here, low-cardinality, answers something the layout doesn't) plus overlap and job-shape factors. - Workflow 6 steps to 5; "Declaring scope well" folded into "Others", cut to what the interactions section doesn't already say. - Control placement is a principle (position advertises reach) instead of the always-top-of-canvas recipe, which was only right for page-wide controls. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses the four review comments on #27, and corrects what checking them turned up. Corrections: - generate_viz returns the whole Viz AML, `content:` included. The skill said it "scaffolds the query" while "you write only the content: template", which is wrong, and the viz-type table repeated it ("you craft its template"). Both now say to describe the card in the `query` instead. - The workflow said "scaffold-then-craft for a dynamic content block", the same mistake in the step an author actually follows. Review comments: - The strip rule is a preference, not an absolute: "usually shouldn't keep", with a carve-out for a filter that is part of what the chart is, and a user instruction outranking the test. - The strip/keep guidance was three enumerated sub-bullets that contradicted each other (one said strip filters, the next said keep some). Collapsed to the test plus the distinction that resolves it; every property enumeration is gone, so it no longer dates as the product gains features. - Dropped "prefer the dataset's predefined metrics" as low-value. Note the parent does still choose the metric — it names one in the generate_aql prompt — so the line was cut for weight, not because it was inaccurate. Rework: - "data block" was invented; the construct is VizBlock. Renamed throughout, including the title and the description's first line, which had been defining the invented term against the real one. - "data job" -> "analytics task". - Input section reshaped to the numbered form the sibling skills use, with the question phrased in the reader's words. - Workflow rebuilt around what generate_viz actually is — a sub-agent that decides anything the prompt leaves unstated. The steps now centre on composing that prompt (name the type, the field roles, the formatting, the exclusions) and checking what returns; retry guidance added, since passing prior errors back is documented as helping it self-correct. - Viz-type table split into preferred vs other types, so the CombinationChart preference sits where the choice is made rather than only in Conventions, and slash-lists that implied interchangeability (ScatterChart/BubbleChart, the three map types) now say when each applies. - The strip rule is stated once, in Conventions, instead of five times. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…enerate_viz Closes out the remaining half of @datbth's "isn't this the responsibility of generate_viz?" — the dynamic content block section still carried the template mechanics ({{ … }} / {% … %}), the full MarkdownViz AML shape with its rows/values bindings, and a pointer to the syntax reference. generate_viz authors the block including its template, so all of that is its knowledge, not the caller's. What the caller still owns stays: routing (the three confusable boundaries in "Choosing the viz type" are unchanged) and the content of the prompt — a card needs describing as a finished card, since only the caller knows which card. Also merged the two authoring paragraphs. The second one existed to say a dynamic content block is authored the same way as a built-in, which is a paragraph earning its keep by announcing it isn't needed. One paragraph now covers the flow for every type, and a second names the single axis that varies: what the `query` has to carry. The minimum-output bullet lost "valid HTML/CSS/Markdown, every {{ … }} bound to a declared query field" for the same reason — that is generate_viz's correctness, not a bar the caller can hold. It now states the bar the caller does control: the block shows something a built-in can't, with live values. Note this assumes generate_viz carries its own MarkdownViz authoring context. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ls a plugin source Mirrors what build-dashboard and build-custom-chart already do: the plugin copy under plugins/holistics-development/skills/ is the source, and the native-skills directory holds a synced copy with a .link back to it. build-visualizations and build-dashboard-controls now ship in the holistics-development plugin alongside build-dashboard, so the three skills that split the DashboardBlock union stay together in both runtimes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ons-and-controls feat(native.system): add build-visualizations and build-dashboard-controls
…isting dimension proposal skill now proceed to breakdown immediately
…re is not enough data
thinhnguyentruong
force-pushed
the
system/vy-improve-change-analysis4
branch
from
August 17, 2026 07:41
bd8688e to
620d28a
Compare
Contributor
|
Superseded by #32, which carries the same head branch ( All 7 commits in this PR are in #32; the remaining commits shown here are already on Closing in favour of #32. |
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
3 participants
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.
Summary
base_periodandcomparison_periodinstead of a singletime_reference, and fetch actual values + compute delta with formatting rules/detect_anomoly→/detect_anomalyDepdendency→DependencyTest plan
analyze-changesskill with a metric drop/increase question and verify Step 1 outputs actual values and formatted delta/detect_anomalycorrectly🤖 Generated with Claude Code