mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-06-08 11:17:27 +00:00
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>
71 lines
8.6 KiB
Markdown
71 lines
8.6 KiB
Markdown
# HANDOVER — Modelling stage rebuild
|
||
|
||
**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 |
|
||
| #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** |
|
||
|
||
## What this session did
|
||
|
||
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".
|
||
|
||
## Design decisions locked this session (READ THESE)
|
||
|
||
- **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.
|
||
|
||
## What's built (all in `domain/modelling/`, `infrastructure/postgres/`, `repositories/`, `orchestration/`)
|
||
|
||
- 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).
|
||
|
||
## Gotchas (will bite a fresh agent)
|
||
|
||
- **`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).
|
||
|
||
## Conventions
|
||
|
||
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. **#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.
|
||
|
||
## Key references
|
||
|
||
- 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.
|