# Changelog All notable changes to Massing. Releases are signed, auto-updating desktop builds (Windows / macOS / Linux); the updater always serves the latest. Format loosely follows [Keep a Changelog](https://keepachangelog.com/). ## Unreleased ### ROUTE-SHADOW — four routes were registered on a URL another route already owned Starlette matches the **first** route whose path and method fit, so a second registration on the same pair is not an override — it is dead code that looks live. FastAPI then builds the OpenAPI document into a dict keyed by path and method, so the **last** registration is the one that gets described. The server ran one handler and `/docs`, the OpenAPI JSON and `apps/web/src/api/schema.d.ts` all described the other. | URL | served | never ran | |---|---|---| | `GET …/drawings/plan.svg` | `routers/drawings.py::plan` | `routers/authoring_docs.py::plan_svg` | | `GET …/drawings/sheet.svg` | `routers/drawings.py::sheet_svg` | `routers/authoring_docs.py::sheet_svg` | | `GET …/drawings/sheet.pdf` | `routers/drawings.py::sheet_pdf` | `routers/authoring_docs.py::sheet_pdf` | | `GET …/mep` | `routers/analysis.py::mep` | `routers/authoring_analysis.py::mep_summary` | **Fixed — the MEP systems browser has never worked.** `apps/web/src/api/mep.ts` declared two shapes for `/projects/{pid}/mep`: `mepSummary()` with `systems` as a **list** of per-system counts, and `mep()` with `systems` as a **Record**. The inventory handler served, so `apps/web/src/viewer/tools/mepSection.ts` read `s.systems.length` off a Record, got `undefined`, and printed *"No distribution systems yet — add duct/pipe runs + fittings."* on every model that has ever had one — then returned, hiding the by-discipline rollup, the fire-protection check and the sizing checks below it. `/projects/{pid}/mep` now resolves to `mep_summary`, which is what the panel, the client method and the published contract all already expected. **Also fixed: the sheet parameters `apps/web/src/viewer/sheetSpecs.ts` was written to work around.** That module records, from a measurement on 2026-08-09, that the rail sent `number`, `title` and `scale` and the route dropped all three. It fixed the caller and diagnosed the cause as a caller speaking a dialect the callee does not. The dialect was real and published: `number`, `title` and `scale` are exactly the shadowed handler's parameters, which is what `/docs` describes for that URL. **The decisions ran in opposite directions, and consumers is what decides.** The three drawing handlers in `authoring_docs.py` went, because `drawings.py` serves a strict superset and the whole client is built on it. The **serving** `/mep` handler went, because `energy.mep_inventory` had one caller in the tree — that route — and its client method `api.mep()` had none at all. The engines stay in `aec_data`, still tested, so publishing the ARCH-D titleblock sheet under its own URL remains available as a product decision rather than a gap. **Gated: `services/api/test_route_shadow.py`.** Derives every `(path, method)` the app holds by walking FastAPI's deferred router placeholders; fails closed on any route object it cannot classify; floors the walk's size and requires the four routers that carried the collisions to be present, so a broken walk cannot report a clean tree in the words of a clean tree. It replays the four shipped registrations (built here, not fetched from git, which CI's shallow checkout cannot do) and must find **all four and nothing else**, and a walk narrowed to top-level routes must **miss** all four. **The gate asks Starlette, not `==`.** The four shipped instances were identical path strings, so an equality test would have found all four — but equality is a *precondition*, not the rule. Starlette matches by regex, so `/projects/{pid}/drawings/{name}` registered before `/projects/{pid}/drawings/sheet.svg` swallows it just as completely. Measured before widening: the live tree holds **0** of that non-identical form, so using Starlette's own matcher costs no exemptions and no noise — it removes a precondition rather than chasing a finding. What it buys is visible under mutation: a single misplaced `/projects/{pid}/mep/{probe}` silently kills **eleven** MEP endpoints, and the widened rule names every one while the equality version reports that tree clean. Its soundness rests on no route declaring a `:path` converter, which is now asserted rather than assumed. **FastAPI's own warning saw two of the four.** It keys on a duplicate *operationId*, built from the function name plus the path, so it fired for `sheet_svg` and `sheet_pdf` — two handlers spelled the same — and stayed silent for `plan`/`plan_svg` and `mep`/`mep_summary`. *A warning keyed on the name cannot see a collision that is about the path*, and half a population reported confidently reads exactly like all of it. **Review found two ways the gate could still report clean, and both were preconditions I had not written down.** It probed `{param}` with a plain segment, so an `:int`, `:float` or `:uuid` route rejected its own probe and a duplicate on one came back clean — the first draft asserted only that no `:path` converter existed, which is *asserting the one case you thought of, not the property*. And it read a `prefix` attribute off FastAPI's deferred-inclusion placeholder; **this FastAPI has no such attribute**, so it silently walked the un-prefixed router and two handlers colliding at a prefixed URL passed. Every replay fixture used an empty prefix, so **no test in the file could have caught it**. Both are now fixed at the source rather than patched: the walk takes FastAPI's own effective route contexts, whose `path_regex` already carries the prefix, and probe validity is *self-checking* — every probe must match the route it was built from, so an unhandled converter reds the build instead of quietly shrinking the population. Two fixtures added (`:int` collision, prefixed collision), each mutation-verified to red the right check. **Why nothing went red.** Every MEP assertion in `services/api/test_mep_systems.py` calls the engine `mep.mep_summary(m)` directly; not one goes through the route. *A test of the engine is not a test of the door.* **And the gate that would have named it misdiagnosed instead.** `apps/web/src/api/clientCallers.test.ts` already held `mep` in `UNCALLED` — it had measured the method as callerless and frozen that. Removing the method made its check fail with *"1 method(s) in UNCALLED now HAVE a caller: mep"*. An entry leaves that set either because it gained a caller or because it stopped existing, and the message asserted the first for both. The named repair happened to be right and the stated reason would have sent a reader looking for a screen that does not exist; the two are now separate assertions, each mutation-checked. Everything below ships in the next release. **These were 34 separate `## Unreleased` headings** until 2026-09-07 — one per change, each a peer of a real version — so the word had stopped meaning anything as a heading and the file read as 34 pending releases rather than one. The titles are unchanged and now sit at `###` beneath this, in the same order; no text was edited, added or dropped in the fold. ### A wire type declared twice is a field no audit can see `ClashResult.truncated` was returned by the server and read by nobody for months, and the reason was never that anyone decided to ignore it: the clash panel had re-spelled the response shape, so no property access resolved to the declared field and no type error, lint or unread-field audit could reach it. Two more shapes were written out twice the same way — `Vital` and `LogisticsResource`, both character-for-character copies of `api/types.ts`, one of them under a comment reading "as the API returns it". Both now import the type they were copying. A new check refuses the next **exact** copy: same type name and same member set. That is narrower than the case above — a local shape that drops a field, as the clash panel's inline cast did, has a different member set and is not caught. The narrowness is deliberate: a subset rule flags twenty shapes here, eighteen of them deliberate narrowings, and gating it would need an exemption list. ### The Discipline Spine says how many spec sections it is NOT counting The chain-coverage bars are computed over the *enforced* spec population — sections a withdrawal has taken out of the chain are excluded, and correctly so: a withdrawn section is not an unpackaged one. The card printed the enforced number and nothing else, so on a job with a void section its "18 specs" silently disagreed with the 21 in the spec register, with nothing on the page accounting for the difference. It now reads "21 spec sections on the job · **3 withdrawn (excluded)**, so every bar above is taken over 18", with the sections themselves in a disclosure below it. The server has always returned both numbers for exactly this reason, and the submittal-log card has always rendered the same caveat. Two of the four bars, not all four: `sheets_specced_pct` divides by the drawing count and `packages_costed_pct` by the package count, neither of which a withdrawn spec changes. The caveat names the two it applies to. ### Sources & Uses shows the loan fees The PDF's Sources & Uses itemises six rows; the on-screen table showed five. The missing one is the origination points — real money, and invisible because its absence balances: the fee is folded into Total uses and funded out of equity, so uses still equalled debt plus equity and every visible number reconciled. On two points against a $13m loan that is $260k of the equity cheque with no line of its own. ### The coordination matrix now counts the whole clash run, not the first page of it A federated clash run returns two numbers: `count`, every interference it found, and `clashes`, the first 200 of them. The Clash panel printed the first beside the list and used the second to build the discipline-pair matrix — the screen a coordination meeting reads to decide which trades still have to talk to each other. That matrix is the one place this product makes a claim about what was *checked*, and it has a third state for exactly this: a pair nobody ran is `untested`, never `clean`. Building it from the page defeated that from outside. Every returned clash on a large model tends to belong to one or two busy pairs, so a pair whose interferences all sat past the 200th arrived with no evidence at all — and with every pair declared tested, it was reported **clean**. Measured on a 1,438-clash fixture with 918 of them between architecture and MEP, none on the first page: the matrix said 100% pair coverage, nothing untested, and called that pair clean. Both federated paths now tally clashes per discipline pair over the *whole* run, where the full list still exists, and the tally travels beside the page: the list is what a person clicks, the tally is what a report may claim. Against an older server that truncates and sends no tally, the panel declares nothing tested rather than guessing — those pairs read `untested`, which is what the third state is for. Two smaller things on the same screen. The list header counted the page while the summary above it counted the run, so two different numbers sat under the word "clashes" with nothing saying a cap was in play; it now reads "200 of 1438 clashes — click to inspect (list capped)". And a single-model run that creates issues says when the cap is why fewer issues exist than clashes. `truncated` was a field the server had always returned and nothing had ever read, which is how this survived: the panel re-spelled the response shape inline rather than using the declared one, so no type, lint or audit could see the omission. The shapes are now declared once in `apps/web/src/api/clash.ts` and the audit pins them as read. Review then found the same premise one level further down. The panel declared *every* pair of discipline models tested, including each model against itself — and a federated run excludes same-model overlaps by construction, because a beam meeting its own column is a joint, not a coordination finding. So the diagonal was reported clean by a run that could not have looked at it. It is now filtered on the model key, which keeps two distinct models that happen to share a discipline label correctly counted as tested. ### Clicking a register pin on the model now opens its record A pin on the 3D model is a register record's marker — an RFI, a punch item, an observation. Clicking one selected the element it sits on and wrote a line to the status bar, and that was all: the record the pin stood for had no way in. It now selects, reports, and opens the record. The behaviour was already written down as shipped. `apps/web/src/pins/pins.ts` branches topic-versus-record on click and explains why — *"a topic restores a saved viewpoint, a record opens its register row"*. Only the first half was true. Nothing new was designed for it. `apps/web/src/main.ts` has jumped to a record since the command palette shipped, whenever a search hit is chosen; the pin now goes through that same `jumpToRecord`, so there is one answer to "where does a record open" rather than two that drift. The item had been filed as needing a UX decision on the strength of a grep over the viewer directory finding no opener — there is none *there*, and the opener was one directory over. Selection stays in the viewer rather than following the user across: coming back to the Model workspace finds the element still highlighted. The handler lives in `apps/web/src/viewer/pinOpen.ts` so the viewer's `app.ts` did not grow, and the opener reaches the viewer as a required callback rather than an import — required so a dropped wire is a compile error instead of a dead pin, injected so the viewer never reaches into the portal. Three things review found, all real. A pin marker's click also reached the viewport, whose raycast misses a DOM overlay and then clears the selection a few frames later — so the highlight the click had just set was being undone; both marker kinds now stop the event. A rejected `selectByGuid` took the record jump with it, although the record id came from the pin and not from the scene; selection is best effort now, and says so rather than failing silently. And the jump waited on a fixed 500 ms instead of the portal's own init — **there were three copies of that guess**, and all three now share one wait that resolves when the portal is actually ready and reports when it cannot open at all. That last fix created a fourth defect, which the next review round found. Making the portal's init retryable turned every partial mutation on the way to a failure into reachable state: `init()` reassigns its own root to the content pane and registers a window listener before it awaits the first render, which can reject — so a retry would have nested a second shell inside the first and added a duplicate listener. Making a failure recoverable makes every partial mutation on the way to it reachable. ### The project's parcel boundary now draws on the model Load a cadastral boundary on Feasibility → Massing and the lot line is available in the 3D viewer, under Open ▾ → Site context. It is the same ring the building was sized against, not a second copy loaded separately — the two cannot drift. The roadmap carried this as "parcel overlays", which sounds like a drawing task, and both halves of the drawing already shipped: the viewer draws OSM land-use parcels, and the server parses a real cadastral boundary that sizes the building on the lot's true outline. What was missing sat one layer down — nothing persisted the ring. The feasibility tab held it in a local variable and sent it straight to the massing engine; a tab re-render dropped it, and no other screen could ask for it. The ring is stored beside the parcel and tax keys it belongs with, through the merging write that already serialises the property form against the feasibility tab. It is drawn in metres rather than being re-projected through a lon/lat anchor, which is how a lot line ends up a continent from its building. Review then found the frame was wrong in a smaller, quieter way. A saved ring is shifted to its own bounding-box *minimum*, so it lies wholly in the positive quadrant, while a generated massing model is built around the origin — the lot line sat off the corner of its own building. It is now recentred on the bounding-box **centre**, which is the anchor the massing generator already uses to put the parcel in the building's frame. The test that claimed to pin the frame passed before and after that fix, because its only fixture was a square centred on the origin and the translation was a no-op on it. And the save is now serialised per project. Selecting one parcel and then another fired two independent writes; the project lock decides which lands last by arrival, not by which was chosen, so the viewer could end up drawing a parcel the tab was not showing — and a select-then-clear pair could resurrect a parcel the user had removed. Selecting a parcel with no project open now says so instead of skipping the save silently. The queue outlives one render of the tab, which is what makes it a queue: the feasibility tab re-renders whenever the proforma does, including immediately after "Generate IFC model", and a queue rebuilt at that moment cannot order the save that was already in flight across it. **Saving the property no longer deletes the rest of it.** `PUT /projects/{pid}/property` preserved one key across a write — the saved appraisal — and let the request body replace everything else. That read as a merge for as long as the only caller was the property form, which reads the whole record, edits it and sends it back. This change added a caller that sends the parcel boundary alone, and that write took the address, the block and lot, the purchase price, the areas and every tax line with it. The route now merges the body over the stored record: the body still wins where they overlap, so the form can still edit, and an explicit null still clears, because what matters is a key omitted versus a key sent as null. The viewer also waits for a pending boundary save before reading the property back, so opening the overlay straight after selecting a parcel no longer reports that none is saved. ### Four records called concurrency gaps open that the sweep had already closed `services/api/test_rmw_sweep.py` reports *"43 ORM sites: 43 reasoned exempt or locked, 0 named open"*. `docs/roadmap.md` still carried RMW-LOCKGAP and RMW-TOKEN as 🟡 **OPEN**, and `docs/security/threat-model.md` still carried G-10 and G-12 as open gaps with a summary line reading "**Four** sites remain open". All four are corrected against the code, each naming the pull request that closed it and the date. A stale OPEN is a work item, not a typo: the roadmap's ranking sends the next reader at it, the threat model presents a reviewer with a live exposure, and RMW-TOKEN's entry had priced its fix as "a migration (add the column, backfill, route both writers through a CAS)" — work nobody ever did, because both of its sites closed with an advisory lock already in the tree and already serving the gap next door. Half of that entry was wrong on the day it was written: `realestate.save_appraisal` took its lock the day before. `services/api/test_gap_records.py` now fails the build on the class. It derives the sites from the sweep's own ledgers by AST and the records from both documents by indentation, and holds any record that cites the sweep and reads as open to naming at least one still-open site. It replays the four records as they shipped at `53cfa71a` — frozen under `services/api/tests/fixtures/gap_records/` — and must re-find all four, and nothing else, before it may report a clean tree. The first draft read those records out of git history instead of copying them. That works on a full clone and not in CI, whose checkout is shallow, so the gate failed closed on every build. A proof that an analyser finds anything cannot depend on the depth of somebody's clone. Review found two more. The replay was evaluating those frozen records against today's ledger, so a site that legitimately reopened would have red the build blaming the analyser for being right; the statuses are frozen alongside the records now. And the gate's roadmap-marker list knew `✅ ◧ 🟡 ⭐` but not `⛔`, `❌` or a struck-through code — 12 of 90 roadmap items were outside its population entirely, reported as a clean tree in exactly the same words as a clean tree. A loose-versus-strict parity check now reds the build on a marker it does not know, and found one of the three itself. `services/api/test_roadmap_status.py` gained the self-tests it was missing. Its loop reads `if not marked_open(code): PASS; continue`, so a `marked_open` that can never say yes sent every registered item down the passing branch — measured by mutation: breaking its bullet regex with one literal left every line green, the exit status 0, and the verdict *"every item with a measurement agrees with its marker"* printed over nothing. ### The desktop IFC converter can now be interrupted — it runs in a child process the server kills `from_ifc.convert`'s deadline is cooperative: checked between elements and every 4,096 entities. That bounds a model that overran because it had a lot in it, and cannot reach the two costs outside every checkpoint — `ifcopenshell.open`, one call whose time scales with file size and is paid in full before the first check, and any single `create_shape` that never returns. On the desktop app, where there is no Node runtime and the Python path is the only one, `edit_preview` passes `timeout=120` on a synchronous route: a model that hit either cost held a request worker indefinitely instead of returning the 503 the caller is written to expect. The conversion now runs in a child process, and the parent kills it. The entry point re-enters itself through an argv sentinel, so `sys.executable` is the bundle when frozen and the interpreter when not — no `multiprocessing`, and therefore no `freeze_support()` to wire, which is what the roadmap entry had recorded as the blocker for a fortnight. Behaviour where nothing is wrong is unchanged: the child gets the caller's full budget and an ordinary overrun still comes back from its own checkpoint, naming the phase it was in. The kill fires five seconds later, so the outer bound is `timeout + 5s` rather than `timeout` — stated plainly, because shortening the child's deadline to keep the bound exact would make the two branches disagree about what the parameter means. There is no in-process fallback. Spawning failures are not something anyone reports, so a fallback would mean the conversion quietly went back to being uninterruptible while the reader still believed it was bounded. ### The reachability gate was reading 708,650 characters of somebody else's product as evidence about ours `_web_source()` — the blob both reachability gates ask "does any client call this route?" — included `apps/web/src/vendor/**`: verbatim, unedited copies of `MassingCloud/massingifc` and `MassingCloud/massing-pdf`, 83 files. A route leaf occurring there is a fact about another repository. The blob already excluded the generated OpenAPI types for exactly that reason, and the sentence simply had not been finished. **It was not a theory; the consumer had written the defect down twice without naming it.** The gate's own notes blame `/projects/{pid}/cost/calibration` on "85 hits of PDF-takeoff SCALE calibration under `vendor/massingpdf/`, a different trade entirely", and its deterministic regression fixture is a line lifted out of `vendor/massingifc/project-schema/coordination.ts`. The diagnosis stopped at "a leaf that is also a common domain noun" — true, and one layer short, because a vendored tree is a bulk supplier of exactly that noun. *Two symptoms written up separately are how one cause stays unnamed.* Measured: uncalled 57 → 59, nothing lost. Both routes that move — `/projects/{pid}/project-package/contents` and `/projects/{pid}/workflow/{key}` — were already frozen as **invisible** in `LEAF_COLLISION_BLIND`, so this finds no new debt; it converts two routes the rule could not see into two it reports. **That is the direction the ratchet punishes**, which is why nothing was going to drift into it unprompted, and it is also a third way out of the blind-spot list that the list did not anticipate: not "it gained a caller" and not "it was renamed", but *the text that vouched for it was deleted*. The exclusion is asserted the same two ways the generated-types one is — the vendored text is genuinely absent from the blob (probed per package, since the two were vendored on different dates from different repositories), and putting it back changes the answer — plus a third the older check does not have: the differential is **named**, not just counted. Two is small enough that a different pair silently swapping in would leave every other check green. ### The budget form put back the $9,000,000 the GMP sync had just replaced, and both calls said 200 Two findings raised in review on the lock-boundary work and declined there as pre-existing rather than regressions of that diff. They are one mistake in two shapes: **a lock orders writers; it does not tell the second one that what it read is out of date.** **`PUT /projects/{pid}/dev-budget` replaces the whole blob.** An editor whose screen loaded before a "Sync GMP" or "Sync from model" click saves the hard lines that sync had just rebuilt. Serialising the two changes nothing — the stale body lands *after* the sync, in perfect order, and the sync is gone with a 200 on both calls. The route now carries a content-derived `rev` (`dev_budget.budget_rev`), captured under the same lock that does the write, and answers **409** when the budget has moved. `projects` has no `modified_at` to swap on, so the token is the blob's own canonical-JSON hash: two callers holding the same bytes hold the same revision, and no migration is needed. **The comment over that lock justified it by an argument belonging to a different route.** It said the two edits touch different line categories, so serialising lets both survive — which is `connections.update_connection`'s reasoning, and true *there* because that route merges keys into the blob. This one assigns over it. *A justification copied from the route that inspired the control describes that route, not this one,* and it reads as settled precisely because it is true somewhere. **`POST …/dev-budget/sync-from-model` committed a hard cost derived from a model the project no longer had.** It reads the IFC and runs a full geometry takeoff outside the lock — correctly, since holding the project lock across a parse would block every publication for as long as it runs — and a publication landing in between leaves the committed number describing the old model under a 200 saying "synced". *The expensive read belongs outside the lock; what belongs inside is the proof that it is still the right read.* **And the obvious check could not have worked.** `publish_source_ifc` writes to a unique staged path and `os.replace`s it onto the **same** final path, so `Project.source_ifc` is re-assigned the string it already held — comparing it passes on exactly the interleaving it would exist to catch. *A staleness check against a field the writer does not move always passes, and reads as though it were guarding something.* `authoring_shared.ifc_identity` compares `(st_ino, st_size, st_mtime_ns)` instead, and `os.replace` always changes the inode. It lives beside `publish_source_ifc` on purpose: whoever changes how a model is promoted should see what detects the promotion. `services/api/test_budget_rev.py` asserts behaviour and mutation-checks both guards — neutering the `rev` comparison brings back the 200 that loses the synced hard line, and swapping the file identity for the path string (what a check written against `p.source_ifc` compares) lets the mid-takeoff republish commit. It also pins the one caller that actually races: the budget panel must send `rev`, re-read the one the server returns, and handle the 409 by **reloading** rather than retrying — a retry carrying the fresh token would re-send exactly the body the server refused. **`rev` is optional, and that is a bounded decision rather than a fail-open default.** Requiring it would refuse `build_demo_data.py`, `e2e_dome.py` and `e2e_vertfarm.py`, which write a whole budget onto a project they have just created and race nothing. What bounds the decision is the web-source check above, not the sentence in the route saying so. ### Two LPs granted access to one scenario, and one of the grants was never there `proforma.share_scenario` unions a grantee into `Scenario.shared_with` over a value it has already read, so two LPs granted access at the same moment leave one grant behind. **Both callers get a 201 naming their own person** — the response echoes the winner's list — so nothing surfaces until the loser's LP cannot open the scenario and somebody goes looking for a grant that was acknowledged and does not exist. `drawingset.revise_sheet` is the same shape on a sheet's `revisions` list: an accepted revision is counted in its own response and is absent from the sheet and from the cross-sheet register. The scenario site is gap G-10's last band-2 entry, and with it **the ORM read-modify-write sweep closes outright: 43 sites, 43 reasoned exempt or locked, 0 named open.** *That number was rewritten at the moment it became true, not before.* On the base this work was built against it read "four to two, and both survivors are the `dev_budget` pair" — accurate then, and false the instant the lock-boundary change landed and locked that pair. A count carried across a rebase is a measurement of a tree that no longer exists, so it was re-read from `test_rmw_sweep` rather than carried forward. The same failure this file records about narrative copies of a pinned number, arriving by the other door: not prose drifting away from a gate, but prose staying still while the gate moved under it. **A lock, not a compare-and-swap**, for the reason `connections.update_connection` already states in this file: two grants are *independent* claims, so serialising lets both survive where a 409 would refuse an edit that does not conflict. **`db.refresh` inside the lock is half the fix, and the half a lock alone cannot supply.** `_scenario_for` loads the row *before* the wait, so without the refresh SQLAlchemy's identity map hands the loser the list it read on the far side of the lock and the lock serialises two writes of the same stale value — perfectly ordered, and still losing a grant. *A lock makes writers take turns; it does not make the second one re-read.* The key is the scenario, not `Scenario.project_id`, which is nullable. **The ledger keyed the drawing site on `m.data`, and that is the milder half of what is wrong there.** The markup tag it names is guarded by `"carried_from" not in d2` and has no other read-modify-writer; the `revisions` list beside it loses an accepted revision outright. *A derivation keyed by attribute names the site it matched, not the worst thing happening inside it* — so a finding's title is a starting point for reading the function, never a summary of it. **The first fix was wrong in a way that looked like tidying.** Hoisting the body into a `_revise_sheet_locked` helper moved the `with` into the *caller*, and `test_rmw_sweep`'s `under_pid_lock` went red on exactly that: a lock it cannot see in the function it is asked about is a lock it cannot verify. Inlined instead. *A refactor that relocates a guard out of the scope a checker examines turns a verified claim into an asserted one.* `services/api/test_share_revise_lock.py` asserts BEHAVIOUR — both grants survive, both revisions survive, and with the lock neutered the same forced interleaving loses one of each (`shared_with` comes back holding only `lp-a`, `revisions` only `C`). A test that grepped for `pid_lock` would pass on a lock wrapped around the assignment alone. ### LOCK-XPROC — the cross-process lock was proven by two threads in one process **CodeQL flagged this file HIGH on its first push, and it was right.** The connect-failure message printed the DSN verbatim, so a wrong port or an unreachable service container would have written `PGPASSWORD` into the CI log in clear text. *A diagnostic is an output like any other* — and the string most worth printing when a connection fails is exactly the one carrying the credential. **Two rounds of redaction were the wrong fix, and the alert said so by not clearing.** `_safe` and `_scrub` were both correct and neither closed it: CodeQL traces the *flow* from the environment read to the printer, and `str.replace` changes the value without removing the edge. The DSN no longer embeds the password at all — libpq reads `PGPASSWORD` from the environment the children already inherit, so putting it in the URL bought nothing and put the credential into a string that is handed to subprocesses, set as `DATABASE_URL`, and printed on failure. *Sanitising a sink answers "is this output safe"; removing the source answers "is this value sensitive at all", and only the second makes the question stop existing.* The scrubbers stay as defence in depth, and review narrowed what they actually claim: `_safe` redacts userinfo **and** a `password=` query parameter — libpq accepts the secret there, so the `@` split alone never saw it — while `_scrub` removes only `PGPASSWORD`'s value from child output and derives nothing from a caller-supplied `AEC_TEST_PG_URL`. The first wording here said they "cover a caller-supplied URL", which was wider than the code. *A claim about a guard's coverage is a claim like any other, and it is checkable the same way* — so both the function and the sentence were corrected, not just the sentence. **And fixing the sink it named exposed a second one.** The children are handed the DSN through `DATABASE_URL` in their environment and their stdout and stderr are reported in a failure detail, so CodeQL flagged one HIGH after the other was cleared. *Fixing the sink that was reported closes that sink* — the same lesson this branch's lock work keeps arriving at, met again in the security rules. Captured output is scrubbed **where it is captured**, not at each print: a sanitiser applied at every sink is a list that has to stay complete, one applied at the boundary is a property of the value. That call is pinned by reading this file's own source, because `_run_pair` never runs on a machine with no PostgreSQL server — so the guard with the widest blast radius was the one no behavioural check here could reach. *A check that exercises a helper is not a check that the helper is called.* The first draft of the scrub assertion read `… if os.environ.get("PGPASSWORD") == "sekret" else True` — **vacuous on every ordinary run**, the same shape as the `web_files()` sortedness check two entries below. Caught before it was pushed, by the entry about it. The password is a parameter now. It is redacted, splitting on the **last** `@` rather than the first, because a password may contain one and a first-`@` split leaves the tail of the secret in the log. Three assertions cover it, since the only place it runs is a failure path a green run never takes: *a guard that runs only when something has already gone wrong needs a test that runs always.* `test_pid_lock_xproc.py` is named for cross-process serialisation and asserts a great deal about it. Its one exclusion check is **two threads, in one process, on SQLite** — the suite's default database, where `_advisory()` takes nothing and yields `False`, so the exclusion it observes is entirely the in-process `RLock`. `pg_advisory_lock`, the whole of R35-PIDLOCK-XPROC, was therefore exercised by no line of this tree and could have shipped acquiring nothing. **A test named for a property can assert every neighbouring property and never the one in its name.** What it does prove across processes is that two workers derive the same KEY: necessary, and silent on whether taking it excludes anybody. `services/api/test_pid_lock_pgxproc.py` asks the question. Two real OS processes contend for one project's lock against a real server; their held intervals must not overlap. Two mutations, because either alone passes a lock that has stopped locking: two DIFFERENT projects must overlap — otherwise the positive check is measuring the order the harness starts things in, and a lock that excluded everything would look identical — and a per-process advisory key must overlap, which is note 1's `hash()` defect reinstated across real processes. Verified by deleting the acquisition outright rather than by reasoning: the behavioural check reds independently of the static one. One check *stayed green* under that mutation and had its label narrowed rather than left standing — `cross_process_status()` reports the DIALECT, so it says the advisory path was available, never that a lock was taken. It also pins the acquisition as the BLOCKING form. `_advisory` sets `acquired = True` without reading a result, because `pg_advisory_lock` returns void; a `pg_try_advisory_lock` "don't block the request" change would leave the code reporting a lock it does not hold. That static pin is redundant wherever a server exists — with the swap applied the behavioural check reds too, deterministically, since B starts only once A is demonstrably inside. It earns its place on the run with NO server, where the exclusion arm cannot run at all. With no server it does not pass quietly: it asserts that `.github/workflows/db-migrations.yml` still runs it with `AEC_PG_REQUIRED`, so deleting that step reds the ordinary suite instead of silently removing the only place the check is real — the construction `services/api/test_pin_pgnull.py` uses, for the same reason. ### RMW-LOCKBOUND — a bounded wait, and the correctness bug that bounding it nearly introduced `pid_lock.mutating` serialises sidecar read-modify-write per project with `pg_advisory_lock`, which blocks until the holder releases — **with no ceiling**. All 30 call sites are plain `def`, so FastAPI runs them in the threadpool: a stuck holder does not wedge the event loop, it pins threadpool workers, and Starlette's default pool is 40. *The entry on record described it as blocking the server, and named 11 call sites. Both were wrong, and the mechanism mattered more than the count — "it blocks the event loop" and "it exhausts the threadpool" send a reader to different fixes.* `SET LOCAL lock_timeout` now bounds the wait; a timeout raises `LockTimeout(TimeoutError)`. **Three premises, each probed against PostgreSQL 16 rather than recalled.** That `lock_timeout` bounds an ADVISORY lock is not stated in so many words anywhere, so it was measured: SQLSTATE `55P03`. That a plain `SET` is connection-scoped and rides the pool back out was measured too — 1500 ms still in force on the next checkout of the same backend — which is why the statement is `SET LOCAL`, and why the advisory lock surviving that `SET LOCAL`'s commit had to be measured as well. Any one of the three being false sinks the shape. **The third premise is the dangerous one.** A lock timeout and an unreachable server raise the *same* SQLAlchemy class; only the sqlstate separates them. `_advisory` already catches broadly and degrades to in-process, loudly — correct for "there is no Postgres here". Caught as `except OperationalError`, a timeout lands in that same branch and the contended write **proceeds having taken no cross-process lock at all**. Bounding the wait would have converted an availability problem into a silent correctness one, strictly worse than what it replaced. *A bound is only safe if it cannot be mistaken for the absence of the thing it bounds.* **Two bugs in the first draft, both found by the gate rather than by reading.** `SET LOCAL lock_timeout = :ms` is a **syntax error** — `SET` takes no bind parameter — so it did not merely fail to apply the timeout: it raised, fell into the degrade branch, and let the write proceed unserialised. The exact failure above, reached by a door it did not anticipate, in the change written to prevent it. And the static check read its own **prose**: `"SET lock_timeout" not in src` failed on *correct* code, because the comment explaining why a plain `SET` is wrong contains that string. *A check that greps a source file is reading whatever a future author writes ABOUT the code alongside the code — and prose is exactly where the forbidden form gets named.* Comments and docstrings are now stripped, with a self-test in both directions. **A measured limit, stated rather than hidden.** The degrade-branch mutation **survives** a static-only run: the sqlstate constant stays in the source, unreachable. Only the two-process arm sees it, and that needs a server the API gate's job does not have — so it runs in `.github/workflows/db-migrations.yml`, and `services/api/test_pid_lock_bound.py` asserts that step is still there. *A gate whose real arm runs in exactly one place is one deletion away from decoration, and the deletion is in a different file from the gate.* Four mutations: removing the `SET LOCAL` reds five checks (B waits out the full hold); folding the timeout into the degrade branch reds two (B enters the critical section having timed out and written anyway); deleting the CI step reds two; dropping `AEC_PG_REQUIRED` from it reds one. **Review round 2 found the lock's lifetime was wrong in BOTH directions, and the fix is the shape that satisfies neither constraint alone.** A session-level advisory lock lives on a connection, so its lifetime IS that connection's lifetime — and two requirements pull opposite ways: * **End the transaction and the connection goes back to the pool.** Measured under contention: the backend changed between acquisition and unlock, `pg_advisory_unlock` returned `False`, and the real lock stayed stranded until recycling while another caller held that connection. * **Leave it open across the critical section and `idle_in_transaction_session_timeout` kills it.** Measured with that timeout at 1 s and a 3 s section: the acquiring session was terminated, the lock died with it, and a rival writer walked in. **A lost update** — precisely what the lock exists to prevent, arriving through the fix for the other half. A `Session` can satisfy neither cleanly. A checked-out `Connection` whose acquisition transaction is *committed* satisfies both: the connection is held to `close()`, nothing is left idle in a transaction, and `SET LOCAL` still reverts before the connection is released. *Three of this change's verifications were themselves wrong before they were right, each in the same shape.* The connection probe cleared the pooling bug because an idle pool hands the same connection straight back — five rival Sessions were needed to tell "retained" from "released and re-issued". The static check for it matched a substring that tokenising makes unmatchable. And the idle-timeout check set the GUC on **its own** connection while `pid_lock` opens another, so the Session-shape mutation sailed through it; it now sets it on the DATABASE, which is also how a managed PostgreSQL imposes it. **Each one reported success while measuring nothing, and only a mutation distinguished them from a real check.** ### DESKTOP-CONVERT-TIMEOUT — the parameter reached one of the two branches `fragconvert.convert_ifc` has always taken a `timeout`. It reached `subprocess.run` on the Node path, where an overrunning child can be killed, and reached **nothing at all** on the Python path, which calls IfcOpenShell in the caller's own process. `_publish` runs on a background worker and does not care. **`edit_preview` is a synchronous route passing `timeout=120`**, so a slow model held a request worker past its own deadline instead of returning the 503 that route is written to give — and the parameter's *presence* is what made the gap invisible. It was filed rather than fixed because the obvious repair is a killable child process, which is net-new spawning machinery in a PyInstaller bundle. **That was the right call about that repair and the wrong conclusion about the problem**: `from_ifc.convert` already had a per-element loop with a progress hook, so a *cooperative* deadline fits with no process isolation at all. It now checks the clock after the parse, between elements, and every 4,096 entities in the index loop; `convert_ifc` computes the deadline from the caller's `timeout` (not the duration — the callee would restart the clock, and `edit_preview` authors a one-element IFC before it calls). Both paths raise `TimeoutError`: Node's `subprocess.TimeoutExpired` is a `SubprocessError`, so telling "too slow" from "broke" meant knowing which converter ran, which is the fact this function exists to hide. **The entry stays OPEN, deliberately — and the first draft of this paragraph overstated what the fix does.** It said the change "bounds the cost that scales". It does not: `ifcopenshell.open` is a single call whose time scales with file size, and the checkpoint is *after* it, so a large file is parsed in full and only then refused. The accurate claim is **enforced at phase boundaries and within the two loops** — the geometry loop per element, the entity index every 4,096 — with the parse and any one hanging `create_shape` still outside every checkpoint. *Caught by re-reading my own claim against the code before review did, which is the only reason it is not in the shipped prose.* *A partial fix described as a fix is how the original asymmetry became invisible in the first place.* **Two defects in the gate, both found by its own mutations rather than by review.** Reinstating the pre-fix state red it, but deleting the parse and geometry checkpoints while leaving the entity-index one **passed** — that tick fires at entity 0, so the deadline was still detected, after all the geometry work was done. The timing assertion meant to catch that was vacuous: `elapsed < max(0.25, baseline * 0.9)` against a 33 ms fixture is satisfied by doing the whole conversion. *A threshold generous enough not to flake on a slow runner is generous enough to measure nothing.* The check now asserts the **stage name** in the raised message — a deadline already past must be caught at `parse`, before any geometry — and a second case crosses the deadline mid-loop through a deliberately slow `progress` hook, so both checkpoints are load-bearing. And the always-raise mutation used to kill the run with a traceback instead of naming a contract; the positive control now catches it. The first draft of that positive control also failed on *correct* code, asserting byte equality between two conversions — `metadata` embeds `"created": datetime.now(...)`. It compares the parsed model instead. *An assertion that fails on correct code costs a round exactly like one that passes on broken code.* **And forcing one branch to reach the code under test stopped testing the other.** Every check above sets `have_converter()` false to drive the Python path, so the Node half of "both paths raise `TimeoutError`" was asserted by nothing — a regression letting `subprocess.TimeoutExpired` escape would have passed the gate that exists for exactly that promise. Raised in review. *The act of forcing a branch is itself an act of un-testing its alternative, and it leaves no trace in the output.* Two arms, because neither covers the other. **Injected**: `subprocess.run` raises `TimeoutExpired` directly, proving the handler converts it; runs everywhere. **Real**: a Node process that does not finish, under a real budget, proving the whole chain including the argument being passed. Measured rather than argued — with `timeout=timeout` deleted from the call, the injected arm still **passes**, because its exception came from the stub and not from the runtime. The real arm needs a `node` the API gate's job does not install, so it is conditional, and a conditional arm that skips everywhere is a check reporting good news: the arms that ran are named in the output and at least one is required. **The real arm's first draft turned that mutation into a hang rather than a failure.** Its script only blocked, so dropping `timeout=` left `subprocess.run` waiting for ever — caught, in the sense that the gate never went green, but reported as nothing at all and paid for with a CI job's whole budget. The script now exits on its own timer too, so the same mutation returns normally and the arm fails **by name**, and a margin check separates "gave up at the budget" from "waited for the script". *Make a check fail; do not settle for it not passing.* The lane table's row for this item was also still carrying the pre-fix text two paragraphs of narrowing later — and its stated REASON, "the fix is process/packaging shape", is why the item read as unactionable for thirteen days: that is the repair the residue needs, recorded as the repair the whole item needed. *A correct decision with a wrong reason attached is worse than no reason, because the reason is what the next reader plans against.* ### An exclusion whose own differential is blind to what it wrongly excludes The vendored-tree exclusion below shipped as `"/src/vendor/" in q` — a match on the *parent* directory rather than on the packages. Review named the hole, and it is the same shape the exclusion exists to close, one level out. A first-party caller added under `apps/web/src/vendor/` would be dropped from the blob. **And the differential beside the exclusion cannot see that**, because it asks what restoring the vendored text *vouches for*: if the new caller calls `/projects/{pid}/workflow/{key}`, the vendored copies already vouch for that route, the difference is unchanged, and the route stays frozen as uncalled while the client calls it. *A differential over what an exclusion REMOVES says nothing about what it removes WRONGLY* — a file wrongly excluded is absent from **both** sides of that subtraction, so it moves nothing and is invisible. The excluded set is now named — `vendor/massingifc/`, `vendor/massingpdf/`, matched on the path relative to the web root. Two checks, because the first alone would rot: - The tuple must equal the directories under `vendor/` carrying a `VENDOR.md`, **in both directions**, so the list cannot drift from the tree. - A synthetic sibling package the tree does not contain must still be READ. Every real directory under `vendor/` is vendored, so the tree *cannot* distinguish a named exclusion from a parent-directory one — the same empty-population trap that made the `.tsx` sort check vacuous. The list is literal rather than derived from the `VENDOR.md` markers, deliberately: a blob that decides what to read from a marker file changes meaning when somebody deletes one. The marker checks the list instead. *Hardcode what is load-bearing; derive the proof that it is right.* Reinstating the parent-directory match reds the sibling probe **and nothing else**, which is the finding restated as a measurement: no check that existed could catch it. Dropping a package from the list reds five, the derivation check among them. And the fixture itself was wrong on its first run — it restored `glob` from a name bound two hundred lines further down the file, so it crashed instead of asserting. *A fixture that restores from a name defined later restores nothing.* ### Looking one way down the list is not reading the list The round below stopped asking "is it written down" and started asking "can it fail the job", and then answered it by looking at the separator that **follows** the invocation. A review pass named two shapes that pass with the status masked by nothing at all: - `true || python ` — nothing follows the invocation, so nothing is masked, and bash never reaches it. Reading one token forward is not reading the command list. - `python && echo passed` on one line, `echo done` on the next. `bash -e` suspends itself for every command of a `&&`/`||` list except the last, so a failing test does not end the step; the list's own status is then discarded, because a later list runs and the step exits with `echo done`'s zero. **The single-line form of that same text is correct** — there the failing list is the last one and its status *is* the step's — and the check added in the round below tested only the single-line form. *A shape and the same shape one line longer are different programs.* `bash -e` reasons about command **lists**, not about commands, so the guard now does too: a `run:` block is cut into lists at `;` and at line ends, each list into segments at `&&`/`||`/`|`/`&`, and an invocation counts only when its status can reach the runner — nothing masking after it, no `||` before it in the same list, and, if the list is conditional at all, that list must be the last in the block. A `&&` *before* the invocation is deliberately still accepted: `cd services/api && python ` is the ordinary idiom, and the same limit `_parked` already states applies — a constant written to be false is caught, a condition that merely evaluates false is not. Three new self-tests, and the command test is split out from the control-flow test so each can be mutated alone. Deleting either guard reds exactly its own self-test and nothing else; widening the conditional-list rule to reject *every* conditional list reds four positive checks, including the shipped step's own shape. Three further mutations against the real `db-migrations.yml` — the step behind `true ||`, the step as a two-line `&&` block, and the status masked with `|| true` — each take the found-step count to zero. ### "Does it run" and "can it fail the job" are different questions I had asked review whether `_invokes` could ever fail **open**. It can, three ways, none of them the way I asked about — and all three are the same class the last four rounds were about, one level further out than any of them reached. **The step can be skipped, its failure can be ignored, or its exit status can be thrown away.** `if: false` is the ordinary way to park a step; `continue-on-error: true` runs it and shrugs; `python || true` discards the status, and `python | tee log` hands it to `tee`, because GitHub runs `bash -e {0}` with no `pipefail` unless `shell:` says otherwise. In the last two the file *is* executed — which is exactly what the previous rounds established — and in the first it is not executed at all; what all three share is that the `run:` text, the `AEC_PG_REQUIRED` value and the step selection are identical to a live step, so every check added so far stays green while CI asserts nothing. (The first draft of this paragraph claimed execution in all three, which `if: false` plainly contradicts — found in review. *A sentence generalising over cases it lists is worth re-reading against each of them.*) *Each round asked a sharper version of "is it written down" and none asked "can it fail".* The separator that **follows** a command segment now decides whether it counts (`;` and `&&` keep the status; `||`, `|` and `&` do not), and a job or step that is parked or cannot fail is excluded. Explicit `continue-on-error: false` still counts, however it is spelled — `bool("false")` is `True`, so the obvious predicate would have red a correct workflow, and a gate that cries wolf gets deleted. Only the *constant* false is treated as parked: a real condition is not evaluated here and is not assessed, which is stated rather than implied. **And the budget panel's reload was still racing the editor.** Holding the request slot across the conflict reload stops a save from *leaving*; it does nothing about the keystroke. `lines.splice` replaces the array and `paint` rebuilds the inputs from it, so an edit typed in that window is gone from both the model and the screen, and the queued follow-up then saves the server's copy back. Editing is now closed while the reload runs and restored by `paint` — which is also what the reload calls when it finishes, so a disable applied anywhere else would be undone by the next repaint. Reconciling a keystroke against a list replaced wholesale needs a rule nobody has chosen, and guessing one silently is how a budget acquires a number no one typed. Seven new self-tests. Four mutations, three of them against the real workflow: the step's failure masked with `|| true`, the step parked with `if: false`, and `continue-on-error: true` — each takes the found-step count to zero. ### Three fixes that each narrowed the right thing and stopped one question short A second review pass over the round above found one more defect behind each of its three fixes. The shape repeats, which is the interesting part: every one of them narrowed the correct subject and then asked the old question about it. **Serialising the requests was not serialising the state.** One save in flight stops two sends carrying one `rev` — but `again` only covers a send *that flow* queued, and says nothing about a `setTimeout` the debounce armed while the save was on the wire. That timer fires during the reload carrying the pre-conflict lines and the stale rev (another self-inflicted 409) or after it, carrying the server's own copy straight back. And the reload is asynchronous: releasing the in-flight slot when the *save* settles lets a keystroke start a request against a `lines` that is mid-replacement. The 409 path now cancels the pending timer and holds the slot until the reload resolves. *The reload mutates the state every request is built from.* **Extracting the right workflow step was not asking whether that step runs anything.** The guard matched a step whose `run:` merely *contained* the filename — so commenting the invocation out, the ordinary way somebody parks a slow step, leaves the substring in place, the step is still found, its `AEC_PG_REQUIRED` is still set, and the guard reports that CI runs this file while CI runs nothing. Comments are stripped per line and the name is compared as a whole basename. The `#` split is crude and fails in the safe direction: a `#` inside a quoted string can only *lose* an invocation, which reds the caller, because the caller requires exactly one step. **And that first fix was still wrong, for the reason it was written.** It rejected a commented-out invocation and a longer filename and went on accepting `echo test_pid_lock_pgxproc.py` — review declined to close the finding on exactly that ground. *Excluding the shapes you were shown is not answering the question those shapes were examples of.* The head word of the command segment holding the name must now be a Python runner, or be the script itself. The runner list can go stale, and it does so in the direction that gets noticed: a missing runner makes the step invisible, the caller requires exactly one step, the build reds, somebody adds the name — while the opposite error passes quietly. Emptying the list is a mutation and it reds three self-tests. *When a list must be incomplete, put the incompleteness where it fails loudly.* **The positive control earned its place the same hour.** The first version of that narrowing matched the filename as a whole token before asking what the command was, and the token pattern excludes a preceding `/` on purpose — so a direct exec `./test_pid_lock_pgxproc.py` never reached the command test. Comparing basenames answers both questions at once with no lookbehind to get wrong. Without a check that real invocations are still *accepted*, that would have shipped as a stricter-looking gate that had quietly stopped recognising a legitimate way to run the file. **And then the claim I put to review turned out to be false.** Asking whether the command split could ever *invent* an invocation, I asserted that every parsing error loses one instead — and measured it before the answer came back. `echo "a; python test_pid_lock_pgxproc.py ; true"` returned true: a regex split cannot see quotes, so a `;` inside a string ended the `echo` early and handed the rest back as its own command. *A claim about which way a heuristic fails is a claim like any other, and this file's entire subject is that an unchecked one reads as settled.* The line is now tokenised with `shlex` in POSIX mode with `punctuation_chars`, which respects quoting for the comment marker **and** the separators, so one change closes both — including a case the character-level strip got wrong in the other direction, where a quoted `#` swallowed a real invocation after it. Per line, because a `run: |` block is a sequence of commands and newlines are whitespace to `shlex`. An unbalanced quote raises and that line is skipped: unreadable is not "runs it", and the caller requires exactly one step, so skipping reds rather than vouches. **Merging the environment in force was not reading the value.** `AEC_PG_REQUIRED: ""` is exported by GitHub as an empty string, which the consumer reads as opted *out* — so the key-presence check passed a workflow whose step would take the no-server branch and exit 0. The predicate is now derived from the consumer rather than restated, which is also why `"false"` and `"0"` count: a workflow `env:` value is exported as a string, and any non-empty string is truthy there. That is a fact about GitHub Actions, not a choice. Four mutations, two of them against the real workflow — commenting out the invocation, and setting the flag to `""` — plus two on the panel. The cross-process gate was re-run against a live PostgreSQL 16 after the change: 24 checks, all green. ### The budget panel manufactured its own 409, and two of this PR's own checks were measuring the runner Three findings from a full review pass on the head that added the `rev` precondition. All three are inside work landed earlier in this same branch, and the first is a product defect the precondition itself created. **A debounced save that races itself supplies its own stale write.** `clearTimeout` cancels a save that has not left yet; it does nothing to one already on the wire. On a connection slower than the 500 ms debounce, two saves go out carrying the same `rev` — the server commits the first, refuses the second, and `reloadAfterConflict` replaces the user's newest keystroke with the server's copy while telling them "the development budget changed elsewhere (a GMP or model sync, or another editor)". *A staleness token turns a lost write into a refused one; it does not decide WHOSE write is stale.* One send is now in flight at a time, the body is built when the request starts, and a keystroke during a save sets a flag rather than queueing a second request — so one follow-up send carries every edit made while the first was in flight, instead of N sends of the same final state. **The workflow guard searched the whole file for a flag its neighbour also sets.** With no PostgreSQL server the cross-process gate cannot run its assertions, so it proves instead that `db-migrations.yml` still runs it *with* `AEC_PG_REQUIRED`. It asked whether that string occurred anywhere in the workflow — and the pin-sweep step sets it too. Measured: deleting the flag from this file's own step leaves three occurrences and the old check green, so the CI step would have taken the no-server branch and exited 0 with nothing exercising the advisory lock anywhere. The workflow is now parsed, the step that runs this file is extracted, and the flag is looked for in the environment actually in force for *that* step — workflow, job and step `env` merged, so hoisting the flag to the job stays valid. *A check scoped to the FILE cannot speak about a STEP, and a neighbour's correct configuration is what makes the difference invisible.* **And two timing checks had a window that was a constant.** Child A held the lock for a fixed 1.5 s while the parent started child B; if B's interpreter, import, status call and connection took longer than that, B entered after A left, and the two MUTATION checks — which *require* an overlap — failed on a lock working perfectly. A now waits for B's own log line before starting its hold. Measured on a real server: with the hold shrunk to 50 ms, the old shape fails both mutations and the new one passes all three. *A timing check whose window is a constant is measuring the runner.* The extractor's three self-tests run on **every** invocation, including the one with a server — otherwise the only branch that exercises it is the branch a broken extractor would make vacuous. ### Round 16: the RVT translation was still on the event loop, and the sort check could not fail A full review pass over the current head found two, and both were inside this PR's own earlier fixes. **The APS translation in `import_rvt` still ran on the event loop.** Round 12 moved `staged.write_bytes` off it, with a comment explaining that a converted IFC is hundreds of megabytes and blocking the coroutine freezes every other request in the process. The call that *produces* those bytes — `aps.translate_rvt_to_ifc`, which `convert.py` documents as running "for many minutes" and already sends to the threadpool for that reason — sat directly above it, unwrapped. *Fixing the blocking call you are looking at is not the same as fixing the blocking call that dominates the route*, and the smaller one was the one that already had a comment pointing at it. **And the new determinism check could not fail.** `web_files()` sorted inside its glob loop, yielding `[every .ts sorted] + [every .tsx sorted]` — deterministic, but not sorted. The assertion that it is sorted passed anyway, because the tree holds **0 `.tsx` files against 658 `.ts`**: the fix was accidentally right and the check was accidentally green. The day somebody added a `.tsx` the check would have gone red with a message blaming the glob — *a check whose failure message can misdiagnose is worse than one that stays silent, because somebody acts on it.* The sort is now one pass over the whole list, and the contract is probed by feeding a synthetic `.tsx` through the real function, since the tree cannot supply one. *A check that passes because its population is empty has the same shape as one that passes because the code is correct.* Reverting either fix reds exactly one assertion, naming the right cause. ### The "CI flake" in the reachability gate was a backtick in a comment, and the answer depended on directory order `test_route_reachability` went red on 2026-09-23, passed on a re-run of a byte-identical tree, and went red again on the next push. It was filed as a transient of unknown cause. It is neither transient nor unknown: **one call in the file handed `leaf_is_called` the RAW web source instead of the comment-stripped one**, and that one argument made the gate's answer depend on the order `glob.glob` happened to return ~400 files in. `string_blob`'s template-literal alternative spans newlines, because a real template literal does. This tree quotes identifiers in backticks throughout its **comments**, so an unpaired backtick in one comment opens a phantom template that closes at the next comment's backtick — and everything between is emitted as a single string body **with its inner quotes intact**. A leaf whose only vouching occurrence is a quoted word then sits in that body preceded by `"` rather than by the NUL that anchors "opens a string", and reads as *uncalled*. Which comments pair with which depends entirely on the concatenation order. Measured: **1 of 25 shuffled orders flips `workflow`**, and `string_blob(BLOB)` ranges from **933k to 2,014k characters** across orders; read through the stripped source, the derived verdicts are identical in every order. *A gate whose answer depends on directory enumeration order cannot be reproduced from its own failure* — which is exactly why the first instance was dismissed. "Flake" was a description of the symptom standing in for a cause, and it bought a second red build. **Fixed four ways, because the one-word fix is the one that drifts back.** The call is stripped; `web_source.web_files()` is split out and **sorted**, so the blob is at least the same blob twice; and both properties are asserted — a four-line fixture that reproduces the backtick swallow without depending on anything in the tree, and an order-invariance check that rebuilds the real blob reversed and requires the same uncalled set. The fourth is the one that matters most: **the file now reads its own source** and fails if any call passes the raw blob, because neither behavioural check reds when somebody writes `BLOB` there again, and *for a 1-in-25 defect "run it again" is not an alternative to a check.* All four are mutation-checked — reverting the call, inverting the fixture, blinding the AST scan, and dropping `sorted` each red exactly one assertion, with a message that names the right cause. The `sorted` assertion exists precisely because dropping it changes no verdict today: *the change nobody can measure is the change nobody notices reverting.* ### LOCK-BOUNDARY round 15 — the answer collected and then discarded, and a test sharing a directory with its own failures Two findings. The first is inside round 14's fix; the second is a test-hygiene defect that would have broken the suite permanently for anyone whose run died at the wrong moment. **1. `pid_lock_names` collected the bare-lock names and returned a boolean.** The names were gathered into `bare_names`, the shadowing was subtracted correctly — and then the function answered yes/no, so `is_the_lock` fell back to the literal spelling `mutating`. Both directions are wrong at once: ```python from aec_api.pid_lock import mutating as held def writer(db, pid, mutating): with mutating(pid): # the PARAMETER certifies p.source_ifc = "x" ``` `bare_names` is `{"held"}`, `shadowed` holds `"mutating"`, the intersection is empty, the flag says yes — about a name nobody bound to the lock. And the genuine `held(pid)` is **never** certified, so the docstring promising the bare form was false for the one spelling that actually reaches it. *Asking the right question and then throwing the answer away* — the lesson `services/api/test_pin_pgnull.py` already records, arriving here by a different door. `lock_spans` now checks membership in the names the import bound. Probes `aliased_bare` (must certify) and `bare_alias_shadow` (must not); restoring the boolean reds them by name. **2. `test_ifc_publish_atomic` ran in a fixed `./_ifc_atomic` and never removed it.** That file deliberately *keeps* one `.rollback-*` backup and then asserts there is exactly one and that no other rollback scratch is left behind. In a shared directory those two assertions are armed by any run that dies between creating the kept backup and the cleanup at the end — a crash, a timeout, a cancelled CI job — and then **every later run fails**, for a reason unrelated to the code under test, until somebody deletes the file by hand. `run_tests.py` reports `_ifc_*` as residue it does not own, so the runner never clears it either. **Demonstrated, not argued**: with the old fixed directory and a single planted `.rollback-stale-from-a-crashed-run`, the run fails on *both* named assertions. The directory is now `tempfile.mkdtemp()` per run, removed only on a pass — a failed run's staged files, backups and published bytes are the evidence somebody will want. *A test that asserts "nothing was left behind" must not share a directory with its own previous failures.* ### LOCK-BOUNDARY round 14 — the same fail-open, in the two spellings the fix did not look at One finding, and it is inside round 13's own fix. Round 13 closed a fail-open where `_lock_import_names` accepted `pid_lock` imported from **any** module, so `from .fakes import pid_lock` certified as the project lock. The repair was to recognise the lock by its **source module** — and the docstring said exactly that. The code under it went on matching a **suffix**: - `(node.module or "").endswith("pid_lock")` accepts `from .fake_pid_lock import mutating`, `from .fakes.pid_lock import mutating`, and `from tests.pid_lock import mutating`. Each returns `mutating`, `pid_lock_names` returns `bare=True`, and `lock_spans` certifies `with mutating(pid):`. - the `ast.Import` branch matched `a.name.split(".")[-1]`, so `import fakes.pid_lock as pid_lock` is certified too — and for `import x.pid_lock` **without** `as`, Python binds `x`, not `pid_lock`, so the analyser was recording a binding the language does not make. *A fix aimed at the spelling that was reported closes that spelling.* The rule was right and stated plainly one paragraph above the code that ignored it, which is the part worth keeping: **the prose was true and the test was not**, and prose agreeing with itself is not evidence. Recognition is now by exact module path — relative `pid_lock` or absolute `aec_api.pid_lock`, with `level` checked so a top-level module of that name cannot stand in — and the `import` form is accepted only when aliased, the one shape that binds a usable name. Two probes added, `fake_bare_import` and `fake_module_import`; restoring either the suffix test or the last-segment match reds the violation list by name. ### LOCK-BOUNDARY round 13 — a live unlocked writer the analyser could not see, by construction Four findings, all real. One is a **product defect**, not a gate defect, and it is the kind this whole pull request exists to close. **1. `bim.patch_project` swapped `Project.source_ifc` outside the lock.** `ProjectPatch` declares `source_ifc`, so an admin PATCH can move the pointer that `bake_layers`, `edit` and every authoring route read, spend a recipe on, and write back — the same race the five writers in this PR's title had. It survived twelve rounds of a gate built to find exactly this, **for a structural reason rather than an oversight**: the write is `setattr(p, k, v)`, which carries no `Store` context and no `Attribute` node, so an analyser that asks the grammar for attribute writes is blind to it however carefully it reads. `_stored_attrs`' docstring claimed `ctx=Store` was *"the complete, exact marking of an attribute write in EVERY spelling."* It is every **syntactic** spelling — a different and smaller claim. Python spells attribute writes two ways, syntactically and through the reflection API, and *a derivation that asks the grammar is only as complete as the grammar's own account of what it is looking for.* `dynamic_writes` now covers `setattr`, judged by the same `lock_spans` traversal keyed on the call node; the route takes `pid_lock.mutating(pid)` around the lookup, the refresh and the commit. **The gate found the defect on its own once taught the spelling** — it reported `patch_project writes Project. with mentions outside the lock at [759]` before the fix. The dynamic rule is coarse in the fail-closed direction — an unlocked `setattr` on a model with *any* protected attribute is a violation, because the analyser cannot prove it wrote a different one — and that is affordable because the population is seven sites tree-wide, measured. Three receivers do not resolve (two ifcopenshell entities, one `Viewpoint` reached through a generator) and are declared in the exemption ledger with stated reasons rather than inferred away. **2. The shadowing list was short again — a fifth family.** `case pid_lock:` binds through `MatchAs.name`, with no `Name` node anywhere, so a captured subject certified as the project lock. `MatchAs`, `MatchStar` and `MatchMapping.rest` are now handled. The sting is where the claim was made: round 12's docstring said the list of string-field binding forms was **"closed by the language"** while naming four of seven. *A completeness claim about a hand-written list is a claim the list cannot check* — asserted in the very sentence explaining why enumerations fail. The docstring now says what is true: as complete as the last reading of the grammar. **3. Lock recognition disagreed with itself in both directions at once.** `_lock_import_names` and `pid_lock_names` each matched `a.name == "pid_lock"` from *any* module, so `from .fakes import pid_lock` certified — the `import_shadow` probe missed it because that probe renames and this substitution does not. *A probe written against one spelling of a substitution does not cover the substitution.* And in the other direction, round 12's own shadowing fix made `from ..pid_lock import mutating` **unreachable**: the import bound no lock name, so `"mutating"` entered the shadow set and `bare` could never be true. The form the docstring promised to accept had been switched off by a fix, invisibly, because nothing in this tree spells it that way. Recognition is now by **source module**, in one place both callers read. **4. `_rel` emitted backslashes on Windows**, which is where this repository is developed, so the `/`-separated keys in the exemption ledger never matched, `stamp_conformance`'s receiver fell through to UNRESOLVED and the real-tree verdict red — **there and only there**, because the ledger self-test normalises separators before comparing. *A check that normalises in one place and not the other is green where it looks and red where it runs.* The self-test uses `PureWindowsPath`, which is platform-independent, so it reproduces the Windows string on Linux instead of asserting something that can only hold here. Each fix is mutation-checked: dropping the match forms, matching the lock import by name alone, returning `str()` from `_rel`, removing the `setattr` collector, and unlocking `patch_project` each red a specific named self-test. **One of those five did not, at first, and the gap was in the check rather than the fix.** The Windows self-test asserted the property against `PureWindowsPath` directly and never called `_rel`, so reverting `_rel` to `str()` red nothing — a check written beside the guard instead of through it. The separable half is now `_posix_rel(p, root)`, which the self-test calls, and the mutation reaches it. *A static check that still passes with the guard removed is not testing the guard* — this file states that rule about `_system_field` and then broke it about itself one round later, which is the argument for running every mutation rather than the ones expected to bite. ### LOCK-BOUNDARY round 12 — the third statement-type list, and a sentinel spent as a model Four findings, all in-diff, all real. Three are in `services/api/test_lock_boundary.py`; one is a route that blocks the event loop. **1. The scope merge inherited the enclosing model for a name the inner scope had taken over.** `_bindings` marked a name meaning two models `CONFLICT` and then *dropped the entry at its own return* — so `collect`'s `dict.update` chain found nothing for that name in the inner scope and the **outer** function's model flowed in. An unlocked `Project.source_ifc` write inside a helper that rebinds `p` was therefore classified as the outer `SavedView`, and reached neither the violations nor the unknowns. Two sentinels now survive the merge — `CONFLICT` for a name meaning two models and a new `UNRESOLVED` for one this analyser cannot pin down at all — and `_resolved` removes them **after the last merge**. *A sentinel filtered before the merge cannot shadow anything*, which is the whole mechanism by which an inner binding is supposed to beat an outer one. The probe runs through `collect` in a temp directory, because nothing calling `_bindings` alone can see a defect in the merge. **2. The shadowing check in `pid_lock_names` was a list of statement types, and the list was short.** Third time in this analyser, third time short. It named five forms and still certified an arbitrary object as the project lock for `pid_lock, _x = other, 1`, `from .fakes import stub as pid_lock`, `except Exception as pid_lock:`, a nested `def pid_lock(...)` — and a **module-level** rebinding beside the import, which a per-function scan cannot see at all although it is the one rebinding whose blast radius is the entire file. **The fix for an incomplete enumeration is not a longer enumeration**: the new `_bound_names` asks the grammar — `ctx=Store` on `ast.Name` is the exact, complete marking of a name assignment in every spelling, exactly as `_stored_attrs` learned one node type over — and names explicitly only the four forms that carry their name as a plain string field rather than as a node (`except ... as`, `def`/`class`, import aliases, `global`/`nonlocal`). That list is closed by the language, not by what this tree happens to contain. Module scope is now in the chain the check scans. **3. `NOT_A_MODEL` was spendable as a model.** The sentinel meaning *"proved not to be a mapped instance"* is a non-empty string, so `if m and ok` accepted it and a locked write through `self` in any non-mapped class seeded a protected pair keyed on the sentinel. Nothing ever resolves *to* it, so no violation could match — but its **attribute** entered `prot_attrs`, the set that decides whether an unresolved receiver is worth reporting, so unrelated unlocked writes were raised as UNKNOWN on the strength of a pair protecting nothing. **4. `import_rvt` wrote a whole converted IFC on the event loop** — `staged.write_bytes(ifc)` on the coroutine, one line above a `run_in_threadpool` the same route already used for the publish. A converted model is routinely hundreds of MB; the loop stalls for every other request while it lands. This is the v0.3.703 SSE failure in miniature. Deriving that shape over the tree found **three more instances, all in one other route plus the converter**, and fixing only the line review pointed at would have left the *largest* blocker two lines above it: `raise_plan_to_bim` ran the DXF parse, the ifcopenshell raise, a whole-file read, a whole-file write and a `storage.put` network round trip on the loop; `convert` read its fragments output back on the loop having just handed the translation *off* it. All are now awaited through the threadpool. **No gate is claimed for this** — a check matching `read_bytes`/`write_bytes` would be a completeness claim with a filter under it, since the honest population of "blocking work in an `async def`" also includes `storage`, `subprocess` and every synchronous `db` call. That axis is filed as its own task rather than half-gated here. Each of the three analyser fixes is mutation-checked: filtering the sentinels inside `_bindings` reds the scope-merge probe by name; narrowing `_bound_names` back to statement types loses all four shadow probes from the violation list; taking module scope back out reds the module-rebinding probe; letting `NOT_A_MODEL` seed again brings back both the phantom pair and the UNKNOWN it manufactured. ### LOCK-BOUNDARY round 11 — the third traversal, and a correction to what round 10 claimed Two High findings, both real, both fail-opens, both in `test_lock_boundary.py`. **1. `lock_spans` crossed nested function boundaries — and round 10's commit message said this could not happen.** That commit asserted *"there are now exactly two function-taking traversals and both use `_own_nodes`."* **That claim was false.** `lock_spans` is a third, and it was missed because the search that produced the claim was `grep 'ast.walk(fn)'` — and `lock_spans` does not call `ast.walk`, it recurses through a hand-written closure. **Searching for a spelling is not searching for the property**, and the property was "descends into a nested scope". The same lesson this PR has been prosecuting for four rounds, applied to my own verification of it and not caught by me. The consequence is round 10's shape exactly: a nested helper's *unlocked* mention of an attribute made the **enclosing** function's locked write report as unlocked; that write is the SEED; losing it means an unlocked writer of the pair elsewhere is no longer a violation. All three function-taking traversals now stop at `FunctionDef`, `AsyncFunctionDef` and `ClassDef`. **2. `pid_lock_names` accepted shadowed names.** `def writer(p, pid_lock):` binds a *parameter*, and Python resolves `with pid_lock.mutating(...)` inside that function to the parameter, not the imported module — as does `pid_lock = other`. The analyser kept `"pid_lock"` in `lock_names` and certified an arbitrary object as the project lock. The `impostor_alias` probe added in round 9 could not reach this: it binds the name in a *different* function. **The dangerous case is the same scope, where the name is right and the binding is wrong.** Names bound by a parameter, assignment, `for` target or `with ... as` in any scope on the chain are now subtracted — removed rather than resolved, because this is a certifier and refusing what it cannot prove is the fail-closed direction. **And the reviewer confirmed, unprompted, the thing this round was asked to check:** *"The stable 432 sites / 6 pairs count does not detect either defect."* It was right to say so. Finding 1 can remove one seed while a second locked writer keeps the pair alive; finding 2 can add a **false** seed without changing the count at all. That number has been cited in five consecutive entries as evidence the tightenings lost nothing — **it is evidence of much less than that**, and it is now retired as an argument. The probes are the evidence; the count is a description. Sites unchanged at **432**, six protected pairs. Each fix mutation-checked: restoring the nested descent drops `(Project, prop_layers)` and loses `span_victim`; restoring the unshadowed names loses `param_shadow` and `local_shadow`. ### LOCK-BOUNDARY round 10 — two defensible rules that were a hole together One finding, and it is the one I asked for: *is there another place where the two traversals still disagree about where a function ends?* Yes — `_bindings` was the last `ast.walk(fn)` in the analyser. Round 9 fixed `_stored_attrs` to use `_own_nodes` (writes belong to their innermost function) and added `_bind` (a name meaning two models resolves to neither). Each is right on its own. **Together they opened a fail-open neither could cause alone**, because `_bindings` still descended into nested bodies: a name rebound inside a *helper* polluted the enclosing function's map, `_bind` marked it CONFLICT, and the enclosing function's **locked** write lost its receiver. That write is what SEEDS the `(model, attribute)` pair. Lose the seed and an unlocked writer of the same pair elsewhere is no longer a violation — because nothing is protecting it. **This is a fail-open that works by REMOVING A SEED rather than by excusing a write**, which is why it survived every self-test aimed at the judgement. All of those ask "is this unlocked write reported?"; none asked "did the thing that makes it reportable survive?" The new probe is built the other way round: a correct locked writer whose nested helper rebinds the same name, plus a genuine unlocked writer of that pair. Reverting `_bindings` to `ast.walk` drops `(Connection, config)` out of the protected set entirely and the unlocked writer silently leaves the violations — exactly the shape described, reproduced. Two lessons, and the second is the one worth carrying: - **Every traversal in this analyser must agree on where a function ends.** Third instance in two rounds: round 9's first attempt attributed a nested write to its enclosing function while the scope maps stopped at the boundary, and this is the same disagreement in the remaining direction. - **A rule can be correct in isolation and unsafe in composition.** `_own_nodes` and `_bind` were each argued for on their own merits and reviewed on their own merits. The hole existed only in the interaction, which is precisely what a diff-shaped review cannot see and why every round of this PR has been requested as a full pass over the whole state. Sites unchanged at **432**, six protected pairs, no new violations — the fix corrects *which scope* a binding belongs to, not how many there are. ### LOCK-BOUNDARY round 9 — three fail-opens in the analyser, and a completeness claim with a filter under it Three findings from the round-9 review, all inside the diff, all in `test_lock_boundary.py`, all real. Each mutation-checked. **1. `_stored_attrs` claimed completeness and then filtered.** Round 8's docstring argued that an attribute write *is* `ast.Attribute` carrying `ctx=Store` — the grammar's own marking, complete by construction — and the very next line required `isinstance(n.value, ast.Name)`. So `ctx.project.source_ifc = v`, whose outer node carries `Store` exactly like any other write, never entered the site list and the gate **accepted** it. *A completeness claim with a filter under it is not a completeness claim* — and this is the third round running where the fix for an over-narrow population was itself over-narrow, now at the level of a single `and` clause. Receivers are no longer filtered here: resolving one is `collect`'s job, and an unresolvable receiver is reported UNKNOWN, which reds. **The narrow version traded a fail-CLOSED report for silence**, which is the worse of the two by the same argument this whole PR rests on. **2. Imports were collected file-wide, not lexically.** `_alias_map` and `pid_lock_names` walked the whole module, so a function-local `from .. import pid_lock` — how most of this tree spells it — became a binding for *every* function in the file. One function's `import pid_lock as lock` would certify an unrelated `lock.mutating(...)` elsewhere, and a reused model alias could resolve another function's receiver to the wrong mapped model. Bindings now resolve along the real scope chain: module level, then each enclosing function, then the function itself, with normal shadowing. **3. One binding per name, applied to every write in the function.** `_bindings` kept the last binding and `collect` applied it everywhere, so in `p = Project(...)` → `p.source_ifc = v` → `p = SavedView(...)` the unlocked `Project.source_ifc` write was reclassified by a later line and left the violations entirely. Resolving per source position needs real flow analysis; refusing to resolve a name that means two different things does not, and errs the only safe way — UNKNOWN, which reds. Every real site in this tree has one consistent meaning, so nothing legitimate is affected. **The first version of fix 2 produced two false positives, and reading the code is what caught it.** It reported `authoring.py::import_families` and `::content_import` as unlocked writers of `Project.source_ifc`. Both are in fact correct — the write sits inside a nested helper that holds `pid_lock.mutating(pid)` and imports it locally, the `run_in_threadpool` shape every `async def` route here uses. The bug was mine: `_stored_attrs` walked *into* nested functions while the new scope maps stopped *at* them, so a write was attributed to the enclosing function while its lock and its import were not. **Two traversals over one tree must agree on where a function ends.** Writes are now attributed to the innermost enclosing function — which `collect` visits in its own right, so nothing is lost — while bindings accumulate down the chain, because a nested helper closes over the receiver its caller fetched. *Had this been reported instead of read, it would have been two invented defects in working concurrency code.* Sites scanned **375 → 432**. Fifty-seven more real attribute writes, no new violations, and the six protected pairs are unchanged before and after — which is the evidence that the scoping tightened nothing legitimate. Three self-tests are planted and each fix is mutation-checked: restoring the `Name` filter loses the chained receiver, going back to file-wide imports makes the impostor alias *vanish from the violations*, and keeping the last binding loses the rebound site. One implementation note worth keeping: the first scope-chain build was quadratic (a parent map per call, three calls per function) and the gate stopped finishing. It is memoised per module now. ### LOCK-BOUNDARY round 8 (review) — a certifier that matches on a method NAME, and an inference wearing the shape of a proof Four findings from the round-8 review, all inside the diff, all real, all fixed. **1. `lock_spans` certified on the method name alone.** It matched any `.mutating(...)`, so `with session.mutating(pid):` — an unrelated object exposing a method of that name — marked every write in its body as locked. *A certifier that matches on a method NAME is trusting a string anybody may define*, and it fails in the one direction a certifier must not: silently, toward "yes". The receiver is now resolved against the file's own `pid_lock` imports (`pid_lock_names`, per file, because most call sites import it function-locally), so a module with no such import certifies nothing. An impostor is planted in the self-test population; loosening back to name-matching reds it. Six protected pairs before and after — the tightening lost no real lock. **2. Form 7 was an inference wearing the shape of a proof.** Round 6 reasoned: "a variable bound from an attribute chain touching no mapped attribute name cannot be a mapped instance, because reaching one by attribute access means traversing a mapped relationship." That holds for chains *through the ORM* and says nothing about a chain through anything else — `p = ctx.row` on a plain container holding a `Project` touches no mapped attribute name and is a mapped instance. **It was a tree-wide fail-open, the same defect as the module predicate it replaced, one layer smaller and better argued.** The repair is the `test_court_primary` move: *a premise that cannot be checked has already drifted; stop inferring it and DECLARE it.* What made that affordable is that the population turned out to be **one site** — measured by deleting the rule and re-deriving, not assumed. `NON_MODEL_RECEIVERS` now carries that one entry with its reason, everything else falls through to UNKNOWN (fail-closed), and the ledger is checked in **both** directions: emptying it must bring every declared site back as an unresolvable unlocked write (so no entry is decoration, and none can rot into an exemption for its next occupant), and nothing undeclared may need one (the direction round 6 failed in). `MAPPED_ATTRS` and `_chain_attrs` went with the inference. Its self-test went too — a check on a variable nothing reads is precisely the thing this PR is about — and the lesson it carried moved to `MODELS`, which *is* load-bearing: an unconfigured registry resolves no receiver and leaves an empty seed. **That repointed check immediately failed on a threshold copied from the old one** (34 models, not the 183 attributes), which is the argument for pointing a check at something live. **3. `test_ifc_publish_atomic` asserted SOURCE TEXT.** One check read `authoring_shared.py` and looked for the literal `"os.link(final, backup)"`. Same defect `test_pin_pgnull` exists to record: *asking the right question and then asserting something adjacent to the answer.* It passes on a spelling and on dead code alike, and a faithful rewrite (`os.link(src=…, dst=…)`, or the call moved into a helper) reds it for nothing. It now wraps `authoring_shared.os.link` and runs a real publication through it; reverting the helper to `os.replace` reds it with the call list empty. **4. A stale `LOCK-BOUNDARY` comment in `generate.py`.** Two paragraphs had stacked: the first still claimed generation runs inside the lock and writes the fixed published path, which round 1 stopped being true. The second, correct one stays. *The same failure as round 6's "`mutating` is reentrant, so this nests safely" — a comment asserting the property the code no longer has, which reads as reasoning rather than as the claim it is.* ### LOCK-BOUNDARY round 8 — the fix for an incomplete enumeration is not a longer enumeration Round 7 replaced `test_lock_boundary`'s `ast.Assign`-only matcher with a three-statement one (`+AnnAssign`, `+AugAssign`) and called the hole closed. It was the same mistake one size larger. **Five further spellings of an attribute write were still invisible, and the gate ACCEPTED each:** ```python p.source_ifc, _rest = a, b # a Tuple target -- the top-level target is not an Attribute p.source_ifc, *_tail = xs (_a, (p.source_ifc, _c)) = v # nested arbitrarily deep for p.source_ifc in rows: ... # `For.target` -- reached by no assignment statement at all with ctx as p.source_ifc: ... # `withitem.optional_vars` -- likewise ``` All legal Python, all writes, none of them a target of the three statements the matcher knew. The repair is not a sixth case. **An attribute write is not a list of statement types — it is `ast.Attribute` carrying `ctx=Store`, which is how Python itself marks one, in every form it has.** `_stored_attrs` now yields exactly those, and admits nothing else: a read (`v = p.x`), a call argument (`f(p.x)`) and `del p.x` carry `Load`, `Load` and `Del`. There is no seventh form to miss, because the set is no longer a list somebody maintains. Sites scanned went **346 → 375** — twenty-nine more real attribute writes in the tree, none of them a new violation. *This is the third round in a row where the previous round's fix was the next round's defect, and the first where the shape repeated at the same layer rather than a deeper one.* Round 6 removed a module-level predicate for excluding what it had not proved; round 7 widened a statement matcher from one spelling to three; round 8 stopped spelling. The lesson the first two did not quite reach: **widening an enumeration leaves an enumeration, and its remaining blind spot is invisible in exactly the same way** — the count looked more complete each time, and 331, 346 and 375 are all confident numbers. Only the last one is derived from something that cannot silently shrink. Seven non-plain spellings are now planted unlocked in the self-test population, and narrowing `_stored_attrs` back to the three statement forms reds two checks, naming the five it stopped seeing. Worth stating because it is the *reason* only this one site needed fixing: the analyser's other two `ast.Assign`-only matchers — Form 7's `NOT_A_MODEL` exclusion and the model-resolving pass — fail **closed**. A form they miss leaves the receiver unresolved, which the gate reports as UNKNOWN and reds. `collect` was the only one where a missed form meant a site was never looked at. ### LOCK-BOUNDARY round 7 — a `finally` that deletes the only copy, and two spellings of a write Three fixed, two declined. The first is the most serious defect this PR has produced, and round 6 made it worse. **1. The rollback backup was deleted even when the restore FAILED.** The `finally` unlinked it unconditionally. If `os.replace(backup, final)` raises, the published path is left holding bytes the system refused **and the backup — the user's previous model — is destroyed.** Round 6's `os.link` sharpens it: backup and `final` begin as one inode, so after the promotion the backup holds the **last** reference to the old bytes. *Cleanup in a `finally` runs on the branch where cleanup is exactly the wrong thing to do* — the one case that still needs the file is the case that got there by failing. The backup is now removed only when publication succeeded **or** restoration succeeded; a surviving `.rollback-*` is evidence that a publication failed and could not be undone, not litter. **2. `test_lock_boundary` matched `ast.Assign` only.** `p.source_ifc: str = v` is an `AnnAssign` and `p.source_ifc += v` an `AugAssign` — both writes, and the augmented one is *literally* a read-modify-write, the shape the gate exists for. Neither ever entered the site list, so the gate **accepted** them. Same defect as round 6's module predicate one layer down: *a population derived by matching one spelling of a thing silently excludes the others, and the exclusion never appears in the output.* Sites scanned went **331 → 346** — fifteen attribute writes the gate had never seen. Both forms are now planted unlocked in the self-test population, and narrowing the matcher back to `ast.Assign` reds two checks. **3. The hard-link continuity claim was unconditional.** It is a property of the hard-link path; on a filesystem without hard links the helper falls back to the rename and the window returns. Qualified. **Declined, both real, both about the proforma module rather than this diff:** a full-budget `PUT` can still overwrite a concurrent sync's hard lines (needs a revision token, i.e. a client contract change); and `source_ifc_path` / `takeoff_file` / `open_model` run before the lock in the model-sync route, so a publication can swap the IFC between the read and the commit. Both predate this PR, both need their own change, and both are tracked. ### LOCK-BOUNDARY round 6 — the gate's own look-at predicate, and a fix that cost an unrelated property Three real findings from a full re-review of the head. Two are defects **inside earlier rounds of this PR**, which is now the pattern rather than the exception. **1. The published path was briefly ABSENT, and that is not the same property as "never partial".** Round 4 took its rollback backup with `os.replace(final, backup)`, so `source.ifc` did not exist between that rename and the promotion. `bake_layers` calls `project_with_source` **before** it takes the lock, and that helper raises 409 *"project has no accessible source IFC"* on a missing file — so a concurrent read could be refused during a publication that then **succeeded**. The backup is now an `os.link` — the same inode under two names, so the path stays present and `os.replace` still swaps it atomically. **That continuity is a property of the hard-link path, not an unconditional guarantee:** on a filesystem without hard links (FAT, some network mounts) the helper falls back to the rename and the window returns, because copying what may be hundreds of MB on every publish is the worse trade. The fallback costs a spurious 409 on a concurrent read, never data. *A fix for one property can cost an unrelated one that nothing was asserting* — the bare rename had given continuity for free since the beginning, and round 4 spent it without noticing. **And the first test written for this was vacuous.** It asserted no `` observation during a normal publish; reverting the fix left it **passing**, because two back-to-back renames take microseconds and the reader polls every millisecond. *A check that cannot observe the interval it describes reports on nothing.* It now tests the primitive with a deliberately wide window — with a rename the reader must catch the gap, with a link there is no gap at any width — plus an assertion that the helper is what uses the link. **2. `test_lock_boundary`'s module filter was the defect its own docstring warned about.** `touches_orm` skipped any module importing neither `models` nor `Session`, justified as *"provably incapable of the defect"*. That is **false**: `def stamp(p): p.source_ifc = v` holds a mapped instance its caller passed in, with neither import. Such a file was skipped whole, so the write never reached `verdicts` and could not be reported UNKNOWN — **the fail-closed rule was defeated one layer above the code enforcing it.** The docstring quotes CLAUDE.md's lesson that *a predicate deciding what to LOOK at is more dangerous than one deciding what to report* two lines before violating it. The module skip is gone. Two per-receiver rules replace it, both derived and provable in both directions: `self` inside a class resolves to that class when it is mapped and to a proven non-model when it is not; and a receiver bound from an attribute chain touching **no** mapped attribute name cannot be a mapped instance, because reaching one by attribute access means traversing a mapped relationship. Deliberately not *"an attribute chain is not a model"* — `project.owner` would be — so the test is against the registry, not the shape. `MAPPED_ATTRS` needs `configure_mappers()`: the first probe returned **0 of 183**, because mappers configure lazily. An empty set would mark every chain "provably not a model" — a fail-open across the tree — so a self-test asserts it is populated before any verdict prints. The removal itself has a regression test with a mutation: reinstating the predicate makes the planted site disappear. **3. Wording corrected in three places.** "`os.replace` undoes itself for free" overstates it: a rename is cheap to undo — one more rename, nothing copied — but nothing undoes it automatically. **Declined, with reasons:** the object-storage arm of a failed commit is the residual already recorded in the helper's docstring and filed as **#558**, blocked on a retention decision that is the maintainer's; and `pid_lock._advisory` degrading to in-process on an acquisition error is a pre-existing, documented property of that module affecting all eleven call sites, not something this PR introduced — it is tracked separately rather than changed inside a PR already six rounds deep. ### LOCK-BOUNDARY round 5 — the rollback had a branch that existed doing nothing Round 4 made publication failure-atomic by moving the previous `source.ifc` aside and putting it back on any exception. Review found the half that does not exist: **when there is no previous file, there is no backup, and the `except` branch did nothing at all** — while `os.replace(staged, final)` had already put the refused bytes at the published path. That is every FIRST publication: blank create, first upload, first generate, first `ensure_model`. *The inverse of a rename is a rename OR a delete, depending on what was displaced, and a rollback written only for the first case is a no-op in the second.* The orphan is the smaller half. The larger one is what it does to the NEXT publication: that one finds `final.exists()` true, adopts the refused bytes as its backup, and on its own failure **restores them** — promoting a model the system refused into "the previous model worth keeping". **Why the round-4 test could not see it.** `test_ifc_publish_atomic`'s rollback arm calls `reset()`, which writes the old model to the published path every time, so `final.exists()` was always true and the no-backup branch was never entered. *A rollback test that always has something to roll back never exercises the branch where there is nothing.* The new arm deletes the published file first, asserts the failure leaves nothing behind, and then asserts the follow-on — a later successful publish has no refused model to inherit. Deleting the fix reds it with `source.ifc survived holding 4144 refused bytes`, so the branch is exercised rather than merely present. ### LOCK-BOUNDARY round 4 — a comment that asserted the safety property the code lacked Two Major review findings, both real. **The publication and the budget seed were two lock intervals, not one.** `publish_source_ifc` took the lock and RELEASED it; a second `with` then re-took it — and the comment beside it said "`mutating` is reentrant, so this nests safely", describing a nesting that did not exist because the publisher had already returned. **A comment can assert the exact safety property the code lacks, and it reads as the reasoning rather than as the claim it is.** The gap lets generations A and B publish in order A, B — so the model is B's — and then A, still to seed, finds `dev_budget` empty and seeds it from A's metrics: model B priced by budget A, reported by nothing. Now one outer interval, with the publisher re-entering it — which is what the comment had claimed all along. **Publication was not failure-atomic.** A rename is CHEAP to undo — one more rename, no data copied — but nothing undoes it automatically, and `put_stream` and `commit` do not. A failure after the rename left the published local path — the file `bake_layers` and the converter open — holding the new bytes while nothing else had been published. *Atomic at each step is not atomic across the sequence.* The previous file is now kept aside and restored on any failure. **What that does NOT cover is stated in the helper rather than implied:** if `put_stream` succeeds and the commit then fails, object storage holds newer bytes than the local file and the row. Storage is the durable copy rather than what readers open, and the next successful publish overwrites it — but it is a real window, and closing it means publishing immutable versioned artifacts and moving the pointer last, which is a design change rather than a guard. `test_ifc_publish_atomic` gains three checks: a midway failure raises rather than reporting success, the previous model is restored byte-for-byte, and no rollback scratch is left. Removing the restore reds the second one (`final is 4144 bytes, expected the old 57`) — the guard is exercised, not merely present. ### LOCK-BOUNDARY round 3 — the gate's own seed was failing open, and it hid three live defects Round 1's gate reported nothing about `Project.dev_budget`, and the reason was in the gate, not the code. `generate._finalize_generated` wrote it entirely inside the lock — but took `p` as an **un-annotated parameter**, so the receiver was UNKNOWN, the pair never entered the protected seed, and the three unlocked writers in `routers/proforma.py` were never compared against anything. **UNKNOWN was fail-CLOSED where it could excuse an unlocked write and fail-OPEN where it could establish that a field is protected at all.** *A predicate that decides what to LOOK at is more dangerous than one that decides what to report* — and a seed is exactly such a predicate, so the gate was not exempt from the rule it enforces. Found by running the analyser and reading its output, not by any test: nothing was red. Closed in both halves, because either alone is useless. A fifth binding form (a parameter's type annotation) put the pair in the seed, which immediately named `proforma.put_dev_budget`, `sync_gmp_to_hard` and `sync_model_to_hard`; all three now hold `pid_lock.mutating(pid)`. **Note which half did the finding: a one-line signature annotation turned a silent gate into one that named three live defects.** The defect itself is the `connections` shape again on money. `put_dev_budget` replaces the whole budget blob from the editor's form; the two `sync_*_to_hard` routes rebuild only the `hard` lines and preserve soft/acquisition/contingency. An editor saving the budget while someone clicks "sync GMP" lost one of the two silently — each caller's 200 was true of its own write. A lock and not a compare-and-swap, for the same reason `connections` chose one: the edits are claims on different line categories of one blob, so serialising lets both survive where a 409 would refuse an edit that conflicts with nothing. `test_rmw_sweep`'s ledger is updated from OPEN to LOCKED for the two sync routes, with the reason recorded: that sweep could only ever see two of the four writers, because the third is a plain replace and the fourth a conditional seed. **A pair is only as closed as its widest member, and that derivation could not see the widest.** Sweep open count 4 → 2; the two remaining are `drawingset.revise_sheet` and `share_scenario`. ### LOCK-BOUNDARY round 2 — the lock covered the pointer and not the bytes Review on #557 caught that the first pass locked the `source_ifc` POINTER and left the file PRODUCTION outside. Every producer wrote the published `source.ifc` directly: `import_rvt` with a bare `write_bytes`, both massing generators and the blank create onto the fixed path, and the upload through `stream_to_path` — which does write `.part` and rename, but **the part name is derived from the destination**, so two concurrent uploads to one project share `source.ifc.part` and interleave into it before either renames. *A staging name that is a function of the destination is not staging, it is a second shared path.* `bake_layers` and `/edit` take the project lock and then open `p.source_ifc`, so a reader could open a file being rewritten underneath it. Locking the assignment alone moved the race rather than closing it — the same correction PR #552 forced on `under_pid_lock`, one layer down: *a lock proves something about the interval it spans, and the interval has to contain the bytes as well as the pointer.* Now every producer writes to a unique staged path and `authoring_shared.publish_source_ifc` promotes it — `os.replace`, storage publish, pointer, commit — inside one critical section. One helper rather than the same critical section at five producers, for the reason `connections._lock_key` is one. `staged_ifc` is a context manager so a producer that fails before promoting cannot leak its staged file; this tree has already run a disk out on orphaned scratch once. `services/api/test_ifc_publish_atomic.py` is the behavioural half, and it exists because `test_lock_boundary` structurally cannot see this: an AST analyser can confirm every `p.source_ifc = ...` sits inside the lock and says nothing about which path the bytes went to — the whole defect passes it. The test polls the published path while a producer runs and asserts the reader only ever sees a complete old or complete new model; the mutation writes straight to the published path and the reader catches partial files, so it measures the staging and not the timing. ### LOCK-BOUNDARY — G-11 was closed yesterday and five writers of the same column were still unlocked `test_rmw_sweep` derives **read-modify-writes**. `p.source_ifc = str(ifc_path)` reads nothing — it installs a new pointer — so it was never in that population, and neither was the conditional-create `if not p.source_ifc: ...`. G-11 was closed on 2026-09-13 by locking every `source_ifc` read-modify-write in `routers/authoring.py`; **five writers of that same column were left open, two of them inside that file.** A pointer swap that reads nothing still orphans the version a concurrent `bake_layers` is deriving from. Now locked: `authoring.upload_source_ifc`, `authoring.import_rvt` (both `async def`, so the lock is taken off the event loop via `run_in_threadpool` — the v0.3.703 SSE failure otherwise), `generate.create_blank_model`, `generate.ensure_model`, `generate._finalize_generated`. `generate.py` had no `pid_lock` import at all. **`ensure_model` was the sharpest.** It is a CHECK-THEN-ACT, not a read-modify-write: it reads `source_ifc`, concludes from its emptiness that no model exists, and installs a blank one. Unlocked, two concurrent calls both read empty, both generate over the same fixed `source.ifc` path, and both answer `created: true`. The lock spans the DECISION — the early returns are inside it — because a lock taken after the branch protects nothing this route does. **The first diagnosis was wrong, and the correction is the point.** It looked like a FILE boundary: `generate.py` contained no lock at all, so the closure seemed to have stopped at the edge of the file being worked in. It had not — two of the five are in the "closed" file. What bounds the coverage is the SHAPE. *A coincidence that corroborates a wrong theory is the expensive kind, because it ends the search.* `services/api/test_lock_boundary.py` derives the rule from the code rather than a ledger: a `(model, attribute)` pair locked in **any** writer must be locked in **every** writer. It resolves each receiver to a mapped model — four binding forms, every one added because a probe reported UNKNOWN rather than because it was anticipated — and fails closed, because a resolver that guesses turns each of those into a confident wrong answer. Receiver resolution is load-bearing, not tidy: `modules.save_view` writes `.config` and matches `Connection.config` by NAME, but it is a `SavedView` and not a violation at all. **It states its own limit rather than hiding it:** this finds INCONSISTENT protection, not ABSENT protection. `Project.dev_budget` has four writers and no lock anywhere, so it never enters the seed and this gate is silent on it — the sweep names two of those four instead. Each derivation is blind where the other sees, which is why both exist. ### Two admins saving one connection: one of the two saves vanished `Connection.config` had **two** writers and neither took a lock. `update_connection` merges the body's keys into the blob — keeping a stored secret where the form sent it blank — and `put_mappings` rewrites the whole blob to replace its `mappings` key. So an admin saving credentials while another saved field mappings lost one of the two, silently: each caller's 200 is true of its own merge. Gap G-12's `connections` pair is closed; the sweep's open count drops six to four. **A lock, not a compare-and-swap — deliberately the opposite call from the register rows.** There the whole row is one logical edit, so a stale write should be REFUSED with a 409. Here the two writers touch near-disjoint keys, credentials against `mappings`, so serialising lets the second merge onto the first's committed blob and **both survive**, where a 409 would refuse an edit that does not actually conflict. *The right control depends on whether the concurrent edits are rival claims on one value or independent claims on different ones.* `connections` carries no token to swap on in any case — only `created_at`. **Two checks, and the measurement showing neither is sufficient alone.** `services/api/test_connection_lock.py` asserts both edits survive and that neutering the lock loses one. Then two mutations were run: | mutation | sweep | behavioural test | |---|---|---| | one route's lock deleted | ❌ red | ❌ red | | the two routes lock on **different keys** | ✅ **green** | ❌ red | *Static analysis can see that a lock is present; only behaviour can see that it is the SAME lock.* `under_pid_lock` asks whether each mention sits inside *a* `pid_lock.mutating(...)`, and under the second mutation both still do — "a lock one side does not take protects nothing" wearing the shape of a lock. Hence `_lock_key` is a shared helper rather than an f-string at each call site. **And the first mutation found a hole in the gate itself.** `test_rmw_sweep`'s "every site this ledger calls locked IS lexically under `pid_lock.mutating`" derived from `IFC_PIPELINE` only — the one ledger that had LOCKED rows when the check was written. The two new LOCKED rows landed in `CROSS_FIELD`, where **nothing asserted them**: the status was prose again, and deleting either lock left the sweep green. The derivation now spans every ledger, taking verified locked sites from 6 to 18. *A gate scoped to the ledger that existed when it was written silently stops covering the ledgers that arrive later* — and only mutating the shipped fix surfaced it. ### Two people authoring at once: one of them lost the whole model version Six routes in `services/api/src/aec_api/routers/authoring.py` — `bake_layers`, `import_families`, `import_family_pack`, `place_family`, `content_import` and the shared `_restore_version` behind undo/redo — read `Project.source_ifc`, produced a NEW IFC version from it, and wrote the pointer back, all outside the per-project lock. **Five sibling routes in the same file do the identical thing inside `pid_lock.mutating(pid)`**, and `edit` carries a comment describing exactly this race. So two concurrent authoring operations each derived from version N, each wrote their own N+1, and the second commit orphaned the first user's work — **silently, because each caller's response is true of its own run**. What is lost is a baked override set, an imported family pack, a placed family, a content import, or a restored version: the model the product exists to author. All six now hold the lock across the whole read → derive → apply → pointer-swap. Gap G-11 is closed and the sweep's open count drops from twelve to six. **The lock is proved by behaviour, not by its presence.** `services/api/test_authoring_lock.py` runs two concurrent bakes against a stubbed recipe and asserts the second reads the FIRST's output; neutering `pid_lock.mutating` makes both read the same base. A test that grepped for the lock would pass on a lock spanning only the assignment — the defect review caught one level up on PR #552. **Three things the uniform fix got wrong**, each corrected before it shipped: - **Two of the six are `async def`.** `pid_lock.mutating` is synchronous and blocks on a Postgres advisory lock, so taking it inline would park the event loop behind somebody else's upload — *a rare lost model version traded for a routine server-wide stall*, which is the v0.3.703 SSE failure arriving by a different door. Both now run the locked section through `run_in_threadpool`. - **`import_family_pack` first had `db.commit()` outside the lock.** An uncommitted pointer swap is invisible to the next lock-holder, which reads the old version and derives from it — the same race moved rather than closed. *A lock proves something about the interval it spans, and that interval has to end after the write is durable.* - **`_restore_version` re-read the column once more after the lock released.** Harmless today, and the sweep refused to certify it anyway — *a certifier that reasons about which stragglers are harmless is one bad reading away from blessing one.* The undo/redo lock sits in the shared helper rather than in `edit_undo` and `edit_redo`: the narrowest place that can carry the fact, so both routes inherit it instead of two call sites having to remember. ### The re-key voided one of the gate's own regression checks, silently Found in review, and it is the same defect the pull request exists to fix, committed by the fix. The gate carried a regression check that the share-token view counter is no longer an ORM read-modify-write, written as `("…client_portal.py", "model_fragment") not in ORM`. That was true while `ORM` held `(path, function)` pairs — and it stayed true, permanently, the instant this change re-keyed `ORM` to `(path, function, attribute)`: **a 2-tuple is not a member of a set of 3-tuples no matter what is in it.** Reinstating the original `row.view_count += 1` would have left those three lines printing PASS. *A membership test written against a key SHAPE fails OPEN the moment the shape changes*, and nothing about the diff looks like it touched that check. The projection to `(path, function)` is now derived by a named function rather than written out at the call site, and the gate plants a synthetic site and requires the projection to find one that raw membership cannot — so the shape bug has to come back through an assertion that is watching for it. The canary's first draft was itself conditioned on the real population, and reinstating the genuine regression made it report *"the projection did not distinguish the planted site"* — blaming the analyser for a defect in its subject. It now runs on a two-element synthetic population and touches the tree not at all. *A check whose failure message can misdiagnose is worse than one that stays silent, because somebody acts on it.* ### The concurrency ledger was keyed by function, and reasoned about one field at a time `services/api/test_rmw_sweep.py` keys its read-modify-write ledgers by `(path, function)`. **Six functions write more than one field**, so a single status and a single sentence covered several distinct races — and the sentence names whichever field the author happened to be looking at. The rest are exempted by reasoning about a different line. That is this file's own subject one level up: *an exemption that reasons about part of its subject and reports on all of it.* And it had already hidden something. `connections.update_connection` was EXEMPT because "`body.name or c.name` is a PATCH keeping an unsent field" — true of `name`, and silent about `merged = dict(c.config or {})` … `c.config = merged`, an accumulating merge that keeps a stored secret where the form sent it blank. `connections` carries no version column at all, only `created_at`. Worse, it is the **second** writer of the blob `connections.put_mappings` writes, which was already listed OPEN. One admin saving connection settings while another saves field mappings loses a write, and whatever control that table eventually gets must cover both — *a lock one side does not take protects nothing*, the lesson the `dev_property` pair paid for a day earlier. The key is now `(path, function, attribute)`. The derived population went **32 → 41** and the open count **11 → 12**; the count going up is the sweep working rather than failing. Two side effects worth naming: `under_pid_lock(*k)` now takes the column straight from the key instead of being told it a second time, and the `dev_property` pair is derived by `k[2] == "dev_property"` rather than by searching for that string **inside the reason prose** — *a derivation over prose, in the file whose own lesson is that a count derived by grepping prose is prose.* The other five multi-field functions were re-read and their exemptions hold for every field, now said per field rather than inherited: `bim._with_kind` sets non-column serialisation fields, `verification.set_status` and the three `cloud` sites take whole values or fallbacks from a provider response. `cloud_refresh_profile`'s `cloud_roles` and `cloud_tier` get their own sentences despite reaching the same verdict, because they are authorisation-bearing and inheriting a reason about a display name is how that stops being examined. A check now asserts that at least one function's fields carry **different** statuses — the thing the old key could not express, so the extra column is load-bearing rather than decorative. Its failure message names both causes, because today `update_connection` is the only such function and closing it legitimately would red the check for the opposite reason: *a check whose failure message can misdiagnose is worse than one that stays silent, because somebody acts on it.* ### Two people pressing "promote" on one markup got two RFIs, and one of them vanished `bim.promote_markup` checked `if m.topic_id: 409` and then claimed the back-link with a plain `m.topic_id = t.id`. The check reads the session's own copy, and `SessionLocal` is `expire_on_commit=False`, so a request that loaded the markup before a concurrent promote committed still sees `None` however long it holds it. Both requests therefore passed the guard, both minted an RFI Topic, and the later write won — leaving **two Topics for one markup, the first orphaned** because nothing points at it any more, with both callers told 201. A double-click is enough; no load is required. It now claims the link with a conditional `UPDATE ... WHERE topic_id IS NULL` and rolls back the flushed Topic on a loss, so the loser leaves nothing behind. That is not a new idea here — it is exactly what `modules.promote_comment` was given when the same defect was fixed there, one module over. **This instance was found by the read-modify-write sweep's derivation, not by anyone reading the route**, which is the argument for deriving a population rather than listing one: *an already-solved defect living on elsewhere is the cheapest kind to close and the easiest to never look for.* Two smaller things the fix exposed. The Core `UPDATE` bypasses the ORM, so the session's `m` kept its null `topic_id` and the response would have reported the markup unpromoted **in the very call that promoted it** — a `db.refresh` before the response fixes that. And closing the site made its own ledger entry stale, which `services/api/test_rmw_sweep.py` reported before the ledger was touched: *closing a gap has to force the record to move, or the tree ends up describing a defect nobody can still find.* Open gaps 12 → 11; G-12 five → four. ### Two people editing one record both saved; only one of the edits survived RMW-SWEEP. `transition` was one instance of a class, and PR #551 fixed only that instance. The shape is: **read a column, compute a new value FROM what you read, write it back under `where(id == rid)` alone.** Nothing holds the row still in between, so two callers both read the old value and the second write erases the first — and *both* callers are told they succeeded. Reproduced against the real functions before anything was written: ``` set_element_guids: A reads ['GUID-A'] · B commits ['GUID-A','GUID-B'] · A commits ['GUID-A','GUID-C'] FINAL ['GUID-A','GUID-C'] GUID-B survived? False link_record: B commits a link · A commits a link · FINAL holds one of the two ``` *A lost update that refuses one caller is nearly fixed; one that tells both they succeeded has to be found twice* — the same severity inversion the seeding sweep recorded, where a primary key refuses the loser and a non-unique index refuses nothing. **Five sat on the register row**, and `update_record` is the one that matters. It is the path the register's inline text and select cells use; it writes `{**what we read, **what you sent}`; and the control `docs/security/threat-model.md` names for exactly this threat — `expected_modified_at` — is **opt-in**, passed by one of five web call sites. So two people editing *different fields* of one record both merged onto the same pre-image and the second erased the first person's field. The documented control was real and it was not on the path people use. All five now route through `modules._cas_row_edit`, which re-reads, recomputes, and writes under `WHERE id = ? AND modified_at = ?` — a compare-and-swap in SQL rather than a check in Python. **Three decisions in it are worth naming, because each has a wrong-looking alternative.** - **The swap is on `modified_at`, not on the collection.** `element_guids` is a JSON column and JSON equality is not comparable the same way on SQLite and Postgres — the exact trap `with_for_update()` is, which Postgres honours and SQLite silently ignores. `modified_at` is a plain timestamp on both. And **every register-row writer sets it**, which is now asserted rather than assumed: the dynamic register table declares the column with **no `onupdate=`**, so that coverage is a property of the call sites, and one writer forgetting it would make this predicate match a row somebody else had just changed — *a lock that looks present.* - **It retries rather than refusing.** A lost `transition` race means the action genuinely is no longer available, so a 409 is honest. "Add GUID-C" is still the caller's intent after somebody else adds GUID-B, so re-reading and re-applying gives the user what they asked for. - **…except where the caller does not own the transaction.** A retry rolls back, and `update_record(commit=False)` has callers (`responsibility`, renaming a role column across every row) that have already staged rows in that transaction. Rolling those back and then succeeding here would commit a partial edit and report it as whole — *the very shape of bug this exists to remove.* So `commit=False` gets one attempt and a 409, which aborts the caller's whole edit. **`revise` was hiding a second defect behind the first.** Its "record already revised" check was itself a read-then-write: two concurrent revises both passed it, both minted a revision row, and the second supersede write erased the first's `superseded_by` — leaving two live revisions of one record, each believing it is the only one, and only one findable from the source. The CAS refuses the second. **Two off-register instances were found, and one of them was not a race at all.** `client_portal`'s share-token view counter was `row.view_count = (row.view_count or 0) + 1` — two people opening the same link both read 7 and both wrote 8, so the count drifts *down* under exactly the load that makes it interesting. It moved into SQL (`view_count + 1` evaluated by the database), with no CAS and no retry, because there is then nothing read in this process for anyone to invalidate. The other was the opposite of what the sweep was looking for: `proforma.save_property` wrote `p.dev_property = body` **wholesale**, and `realestate.save_appraisal` keeps `appraisal` inside that same blob — so every ordinary re-save of the property tab deleted the saved appraisal overrides. No concurrency required. *A write that reads nothing cannot lose an update — it just deletes one.* *That sentence is about the code as it WAS, and review pointed out it stops being true of the fix: carrying `appraisal` across means reading `dev_property` first, so the repaired route can now lose a concurrent appraisal save — a smaller fault than the one it removes, on a table with no concurrency token, and it is filed as open rather than implied away.* The same round found the response still answering with `body` while the merged dict was what got stored — persisting one value and returning another, which is the kind of mismatch that gets discovered from two screens away. **What the sweep could NOT reach is named rather than left silent**, because that is precisely how the seeding sweep lost four sites. `Scenario.shared_with` (a share grant) and `Project.dev_property` (the appraisal merge) still lose a concurrent write: neither table carries a `modified_at`, so the register row's token does not exist for them. They are frozen in `test_rmw_sweep.py`'s `BAND_2` ledger and filed as gap **G-10** — the set cannot grow silently, because a new instance reds the build. **The gate**, `services/api/test_rmw_sweep.py`, derives the population by AST with transitive taint from `get_record`, fails **closed** on an `update()` whose target it cannot resolve, and runs itself against the shipped pre-fix bodies of `set_element_guids` *and* `update_record` before it may print any verdict — the second because `update_record` splats `**vals`, which is the blind spot that cost PR #551 a review round. The behavioural half asserts the **outcome** through the real functions on a hand-built interleaving, and a mutation removing the swap predicate must lose the GUID again. **The review round found three more things, and two of them were holes in this gate rather than in the code it guards.** The swap-predicate test asked only whether the `WHERE` named a non-identity column — so `WHERE deleted_at IS NULL` read as a compare-and-swap though it compares against a constant and swaps on nothing: *a fail-OPEN verdict inside a gate whose whole design is failing closed.* Tightening it then flagged two correct writers, which taught the distinction the first two drafts had collapsed: a constant predicate on the column being **written** (`promote_comment`'s `topic_id IS NULL`) is a real guard, and one on a column the statement does not write is a filter. And the token-coverage check searched the function's text for `"modified_at"` — which a **docstring** satisfies, and every function here has a long one. It now reads the assignment with docstrings stripped and demands `_next_stamp`; strengthening it immediately found `transition`, whose stamp sits in a dict literal the detector could not see. *Three shapes write that column and the first draft of the detector knew two.* **The third was a real hole in the lock, not the token.** `_now()` can equal the row's existing `modified_at` — same microsecond, or an NTP step backwards — and then the winning write leaves its own predicate reusable by the writer it just excluded. Every register-row writer now stamps through `modules._next_stamp`, which is strictly monotonic per row. *Rare is not impossible, and a concurrency control that is correct except under contention is wrong exactly where it is load-bearing.* **And widening the gate's ORM derivation caught this pull request hiding a site from itself.** The `save_property` fix hoisted its read into a local (`prior = p.dev_property or {}`), and the analyser only looked for the object named directly on the right-hand side — so the commit that created an RMW site removed it from the count in the same edit, and nothing said anything. *Hoisting a read into a variable is the cheapest way to make a pattern-matching check stop matching, and it is what a tidy-up commit looks like.* With taint propagated one hop the population went **12 → 33**, and the 21 new sites are read and classified rather than counted. **One of them is the biggest thing this round turned up, and no reviewer asked for it.** Twelve routes read `Project.source_ifc`, derive a new IFC version from it, and write the pointer back. **Six wrap that in `pid_lock.mutating(pid)` and six do not** — and one of the six that does carries the comment *"same RMW race as /edit — serialize per project"*, so the control exists, is named in the threat model, and covers half its population. Two concurrent `place_family` calls lose one placement with both callers answered 200. It is **filed, not fixed** (gap G-11, roadmap RMW-LOCKGAP): wrapping six routes that do long IFC I/O is a different blast radius from the register row, in a lane this change did not otherwise touch. The set is frozen instead — the gate asserts the split in *both* directions, so a new unlocked writer reds the build and closing one of the six forces the ledger to be updated rather than leaving a recorded gap that no longer exists. **A second review round found four more, and three were the gate reporting safety it had not established.** Each is the same failure wearing a different mask, and each was introduced by the fix for the one before it: - **The conditional-write exemption protected the wrong column.** The first tightening made a constant predicate count as a swap when the statement *writes* that column — correct for `promote_comment`. But `.where(status == "open").values(status=…, element_guids=)` also satisfied it, and a concurrent `element_guids` change does not move `status`, so the stale write still lands. The guard now has to cover **every** column carrying a read-derived value, and a splatted `**vals` — which hides the column-to-expression mapping — makes coverage unprovable rather than assumed. *Fixing an over-broad rule by narrowing one clause can leave the same hole one column to the left.* - **The stamp check asked "somewhere", not "every".** It returned on the first valid `modified_at=_next_stamp(...)` anywhere in the function, so a writer with two updates passed while the second omitted the stamp. Now validated per update — and doing so immediately failed `_cas_row_edit`, whose stamp is a *subscript assignment* into a dict built earlier. That is the third shape, and **every draft of this detector has known one fewer than exist**. - **The lock check measured the write, not the interval.** `under_pid_lock` asked only whether the `source_ifc` assignment sat inside `pid_lock.mutating`. A route can read the pointer, spend a minute deriving a new IFC from it, and *then* enter the lock to assign — passing a check that looks at the write alone while two callers derive from the same old version. Every mention of the column, read or write, must now be inside. *A lock proves nothing about the statement it contains; it proves something about the interval it spans.* All six certified routes survive the stricter test, which is the answer that makes the certification worth having. - **And the open-gap count was computed by grepping its own prose.** `_open` matched the substring `"OPEN"`; one entry had been written `"Open"`; the gate reported **13** open sites where there are **14**, and the threat model inherited the wrong number. Status is now a structured field with a declared value set, asserted, so a typo reds the build instead of deleting a gap from the tally. The six non-IFC sites that tally had been under-reporting are now their own named gap (**G-12**), `bim.promote_markup` among them — which is `promote_comment`'s pre-fix shape, a live instance of a defect already solved one module over. **And the monotonic-stamp branch was itself untested until it was asked to be.** `_next_stamp`'s `prev + 1µs` path runs only when the clock fails to advance, so no ordinary test reaches it — *a branch that only runs under contention is a branch only production runs.* It is now exercised against the real table with the clock frozen, which also settles a question the fix quietly raised: that path returns a **tz-aware** datetime while SQLite hands `modified_at` back **naive**, and if the two rendered differently the next `WHERE modified_at = ` would match nothing and every edit would 409. **A third review round found two more, and both were the gate accepting a SHAPE where it meant to check a FACT.** Neither is a live defect in this tree — the population and every count are unchanged — which is precisely what makes them worth recording: each one would have gone quiet on the *next* ordinary edit, with every number still printing. - **Naming the guard counted as calling it.** The stamp check asked whether the name `_next_stamp` occurred anywhere in the `modified_at` expression, so `_next_stamp(None)` satisfied it in all three shapes. That argument is not a near-miss: `None` skips the `now <= prev` comparison inside the helper and it returns a bare `_now()` — the exact unguarded clock the helper exists to replace. Under a frozen clock the writer re-emits the token it just read, the loser's CAS predicate still matches, and the swap protects nothing while the gate reports it protected. The argument is now traced back to the row's prior `modified_at`, through a local if it was hoisted into one, and all three shapes route through one function so a fourth inherits the rule. *Naming the guard is not calling it, and calling it is not feeding it.* - **Both taint analyses walked `ast.Assign` and not `ast.AnnAssign`.** An annotated `rec: dict = get_record(...)` breaks the Core chain and the write behind it classifies as PLAIN; an annotated `prior: dict = p.dev_property` drops the ORM site out of the population *entirely*, taking its ledger entry with it — at which point the staleness check would have blamed the ledger entry and pointed the next reader at the wrong file. **The blind spot is one `mypy --strict` pass wide.** Adding annotations changes no behaviour, is the most ordinary edit in the language, and would have silently emptied both derivations. *A gate whose reach depends on the syntax somebody happened to use is measuring the author, not the code.* The ORM analyser was also split out per-file so a **fixture** can be run through it. It could previously only be fed the repository — and the shape review found missing does not occur here, so no amount of reading the source would have produced a failing case. *An analyser that can only be fed the tree cannot be given the input it gets wrong.* **A fourth review round found three more, and the first two are the third round's own fixes failing one level down.** The pattern is worth naming on its own: *a fix can recreate, at the next level of detail, exactly the defect it just closed — and it reads as the same code both times.* - **Data dependence is not identity.** Tracing the `_next_stamp` argument accepted a `"modified_at"` read off *any* object, and carried names through *any* expression mentioning an accepted one — so `_next_stamp(metadata.get("modified_at"))` and `_next_stamp(stamp + timedelta(days=1))` both passed. Neither is the value the CAS predicate is compared against, so a writer feeding either would swap on a token no row ever held and be certified correct on the way past. Row *aliases* are now tracked separately (`rec = get_record(...)`, `other = rec`), the read must be off one of them, and prior-token names propagate through a bare name only. *An alias is an identity claim; provenance is a different question, and this file has now been wrong in both directions by conflating them.* - **"Yes, somewhere" came back for the dicts.** Round two fixed it for updates — each is validated against its own `.values(...)`. That fix then pooled every `modified_at` assignment in the function into one list, so one correctly-stamped dict authorised a second `.values(**other)` whose dict had no stamp at all. Verdicts are now keyed by dict name, and each splat resolves its own. - **And the `dev_property` merge was the class this sweep is about, created by this sweep's own fix.** Preserving `appraisal` turned a wholesale write into a read-modify-write, with `realestate.save_appraisal` merging into the same blob from the other side. Both now take `pid_lock.mutating(pid)` — a lock rather than a swap, because that table has no token to swap on — and **both** is the load-bearing word: *a lock one side does not take protects nothing*, so locking only the route review named would have left the pair exactly as racy. `db.refresh` matters as much as the lock, since `db.get` can answer from the session identity map and the waiter would otherwise merge into the pre-image it read before the winner committed. Open gaps 14 → 12. *The first draft of that check passed under its own mutation.* It asserted that a frozen-clock edit LANDS and that the next edit matches — both still true with the branch deleted, because SQLite serialises writers and each session sees its own write. It is now the losing interleaving run with the clock frozen, and removing the branch reproduces the loss exactly: `['GUID-A', 'GUID-C']`, GUID-B gone. **Two checks that pass under the mutation are two checks measuring the round-trip, not the guard.** *Its own self-tests earned their keep within the hour.* Teaching the analyser about mapped model classes — so two correctly-guarded writes stopped reporting as UNKNOWN — made it read the `t.c` accessor as a column name, so every write looked swapped-on and both pre-fix bodies came back clean. The population check would have printed "0 blind" over a tree it had not examined. *A widening and a rule change are different edits, and widening one can break the other.* And the sibling gate did the same for its neighbour: routing `revise` through the helper removed `update(` from its body, so `test_transition_cas`'s exemption for it went stale and redded the build the hour the helper landed. *An exemption that outlives its site is a hole nobody opened on purpose.* ### A second "send RFQ" minted a second solicitation, and a test called that idempotent `POST /projects/{pid}/procurement/packages/{rid}/send-rfq` called `create_record` — which commits — and only THEN looked at `workflow_state`, under an `if … == "draft"` that took the transition and an `else` that skipped it and reported the unchanged state as a success. So a second send persisted **another** Bid Solicitation while making no move. Measured on the old code: three sends produced **ITB-001, ITB-002 and ITB-003**, every response 200 / `rfq_sent`. The same three sends now produce **one ITB and two 409s**. *(That sentence read "one ITB and a 409" — singular — against a three-send example, which is the kind of count a reader checks against the numbers directly above it. Caught in review.)* **The roadmap filed this as needing a contract decision** between a 409, an idempotent no-op, and a deliberate re-solicitation with its own round number. **It was already decided, in `services/api/modules/procurement_package/module.json`.** That workflow is linear — draft → rfq_sent → quotes_in → awarded — with no path back to draft and no re-solicit action, so `modules.transition` answers a second `send_rfq` with a 409 of its own accord; the probe prints `action 'send_rfq' not allowed from state 'rfq_sent'`. Deliberate re-solicitation would need a new declared transition, which is a product change and not this fix. *The only thing that made the refusal invisible was a branch written to route around it* — so there was no decision to take, only a declaration to stop overriding. The route now moves first and mints only if the move landed. **The existing test asserted the bug as the contract, and could not have noticed.** It read *"a second send on the already-sent package doesn't error and reports the current state"*, asserted a 200 and an unchanged `package_state`, and its summary line called the route *"idempotent on re-send"*. It asserted the two things that stay the same and **never counted the solicitations** — the one thing that changed. *A word like "idempotent" in a summary is a claim, and this one had a test shaped around it.* It now asserts the 409 **and** that the refused send minted nothing. ### And reversing the order installed a worse bug than the one it fixed Raised in review on PR #551, before this shipped anywhere. The fix above moved the package FIRST and minted second — correct about the duplicate, and **wrong about a failure in between**. `transition` commits, so a solicitation refused after the move left the package in `rfq_sent` with no ITB — and this workflow declares **no path back to `draft`**, so the package was stranded where a retry earns only another 409. The old bug was recoverable by retrying (with a duplicate); the new one was not recoverable at all. **It needed no bug to reach.** `create_record` consults `fin_gov.locked_reason`, which raises 409 on a locked accounting period — an ordinary business rule, on an ordinary day. The fix is to STAGE rather than reorder: `create_record(..., commit=False)` writes the solicitation into the session without committing, and `transition`'s own commit then carries both writes as one transaction. Every refusal now leaves nothing behind — a refused mint never reaches the move, and a refused move (sequential 409 or a lost CAS race) discards the staged ITB with the transaction. Nothing in `transition` had to change; `create_record` already took `commit=False`. *A fix that reverses an order can install a worse fault than the one it removes, because the second write's failure is the one with nothing left to roll back.* The test drives the real `fin_gov.locked_reason` path and asserts the package is still `draft`; reinstating move-first fails it with **"refused mint left the package moved"**, so the ordering cannot quietly come back. ### `transition` was a read-then-write race — in all 139 modules at once Found while fixing the above, and the more severe half. `modules.transition` read the record, proved the move legal **from the state it read**, and then wrote with `update(t).where(t.c.id == rid)` — keyed on the id ALONE, with no state predicate and no row lock. Two callers could both read `draft`, both pass the check, and both commit. Two concurrent `award` calls both succeeded. The reach is what makes it worth a release note rather than a line: `transition` is the **only** place a record's `workflow_state` moves after creation (`revise` inserts a new row; every other mention of the column reads it), so that one statement backed **139 modules and 342 declared transitions**. It is now a compare-and-swap — the decided-from state is a predicate, and `rowcount != 1` is a 409 naming the race, deliberately worded differently from the ordinary "not allowed from state" refusal so the two are distinguishable by something other than a status code. Not `with_for_update()`: Postgres honours a row lock and **SQLite treats it as a no-op**, so it would read as protected on the backend the suite runs on while protecting nothing — the identical trap `next_counter` had to be dug out of in August. `services/api/test_race_conditions.py` gated the job claim and the ref counter; transitions were never in its population. **The gate is `services/api/test_transition_cas.py`**, and two of its twelve checks exist because of mistakes made writing it. It reinstates the **pre-fix statement shape** against an already-moved row and requires the blind write to land, so it cannot pass with the guard removed. And its derivation of "who writes this column" is taken at FUNCTION scope rather than from the text inside `.values(...)`: the first attempt read only the call's own source, and because `transition` builds a dict and splats it as `**vals`, **it found one writer and missed the one under repair** — reporting a tree that looked clean. The gate now asserts it reaches `transition` before it may print any verdict. *A predicate that decides what to LOOK at is more dangerous than one that decides what to report.* `apps/web/src/portal/panels/buyoutKeep.ts`'s `rfqSummary` was written against the old rule and told the user *"a solicitation has been minted anyway, so check whether this package already had one"*. After the fix that is the one thing such a response has failed to establish, and the duplicates it sends them hunting for can no longer be created. The branch is kept — a response that is not `rfq_sent` now contradicts the contract — but it says that instead. *A defensive branch that outlives its cause keeps giving the advice that fitted the bug.* ### The 3D pin overlay made two calls, and the route meant to replace them had no caller `apps/web/src/pins/pins.ts` fetched BCF topics through `api.pins()` and register records through `api.modulePins()`, building a marker from each of two row shapes. It now makes **one** request to `/pins/all` and runs one loop; the only remaining branch is the CLICK, which is genuinely different — a topic restores a saved viewpoint, a record opens its register row. **The roadmap entry understated this by one whole half.** It said the remaining gap was *"what each row CARRIES"*. That was true and incomplete: **`/pins/all` had ZERO frontend consumers** — no client method, no call site anywhere in `apps/web`. The route was built, server-tested and dark, so the overlay made two calls not only because the envelope lacked fields but because nothing had ever been wired to the route that replaces them. *An item scoped from what a file lacks will miss what nothing does.* **CORRECTION, made before this shipped rather than left standing.** The paragraph above originally ended *"Route-to-client reachability is a different question, and no gate here asks it."* **That is false.** `services/api/test_route_reachability.py` asks exactly that question, and it is the gate that caught this change. `services/api/test_reachable.py` — the one I checked — measures whether a MODULE is reachable, and `pins.py` is imported by two routers, so it passes honestly; I concluded from one gate's silence that no gate was listening. **And the real finding is worse than the one I claimed.** The route-reachability gate did not miss `/pins/all` — it VOUCHED for it. Measured both ways: on `origin/main` its uncalled set has 56 entries and `/pins/all` is not among them, while a grep of `apps/web` finds no reference to it at all. Its leaf is the single common word `all`, and `leaf_is_called` is the rule that file's own comments already describe as a standing cost (*"a shared leaf decides nothing"*; *"an English word containing a route leaf, which no naming convention prevents"*). *A false negative in a reachability gate is worse than no gate, because the question looks asked.* I did not isolate which string vouched for it, and say so rather than guess. PIN-ONE-CALL corrects the gate in both directions at once: `/pins/all` becomes genuinely called, and `/module-pins` becomes genuinely uncalled — which the gate immediately reported as a new unreachable route. It is recorded in `KNOWN_UNCALLED` with a reason and an expiry condition (delete the entry and the route together at the next public-API revision), not silently exempted. **The design question the entry left open is settled by measurement.** It asked whether the envelope should carry the viewer's presentation fields or whether the viewer should look them up from `/modules`. The viewer does not fetch `/modules` anywhere, so the lookup trades one request for another and defeats the item. The envelope carries `icon` and `source_name` for BOTH kinds: a register's `source_name` and `icon` from its own `module.json` — which is where the old `/module-pins` row already read them — and a topic's from a glyph table lifted out of `pins.ts`, where it was a literal the server could not see, so no server could name a topic pin. *That last clause replaces a claim that was simply false, and it is worth leaving the correction visible.* This read "which is precisely why the overlay needed a branch per topic type and a second loop for register records". Neither half survives looking: the glyph was a **map lookup**, not a branch per type, and the register loop **already took `icon` off the server row**, so the literal cannot have been what forced it. What forced the second loop was the two ROW SHAPES — `Topic` (`anchor`/`title`/`type`) against `ModulePin` (`anchor`/`ref`/`module_name`/`icon`) — which one envelope collapses. The click branch stays, because restoring a viewpoint and opening a register row are different acts. *A causal sentence is the easiest kind to write without checking, because it reads as an explanation whether or not anything caused anything.* Found in review, not by me. `api.modulePins()` is deleted as the dead client method it became; the `/module-pins` route stays. **The gate already existed and was asserting the wrong number.** `apps/web/src/kernel/markupPlugin.test.ts` pinned `["load:proj-1", "modulePins:proj-1"]` — the two-call behaviour, recorded as a fact. It now pins one entry and a length, so a second request fails a build. Mutation-verified: reinstating a second `pins.load` reports `expected [ 'load:proj-1', 'load:proj-1' ] to deeply equal [ 'load:proj-1' ]`. One test changed meaning rather than being deleted. *"A failure part-way through is still contained"* asserted that a failure in the SECOND call left the first one's pins drawn — "it got that far". That half-loaded overlay is unreachable now, so the test asserts its **absence**; deleting it would have removed the record that the state was ever possible. *And `test_file_sizes` caught `app.ts` at 2,445 against its 2,442 cap — three lines of comment, the same shape as the `register.ts` growth one PR earlier. The explanation belongs on the `ResolvedPin` type, not inside a file under a shrink ratchet. Learning a lesson and applying it are apparently still two events.* ### Ball-in-court was computed twice, by two rules, and 10 states disagreed The register screen and every server-side report answered "whose move is it" differently. `modules_query.court_party` took the primary outgoing transition; `register.ts` unioned the parties of **every** outgoing transition. Derived over all 139 modules and 267 workflow states, **10 states in 9 modules** produced different answers. On an open RFI the screen showed Consultant, OwnersRep *and* GC, while `changeorders.py`, `closeout.py`, `quality.py` and every `ball_in_court` rollup said Consultant/OwnersRep — the GC appearing only because it can `void`. *An escape hatch is not a move somebody owes.* The server's reading survives, because it is what the money reports are built on. But it rested on a premise nothing could check: *"module authors list the primary forward action first."* **That premise is false in two places, and they are the ones that matter:** | | listed first | actually owed | |---|---|---| | `pull_plan_task.made_ready` | `reconstrain` (back to `pulled`) | `commit`, by the Subcontractor | | `permit.applied` | `issue` | `start_review` — nothing is issued before review | **And it cannot be rescued by deriving "forward" from the declared state order**, which was the obvious fix and is measurably wrong: `action_item.states` is `['done', 'open']` — alphabetical, not a progression. Under that reading 45 states have no forward transition at all and 39 have several. *A convention nothing can check is a convention that has already drifted.* So the primary is **declared**: `"primary": true` on one transition, or the state is listed as `resting` — reachable, but owing nobody anything. **Only 17 of 267 states need to declare anything**, because 199 have a single outgoing transition and 51 have several that all carry the same parties; for 250 of them the choice cannot change the answer. That bound is the design — a rule demanding 267 edits would have been abandoned halfway and left the tree half-declared. Five answers change. Two are the corrections above. Three states become **resting** — `decision.decided`, `information_container.published`, `spec_section.issued` — and stop naming a party at all, which is what had been putting closed records into ball-in-court rollups as though someone were sitting on them. Transitioning into a resting state leaves `party_owner` as it was, exactly as a terminal state already does. The register now **reads** the value instead of deriving it, and deliberately has no local fallback: recomputing would be the second implementation again. Both the list and the single-record route send it, resolved through `workflow_config.effective` — the project's overridden workflow, which is what `transition` uses, so the screen and the engine cannot disagree by that door either. Enriched at the two **routes** rather than in `modules_query.list_records`, which has **274 call sites** including a BCF export built from the row's keys: *the narrowest place that can carry a fact is where it belongs.* Gated by `services/api/test_court_primary.py`, which derives the ambiguous states rather than listing them, strips a declaration and requires that to be caught, and mutates `court_party` back to the first-transition rule and requires **both** wrong answers to return — so the assertions test the reader and not merely the config. Two gates went red on that change and both were right, so both are recorded rather than quietly re-pinned: **`test_json_null_filter` exempted its sites by LINE NUMBER, and went stale twice in two days.** Not because the code it describes changed — because unrelated edits further up `modules_query.py` pushed those sites down the file. *A line number is a coordinate every edit to the file moves, so an exemption keyed on one measures when the file was last touched rather than whether the site is still there.* Re-pinning it a second time would have been a fix already known to fail, so the key is now `(file, enclosing function, subject)`, which moves only when the code genuinely moves. Collapsing the line into the function costs the ability to tell two sites in one function apart — `_apply_filters` has two — so each entry now carries **how many sites it covers**, and the gate deletes one of that pair and requires the count to catch it. Otherwise this would have traded a noisy failure for a silent one: an exemption still covering the survivor after its twin was deleted. Three mutations pin it: claiming a wrong count, restoring the line-number key, and dropping an entry outright are each reported. **`test_file_sizes` caught `register.ts` growing past its ratchet — by six lines of COMMENT.** The code shrank five lines (the local ball-in-court union went away); a seventeen-line doc comment explaining why replaced it. The ratchet was right: the substance of that comment is about the SERVER's rule and belongs with `court_party` and its gate, not restated in a 2,458-line file that is under a shrink ratchet precisely because it accumulates. Trimmed to six lines and the now-dead `ModuleDef` parameter dropped from `ballCell`; **2,458 → 2,447**. *Both were found by running the suite rather than by reading the diff, and CI's API gate failed on the same two — which is the argument for running it before claiming a PR is green.* **A review round on that fix found three more, and two of them were the same defect I had just fixed one file over.** `test_court_primary`'s list lookup unwrapped `{"items": ...}` and then iterated whatever was left — a 4xx body is a dict with no `items`, so the unwrap hands the ERROR OBJECT back, iteration yields its string keys, and `x.get` raises before `check` can report the route failure. And the new self-tests in `test_json_null_filter` opened with a bare `next(...)`, which raises `StopIteration` and kills the gate if the site they are built on is ever deleted. *Both are "dies instead of reporting", written hours after fixing that exact shape — knowing a defect class by name is not the same as recognising it in your own next paragraph.* Both now report through `check` and reach the summary; each is mutation-proved by breaking its precondition. The third was in the rekey itself. `_sites()` mapped each node to its enclosing function with `ast.walk` + `setdefault`, which binds to the **outermost** enclosing function, not the nearest — so a NULL test inside a nested `def` resolves against the wrong locals. That was already true when the owner map only fed `_resolve`; making the function name part of the site's **identity** is what turned it into a misattributed key no exemption could match. Replaced with a scope-stack descent. **Measured on today's tree the two walkers agree on every one of the 21 sites** — which is precisely why a synthetic nested case is asserted: *a fix nothing can distinguish from the bug is a fix nobody can keep.* **And the first draft of THAT self-test tested a copy of the fix.** It mirrored the traversal into a local `_probe` instead of calling `_owners`, so `_owners` could regress to outer-function attribution and the check would still pass — *a self-test written to stop a fix being silently reverted, unable to observe the fix.* Worse, the mutation run to verify it was aimed at the copy, so it proved the copy behaved as advertised and said nothing about the gate. **Mutate the thing under test, and prove the test can reach it.** It now calls `_owners` directly, and regressing `_owners` itself is what reds the build. The fixture was also shadowing an unrelated `_NESTED` used by the resolver self-tests, harmless only because those happen to run first; renamed. ### The PostgreSQL pin gate could be handed a dead server and still exit 0 Follow-up to the pin-sweep work, and a finding that survived two review rounds because it sat in the review **body** rather than an inline thread — every inline finding was fixed, and this one was not seen until after the PR merged. *Where a finding is posted decides whether it gets read.* `test_pin_pgnull.py` connects to the PostgreSQL a run asks for, and failed the build on an unreachable one only `if _REQUIRED` — that is, only when `AEC_PG_REQUIRED` was set. But `_url()` returns a URL under **two** opt-ins: that one, and `AEC_TEST_PG_URL`. So a run that set only `AEC_TEST_PG_URL`, pointing at a dead port, took the connect-failure branch, recorded nothing, fell through to the no-server path and exited **0** — printing *"no opt-in"* at a caller who had just opted in. Measured before the fix, not reasoned about: AEC_TEST_PG_URL=postgresql+psycopg://postgres@127.0.0.1:1/nope (AEC_PG_REQUIRED unset) -> test_pin_pgnull OK (no opt-in; ...) exit 0 This is the same shape as the defect the file's own comment already records one flag earlier — *"a flag that guards one branch of two guards nothing"* — and it recurred because the fix at the time asked *is this run required to reach a server*, when the question the branch needed was *did anything ask for this server at all*. By the time that `except` runs, something always has: the failure is now unconditional. **A message that names the wrong cause is worse than a bare failure, because somebody acts on it** — the reader would have gone hunting for the opt-in they had supplied. Verified in four directions: the dead explicit URL now exits 1; `AEC_PG_REQUIRED` with a dead `PGHOST` still exits 1; no opt-in at all still passes through the genuine no-server path; and the live-PostgreSQL run still passes. CI was never exposed — it sets `AEC_PG_REQUIRED` — which is exactly why nothing went red. ### Sorting a register by `updated_at` was a 500; `SYSTEM_COLUMNS` named two columns that do not exist `modules_query.SYSTEM_COLUMNS` is the allowlist of row columns a caller may filter and sort a register by. Two of its six names were not columns at all: **`updated_at`** — the register column is `modified_at` — and **`ball_in_court`**, which is derived from `workflow_state` by `court_party` and never stored. `_resolve_field` returned `{"_system": True}` for both, so `_field_expr` and `_display_expr` went straight to `t.c[name]` and raised `KeyError` inside the request handler. Measured over HTTP against the pre-fix code, on a live register: | request | before | after | |---|---|---| | `?sort=not_a_field` | 400 | 400 | | `?sort=workflow_state` | 200 | 200 | | `?sort=updated_at` | **500** | **400** | | `?sort=ball_in_court` | **500** | **400** | | `?sort=modified_at` | 400 | **200** | | save a view with `{"sort": "updated_at"}` | **201, accepted** | **400, refused** | That last row is the one worth reading twice. `validate_view_config` resolves `sort`, `group_by`, `agg_field`, `columns` and `filters` through the same `_resolve_field`, with no table in scope at all — so the phantom could be **stored**. The view saved, the view list returned 200, and the 500 arrived only when somebody applied it. *The stored form is not a louder version of the transient one; it is a quieter one, and its cause sits two screens from its symptom.* The fix is both halves, because either alone passes while the other is broken. `_system_field` validates the name against the register table before the system stub may be returned, so the next phantom is the 400 an unknown field was always supposed to get; and the list is corrected — `updated_at` renamed to the column it meant (checked against all 139 modules: none declares a field by that name, so it shadows nothing), `ball_in_court` dropped. **`?sort=modified_at` now works**, which is the capability the wrong name was reaching for. `updated_at` is the **third** instance of that typo on a module row. `quality.py` and `rfi.register` both read it and got a permanent `None` for `avg_days_to_close`, fixed in August. Those two failed silently and this one is reachable from a URL — *the same wrong name is not the same severity in two places, and the loud one was found last.* Gated by `services/api/test_system_columns.py`, which asserts the allowlist against the live `TABLES` metadata (139 tables), requires its own derivation to re-find both shipped phantoms before it may report anything, and then **deletes the runtime guard and requires the `KeyError` to come back** — a static check that passes with the guard gone is not testing the guard. ### The pin sweep skipped exactly the rows it was written to convert, on PostgreSQL only `b3c9e42d18a5` — the migration that clears empty pin values so `/pins/all` stops counting them as candidates — selects every row where either pin column is non-NULL and then decides per column in Python, beginning `if v is None: continue`. A `json` column holding the JSON scalar `null` is **not SQL NULL**, and psycopg decodes it, so the driver hands back Python `None`: by the time that line runs, a row that was never pinned and a row holding `null` are indistinguishable. It walked past every one of them. Those are precisely the legacy rows the sweep exists for. Every `None` written to `anchor` or `element_guids` before that same change added `JSON(none_as_null=True)` went in as the scalar `null`; they satisfy the SQL half of the pin predicate, fail the exact test in `pins.pin_fields`, and are what puts the `~` on the plan sheet's project total. Measured against PostgreSQL 16 on a four-row fixture before anything was written: **the `null` row came out exactly as it went in, and `SKIPPED` was empty** — so the run neither converted it nor said anything about having seen it. *(An earlier draft of this entry led with "`changed=1` → `changed=4`", and review was right that the comparison is unsound: the corrected sweep issues one UPDATE per column where the old one grouped a row's columns into one, so the number moved from ROWS to COLUMN VALUES in the same change. Two counts in different units read as a measurement and are not one. The row states are the evidence; the unit change is now pinned by a both-columns fixture in `services/api/test_pin_empty.py`, which nothing caught before because every earlier fixture seeded a single column.)* **SQLite hid it, and not by accident.** There a JSON column is TEXT, the scalar arrives as the four characters `null`, and the decode happens *after* the guard has already let the row through — so the conversion works and every local test passes. This is the same `null`-is-not-`NULL` distinction that migration was itself about, reappearing one layer up in its own reader. *A migration that fixes a representation problem has to be careful not to inherit it.* `d7f1a5c3e094` re-sweeps with the question asked in SQL instead: one SELECT per column, each narrowed to that column being non-NULL, so everything it returns is non-NULL in SQL and a Python `None` can only be the scalar. It is a new revision rather than an edit, because `b3c9e42d18a5` has run — editing it would change nothing on any database that ran it while making the file disagree with what was executed there. Verified on the same fixture: the `null` rows are now SQL NULL, the already-NULL row is untouched, and a real anchor survives. **The dialect was never the mechanism — the driver decoding the column is**, and that is why the gate does not need a PostgreSQL to be real. `test_pin_empty.py` registers a sqlite3 converter so its driver decodes too, and asserts the corrected sweep converts the row while the migration it corrects walks past it. The first draft of that section checked only the emitted SQL, and **reinstating the exact defect on top of the corrected SELECT passed it** — asking the right question and then throwing the answer away are two different failures, and only one is visible in the statement. `test_pin_pgnull.py` runs the same shapes against a live PostgreSQL in `db-migrations.yml`; with no server it does not pass quietly, it asserts that the CI step which runs it is still there. ### A NULL test on a JSON column is a filter that filters nothing — now a gate Second half of the same defect, as a class rather than an instance. SQLAlchemy persists a Python `None` in a `JSON` column as the scalar `null` unless the column declares `none_as_null=True`, so `WHERE IS NOT NULL` matches every row ever written and `IS NULL` matches none. `services/api/test_json_null_filter.py` derives every NULL test in `services/api/src/aec_api` by AST — **21 sites**, re-derived rather than taken from the roadmap's 20 — and resolves each subject against the model metadata. Three answers are safe: not a JSON column, a JSON column with `none_as_null=True`, or a JSON *extraction* (`data ->> 'x'`), which is a text scalar and yields SQL NULL on both backends for a missing key and for a stored `null` alike. One is not. Result: 15 safe, 5 unresolvable subjects each read and exempted with what the subject actually is, and **one live member of the class** — `Topic.labels` in `client_portal.py`, which narrows on `type == "info"` first and applies the real test in Python, so it over-fetches and does not answer wrongly. It is recorded rather than fixed: making it right needs a sweep of the existing `'null'` rows. It fails closed — an unresolved subject reds the build rather than being assumed safe — and it runs itself against the known instance before it is allowed to report anything, because a derivation that reports a clean tree is indistinguishable from one that reports nothing. **What it deliberately does not assert** is that every JSON column declares `none_as_null=True`. Measured: 588 JSON columns, 308 without it. Turning those on is a write-behaviour change on 308 columns, not a gate, and almost none of them is ever the subject of a NULL test — which is what makes the reader the right place to stand. ### @thatopen/ui dropped — the guard was demanding a browser verification of a package that cannot reach the browser `@thatopen/ui` was a declared dependency that **nothing imported**: zero source references tree-wide, absent from the built bundle, and a peer dependency of nothing here — it appears only as a *devDependency* of the upstream `components` packages, which imposes nothing on us. Yet `scripts/check-fragments-version.mjs` pinned it in `KNOWN_GOOD`, and that guard's failure message sets a specific contract: re-verify the viewer — load a model, author, section — and move the pin in the same commit. So every dependabot patch bump demanded a full live viewer pass for a library that **cannot affect the running app**. That bill was paid twice: #416 was closed on exactly this cost/benefit, and #545 paid it again — the live pass was done, and its most useful finding was that there was nothing to verify. Removed from `apps/web/package.json`, the README pinned-version table, `KNOWN_GOOD`, and `docs/credits.md` — the last of those mattering because credits name what is *shipped*, and it no longer is. The npm lock was regenerated with `--package-lock-only` rather than hand-edited: the removal orphans fourteen packages in total (`lit` and its three internals, `chart.js`, `chartjs-plugin-datalabels`, `iconify-icon`, the three `@floating-ui` packages, `@iconify/types`, `@kurkle/color`), and computing a transitive orphan closure by hand is how a lockfile stops matching what `npm ci` will install. **Zero packages added, zero versions changed** — verified by diffing the two lockfiles rather than by reading npm's summary, which matters because twenty-one dependencies carry `^` ranges that a careless regeneration could have floated. The general point is worth keeping: **a guard that names something unreachable does not protect it, it only charges for it.** The pin looked like coverage and behaved like a toll. ### numpy 2.5.3 and manifold3d 3.5.3 — two dependabot PRs that could never have gone green alone #516 (`numpy>=2.5.3`) and #517 (`manifold3d>=3.5.3`) both failed the API test gate, and the reason is written in the tree they were editing. `services/api/requirements.in` carries it on the numpy line itself: *"floor raised WITH services/data/requirements.txt: pip-compile reads only this file, so a floor raised only there cannot move the lock."* Dependabot edits `services/data/requirements.txt`, which pip-compile never reads — so the lock kept pinning 2.5.2 and 3.5.2, and `test_lock_satisfies_requirements`, which deliberately reads **both** requirements files, kept failing. **Structural, not incidental:** no amount of rebasing could have fixed either PR. Raising both floors lets the recompile follow, and it moves **exactly two pins** — nothing added, nothing removed. That was predicted from a measurement before the change was made, then confirmed by CI's `Verify` step on the committed bytes. **This lock was compiled locally, which is the opposite of the previous entry's conclusion — and the reversal is the point.** There the bytes were transcribed out of CI because a local resolve had produced an unrecognisable answer. Once the cause was understood (a fresh output path preserves nothing; an in-place one constrains to the existing pins), the local tool became the same tool CI runs, and `Verify` confirmed byte-identity on a lock carrying hundreds of hash lines rather than alembic's two. **The clock said it before the diff did.** The fresh-path compile took over forty minutes because it re-resolved and re-hashed the whole graph; the in-place compile took **thirty-nine seconds** because it only fetched what moved. A forty-minute run and a forty-second run were never doing the same work, and nobody had to read a single package name to see that. *When two runs of "the same" command differ by sixty-fold in cost, they are not the same command, and the cheap explanation for their differing output is probably not the true one.* Transcription would not have worked here in any case: the `Verify` step prints `head -60` of its diff, which alembic's two hashes fit inside and `manifold3d`'s dozens of wheel hashes do not, and the artifact holding the full file sits behind an egress block. Either route ends at the same gate, which is what makes either safe. ### alembic 1.18.5 → 1.20.0, and a local resolver that confidently reported the wrong answer Dependabot's #519 carried this floor bump with a recompiled lock and went fully green, including the Postgres `alembic upgrade + check + runtime-parity` job. It then became unmergeable: three later bumps each recompiled `services/api/requirements.lock`, so the branch conflicted in a generated file that must never be merged by hand. Handing it back to dependabot was not available either — comment commands posted from this session are rewritten in transit, with U+00B7 injected around the `@` and inside the command word, so `recreate` and `rebase` both read correctly in the timeline and did nothing. **A command that looks delivered and is inert is worse than one that visibly fails.** Redone here, with the lock taken from `lockfile.yml`'s own compile in `python:3.12-slim` rather than from a local one — and that distinction turned out to matter enormously. **The local resolve said 34 packages would move.** `anthropic` 1.2.0 → 1.5.0, `websockets` 16.1 → 17.1, `cryptography`, `sqlalchemy`, `certifi`, and thirty more: a one-line floor bump becoming a tree-wide sweep, which is exactly the kind of scope change worth stopping for. It was wrong. CI's `Verify` step prints its own diff, and that diff carries `annotated-doc==0.0.4` as an unchanged **context** line — the very next package after alembic — while the local resolve moved it. `Verify` then passed on the transcribed lock, proving the real change is alembic **and nothing else**. **The cause was the invocation, not the environment** — and the first explanation written here was wrong in a way worth preserving. It blamed the sandbox's package index for serving something different from the prod container. That was a guess dressed as a diagnosis, and it would have libelled perfectly good local tooling. `pip-compile` treats an EXISTING output file as constraints and moves only what the inputs force; the local runs wrote to fresh scratch paths, where there were no pins to preserve, so everything resolved to latest. Compiling into a copy of the real lock instead moves **zero** packages — which is also why only alembic moved in CI: its floor was the only one that changed. So the reproduction rule is concrete: **compile in place, to the lock you intend to replace.** A lockfile regeneration aimed at a new path is not the same operation and does not answer the same question. *A number with a package list attached reads as evidence.* The list was specific, ordered and plausible — it was very nearly reported as a finding, and its wrong explanation very nearly outlived it. What exposed the number was a context line in someone else's diff; what exposed the explanation was checking whether the two versions even existed upstream, and finding that they did, with cp312 wheels, which left no room for an index story. **When a measurement and a gate disagree, the gate is the one that graded the artifact — and the first story that explains the gap is not automatically the true one.** Worth recording that the floor and the pin differ on purpose. `requirements.in` asks `>=1.19.2`, what dependabot requested; the lock resolves to the latest satisfying release, 1.20.0. So the behaviour under test is 1.18.5 → 1.20.0, wider than the floor names, with alembic 1.19's autogenerate CHECK-constraint change inside that span. ### A digest pin freezes the base image, and the CRITICAL gate blocks on it going stale The API container build went red on three fixed CRITICAL CVEs in `perl-base` — CVE-2026-13221, CVE-2026-42496 and CVE-2026-8376, all `Status: fixed` at `5.40.1-6+deb13u1` — with nothing in the diff that touched a Dockerfile. `services/api/Dockerfile` is byte-identical to main, the base images are pinned by digest, and the two other images in the same matrix (`node:24-slim`, `nginx:1.31-alpine`) passed. Nothing about the change caused it; the pinned `python:3.12-slim` simply predates a Debian security update, and the Trivy CRITICAL gate is doing exactly its job. **The obvious fix is a bet, not a fix.** Bumping to a newer `python:3.12-slim` digest only helps if upstream happened to rebuild after the DSA landed — and the tag's head at the time was built 2026-09-01, with no way from here to establish which packages that rebuild picked up. The egress policy in this environment blocks both the Docker blob CDN and `deb.debian.org`, so the candidate digest could not be opened and read either; a digest chosen and pushed on the assumption it was newer-and-therefore-fixed would have been a guess wearing a checksum. So the runtime stage applies Debian security updates at build time instead. That is not a weaker supply-chain position than what the stage already did — the same `apt`, the same signed archive, the same build-time resolution as the `apt-get install` one line below it — and the digest pin is untouched, so the base layers are still the reviewed ones. **A pin fixes the starting point; it does not stop the starting point going stale**, and a gate that blocks on staleness needs a fix aimed at staleness. **And the first draft of that fix was stale by construction — review caught it.** This entry claimed the upgrade "keeps working: the next DSA is picked up by the next build". It would not have. Both container jobs build with `cache-from: type=gha`, and the cache key for that `RUN` is its parent (a pinned digest, which never moves) plus its command string (which also never moves). The layer would have been restored from cache on every later build, so the upgrade would have run **exactly once** — on the build that introduced it — and then frozen at whatever the archive held that day. `ci.yml` now passes `no-cache-filters: runtime` for the api image, and the stage is named `AS runtime` to be addressable; the expensive `pybuild` and `converter` stages stay cached. The shape is the one this repo keeps meeting: **a mitigation whose self-maintaining property was asserted rather than checked.** It is the same failure as a gate that answers instead of failing, moved one layer out — the remediation would have been real on the day it was written and inert every day after, with a green build both times. *A fix that depends on re-running needs the re-running proved, not assumed.* Worth noting where it was caught: not by a test, because nothing here can execute a container build — no docker daemon in the authoring environment, and the egress policy blocks the registry's blob CDN. The check that could fail was CI, and the reader who spotted the gap ahead of it was a review bot. ### The escaping gate could not see a helper's parameters, so 108 sinks sat outside it The identity ratchet shipped in the previous entry freezes `file :: expression`. That constrains a sink whose expression NAMES its source — `${a.filename}` cannot start carrying something else without the identity moving. It does nothing for a bare value binding: ```ts const card = (label: string, value: string) => { c.innerHTML = `
${value}
${label}
`; // identity: "value" }; ``` `value` is the identity whatever the caller passes. Every call site could switch from a literal to a user-controlled field and the baseline would not move — **the sink goes live while the gate stays green**. That is the same defect the ratchet was built to fix, one level up: there the frozen key was a count with no identity, here it is an identity with no source. **108 interpolations across 27 files**, all now escaped. The population was derived by AST — bare identifiers reaching `innerHTML` that resolve to a function parameter or a destructuring binding — and the fix is verifiable in a way most sweeps are not: escaping a sink can only ever REMOVE entries from the ratchet's baseline, never add one. It went **119 → 74 with zero additions**. Three things this found that the existing gate could not: - **The HOT term list was hiding 53 of them.** `budget.ts` interpolated a bare `val` and `evm.ts` a bare `sub`; neither word matches any term in the list, so both were invisible to the ratchet while being exactly the shape the new rule exists to catch. Under the term filter the population read 32; under the shape rule it read 85. *A keyword list derived from what someone happened to be looking at cannot bound a population — a shape can.* - **The first draft of the new rule tested only for parameters, and reported the tree clean at 85 fixed sites.** 23 more were live in `for (const [label, val] of cards)` — a destructuring binding is a value handed in from elsewhere exactly as a parameter is, and the name says nothing about the source either way. *A predicate that decides what to LOOK at hides everything it excludes from its own count*, which is why the count looked complete both times. - **`register.ts` carried a comment reading "textContent/esc throughout: ref+title are user data (stored-XSS guard)"** directly above the one interpolation on that line that was not escaped. The new rule takes **no baseline and admits no exemption** — the fix is always available and always correct, so there is nothing to grandfather, and grandfathering one would freeze a sink open by definition. Its boundary is stated rather than implied: a plain `const x = …` local is out of scope, because 157 of those reach `innerHTML` in this tree and most hold markup assembled a few lines earlier (`rows`, `thead`, `bars`, `cells`) where escaping would destroy it. A local assigned from user data and interpolated bare is caught by neither rule in this file, and the test says so. **The excluded class was then triaged, not assumed** — an exclusion nobody measures is a blind spot with a justification attached. All 134 bare locals were classified by their declaration's initialiser (41 build markup, 39 literal, 15 numeric, 1 already escaped, 36 suspect, 2 unresolvable) and every suspect and unresolvable one was read. Exactly **one was a real sink**: the CPM panel's critical-path line, where the server builds `critical_path` as `[r["ref"] or r["id"] …]` — stored, user-entered activity refs — and the panel joined and interpolated them raw into `innerHTML`. Anyone who can name a schedule activity could put markup in it and have it run for everyone who opens that panel. Fixed. Two things about that triage are worth keeping. Its first draft resolved declarations **file-wide and kept the last match**, so one panel's `gap` (a number) was attributed to an unrelated `el("div")` hundreds of lines away and reported suspect — the lookup is scope-aware now, and the bug surfaced only because the suspects were *read* rather than counted. And the single real finding was a **local**, i.e. in the class the new rule deliberately excludes: *the documented gap was live, not theoretical,* which is the whole argument for measuring an exclusion instead of asserting one. **A review round then found the detector's real boundary, and it was hiding a live sink.** The scanner walked only the interpolations lexically inside an `innerHTML =` expression. But markup is usually assembled into a local FIRST and only then interpolated — `const rows = deals.map(d => \`${d.name}\`)`, then `panel.innerHTML = \`…${rows}…\``. Every span inside that builder was invisible to **both** rules: not baselined, not flagged, live. The reviewer found one symptom (`d.name`, a saved scenario name that any user can set); behind that one hop sat **15 hot interpolations across 8 files**. *The scanner was measuring syntax where the question is reachability.* It now follows one hop, and stops there deliberately — a local built from another local is still uncovered, and the test asserts that limit rather than leaving it to be discovered. Nine of the fifteen were real and are escaped: a scenario name, vendor names in a bid-levelling matrix, carbon hotspot element names, a takt trade name, project-health domain labels, sources-uses line labels, forecast line names, and test-fit scheme names. The remaining six are one shape — a ternary whose branches are literal markup or a glyph (`s.name === r.best ? " ★" : ""`) — which match the term list on their *condition*, never their value. Escaping any of them would render markup as visible text, so they are baselined with that reason: **the fix would be the bug.** Three more findings from the same round, all real. The value-binding rule ignored **constructor and setter parameters** — a coverage hole in a security gate. It resolved bindings **file-wide**, so a destructured `rows` in one function made an unrelated plain local `rows` in a sibling fail the plain-local exclusion; the rationale written beside that code ("a false positive only costs one `esc()` that was already correct") was wrong, because for *this* exclusion the suggested fix escapes assembled HTML and breaks the page — *a false positive is only cheap when the fix it suggests is safe.* Bindings resolve lexically now. And a milestone `