Update LayerChart to 1.0.1 fixing overlapping high point series highlight and improved LineChart perf (up to 33%) #451
Conversation
…ight and improved LineChart perf (up to 33%)
WalkthroughThis pull request updates the Changes
Possibly related PRs
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsGitHub Actions and Pipeline Checks: Resource not accessible by integration - https://docs.github.com/rest/actions/workflow-runs#list-workflow-runs-for-a-repository. Please grant the required permissions to the CodeRabbit GitHub App under the organization or repository settings. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…ure chart change)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
frontend/src/routes/[conf]/[name]/observability/summary/+page.svelte(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: frontend_tests
- GitHub Check: enforce_triggered_workflows
| onbrushend={(detail: { xDomain?: DomainType }) => { | ||
| xDomain = detail.xDomain; | ||
| }} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Type safety concern with direct assignment.
Direct assignment of potentially undefined detail.xDomain may cause inconsistent behavior.
🏁 Script executed:
#!/bin/bash
# Check how xDomain is handled elsewhere in the codebase
rg "xDomain\s*=\s*\w+\.xDomain" --type svelte
# Check if null checks were previously used with xDomain assignment
rg "xDomain\s*=.*\?\?" --type svelteLength of output: 148
Below is a revised shell script to re-run the search using a glob pattern (since Svelte isn’t recognized as a file type) so we can verify how xDomain is assigned throughout the code:
Re-verify xDomain usage for type safety.
Please check the output of these commands to ensure that assignments, especially of potentially undefined values, are handled safely.
🏁 Script executed:
[run_scripts]
#!/bin/bash
# Re-run search in .svelte files for xDomain assignments.
rg "xDomain\s*=\s*\w+\.xDomain" --glob "*.svelte"
echo "------"
# Look for any null-coalescing checks in xDomain assignments.
rg "xDomain\s*=.*\?\?" --glob "*.svelte"Length of output: 648
Action Required: Add safeguard for xDomain assignment.
Direct assignment of detail.xDomain in both summary and drift pages may cause type-safety issues. Consider adding a fallback or explicit null/undefined check (e.g., using ?? defaultValue) to ensure a valid value.
- Files affected:
frontend/src/routes/[conf]/[name]/observability/summary/+page.svelte(line 74-76)frontend/src/routes/[conf]/[name]/observability/drift/+page.svelte
Summary
Fixes close/overlapping series points not properly triggering highlight. Video after update:
https://github.com/user-attachments/assets/fc4d8631-1525-4196-81b7-2092e47c58f1
Release also improves performance of Spline/LineCharts by up to 33% from my testing. More work to make the summary/distributions page initial render much faster, but moving in the right direction.
This PR also fixes distribution zoom resetting on chart click (similar to drift/feature chart change in #445)
Checklist
Summary by CodeRabbit
PercentileLineChartcomponent to ensurexDomainis assigned directly from event details.