docs(modelling): handover — #1157 + #1160 closed, #1161 next

Brings HANDOVER_MODELLING.md fully current: #1157 (Plan persistence) and
#1160 (Optimiser) closed this session; records the locked design
decisions (multi-phase deferred, Plan Measure term, reuse-live-tables
via SQLModel mirrors, pure-Python knapsack not mip), the gotchas (mip/CBC
broken on aarch64, moto missing, drive-Modelling-directly for fixtures
without lodged perf, seed materials per fired measure type), and the
remaining work (#1161 ventilation Measure Dependency + deferred fronts).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Khalim Conn-Kowlessar 2026-06-03 13:11:43 +00:00
parent 34d4748a3a
commit 42d9411954

View file

@ -1,81 +1,71 @@
# HANDOVER — Modelling stage rebuild
**Branch:** `feature/bill-derivation` (worktree `/workspaces/home/hestia-worktrees/model-assemble-new-backend`). **HEAD:** `a0b6a952`.
**Branch:** `feature/bill-derivation` (worktree `/workspaces/home/hestia-worktrees/model-assemble-new-backend`). **HEAD:** `34d4748a`.
**PRD:** GitHub `Hestia-Homes/Model#1152`, sliced into #1153#1161.
## Issue status
| Issue | What | State |
|---|---|---|
| #1153 | Overlay Applicator + `EpcSimulation` | ✅ closed (`350f4c8e`) |
| #1154 | Package Scorer | ✅ closed — Elmhurst cascade pin landed (`4c0a907a`) |
| #1155 | wall Recommendation Generator | ✅ closed (`bb2c0068`); cascade-pinned (`4c0a907a`) |
| #1156 | score Options + attribution | ✅ closed (`13dd5fe8`) |
| #1157 | persist a Plan via `ModellingOrchestrator` | **not started — HITL (persistence-schema review)** |
| #1158 | roof (loft) generator | ✅ closed — 270→300 mm + cascade pin (`44d62c0c`) |
| #1159 | floor generator | ✅ closed — overlay insulation-type field + solid/suspended pins (`a0b6a952`) |
| #1160 | Optimiser (knapsack + greedy repair) | not started (blocked by #1157) |
| #1161 | Measure Dependency (ventilation) | not started (blocked by #1160) |
| #1153 | Overlay Applicator + `EpcSimulation` | ✅ closed |
| #1154 | Package Scorer | ✅ closed — Elmhurst cascade pin (`4c0a907a`) |
| #1155 | wall Recommendation Generator | ✅ closed; cascade-pinned |
| #1156 | score Options + attribution | ✅ closed |
| #1157 | persist a Plan via `ModellingOrchestrator` | **closed this session** (`772cdd4f``c7e2aa37`) |
| #1158 | roof (loft) generator | ✅ closed — 300 mm + cascade pin |
| #1159 | floor generator | ✅ closed — overlay insulation-type field + pins |
| #1160 | Optimiser (knapsack + greedy repair) | **closed this session** (`77983cae``34d4748a`) |
| #1161 | Measure Dependency (ventilation) | **NOT STARTED — next** |
## Parser gate — CLEARED (was the blocker; turned out not to be)
## What this session did
The Elmhurst recommendation Summaries route cleanly through the **same chain the
worksheet e2e fixtures use**: `pdftotext -layout``ElmhurstSiteNotesExtractor`
`EpcPropertyDataMapper.from_elmhurst_site_notes`. Helper:
`tests/domain/modelling/_elmhurst_recommendation.py::parse_recommendation_summary`.
The `parse_site_notes_pdf` Textract path still throws `'Manufacturer'` on cert
001431 (`_extract_windows` multi-token bug) — but the Modelling pins never use
it, so it does **not** block this work. The before/after Summaries are mirrored
into `tests/domain/modelling/fixtures/` so the pins don't depend on the unstaged
`/workspaces/model` workspace.
1. **Cascade pins for #1154/#1158/#1159**`tests/domain/modelling/test_elmhurst_cascade_pins.py`. Parse Elmhurst before/after recommendation Summaries via the extractor chain (NOT `parse_site_notes_pdf`), apply the generator's overlay, score, assert delta 0 vs the after-cert. Found+fixed: loft 270→**300** mm; suspended floor needs the overlay to also set `floor_insulation_type_str='Retro-fitted'`.
2. **`ProductJsonRepository`** (`cc0bb8f9`) — file-backed catalogue behind the `ProductRepository` port.
3. **#1157 — persist a Plan.** Design review (`/grill-with-docs`) + 5 TDD slices. See "Design decisions" below.
4. **#1160 — the Optimiser.** 4 TDD slices. See "Design decisions".
## Cascade pins (`test_elmhurst_cascade_pins.py`) — all 4 at delta 0
## Design decisions locked this session (READ THESE)
Each pin: parse Elmhurst `before` Summary → drive the matching generator →
score its Option's overlay through `PackageScorer` → assert `abs(diff) <= 1e-4`
on SAP/CO2/PE vs the calculator's score on the parsed `after` re-lodgement.
Two real gaps surfaced and were fixed: **loft** Elmhurst re-lodges at 300 mm
(generator was 270 → +0.17 SAP, now 300); **suspended floor** needed the overlay
to also set `floor_insulation_type_str='Retro-fitted'` (calculator's sealed/
unsealed seal logic, `cert_to_inputs.py:4111` — was +1.40 SAP). Cavity wall and
solid floor closed at delta 0 with no generator change.
- **Multi-phase is DEFERRED** (speculative prospective-client ask). **ADR-0005 rewritten to "Deferred".** No `plan_phase` table, no `phase` column. `CONTEXT.md` no longer has Scenario Phase / Plan Phase / Rolled-over Options. Everything is **single-phase**. Future: a migration adds `plan_phase` + back-fills live plans as 1-phase.
- **Plan Measure** is the new term (in `CONTEXT.md`): the persisted selected Option + its role-3 attributed impact + cost. **Recommendation** stays the *candidate* (never persisted; no stored impact).
- **Reuse the LIVE tables** (`plan`, `recommendation`) — they exist in the live product (`backend/app/db/models/recommendations.py`, SQLAlchemy `Base`) and the FE reads them. The rebuild writes the **same physical tables via SQLModel mirrors** (`infrastructure/postgres/plan_table.py`) — the established pattern (`task_table.py``tasks`, `product_table.py``material`). **ADR-0017** records this.
- Added **`recommendation.plan_id`** (FK→plan, ON DELETE CASCADE); retire the `plan_recommendations` m2m for new writes. FE-owned Drizzle migration: `docs/migrations/recommendation-plan-id.md`.
- Tracer persists **SAP + CO₂ (tonnes = calc kg ÷ 1000) + cost + derived `post_epc_rating`**. Energy/bill columns deferred. Idempotent replace per (property_id, scenario_id).
- **Optimiser = exact pure-Python multiple-choice knapsack**, NOT `mip`. Recycles `GainOptimiser`/`CostOptimiser`'s *formulation* (≤1/group, maximise gain s.t. budget) but not the dependency — **`mip`'s CBC backend does not load on this aarch64 container** (`NameError: cbclib`), so the legacy solver can't run/be tested here. ADR-0016's MILP is only a warm-start signal, so exact small-scale enumeration is ample. Re-score + greedy-repair toward the goal's SAP target gives the truth.
## Design (already recorded — read these)
## What's built (all in `domain/modelling/`, `infrastructure/postgres/`, `repositories/`, `orchestration/`)
- **CONTEXT.md** terms: Recommendation (a *target surface*; Recommendations **partition** the modifiable EPD surface so overlays never collide), Measure Option (bundle-capable; deduped by overlay), **Simulation Overlay** (`EpcSimulation`), Product, Cost, Contingency, Measure Dependency. Targeting: building parts by `BuildingPartIdentifier`; **windows by index**; systems direct.
- **ADR-0016**: the three scoring roles (per-Option signal → whole-package re-score → final-package marginal cascade attribution) + warm-start MILP → dependency injection → package re-score → greedy repair. Resolves ADR-0005 §14.
- Governing: **ADR-0005** (multi-phase scenarios, per-phase recompute vs rolling Effective EPC), **ADR-0011** (composable stage orchestrators), **ADR-0012** (one Unit of Work per stage, commit once).
- Generators: `recommend_cavity_wall` / `recommend_loft_insulation` (300 mm) / `recommend_floor_insulation` (sets `floor_insulation_type_str`).
- `simulation.py` overlay + `overlay_applicator.apply_simulations` (generic field-fold) + `package_scorer.PackageScorer.score` (role 2) + `scoring.py` (`marginal_impacts` role 3, `independent_option_impacts` role 1).
- `scenario.py` `Scenario(id, goal, goal_value, budget, is_default)`; `plan.py` `Plan` + `PlanMeasure` (derives cost_of_works/contingency_cost/co2_savings/post_epc_rating).
- `optimiser.py``optimise(groups, budget)` (exact knapsack) + `optimise_package(...)` (re-score + greedy repair, `Scorer` Protocol, `OptimisedPackage`).
- `infrastructure/postgres/`: `scenario_table.ScenarioRow`, `plan_table.{PlanRow,RecommendationRow}` (mirrors of live tables; `from_domain`).
- `repositories/`: `scenario/`, `plan/`, `product/` (Postgres + Json) — all on the `UnitOfWork` (`uow.scenario`/`uow.product`/`uow.plan`).
- `ModellingOrchestrator.run(property_ids, scenario_ids, portfolio_id)` — one UoW, commit once; generate (wall/roof/floor) → role-1 score → `optimise_package` → role-3 attribute → persist. Wired into `AraFirstRunPipeline` + `handler.py`.
- `datatypes/epc/domain/epc.py::Epc.sap_lower_bound()` (band → min SAP, target for INCREASING_EPC).
## What's built
## Gotchas (will bite a fresh agent)
All in `domain/modelling/`, `domain/building_geometry.py`, `repositories/product/`, `infrastructure/postgres/product_table.py`. **25 tests green, pyright strict clean, purely additive.**
- **`mip` / CBC is broken on aarch64** here — never build runnable code on `mip`. The legacy `recommendations/optimiser/` tests only "pass" because they avoid constructing a `mip.Model`.
- **`moto` is not installed** — `tests/orchestration/test_postcode_splitter_orchestrator.py` and `tests/repositories/unstandardised_address/` fail at *collection*. Pre-existing, unrelated; `--ignore` them when sweeping.
- **Run tests:** `python -m pytest <path> -q` (do NOT pass `-p no:cov`). Ephemeral Postgres via the `db_engine` fixture builds **only `SQLModel.metadata`** — legacy `Base` tables are absent in tests, which is why mirrors work.
- **Worktree import trap:** `python /tmp/foo.py` imports `/workspaces/model`, not this worktree. Use `pytest` (rootdir handles it) or a `python -c` from the worktree root.
- **Driving Modelling in an integration test:** the calculator fixtures (`_elmhurst_worksheet_000490.build_epc()`) lack lodged recorded-performance fields, so the **Baseline stage can't run on them**. Drive `ModellingOrchestrator` directly off a repo-seeded EPC (`EpcPostgresRepository(session).save(epc, property_id, portfolio_id)`) — see `test_modelling_optimises_and_persists_a_multi_measure_plan`. The sample API EPC (`_lodged_epc()`) does go through the full pipeline.
- **`PortfolioGoal.INCREASING_EPC` value is `"Increasing EPC"`** (with a space) — the orchestrator compares `scenario.goal == "Increasing EPC"`.
- A generator calls `products.get(...)` during candidate generation, so the integration test must **seed a `material` row for every measure type that fires** (e.g. the sample EPC's uninsulated solid floor needs `solid_floor_insulation`).
- **Don't edit the SAP calculator's `heat_transmission.py`** (another agent owns it).
- `simulation.py``EpcSimulation(building_parts: Mapping[BuildingPartIdentifier, BuildingPartOverlay])`; `BuildingPartOverlay` (all-optional: `wall_insulation_type`, `roof_insulation_thickness`, `floor_insulation_thickness`).
- `overlay_applicator.py``apply_simulations(baseline, simulations) -> EpcPropertyData`. **Generic field-fold** (adding overlay fields needs NO change here — proven by roof/floor), sequential (later overlay wins), deep-copies (baseline never mutated), targets parts by identifier, writes the `sap_*` fields. Returns a throwaway EPD.
- `recommendation.py``Recommendation(surface, options)`, `MeasureOption(measure_type, description, overlay, cost)`, `Cost(total, contingency_rate)`.
- `product.py` / `contingencies.py``Product(measure_type, unit_cost_per_m2, contingency_rate)`; per-type contingency (cavity 0.10, loft 0.10, suspended floor 0.20, solid floor 0.26).
- `package_scorer.py``PackageScorer(calculator: SapCalculator).score(baseline, simulations) -> Score(sap_continuous, co2_kg_per_yr, primary_energy_kwh_per_yr)`. The reusable scoring primitive (role 2).
- `scoring.py``marginal_impacts(scorer, baseline, overlays) -> list[MeasureImpact]` (telescoping cascade, role 3); `independent_option_impacts(scorer, baseline, options)` (role 1, scores each *distinct* overlay once). `MeasureImpact(sap_points, co2_savings_kg_per_yr, energy_savings_kwh_per_yr)`.
- `wall_recommendation.py``recommend_cavity_wall(epc, products)`: detect cavity (`wall_construction==4`) + uninsulated (`wall_insulation_type==4`) → overlay sets `wall_insulation_type=2` (Table 6 "Filled cavity").
- `roof_recommendation.py``recommend_loft_insulation(epc, products)`: detect `roof_insulation_thickness==0` → overlay `roof_insulation_thickness=270`.
- `floor_recommendation.py``recommend_floor_insulation(epc, products)`: detect uninsulated ground floor + construction (`floor_construction_type` "Suspended"/"Solid") → overlay `floor_insulation_thickness=100`.
- `building_geometry.py``gross_heat_loss_wall_area`, `roof_area`, `ground_floor_area` (per part, by identifier; party walls excluded; areas are heat-loss/§3.8 quantities, not totals).
- `repositories/product/``ProductRepository` (ABC port, `get(measure_type)->Product`); `ProductPostgresRepository` reads the externally-owned `material` table (defensive SQLModel view `MaterialRow`; `total_cost → unit_cost_per_m2`; joins contingency). A `ProductJsonRepository` (file source, for ETL-gap costs) is intended behind the same port — **the one remaining parser-independent AFK task**.
## Conventions
## Key facts / gotchas
- **Hand-built baseline fixture** (no PDF): `tests/domain/sap10_calculator/worksheet/_elmhurst_worksheet_000490.build_epc()`. Its MAIN is an uninsulated cavity wall + uninsulated suspended ground floor + 300 mm (insulated) loft. Used as the baseline in every generator/scorer test. MAIN gross heat-loss wall area = **45.93 m²**, roof area = **14.85 m²**, ground floor = **14.85 m²**.
- **Calculator entry:** `Sap10Calculator().calculate(epc) -> SapResult` (`sap_score_continuous`, `co2_kg_per_yr`, `primary_energy_kwh_per_yr`). Depend on the **`SapCalculator`** abstraction. Filled-cavity wall code = **2** (`domain/sap10_ml/rdsap_uvalues.py::u_wall`). Calculator reads wall/roof/floor from `SapBuildingPart` structured fields, NOT `EnergyElement` descriptions (those are detection-only).
- **Worktree vs main import trap:** `python /tmp/foo.py` imports the repo from `/workspaces/model` (editable install), NOT this worktree. Run with `PYTHONPATH=<worktree>` or via `pytest` (rootdir handles it). `pytest` already uses worktree code.
- **Running tests:** `python -m pytest <path> -q`. Do NOT pass `-p no:cov` (pytest.ini injects `--cov` args that then error). DB repo tests spin up ephemeral Postgres via the `db_engine` fixture (`tests/conftest.py`) — slower; SQLModel tables auto-register on import.
- **Conventions:** commit per TDD slice; conventional-commit message ending `Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>`; stay on `feature/bill-derivation` (user's choice). Tests use literal `# Arrange / # Act / # Assert`; assert with `abs(x - y) <= tol` (not `pytest.approx`); pyright strict, zero errors; annotate call-return locals.
Commit per TDD slice; conventional-commit message ending `Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>`; stay on `feature/bill-derivation`. Tests use literal `# Arrange / # Act / # Assert`; assert with `abs(x - y) <= tol` (not `pytest.approx`); pyright strict, zero errors; annotate call-return locals. Cascade pins target the worksheet at delta 0.
## What's left
1. **#1157 persist a Plan (HITL).** Design-review the Plan / Plan Phase / Recommendation persistence schema + `ScenarioRepository` method shapes **with Khalim**, then build `ModellingOrchestrator.run(property_ids, scenario_ids)` per ADR-0011/0012 (one UoW, commit once, thread only IDs, read via repos). Template: `orchestration/property_baseline_orchestrator.py`. Unblocks #1160 optimiser + #1161 ventilation dependency.
2. **`ProductJsonRepository`** behind the existing `ProductRepository` port (file source for ETL-gap costs) — the only parser-independent AFK task remaining besides #1157.
1. **#1161 — Measure Dependency (ventilation).** Per ADR-0016: a forced dependency (wall/roof insulation requires adequate ventilation) is **excluded from the optimiser's candidate pool** but **injected into the Optimised Package BEFORE the re-score**, so its (negative) SAP contribution lands in the truthful figure and the repair decision. Trigger set held as data (cf. legacy `assumptions.measures_needing_ventilation`). The injection point is `optimise_package` in `domain/modelling/optimiser.py` (inject after warm-start, before/within the re-score).
2. **Deferred fronts** (open, post-#1161): exclusion-filtering of the candidate pool (deferred from #1160); a **Bill-Derivation slice** that re-runs bills on the post-package EPC to fill the deferred energy/bill columns (`plan.post_energy_consumption`/`post_energy_bill`, `recommendation.kwh_savings`/`energy_cost_savings`); persist **unselected alternatives** (`default=False` rows linked via `plan_id`) for the swap-in UX — open ADR-0016 question: what impact figure they carry; promote `ProductRepository` to the DB+file composite; non-EPC goal objectives (Energy Savings, Reducing CO2) in the optimiser.
## Relevant memories (auto-loaded)
## Key references
- `project_openos_conservation_data_gap` — EWI eligibility needs listed/conservation status, not ingested; blocks the solid-wall EWI slice (later), NOT the fabric tracers.
- `project_calculator_geometry_extraction` — the calculator holds reusable geometry; `building_geometry.py` is the start; DRY the calculator onto it later (coordinate with the calculator branch); **don't edit `heat_transmission.py` now**.
- ADRs: **0005** (multi-phase deferred), **0011/0012** (orchestrators + UoW), **0016** (three scoring roles + warm-start/re-score/repair), **0017** (Plan persistence — evolve live tables).
- `CONTEXT.md`: Plan, Plan Measure, Recommendation, Measure Option, Optimised Package, Scenario, Measure Dependency.
- Auto-memory `project_modelling_stage_state` has the running state.