Add UK transport and energy source facts - #255
Conversation
vahid-ahmadi
left a comment
There was a problem hiding this comment.
Review at b3cc9c5d with the bundle-count follow-up 11314345 read afterwards (Claude Code, high effort; fresh checkout; every one of the 16 packages validate-package clean; 16 of 16 artifact sha256s match the committed files, 14 of 16 re-downloads are byte-identical and the two HTML pages differ only in a CSRF and a Sentry token; 904 passed / 1 skipped on the suite excluding the bundle test; the bundle test, which the body says was stopped after nine minutes, was run to completion — see 1).
More than thirty cells across 14 packages spot-checked against the pinned artifacts all match: HMRC 2020-21 petrol 11,670.76 million litres, DESNZ January 2023 ULSP 148.451 p/l, NEED detached-gas mean 15,518 kWh, ORR 2015-16 total support £10,852m and 2024-25 fare income £11,252m, the 1995 = 100 fares index, Scotland's 2.9 support split summing to 438.9, the Ofgem Q1 2024 four GB rates, the Welsh journeys, all 49 OBR numbers with rows summing. Provenance mechanics are sound and no bus05i, bus0415 or nts0705 file is touched, so microcosm's #874 bus bindings are unaffected. The problems are in period semantics and metadata, not values.
Findings
1. Was blocking, fixed at 11314345. At the reviewed head the whole-database bundle test failed on two stale expectations (geography_count 12539 vs 12540, period_count 269 vs 272) — the body's "had not failed" was untested. The follow-up commit sets 12540 and 272; the +1 geography is the new desnz:england_and_wales id (finding 3) and the +3 periods are the new months. The fact count 196,389 (+1,196) is now machine-checked.
2. Blocking — sub-annual facts are carried under annual and monthly period types, and ALLOWED_PERIOD_TYPES (chronicle/core.py:34-40) has no quarter or week. Three packages:
ons/consumer_trends_current_price_2026: 50 of 62 record sets are quarterly rows (e.g.ons.consumer_trends.04cn.2026_q1) declaredperiod_type: calendar_yearwith the quarter only infilters. A consumer selecting calendar-year 2020 for a COICOP class gets the annual row plus four quarterly rows under the same period — a double count unless it knows to filterfrequency— and the 2026 "calendar year" fact is one quarter.ofgem/energy_price_cap_q1_2024: the January–March 2024 cap iscalendar_year 2024.desnz/weekly_road_fuel_prices_september_2026: 193 weekly observations declaredperiod_type: month, four or fiveaggregation: meanfacts sharing each month.
Either drop these rows (the monthly and annual DESNZ package and the ONS annual rows already cover the consumer's stated needs) or extend the schema; they should not ship ascalendar_yearormonth.
3. Should-fix — no period_coverage on any of the 16 packages (0 of 747 record sets), while the repo's own bus packages declare it with start and end dates and a basis. The values are right — every fiscal package uses the opening year: HMRC "2020 to 2021" → 2020, ORR "Apr 2024 to Mar 2025" → 2024, DfI "2019-20" → 2019, Transport Scotland "2023-24" → 2023, Welsh "2024 to 2025" → 2024, OBR "2022-23" → 2022 — but the convention is undeclared, which is exactly the failure mode #874 hit with DfT's closing-year labels.
4. Should-fix — row guards pin the wrong column in several packages. dft/veh1103 row 1951 is guarded by D == 'Cars' only, which every cars row satisfies, while the discriminating "2023 Q4 (end December)" in column B is unguarded (and the fact is an end-December snapshot labelled as the calendar year without saying so); Transport Scotland Table 2.9 rows 6, 9, 15, 18 are guarded on column B, which rows 6–8 share, with the Scotland / GB / outwith-London split in column C unguarded; the DESNZ 4.1.1 rows guard the year only; the Ofgem, OBR and Welsh document_numbers rows guard on the number text itself, which is tautological — guard on the context text.
5. Should-fix — desnz:england_and_wales is a non-standard geography id (72 record sets in the NEED package); existing packages use K04000001.
6. Should-fix — the fuel-duty rate is in the pinned artifact and not emitted. The weekly workbook's column E is "ULSP: Duty rate (p/litre)", 52.95 from 2023-01-02 through 2026-09-07 (the 57.95 → 52.95 cut predates the emitted range; no 2026 change appears as of 7 September). The consumer's spend-to-litres conversion needs it, and it is one measure away.
7. Should-fix — two source_page URLs are dead (the artifact URLs all resolve): the DfI page is the discontinued series #254 warned about (live: …/public-transport-statistics-northern-ireland-2024-25), and the Transport Scotland chapter page moved (live: …/chapter-2-bus-and-coach-travel/).
8. Should-fix — docs/pe-uk-source-checklist.md not updated for road_fuel_volume.py, lcfs_consumption_anchors.json, need_energy_targets.json or rail subsidy, all named in #254 as what these packages close.
Questions
- Ofgem boundary. The pinned page itself links regional rates for October 2023 to March 2024; the "four GB rates, not a regional matrix" boundary describes the chosen press release, not what Ofgem publishes. And #254 asked for cap periods from at least Q1 2024 while the consumer prices at Q2 2026; one quarter does not close that item and the body does not declare it as a boundary.
- ORR 7271 versus 7270 differ by definition (2024-25 £11,857m "total support" versus £21,621m including the Network Rail grant); both are emitted under different concepts, and the consumer needs to know which the #790 rail scaler binds.
- The Scotland NEED "region" record set is the all-dwellings column of Table 4 by property type; the boundary is true, the label is not.
- The three LLM-judge verdicts are still "not run".
Nits
New ruff I001 in the issue-254 test; ruff check . is 233 on the PR versus 232 on main, so "ruff passed" is not clean on either branch. The test table's years disagree cosmetically with artifact_year for four packages. The monthly package's manifest cites sheets 4.1.1 and 4.1.2 while the file name says 411_413.
Not verified
R2 object existence, the LLM-judge verdicts, and consumer behaviour in microcosm beyond the bus concepts being untouched.
Not mergeable yet: 2 is a semantics problem that will silently double-count any consumer that filters on period alone. 3 through 8 are metadata, all cheap. The values themselves are clean throughout.
|
@vahid-ahmadi Thanks for the detailed review — addressed in 5360644.
The reviewed packages now contribute 1,385 publisher facts. Local verification is green: 910 passed / 1 skipped outside the bundle module, all 14 fast bundle tests passed, the full merged-bundle contract test passed in 21m36s, and Ready for a second review pass. |
vahid-ahmadi
left a comment
There was a problem hiding this comment.
Second pass at 53606447 (Claude Code, high effort; fresh checkout of the response commit read against the round-one findings; all 16 packages validate-package clean; ruff check clean at the pinned 0.14.10; the suite outside the bundle module 910 passed / 1 skipped, and the whole-database bundle test — the one the first body had stopped — 14 passed in 15 minutes; the 27-file diff since 11314345 touches only what the dispositions require).
1 and 2 — both closed, and closed properly. ALLOWED_PERIOD_TYPES now carries quarter and week; the 50 ONS quarterly record sets are period_type: quarter with YYYY-QN values and the 12 annual rows stay calendar_year, so a consumer filtering on the year no longer picks up the quarters; the Ofgem cap is 2024-Q1; the 193 weekly DESNZ observations are ISO weeks (2023-W01 onward). The bundle test now pins the new period vocabulary (quarter and week counts per period) and the totals reconcile exactly: 196,389 + 193 duty-rate facts − 4 Scotland pseudo-region facts = 196,578, geography count back to 12,539 with the England-and-Wales id normalised to K04000001.
3. Verified. Every record set in all 16 packages declares period_coverage — 561 of 561 by my count across the packages (47, 72, 42, 193, 12, 3, 84, 7, 4, 62, 10, 10, 7, 1, 4, 3). The opening-year convention is now declared rather than inferred.
4 through 8. Verified. The discriminating columns are now guarded in the DfT, Transport Scotland, DESNZ, OBR, Ofgem and Welsh packages; the two source pages resolve (both 200); the DESNZ table naming matches the file; the test artifact years are corrected; the checklist rows are added; the fuel-duty rate is emitted as 193 desnz.road_fuel.ulsp_duty_rate facts at 52.95 p/l from column E; the four Scotland NEED pseudo-region facts are gone and the retained property-type figures are labelled as all-Scotland totals.
Questions. The 7270 versus 7271 distinction (broad total including operational, enhancement and miscellaneous support versus the operational source-by-recipient matrix) is now in the package notes, with the scaler binding left to the consumer; the Ofgem package states its four GB-average direct-debit rates as partial coverage and claims neither the regional matrix nor a Q2 2026 vintage. The three LLM-judge verdicts remain un-run, as the body says.
Two notes for later, neither blocking. validate_fact still does no value-format check for week or quarter values (the same is true of month, so pre-existing in kind). And a weekly period_coverage is the single observation Monday rather than the seven-day span — consistent with the checklist wording, but a consumer should know that is the convention.
Approving. The values were clean in round one; the semantics are now right too.
Summary
Source boundaries
Chronicle governance
ledger-source-ingestorledger-source-fidelity: not runledger-contract: not runledger-boundary: not runTests
uv run --locked pytest -q --ignore=tests/test_chronicle_bundle.py— 898 passed, 7 skippeduv run --locked pytest tests/test_chronicle_bundle.py -q -k 'not test_build_bundle_writes_merged_consumer_contract'— 13 passed, 1 deselecteduv run --locked pytest tests/test_issue_254_transport_energy_sources.py -q— 34 passeduv run --locked ruff check .uv run --locked ruff format --check chronicle/source_package.py chronicle/bundle.py tests/test_issue_254_transport_energy_sources.pyThe whole-database bundle integration test was also started and stopped after more than nine minutes while inserting existing database rows; it had not failed.
Fixes #254