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
30 changes: 19 additions & 11 deletions src/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -171,17 +171,25 @@ function App() {
const handleGlobalParsingConfigChange = useCallback((newConfig) => {
setGlobalParsingConfig(newConfig);

// Sync parsing config to all files
setUploadedFiles(prev => prev.map(file => ({
...file,
config: {
...file.config,
metrics: newConfig.metrics.map(m => ({ ...m })),
useStepKeyword: newConfig.useStepKeyword,
stepKeyword: newConfig.stepKeyword
}
})));
}, []);
// Sync parsing config to files that still use the global metrics
setUploadedFiles(prev => prev.map(file => {
const fileConfig = file.config || {};
const usesGlobalMetrics = !fileConfig.metrics ||
JSON.stringify(fileConfig.metrics) === JSON.stringify(globalParsingConfig.metrics);

return {
...file,
config: {
...fileConfig,
...(usesGlobalMetrics && {
metrics: newConfig.metrics.map(m => ({ ...m }))
}),
useStepKeyword: newConfig.useStepKeyword,
stepKeyword: newConfig.stepKeyword
}
};
}));
}, [globalParsingConfig]);

// Reset configuration
const handleResetConfig = useCallback(() => {
Expand Down
26 changes: 20 additions & 6 deletions src/components/ChartContainer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,13 @@ export default function ChartContainer({
return results;
};

metrics.forEach(metric => {
metrics.forEach((metric, idx) => {
const fileMetric = file.config?.metrics?.[idx] || metric;
let points = [];
if (metric.mode === 'keyword') {
points = extractByKeyword(lines, metric.keyword);
} else if (metric.regex) {
const reg = new RegExp(metric.regex);
if (fileMetric.mode === 'keyword') {
points = extractByKeyword(lines, fileMetric.keyword);
} else if (fileMetric.regex) {
const reg = new RegExp(fileMetric.regex);
lines.forEach(line => {
reg.lastIndex = 0;
const m = reg.exec(line);
Expand All @@ -278,7 +279,20 @@ export default function ChartContainer({
}
});
}
metricsData[metric.name || metric.keyword] = points;

let key = '';
if (metric.name && metric.name.trim()) {
key = metric.name.trim();
} else if (metric.keyword) {
Comment on lines +283 to +286

Choose a reason for hiding this comment

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

[P1] Align metric lookup with sanitized metric keys

The parsing loop now strips punctuation when storing metric data (key = metric.keyword.replace(/[::]/g, '').trim() and similar for regex). However, the chart still looks up datasets by the raw metric.name || metric.keyword string when building metricElements. For metrics without a name that contain punctuation (e.g. { mode: 'keyword', keyword: 'loss:' }), data are stored under "loss" but later retrieved using "loss:", so the rendered chart contains no series even though values were parsed. Apply the same normalization during lookup (or reuse the metricNames array) so metrics defined only by keyword/regex continue to display.

Useful? React with 👍 / 👎.

key = metric.keyword.replace(/[::]/g, '').trim();
} else if (metric.regex) {
const sanitized = metric.regex.replace(/[^a-zA-Z0-9_]/g, '').trim();
key = sanitized || `metric${idx + 1}`;
} else {
key = `metric${idx + 1}`;
}

metricsData[key] = points;
});

const range = file.config?.dataRange;
Expand Down
34 changes: 34 additions & 0 deletions src/components/__tests__/ChartContainer.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,4 +175,38 @@ describe('ChartContainer', () => {
opts.plugins.zoom.pan.onPanComplete({ chart: { scales: { x: { min: 0, max: 10 } } } });
opts.plugins.zoom.zoom.onZoomComplete({ chart: { scales: { x: { min: 2, max: 4 } } } });
});

it('uses per-file metric configuration when provided', () => {
const onXRangeChange = vi.fn();
const onMaxStepChange = vi.fn();
const files = [
{ name: 'a.log', enabled: true, content: 'loss: 1\nloss: 2' },
{
name: 'b.log',
enabled: true,
content: 'train_loss: 3\ntrain_loss: 4',
config: { metrics: [{ mode: 'keyword', keyword: 'train_loss:' }] }
}
];
const metrics = [{ name: 'loss', mode: 'keyword', keyword: 'loss:' }];

render(
<ChartContainer
files={files}
metrics={metrics}
compareMode="normal"
onXRangeChange={onXRangeChange}
onMaxStepChange={onMaxStepChange}
/>
);

const mainChart = [...__lineProps].reverse().find(p =>
p.data.datasets && p.data.datasets.some(d => d.label === 'b')
);
const ds = mainChart.data.datasets.find(d => d.label === 'b');
expect(ds.data).toEqual([
{ x: 0, y: 3 },
{ x: 1, y: 4 }
]);
});
});
94 changes: 94 additions & 0 deletions src/components/__tests__/GlobalConfigOverride.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import React from 'react';
import { render, screen, within, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { describe, it, expect, beforeEach, vi } from 'vitest';
import '@testing-library/jest-dom/vitest';
import App from '../../App.jsx';
import i18n from '../../i18n';

// Mock chart.js and react-chartjs-2 to avoid canvas requirements
vi.mock('chart.js', () => {
const Chart = {
register: vi.fn(),
defaults: { plugins: { legend: { labels: { generateLabels: vi.fn(() => []) } } } }
};
return {
Chart,
ChartJS: Chart,
CategoryScale: {},
LinearScale: {},
PointElement: {},
LineElement: {},
Title: {},
Tooltip: {},
Legend: {},
};
});

vi.mock('react-chartjs-2', async () => {
const React = await import('react');
return {
Line: React.forwardRef(() => <div data-testid="line-chart" />)
};
});

vi.mock('chartjs-plugin-zoom', () => ({ default: {} }));

function stubFileReader(result) {
class FileReaderMock {
constructor() {
this.onload = null;
}
readAsText() {
this.onload({ target: { result } });
}
}
global.FileReader = FileReaderMock;
}

describe('Global config override', () => {
beforeEach(() => {
localStorage.clear();
});

it('retains file metric config after global change', async () => {
stubFileReader('train_loss: 1');
const user = userEvent.setup();

render(<App />);

const input = screen.getByLabelText(i18n.t('fileUpload.aria'));
const file = new File(['train_loss: 1'], 'a.log', { type: 'text/plain' });
await user.upload(input, file);

await screen.findByText('a.log');

const configBtn = screen.getByLabelText(i18n.t('fileList.config', { name: 'a.log' }));
await user.click(configBtn);

const modal = screen.getByRole('dialog');
const keywordInputs = within(modal).getAllByPlaceholderText('keyword');
await user.clear(keywordInputs[0]);
await user.type(keywordInputs[0], 'train_loss:');

const saveBtn = screen.getByText(i18n.t('configModal.save'));
await user.click(saveBtn);

await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument());

// Update global config
const globalKeyword = screen.getAllByPlaceholderText('keyword')[0];
await user.clear(globalKeyword);
await user.type(globalKeyword, 'val_loss:');
expect(globalKeyword.value).toBe('val_loss:');

// Reopen file config to verify keyword remains
const configBtn2 = screen.getByLabelText(i18n.t('fileList.config', { name: 'a.log' }));
await user.click(configBtn2);

const modal2 = screen.getByRole('dialog');
const updatedInputs = within(modal2).getAllByPlaceholderText('keyword');
expect(updatedInputs[0].value).toBe('train_loss:');
});
});