Skip to content

Commit f0e769a

Browse files
gkaclaude
gka
and
claude
authored
fix(Regression): preserve time scale and activate fx/fy faceting (#549)
## Summary - **Time scale broken by RegressionY/X**: `Regression.svelte` was calling `toOutputX(value, plot.scales[independent].type)` to decide whether to convert epoch-ms numbers back to `Date` objects for the inner `Line` mark. On the first render pass the scale type isn't resolved yet (`"linear"`), so numbers were emitted. Scale inference then saw a mix of `Date` values (from the user's marks) and numbers (from Regression's `Line`) and permanently fell back to `"linear"`, destroying nice time-axis ticks. **Fix**: derive temporality from `filteredData[0]` (`instanceof Date`) instead of the scale type, breaking the circular dependency. - **fx/fy faceting not activated**: the outer `<Mark type="regression">` in `RegressionY`/`RegressionX` had no `data` and no `fx`/`fy` props, and the inner `<Line>` explicitly set `fx: null, fy: null`. Neither contributed to the fx/fy scale domain. `FacetGrid` checks `plot.scales.fx.domain.length > 0` to decide whether to split into facet cells — with an empty domain it never did. **Fix**: pass `{data}`, `fx={options.fx}`, and `fy={options.fy}` to the outer `<Mark>` in both `RegressionY` and `RegressionX`. ## Test plan - [ ] `pnpm test` — all 834 tests pass (added regression tests for both bugs) - [ ] New test: `preserves time scale when x channel contains Date values` — verifies regression path is valid and NaN-free on a time x-axis - [ ] New test: `activates fx faceting when fx channel is set` — verifies two `g.facet` elements are created and each contains a regression line 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent abb4594 commit f0e769a

File tree

5 files changed

+113
-9
lines changed

5 files changed

+113
-9
lines changed

‎src/lib/marks/RegressionX.svelte‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
);
3535
</script>
3636

37-
<Mark type="regression">
37+
<Mark type="regression" {data} fx={options.fx} fy={options.fy}>
3838
{#each groups as group, g (g)}
3939
<Regression data={group as any} dependent="x" {...options as any} />
4040
{/each}

‎src/lib/marks/RegressionY.svelte‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
);
3434
</script>
3535

