Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
ec704b6
feat(component): accessibility improvements — ARIA, keyboard nav, con…
alfredorubin96 Mar 23, 2026
d6305d8
fix(component,app): bracket wrapping in code editor + radar chart scale
alfredorubin96 Mar 23, 2026
5c166f6
chore: add Chart Catalog seed dashboard with full feature showcase
alfredorubin96 Mar 23, 2026
6dd50c9
fix(component): resolve nested button hydration error in CrossFilterTag
alfredorubin96 Mar 23, 2026
f31270f
chore: expand Chart Catalog to cover ALL chart options comprehensively
alfredorubin96 Mar 23, 2026
943834a
chore: add Map Chart page with geo data (filming locations + birthpla…
alfredorubin96 Mar 23, 2026
b7513f0
fix: markdown newlines and iframe URL in Chart Catalog seed
alfredorubin96 Mar 23, 2026
89dbb1d
fix: markdown table rendering, graph performance, and query typo
alfredorubin96 Mar 23, 2026
57b05f0
fix(component): prevent graph chart nodes clumping on initial render
alfredorubin96 Mar 23, 2026
bbea43b
feat(component): add number formatting to single value and tooltips
alfredorubin96 Mar 22, 2026
8e4a745
fix(ci): add missing ECharts component mocks for CI
alfredorubin96 Mar 23, 2026
3b02566
fix: aria description merge order and dependency array
alfredorubin96 Mar 24, 2026
357b181
fix: radar max fallback, seed data shape, and markdown ReDoS
alfredorubin96 Mar 24, 2026
1c04d9c
fix(e2e): use keyboard fallback when CM6 editor reports readonly
alfredorubin96 Mar 24, 2026
33fcfe1
fix: resolve TypeScript compilation errors
alfredorubin96 Mar 24, 2026
8592737
Merge remote-tracking branch 'origin/fix/editor-brackets-radar-scale'…
alfredorubin96 Mar 24, 2026
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
7 changes: 4 additions & 3 deletions app/e2e/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,9 @@ export async function typeInEditor(
return;
}

// Strategy 2: Keyboard fallback (for environments where cmView is not accessible)
if (dispatched === "no-view") {
// Strategy 2: Keyboard fallback (for environments where cmView is not accessible
// or when the view is temporarily readonly during initialization)
if (dispatched === "no-view" || dispatched === "readonly") {
await expect(cm).toHaveAttribute("contenteditable", "true", { timeout: 2_000 });
await cm.click();
await page.keyboard.press("ControlOrMeta+a");
Expand All @@ -162,7 +163,7 @@ export async function typeInEditor(
return;
}

// Retry-worthy states: no-editor, readonly, dispatch-failed
// Retry-worthy states: no-editor, dispatch-failed
throw new Error(`CM6 dispatch returned "${dispatched}" — retrying`);
}).toPass({ timeout: 20_000 });
}
Expand Down
122 changes: 119 additions & 3 deletions app/src/lib/__tests__/chart-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1491,21 +1491,137 @@ describe("radar transform", () => {
expect(result.indicators[0].name).toBe("X");
});

