mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-07-12 13:29:04 +00:00
68 lines
5.6 KiB
Markdown
68 lines
5.6 KiB
Markdown
# PRD: Extract ventilation audit sheet population into the magicplan domain
|
||
|
||
**Status:** Backlog
|
||
|
||
---
|
||
|
||
## Problem Statement
|
||
|
||
The logic that maps a `Plan` into spreadsheet cells — which column receives `pct_openable / 100`, which rows are rooms vs windows vs doors, what the 50-row capacity limit is, how column Y conditional formatting is applied — currently lives inside the orchestrator. Developers reading `AuditGeneratorOrchestrator` have to wade through cell-writing details to understand the orchestration flow, and there is no way to test the sheet-population rules in isolation without invoking the full orchestrator (which requires a mocked UoW, mocked S3, and the real XLSX template file).
|
||
|
||
## Solution
|
||
|
||
Move all sheet-population logic into the magicplan domain as a dedicated module (`ventilation_audit`), exposing a single public function `populate_sheet(sheet, plan)`. The orchestrator delegates to this function and retains only its infrastructure responsibilities: loading the template, serialising the workbook, uploading to S3, and persisting metadata.
|
||
|
||
This makes the mapping rules directly testable against a plain `openpyxl` sheet with no orchestration overhead, and keeps the orchestrator focused on coordination rather than domain rules.
|
||
|
||
## User Stories
|
||
|
||
1. As a developer debugging a malformed audit spreadsheet, I want the cell-mapping rules to live in the domain so that I can locate the logic without reading through orchestration code.
|
||
2. As a developer writing a test for ventilation audit content, I want to call `populate_sheet` directly with a synthetic `Plan` and a blank sheet so that I can assert cell values without mocking S3 or a unit of work.
|
||
3. As a developer adding a new opening type or ventilation field, I want the affected mapping logic to be co-located with the `Plan` domain models so that the change is easy to find and the impact is obvious.
|
||
4. As a developer reading the orchestrator, I want the `run()` method to read as a sequence of high-level steps (fetch → populate → serialise → upload → persist) with no cell-writing detail so that the orchestration intent is immediately clear.
|
||
5. As a developer running the test suite, I want the 50-row overflow validation to be covered by a domain-level test so that regressions in that constraint are caught without running the full orchestrator.
|
||
6. As a developer extending the audit template to a second sheet, I want the sheet-population contract to be a clearly bounded function so that I can add a second `populate_*` function in the same module without touching the orchestrator.
|
||
|
||
## Implementation Decisions
|
||
|
||
- **New module `domain/magicplan/ventilation_audit.py`** contains the public function `populate_sheet(sheet, plan)` and all private helpers (`_write_cell`, `_apply_column_y_formatting`) and constants (`_DATA_START_ROW`, `_MAX_ROWS`, `_Y_CF_RANGE`, `_Y_THRESHOLD`, `_Y_HEADER`). These are moved verbatim from the orchestrator — no logic changes.
|
||
|
||
- **`populate_sheet` is the sole public surface.** Helpers remain private to the module. This follows the existing `mapper.py` pattern (stateless module-level functions, no class wrapper).
|
||
|
||
- **The orchestrator imports `populate_sheet`** and replaces its `_populate_sheet(sheet, plan)` call. All `openpyxl.cell.rich_text`, `openpyxl.cell.text`, `openpyxl.formatting.rule`, and `openpyxl.styles` imports move with the logic. `openpyxl.load_workbook` stays — loading the template is an infrastructure step.
|
||
|
||
- **`_serialise_workbook` stays in the orchestrator** — converting a workbook to bytes is a serialisation step, not domain logic.
|
||
|
||
- **No interface change to the orchestrator's public API** — `AuditGeneratorOrchestrator.__init__` and `run()` signatures are unchanged.
|
||
|
||
## Testing Decisions
|
||
|
||
Good tests for `populate_sheet` assert observable outputs (cell values, conditional formatting rule count) given a controlled `Plan` input. They do not assert on internal call sequences or private helper invocations.
|
||
|
||
Tests should use a fresh `openpyxl.Workbook().active` sheet — no template file needed, which keeps them fast and dependency-free.
|
||
|
||
Modules to test (new file: `tests/domain/magicplan/test_ventilation_audit.py`):
|
||
|
||
| Scenario | Assertion |
|
||
|---|---|
|
||
| Rooms written correctly | Col B = room name, col D = area_m2, starting at `_DATA_START_ROW` |
|
||
| Windows written correctly | Cols G–I, K–M, Q–R populated; pct_openable divided by 100 |
|
||
| Windows with null ventilation | Ventilation columns default to 0 |
|
||
| Doors written correctly | Cols V–X populated with room name, width_mm, undercut_mm |
|
||
| Room overflow | > 50 rooms raises `ValueError` |
|
||
| Window overflow | > 50 windows raises `ValueError` |
|
||
| Door overflow | > 50 doors raises `ValueError` |
|
||
| Column Y formatting applied | Sheet has two conditional formatting rules after `populate_sheet` |
|
||
|
||
Prior art: `tests/orchestration/audit_generator/test_audit_generator_orchestrator.py` shows the `_make_plan` / `_make_window` / `_make_door` fixture pattern to reuse. The existing orchestrator tests need no changes.
|
||
|
||
## Out of Scope
|
||
|
||
- Changes to the spreadsheet template or column layout.
|
||
- Support for plans with more than 50 rooms, windows, or doors (the 50-row limit is a template constraint, not lifted here).
|
||
- Extracting `_serialise_workbook` or template-loading into the domain.
|
||
- Any changes to the `AuditGeneratorOrchestrator` public API or the Lambda entry point.
|
||
|
||
## Further Notes
|
||
|
||
The orchestrator test suite already provides integration-level coverage (S3 call order, `UploadedFile` enums, error paths). This refactor adds the missing unit-level coverage for the mapping rules, which are currently exercised only incidentally via the happy-path orchestrator tests.
|