Skip to content

Commit 4422081

Browse files
authored
fix: respect per-file metric config (#49)
* fix: respect per-file metric config * test: ensure custom metrics survive global updates
1 parent 9b8c680 commit 4422081

File tree

4 files changed

+167
-17
lines changed

4 files changed

+167
-17
lines changed

src/App.jsx

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -171,17 +171,25 @@ function App() {
171171
const handleGlobalParsingConfigChange = useCallback((newConfig) => {
172172
setGlobalParsingConfig(newConfig);
173173

174-
// Sync parsing config to all files
175-
setUploadedFiles(prev => prev.map(file => ({
176-
...file,
177-
config: {
178-
...file.config,
179-
metrics: newConfig.metrics.map(m => ({ ...m })),
180-
useStepKeyword: newConfig.useStepKeyword,
181-
stepKeyword: newConfig.stepKeyword
182-
}
183-
})));
184-
}, []);
174+
// Sync parsing config to files that still use the global metrics
175+
setUploadedFiles(prev => prev.map(file => {
176+
const fileConfig = file.config || {};
177+
const usesGlobalMetrics = !fileConfig.metrics ||
178+
JSON.stringify(fileConfig.metrics) === JSON.stringify(globalParsingConfig.metrics);
179+
180+
return {
181+
...file,
182+
config: {
183+
...fileConfig,
184+
...(usesGlobalMetrics && {
185+
metrics: newConfig.metrics.map(m => ({ ...m }))
186+
}),
187+
useStepKeyword: newConfig.useStepKeyword,
188+
stepKeyword: newConfig.stepKeyword
189+
}
190+
};
191+
}));
192+
}, [globalParsingConfig]);
185193

