-
Notifications
You must be signed in to change notification settings - Fork 0
fix: improve chart precision and scaling #55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const parsedData = useMemo(() => { | ||
| const enabled = files.filter(f => f.enabled !== false); | ||
| return enabled.map(file => { | ||
| // Use pre-parsed data from worker | ||
| let metricsData = file.metricsData || {}; | ||
|
|
||
| const extractByKeyword = (linesArr, keyword) => { | ||
| const results = []; | ||
| const numberRegex = /[+-]?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?/; | ||
| linesArr.forEach(line => { | ||
| const idx = line.toLowerCase().indexOf(keyword.toLowerCase()); | ||
| if (idx !== -1) { | ||
| const after = line.substring(idx + keyword.length); | ||
| const match = after.match(numberRegex); | ||
| if (match) { | ||
| const v = parseFloat(match[0]); | ||
| if (!isNaN(v)) { | ||
| const step = extractStep(line); | ||
| results.push({ x: step !== null ? step : results.length, y: v }); | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| return results; | ||
| }; | ||
|
|
||
| metrics.forEach((metric, idx) => { | ||
| const fileMetric = file.config?.metrics?.[idx] || metric; | ||
| let points = []; | ||
| 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); | ||
| if (m && m[1]) { | ||
| const v = parseFloat(m[1]); | ||
| if (!isNaN(v)) { | ||
| const step = extractStep(line); | ||
| points.push({ x: step !== null ? step : points.length, y: v }); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| let key = ''; | ||
| if (metric.name && metric.name.trim()) { | ||
| key = metric.name.trim(); | ||
| } else if (metric.keyword) { | ||
| 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}`; | ||
| } | ||
| // Clone to avoid mutation during range application | ||
| metricsData = { ...metricsData }; | ||
|
|
||
| metricsData[key] = points; | ||
| }); | ||
| const stepCfg = { | ||
| enabled: file.config?.useStepKeyword, | ||
| keyword: file.config?.stepKeyword || 'step:' | ||
| }; | ||
|
|
||
| const range = file.config?.dataRange; | ||
| if (range && (range.start > 0 || range.end !== undefined)) { | ||
| const applyRange = data => { | ||
| if (data.length === 0) return data; | ||
| if (!data || data.length === 0) return data; | ||
| const start = Math.max(0, parseInt(range.start) || 0); | ||
| const end = range.end !== undefined ? parseInt(range.end) : data.length; | ||
| const endIndex = Math.min(data.length, end); | ||
| return data.slice(start, endIndex); | ||
| }; | ||
|
|
||
| // If not using step keyword, reindex x to 0, 1, 2... after slicing | ||
| // This matches original behavior where x-axis resets if we just treat lines as steps | ||
| const reindex = data => stepCfg.enabled ? data : data.map((p, idx) => ({ x: idx, y: p.y })); | ||
|
|
||
| Object.keys(metricsData).forEach(k => { | ||
| metricsData[k] = reindex(applyRange(metricsData[k])); | ||
| if (metricsData[k]) { | ||
| metricsData[k] = reindex(applyRange(metricsData[k])); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| return { ...file, metricsData }; | ||
| }); | ||
| }, [files, metrics]); | ||
| }, [files]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Persisted uploads never reparse after reload
Lines 217‑254 now assume that every file already carries metricsData and the parser never falls back to file.content. However, App.jsx still rehydrates uploads from localStorage with only { id, name, enabled, content, config } and there is no effect that re-sends those stored files through the worker. After a page refresh, each restored file has an empty metricsData, parsedData stays empty, and the charts never render again unless the user re‑uploads every file. The previous version parsed the content in this hook so persistence worked. Please trigger the worker for rehydrated files (or keep parsing content as a fallback) so persisted sessions actually display data.
Useful? React with 👍 / 👎.
Fixes decimal precision issues and improves Y-axis scaling.