From 42d941195462a3c9f4dc149b61bc4a038dfb358e Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 3 Jun 2026 13:11:43 +0000 Subject: [PATCH] =?UTF-8?q?docs(modelling):=20handover=20=E2=80=94=20#1157?= =?UTF-8?q?=20+=20#1160=20closed,=20#1161=20next?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/HANDOVER_MODELLING.md | 106 +++++++++++++++++-------------------- 1 file changed, 48 insertions(+), 58 deletions(-) diff --git a/docs/HANDOVER_MODELLING.md b/docs/HANDOVER_MODELLING.md index d960d1b8..e4ad963d 100644 --- a/docs/HANDOVER_MODELLING.md +++ b/docs/HANDOVER_MODELLING.md @@ -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 -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=` or via `pytest` (rootdir handles it). `pytest` already uses worktree code. -- **Running tests:** `python -m pytest -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 `; 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 `; 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.