Phase 3.5 — Dashboard rewrite (multi-plan ranked view)#76
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughPR adds documentation for Phase 3.2–3.5 work: release notes covering a rewritten multi-plan ranked dashboard, named comparator drill-in, universal backfill pipeline, and period rollup sensors; plus a design spec explaining PriceHawk's deliberate divergence from the Claude styling. ChangesDocumentation: Phase 3.2–3.5 Release Notes and Design Specification
🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideRewrites the PriceHawk Home Assistant dashboard into a multi-plan ranked-alternatives view using existing Phase 3 sensors, replacing the prior Amber-vs-current two-comparator layout, and updates CSP plus design docs accordingly. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 security issues, and 3 other issues
Security issues:
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="custom_components/pricehawk/www/dashboard.html" line_range="1074" />
<code_context>
-// State
-const st = {};
-const at = {};
-const pendingRequests = new Map();
-let ws = null;
-let msgId = 1;
</code_context>
<issue_to_address>
**suggestion:** Remove or repurpose `pendingRequests` now that no requests use it.
The dashboard no longer issues `call_service` or `history/period` messages that add entries to `pendingRequests`, so it will stay empty. Please either remove `pendingRequests` and its related WebSocket handling, or connect it to upcoming service calls (e.g., opt‑in actions from the drill‑in card) so it isn’t dead code.
Suggested implementation:
```
```
To fully implement the suggestion and avoid dead code, you should also:
1. Remove any code that adds entries to or reads from `pendingRequests`, for example:
- `pendingRequests.set(msgId, ...)`
- `const pending = pendingRequests.get(data.id);`
- `pendingRequests.delete(data.id);`
2. Remove any WebSocket `onmessage` / `addEventListener("message", ...)` branches that exist solely to route replies to `pendingRequests` (typically handling responses to `call_service` or `history/period` messages by ID).
3. If there are helper functions that only exist to create request IDs and manage `pendingRequests` (e.g., `sendRequestWithResponse`, `callServiceWithAck`, etc.), delete them or refactor them to fire‑and‑forget WebSocket calls that don’t rely on `pendingRequests`.
You’ll need to search in this file for `pendingRequests` and remove or refactor every reference so that no dead code remains and the WebSocket handling matches the new behavior (no `call_service` or `history/period` request/response tracking).
</issue_to_address>
### Comment 2
<location path="custom_components/pricehawk/www/dashboard.html" line_range="532-537" />
<code_context>
- </div>
+ <!-- ─────────────── PERIOD TABS ─────────────── -->
+ <div class="row tabs-row">
+ <div id="periodTabs" class="period-tabs" role="tablist" aria-label="Rollup period">
+ <button class="period-tab" data-window="today" role="tab">Today</button>
+ <button class="period-tab" data-window="week" role="tab">Week</button>
+ <button class="period-tab active" data-window="month" role="tab">Month</button>
+ <button class="period-tab" data-window="3month" role="tab">3 Month</button>
+ <button class="period-tab" data-window="year" role="tab">Year</button>
</div>
</div>
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Align tab ARIA attributes with the visual selection state for better accessibility.
These buttons use `role="tab"`, but only the `active` class changes when the active window updates; `aria-selected` and focus aren’t kept in sync, so assistive tech won’t see the correct selection. In `setActiveWindow`, also toggle `aria-selected="true/false"` and `tabindex="0/-1"` on each `.period-tab`, and consider wiring `aria-controls` from each tab to its corresponding content panel to follow the ARIA tabs pattern.
Suggested implementation:
```
<!-- ─────────────── PERIOD TABS ─────────────── -->
<div class="row tabs-row">
<div id="periodTabs" class="period-tabs" role="tablist" aria-label="Rollup period">
<button
class="period-tab"
id="periodTab-today"
data-window="today"
role="tab"
aria-selected="false"
tabindex="-1"
aria-controls="periodPanel-today"
>
Today
</button>
<button
class="period-tab"
id="periodTab-week"
data-window="week"
role="tab"
aria-selected="false"
tabindex="-1"
aria-controls="periodPanel-week"
>
Week
</button>
<button
class="period-tab active"
id="periodTab-month"
data-window="month"
role="tab"
aria-selected="true"
tabindex="0"
aria-controls="periodPanel-month"
>
Month
</button>
<button
class="period-tab"
id="periodTab-3month"
data-window="3month"
role="tab"
aria-selected="false"
tabindex="-1"
aria-controls="periodPanel-3month"
>
3 Month
</button>
<button
class="period-tab"
id="periodTab-year"
data-window="year"
role="tab"
aria-selected="false"
tabindex="-1"
aria-controls="periodPanel-year"
>
Year
</button>
</div>
</div>
<div class="row tabs-row">
<div class="nav-right">
```
To fully implement the ARIA tabs pattern and keep it in sync with the visual state, you should also:
1. Update the `setActiveWindow` (or equivalent) JavaScript that currently only toggles the `active` class so that it:
- Loops over all `.period-tab` elements and for each:
- Sets `tab.classList.toggle('active', tab.dataset.window === window)` (or your existing logic).
- Sets `tab.setAttribute('aria-selected', tab.dataset.window === window ? 'true' : 'false')`.
- Sets `tab.tabIndex = tab.dataset.window === window ? 0 : -1`.
- Moves focus to the newly selected tab when changes are user-initiated: `activeTab.focus();`.
2. Ensure you have corresponding tab panels with matching `id`s, e.g.:
- `<div id="periodPanel-today" role="tabpanel" aria-labelledby="periodTab-today" hidden>…</div>`
- Repeat for `week`, `month`, `3month`, and `year`.
In `setActiveWindow`, hide/show these panels (`hidden` attribute or equivalent) in sync with the selected tab.
3. (Optional but recommended) Add keyboard handling for left/right arrow keys on `.period-tab` elements to move focus and activate the previous/next tab according to the WAI-ARIA Authoring Practices for tabs.
</issue_to_address>
### Comment 3
<location path="CHANGELOG.md" line_range="25" />
<code_context>
+ - NAV bar (brand + connection status pill + clock + theme toggle).
+ - HERO row: current-cost card + savings-vs-best-alt card (with
+ projected-annual extrapolation).
+ - PERIOD TABS: `[Today][Week][Month][3 Month][Year]` — clicking a
+ tab swaps the entity binding for every rollup card to the matching
+ `_today` / `_week` / `_month` / `_3month` / `_year` sensor in
</code_context>
<issue_to_address>
**nitpick (typo):** Consider changing the tab label from "3 Month" to "3 Months" for grammatical consistency.
If this is the user-visible tab label (not just an internal identifier), consider pluralizing it to “3 Months” to better match the other period names and read more naturally.
```suggestion
- PERIOD TABS: `[Today][Week][Month][3 Months][Year]` — clicking a
```
</issue_to_address>
### Comment 4
<location path="CHANGELOG.md" line_range="48" />
<code_context>
- **CSP `connect-src` extended** to include `ws://*.local:*` +
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
*Source: opengrep*
</issue_to_address>
### Comment 5
<location path="CHANGELOG.md" line_range="63" />
<code_context>
- `location.protocol === 'https:' ? 'wss://' : 'ws://'` for the WS
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
*Source: opengrep*
</issue_to_address>
### Comment 6
<location path="CHANGELOG.md" line_range="64" />
<code_context>
URL (AEGIS rule: never hardcode `ws://`).
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
*Source: opengrep*
</issue_to_address>
### Comment 7
<location path="custom_components/pricehawk/www/dashboard.html" line_range="633" />
<code_context>
// NEVER hardcode a token. NEVER hardcode ws:// — derive from location.protocol
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| $('footerLastRankingSub').textContent = lastRun | ||
| ? new Date(lastRun).toLocaleString('en-AU', { hour12: false }) | ||
| : 'awaiting first run'; | ||
|
|
There was a problem hiding this comment.
suggestion: Remove or repurpose pendingRequests now that no requests use it.
The dashboard no longer issues call_service or history/period messages that add entries to pendingRequests, so it will stay empty. Please either remove pendingRequests and its related WebSocket handling, or connect it to upcoming service calls (e.g., opt‑in actions from the drill‑in card) so it isn’t dead code.
Suggested implementation:
To fully implement the suggestion and avoid dead code, you should also:
- Remove any code that adds entries to or reads from
pendingRequests, for example:pendingRequests.set(msgId, ...)const pending = pendingRequests.get(data.id);pendingRequests.delete(data.id);
- Remove any WebSocket
onmessage/addEventListener("message", ...)branches that exist solely to route replies topendingRequests(typically handling responses tocall_serviceorhistory/periodmessages by ID). - If there are helper functions that only exist to create request IDs and manage
pendingRequests(e.g.,sendRequestWithResponse,callServiceWithAck, etc.), delete them or refactor them to fire‑and‑forget WebSocket calls that don’t rely onpendingRequests.
You’ll need to search in this file for pendingRequests and remove or refactor every reference so that no dead code remains and the WebSocket handling matches the new behavior (no call_service or history/period request/response tracking).
| <div id="periodTabs" class="period-tabs" role="tablist" aria-label="Rollup period"> | ||
| <button class="period-tab" data-window="today" role="tab">Today</button> | ||
| <button class="period-tab" data-window="week" role="tab">Week</button> | ||
| <button class="period-tab active" data-window="month" role="tab">Month</button> | ||
| <button class="period-tab" data-window="3month" role="tab">3 Month</button> | ||
| <button class="period-tab" data-window="year" role="tab">Year</button> |
There was a problem hiding this comment.
suggestion (bug_risk): Align tab ARIA attributes with the visual selection state for better accessibility.
These buttons use role="tab", but only the active class changes when the active window updates; aria-selected and focus aren’t kept in sync, so assistive tech won’t see the correct selection. In setActiveWindow, also toggle aria-selected="true/false" and tabindex="0/-1" on each .period-tab, and consider wiring aria-controls from each tab to its corresponding content panel to follow the ARIA tabs pattern.
Suggested implementation:
<!-- ─────────────── PERIOD TABS ─────────────── -->
<div class="row tabs-row">
<div id="periodTabs" class="period-tabs" role="tablist" aria-label="Rollup period">
<button
class="period-tab"
id="periodTab-today"
data-window="today"
role="tab"
aria-selected="false"
tabindex="-1"
aria-controls="periodPanel-today"
>
Today
</button>
<button
class="period-tab"
id="periodTab-week"
data-window="week"
role="tab"
aria-selected="false"
tabindex="-1"
aria-controls="periodPanel-week"
>
Week
</button>
<button
class="period-tab active"
id="periodTab-month"
data-window="month"
role="tab"
aria-selected="true"
tabindex="0"
aria-controls="periodPanel-month"
>
Month
</button>
<button
class="period-tab"
id="periodTab-3month"
data-window="3month"
role="tab"
aria-selected="false"
tabindex="-1"
aria-controls="periodPanel-3month"
>
3 Month
</button>
<button
class="period-tab"
id="periodTab-year"
data-window="year"
role="tab"
aria-selected="false"
tabindex="-1"
aria-controls="periodPanel-year"
>
Year
</button>
</div>
</div>
<div class="row tabs-row">
<div class="nav-right">
To fully implement the ARIA tabs pattern and keep it in sync with the visual state, you should also:
-
Update the
setActiveWindow(or equivalent) JavaScript that currently only toggles theactiveclass so that it:- Loops over all
.period-tabelements and for each:- Sets
tab.classList.toggle('active', tab.dataset.window === window)(or your existing logic). - Sets
tab.setAttribute('aria-selected', tab.dataset.window === window ? 'true' : 'false'). - Sets
tab.tabIndex = tab.dataset.window === window ? 0 : -1.
- Sets
- Moves focus to the newly selected tab when changes are user-initiated:
activeTab.focus();.
- Loops over all
-
Ensure you have corresponding tab panels with matching
ids, e.g.:<div id="periodPanel-today" role="tabpanel" aria-labelledby="periodTab-today" hidden>…</div>- Repeat for
week,month,3month, andyear.
InsetActiveWindow, hide/show these panels (hiddenattribute or equivalent) in sync with the selected tab.
-
(Optional but recommended) Add keyboard handling for left/right arrow keys on
.period-tabelements to move focus and activate the previous/next tab according to the WAI-ARIA Authoring Practices for tabs.
| - NAV bar (brand + connection status pill + clock + theme toggle). | ||
| - HERO row: current-cost card + savings-vs-best-alt card (with | ||
| projected-annual extrapolation). | ||
| - PERIOD TABS: `[Today][Week][Month][3 Month][Year]` — clicking a |
There was a problem hiding this comment.
nitpick (typo): Consider changing the tab label from "3 Month" to "3 Months" for grammatical consistency.
If this is the user-visible tab label (not just an internal identifier), consider pluralizing it to “3 Months” to better match the other period names and read more naturally.
| - PERIOD TABS: `[Today][Week][Month][3 Month][Year]` — clicking a | |
| - PERIOD TABS: `[Today][Week][Month][3 Months][Year]` — clicking a |
| replaced with an "Accruing… [n/365]" pill instead of showing a | ||
| misleading `$0.00`. Surfaces clearly that we don't have enough | ||
| history yet. | ||
| - **CSP `connect-src` extended** to include `ws://*.local:*` + |
There was a problem hiding this comment.
security (javascript.lang.security.detect-insecure-websocket): Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
Source: opengrep
|
|
||
| - **WebSocket auth + URL detection preserved verbatim** from the prior | ||
| dashboard: | ||
| - `location.protocol === 'https:' ? 'wss://' : 'ws://'` for the WS |
There was a problem hiding this comment.
security (javascript.lang.security.detect-insecure-websocket): Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
Source: opengrep
| - **WebSocket auth + URL detection preserved verbatim** from the prior | ||
| dashboard: | ||
| - `location.protocol === 'https:' ? 'wss://' : 'ws://'` for the WS | ||
| URL (AEGIS rule: never hardcode `ws://`). |
There was a problem hiding this comment.
security (javascript.lang.security.detect-insecure-websocket): Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
Source: opengrep
|
|
||
| // ─────────────── URL params + WebSocket URL (AEGIS rules) ─────────────── | ||
| // Token from URL params or postMessage / parent hassConnection / localStorage. | ||
| // NEVER hardcode a token. NEVER hardcode ws:// — derive from location.protocol |
There was a problem hiding this comment.
security (javascript.lang.security.detect-insecure-websocket): Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
Source: opengrep
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CHANGELOG.md (1)
227-244:⚠️ Potential issue | 🟠 MajorFix test count and entity documentation in Phase 3.3.
The CHANGELOG has multiple inaccuracies:
Test count: Claims "27 stdlib-only tests" in rollup.py, but the actual count is 14 tests.
Entity count and types: Claims "15 new entities registered: 3 sensor types × 5 windows" but the actual implementation defines 20 entities across 4 sensor types:
current_cost_{today,week,month,3month,year}(5)best_alt_cost_{today,week,month,3month,year}(5)savings_cost_{today,week,month,3month,year}(5)named_cost_{today,week,month,3month,year}(5) ← omitted from CHANGELOGThe
PeriodRollupSensorbase class and its three documented subclasses are correct, but the CHANGELOG missing thenamed_costsensor type entirely is a significant gap. Update the entity count to 20 and document all four sensor types.🤖 Prompt for 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. In `@CHANGELOG.md` around lines 227 - 244, The CHANGELOG entry incorrectly states test and entity counts: update the rollup.py test count from "27 stdlib-only tests" to "14 tests" referencing cdr/rollup.py, and update the entity section to reflect 20 sensors (4 sensor types × 5 windows) by adding the missing named_cost sensor type; explicitly list the four sensor families (CurrentCostRollupSensor, BestAlternativeRollupSensor, SavingsRollupSensor, and the omitted NamedCostRollupSensor/named_cost sensors) and change "15 new entities" to "20 new entities" so the documentation matches the actual implementation.
🤖 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 `@assets/DESIGN.claude.md`:
- Around line 639-652: The fenced token map code block containing CSS custom
properties (e.g., --bg-base, --bg-surface, --text-primary, --accent-positive,
--card-radius) must include a language identifier; update the opening fence from
``` to ```css so the block is marked as CSS for proper Markdown syntax
highlighting and linting compliance.
---
Outside diff comments:
In `@CHANGELOG.md`:
- Around line 227-244: The CHANGELOG entry incorrectly states test and entity
counts: update the rollup.py test count from "27 stdlib-only tests" to "14
tests" referencing cdr/rollup.py, and update the entity section to reflect 20
sensors (4 sensor types × 5 windows) by adding the missing named_cost sensor
type; explicitly list the four sensor families (CurrentCostRollupSensor,
BestAlternativeRollupSensor, SavingsRollupSensor, and the omitted
NamedCostRollupSensor/named_cost sensors) and change "15 new entities" to "20
new entities" so the documentation matches the actual implementation.
🪄 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: 55c56f92-f962-4c02-86d6-25c13e28708d
📒 Files selected for processing (3)
CHANGELOG.mdassets/DESIGN.claude.mdcustom_components/pricehawk/www/dashboard.html
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Verify: no broken links, code examples match actual implementation, version numbers are current, no TODO left unfixed.
Files:
assets/DESIGN.claude.mdCHANGELOG.md
**/CHANGELOG.md
⚙️ CodeRabbit configuration file
**/CHANGELOG.md: Entries MUST follow Keep a Changelog format. New version section MUST be present for this PR's changes.
Files:
CHANGELOG.md
🪛 LanguageTool
CHANGELOG.md
[grammar] ~91-~91: Ensure spelling is correct
Context: ...prior dashboard already used. - 30s setInterval re-render for the ranked + footer car...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~93-~93: Ensure spelling is correct
Context: ...aiting on a state_changed event. Cheap (<1ms per tick on HA Green). - **XSS hardenin...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.22.1)
assets/DESIGN.claude.md
[warning] 639-639: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (6)
CHANGELOG.md (4)
174-195: All Phase 3.2 backfill implementation details match actual code. Modules, functions, and test infrastructure verified.
5-5: No issues found. The external link to Keep a Changelog is valid and reachable.
122-148: ✅ Phase 3.4 section checks out. All referenced config keys, functions, classes, and test files are present in the codebase and match the documented implementation details.
20-57: All referenced file paths and entity names are verified to exist in the codebase and match the CHANGELOG description. No issues found.assets/DESIGN.claude.md (2)
654-658: No issues found. The localStorage key names in the documentation match the implementation:
'pricehawk-theme'is used consistently in both DESIGN.claude.md and dashboard.html (lines 673, 686, 698)'pricehawk-window'is used consistently in both CHANGELOG.md and dashboard.html (lines 720, 727)The documentation uses bracket notation (
localStorage['key']) while the code uses method calls (localStorage.getItem('key')), but both access the same storage with identical key names.
595-596: No issues with referenced file paths. All specified locations exist.
Artic0din
force-pushed
the
phase-3.4-named-comparator
branch
from
c010f01 to
c06117a
Compare
|
@coderabbitai review |
Artic0din
force-pushed
the
phase-3.5-dashboard-rewrite
branch
from
b3cd5c9 to
88b9ded
Compare
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
assets/DESIGN.claude.md (1)
639-652:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifier to fenced code block.
The token map code block lacks a language identifier. For proper syntax highlighting and markdown compliance, specify the language.
📝 Proposed fix
-``` +```css --bg-base: `#070B14` // OLED-friendly true black --bg-surface: `#0C1220`As per coding guidelines, code examples must be properly formatted.
🤖 Prompt for 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. In `@assets/DESIGN.claude.md` around lines 639 - 652, The fenced code block containing CSS custom properties (e.g., --bg-base, --bg-surface, --text-primary, --card-radius) needs a language identifier; update the opening fence from ``` to ```css so the block becomes a CSS block for proper syntax highlighting and markdown compliance.
🤖 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.
Duplicate comments:
In `@assets/DESIGN.claude.md`:
- Around line 639-652: The fenced code block containing CSS custom properties
(e.g., --bg-base, --bg-surface, --text-primary, --card-radius) needs a language
identifier; update the opening fence from ``` to ```css so the block becomes a
CSS block for proper syntax highlighting and markdown compliance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e8cf0766-4821-4c0f-9db5-334ba240ee61
📒 Files selected for processing (3)
CHANGELOG.mdassets/DESIGN.claude.mdcustom_components/pricehawk/www/dashboard.html
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Verify: no broken links, code examples match actual implementation, version numbers are current, no TODO left unfixed.
Files:
assets/DESIGN.claude.mdCHANGELOG.md
**/CHANGELOG.md
⚙️ CodeRabbit configuration file
**/CHANGELOG.md: Entries MUST follow Keep a Changelog format. New version section MUST be present for this PR's changes.
Files:
CHANGELOG.md
🪛 LanguageTool
CHANGELOG.md
[grammar] ~91-~91: Ensure spelling is correct
Context: ...prior dashboard already used. - 30s setInterval re-render for the ranked + footer car...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~93-~93: Ensure spelling is correct
Context: ...aiting on a state_changed event. Cheap (<1ms per tick on HA Green). - **XSS hardenin...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (5)
CHANGELOG.md (3)
5-5: No action needed. The Keep a Changelog link is valid and accessible (HTTP 200).
30-32: CHANGELOG references unverified sensor entities.The verification found that most functions and services referenced in the changelog exist in the codebase. However, the following sensor entities referenced in the changelog could not be found:
sensor.pricehawk_ranked_alternatives(lines 30-31) — no definition found- The 15 rollup sensors mentioned at lines 237-239 (
pricehawk_current_cost_*,pricehawk_best_alt_cost_*,pricehawk_savings_cost_*variants) — no definition foundVerify these sensors are actually defined in the codebase. If they are defined through Home Assistant's entity registry or other dynamic means, update the CHANGELOG to clarify how they are created or provide accurate references.
14-14: No issues found. All referenced files exist in the repository and CHANGELOG.md follows Keep a Changelog format with PR changes documented in the [Unreleased] section.assets/DESIGN.claude.md (2)
617-617: All PriceHawk asset file references are correct—custom_components/pricehawk/icon.png,assets/dashboard-v3-apple.html, andcustom_components/pricehawk/www/dashboard.htmlexist and are properly documented.
640-652: Documentation is accurate. All CSS token values listed in lines 640-652 match the implementation indashboard.html(lines 30-48).
Artic0din
force-pushed
the
phase-3.4-named-comparator
branch
from
c06117a to
cb979c4
Compare
Artic0din
force-pushed
the
phase-3.5-dashboard-rewrite
branch
from
88b9ded to
b14c421
Compare
Artic0din
force-pushed
the
phase-3.4-named-comparator
branch
from
cb979c4 to
5449eed
Compare
Artic0din
force-pushed
the
phase-3.5-dashboard-rewrite
branch
from
b14c421 to
cbccfd5
Compare
…multi-plan layout Full rewrite of custom_components/pricehawk/www/dashboard.html (2447 -> 940 LOC) replacing the Amber-vs-current-plan two-comparator view with the multi-plan ranked layout from plan section 5.1. Visual language ported from assets/dashboard-v3-apple.html (dark default, Outfit + IBM Plex Mono, noise + ambient bg). Per-provider colour tokens (--amber-primary, --globird-primary) replaced with semantic ones (--accent-positive, --accent-negative, --accent-neutral) per the Phase 3.0 pivot away from provider-specific branding. Scaffold layout per plan section 5.1: - NAV bar (brand + connection status + clock + theme toggle) - HERO row: current cost card + savings-vs-best-alt card - PERIOD TABS: [Today][Week][Month*][3 Month][Year], active swaps data - RANKED ALTERNATIVES table: #/plan/peak/supply/saving, click -> drill - DRILL-IN CARD: per-plan stats + "Pin as Named Comparator" button - DATA HEALTH FOOTER: backfill state / days loaded / last ranking / count Entity reads are NOT wired yet — sample data renders so the scaffold is visually verifiable before commit 3.5/2 binds real sensor values. WebSocket connection logic copied verbatim from the previous dashboard: - WS URL derived from location.protocol (AEGIS rule: never hardcode ws://) - Token from URL params, postMessage, parent.hassConnection, or localStorage hassTokens (AEGIS rule: never hardcode the token) CSP connect-src extended to include ws(s)://*.local:* so the dashboard works on Ryan's HA Green at homeassistant.local (plan section 5.3 surprise #1). Existing localhost + Nabu Casa entries preserved. Active period tab persists to localStorage so re-opens land on the user's last view rather than defaulting to month every time. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ll entity reads
Hooks the scaffold from commit 3.5/1 up to the Phase 3.2 / 3.3 / 3.4
sensors. After WebSocket auth completes, fires a get_states + subscribes
to state_changed events for the 16 tracked entities.
Tracked entities (per plan section 5.2 + Phase 3.3 / 3.4 worker notes):
- 5 x sensor.pricehawk_current_cost_{today,week,month,3month,year}
- 5 x sensor.pricehawk_best_alt_cost_{...} (NOT _best_alternative_cost_)
- 5 x sensor.pricehawk_savings_{...}
- 5 x sensor.pricehawk_named_cost_{...} (NOT _named_comparator_cost_)
- sensor.pricehawk_ranked_alternatives
- sensor.pricehawk_backfill_status
Hero row binding:
- Current cost card reads sensor.pricehawk_current_cost_<window>.
- Savings card reads sensor.pricehawk_savings_<window> and colours green
/ red / muted around the +/- $0.005 deadband.
- Best-alt name pulled from ranked_alternatives.attributes.alternatives[0]
(sensor is sorted ascending by cheap-rank score per summarize_for_sensor).
- Projected annual extrapolates active-window savings * 365/window_days.
Period tabs swap activeWindow and re-call renderHero() — all rollup
bindings re-evaluate against the new window's entity ID. Active class
mirrors localStorage so the tab UI stays in sync on cold loads.
Ranked alts table render:
- Pulls ranked_alternatives.attributes.alternatives[].
- Renders rank-pill (#1 gold), plan name + brand, peak rate, supply,
saving. Saving column only fills for the #1 plan (the cheapest); #2..N
show "—" because we don't have per-alt cost rollups — only the best-alt
rollup. Avoids fabricating numbers that don't match the sensor.
- Click row → drill-in card slides up below + plan ID persists in
selectedPlanId so re-renders after state_changed events preserve
selection.
- Empty state ("Waiting for the daily ranking job…") covers first-install
before the first ranking run completes.
Drill-in card:
- Stats grid: peak rate, daily supply, customer type, plan ID,
cheap-rank score (when present).
- "Pin as Named Comparator" deep-links to the integration's Configure
page (/config/integrations/integration/pricehawk). Per plan section 5.3
surprise #2 + plan section 9 REVISIT 4: HA doesn't support per-step
deep-linking; the deep-link is the locked UX for this phase.
Data Health footer renders backfill state with state-coloured value
(green=complete / amber=running / red=failed / muted=idle), days_loaded,
ranked_alternatives.last_run as relative + absolute time, and the
alternatives count.
Empty-state UI for first-run users (plan section 5.3 surprise #3): when
backfill_status.days_loaded < 7, hero rollup values are replaced with an
"Accruing… [n/365]" pill instead of showing $0.00 — surfaces clearly
that we don't have enough history yet rather than implying zero spend.
XSS hardening: all attribute-sourced strings (plan_id, display_name,
brand, customer_type) pass through escapeHtml() before innerHTML
insertion. Catches any future CDR registry payloads that include
HTML-ish characters in brand names.
30s setInterval re-renders the ranked + footer cards so the relative
timestamps ("ran 27s ago / 3h ago") tick forward without waiting for
the next state_changed event.
TDZ fix: the period-tab boot block previously called setActiveWindow()
before the entity state store consts were declared, which tripped a
ReferenceError on attrs in strict mode. Boot now defers the first full
render to the explicit boot block at the script bottom.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…GELOG entry Wraps Phase 3.5 with the two non-code deliverables called out in plan section 5.2 commit 3.5/3. assets/DESIGN.claude.md: - New "PriceHawk Dashboard (divergence from this spec)" section at the end of the file. Explains WHY PriceHawk doesn't follow the Claude marketing-site spec (different surface context, different information density, different brand) and WHAT it does inherit (typographic rationale, card-as-surface model, accent-discipline rule). - Documents the PriceHawk token map (--bg-base, --accent-positive etc) for cross-reference. - Keeps the rest of the Claude marketing-site spec intact — no edits outside the new appended section. CHANGELOG.md: - Phase 3.5 block at the top of [Unreleased] above the existing 3.4 entry. Documents the dashboard rewrite (entity bindings, period-tab swap, ranked alts render, drill-in, footer, empty-state), the CSP connect-src extension for *.local deployments, the deleted per-provider colour tokens, the deleted Amber-specific cards, and the manual-UAT-only test strategy (per plan section 6.3 table). dashboard_config.py: NO behavioural change. Plan section 5.2 commit 3.5/3 calls for a "verify cache-busting still works" check; verified that `?v=<version>.<epoch>` is appended in setup_panel_iframe and is independent of dashboard.html contents — the rewrite doesn't affect it. No source edit needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeRabbit / markdownlint MD040: the fenced code block listing the PriceHawk CSS custom properties (--bg-base, --bg-surface, --accent-positive et al) opened with a bare ``` instead of ```css. Tag the fence as ``css`` so the markdown renderer applies CSS syntax highlighting and so MD040 stops flagging the block. Content is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Artic0din
force-pushed
the
phase-3.4-named-comparator
branch
from
5449eed to
507db8b
Compare
Artic0din
force-pushed
the
phase-3.5-dashboard-rewrite
branch
from
cbccfd5 to
0e1ba0e
Compare
1 participant
Summary
Rewrites the Amber-centric dashboard (2447 LOC) as a multi-plan ranked-alternatives view. Hero row shows current monthly cost + savings vs best alternative. Period tabs swap rollup cards (today/week/month/3month/year). Ranked alts table with click-to-drill-in card. Data health footer surfaces backfill status + last ranking timestamp.
Visual seed:
assets/dashboard-v3-apple.html(dark default, Outfit + IBM Plex Mono, semantic colour tokens).Final phase in the Phase 3 stack. Stacked on PR #75 → #74 → #73. Merge in order.
Changes
custom_components/pricehawk/www/dashboard.html(~1250 LOC, down from 2447)connect-srcextended forwss://*.localassets/DESIGN.claude.md— adds PriceHawk-Dashboard divergence sectionTest plan
🤖 Generated with Claude Code
Summary by Sourcery
Rewrite the PriceHawk Home Assistant dashboard into a multi-plan ranked alternatives view driven by new rollup and ranking sensors, with a new card hierarchy, improved theming tokens, and a data-health footer.
New Features:
Enhancements:
Build:
Documentation:
What changed
#75→#74→#73; must merge in order.Why
Breaking changes
Files changed (lines added / removed)