186194
// Reset configuration
187195
const handleResetConfig = useCallback(() => {

src/components/ChartContainer.jsx

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -260,12 +260,13 @@ export default function ChartContainer({
260260
return results;
261261
};
262262

263-
metrics.forEach(metric => {
263+
metrics.forEach((metric, idx) => {
264+
const fileMetric = file.config?.metrics?.[idx] || metric;
264265
let points = [];
265-
if (metric.mode === 'keyword') {
266-
points = extractByKeyword(lines, metric.keyword);
267-
} else if (metric.regex) {
268-
const reg = new RegExp(metric.regex);
266+
if (fileMetric.mode === 'keyword') {
267+
points = extractByKeyword(lines, fileMetric.keyword);
268+
} else if (fileMetric.regex) {
269+
const reg = new RegExp(fileMetric.regex);
269270
lines.forEach(line => {
270271
reg.lastIndex = 0;
271272
const m = reg.exec(line);
@@ -278,7 +279,20 @@ export default function ChartContainer({
278279
}
279280
});
280281
}
281-
metricsData[metric.name || metric.keyword] = points;
282+
283+
let key = '';
284+
if (metric.name && metric.name.trim()) {
285+
key = metric.name.trim();
286+
} else if (metric.keyword) {
287+
key = metric.keyword.replace(/[:]/g, '').trim();
288+
} else if (metric.regex) {
289+
const sanitized = metric.regex.replace(/[^a-zA-Z0-9_]/g, '').trim();
290+
key = sanitized || `metric${idx + 1}`;
291+
} else {
292+
key = `metric${idx + 1}`;
293+
}
294+
295+
metricsData[key] = points;
282296
});
283297

284298
const range = file.config?.dataRange;

src/components/__tests__/ChartContainer.test.jsx

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,4 +175,38 @@ describe('ChartContainer', () => {
175175
opts.plugins.zoom.pan.onPanComplete({ chart: { scales: { x: { min: 0, max: 10 } } } });
176176
opts.plugins.zoom.zoom.onZoomComplete({ chart: { scales: { x: { min: 2, max: 4 } } } });
177177
});
178+
179+
it('uses per-file metric configuration when provided', () => {
180+
const onXRangeChange = vi.fn();
181+
const onMaxStepChange = vi.fn();
182+
const files = [
183+
{ name: 'a.log', enabled: true, content: 'loss: 1\nloss: 2' },
184+
{
185+
name: 'b.log',
186+
enabled: true,
187+
content: 'train_loss: 3\ntrain_loss: 4',
188+
config: { metrics: [{ mode: 'keyword', keyword: 'train_loss:' }] }
189+
}
190+
];
191+
const metrics = [{ name: 'loss', mode: 'keyword', keyword: 'loss:' }];
192+
193+
render(
194+
<ChartContainer
195+
files={files}
196+
metrics={metrics}
197+
compareMode="normal"
198+
onXRangeChange={onXRangeChange}
199+
onMaxStepChange={onMaxStepChange}
200+
/>
201+
);
202+
203+
const mainChart = [...__lineProps].reverse().find(p =>
204+
p.data.datasets && p.data.datasets.some(d => d.label === 'b')
205+
);
206+
const ds = mainChart.data.datasets.find(d => d.label === 'b');
207+
expect(ds.data).toEqual([
208+
{ x: 0, y: 3 },
209+
{ x: 1, y: 4 }
210+
]);
211+
});
178212
});
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import React from 'react';
2+
import { render, screen, within, waitFor } from '@testing-library/react';
3+
import userEvent from '@testing-library/user-event';
4+
import { describe, it, expect, beforeEach, vi } from 'vitest';
5+
import '@testing-library/jest-dom/vitest';
6+
import App from '../../App.jsx';
7+
import i18n from '../../i18n';
8+
9+
// Mock chart.js and react-chartjs-2 to avoid canvas requirements
10+
vi.mock('chart.js', () => {
11+
const Chart = {
12+
register: vi.fn(),
13+
defaults: { plugins: { legend: { labels: { generateLabels: vi.fn(() => []) } } } }
14+
};
15+
return {
16+
Chart,
17+
ChartJS: Chart,
18+
CategoryScale: {},
19+
LinearScale: {},
20+
PointElement: {},
21+
LineElement: {},
22+
Title: {},
23+
Tooltip: {},
24+
Legend: {},
25+
};
26+
});
27+
28+
vi.mock('react-chartjs-2', async () => {
29+
const React = await import('react');
30+
return {
31+
Line: React.forwardRef(() => <div data-testid="line-chart" />)
32+
};
33+
});
34+
35+
vi.mock('chartjs-plugin-zoom', () => ({ default: {} }));
36+
37+
function stubFileReader(result) {
38+
class FileReaderMock {
39+
constructor() {
40+
this.onload = null;
41+
}
42+
readAsText() {
43+
this.onload({ target: { result } });
44+
}
45+
}
46+
global.FileReader = FileReaderMock;
47+
}
48+
49+
describe('Global config override', () => {
50+
beforeEach(() => {
51+
localStorage.clear();
52+
});
53+
54+
it('retains file metric config after global change', async () => {
55+
stubFileReader('train_loss: 1');
56+
const user = userEvent.setup();
57+
58+
render(<App />);
59+
60+
const input = screen.getByLabelText(i18n.t('fileUpload.aria'));
61+
const file = new File(['train_loss: 1'], 'a.log', { type: 'text/plain' });
62+
await user.upload(input, file);
63+
64+
await screen.findByText('a.log');
65+
66+
const configBtn = screen.getByLabelText(i18n.t('fileList.config', { name: 'a.log' }));
67+
await user.click(configBtn);
68+
69+
const modal = screen.getByRole('dialog');
70+
const keywordInputs = within(modal).getAllByPlaceholderText('keyword');
71+
await user.clear(keywordInputs[0]);
72+
await user.type(keywordInputs[0], 'train_loss:');
73+
74+
const saveBtn = screen.getByText(i18n.t('configModal.save'));
75+
await user.click(saveBtn);
76+
77+
await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument());
78+
79+
// Update global config
80+
const globalKeyword = screen.getAllByPlaceholderText('keyword')[0];
81+
await user.clear(globalKeyword);
82+
await user.type(globalKeyword, 'val_loss:');
83+
expect(globalKeyword.value).toBe('val_loss:');
84+
85+
// Reopen file config to verify keyword remains
86+
const configBtn2 = screen.getByLabelText(i18n.t('fileList.config', { name: 'a.log' }));
87+
await user.click(configBtn2);
88+
89+
const modal2 = screen.getByRole('dialog');
90+
const updatedInputs = within(modal2).getAllByPlaceholderText('keyword');
91+
expect(updatedInputs[0].value).toBe('train_loss:');
92+
});
93+
});
94+

0 commit comments

Comments
 (0)