36-
<Mark type="regression">
36+
<Mark type="regression" {data} fx={options.fx} fy={options.fy}>
3737
{#each groups as group, i (i)}
3838
<Regression data={group as any} dependent="y" {...options as any} />
3939
{/each}

‎src/lib/marks/helpers/Regression.svelte‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
regressionLoess
4444
} from '../../regression/index.js';
4545
import { resolveChannel } from '../../helpers/resolve.js';
46-
import { isTemporalScale } from '../../helpers/typeChecks.js';
4746
import { confidenceInterval } from '../../helpers/math.js';
4847
import callWithProps from '../../helpers/callWithProps.js';
4948
import type { DataRecord, FacetContext, RawValue } from 'svelteplot/types/index.js';
@@ -93,8 +92,12 @@
9392
}
9493
9594
// Convert generated points back to Date for time scales so downstream marks render correctly.
96-
function toOutputX(value: number, scaleType: string): RawValue {
97-
return isTemporalScale(scaleType) ? new Date(value) : value;
95+
// Takes a boolean instead of the scale type to avoid a circular dependency: if we used
96+
// plot.scales[independent].type, the Line mark would register numeric __x values during the
97+
// first render pass (before the scale type is resolved), causing scale inference to see a mix
98+
// of Dates and numbers and fall back to 'linear' permanently.
99+
function toOutputX(value: number, isTemporal: boolean): RawValue {
100+
return isTemporal ? new Date(value) : value;
98101
}
99102
100103
function makeTicks(domain: [number, number], count = 40): number[] {
@@ -129,6 +132,14 @@
129132
130133
const regressionFn = $derived(maybeRegression(type));
131134
135+
// Detect temporality from the raw data rather than from the computed scale type to avoid a
136+
// circular dependency that would cause the scale type to be permanently inferred as 'linear'.
137+
// Scan for the first row that resolves to a non-null independent value — checking only
138+
// filteredData[0] would give false negatives when the leading row has a missing/null value.
139+
const independentIsDate = $derived(
140+
filteredData.some((d) => resolveChannel(independent, d, options as any) instanceof Date)
141+
);
142+
132143
// Build a clean numeric input set for regression fitting, dropping invalid rows early.
133144
const regressionInput = $derived(
134145
filteredData
@@ -193,18 +204,18 @@
193204
// Prefer batch prediction when supported, then per-point predict, then raw curve output.
194205
if (typeof regression.predictMany === 'function') {
195206
return regression.predictMany(regrPoints).map((__y, i) => ({
196-
__x: toOutputX(regrPoints[i], plot.scales[independent].type),
207+
__x: toOutputX(regrPoints[i], independentIsDate),
197208
__y
198209
}));
199210
}
200211
if (typeof regression.predict === 'function') {
201212
return regrPoints.map((point) => ({
202-
__x: toOutputX(point, plot.scales[independent].type),
213+
__x: toOutputX(point, independentIsDate),
203214
__y: regression.predict!(point)
204215
}));
205216
}
206217
return regression.map(([__x, __y]) => ({
207-
__x: toOutputX(__x, plot.scales[independent].type),
218+
__x: toOutputX(__x, independentIsDate),
208219
__y
209220
}));
210221
});
@@ -230,7 +241,7 @@
230241
return regrPoints.map((x) => {
231242
const { x: __x, left, right } = confBandGen(x);
232243
return {
233-
__x: toOutputX(__x, plot.scales[independent].type),
244+
__x: toOutputX(__x, independentIsDate),
234245
__y1: left,
235246
__y2: right
236247
};

‎src/tests/regressionY.facet.test.svelte‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<script lang="ts">
2+
import { RegressionY, Plot } from '$lib/index.js';
3+
import type { ComponentProps } from 'svelte';
4+
let args: ComponentProps<typeof RegressionY> = $props();
5+
</script>
6+
7+
<Plot width={200} height={100} axes={false}>
8+
<RegressionY {...args} />
9+
</Plot>

‎src/tests/regressionY.test.svelte.ts‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { render } from '@testing-library/svelte';
33
// @ts-ignore - tsc in lint:types does not see default export for .svelte test component
44
// @ts-ignore - svelte-check errors on .svelte imports, tsc does not
55
import RegressionYTest from './regressionY.test.svelte';
6+
// @ts-ignore
7+
import RegressionYFacetTest from './regressionY.facet.test.svelte';
68

79
const linearData = [
810
{ x: 1, y: 2 },
@@ -184,6 +186,88 @@ describe('RegressionY mark', () => {
184186
expect(paths.length).toBeGreaterThanOrEqual(1);
185187
});
186188

189+
it('preserves time scale when x channel contains Date values', () => {
190+
const start = new Date('2024-01-01');
191+
const timeData = Array.from({ length: 5 }, (_, i) => ({
192+
x: new Date(start.getTime() + i * 30 * 24 * 60 * 60 * 1000),
193+
y: i * 2 + Math.random()
194+
}));
195+
196+
const { container } = render(RegressionYTest, {
197+
props: {
198+
data: timeData,
199+
x: 'x',
200+
y: 'y'
201+
}
202+
});
203+
204+
// Regression line should render — if scale type wrongly becomes 'linear', Date values
205+
// fail to map and the path is empty or missing.
206+
const paths = container.querySelectorAll('g[class*="regression-x"] g.lines path');
207+
expect(paths.length).toBeGreaterThanOrEqual(1);
208+
const d = paths[0].getAttribute('d');
209+
expect(d).toBeTruthy();
210+
expect(d).toMatch(/^M/);
211+
// The path should contain multiple numeric segments (not NaN/undefined positions).
212+
expect(d).not.toContain('NaN');
213+
});
214+
215+
it('preserves time scale when first row has null x value', () => {
216+
const start = new Date('2024-01-01');
217+
const timeDataWithLeadingNull = [
218+
{ x: null, y: null }, // leading invalid row
219+
...Array.from({ length: 5 }, (_, i) => ({
220+
x: new Date(start.getTime() + i * 30 * 24 * 60 * 60 * 1000),
221+
y: i * 2 + 1
222+
}))
223+
];
224+
225+
const { container } = render(RegressionYTest, {
226+
props: {
227+
data: timeDataWithLeadingNull,
228+
x: 'x',
229+
y: 'y'
230+
}
231+
});
232+
233+
const paths = container.querySelectorAll('g[class*="regression-x"] g.lines path');
234+
expect(paths.length).toBeGreaterThanOrEqual(1);
235+
const d = paths[0].getAttribute('d');
236+
expect(d).toBeTruthy();
237+
expect(d).toMatch(/^M/);
238+
expect(d).not.toContain('NaN');
239+
});
240+
241+
it('activates fx faceting when fx channel is set', () => {
242+
const facetedData = [
243+
{ x: 1, y: 2, cat: 'A' },
244+
{ x: 2, y: 4, cat: 'A' },
245+
{ x: 3, y: 6, cat: 'A' },
246+
{ x: 1, y: 10, cat: 'B' },
247+
{ x: 2, y: 8, cat: 'B' },
248+
{ x: 3, y: 6, cat: 'B' }
249+
];
250+
251+
const { container } = render(RegressionYFacetTest, {
252+
props: {
253+
data: facetedData,
254+
x: 'x',
255+
y: 'y',
256+
fx: 'cat'
257+
}
258+
});
259+
260+
// With fx faceting active, there should be two facet groups
261+
const facets = container.querySelectorAll('g.facet');
262+
expect(facets.length).toBe(2);
263+
264+
// Each facet should contain a regression line
265+
for (const facet of facets) {
266+
const paths = facet.querySelectorAll('g[class*="regression-x"] g.lines path');
267+
expect(paths.length).toBeGreaterThanOrEqual(1);
268+
}
269+
});
270+
187271
it('handles empty data', () => {
188272
const { container } = render(RegressionYTest, {
189273
props: {

0 commit comments

Comments
 (0)