Repository navigation
Stop CPI-uprating energy, which the data build already prices - #1868
Conversation
Reverses the direction of #1860, which I wrote. Fixes #1867. policyengine-uk-data calibrates the energy variables to NEED mean kWh by income band and converts to pounds at Ofgem Q2 2026 unit rates. Its imputations/consumption.py says so outright: "the stored £/yr values represent FY26/27 price levels. Energy variables are not CPI-uprated, so this avoids the need for price-level adjustment at simulation time." uprating_indices.yaml uprates from the dataset's DATA year, 2024. So applying CPI to these variables re-applies 2024-2026 price changes the build had already priced in — about 5.8pp of a 7.9% uplift. The cap parameter falls over this period, which makes the stored price level testable. On enhanced_frs_2024_25 the modelled mean of electricity + gas is £1,584, against cap parameters of £2,389 (2024), £1,849 (2025) and £1,641 (2026). A 2024-priced series would have to sit near the 2024 cap; it sits just below the 2026 one, which is the April 2026 level the docstring describes. #1859 read the symptom backwards and #1860 followed it. The flat pair was right — accidentally, being absent from the YAML rather than deliberately excluded — and domestic_energy_consumption, on the CPI list since before 2.90.2, was carrying the double count. #1860 restored the documented identity by propagating the defect to all three. Removing all three restores the identity AND holds the level: electricity £895 + gas £689 = £1,584 = domestic, in every year from 2024 to 2027. Other consumption variables come from the LCFS at data-year prices and do need uprating, so only these three are excluded. The rationale is recorded at the top of the YAML, and a code-health test asserts they appear under no index. Verified that test fails when one is re-added rather than only passing today. Downstream: modelled energy spending in projected years falls about 8%, back to the build's own price level. Anyone with pinned regression values on domestic_energy_consumption will see them move; that movement is the correction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Program reviewBase repository: PolicyEngine/policyengine-uk Source Documents
CriticalC1 — CRITICAL (Must Fix): the new changelog file is not a Towncrier fragment and will be omitted from the release (OPEN)Location: Trigger: run the repository's release command ( Impact / why this diff: this PR adds the malformed file, so its release note is silently absent. This is a demonstrated failure of an explicit repository completion requirement, which the workflow rubric classifies as critical rather than a style suggestion. Should AddressA1 — SHOULD ADDRESS — Scope the “not uprated by any index” invariant to dataset projection (OPEN)
Coordinator assessment: Complementary executed evidence from the code review: the new YAML guard and the dataset freeze/identity diagnostic pass, while a separate direct-situation path still applies the unchanged class-level CPI metadata (2024→2027 factor 1.078934022). That observation proves the wording/test are overbroad, but not that arbitrary manual inputs carry the dataset artifact's Q2-2026 price basis or produce an incorrect result. A2 — SHOULD ADDRESS: published growth-factor documentation still says domestic energy is CPI-uprated (OPEN)Location: Trigger: consult the economic-assumptions documentation after this change. Expected: the CPI list excludes the three FY26/27-priced energy variables and explains the exception. Observed: line 19 still says CPI applies to all consumption categories and explicitly lists SuggestionsNone. Evidence Gaps
Notes
Validation SummaryRemote CI: 6/6 checks passed at the reviewed head. Local bounded invocation: the changed guard, two relevant repository tests, the dataset freeze/identity diagnostic and three direct-path observations passed (7 passed); one diagnostic snapshot-path assertion false-failed only because macOS canonicalizes /tmp as /private/tmp. git diff --check passed. Towncrier 26.9 draft rendered no fragment. Source cache: 3/3 originals validated, 0 rejected, 0 derivatives; no PDF renders. Timingsetup seconds: 4.00s; scope seconds: 115.00s; parallel review seconds: 853.00s; policy role seconds: 853.00s; code role seconds: 723.00s; adjudication seconds: 36.00s; consolidation cleanup seconds: 82.00s; elapsed seconds: 1005.00s Review SeverityREQUEST_CHANGES. Open findings: 1 critical, 2 should address, 0 suggestions. |
Addresses the three findings on this PR. C1: the changelog fragment had no type component, so towncrier omitted it entirely — the draft rendered "No significant changes". Renamed to energy-not-cpi-uprated.fixed.md, per changelog.d/<branch>.<type>.md in CONTRIBUTING, and the leading bullet removed so towncrier does not double it. Verified the draft now renders it under Fixed. A1: the wording claimed more than the change delivers, and the reviewer is right. uprating_indices.yaml governs dataset projection. It does not govern the class-level `uprating` attribute, which policyengine-core still uses to carry a direct situation input forward: a 2024 electricity_consumption input queried at 2027 returns 1078.93 against 1000, which I confirmed directly. That path is deliberately left alone. A value a user supplies for 2024 may be their own contemporaneous bill rather than this build's artifact, so it has no claim on the build's price basis. The YAML header, the test docstring and the test name now say "projection" rather than making a global claim. Worth recording because #1860 and #1862 both described the class-level attribute as dead metadata that nothing reads. That is true of dataset projection and false of the direct-input path; core reads it there. A2: docs/book/assumptions/growthfactors.md still listed domestic_energy_consumption under "all consumption categories" get CPI. The list now excludes the three energy variables and states why. Full suite: 266 passed, 45 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
All three findings fixed. C1 — the fragment was genuinely broken. A1 — the wording claimed more than the change delivers, and you're right. I confirmed the path directly rather than taking it: So Scoped rather than extended, for the reason you give: a value a user supplies for 2024 may be their own contemporaneous bill, not this build's artifact, so it has no claim on the build's price basis. The header, the test docstring and the test name now say projection. The test itself is unchanged in what it checks — it was always a YAML check; only the claim around it was too broad. This also corrects something I asserted twice. #1860's root-cause section and #1862 both called the class-level A2 — On the evidence gap you flagged — whether 2027+ should stay in constant FY26/27 pounds or follow a nominal energy-price series — I agree that is unresolved and should not be settled inside this PR. This PR removes a demonstrable double count; what the right post-anchor projection is needs the data build's own contract, since only it knows the anchor. Worth its own issue against On the £1,641 comparison — fair, and I should have said so. The builder excludes standing charges, so mean spend against a typical-consumption cap is not like-for-like. It was directional evidence for which year the level sits in, not a level check; your unit-rate and kWh verification is the right test. Full suite: 266 passed, 45 skipped. |
Program reviewBase repository: PolicyEngine/policyengine-uk Source Documents
CriticalC1 — RESOLVED (was CRITICAL): Towncrier now recognizes and renders the fragment (RESOLVED)
Should AddressA1 — RESOLVED (was SHOULD ADDRESS): the invariant is now correctly scoped to dataset projection (RESOLVED)
Coordinator assessment: Policy/source confirmation: the scoped wording no longer assigns the dataset artifact's Q2-2026 price basis to arbitrary direct inputs, so it resolves the prior overbroad contract without inventing a source premise. A2 — RESOLVED — Growth-factor documentation now matches the exact variable split (RESOLVED)
SuggestionsNone. Evidence Gaps
Notes
Validation SummaryIncremental ancestry and exact head verified. One bounded local invocation passed 8/8 checks with 0 failures (pytest 2.94s; runner 3.90s); Towncrier 26.9 rendered the typed fragment under Fixed; full and incremental git diff --check passed. Source cache revalidated 3/3 originals with 0 rejected or discarded derivatives and no new acquisition/renders. Remote CI: 6 passed, main Test pending at capture. Timingsetup seconds: 4.00s; scope seconds: 84.00s; parallel review seconds: 235.00s; policy role seconds: 294.00s; code role seconds: 231.00s; adjudication seconds: 0.00s; consolidation cleanup seconds: 64.00s; elapsed seconds: 375.00s Review SeverityCOMMENT. Open findings: 0 critical, 0 should address, 0 suggestions. |
|
All three findings resolved, and CI is now 7/7 including the Test job that was in progress at your capture. I have raised the evidence gap as PolicyEngine/policyengine-uk-data#485 rather than leaving it in a review thread. The question is which of two readings the build intends: that the values are deliberately constant FY26/27-price measures after the anchor, or that they are nominal and should follow an approved energy-price path once Ofgem's Q2 2026 rates run out. Only that repository can answer it, since only it knows the anchor. I flagged there that it matters beyond bookkeeping: downstream cap and subsidy calculations combine these values with cap parameters that do move by year, so a real series beside a nominal cap drifts with nobody intending it. And On your non-blocking note about the guard: agreed that a custom energy mutation added outside the YAML would evade a configuration test, and that an end-to-end projection assertion would be defence in depth. I have left it as a configuration test because YAML membership is the mechanism this PR changes, and a test that asserts more than the change controls is what produced finding A1 in the first place. Worth adding when there is a second projection mechanism to defend against. Ready to merge whenever you are happy — I am not merging it myself, since the last time a change of mine went in unreviewed it took two rounds of your review to find what it had left stale. |
87c8ae3 to
c6bcc2d
Compare
…lose the gaps it now finds - ruff format test_uprating_declarations.py (the Lint failure). - Rename the fragment to uprate-four-remaining-columns.fixed.md so towncrier renders it under Fixed rather than dropping it. - After rebasing onto main, the guard flags ten variables declared since: seven are genuine gaps and now uprate by the index they declare (cash_isa, stocks_and_shares_isa, mortgage_debt, consumer_debt, rent_paid_as_boarder, rent_paid_as_lodger by per-capita GDP; self_employment_gross_receipts by per-capita mixed income); the three energy columns are listed as deliberately unuprated, per #1868. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lose the gaps it now finds - ruff format test_uprating_declarations.py (the Lint failure). - Rename the fragment to uprate-four-remaining-columns.fixed.md so towncrier renders it under Fixed rather than dropping it. - After rebasing onto main, the guard flags ten variables declared since: seven are genuine gaps and now uprate by the index they declare (cash_isa, stocks_and_shares_isa, mortgage_debt, consumer_debt, rent_paid_as_boarder, rent_paid_as_lodger by per-capita GDP; self_employment_gross_receipts by per-capita mixed income); the three energy columns are listed as deliberately unuprated, per #1868. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 participants
Fixes #1867. This reverses the direction of #1860, which I wrote and @juaristi22 reviewed — neither of us caught it at the time.
The defect
policyengine-uk-datacalibrates the energy variables to NEED mean kWh by income band and converts to pounds at Ofgem Q2 2026 unit rates. Itsimputations/consumption.pysays so outright:But
uprating_indices.yamluprates from the dataset's data year, 2024. So applying CPI to these variables re-applies 2024–2026 price changes the build had already priced in — roughly 5.8pp of a 7.9% uplift.Evidence
The cap parameter falls over this period, which makes the stored price level testable. On
enhanced_frs_2024_25:gov.ofgem.energy_price_cap2024gov.ofgem.energy_price_cap2025gov.ofgem.energy_price_cap2026A 2024-priced series would have to sit near the 2024 cap. It sits just below the 2026 one — the April 2026 level the docstring describes.
Why #1860 was the wrong fix
#1859 observed that
electricity_consumptionandgas_consumptionwere flat whiledomestic_energy_consumptionrose, and treated the flat pair as the defect. It was the other way round. The flat pair was right — accidentally, being absent from the YAML rather than deliberately excluded — anddomestic_energy_consumption, on the CPI list since before 2.90.2, was carrying the double count.#1860 restored the documented identity by propagating the defect to all three. The identity held; the price level was wrong for all of them.
The fix
Removing all three restores the identity and holds the level:
Other consumption variables come from the LCFS at data-year prices and do need uprating, so only these three are excluded. The rationale is recorded at the top of the YAML so the absence reads as deliberate rather than as the gap #1859 reported.
A code-health test asserts they appear under no index. I verified it fails when one is re-added, rather than only passing today.
Verification
Full suite: 266 passed, 45 skipped.
Downstream
Modelled energy spending in projected years falls about 8%, back to the build's own price level. Anyone with pinned regression values on
domestic_energy_consumptionwill see them move — that movement is the correction.For
impact-iran-war-living-standards, which defines every scenario percentage against the pre-conflict April–June 2026 baseline, this means the baseline is already at the right price level and takes no uplift. I had been reporting a pending +4% correction to that study's headline on the strength of #1860; that figure is withdrawn.Interacts with #1863 — if that merges first its guard will need an exception entry for these three, pointing here.