feat: add bill history sensors and fix energy_cost alignment#26
Merged
Conversation
Adds _mock_electric_bill_history() and _mock_gas_bill_history() helpers, and updates _mock_billing_account() with customerNumber and fuelTypes fields required by the bill history fetch path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds electric_bill_history and gas_bill_history fields to
NationalGridCoordinatorData, a _fetch_bill_history() method that
calls get_electric_bill_history/get_gas_bill_history from the
business portal SSO, and helper methods
get_latest_electric_bill_record/get_latest_gas_bill_record.
customerNumber is coerced to str before the API call (the model
returns int but the endpoint expects a string in the request body).
fuelTypes entries are dicts like {"type": "ELECTRIC"}, extracted
with ft.get("type") — not flat strings.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds three new meter-level sensors driven by the business-portal bill history API: - last_bill_utility_charges - last_bill_supplier_charges - last_bill_avg_daily_usage (kWh for electric, CCF for gas) Also fixes the existing energy_cost sensor: it previously used get_energy_usage_costs (consumer portal monthly estimates) which does not match billing cycles. Replacing its value_fn with _get_total_charges (from bill history) makes electric + gas totalCharges sum equal the current_bill sensor, matching what National Grid shows on the account page. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughBill history is fetched per account from the business-portal API during account refresh, cached by account and fuel type in the coordinator, and exposed via getters used by new sensor helpers. Sensors derive last-bill utility charges, supplier charges, average daily usage, and energy-cost now reads total charges from bill history. ChangesBill History Feature
Sequence DiagramsequenceDiagram
participant DataUpdateCoordinator
participant BusinessPortalAPI
participant CoordinatorData
participant SensorHelper
DataUpdateCoordinator->>BusinessPortalAPI: _fetch_bill_history(account_id, customerNumber, fuelTypes)
BusinessPortalAPI-->>DataUpdateCoordinator: electric_bill_records / gas_bill_records
DataUpdateCoordinator->>CoordinatorData: store in electric_bill_history[account_id] / gas_bill_history[account_id]
SensorHelper->>DataUpdateCoordinator: get_latest_electric_bill_record(account_id)
DataUpdateCoordinator-->>SensorHelper: latest record or None
SensorHelper->>SensorHelper: _get_total_charges(record)
SensorHelper-->>SensorHelper: return totalCharges or None
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/test_coordinator.py (1)
1095-1119: ⚡ Quick winAssert
customerNumberis passed asstrto bill-history API calls.These tests validate stored results, but they don’t currently lock down the coercion contract. A regression could pass an
intand still pass with permissive mocks.Proposed test hardening
async def test_fetch_bill_history_electric(hass: HomeAssistant) -> None: @@ records = coordinator.data.electric_bill_history.get(MOCK_ACCOUNT_ID, []) assert len(records) == 2 assert records[0]["utilityCharges"] == 98.40 assert records[0]["supplierCharges"] == 47.10 assert records[0]["avgDailyUsage"] == 16.77 + call = api.get_electric_bill_history.await_args + customer_number = call.kwargs.get("customer_number") + if customer_number is None and call.args: + customer_number = call.args[0] + assert isinstance(customer_number, str) async def test_fetch_bill_history_gas(hass: HomeAssistant) -> None: @@ records = coordinator.data.gas_bill_history.get(MOCK_ACCOUNT_ID, []) assert len(records) == 1 assert records[0]["utilityCharges"] == 28.80 assert records[0]["avgDailyUsage"] == 1.03 + call = api.get_gas_bill_history.await_args + customer_number = call.kwargs.get("customer_number") + if customer_number is None and call.args: + customer_number = call.args[0] + assert isinstance(customer_number, str)🤖 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 `@tests/test_coordinator.py` around lines 1095 - 1119, The tests test_fetch_bill_history_electric and test_fetch_bill_history_gas currently validate stored records but don’t assert the type of customerNumber passed to the bill-history API; update each test (using the existing _make_api mock and MOCK_ACCOUNT_ID) to also assert that the API’s bill-history call was invoked with customerNumber equal to str(MOCK_ACCOUNT_ID) (i.e., a string), for example by inspecting the mock call args after coordinator._async_update_data() completes so the contract enforces coercion to str before calling the API.tests/test_sensor.py (1)
448-463: ⚡ Quick winAdd gas-path coverage for
_get_supplier_charges.A gas-specific assertion is missing here; a gas-branch regression could slip through while current tests still pass.
Proposed additional test
def test_get_supplier_charges_electric() -> None: @@ coordinator.get_latest_electric_bill_record.return_value = electric_rec assert _get_supplier_charges(coordinator, meter_data) == 47.10 +def test_get_supplier_charges_gas() -> None: + """Test _get_supplier_charges returns supplierCharges from gas record.""" + meter_data = _make_meter_data("Gas") + coordinator = MagicMock() + coordinator.get_latest_gas_bill_record.return_value = _make_gas_bill_record() + assert _get_supplier_charges(coordinator, meter_data) == 16.20 + def test_get_supplier_charges_none_when_no_record() -> None:🤖 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 `@tests/test_sensor.py` around lines 448 - 463, Add a gas-path unit test mirroring the electric tests: create a meter_data via _make_meter_data("Gas"), stub coordinator.get_latest_gas_bill_record to return a gas bill record (use or add _make_gas_bill_record if needed), call _get_supplier_charges(coordinator, meter_data) and assert it equals the expected supplierCharges value; also add a companion test where coordinator.get_latest_gas_bill_record returns None and assert _get_supplier_charges returns None. Ensure tests are named e.g. test_get_supplier_charges_gas and test_get_supplier_charges_gas_none and reference _get_supplier_charges, _make_meter_data, coordinator.get_latest_gas_bill_record, and _make_gas_bill_record so the gas branch is covered.
🤖 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.
Nitpick comments:
In `@tests/test_coordinator.py`:
- Around line 1095-1119: The tests test_fetch_bill_history_electric and
test_fetch_bill_history_gas currently validate stored records but don’t assert
the type of customerNumber passed to the bill-history API; update each test
(using the existing _make_api mock and MOCK_ACCOUNT_ID) to also assert that the
API’s bill-history call was invoked with customerNumber equal to
str(MOCK_ACCOUNT_ID) (i.e., a string), for example by inspecting the mock call
args after coordinator._async_update_data() completes so the contract enforces
coercion to str before calling the API.
In `@tests/test_sensor.py`:
- Around line 448-463: Add a gas-path unit test mirroring the electric tests:
create a meter_data via _make_meter_data("Gas"), stub
coordinator.get_latest_gas_bill_record to return a gas bill record (use or add
_make_gas_bill_record if needed), call _get_supplier_charges(coordinator,
meter_data) and assert it equals the expected supplierCharges value; also add a
companion test where coordinator.get_latest_gas_bill_record returns None and
assert _get_supplier_charges returns None. Ensure tests are named e.g.
test_get_supplier_charges_gas and test_get_supplier_charges_gas_none and
reference _get_supplier_charges, _make_meter_data,
coordinator.get_latest_gas_bill_record, and _make_gas_bill_record so the gas
branch is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d65a34eb-233c-4a4d-93f4-4ca0a23b3d2c
📒 Files selected for processing (8)
custom_components/national_grid/coordinator.pycustom_components/national_grid/sensor.pycustom_components/national_grid/strings.jsoncustom_components/national_grid/translations/en.jsonrequirements_test.txttests/conftest.pytests/test_coordinator.pytests/test_sensor.py
…s tests
In test_coordinator.py: assert get_electric_bill_history and
get_gas_bill_history are called with customerNumber as a string
("987654321") so the int→str coercion in _fetch_bill_history is
a hard contract, not just an implementation detail.
In test_sensor.py: add test_get_supplier_charges_gas and
test_get_supplier_charges_gas_none to cover the gas branch of
_get_supplier_charges, mirroring the existing electric tests.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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.
What
Adds bill history sensors powered by the National Grid business portal API (
get_electric_bill_history/get_gas_bill_historyfrom py-nationalgrid 0.6.3). These return per-billing-period breakdowns not available from the existing GraphQL billing calls.Also fixes the existing Last Billing Cost (
energy_cost) sensor: it previously sourced fromget_energy_usage_costs(consumer portal monthly estimates) which does not correspond to actual billing periods. Replacing it withtotalChargesfrom bill history makeselectric.totalCharges + gas.totalCharges = current_bill, matching what National Grid shows on the account page. Note: on solar-heavy monthstotalChargescan be less thanutilityChargesalone because National Grid applies credits after the utility+supplier calculation.New sensors (meter-level, one set per meter)
last_bill_utility_chargeslast_bill_supplier_chargeslast_bill_avg_daily_usageCoordinator changes
electric_bill_history/gas_bill_historyfields added toNationalGridCoordinatorData_fetch_bill_history()called after_fetch_bills()in_fetch_account_data()customerNumbercoerced tostr(model returnsint, endpoint expects a string)fuelTypesentries are dicts{"type": "ELECTRIC"}— extracted withft.get("type"), not cast to string directlyTest plan
pytestpasses (221 tests, 100% coverage)ruff check/ruff format --checkcleanlast_billing_costsensors sum equalscurrent_billin live HA🤖 Generated with Claude Code
Summary by CodeRabbit