Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .github/workflows/docs-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,18 @@ jobs:

- name: Build docs site
run: npm run build --prefix docs

# `astro build` exits 0 even when the `docs` collection resolves empty —
# it emits a lone 404 page and mentions "The collection "docs" does not
# exist or is empty" in passing. That is exactly what the Starlight 0.41
# upgrade did before the content loader was added (#1461), and with the
# site not yet deployed (#1318) nobody would have noticed. A green build
# is not evidence the site has content.
- name: Fail if the build emitted no content
run: |
pages=$(find docs/dist -name '*.html' | wc -l | tr -d ' ')
echo "emitted $pages HTML pages"
if [ "$pages" -lt 10 ]; then
echo "::error::Docs build emitted only $pages page(s) — the content collection is probably empty."
exit 1
fi
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ Chart-experience release: chart authoring, editing, rule-based styling and click

### Fixed

- A table whose saved `groupBy` was an **array** rendered completely flat — no group rows, no aggregates, no error — so individual rows read as group totals, a wrong answer with nothing to suggest anything had been dropped. Three seeded Chart Reference tiles holding `groupBy: ["region"]` were pixel-identical to the ungrouped tile. `parseGroupByColumns` accepted only a comma-separated string and its `typeof groupBy === "string" ? groupBy : ""` guard turned any array into an empty string. Note the shape of the bug is the opposite of what it looks like: the widget editor emits `vals.join(",")`, so configuring grouping through the UI always worked — the array reaches storage from seeded layouts, imported dashboards and NeoDash conversions. Both forms are now accepted, order preserved as the nesting hierarchy (#1395)
- A parameter widget's **Default value** was never applied, so every dashboard relying on defaults rendered empty on arrival. `extractParamDefaults` walked the layout correctly, had its own unit test, and had **zero production callers** — the editor's field wrote a value into the saved layout that was read only by a test. The seeded Chart Playground carries 21 configured defaults across 8 pages and showed `Waiting for parameters… $param_cat_dimension $param_cat_metric …` on every chart until the user set each knob by hand. Defaults are now seeded on load, filling only parameters that are not already set — which gives the precedence `URL > restored session > default` without any ordering machinery, since both of those are applied before the layout finishes loading. A once-per-dashboard guard stops a cleared parameter snapping straight back. This was the third instance of the same shape after #1234 and #1388, so a guard now asserts this helper has a production caller; the general form, a dead-export check across `lib/`, is #1477 (#1421)
- Every dashboard page you had ever visited kept auto-refreshing, forever, so query load scaled with browsing history rather than with what was on screen. Measured on Chart Reference sitting on the same page with the same 18 visible widgets: 16 `/api/query` POSTs in 40s having opened only page 1, against 77 after touring six pages and returning — **4.81x the database traffic for an identical view**, ~68 hidden widgets polling for pages nobody was looking at. Tour all 20 pages and leave the tab open and roughly 230 widgets keep polling. All of it is refresh-tier work against the customer's database, which is exactly the load the query scheduler exists to shed, so one user exploring a large dashboard could crowd out interactive queries for everyone on that connector. `isActive` was already computed and already used to hide the page and to gate `onLayoutChange` — it just never gated the refresh. Visited pages still stay mounted, so tab switching remains instant and does not re-query; they simply stop polling while hidden (#1419)
- Two options were advertised in the widget editor, implemented in the component library, and silently discarded in between: `trendEnabled` (single-value) and `thresholdZones` (gauge). Setting either did nothing, with no error, and the seeded reference dashboard shipped tiles demonstrating both — the `thresholdZones (3 bands)` tile was pixel-identical to `default`. The cause was not the Zod schema, which uses `.passthrough()` and preserved the keys intact; it was the plugin's explicit prop mapping, which never passed them to the chart. The worse half was the KPI: `transformToValueData` took the first column **positionally**, so following the editor's own "requires 2 rows" instruction on a `label, value` query rendered the date as the headline metric — `$2026-03`. It now picks the first numeric column and carries the previous row so a trend can be computed. A ratchet asserts that every option offered for a chart type is actually read by its plugin; it found the same bug in five more plugins, allowlisted and tracked in #1472 (#1397)
- A long-format result — `category, series, value`, what `GROUP BY a, b` naturally produces — rendered a bar or line chart that looked plausible and was wrong. `resolveValueKeys` treats every non-label column as a value series, so the `series` column became a series of its own whose cells all coerced to null: 12 bars with duplicated x labels, a ghost legend swatch with no bars, and **no stacking at all** despite `stackMode` being set. Line was worse — a single revenue line drawn across duplicated x values, interleaving delivered -> shipped -> delivered, which reads as a violently spiky time series but is pure artifact. Nothing marked either as an error, and the seeded reference dashboard itself demonstrated two `stackMode` options with this exact query shape, which is how easy it is to fall into. Bar and line now reject a result whose plotted value columns contain no numeric values, naming the offending column and showing the pivot that fixes it. A column that is entirely null stays legal — that is a sparse series, what a LEFT JOIN produces. `validate` now receives the same column mapping as the transform, so a result whose text columns the user already mapped away is not wrongly rejected. The 11 affected reference tiles are rewritten to wide format, so the stacking options finally demonstrate stacking (#1400)
- `numberFormat: "percent"` appended a `%` sign and scaled nothing, so a KPI computing `401 / 2000` rendered **`0.2%`** where the true share was **`20.05%`** — wrong by a factor of 100, rendered cleanly, with nothing in the UI to distinguish it from a real value. `0.2%` and `20.05%` are both believable share figures. The convention was never stated anywhere and the two option surfaces documented **opposite** contracts: the table's option was labelled `Percent (12%)`, asserting input was already 0–100, while the seeded reference tile was authored with ratio semantics — the clearest evidence the contract was not discoverable. `percent` now takes a **ratio** and scales it, via `Intl.NumberFormat`'s own `style: "percent"`, which also brings locale grouping (`12.5` → `1,250%`) that the hand-rolled branch never had. With no explicit decimal places it caps at 2 rather than Intl's default of 0, since rounding `20.05%` to `20%` is the same class of silent precision loss. **Breaking** for anyone who worked around the old behaviour by pre-multiplying by 100 in their query. Both option descriptions, both option labels, the single-value docs page and the demo tile now state the expected input range (#1396)
- A client-side `groupBy` reported `count` as the number of **rows** in the group while `sum`, `avg`, `min` and `max` all skipped nulls, so the three contradicted each other: a group of `[100, 200, null]` returned `count: 3, sum: 300, avg: 150`, and `sum / count` came to 100. A reader saw "3 revenue values totalling 300" with no way to know only two rows had revenue at all — and because `count` is typically the denominator someone divides by, the error propagated into everything derived from it. Nulls in an aggregated column are not an edge case; they are what a `LEFT JOIN` produces. `count` now counts non-null values, matching SQL's `COUNT(col)` — the output key is `revenue_count`, named for the column, so the column form is the only one it could have meant. The existing test used sample data with no nulls and passed on both the wrong and the right behaviour; the new invariant test (`sum / count === avg` for every group) fails on the old code and cannot be silently re-broken (#1414)
- Overlays ignored `prefers-reduced-motion`. All ten Radix primitives (dialog, alert-dialog, sheet, popover, tooltip, toast, select, dropdown-menu, context-menu, navigation-menu) animated unconditionally, even though spinner, progress and skeleton had honoured the preference since the component audit. Per-component `motion-reduce:animate-none` cannot fix this: the overlays animate through data-attribute variants like `data-[state=open]:animate-in`, which compile to `.class[data-state=open]` — specificity (0,2,0) — while the `motion-reduce:` utility is (0,1,0) and a media query adds no specificity, so the guard loses the cascade. The reset now lives once in `design-tokens.css`, the only stylesheet both packages import. It also fixed a CI flake nobody had connected to accessibility: Radix keeps an overlay mounted until its exit animation reports `animationend`, and when an NVL/WebGL widget mounted alongside the close that animation stalled, leaving `data-state="closed"` dialogs visible past a 5s budget — every "flaky graph chart" failure on `dev` was this, never a graph assertion. The E2E suite now runs as a reduced-motion user, so the accessibility branch is exercised in CI rather than merely declared (#1458)
- The "Sync to URL" toggle on a parameter-select widget did nothing. `buildUrlParams()` was called with no exclude set and `extractNoSyncParams()` — the helper written to supply one — had **zero production callers**, only its own unit tests. Switching the toggle off still put the value in the address bar. URL sync is now **opt-in**: only a parameter whose widget sets `syncToUrl: true` reaches the query string. **Breaking for shared links** — a URL carrying `?param_…` for a widget that never opted in will no longer reproduce those values, and the parameter is stripped from an inbound URL rather than silently ignored (#1388)
- Leaflet's zoom-transition timer fired after the map was torn down, throwing `Cannot read properties of undefined (reading '_leaflet_pos')`. Four such unhandled errors made the entire Storybook browser project exit 1 even though every story passed, which is why the visual-regression gate could only be pointed at two files. The timer is now disarmed before teardown, so `test:visual` runs the whole project (#1384)
Expand All @@ -31,10 +38,12 @@ Chart-experience release: chart authoring, editing, rule-based styling and click

### Changed

- Docs site upgraded to astro 7.1.4 and starlight 0.41.5. Dependabot had split this into two PRs that could never pass — each broke the other's peer range — so they are replaced by one combined bump (#1461). Three breaking changes came with it: `social` takes an array of link items (starlight 0.33), autogenerated sidebar groups can no longer carry a `label` beside `autogenerate` (0.39), and content collections now require an explicit loader. That last one is worth knowing about: without it the `docs` collection resolves empty and `astro build` still **exits 0**, emitting a single 404 page while mentioning the empty collection only in passing. CI now fails the docs build when it emits fewer than 10 pages, because a green build was not evidence the site had any content — and with the site not yet deployed (#1318) an empty one would have gone unnoticed
- `zod` upgraded from 3.25.76 to 4.4.3. The tree previously held **two majors at once**: `component` declared `zod@^4.3.6` while importing it zero times, and that phantom dependency hoisted v4 to the root, forcing `app` to carry a nested v3. The unused declaration is removed, so there is now one zod. Migration was smaller than a 51-file footprint suggests — `.passthrough()` is unchanged and still preserves unknown keys, which matters because chart options depend on it (#1397). Three real changes: `ZodError.errors` is gone in favour of `.issues` (8 API routes, where leaving it would have turned a helpful 400 into a 500), `z.record()` now requires an explicit key type at the type level (11 sites), and `issue.path` widened to `PropertyKey[]` so it can contain symbols (#1436)
- `AlertDialog` now uses the same exact-centre animation as `Dialog`. It had kept upstream shadcn's `top-[48%]`, a deliberate 2%-of-height rise, so the two sibling modals animated differently and its centring classes carried no comment protecting them. Both now use `top-1/2` on both axes, and the geometry scrub is shared by both story files (#1373)
- `CLAUDE.md` moved to `.claude/CLAUDE.md`, alongside the hooks, skills and agents it belongs with, and out of the repo root where every visitor saw it. Claude Code reads both paths, so nothing changed about how it loads
- `npm run review:local` targets the active release branch instead of the previous one
- Vitest caps worker forks at 50% of available parallelism in `app` and `component`. The default is roughly one fork per core, and each fork loads jsdom + React + the component library, so on a many-core machine the suite ran out of headroom before it ran out of cores: workers began failing to boot with `Timeout waiting for worker to respond`, taking whole test files down with them. Measured on a 10-core machine, the `app` suite went from **889s with 96 failures and 9 files never collected** to **37s with 3480/3480 passing** — and a run that silently collects nine fewer files still prints a summary that looks complete, so the lost coverage was invisible. Expressed as a percentage rather than a fixed count so CI runners scale down with it (#1240)

### Added

Expand Down
133 changes: 133 additions & 0 deletions app/e2e/hidden-page-refresh.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import { test, expect, ALICE, createTestDashboard } from "./fixtures";

/**
* #1419 — every page you had ever visited kept auto-refreshing, so query load
* scaled with browsing history rather than with what was on screen. Measured on
* the Chart Reference dashboard: 4.81x the `/api/query` volume for the same 18
* visible widgets after touring six pages.
*
* This is that measurement as a regression test. It counts `/api/query` POSTs
* over a fixed window while sitting on page 1, then repeats the count after
* visiting the other pages and returning to page 1. The visible view is
* identical in both windows, so the counts must be too.
*/

const REFRESH_SECONDS = 5;
/** Long enough for two refresh ticks, short enough to keep the test bearable. */
const SAMPLE_MS = 12_000;

/**
* Each page must query something *different*.
*
* `use-widget-query` keys on `[connectionId, database, query, params,
* staleTime]` with no widget id, so widgets sharing a query across pages share
* one TanStack cache entry — and therefore one refetch, however many are
* mounted. An earlier version of this fixture gave every page the same query
* and passed identically with and against the fix, measuring nothing. The
* distinct `LIMIT` is what makes the pages independently pollable.
*/
function page(id: string, title: string, limit: number) {
return {
id,
title,
widgets: [
{
id: `${id}-w1`,
chartType: "table",
connectionId: "conn-neo4j-001",
query: `MATCH (m:Movie) RETURN m.title AS title LIMIT ${limit}`,
settings: { title: `${title} widget` },
},
],
gridLayout: [{ i: `${id}-w1`, x: 0, y: 0, w: 12, h: 4 }],
};
}

test.describe("Hidden pages do not auto-refresh (#1419)", () => {
test.beforeEach(async ({ authPage }) => {
await authPage.login(ALICE.email, ALICE.password);
});

test("query volume depends on the visible page, not on browsing history", async ({
page: pw,
}) => {
test.setTimeout(120_000);

const { id, cleanup } = await createTestDashboard(
pw.request,
`Hidden refresh ${Date.now()}`,
);

try {
await pw.request.put(`/api/dashboards/${id}`, {
data: {
layoutJson: {
version: 2,
pages: [
page("p1", "One", 1),
page("p2", "Two", 2),
page("p3", "Three", 3),
],
settings: {
autoRefresh: true,
refreshIntervalSeconds: REFRESH_SECONDS,
},
},
},
});

// Count every widget query the browser issues, regardless of page.
let queries = 0;
pw.on("request", (req) => {
if (req.url().includes("/api/query") && req.method() === "POST") {
queries += 1;
}
});

await pw.goto(`/${id}`);
await expect(
pw.locator("[data-testid='widget-card']").first(),
).toBeVisible({
timeout: 20_000,
});
await expect(pw.locator("table").first()).toBeVisible({
timeout: 20_000,
});

// ── Baseline: only page 1 has ever been opened ────────────────────
queries = 0;
await pw.waitForTimeout(SAMPLE_MS);
const baseline = queries;

// Auto-refresh must actually be running, or this test proves nothing.
expect(
baseline,
"expected auto-refresh to issue queries during the baseline window",
).toBeGreaterThan(0);

// ── Tour the other pages, then come back to page 1 ────────────────
for (const title of ["Two", "Three", "One"]) {
await pw.getByRole("tab", { name: title }).click();
await pw.waitForTimeout(500);
}
await expect(pw.locator("table").first()).toBeVisible({
timeout: 20_000,
});

// ── Same visible view, same window, same count ────────────────────
queries = 0;
await pw.waitForTimeout(SAMPLE_MS);
const afterTour = queries;

// Before the fix this was ~3x baseline with three pages mounted. Allow
// one extra tick of slack for timer alignment rather than demanding
// equality, but nothing close to a second page's worth.
expect(
afterTour,
`hidden pages are still polling: ${afterTour} queries after touring vs ${baseline} on a fresh load`,
).toBeLessThanOrEqual(baseline + 1);
} finally {
await cleanup();
}
});
});
Loading
Loading