feat: add historical data collection and time-series charts - #36
Merged
Merged
Conversation
Introduce a background daemon thread that polls UPS variables every 60s and stores numeric snapshots in SQLite (WAL mode) at /var/lib/nutwatch/history.db with 90-day retention. Two new API endpoints expose the data: GET /api/history/<ups> (time-range query with optional variable filter) and GET /api/history/<ups>/variables (available series). On the frontend, a d3-based HistoryChart component renders interactive line charts with a time range selector (1h/24h/7d/30d), variable checkboxes (filtering out static config values), grid lines, hover tooltip, and crosshair. The UpsDetail page gains a tab bar (Info / Charts) so the existing real-time gauges and the new historical charts coexist without layout conflict. Full test coverage for the SQLite service layer, route handlers, and the React component, plus minor test hygiene fixes (silencing console.error in render-throw assertions).
|
Warning Review limit reached
More reviews will be available in 45 minutes. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds end-to-end historical UPS metric charting to NutWatch. A new SQLite-backed service records numeric UPS variable snapshots via a daemon background collector thread. Two admin-protected Flask API endpoints expose time-series history and variable listings. A new ChangesHistorical UPS Charts
Sequence DiagramsequenceDiagram
rect rgba(100, 149, 237, 0.5)
Note over create_app,SQLite snapshots: Background collection (daemon thread)
participant create_app
participant start_collector
participant NUT UPS service
participant SQLite snapshots
create_app->>start_collector: daemon thread with interval
loop every interval seconds
start_collector->>NUT UPS service: list UPS, fetch variables
NUT UPS service-->>start_collector: ups_name, variables dict
start_collector->>SQLite snapshots: record_snapshot (bulk INSERT numeric rows)
start_collector->>SQLite snapshots: prune() every 100 cycles
end
end
rect rgba(60, 179, 113, 0.5)
Note over HistoryChart,history_bp: Frontend query flow
participant HistoryChart
participant history_bp
HistoryChart->>history_bp: GET /api/history/{ups}/variables
history_bp-->>HistoryChart: available variable list
HistoryChart->>history_bp: GET /api/history/{ups}?range=24h&variables=...
history_bp->>SQLite snapshots: get_history(ups, variables, since)
SQLite snapshots-->>history_bp: per-variable [timestamp, value] series
history_bp-->>HistoryChart: JSON time-series response
HistoryChart->>HistoryChart: drawChart (SVG paths, crosshair, tooltip)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/historical-charts-plan.md`:
- Around line 110-115: Fix markdown linting violations across the file. At lines
110-115, 245-250, 267, 302, and 318, add blank lines before and after each
fenced code block to meet markdown formatting requirements. At line 228, add a
language identifier (such as "python" or "json") to the opening fence marks of
the code block. At line 394, ensure the file ends with a single trailing newline
to satisfy EOF requirements. These changes will resolve all markdownlint
warnings in the document.
In `@src/backend/app.py`:
- Around line 41-49: The current validation for NUTWATCH_HISTORY_INTERVAL only
catches ValueError when parsing the integer, but does not validate that the
interval is a positive number. After successfully converting the environment
variable to an integer in the try block, add an additional check to ensure the
interval value is greater than zero (interval > 0). If the validation fails, log
a warning message and set interval to the default value of 60, similar to the
existing ValueError handling. This ensures that the start_collector function
receives only valid positive interval values.
In `@src/backend/services/history.py`:
- Line 43: The os.makedirs call at line 43 in the history.py file fails when
HISTORY_DB is set to a filename-only path like "history.db" because
os.path.dirname() returns an empty string, which causes FileNotFoundError. Guard
the directory creation by checking that the directory path from
os.path.dirname(HISTORY_DB) is not empty before calling os.makedirs(), ensuring
it only attempts to create the directory when a parent directory actually
exists.
- Line 11: The DEFAULT_RETENTION_DAYS initialization in history.py can crash
module import if NUTWATCH_HISTORY_RETENTION_DAYS is not a valid integer; update
the module-level parsing to safely fall back to the default value instead of
raising, and keep the logic localized around DEFAULT_RETENTION_DAYS so app
startup cannot fail from a bad environment value.
In `@src/frontend/src/__tests__/components/ConfirmDialog.test.jsx`:
- Around line 25-31: Restore the console.error spy in a finally block in both
affected tests to prevent mocked console state from leaking across tests. In
ConfirmDialog.test.jsx around the useConfirm render/assertion and in
Modal.test.jsx around the equivalent render/assertion, keep the spy setup and
expectation together but ensure mockRestore runs in finally even if render or
the thrown-error assertion fails, using the existing console.error spy variable
in each test.
In `@src/frontend/src/components/HistoryChart.jsx`:
- Around line 291-310: Guard the async responses in HistoryChart’s effects so
older requests cannot overwrite newer state. In the effect that loads variables
and the effect that loads chart data, add a request-id or cancellation check
before calling setAvailableVars, setSelectedVars, setData, and setLoading, and
ignore any response that is no longer current after upsName/range changes.
Update both useEffect blocks in HistoryChart.jsx so only the latest api(...)
result applies state.
- Around line 252-253: The tooltip.innerHTML assignment on line 252-253 directly
injects HTML built from API-derived values (l.name and l.val), which creates an
XSS vulnerability if those values contain unexpected markup. Replace this
innerHTML assignment with DOM manipulation using createElement and textContent
instead. Create the tooltip structure programmatically by creating div and span
elements, and use textContent (rather than innerHTML) to set the name and value
content, which will automatically escape any special characters. Append the
created elements to the tooltip element rather than setting its innerHTML
property.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 98fa880c-9974-46cf-a4c5-245be25c3454
📒 Files selected for processing (15)
docs/historical-charts-plan.mdsrc/backend/app.pysrc/backend/routes/__init__.pysrc/backend/routes/history.pysrc/backend/services/history.pysrc/backend/tests/test_routes.pysrc/backend/tests/test_services_history.pysrc/frontend/src/__tests__/components/ConfirmDialog.test.jsxsrc/frontend/src/__tests__/components/HistoryChart.test.jsxsrc/frontend/src/__tests__/components/Modal.test.jsxsrc/frontend/src/__tests__/components/UpsDetail.test.jsxsrc/frontend/src/components/HistoryChart.jsxsrc/frontend/src/components/UpsDetail.jsxsrc/frontend/src/constants/index.jssrc/frontend/src/styles/components.css
…anipulation This commit makes several improvements to the HistoryChart component and related code: 1. Refactored tooltip creation in HistoryChart.jsx to use DOM manipulation instead of innerHTML for security and better control over tooltip content 2. Added useEffect cleanup with cancelled flags to prevent memory leaks and handle component unmounting properly 3. Added validation for NUTWATCH_HISTORY_INTERVAL and NUTWATCH_HISTORY_RETENTION_DAYS environment variables in app.py and history.py with proper fallback values 4. Fixed test cleanup in ConfirmDialog and Modal tests to ensure mockRestore is called even on test failures These changes improve code quality, security, and reliability of the historical charts feature.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduce a background daemon thread that polls UPS variables every 60s and stores numeric snapshots in SQLite (WAL mode) at /var/lib/nutwatch/history.db with 90-day retention. Two new API endpoints expose the data: GET /api/history/ (time-range query with optional variable filter) and GET /api/history//variables (available series).
On the frontend, a d3-based HistoryChart component renders interactive line charts with a time range selector (1h/24h/7d/30d), variable checkboxes (filtering out static config values), grid lines, hover tooltip, and crosshair. The UpsDetail page gains a tab bar (Info / Charts) so the existing real-time gauges and the new historical charts coexist without layout conflict.
Full test coverage for the SQLite service layer, route handlers, and the React component, plus minor test hygiene fixes (silencing console.error in render-throw assertions).
Summary by CodeRabbit
New Features
Bug Fixes