feat: migrate dashboard charts from Plotly to Monster UI ApexCharts #199
Conversation
Complete migration of anomstack dashboard from Plotly.js to Monster UI's native ApexChart components while maintaining all existing functionality and improving user experience. Key changes: - Replace Plotly with ApexCharts across all chart types (regular, expanded, sparklines) - Implement dual y-axes with percentage-formatted score axis (0%-100%) - Add properly sized alert markers at 100% line for anomaly detection - Ensure metric value dots match blue line color for visual consistency - Fix JSON serialization issues with pandas data types - Add client-side percentage formatting via JavaScript - Improve chart dimensions and responsive design - Remove debug statements and optimize chart initialization - Maintain all interactive features (zoom, tooltips, themes, legends) Technical improvements: - Use native Monster UI ApexChart components instead of raw HTML injection - Eliminate circular imports and improve code organization - Reduce codebase complexity while preserving full functionality - Better integration with Monster UI design system 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe codebase transitions from using Plotly.js to ApexCharts for all dashboard chart rendering. This involves updating the dashboard app's JavaScript and CSS, refactoring chart creation logic to build ApexCharts configurations, and modifying routes to handle new chart parameters and output XML via Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant App (FastAPI)
participant ChartManager
participant ApexCharts
User->>Browser: Requests dashboard page
Browser->>App (FastAPI): HTTP request for charts
App (FastAPI)->>ChartManager: create_chart(...) with config
ChartManager->>App (FastAPI): Returns ApexChart config as XML
App (FastAPI)->>Browser: Sends HTML with <uk-chart> & config
Browser->>ApexCharts: Initializes charts on <uk-chart> via JS
ApexCharts-->>Browser: Renders chart in DOM
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR completely migrates the anomstack dashboard's charting system from Plotly.js to Monster UI's native ApexCharts components, maintaining all functionality while improving integration and visual consistency.
Key Changes:
- Replaced all Plotly chart generation with ApexCharts implementation in
ChartManagerclass - Added ApexCharts CDN and JavaScript initialization logic for proper chart rendering
- Updated chart rendering calls to pass configuration parameters and use
to_xml()for serialization
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| dashboard/app.py | Added ApexCharts CDN, custom styling for chart containers, and JavaScript initialization logic |
| dashboard/charts.py | Complete rewrite of ChartManager class methods to generate ApexChart components instead of Plotly HTML |
| dashboard/routes/batch.py | Updated chart method calls to pass configuration parameters and serialize charts with to_xml() |
| uk-chart { | ||
| display: block !important; | ||
| width: 100% !important; | ||
| min-height: 300px !important; | ||
| height: auto !important; | ||
| } | ||
| uk-chart .apexcharts-canvas { |
There was a problem hiding this comment.
The CSS selector 'uk-chart' targets a custom element but lacks specificity. Consider using a more specific selector like '.uk-chart' or 'uk-chart[data-chart]' to avoid potential conflicts with other elements.
| uk-chart { | |
| display: block !important; | |
| width: 100% !important; | |
| min-height: 300px !important; | |
| height: auto !important; | |
| } | |
| uk-chart .apexcharts-canvas { | |
| uk-chart[data-chart-initialized] { | |
| display: block !important; | |
| width: 100% !important; | |
| min-height: 300px !important; | |
| height: auto !important; | |
| } | |
| uk-chart[data-chart-initialized] .apexcharts-canvas { |
| # Convert timestamps to milliseconds for ApexCharts | ||
| timestamps = [int(pd.to_datetime(ts).timestamp() * 1000) for ts in df_metric["metric_timestamp"]] | ||
|
|
||
| # Prepare main metric data (ensure native Python types for JSON serialization) | ||
| value_data = [[int(ts), float(val)] for ts, val in zip(timestamps, df_metric["metric_value"])] | ||
| score_data = [[int(ts), float(val)] for ts, val in zip(timestamps, df_metric["metric_score"])] |
There was a problem hiding this comment.
The nested list comprehensions for data preparation could be optimized by using vectorized pandas operations or numpy arrays, especially for large datasets. Consider using df_metric[['metric_timestamp', 'metric_value']].values.tolist() after proper timestamp conversion.
| # Convert timestamps to milliseconds for ApexCharts | |
| timestamps = [int(pd.to_datetime(ts).timestamp() * 1000) for ts in df_metric["metric_timestamp"]] | |
| # Prepare main metric data (ensure native Python types for JSON serialization) | |
| value_data = [[int(ts), float(val)] for ts, val in zip(timestamps, df_metric["metric_value"])] | |
| score_data = [[int(ts), float(val)] for ts, val in zip(timestamps, df_metric["metric_score"])] | |
| # Convert timestamps to milliseconds for ApexCharts (vectorized) | |
| df_metric = df_metric.copy() | |
| df_metric["timestamp_ms"] = pd.to_datetime(df_metric["metric_timestamp"]).astype("int64") // 10**6 | |
| # Prepare main metric data (ensure native Python types for JSON serialization, vectorized) | |
| value_data = df_metric[["timestamp_ms", "metric_value"]].astype({"timestamp_ms": int, "metric_value": float}).values.tolist() | |
| score_data = df_metric[["timestamp_ms", "metric_score"]].astype({"timestamp_ms": int, "metric_score": float}).values.tolist() |
| "width": [line_width, line_width, 0, 0], # Line widths for each series | ||
| "dashArray": [0, 5, 0, 0] # Dash patterns (score line dashed) | ||
| }, | ||
| "markers": { | ||
| "size": [4 if show_markers else 0, 0, 4, 4], # Much smaller alert markers | ||
| "colors": [colors["primary"], "undefined", colors["alert"], colors["llmalert"]], # Value markers same as blue line | ||
| "strokeWidth": [1, 0, 1, 1], | ||
| "strokeColors": ["#ffffff", "#ffffff", "#ffffff", "#ffffff"], | ||
| "hover": { | ||
| "size": [6 if show_markers else 0, 0, 6, 6] # Smaller hover too |
There was a problem hiding this comment.
The hardcoded marker sizes array assumes a specific number of series. If the number of series changes (e.g., when alerts are added), this could cause index mismatches. Consider dynamically building this array based on the actual series count.
| "width": [line_width, line_width, 0, 0], # Line widths for each series | |
| "dashArray": [0, 5, 0, 0] # Dash patterns (score line dashed) | |
| }, | |
| "markers": { | |
| "size": [4 if show_markers else 0, 0, 4, 4], # Much smaller alert markers | |
| "colors": [colors["primary"], "undefined", colors["alert"], colors["llmalert"]], # Value markers same as blue line | |
| "strokeWidth": [1, 0, 1, 1], | |
| "strokeColors": ["#ffffff", "#ffffff", "#ffffff", "#ffffff"], | |
| "hover": { | |
| "size": [6 if show_markers else 0, 0, 6, 6] # Smaller hover too | |
| "width": stroke_widths, # Line widths for each series | |
| "dashArray": dash_arrays # Dash patterns (score line dashed) | |
| }, | |
| "markers": { | |
| "size": marker_sizes, # Much smaller alert markers | |
| "colors": marker_colors, # Value markers same as blue line | |
| "strokeWidth": marker_stroke_widths, | |
| "strokeColors": marker_stroke_colors, | |
| "hover": { | |
| "size": marker_hover_sizes # Smaller hover too |
| "height": 600, | ||
| "scale": 1 | ||
| "stroke": { | ||
| "width": [line_width, line_width, 0, 0], # Line widths for each series |
There was a problem hiding this comment.
Similar to marker sizes, the stroke width array is hardcoded for 4 series. This creates a brittle dependency on the exact number and order of series. Consider building this array dynamically based on the series configuration.
| "width": [line_width, line_width, 0, 0], # Line widths for each series | |
| "width": [ | |
| line_width if s.get("name", "").lower() in ["value", "score"] else 0 | |
| for s in series | |
| ], # Line widths for each series |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (3)
dashboard/routes/batch.py (2)
852-852: Fix incorrect button state logicThe positive button class should be secondary when feedback is negative, but the current condition will always evaluate to secondary.
- cls=(ButtonT.secondary if current_feedback == "negative" else ButtonT.secondary) + " p-2", + cls=(ButtonT.primary if current_feedback == "positive" else ButtonT.secondary) + " p-2",
876-876: Fix duplicate button state logic errorSame issue as above - the positive button in the table row update has incorrect conditional logic.
- cls=(ButtonT.secondary if current_feedback == "negative" else ButtonT.secondary) + " sm:p-2 p-1", + cls=(ButtonT.primary if current_feedback == "positive" else ButtonT.secondary) + " sm:p-2 p-1",dashboard/charts.py (1)
512-525: Remove unusedget_common_stylingmethodThe
get_common_stylinghelper is defined in dashboard/charts.py (lines 512–525) but isn’t referenced anywhere else in the codebase. It appears to be leftover from the old Plotly setup and can be safely removed to reduce dead code.• File: dashboard/charts.py
• Lines: 512–525Suggested diff:
--- a/dashboard/charts.py +++ b/dashboard/charts.py @@ -510,20 +510,6 @@ class ChartStyle: @staticmethod - def get_common_styling(colors: dict) -> tuple: - """Return common styling configurations.""" - common_font = dict(size=10, color=colors["text"]) - common_title_font = dict(size=12, color=colors["text"]) - common_grid = dict( - showgrid=True, - gridwidth=1, - gridcolor=colors["grid"], - zeroline=False, - tickfont=common_font, - title_font=common_title_font, - ) - return common_font, common_title_font, common_grid
🧹 Nitpick comments (4)
dashboard/app.py (1)
60-71: Clean up whitespace in CSS blockThe CSS block contains trailing whitespace that should be removed for consistency.
Style(""" uk-chart { display: block !important; width: 100% !important; min-height: 300px !important; height: auto !important; } uk-chart .apexcharts-canvas { width: 100% !important; height: 100% !important; } """),dashboard/routes/batch.py (2)
141-149: Remove trailing whitespaceClean up formatting issues identified by static analysis.
fig = ChartManager.create_chart( - df_metric, + df_metric, chart_index, small_charts=app.state.small_charts, dark_mode=app.state.dark_mode, show_markers=app.state.show_markers, line_width=app.state.line_width, show_legend=app.state.show_legend )
300-306: Remove trailing whitespace in expanded chart creationClean up formatting inconsistencies.
expanded_fig = ChartManager.create_expanded_chart( - df_metric, + df_metric, chart_index, dark_mode=app.state.dark_mode, show_markers=app.state.show_markers, line_width=app.state.line_width )dashboard/charts.py (1)
72-226: Clean up whitespace throughout the methodThis method has numerous trailing whitespace and blank line formatting issues that should be cleaned up for consistency.
Remove trailing whitespace from lines 96, 136, 179 and clean up blank lines containing whitespace on lines 65, 75, 78, 82, 106, 127, 148, 224.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
dashboard/app.py(2 hunks)dashboard/charts.py(3 hunks)dashboard/routes/batch.py(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
dashboard/app.py (2)
dashboard/routes/toggles.py (1)
post(58-64)dashboard/routes/index.py (1)
index(164-198)
dashboard/routes/batch.py (1)
dashboard/charts.py (5)
ChartManager(17-492)create_chart(53-69)create_expanded_chart(228-249)create_sparkline(280-391)create_single_anomaly_expanded_chart(440-492)
🪛 Ruff (0.12.2)
dashboard/app.py
75-75: Blank line contains whitespace
Remove whitespace from blank line
(W293)
81-81: Blank line contains whitespace
Remove whitespace from blank line
(W293)
89-89: Blank line contains whitespace
Remove whitespace from blank line
(W293)
94-94: Blank line contains whitespace
Remove whitespace from blank line
(W293)
dashboard/routes/batch.py
142-142: Trailing whitespace
Remove trailing whitespace
(W291)
301-301: Trailing whitespace
Remove trailing whitespace
(W291)
307-307: Blank line contains whitespace
Remove whitespace from blank line
(W293)
954-954: Blank line contains whitespace
Remove whitespace from blank line
(W293)
dashboard/charts.py
13-13: json imported but unused
Remove unused import: json
(F401)
14-14: datetime.datetime imported but unused
Remove unused import: datetime.datetime
(F401)
65-65: Blank line contains whitespace
Remove whitespace from blank line
(W293)
75-75: Blank line contains whitespace
Remove whitespace from blank line
(W293)
78-78: Blank line contains whitespace
Remove whitespace from blank line
(W293)
82-82: Blank line contains whitespace
Remove whitespace from blank line
(W293)
96-96: Trailing whitespace
Remove trailing whitespace
(W291)
106-106: Blank line contains whitespace
Remove whitespace from blank line
(W293)
127-127: Blank line contains whitespace
Remove whitespace from blank line
(W293)
136-136: Trailing whitespace
Remove trailing whitespace
(W291)
148-148: Blank line contains whitespace
Remove whitespace from blank line
(W293)
179-179: Trailing whitespace
Remove trailing whitespace
(W291)
224-224: Blank line contains whitespace
Remove whitespace from blank line
(W293)
240-240: Blank line contains whitespace
Remove whitespace from blank line
(W293)
245-245: Blank line contains whitespace
Remove whitespace from blank line
(W293)
292-292: Blank line contains whitespace
Remove whitespace from blank line
(W293)
295-295: Blank line contains whitespace
Remove whitespace from blank line
(W293)
299-299: Blank line contains whitespace
Remove whitespace from blank line
(W293)
310-310: Trailing whitespace
Remove trailing whitespace
(W291)
316-316: Blank line contains whitespace
Remove whitespace from blank line
(W293)
334-334: Blank line contains whitespace
Remove whitespace from blank line
(W293)
390-390: Blank line contains whitespace
Remove whitespace from blank line
(W293)
413-413: Blank line contains whitespace
Remove whitespace from blank line
(W293)
421-421: Blank line contains whitespace
Remove whitespace from blank line
(W293)
436-436: Blank line contains whitespace
Remove whitespace from blank line
(W293)
452-452: Blank line contains whitespace
Remove whitespace from blank line
(W293)
461-461: Blank line contains whitespace
Remove whitespace from blank line
(W293)
469-469: Blank line contains whitespace
Remove whitespace from blank line
(W293)
480-480: Blank line contains whitespace
Remove whitespace from blank line
(W293)
491-491: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🔇 Additional comments (1)
dashboard/charts.py (1)
77-81: Good handling of pandas data type serializationThe explicit conversion to native Python types (int/float) properly addresses the JSON serialization issues with pandas data types mentioned in the PR objectives.
| hdrs=( | ||
| Theme.blue.headers(), | ||
| Script(src="https://cdn.plot.ly/plotly-2.32.0.min.js"), | ||
| Script(src="https://cdn.jsdelivr.net/npm/apexcharts@latest"), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a specific version for the ApexCharts CDN
Using @latest in the CDN URL could lead to unexpected breaking changes in production. Pin to a specific version for stability.
- Script(src="https://cdn.jsdelivr.net/npm/apexcharts@latest"),
+ Script(src="https://cdn.jsdelivr.net/npm/apexcharts@3.45.0"),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Script(src="https://cdn.jsdelivr.net/npm/apexcharts@latest"), | |
| Script(src="https://cdn.jsdelivr.net/npm/apexcharts@3.45.0"), |
🤖 Prompt for AI Agents
In dashboard/app.py at line 59, the ApexCharts CDN URL uses '@latest', which can
cause instability due to unexpected breaking changes. Replace '@latest' with a
specific version number to ensure consistent and stable behavior in production.
| Script(""" | ||
| function initializeCharts() { | ||
| const elements = document.querySelectorAll('uk-chart:not([data-chart-initialized])'); | ||
| elements.forEach(function(element) { | ||
| const script = element.querySelector('script[type="application/json"]'); | ||
| if (script) { | ||
| try { | ||
| const config = JSON.parse(script.textContent); | ||
| // Add percentage formatting to score axis (second y-axis) | ||
| if (config.yaxis && config.yaxis[1]) { | ||
| config.yaxis[1].labels = config.yaxis[1].labels || {}; | ||
| config.yaxis[1].labels.formatter = function(val) { | ||
| return Math.round(val * 100) + '%'; | ||
| }; | ||
| } | ||
| // Ensure chart has dimensions | ||
| element.style.minHeight = '300px'; | ||
| element.style.width = '100%'; | ||
| element.style.display = 'block'; | ||
| const chart = new ApexCharts(element, config); | ||
| chart.render().then(() => { | ||
| element.setAttribute('data-chart-initialized', 'true'); | ||
| }); | ||
| } catch (e) { | ||
| console.error('Error initializing chart:', e, element); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
| // Wait for both DOM and ApexCharts to be ready | ||
| document.addEventListener('DOMContentLoaded', function() { | ||
| // Check if ApexCharts is loaded | ||
| if (typeof ApexCharts !== 'undefined') { | ||
| initializeCharts(); | ||
| } else { | ||
| // Wait a bit more for ApexCharts to load | ||
| setTimeout(() => { | ||
| if (typeof ApexCharts !== 'undefined') { | ||
| initializeCharts(); | ||
| } else { | ||
| console.error('ApexCharts library not loaded'); | ||
| } | ||
| }, 1000); | ||
| } | ||
| }); | ||
| // Re-initialize charts after HTMX requests | ||
| document.addEventListener('htmx:afterSwap', initializeCharts); | ||
| document.addEventListener('htmx:afterSettle', initializeCharts); | ||
| """), |
There was a problem hiding this comment.
Add chart cleanup to prevent memory leaks
Charts are re-initialized after HTMX swaps without destroying previous instances, which could cause memory leaks. Also, the percentage formatter should handle edge cases.
function initializeCharts() {
const elements = document.querySelectorAll('uk-chart:not([data-chart-initialized])');
elements.forEach(function(element) {
+ // Destroy existing chart if present
+ if (element._chart) {
+ element._chart.destroy();
+ element._chart = null;
+ }
+
const script = element.querySelector('script[type="application/json"]');
if (script) {
try {
const config = JSON.parse(script.textContent);
// Add percentage formatting to score axis (second y-axis)
if (config.yaxis && config.yaxis[1]) {
config.yaxis[1].labels = config.yaxis[1].labels || {};
config.yaxis[1].labels.formatter = function(val) {
- return Math.round(val * 100) + '%';
+ // Handle edge cases and ensure valid percentage
+ const percentage = Math.round((val || 0) * 100);
+ return Math.min(100, Math.max(0, percentage)) + '%';
};
}
// Ensure chart has dimensions
element.style.minHeight = '300px';
element.style.width = '100%';
element.style.display = 'block';
const chart = new ApexCharts(element, config);
chart.render().then(() => {
element.setAttribute('data-chart-initialized', 'true');
+ // Store chart instance for cleanup
+ element._chart = chart;
});
} catch (e) {
console.error('Error initializing chart:', e, element);
}
}
});
}
+// Clean up charts before page unload
+window.addEventListener('beforeunload', function() {
+ document.querySelectorAll('uk-chart[data-chart-initialized]').forEach(function(element) {
+ if (element._chart) {
+ element._chart.destroy();
+ }
+ });
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Script(""" | |
| function initializeCharts() { | |
| const elements = document.querySelectorAll('uk-chart:not([data-chart-initialized])'); | |
| elements.forEach(function(element) { | |
| const script = element.querySelector('script[type="application/json"]'); | |
| if (script) { | |
| try { | |
| const config = JSON.parse(script.textContent); | |
| // Add percentage formatting to score axis (second y-axis) | |
| if (config.yaxis && config.yaxis[1]) { | |
| config.yaxis[1].labels = config.yaxis[1].labels || {}; | |
| config.yaxis[1].labels.formatter = function(val) { | |
| return Math.round(val * 100) + '%'; | |
| }; | |
| } | |
| // Ensure chart has dimensions | |
| element.style.minHeight = '300px'; | |
| element.style.width = '100%'; | |
| element.style.display = 'block'; | |
| const chart = new ApexCharts(element, config); | |
| chart.render().then(() => { | |
| element.setAttribute('data-chart-initialized', 'true'); | |
| }); | |
| } catch (e) { | |
| console.error('Error initializing chart:', e, element); | |
| } | |
| } | |
| }); | |
| } | |
| // Wait for both DOM and ApexCharts to be ready | |
| document.addEventListener('DOMContentLoaded', function() { | |
| // Check if ApexCharts is loaded | |
| if (typeof ApexCharts !== 'undefined') { | |
| initializeCharts(); | |
| } else { | |
| // Wait a bit more for ApexCharts to load | |
| setTimeout(() => { | |
| if (typeof ApexCharts !== 'undefined') { | |
| initializeCharts(); | |
| } else { | |
| console.error('ApexCharts library not loaded'); | |
| } | |
| }, 1000); | |
| } | |
| }); | |
| // Re-initialize charts after HTMX requests | |
| document.addEventListener('htmx:afterSwap', initializeCharts); | |
| document.addEventListener('htmx:afterSettle', initializeCharts); | |
| """), | |
| Script(""" | |
| function initializeCharts() { | |
| const elements = document.querySelectorAll('uk-chart:not([data-chart-initialized])'); | |
| elements.forEach(function(element) { | |
| // Destroy existing chart if present | |
| if (element._chart) { | |
| element._chart.destroy(); | |
| element._chart = null; | |
| } | |
| const script = element.querySelector('script[type="application/json"]'); | |
| if (script) { | |
| try { | |
| const config = JSON.parse(script.textContent); | |
| // Add percentage formatting to score axis (second y-axis) | |
| if (config.yaxis && config.yaxis[1]) { | |
| config.yaxis[1].labels = config.yaxis[1].labels || {}; | |
| config.yaxis[1].labels.formatter = function(val) { | |
| // Handle edge cases and ensure valid percentage | |
| const percentage = Math.round((val || 0) * 100); | |
| return Math.min(100, Math.max(0, percentage)) + '%'; | |
| }; | |
| } | |
| // Ensure chart has dimensions | |
| element.style.minHeight = '300px'; | |
| element.style.width = '100%'; | |
| element.style.display = 'block'; | |
| const chart = new ApexCharts(element, config); | |
| chart.render().then(() => { | |
| element.setAttribute('data-chart-initialized', 'true'); | |
| // Store chart instance for cleanup | |
| element._chart = chart; | |
| }); | |
| } catch (e) { | |
| console.error('Error initializing chart:', e, element); | |
| } | |
| } | |
| }); | |
| } | |
| // Clean up charts before page unload | |
| window.addEventListener('beforeunload', function() { | |
| document.querySelectorAll('uk-chart[data-chart-initialized]').forEach(function(element) { | |
| if (element._chart) { | |
| element._chart.destroy(); | |
| } | |
| }); | |
| }); | |
| // Wait for both DOM and ApexCharts to be ready | |
| document.addEventListener('DOMContentLoaded', function() { | |
| // Check if ApexCharts is loaded | |
| if (typeof ApexCharts !== 'undefined') { | |
| initializeCharts(); | |
| } else { | |
| // Wait a bit more for ApexCharts to load | |
| setTimeout(() => { | |
| if (typeof ApexCharts !== 'undefined') { | |
| initializeCharts(); | |
| } else { | |
| console.error('ApexCharts library not loaded'); | |
| } | |
| }, 1000); | |
| } | |
| }); | |
| // Re-initialize charts after HTMX requests | |
| document.addEventListener('htmx:afterSwap', initializeCharts); | |
| document.addEventListener('htmx:afterSettle', initializeCharts); | |
| """), |
🧰 Tools
🪛 Ruff (0.12.2)
75-75: Blank line contains whitespace
Remove whitespace from blank line
(W293)
81-81: Blank line contains whitespace
Remove whitespace from blank line
(W293)
89-89: Blank line contains whitespace
Remove whitespace from blank line
(W293)
94-94: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🤖 Prompt for AI Agents
In dashboard/app.py around lines 72 to 126, the chart initialization code
re-creates ApexCharts instances on HTMX events without destroying previous
charts, causing potential memory leaks. Modify the code to keep track of created
chart instances per element and call their destroy method before
re-initializing. Also, update the percentage formatter to safely handle
non-numeric or null values by checking the input before formatting.
| import json | ||
| from datetime import datetime |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused imports
The json and datetime imports are not used in the code.
from monsterui.all import Card, DivLAligned, Loading, LoadingT, TextPresets, ApexChart
import pandas as pd
-import json
-from datetime import datetime📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import json | |
| from datetime import datetime | |
| from monsterui.all import Card, DivLAligned, Loading, LoadingT, TextPresets, ApexChart | |
| import pandas as pd |
🧰 Tools
🪛 Ruff (0.12.2)
13-13: json imported but unused
Remove unused import: json
(F401)
14-14: datetime.datetime imported but unused
Remove unused import: datetime.datetime
(F401)
🤖 Prompt for AI Agents
In dashboard/charts.py around lines 13 to 14, the imports for json and datetime
are not used anywhere in the code. Remove both import statements to clean up the
code and avoid unnecessary dependencies.
Update test_chart_manager_config_and_colors to verify ApexCharts configuration keys instead of Plotly configuration keys after migration. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
📊 Test Coverage ReportCoverage: 56% (yellow) ✅ Coverage maintained or improved!
|
📊 Test Coverage ReportCoverage: 56% (yellow) ✅ Coverage maintained or improved!
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
dashboard/app.py (3)
60-89: Selector still too generic.Past feedback about
uk-chartspecificity hasn’t been addressed; consider[data-chart]or a BEM-style class to avoid style clashes.
59-60: Pin the ApexCharts version instead of using@latest.Unpinned CDN can introduce breaking changes at runtime. Stick to a known good version (e.g.
@3.45.0).
170-210: Destroy previously created charts to avoid memory leaks.
initializeCharts()may run multiple times after HTMX swaps but there’s no call tochart.destroy().
Add a_charthandle on the element anddestroy()it before re-rendering.@@ - const chart = new ApexCharts(el, config); - chart.render().then(() => el.setAttribute('data-chart-initialized', 'true')); + if (el._chart) { + el._chart.destroy(); + } + const chart = new ApexCharts(el, config); + chart.render().then(() => { + el._chart = chart; + el.setAttribute('data-chart-initialized', 'true'); + });dashboard/charts.py (2)
13-14: Remove unused importsjsonanddatetime.
194-205: Hard-coded stroke/marker arrays break if series count changes.Adding or omitting alert series will desynchronise these arrays from
series.length, leading to wrong styling.- "width": [line_width, line_width, 0, 0], + "width": Array(series.length).fill(0).map((_, i) => + i < 2 ? line_width : 0),Apply the same pattern for
dashArray,markers.size, etc.
🧹 Nitpick comments (6)
dashboard/app.py (2)
102-105: DRY up the percentage-formatter block.The same four lines are duplicated; wrap them in a helper to avoid drift.
Also applies to: 139-143
211-230: Remove verbose console logging or gate it behind a debug flag.Frequent
console.login production harms performance and pollutes logs.dashboard/routes/batch.py (2)
10-12: Unused HTML-table imports can be dropped.
Table,Td,Th,Trare no longer referenced after the grid refactor.
142-149: Clean up trailing whitespace flagged by Ruff.Running
ruff --fixwill remove the W291/W293 offences.Also applies to: 301-307, 953-956
dashboard/charts.py (2)
386-395: Duplicate issue in sparkline config – static arrays assume five series.Build arrays dynamically to match
series.lengthas above.
292-299: Ruff: strip blank-line whitespace.Multiple W293 warnings—run
ruff --fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
dashboard/app.py(2 hunks)dashboard/charts.py(3 hunks)dashboard/routes/batch.py(11 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
dashboard/app.py (2)
dashboard/routes/toggles.py (1)
post(58-64)dashboard/routes/index.py (1)
index(164-198)
🪛 Ruff (0.12.2)
dashboard/charts.py
13-13: json imported but unused
Remove unused import: json
(F401)
14-14: datetime.datetime imported but unused
Remove unused import: datetime.datetime
(F401)
65-65: Blank line contains whitespace
Remove whitespace from blank line
(W293)
75-75: Blank line contains whitespace
Remove whitespace from blank line
(W293)
78-78: Blank line contains whitespace
Remove whitespace from blank line
(W293)
82-82: Blank line contains whitespace
Remove whitespace from blank line
(W293)
96-96: Trailing whitespace
Remove trailing whitespace
(W291)
106-106: Blank line contains whitespace
Remove whitespace from blank line
(W293)
127-127: Blank line contains whitespace
Remove whitespace from blank line
(W293)
136-136: Trailing whitespace
Remove trailing whitespace
(W291)
148-148: Blank line contains whitespace
Remove whitespace from blank line
(W293)
179-179: Trailing whitespace
Remove trailing whitespace
(W291)
224-224: Blank line contains whitespace
Remove whitespace from blank line
(W293)
240-240: Blank line contains whitespace
Remove whitespace from blank line
(W293)
245-245: Blank line contains whitespace
Remove whitespace from blank line
(W293)
292-292: Blank line contains whitespace
Remove whitespace from blank line
(W293)
295-295: Blank line contains whitespace
Remove whitespace from blank line
(W293)
299-299: Blank line contains whitespace
Remove whitespace from blank line
(W293)
310-310: Trailing whitespace
Remove trailing whitespace
(W291)
317-317: Blank line contains whitespace
Remove whitespace from blank line
(W293)
330-330: Blank line contains whitespace
Remove whitespace from blank line
(W293)
338-338: Trailing whitespace
Remove trailing whitespace
(W291)
343-343: Blank line contains whitespace
Remove whitespace from blank line
(W293)
350-350: Blank line contains whitespace
Remove whitespace from blank line
(W293)
353-353: Trailing whitespace
Remove trailing whitespace
(W291)
354-354: Trailing whitespace
Remove trailing whitespace
(W291)
357-357: Blank line contains whitespace
Remove whitespace from blank line
(W293)
365-365: Blank line contains whitespace
Remove whitespace from blank line
(W293)
408-408: Blank line contains whitespace
Remove whitespace from blank line
(W293)
442-442: Blank line contains whitespace
Remove whitespace from blank line
(W293)
450-450: Blank line contains whitespace
Remove whitespace from blank line
(W293)
465-465: Blank line contains whitespace
Remove whitespace from blank line
(W293)
481-481: Blank line contains whitespace
Remove whitespace from blank line
(W293)
490-490: Blank line contains whitespace
Remove whitespace from blank line
(W293)
498-498: Blank line contains whitespace
Remove whitespace from blank line
(W293)
509-509: Blank line contains whitespace
Remove whitespace from blank line
(W293)
520-520: Blank line contains whitespace
Remove whitespace from blank line
(W293)
dashboard/routes/batch.py
142-142: Trailing whitespace
Remove trailing whitespace
(W291)
301-301: Trailing whitespace
Remove trailing whitespace
(W291)
307-307: Blank line contains whitespace
Remove whitespace from blank line
(W293)
953-953: Blank line contains whitespace
Remove whitespace from blank line
(W293)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (1)
dashboard/charts.py (1)
90-104:yAxisIndexand per-seriesmarker/fillare not recognised by ApexCharts.ApexCharts maps series to y-axes by array position; unsupported keys are silently ignored.
Double-check that the right axis is actually used or switch to top-levelyaxis[index].seriesNamemapping.
| for row_idx, (_, row) in enumerate(df_page.iterrows(), start=1): | ||
| metric_name = row["metric_name"] | ||
| timestamp = row["metric_timestamp"] | ||
|
|
||
| # Get the metric data for this anomaly | ||
| df_metric = df[df["metric_name"] == metric_name].copy() | ||
| df_metric = df_metric.sort_values("metric_timestamp") | ||
| fig = ChartManager.create_sparkline(df_metric, anomaly_timestamp=timestamp) | ||
| log.info(f"Creating sparkline for {metric_name} with {len(df_metric)} data points") | ||
| fig = ChartManager.create_sparkline(df_metric, anomaly_timestamp=timestamp, dark_mode=app.state.dark_mode) | ||
| log.info(f"Sparkline created: {type(fig)}") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential performance hit: full sparkline generation inside tight loop.
create_sparkline() is called for every anomaly row with the entire metric history; this can dominate render time when pages contain many anomalies.
Cache per-metric sparklines or pre-slice to the last n points before passing into the chart helper.
🤖 Prompt for AI Agents
In dashboard/routes/batch.py around lines 378 to 388, the create_sparkline
function is called inside a loop for each anomaly row using the full metric
history, causing performance issues. To fix this, implement caching of
sparklines per metric to avoid redundant processing, or pre-slice df_metric to
include only the last n data points before calling create_sparkline. This
reduces the data size and avoids repeated full history processing for each
anomaly.
Migrate Dashboard Charts from Plotly to Monster UI ApexCharts
🎯 Summary
Complete migration of the anomstack dashboard from Plotly.js to Monster UI's native ApexChart components, maintaining all existing functionality while improving code organization and user experience.
✨ Key Changes
Charts Migration
Technical Improvements
Preserved Features
📁 Files Modified
dashboard/app.py- Added ApexCharts CDN, JavaScript initialization, and stylingdashboard/charts.py- Complete rewrite of ChartManager class for ApexChartsdashboard/routes/batch.py- Updated chart rendering and imports🧪 Testing
📸 Visual Changes
🚀 Benefits
🔄 Migration Notes
This is a complete drop-in replacement - no breaking changes to the dashboard API or user experience. All existing functionality is preserved while improving the underlying implementation.
Summary by CodeRabbit
New Features
Bug Fixes
Style