feat(sensor): Phase 3.1 commit 6 — RankedAlternativesSensor#70
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughAdds a helper to summarize CDR PlanDetailV2 objects for sensor attributes, converts Decimal values to floats with None fallbacks, wires these summaries into coordinator data as ChangesRanked alternatives sensor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideAdds a summarized ranking projection to the coordinator and exposes it via a new RankedAlternativesSensor, allowing Home Assistant to consume ranked alternative plan data as a lightweight sensor with JSON-serialisable attributes. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
summarize_for_sensor, you recomputecheap_rank_score(plan)even though the coordinator already maintains_cheap_ranked_alternatives; consider threading the existing score through to avoid extra work and potential drift between stored rankings and the sensor summaries if the scoring inputs diverge.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `summarize_for_sensor`, you recompute `cheap_rank_score(plan)` even though the coordinator already maintains `_cheap_ranked_alternatives`; consider threading the existing score through to avoid extra work and potential drift between stored rankings and the sensor summaries if the scoring inputs diverge.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1aff3eb to
6f1f923
Compare
470304b to
11d580a
Compare
6f1f923 to
d022187
Compare
11d580a to
d07a0bb
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Sourcery suggested threading the cheap-rank score through to ``summarize_for_sensor`` rather than recomputing. Recompute risk is near-zero (both call sites use the same pure function ``cheap_rank_score``), but the API change is small and proves the contract supports end-to-end score propagation when callers want it. - ``summarize_for_sensor(plan, *, score=None)`` — keyword-only ``score`` parameter. Default ``None`` triggers a recompute via ``cheap_rank_score`` (preserves single-call usage). Callers that already have it (future: ``cheap_rank`` could return scored tuples) pass it in. Two new tests: - ``test_pre_computed_score_threaded_through`` — passes Decimal(99.99) and confirms summary returns 99.99 unchanged (no recompute). - ``test_none_score_triggers_recompute`` — confirms default recomputes to 51.0 for peak=0.30 / supply=1.00. Coordinator still passes plan only (no scored tuples yet); a future commit can thread scores end-to-end by changing ``cheap_rank`` to return ``list[tuple[Decimal, dict]]`` without further changes to ``summarize_for_sensor``'s signature. Tests: 54/54 ranking + 724/724 full suite green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Final commit of Phase 3.1. Surfaces the daily ranking job's output as a HA sensor: state is the count of ranked alternatives; ``extra_state_attributes`` carries the per-plan summaries for the dashboard. New: summarize_for_sensor(plan) ------------------------------- ``cdr/ranking.py`` gains a per-plan summariser that compresses a CDR PlanDetailV2 body (5-15 KB raw) into a 7-field dict for attribute exposure. HA recorder warns on large attribute payloads, so dashboards consume the summary rather than the raw plan body. Returns: ``plan_id``, ``display_name``, ``brand``, ``customer_type``, ``peak_c_per_kwh``, ``supply_c_per_day``, ``score`` (cheap-rank). All values are plain str / float / None — JSON-serialisable as HA attributes require. Coordinator data dict --------------------- ``_build_data_dict()`` now emits two new keys: - ``ranked_alternatives``: list of summarised plan dicts (sorted ascending by score so [0] is cheapest). - ``ranking_last_run_at``: ISO timestamp of the last successful ranking job (None until the first run completes). RankedAlternativesSensor ------------------------ - State: count of ranked alternatives (0..top_k). 0 == job hasn't succeeded yet, no eligible plans for postcode, or all competitor retailers down. - ``extra_state_attributes``: - ``alternatives``: full summary list. - ``last_run``: ISO timestamp. - ``icon``: ``mdi:format-list-numbered``. - Registered in ``async_setup_entry`` after WinnerExplanationSensor. Tests ----- 4 new tests in ``tests/test_cdr_ranking.py::TestSummarizeForSensor``: headline-field extraction, unscored-plan None handling, missing top-level fields, JSON-serialisability (recorder contract). No tests for ``RankedAlternativesSensor`` itself — it's a thin property class consuming coordinator data, same shape as the other sensors that aren't unit-tested (HA app context required). Full suite: 717/717 pass (was 713 + 4 new). Ruff clean. Closes Phase 3.1. Users can now see cheaper alternatives without leaving their dashboard, after the daily 00:30 job populates (or they call ``pricehawk.rank_alternatives`` service manually). Refs PHASE-3-ROADMAP.md §3.1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sourcery suggested threading the cheap-rank score through to ``summarize_for_sensor`` rather than recomputing. Recompute risk is near-zero (both call sites use the same pure function ``cheap_rank_score``), but the API change is small and proves the contract supports end-to-end score propagation when callers want it. - ``summarize_for_sensor(plan, *, score=None)`` — keyword-only ``score`` parameter. Default ``None`` triggers a recompute via ``cheap_rank_score`` (preserves single-call usage). Callers that already have it (future: ``cheap_rank`` could return scored tuples) pass it in. Two new tests: - ``test_pre_computed_score_threaded_through`` — passes Decimal(99.99) and confirms summary returns 99.99 unchanged (no recompute). - ``test_none_score_triggers_recompute`` — confirms default recomputes to 51.0 for peak=0.30 / supply=1.00. Coordinator still passes plan only (no scored tuples yet); a future commit can thread scores end-to-end by changing ``cheap_rank`` to return ``list[tuple[Decimal, dict]]`` without further changes to ``summarize_for_sensor``'s signature. Tests: 54/54 ranking + 724/724 full suite green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
27a3218 to
5003b66
Compare
Summary
Final commit of Phase 3.1. Stacks on #69. Adds user-visible sensor exposing the daily ranking job's output.
What was broken
Commits 4-5 (#68, #69) wired the ranking pipeline + service but nothing exposed results to the HA frontend. Data sat on coordinator state, unreachable from sensors / dashboards / automations.
What this fixes
cdr/ranking.pygainssummarize_for_sensor(plan)— compresses a 5-15 KB PlanDetailV2 body to a 7-field JSON-serialisable dict.ranked_alternatives(list of summaries) +ranking_last_run_at(ISO timestamp).RankedAlternativesSensor:alternatives(full summary list sorted ascending by score),last_run(ISO timestamp)Now visible as
sensor.pricehawk_ranked_alternativesin HA — dashboards, automations, and templates can consume the data without polling services manually.Test plan
ruff check— cleanpytest tests/test_cdr_ranking.py -q— 52/52 pass (4 new TestSummarizeForSensor)pytest -q— 717/717 full suite pass (was 713 + 4 new)sensor.pricehawk_ranked_alternatives, verify count + attrs populate after first ranking run — deferred to runtimeChanges
custom_components/pricehawk/cdr/ranking.pysummarize_for_sensorhelpercustom_components/pricehawk/coordinator.pyranked_alternatives+ranking_last_run_atin data dictcustom_components/pricehawk/sensor.pyRankedAlternativesSensorclass + registrationtests/test_cdr_ranking.pyWhy
Summary projection (not full plan body) keeps HA recorder attribute payloads under the warning threshold (~2 KB vs ~5-15 KB per raw plan × 20 alternatives × 14-day retention = recorder bloat).
State is the count (not the cheapest plan name) because:
No tests for
RankedAlternativesSensoritself — thin property class consuming coordinator data, same shape as the other sensors that can't be unit-tested without HA app context. Underlyingsummarize_for_sensoris covered.Breaking Changes
None. Pure additive: new sensor class, new data-dict keys, no existing entity touched.
Stacked on
Depends on #68 + #69. Open against
phase-3-1-rank-serviceso the stack reads cleanly. Will retarget todevafter both ancestors merge.Files Changed
custom_components/pricehawk/cdr/ranking.py(+28)custom_components/pricehawk/coordinator.py(+12)custom_components/pricehawk/sensor.py(+44)tests/test_cdr_ranking.py(+56)🤖 Generated with Claude Code
Summary by Sourcery
Expose ranked electricity plan alternatives from the daily ranking job via a new Home Assistant sensor backed by summarized plan data from the coordinator.
New Features:
Tests:
What changed
Why
Breaking changes
Files changed