# Wayleave Coverage Approvals — one model for all four sources - **Date:** 2026-08-13 - **Status:** ✅ Complete — all phases built and tested - **Status Date:** 2026-08-16 - **Phases:** 9 - **Phases Complete:** 9 of 9 — 8 built, phase 2 void (no data was worth converting) - **Notes:** Follows `docs/2026-08-10-wayleave-module-realignment.md`, which left this open deliberately. Replaces the polygon-only approval queue with one model covering all four coverage sources, and in doing so produces the canonical coverage set the project-matching item in `docs/improvement-opportunities.md` has been waiting for. Every decision below is Dave's, taken at interview 2026-08-12/13. ## Plan Phases 0. Investigation — done, findings recorded below 1. ✅ Migration — `wayleave_coverage` and `wayleave_approvals` (`db/042`) 2. ~~Migration — backfill, then drop the two tables this replaces~~ — **void.** No data is worth converting; Dave deleted the dev polygons and started fresh (2026-08-13). The drops moved into `db/043`. 3. ✅ Migration — views, retained layer, map-layer registration (`db/043`) 4. ✅ Permission — `wayleaves_approve` 5. ✅ Backend — the diff query and the decision endpoints (`db/044`, `db/045` + endpoints) 6. ✅ Editor — the Approvals tab 7. ✅ Map — layers, styles, and the feature popup aligned to the project editor 8. ✅ Premises tab — old queues removed early, with the endpoints they called 9. ✅ Docs and backlog ## Problem There are four ways to put a premise on a wayleave — draw a polygon, add a UPRN directly, attach a stocklist, link a Land Registry title — and only one of them asks anyone to confirm it. That asymmetry was raised while testing the realignment (Dave, 2026-08-12) and could not be defended: drawing a polygon around a premise is as explicit an act as typing its UPRN, so "the user did not choose these" does not hold. Four further faults in how the existing queue works, all found during the same review: - **It is disconnected from the tools that feed it.** The queue is a permanent panel in the Premises tab; the polygons that fill it are drawn on the Map tab. Nothing on the panel says it is reviewing UPRNs a polygon caught. - **Two queues split by direction let you approve half a shape change.** "Pending add" and "pending removal" are separate lists with separate buttons, so a single polygon edit drops entries in both and you can accept the premises it gained while refusing the ones it lost. No geometry produces that result. - **Per-premise rejection exists in the data and the endpoint but nowhere in the UI.** `wayleave_coverage_polygon_uprns_approve.php` takes a UPRN array; the editor only ever sends all of them. So the granularity the schema carries has never been reachable. - **Only polygons are materialised.** Polygon UPRNs are real rows resolved by `ST_Within` on save; stocklists and titles are links resolved live by views on every read. Two sources have state, two cannot. ### What the current tables actually do `wayleave_polygon_uprns` encodes four states across two booleans: | `is_assigned` | `is_approved` | Means | |---|---|---| | true | false | pending add | | true | true | covered | | false | true | pending removal | | false | false | withdrawn | It therefore needs no `is_deleted` column — `(false, false)` already is that state. What it lacks is any record of **which** polygon produced a row: the resolution runs against `ST_Union` of every polygon on the wayleave, so "which polygon covers this premise?" is not answerable, and neither is "reject what Polygon #2 just did". `wayleave_uprns` is the direct-add store and carries no approval state at all. ## The model **Coverage is the union of approved claims.** One row per premise per source that has ever claimed it. A premise is covered if **any** of its rows is approved. **Pending is derived, not stored.** Three sets: ``` current = what the four sources resolve to right now (live) approved = rows in state 'approved' (stored) rejected = rows in state 'rejected' (stored) pending additions = current \ (approved ∪ rejected) pending removals = approved \ current ``` Approving a source's change sets its rows to `approved`. Rejecting sets them to `rejected`, which is what stops them reappearing in the queue on every load — the only reason rejection has to be stored at all. **Drift stops being a problem and becomes the feature.** A stocklist that gains premises, or a `basedata.abp` refresh that brings new-builds inside an existing polygon, changes `current` and therefore appears in the queue on its own. No refresh triggers, no staleness to chase. This is the single biggest reason to prefer this shape over a fully materialised coverage cache: the thing that would have needed five triggers to maintain is simply visible instead. **Overlap needs no priority order.** A premise inside a polygon and on an attached stocklist has two rows: | Polygon row | Stocklist row | Covered? | |---|---|---| | approved | pending | yes — by the polygon | | approved | rejected | yes — by the polygon | | approved | approved | yes, two provenances | | rejected | approved | yes — by the stocklist | | rejected | rejected | no | This is the reasoning already recorded on the coverage backlog item: with one row plus a priority order, every detach means recomputing which source now wins. Two rows makes "is it covered?" an `EXISTS` and "why?" the row set. **Rejecting a removal re-sources the row as `retained`.** When a polygon shrinks and a premise falls out, rejecting that removal means the premise stays covered while no source produces it. Left attributed to the polygon it would show as pending removal forever. As `retained` — recording what it was and when the removal was rejected — it stops diffing against the polygon, needs no extra flag, and reads honestly: *covered because someone chose to keep it*. This is the "a deed covers what it covered at signing" case the matching backlog item raises. ## Decisions taken (interview 2026-08-12/13) | Decision | Choice | |---|---| | Scope | All four sources go through approval | | Grain | **The change**, not the premise — one decision per source change | | Queue | Derived diff, grouped by source; one queue, not two | | Direction | An attribute of a change, not a category of queue | | Reject | Does not apply the UPRN change; **never** undoes the polygon, stocklist or title | | Rejections | Persist, with date and source, surfaced on premise click | | Rejected removal | Row re-sourced as `retained` | | Retained premises | Their own map layer, own style, hideable | | Reversal | Batch is the unit — reversing an approval undoes all of it | | Surface | Approvals gets **its own tab**, listing all approvals, not just outstanding ones | | Reference | Per-wayleave sequence, `WL-`, on a batch record | | Audit | Journal line per action. **No history trigger** on the coverage table | | Configurable | No admin toggle — approval is always on | | Permission | New `wayleaves_approve` capability | | Admin matrix | Its read/write oddity left as-is; a better interface is a later job | | Counts | Both gross and net shown, because of overlap | ### Why no history trigger Every other table in this module has one, so the reflex is to add another. Approving a 1,000-premise stocklist would write 1,000 history rows for one click, at a grain nobody reads. The journal line — *"WL42-7 approved — 1,000 premises added"* — is the record people actually want, and the batch record holds the detail. ### Why the permission is a pseudo-module `admin_save.php`'s `$allowed` list already contains five entries that are not modules: `admin_fields`, `admin_users`, `admin_companies`, `admin_map_layers`, `admin_dist_analysis`. Capabilities are already modelled this way, so `wayleaves_approve` follows the pattern rather than bending it. The alternative — a third permission *level* above `write` — means changing `permissionSatisfies()`, two hardcoded lines every endpoint depends on, and is semantically wrong: approve is not "more than write" for the four modules with nothing to approve. **The useful consequence:** the two permissions are independent, so `wayleaves` = read plus `wayleaves_approve` = write is a reviewer who can see wayleaves and approve coverage but cannot edit anything. That role comes free with this design. ## Build ### Phase 1 — Migration: `wayleave_coverage` and `wayleave_approvals` `wayleave.wayleave_coverage` — one row per premise per claiming source: | Column | Note | |---|---| | `id` | identity | | `wayleave_id` | FK to `wayleaves` | | `uprn` | | | `source` | `polygon` \| `direct` \| `stocklist` \| `title` \| `retained` | | `source_ref` | polygon id, stocklist id, title number; null for `direct` | | `state` | `pending` \| `approved` \| `rejected` | | `approval_id` | FK to the batch that last decided it; null while pending | | `proposed_user`, `proposed_datetime` | who caused it to appear | | `decided_user`, `decided_datetime` | who approved or rejected it | | `retained_from_source`, `retained_from_ref` | what it was before a removal was rejected | **One `state` column, not two booleans.** `is_assigned` × `is_approved` is exactly what made the current table unreadable — four states spread across two flags with no name on any of them. `source_ref` is varchar: it holds integers for polygons and stocklists and a title number for titles. Typed columns per source would mean four mostly-null columns and a check constraint to keep them exclusive. `wayleave.wayleave_approvals` — the batch: | Column | Note | |---|---| | `id` | identity | | `wayleave_id` | FK | | `reference` | `WL-`, n from a per-wayleave counter | | `action` | `approve` \| `reject` \| `reverse` | | `source`, `source_ref` | what was decided | | `uprn_added`, `uprn_removed` | counts at the time, so the journal line stays true | | `decided_user`, `decided_datetime` | | | `reversed_by` | FK to the approval that reversed this one, if any | UNIQUE on `(wayleave_id, reference)`. The counter is per wayleave, so `WL42-7` reads as "the seventh decision on wayleave 42" — sortable, collision-free, and meaningful in a journal line in a way a random hash would not be. ### Phase 2 — Migration: backfill and drop `wayleave_polygon_uprns` and `wayleave_uprns` are both subsumed. Backfill: - `wayleave_uprns` where `is_deleted = false` → coverage, `source = 'direct'`, `state = 'approved'` (they were covered, and nobody was ever asked) - `wayleave_polygon_uprns` by its state pairs: `(t,t)` → approved, `(t,f)` → pending, `(f,t)` → pending removal (leave approved; the diff will surface it), `(f,f)` → rejected **`source_ref` cannot be backfilled for polygons.** The old table never recorded which polygon produced a row — resolution ran against `ST_Union` of all of them. Backfilled rows therefore get a null `source_ref` and are re-attributed the first time each polygon is resolved per-polygon under Phase 5. State to check on dev before writing this: how many rows exist, and whether re-resolving on first load is acceptable or wants a one-off pass. Both tables are then dropped. Their data is not preserved beyond the backfill — same basis as the realignment, and to be confirmed against dev volumes first. ### Phase 3 — Migration: views and layers - `vw_wayleave_coverage` — approved rows. **The canonical coverage set**, and what the premise count, the list view and any future project matching should read. - The four `vw_uprns_from_*` views — reworked to read coverage rather than the source tables, carrying `state` so the map can colour by decision. - `vw_uprns_retained` — `source = 'retained'`. Registered as a fifth map layer with its own style, hideable. - `public.map_layers` — the new layer, and `vector_style_field` moved to `state` on the four existing ones so decision state drives colour. Register all of it the way migration 037 did, and **assert the row counts**: the relation is stored three times in `map_layers`, once inside the `url` string, and 025 updated two of three and reported success. ### Phase 4 — Permission `wayleaves_approve` in four places, all known: `$allowed` in `admin_save.php`, `MODULES` and the label map in `admin_users.js`, and a row in the permissions matrix markup in `html_body_admin_users.php`. Then one `getModulePermission()` check in the decision endpoint. No `app_modules` row — that table drives enablement, and a capability is not a module you turn off. **Note for whoever does this:** the module key lives in six places and two were missed during the realignment rename, surfacing only when a role was saved. Check all six. ### Phase 5 — Backend **The diff query** is the heart of it. Per source, resolve what it currently produces, compare against coverage, and return per source: net additions, net removals, and the **gross** figure alongside the net one. Both are needed because of overlap — a stocklist bringing 1,000 premises where 300 are already covered by an approved polygon changes coverage by 700, and a panel reporting 1,000 would not match the premise count afterwards. **Per-polygon resolution.** Coverage attribution needs `ST_Within` per polygon rather than against the union, so `source_ref` is real and "reject Polygon #2's change" means something. This is a change to `wayleave_coverage_polygon_save.php`, not just an addition. **Decision endpoints** — approve, reject, reverse. Each writes an approval batch, updates the affected coverage rows, and writes the journal line. Reject on a removal re-sources the rows to `retained`. All gated on `wayleaves_approve`. #### ✅ Built 2026-08-13 — and four things the plan had wrong Writing it against the 042/043 shape exposed four faults, three of which only show on `reverse`, the operation the whole feature exists for. All four are fixed; two needed migrations that were not in the plan. 1. **A direct UPRN was not its own change.** `source_ref` was `''` for direct claims, so every direct UPRN in a wayleave collapsed into one panel row and one decision — add one today and another tomorrow, and approving accepted both. Every other source groups many premises under one reference; a direct add has no such grouping, because the UPRN *is* the change. `source_ref` is now the UPRN, which also matches the sketch in Phase 6 and makes rejection reversible per premise. **This removed a wart:** the first draft let re-adding a rejected UPRN silently return it to pending, a second route back that no other source has. Dave's point stands — the way back is reversing the decision on the tab, where it leaves a reference and a journal line. Re-adding is now a no-op. 2. **Approving a removal DELETEd the coverage row** (`db/044`). A deleted row cannot be restored, so reversing such a batch brought back its additions and silently dropped its removals, reporting success. A fourth state, `removed`, is the fix: not covered, still present, still reversible, and doubling as the tombstone that stops the source re-proposing the premise — but only while the source is still not producing it. 3. **Reverse could not know what to restore** (`db/044`). It set everything to `pending`, right for an approved addition and wrong for an approved removal, which was `approved` before. `prior_state` records it per row, so reverse is one assignment. 4. **`pending_count` on `vw_wayleave_list` counted stored rows** (`db/045`), which only direct claims ever write. A polygon that grew or a stocklist that gained premises writes nothing, so the badge would have read 0 while a thousand premises waited. `vw_wayleave_pending` does the diff; the list view loses the count entirely, because computing it unfiltered is a spatial join across every polygon in the database. Also settled while building: - **One decision may cover several changes from the same source** (`source_refs`), so "approve all direct UPRNs" is one click and one reference. Cross-source batching is not offered — `wayleave_approvals` stores one source per row and could not describe or reverse a mixed batch honestly. - **The batch row is written before the work, not after**, so every coverage row it touches carries its `approval_id` explicitly. The original swept for rows that happened to be unstamped, which misses any row a previous decision already claimed. - **`retained` rows keep their origin in `source_ref`** as `polygon:12`, not `''` — two retentions of the same premise from different sources would otherwise collide on the unique key and throw. - **`wayleave_load.php` Q7 was reading both dropped tables**, so the editor would not load at all. Found while re-pointing the counts. - **The map style registry still keyed on `src`** while 043 had repointed every wayleave premise layer to `map_state`, so all five layers rendered **invisibly** — the precise trap the note in that block warns about, walked into by the migration that warns about it. Rekeyed to approved / pending / removing / rejected / retained, with pending and removing drawn as hollow rings so the difference survives a bad projector. - **Land Registry titles had no autocomplete and asked the user for tenure** (Dave, 2026-08-13). Tenure is a fact of the register — `landregistry.ccod.tenure` — so the select is gone and the value is looked up on add; `COALESCE` in the load repairs rows already saved blank without needing a migration. The panel now also shows the proprietor and how many premises the title resolves to, because a title that links cleanly and resolves to nothing looked identical to a failed add. - **The Direct UPRNs box demanded a UPRN** (Dave, 2026-08-14): "it is unreasonable to expect someone to know a UPRN when searching for a single address". It now searches Address Base by address, postcode or UPRN through a new `premiseaddress` case on the shared autocomplete path — not `stocklist_address_search.php`, which is stocklist-scoped and predates the aligned pattern. Digits and text take **separate queries** rather than one with an `OR`: a UPRN matches exactly on the btree, while `uprn::text LIKE` in an OR is unservable by any index on that table and would scan Address Base per keystroke. Pasting a list of UPRNs still works — the search is for the person holding an address, the paste for the person who arrived with a list. - **`autocomplete.php` had `display_errors = '1'`** — the only endpoint in `fn/` that did, and it returns JSON. Any PHP notice prepended HTML to the body; the caller `JSON.parse`s that inside an ajax success handler, where jQuery swallows the throw. The request returns 200 with the rows plainly visible in the network tab and the suggestion list stays empty (Dave, 2026-08-14). This affected **every** autocomplete type, not just the new one. - **The first premise query had an `ORDER BY`, and that was the slowness** (Dave, 2026-08-14). An `ORDER BY` forces the planner to resolve every match before it can know which 15 sort first, and "high street" matches hundreds of thousands of rows nationally; `LIMIT` alone lets it stop at 15. Postcode and address also split into separate branches rather than an `OR`. **Address Base needed no new index** — it already carries FTS, trigram, postcode `text_pattern_ops` and uprn indexes, all of which this now uses. Requests are also aborted between keystrokes: jQuery UI does not dedupe them, so a slower earlier query returning after a faster later one overwrote the list with stale results, which reads as the suggestions never appearing. - **`landregistry.ccod` had no index on `title_number` at all** (`db/046`), so every keystroke in that box was a sequential scan of the whole register — and the search only ever matched the number, while the label showed the proprietor, so a company name could be read in the list and then not found by typing it. Both fixed: three indexes, and the query now matches proprietor too. This is not wayleave-specific — `?do=landregistry` and every stored-title label resolution were paying the same cost. - `wayleave_coverage_stocklist_save.php` and `wayleave_landreg_save.php` needed **no change to their link handling**: they already only manage links. The plan's "simplify" for those two rested on an assumption that did not survive reading them. ### Phase 6 — Editor: the Approvals tab A new tab, out of Premises. Lists **all** approvals, not just outstanding ones, so approve, reject and reverse live in one place: ``` Movement Item Type Adding Removing Action Polygon #1 Polygon 10 (10 new) 0 Detail | Accept | Reject UPRN 100031234567 Direct 1 (1 new) 0 Detail | Accept | Reject Stocklist1 Stocklist 1000 (700 new) 0 Detail | Accept | Reject Polygon #2 Polygon 0 25 Detail | Accept | Reject Title HP123456 Title 0 20 Detail | Accept | Reject WL42-6 Stocklist 412 approved 0 Detail | Reverse ``` Detail opens a modal with the premise list — 1,000 premises is too many to accept on a count alone. Reversal confirms in net terms: *"this will uncover 700 premises; 300 remain covered by other sources"*, or the count will not move by what the user expects. #### ✅ Built 2026-08-14 **Two tables, not one.** The sketch above draws a single table with `Reverse` replacing Accept/Reject on decided rows, and that does not survive contact with the columns: an outstanding change has gross and net counts and no reference yet; a decision has a reference, an actor and a time and no net anything. Merging them gives a column meaning one thing on some rows and something else on the rest. Both live on the one tab, which was the actual requirement — say if the single table mattered for its own sake. **Gross shows its net only when they differ.** `10 (10 net)` on every row teaches people to skip the brackets, so the brackets appear exactly when they carry information. **Bulk decisions are same-source only,** enforced in the UI with the reason stated in the selection note. It is a schema constraint, not a UI shortcut: `wayleave_approvals` stores one source per row, so a mixed batch could be neither described in the log nor reversed as a unit. This is what `source_refs` was added for — twenty direct UPRNs are one click and one reference. **Reversed decisions are struck through, not hidden.** The log is the audit trail, so a decision that was undone still happened. **`wayleave_approval_detail.php`** serves both halves: `pending` resolves the source live, `approval` reads the coverage rows stamped with that `approval_id`. Every premise carries `covered_elsewhere`, which is the single flag explaining the gap between gross and net — shown per row so nobody has to work out which premises the subtraction referred to. Capped at 500 with the cap declared, and scrolled rather than paged: the modal exists to be scanned for surprises, and pagination hides the row being hunted for. ### Phase 7 — Map Four source layers coloured by `state` (approved / pending / rejected), plus the retained layer with its own style. Clicking a premise shows every claim on it and each claim's state and date — *"Polygon #1 (approved 12 Aug), Stocklist1 (rejected 13 Aug)"* — which is the only place "why is this covered when I rejected it?" has an answer. Point styles must key on a column the view exposes. A key that does not resolve falls through to `pointStyleDefault`, which has no `image` and renders points **invisibly** rather than wrongly — the trap migration 041 was written to avoid. ### Phase 8 — Premises tab ✅ Remove the pending add / pending remove queues and their buttons. The premises list stays and gains nothing: it already shows Added method / by / when, which now come from the coverage row rather than the source tables. **Done in Phase 5, not left to last.** The queue's two endpoints read tables `db/043` dropped, so they were already dead; leaving the buttons on screen to 404 would have made Phase 5 untestable. `wl-pending-pane`, `wlRefreshPending()`, `wlRenderPendingList()` and the approve/reject handler are gone. The Premises tab now shows confirmed premises and the four source panels only, until Phase 6 adds the tab that replaces the queue. ### Phase 9 — Docs and backlog ✅ `CLAUDE.md` gains a **Wayleave Coverage** section: the canonical table, that pending is derived rather than stored, the four states and the two flags that are not states, that decisions record their own membership, and that the source views must stay filterable — each written as the thing someone would otherwise get wrong. `docs/improvement-opportunities.md`: - **Coverage inputs need lookups** — ticked. Both halves built, plus the three `landregistry.ccod` indexes the title search turned out never to have had. - **Wayleave ↔ project matching** — the canonical coverage set it specifies is built, so it reduces to the matching half: read `vw_wayleave_coverage` rather than specifying a set of its own. Its naming is pre-realignment `agreement_*` throughout, flagged for whoever picks it up. - **Attach projects as a fifth source** — superseded by the 2026-08-15 analysis, which prefers a copied boundary to a live fifth source. Eight items were raised during the work and left open: the project link, deletion versus coverage links, rejections blocking stocklist and title sources, approver context on re-requests, decision context captured at the time, the five duplicated map popups, boundary cutouts, and `migrate.php` not surfacing `RAISE NOTICE`. ### ✅ Editor load time (Dave, 2026-08-14) The wayleave editor took 1–2 seconds on `wayleave_load.php`. `EXPLAIN (ANALYZE, BUFFERS)` settled it, and overturned one of the two things I had guessed. **Suspect 2 was wrong.** I claimed `vw_wayleave_list` aggregates the whole coverage table because quals cannot be pushed to the nullable side of a `LEFT JOIN`. The plan shows the opposite — `Index Cond: ((wayleave_id = 1) AND (state = 'approved'))` inside the grouped subquery, whole query **0.335 ms**. The predicate propagates. Nothing to fix. **The dominant cost was JIT, not the query.** Of 675 ms, **598 ms was JIT** compiling 116 functions; the actual work was ~77 ms. JIT fired because the estimated cost was 3.2 million — a nested loop estimated at 2,277,468 rows returned 9. **That estimate was absurd because suspect 1 is real.** Confirmed in the plan: ``` Seq Scan on wayleave_polygons p (no wayleave_id condition — all rows) Seq Scan on stocklist_premises sp (rows=563525) Filter: (COALESCE(p.wayleave_id, wayleave_coverage.wayleave_id) = 1) ``` A qual on a `COALESCE` over a `FULL JOIN` cannot be pushed into either side, so both halves were computed for every wayleave in the database and filtered afterwards. So one fix addresses three symptoms: land the predicate, the estimate collapses, JIT stops firing, and the work shrinks to the wayleave asked for. **And a missing index found in the same plan.** `stocklists.stocklist_premises` has indexes on `(uprn, stocklist_id)` and `(uprn)` — both leading on uprn. "Which premises are in this stocklist" had no index at all, which is the direction every stocklist screen reads in. Not wayleave-specific. - `db/047` — `stocklist_premises (stocklist_id)`. - `db/048` — the three joined source views rebuilt as `UNION ALL` of *what the source produces now* and *decided rows it no longer produces*, so `wayleave_id` comes from a base table in each branch and quals push into set-operation branches. `vw_uprns_from_direct` is untouched: it reads coverage directly and always filtered. Also fixed: Q7 evaluated `vw_wayleave_pending` **twice**, once per column, each running the whole diff. Now one CTE. ### ✅ Geometry columns were constrained where projects leaves them open (Dave, 2026-08-14) Drawing a polygon failed with *Geometry type (MultiPolygon) does not match column type (Polygon)*. `map_get_v2.php` serves every layer through `ST_MakeValid`, which resolves a self-intersecting outline into a **MultiPolygon** — so a boundary drawn with one stray crossing stores fine, comes back from the map as a MultiPolygon, and can then never be saved or deleted again. `projects.projects.geom` is a bare `public.geometry`: no type, no SRID. 033 and 032 pinned both on wayleave. Dave's point on the second half is the one with the longer tail — **27700 is British National Grid, and a projection is a deployment concern, not something to bake into a DDL every client shares**. `db/049` drops both typmods from `wayleave_polygons.geom`, `wayleaves.geom` and `wayleaves_history.geom`; the transform stays in the endpoint where it can differ per deployment. Kept from the first draft of 049, because it is not about types: geometry is now made valid **on write** rather than on every read. The coverage views resolve premises with `ST_Within` against these polygons, and `ST_Within` on an invalid geometry gives unreliable answers rather than an error. `ST_CollectionExtract(…, 3)` handles `ST_MakeValid` returning a `GeometryCollection` when a shape degenerates to a polygon plus a dangling edge. Cutouts were never affected either way — a donut is one OGC Polygon with an interior ring, so it fitted the old type as well as the new. What prevents them is the drawing tool; backlogged. **Repair-on-write was reverted the same day (`db/050`, comment only).** 049 also made geometry valid on write, and Dave caught what that costs: `ST_MakeValid` resolves a bow-tie by **splitting it into two polygons meeting at the crossover**, so the crossing becomes a real vertex in both parts and the shape stops being one ring. The user gets back a boundary they did not draw and cannot drag back — there is no self-intersection left to undo. His question was the right one: *what is the issue with a bow-tie that can be undone?* Less than I had implied. `ST_Within` has no well-defined inside, so the premise count for that **one** polygon is an interpretation rather than an answer — but it does not error, approval gates the result, the map already shows the untangled shape via `map_get_v2.php`, and it is self-correcting whenever anyone notices. A mangled geometry is permanent; a recoverable one is temporary and visible. Repairing traded the second for the first. Rejecting invalid geometry outright was also considered and rejected by Dave — it keeps the shape safe but makes the tool argue with the person using it. One-off consequence: 049's `USING` clause repaired whatever was stored at that moment, so a boundary that was self-intersecting when it ran is now two polygons and must be redrawn. **`ST_Union` does not tolerate what `ST_Within` tolerates.** Saving a fresh bow-tie then failed on `lwgeom_unaryunion_prec: TopologyException: side location conflict` — the recompute of `wayleaves.geom`, not the coverage resolution. My "it does not error" was too broad: the predicate copes, the unary union does not, and the failure aborts the save and leaves the boundary undeletable. Repair now happens **there and only there**: `ST_Union(ST_CollectionExtract(ST_MakeValid(geom), 3))`. `wayleaves.geom` is derived and nobody edits it, so untangling it costs nothing; `wayleave_polygons.geom` stays exactly as drawn, which is what keeps the bow-tie fixable by dragging a vertex. `CollectionExtract` keeps the result polygonal so `st_centroid` in `vw_parent_wayleave_links` is never handed a `GeometryCollection`. ### ✅ No spatial function on editor load (Dave, 2026-08-14) > *"I also thought the purpose of the uprn table was so a spatial function wasn't needed on > wayleave edit load."* Half right, and the half that was wrong was mine. What a wayleave **covers** is a materialised table read — `wayleave_coverage` — and the premise count, the list view and the approved map layer all read it with no geometry involved. The old `wayleave_polygon_uprns` table is not missed for that. What needs `ST_Within` is only the outstanding-**change** question, comparing what a polygon contains right now against what is approved, and that table could never have answered it: it stored decisions, not what the source currently produces. But it was running on **editor load**, which it did not have to be. Q7 carried two spatial operations purely to fill badges: - `pending_add_count` / `pending_remove_count` — the four-source diff. Moved to the Approvals tab, which runs the same query for its rows and now fills the badge from it. - `project_overlap_count` — `ST_Union` of the boundaries then `ST_Intersects` against every project. Moved to the Projects tab, which lists exactly those projects and counts its own rows. Editor load is now table reads only. **Deferring them to the tabs was then wrong in the other direction** (Dave, 2026-08-14): *"the point of the badges is to see these counts at a glance, not after someone's gone looking for them"*. Quite right — a count you only see after opening the tab is a second copy of what the tab already says. Both constraints hold at once by computing them on page load but not **in** it: `wayleave_badge_counts.php` is fired after the record has rendered and is never awaited. Editor load stays pure table reads, the badges fill a moment later, and a failure there costs two badges rather than the page. The tabs still refresh their own badges as they load, and `wlRefreshCoverage()` picks the request up again after any change. ### ✅ Union is intolerant where the predicate is not Saving, and then loading, failed on `lwgeom_unaryunion_prec: TopologyException: side location conflict`. Three call sites, all `ST_Union` over the boundary polygons — `wayleave_coverage_polygon_save.php`, `wayleave_load.php` Q7 and `wayleave_projects_load.php` — none of them `ST_Within`, which tolerates invalid input and still does. Each now unions `ST_CollectionExtract(ST_MakeValid(geom), 3)`: repair belongs on derived geometry that nobody edits, never on the stored shape. ### ✅ The editable layer serves geometry as stored (`db/051`) The saved bow-tie kept gaining a vertex at the crossover, and it was not the save — it was `map_get_v2.php`, which reads every layer through `ST_MakeValid`. Right for display, wrong for editing: the editor was handed the repaired rendition, so dragging a vertex and saving overwrote the original ring, and a crossing could never be untied. `raw_geom=1` now skips the repair, and `st_forcerhr` with it — ring orientation is presentation too, and an editor should round-trip what it was given. Set on `vw_wayleave_polygons` only; every other layer in every module is display-only and keeps the repair, which is why this is a per-layer flag rather than a change to the shared default. ### ✅ Phase 6 feedback (Dave, 2026-08-14) - **`rejectd` in the journal.** The log line built past tense as `$action . 'd'`, which is right for approve and reverse and wrong for reject. Two of three cases being regular is exactly why it read fine to me. Now a map. - **"Accept" on the row buttons, "Approve" everywhere else** — including the permission, `wayleaves_approve`. One button using a different verb reads as a different action. Approve throughout. - **Decision log labels** are past tense and capitalised: *Approved*, *Rejected*, *Reversed*, rather than the raw `action` value. The log records what happened. - **A direct UPRN could only be removed before approval** (`db/052`), unlike a stocklist or title, which can be detached at any point. My reasoning for the restriction was that withdrawing approved coverage is a decision and belongs on the tab — but detaching a stocklist is the same act and has always been allowed, because it *proposes* rather than decides. **The side panel proposes; the tab decides.** Direct claims were the odd one out only because they have no link table to carry the proposal. `wayleave_coverage.withdrawn` is that link table's `is_deleted`. Everything downstream works unchanged because it all keys on `in_source`, which stops being a constant `true`: approve turns the row `removed`, reject turns it `retained`, and reverse leaves `withdrawn` set so the proposed removal reappears as outstanding. A **pending** claim is still deleted outright — nothing has been decided, so there is no decision to reverse and nothing for an approver to look at. `keep` cancels a withdrawal that has not been approved, the equivalent of re-attaching a stocklist detached by mistake. ### ✅ `retained` was a source value; it should have been a flag (`db/053`) Reversing a multi-UPRN direct removal failed: ``` duplicate key value violates unique constraint "wayleave_coverage_uq" DETAIL: Key (wayleave_id, uprn, source, source_ref)=(1, 6, direct, 6) already exists. ``` 042 modelled "kept after a rejected removal" by **rewriting the row** — `source` became `'retained'`, `source_ref` became `'polygon:12'`, originals parked in `retained_from_*`. That conflates two different facts, *which source claims this premise* and *is that source still producing it*, and the rewrite **vacates the row's natural key**. Reject the removal of direct UPRN 6 and `(1,6,direct,'6')` is free; add UPRN 6 again and a new row takes it; reverse the rejection and the restore collides. The reverse `UPDATE` is the one statement in the decision endpoint that writes a natural key, and it can carry no conflict handling — there is no sensible merge of two rows claiming one premise from one source. So the key must never move, and now it doesn't: `retained` is a boolean, reverse is `retained = false`, and the failure is gone by construction rather than guarded against. `retained_from_*` are dropped; the row holds its own source and source_ref for its whole life, so provenance never left. A side benefit on the Premises tab: a retained premise now shows as *"polygon (retained)"* rather than as a source called `retained`, which had lost the provenance the user was asking for. Approving a live claim clears the flag — if a polygon is redrawn to include the premise again it is being produced, not kept despite nothing producing it. Without that it would be covered *and* on the retained layer, which is two answers to one question. ### ✅ Shared confirmation modal, and every decision confirms Reversal used `window.confirm`. `glConfirm()` now sits next to `glToast()` in `main.js`, builds its own markup on demand so no template carries boilerplate, and takes a callback because the surrounding code is ES5-shaped jQuery. Approve and reject confirm too, not just reverse — and the wording carries the **net** figures, the number the premise count will actually move by. Reject states plainly that the polygon, stocklist or title is not altered, since that is the thing people assume rejection undoes. ### ✅ A decision now records its own premises (`db/054`) Only the newest decision in a chain could show a premise list (Dave, 2026-08-15). Approve, reverse, reject the same polygon and `WL1-1` and `WL1-2` open empty; only `WL1-3` has anything. `wayleave_coverage.approval_id` answers *which decision currently owns this premise*, not *which decisions have touched it*. Every new decision overwrites it, and reverse sets it back to `NULL` — so a reversal's detail was always empty and always would be. A decision is immutable, so its membership belongs on the decision. **Arrays, not a join table.** One row per premise per decision is the obvious shape and the wrong one: approving a 1,000-premise stocklist would write 1,000 rows for one click, and this is never queried relationally — it is read back whole, for one decision, to fill a modal. Two `bigint[]` columns are one row and `unnest` gives the modal its list. A GIN index would answer "which decisions touched UPRN X" later without changing the shape. Backfill recovers only decisions that still own their rows, which is exactly the limitation being removed. Anything already superseded stays empty — honest, since it was never recorded. **Two faults found on the way:** - **`uprn_added` / `uprn_removed` meant different things per action.** A rejection reported *"5 added, 3 removed"* for five refusals and three retentions — the opposite of the truth on both halves. They now mean **left covered** and **left not covered** for every action, and are counted from the same arrays so counts and lists cannot drift. - **Reversing a superseded decision was a silent no-op** that still wrote a reversal into the log claiming it had done something. The `UPDATE` matched nothing because a later decision had taken the rows. Now refused with the reason, and the log shows *superseded* in place of the button — shown rather than omitted, because "why can I reverse that one and not this one" is otherwise unanswerable from the screen. ### ✅ The retained layer showed the wrong set (`db/055`) > *"It was approved and in, but we rejected the removal, therefore it is still in."* > — Dave, 2026-08-15 I built `retained` as a third category: covered, but pulled onto a layer of its own on the reasoning that no source produces it so no source layer fits. That is wrong twice. **A refused removal leaves the premise simply IN.** It is approved, it belongs with the approved premises, and it renders on the layer of the source that brought it in. The flag's only job is to stop the diff asking again — without it the polygon no longer contains the premise, so `removing` is recomputed on every load and a decided change reappears in the queue forever. What changes is only what the row then *looks like*: `approved`, not a state of its own. **And the layer should hold what is OUT.** The set worth its own layer is what this wayleave no longer concerns itself with — refused additions and coverage taken off — so someone can see what it used to cover. Those are the `rejected` and `removed` rows, which until now were invisible tombstones serving only the diff and the reversal, leaving the one question a legal document invites with no answer on screen. So the two halves swap. `vw_uprns_retained` → `vw_uprns_excluded`, layer retitled *"UPRNs - removed or rejected"*, and the `retained` map style is gone because nothing is styled that way any more. The **column** keeps its name: `retained` reads correctly for retained *in* the wayleave, and it is internal. It was the layer that misused the word. A rejected addition still appears on its source layer too, styled `rejected` — it is inside the polygon, and *"why is this in my boundary and not covered"* deserves an answer where the user is looking. It is on the excluded layer as well; the two answer different questions and either can be switched off. ### ✅ A kept premise could never be removed again Removing an approved direct UPRN a second time did nothing at all — no feedback, no error, no approval record (Dave, 2026-08-15). After a rejection the row reads `approved / withdrawn=true / retained=true`, and the remove statement matched `withdrawn = false`. It found no row, wrote nothing, and the handler only toasted when something had happened, so the button looked dead. The flags resolve it once their roles are stated: **`withdrawn` is the request, `retained` is the answer.** Asking again clears the previous answer — `retained` is what tells the diff to stop raising the change, so leaving it set would swallow the new request exactly as it swallowed this one. The statement now matches `withdrawn = false OR retained = true` and clears `retained`. Two things surfaced alongside: - **A no-op removal was silent.** Any request matching no row returned success with a count of zero and said nothing. It now says so. - **Nothing on the sidebar said a premise had been kept.** It is covered like any other and carries no special state, but the sidebar is where someone goes to remove it again. It now shows a `kept` badge, and the Premises tab reads "direct (kept)". **Source-driven removals behave differently on purpose.** A polygon that loses a premise, has the removal rejected, regains it and loses it again does *not* re-raise: there is no explicit act of asking again, and the earlier answer stands. That is the deed-at-signing reading. A direct UPRN is different because clicking remove a second time is unambiguously a new request. ### ✅ A removed premise could not be added back Approving the removal of a direct UPRN left it unaddable: *"Already claimed or already decided — nothing added"* (Dave, 2026-08-15). The no-op rule came from Dave's own earlier ruling, and it was right for the case it was written for — re-adding a **rejected** UPRN re-asks a question that has been answered, so the decision is undone by reversing it rather than by a second, unlogged route no other source has. But it was applied to every decided state, and `removed` is a different act: the premise **was** covered and was taken off, so adding it again is a new claim, not a re-litigation of a refusal. `ON CONFLICT DO NOTHING` becomes `DO UPDATE … WHERE state = 'removed'`. The row is reset rather than replaced, because the natural key is taken and a second row for one premise from one source is exactly what `wayleave_coverage_uq` exists to stop. `approval_id` is cleared with the rest, so reversing the old removal finds nothing and correctly reports itself superseded instead of dragging the premise back out from under a live claim. **And then the rule went entirely** (Dave, 2026-08-15). Blocking a re-add of a *rejected* premise assumed the person adding it could reverse the rejection instead. They often cannot, and should not: > **Requesting is not reversing, and they belong to different people.** Reversing undoes > the approver's decision; re-requesting makes a new proposal which the approver decides > afresh. A user holding `wayleaves` write without `wayleaves_approve` — a different team, > which is the normal shape — can legitimately do the second and cannot do the first at > all. The message sent them to a tab they have no access to, to perform an act that was > not theirs to perform. And a refusal answers the request as it stood; it does not settle the premise for all time. The original objection was to a *silent* second route, and that is met by recording it, not by forbidding it — the journal separates new claims from re-requests, and the approver sees it on the tab as an addition like any other. The only remaining no-op is a premise already claimed and undecided, which is genuinely nothing to do. Backlogged from this: the approver sees a re-request as an ordinary addition, with nothing saying it has been refused once already — which is the context a second decision most wants. ### ✅ A rejected premise could never re-enter a polygon Shrinking a polygon past a refused premise and growing it back never proposed it again (Dave, 2026-08-15). Branch 1 of the boundary view joins any non-`removed` coverage row, so a `rejected` premise resolved as rejected forever. This is the polygon twin of the re-add fault, and I had called the asymmetry deliberate — wrongly. The two-teams argument applies identically: redrawing a boundary is a **new proposal** and belongs to whoever draws it, while reversing undoes the approver's decision and needs a permission the person redrawing may not hold. The rule, stated properly: **a rejection answers a proposal, and only suppresses it while the source is still making that proposal.** Once the polygon shrank past those premises the tombstones were suppressing nothing, and keeping them was exactly what blocked the re-proposal. `wayleave_coverage_polygon_save.php` now discards a `rejected` row whose premise has left the polygon. A premise still inside keeps its rejection — that proposal has not changed and has been answered. The decision is not lost: `wayleave_approvals` holds the reference, actor, time and premise list. This clears current state, not history; reversing that approval afterwards finds no rows and reports itself superseded. `removed` rows were never affected — the join already excludes them, which is why the approve-then-regrow path worked. Stocklists and titles have the same fault with no equivalent hook, and are backlogged. ### ✅ A deleted source was not named as deleted Deleting a polygon puts its premises up as a removal offering both Approve and Reject, and Dave questioned whether Reject is logical when the polygon no longer exists (2026-08-15). **It is, and it is the case `retained` exists for.** Nothing produces those premises any more, but the wayleave may still legally cover them — someone tidies away a boundary that was only a drafting aid, or deletes the wrong one. Rejecting keeps them covered permanently: a deed covers what it covered at signing. Removing the option would make deleting a polygon silently destructive, with no way to keep what it had brought in. **But the row named the polygon as though it still existed**, so the decision was being made blind. Labels now carry the state of the source — *(deleted)* for a polygon, *(detached)* for a stocklist, *(unlinked)* for a title — and the reject confirmation says plainly that nothing produces these premises any more and that rejecting keeps them anyway. ### ✅ The map popup was a private, worse copy (Phase 7) The plan described a premise detail popup as new work. It was not new — the popup existed and had diverged (Dave, 2026-08-16). Three faults against the project editor's version: - **It stopped at the first feature.** `forEachFeatureAtPixel` hands back whichever it finds first, so a premise inside a boundary showed one of the two arbitrarily. The project editor collects them all, which is how you check what is at a point and how you choose which of several nearby features to edit. - **It printed raw column names** — `in_source`, `decided_user`, `map_state` — with no formatting and no exclusions, where the other modules hide internal columns. - **Clicking a boundary silently started an edit** rather than describing anything, so a click to see what was there began moving the geometry. Fixed by aligning to the project editor's behaviour — but **not** by copying it, because that block already exists five times over (`project_edit_v2`, `map_v5`, `opportunity_edit`, `stocklist_edit`, `account_edit_v2`), with six copies of `formatString`, all slightly different. `MapLayerUtils.renderFeaturePopup()` is the shared version, matching the precedent this file already sets for styles. Collapsing the other five is backlogged; nothing should add a seventh. **The first attempt extracted the function and then rewrote the presentation** — Edit link on the heading line rather than beside the layer name, Bootstrap spacing utilities instead of the project's inline styles, and the spacing applied to every item but the first. It collided with the close button, and it was a fresh divergence created by the very change meant to remove one (Dave, 2026-08-16). **The shared function now owns the markup**, down to the inline styles and the `edit-feature-link` class, so a caller supplies only its excluded columns and its editable test and cannot drift. Layout is deliberately not a caller's concern. Editable features now offer an **Edit** link in the popup instead of hijacking the click, which is both the project pattern and what stops a stray click on a boundary becoming a geometry change. ## Testing checklist Tested in order. Report back by number. 1. `php db/migrate.php` dry run lists the new migrations and nothing unexpected. 2. `php db/migrate.php --apply` applies them with no error. 3. ~~Every previously covered premise is still covered~~ — void, Phase 2 dropped. 4. ~~Directly added UPRNs backfilled as approved~~ — void, Phase 2 dropped. 5. ~~Polygon UPRNs backfilled from their old boolean pairs~~ — void, Phase 2 dropped. 6. `wayleave_polygon_uprns` and `wayleave_uprns` are gone. 7. A role can be given `wayleaves_approve` in Users & Roles, and it saves. 8. A user with `wayleaves` write but no approve permission sees the Approvals tab read-only. 9. A user with `wayleaves` **read** plus `wayleaves_approve` can approve but cannot edit. 10. Drawing a polygon creates one pending entry showing its net additions. 11. Editing that polygon before approving updates the same entry — it does not create a second. 12. Adding a direct UPRN creates its own pending entry. 13. Attaching a stocklist creates one entry with gross and net counts that differ where premises overlap an approved polygon. 14. Linking a title creates its own entry. 15. A polygon edit that both adds and removes shows as one row with both figures, and one decision. 16. Approving an entry covers its premises and writes a journal line with the `WL-` reference. 17. The premise count moves by the **net** figure, not the gross one. 18. Rejecting an entry leaves the polygon, stocklist or title untouched. 19. A rejected entry does not reappear in the queue on reload. 20. Rejecting a removal keeps the premises covered and moves them to the retained layer. 21. Retained premises render in their own style and the layer can be hidden. 22. Clicking a premise shows every claim on it with each state and date. 23. A premise covered by two sources stays covered when one is rejected. 24. Reversing an approval undoes it, and the confirmation states the net effect. 25. Reversal writes its own journal line referencing the approval it reversed. 26. A reversed approval shows as reversed in the tab, not as pending. 27. Adding premises to an attached stocklist makes them appear as a pending change with no manual refresh. 28. The Premises tab no longer carries approval queues. 29. Map layers colour by decision state. 30. The premises list still shows Added method, by and when, now from the coverage row. 31. Global search, the list page and the dashboard premise counts all agree with the editor. ## Risks - ~~**Backfill is the one-way step.**~~ Retired: Phase 2 was dropped, nothing converted. - ~~**Per-polygon resolution changes an endpoint that currently works.**~~ Retired, and it landed the opposite way round to the risk: `wayleave_coverage_polygon_save.php` got *smaller*. The spatial join, the read of current state and the four-way state machine all went, because `vw_uprns_from_boundary` resolves per polygon at read time. 142 lines to 108, and none of the five transitions it used to have to get right at write time. - **Gross versus net will be got wrong somewhere.** Any count that says gross where it means net makes the premise total look broken. Every count in the tab, the confirmation dialogs and the journal lines needs deciding explicitly. - **A style key that does not resolve renders points invisibly**, not wrongly — the failure looks like missing data. Registering layers against `state` means the views must expose it. - **The module key lives in six places.** Two were missed during the realignment and surfaced only on save. `wayleaves_approve` adds a seventh registration point.