it("auto-scales max from data when max column is missing", () => {
it("auto-scales max from data when max column is missing (single indicator)", () => {
const data = [{ indicator: "Speed", value: 80 }];
const result = transform(data) as { indicators: Array<{ name: string; max: number }>; series: unknown[] };
// 80 * 1.1 = 88, ceil → 88
expect(result.indicators[0].max).toBe(88);
});

it("handles flat tabular data without indicator column (uses column names as indicators)", () => {
it("uses global max across all indicators for relative comparison", () => {
const data = [
{ indicator: "ACTED_IN", value: 172 },
{ indicator: "PRODUCED", value: 15 },
{ indicator: "DIRECTED", value: 44 },
{ indicator: "WROTE", value: 10 },
{ indicator: "REVIEWED", value: 9 },
];
const result = transform(data) as { indicators: Array<{ name: string; max: number }>; series: Array<{ values: number[] }> };
// Global max: ceil(172 * 1.1) = 190
const globalMax = Math.ceil(172 * 1.1);
expect(result.indicators).toHaveLength(5);
// All indicators should share the same max
for (const ind of result.indicators) {
expect(ind.max).toBe(globalMax);
}
// The shape should NOT be uniform — values differ significantly
const values = result.series[0].values;
expect(values[0]).toBe(172); // ACTED_IN
expect(values[4]).toBe(9); // REVIEWED
});

it("preserves explicit max column values when provided", () => {
const data = [
{ indicator: "Speed", value: 80, max: 200 },
{ indicator: "Strength", value: 40, max: 150 },
];
const result = transform(data) as { indicators: Array<{ name: string; max: number }> };
expect(result.indicators[0].max).toBe(200);
expect(result.indicators[1].max).toBe(150);
});

it("uses global max for wide-format tabular data", () => {
const data = [{ Speed: 80, Strength: 60, Agility: 90 }];
const result = transform(data) as { indicators: Array<{ name: string }>; series: Array<{ values: number[] }> };
const result = transform(data) as { indicators: Array<{ name: string; max: number }>; series: Array<{ values: number[] }> };
// Global max: ceil(90 * 1.1) = 99
const globalMax = Math.ceil(90 * 1.1);
for (const ind of result.indicators) {
expect(ind.max).toBe(globalMax);
}
expect(result.indicators.map((i) => i.name)).toContain("Speed");
expect(result.indicators.map((i) => i.name)).toContain("Strength");
expect(result.series[0].values).toHaveLength(3);
});

it("falls back to globalMax when max column contains null/undefined/NaN", () => {
// When the max column exists but values are invalid (null/NaN/0),
// indicators should use globalMax instead of treating 0 or NaN as explicit.
const data = [
{ indicator: "Speed", value: 80, max: null },
{ indicator: "Strength", value: 60, max: undefined },
{ indicator: "Agility", value: 90, max: NaN },
];
const result = transform(data) as { indicators: Array<{ name: string; max: number }> };
// globalMax: ceil(90 * 1.1) = 99
const globalMax = Math.ceil(90 * 1.1);
for (const ind of result.indicators) {
expect(ind.max).toBe(globalMax);
}
});

it("falls back to globalMax when max column value is 0", () => {
const data = [
{ indicator: "Speed", value: 50, max: 0 },
{ indicator: "Strength", value: 30, max: 0 },
];
const result = transform(data) as { indicators: Array<{ name: string; max: number }> };
// 0 is not a valid explicit max (not > 0), so globalMax is used
const globalMax = Math.ceil(50 * 1.1);
for (const ind of result.indicators) {
expect(ind.max).toBe(globalMax);
}
});

it("falls back to globalMax when max column value is negative", () => {
const data = [
{ indicator: "Speed", value: 50, max: -100 },
{ indicator: "Strength", value: 30, max: -50 },
];
const result = transform(data) as { indicators: Array<{ name: string; max: number }> };
const globalMax = Math.ceil(50 * 1.1);
for (const ind of result.indicators) {
expect(ind.max).toBe(globalMax);
}
});

it("mixes explicit and fallback max when some indicators have valid max", () => {
const data = [
{ indicator: "Speed", value: 80, max: 200 },
{ indicator: "Strength", value: 60, max: null },
{ indicator: "Agility", value: 90, max: 150 },
];
const result = transform(data) as { indicators: Array<{ name: string; max: number }> };
const globalMax = Math.ceil(90 * 1.1);
// Speed and Agility have valid explicit max; Strength falls back to globalMax
expect(result.indicators[0].max).toBe(200); // Speed — explicit
expect(result.indicators[1].max).toBe(globalMax); // Strength — fallback
expect(result.indicators[2].max).toBe(150); // Agility — explicit
});

it("falls back to globalMax when max column contains non-numeric strings", () => {
const data = [
{ indicator: "Speed", value: 80, max: "not-a-number" },
{ indicator: "Strength", value: 60, max: "" },
];
const result = transform(data) as { indicators: Array<{ name: string; max: number }> };
const globalMax = Math.ceil(80 * 1.1);
for (const ind of result.indicators) {
expect(ind.max).toBe(globalMax);
}
});

it("falls back to globalMax when max column contains Infinity", () => {
const data = [
{ indicator: "Speed", value: 80, max: Infinity },
{ indicator: "Strength", value: 60, max: -Infinity },
];
const result = transform(data) as { indicators: Array<{ name: string; max: number }> };
const globalMax = Math.ceil(80 * 1.1);
for (const ind of result.indicators) {
expect(ind.max).toBe(globalMax);
}
});

it("transformWithMapping returns same result as transform", () => {
const data = [{ indicator: "Speed", value: 80, max: 100 }];
const result = chartRegistry.radar.transformWithMapping(data, {});
Expand Down
23 changes: 13 additions & 10 deletions app/src/lib/chart-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,21 +494,23 @@ function transformToRadarData(data: unknown): unknown {
const serName = seriesKey ? String(normalizeValue(r[seriesKey]) ?? "Default") : "Default";

if (maxKey) {
const explicitMax = Number(r[maxKey]) || 100;
if (!indicatorExplicitMax.has(indName)) indicatorExplicitMax.set(indName, explicitMax);
const explicitMax = Number(r[maxKey]);
if (Number.isFinite(explicitMax) && explicitMax > 0 && !indicatorExplicitMax.has(indName)) {
indicatorExplicitMax.set(indName, explicitMax);
}
Comment on lines +497 to +500

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Make explicit radar maxima deterministic across repeated indicators.

The new !indicatorExplicitMax.has(indName) check means the first valid max wins. In long-format radar data, the same indicator usually appears once per series, so conflicting valid maxima now make the rendered axis depend on row order.

Suggested fix
       if (maxKey) {
         const explicitMax = Number(r[maxKey]);
-        if (Number.isFinite(explicitMax) && explicitMax > 0 && !indicatorExplicitMax.has(indName)) {
-          indicatorExplicitMax.set(indName, explicitMax);
+        if (Number.isFinite(explicitMax) && explicitMax > 0) {
+          indicatorExplicitMax.set(
+            indName,
+            Math.max(indicatorExplicitMax.get(indName) ?? 0, explicitMax),
+          );
         }
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/lib/chart-registry.ts` around lines 497 - 500, The code currently
keeps the first seen valid explicitMax for an indicator (using
indicatorExplicitMax.has(indName)), causing non-deterministic axes when multiple
rows provide different valid maxima; change the logic in the block that reads
explicitMax from r[maxKey] so that for a valid Number.isFinite(explicitMax) &&
explicitMax > 0 you either set the map entry when absent or update it
deterministically by taking the maximum of the existing value and explicitMax
(e.g., read existing = indicatorExplicitMax.get(indName) and
indicatorExplicitMax.set(indName, existing ? Math.max(existing, explicitMax) :
explicitMax)), keeping the same validation checks around explicitMax and using
the same symbols indName, explicitMax, r[maxKey], and indicatorExplicitMax.

}
indicatorMaxFromData.set(indName, Math.max(indicatorMaxFromData.get(indName) ?? 0, val));
if (!seriesMap.has(serName)) seriesMap.set(serName, new Map());
seriesMap.get(serName)!.set(indName, val);
}

// Use explicit max if provided, otherwise auto-scale from observed values (+10% headroom)
// Use explicit max if provided, otherwise use a single global max across all
// indicators so relative magnitudes are visible (e.g. 172 vs 9).
const indicatorEntries = Array.from(indicatorMaxFromData.keys());
const globalMax = Math.ceil(Math.max(...indicatorMaxFromData.values()) * 1.1) || 100;
const indicators = indicatorEntries.map((name) => ({
name,
max: maxKey && indicatorExplicitMax.has(name)
? indicatorExplicitMax.get(name)!
: Math.ceil((indicatorMaxFromData.get(name) ?? 100) * 1.1) || 100,
max: indicatorExplicitMax.get(name) ?? globalMax,
}));
const series = Array.from(seriesMap.entries()).map(([name, valMap]) => ({
name,
Expand All @@ -519,17 +521,18 @@ function transformToRadarData(data: unknown): unknown {
}

// Wide-format: each column is an indicator, each row is a series
// Auto-scale max from observed values per column (+10% headroom)
const maxPerCol = new Map<string, number>();
// Use a single global max so all axes share the same scale
let wideGlobalMax = 0;
for (const r of records) {
for (const k of keys) {
const v = Number(r[k]) || 0;
maxPerCol.set(k, Math.max(maxPerCol.get(k) ?? 0, v));
if (v > wideGlobalMax) wideGlobalMax = v;
}
}
const wideMax = Math.ceil(wideGlobalMax * 1.1) || 100;
const indicators = keys.map((k) => ({
name: k,
max: Math.ceil((maxPerCol.get(k) ?? 100) * 1.1) || 100,
max: wideMax,
}));
const series = records.map((r, i) => ({
name: String(i + 1),
Expand Down
2 changes: 2 additions & 0 deletions component/src/charts/__tests__/base-chart.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ vi.mock("echarts/components", () => ({
DataZoomComponent: vi.fn(),
AriaComponent: vi.fn(),
RadarComponent: vi.fn(),
MarkLineComponent: vi.fn(),
GraphicComponent: vi.fn(),
}));

describe("BaseChart", () => {
Expand Down
96 changes: 96 additions & 0 deletions component/src/charts/__tests__/format-number.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import { describe, it, expect } from "vitest";
import { formatNumber, buildTooltipFormatter } from "../chart-utils";

describe("formatNumber", () => {
it("returns plain number by default", () => {
expect(formatNumber(1234)).toBe("1234");
});

it("respects decimalPlaces", () => {
expect(formatNumber(3.14159, { decimalPlaces: 2 })).toBe("3.14");
});

it("pads with zeros when decimalPlaces exceeds precision", () => {
expect(formatNumber(5, { decimalPlaces: 2 })).toBe("5.00");
});

it("applies comma formatting", () => {
expect(formatNumber(1234567, { numberFormat: "comma" })).toBe("1,234,567");
});

it("applies comma formatting with decimalPlaces", () => {
expect(formatNumber(1234567.891, { numberFormat: "comma", decimalPlaces: 2 })).toBe("1,234,567.89");
});

it("applies compact notation", () => {
const result = formatNumber(1500000, { numberFormat: "compact" });
expect(result).toMatch(/1\.5M/i);
});

it("applies compact notation with decimalPlaces", () => {
const result = formatNumber(1234, { numberFormat: "compact", decimalPlaces: 1 });
expect(result).toMatch(/1\.2K/i);
});

it("applies percent format", () => {
expect(formatNumber(75, { numberFormat: "percent" })).toBe("75%");
});

it("applies percent format with decimalPlaces", () => {
expect(formatNumber(75.678, { numberFormat: "percent", decimalPlaces: 1 })).toBe("75.7%");
});

it("adds prefix", () => {
expect(formatNumber(100, { prefix: "$" })).toBe("$100");
});

it("adds suffix", () => {
expect(formatNumber(100, { suffix: " items" })).toBe("100 items");
});

it("combines prefix, suffix, decimalPlaces, and comma", () => {
expect(formatNumber(9876.5, { prefix: "$", suffix: "M", numberFormat: "comma", decimalPlaces: 1 })).toBe("$9,876.5M");
});

it("handles zero", () => {
expect(formatNumber(0, { decimalPlaces: 2 })).toBe("0.00");
});

it("handles negative numbers", () => {
expect(formatNumber(-42.567, { decimalPlaces: 1 })).toBe("-42.6");
});

it("returns string values unchanged", () => {
expect(formatNumber("N/A" as unknown as number)).toBe("N/A");
});
});

describe("buildTooltipFormatter", () => {
it("returns a function", () => {
const formatter = buildTooltipFormatter({});
expect(typeof formatter).toBe("function");
});

it("formats a single value with config", () => {
const formatter = buildTooltipFormatter({ decimalPlaces: 1, prefix: "$" });
// ECharts tooltip params shape for axis trigger
const result = formatter({
seriesName: "Revenue",
value: 1234.56,
name: "Jan",
marker: '<span style="color:#3b82f6">●</span>',
});
expect(result).toContain("$1,234.6");
expect(result).toContain("Revenue");
});

it("handles array params (axis trigger with multiple series)", () => {
const formatter = buildTooltipFormatter({ decimalPlaces: 0 });
const result = formatter([
{ seriesName: "A", value: 100.7, name: "Jan", marker: "●" },
{ seriesName: "B", value: 200.3, name: "Jan", marker: "●" },
]);
expect(result).toContain("101");
expect(result).toContain("200");
});
});
Loading
Loading