8e99b3b4c0
A commit that removed a modal's self-closing behaviour shipped two comments in the same file still describing it as self-closing — review checks whether the code is right, not whether the surrounding prose survived. And the property that predicts which modals break on refresh is 'holds no state of its own' (pure projection of a list), not the presence of a detail field.
544 lines
40 KiB
Markdown
544 lines
40 KiB
Markdown
# Admin-UI cleanup playbook (`ZB.MOM.WW.Theme` + Bootstrap Blazor apps)
|
||
|
||
A reusable sweep recipe for the family's Blazor Server admin UIs, distilled from five completed
|
||
rounds:
|
||
|
||
- **ignitionoee router** — `../ignitionoee/docs/plans/2026-08-05-router-ui-sweeps.md` (+ its
|
||
render-validation companion plan)
|
||
- **OtOpcUa AdminUI** — `../OtOpcUa/docs/plans/2026-08-10-adminui-ui-sweeps.md`
|
||
- **MxAccessGateway dashboard** — `../MxAccessGateway/docs/plans/2026-08-11-dashboard-ui-sweeps.md`
|
||
- **HistorianGateway dashboard** — `../HistorianGateway/docs/plans/2026-08-11-dashboard-ui-sweep.md`
|
||
- **ScadaBridge CentralUI** — `../ScadaBridge/docs/plans/2026-08-11-central-ui-cleanup-sweep.md`
|
||
|
||
All five sit on **ZB.MOM.WW.Theme 0.3.1 + self-hosted Bootstrap 5.3** (mxgw included — an earlier
|
||
claim that it was Bootstrap-free was false; see the exemption rule below), and all five found the
|
||
same *classes* of defect with different instances. **Run the discovery phase before assuming which
|
||
instances your app has.** The spread so far: the router had ghost layout classes and OtOpcUa had
|
||
none; OtOpcUa had a dead scoped-CSS bundle and mxgw/HistorianGateway have no `*.razor.css` at all
|
||
while ScadaBridge's bundle was already correctly wired; HistorianGateway's phantom tokens were all
|
||
fallback-masked and ScadaBridge had zero; ScadaBridge's *pages* were right and its *docs* were
|
||
wrong, inverting §1c.
|
||
|
||
**Determine the test harness per app — do not assume.** The router and OtOpcUa have no bUnit, so a
|
||
live browser pass is their only gate. HistorianGateway (`bunit 1.40.0`, ten Dashboard test files)
|
||
and ScadaBridge (`CentralUI.Tests`, 990 tests) both have one, which changes discovery materially
|
||
(see §1a-1). **A live pass is still mandatory everywhere**, because computed styles and cascade
|
||
order are exactly what bUnit cannot see.
|
||
|
||
### Exemptions need their premise re-derived
|
||
|
||
If a family-wide sweep or gate excuses one app on a property claim about that app — "it's
|
||
Bootstrap-free", "it has no dark mode", "it has no scoped CSS" — **re-derive that property from
|
||
that repo at sweep time.** It happened once already: the 2026-07-19 `/admin/secrets` modal gate
|
||
skipped mxgw as "Bootstrap-free so never affected", which was false; mxgw self-hosts Bootstrap
|
||
5.3.3 and *was* affected, and only escaped because it had taken the fix independently. The
|
||
exemption reached the right conclusion on false reasoning and nobody re-checked for three weeks.
|
||
This is the same discipline §1c applies to cited config keys, aimed at scope decisions instead of
|
||
prose.
|
||
|
||
---
|
||
|
||
## 1. Discovery — five read-only scans (1a–1d parallelize per file; 1d also needs a live walk)
|
||
|
||
Run these as parallel read-only agents (or by hand) and write the inventories down with `file:line`
|
||
before changing anything. The inventories become the per-batch fix briefs.
|
||
|
||
### 1a. Foundation / CSS audit
|
||
|
||
The theme ships less than pages assume. Check, in order:
|
||
|
||
1. **Shipped-vs-used class matrix.** Extract every CSS class used in `*.razor` markup and check each
|
||
against (a) the theme package's `staticwebassets/css/*.css`
|
||
(`~/.nuget/packages/zb.mom.ww.theme/<ver>/`), (b) Bootstrap, (c) the app's own stylesheet, (d)
|
||
page `<style>` blocks, **(e) the test sources**. Anything defined in none of (a)–(d) is a
|
||
**silent no-op** — the browser ignores it without any error. (Router: 13 ghost classes from the
|
||
mockups' theme.css that 0.3.1 never shipped. OtOpcUa: zero — it was authored against what ships.
|
||
mxgw: 3, one load-bearing. HistorianGateway: 4. ScadaBridge: 26, but only one lost intent.)
|
||
|
||
**Check (e) BEFORE writing the ghost list, not as a filter afterwards.** A class matched only by
|
||
a test is a **selector, not a ghost** — rename it deliberately or leave it, never delete it. The
|
||
failure mode is nasty: deleting it reds the suite, and the tempting "fix" is to amend the
|
||
assertions, which quietly destroys the coverage. HistorianGateway's 16 `console-*` classes look
|
||
exactly like ghosts and are in fact driven by
|
||
`cut.Find("input.console-tag")` / `button.console-run`, with the Viewer role gate resting on
|
||
`Assert.DoesNotContain("console-write", cut.Markup)` — five tests. That sweep caught them only
|
||
because it happened to read the test files while checking something else; ordering luck, not
|
||
method. Hence: check first.
|
||
|
||
**The constraint is not limited to class names.** A bUnit suite can pin markup by *substring*
|
||
and by *visible text*: `ApiKeysPageRenderTests` asserts `DoesNotContain("secret", …,
|
||
OrdinalIgnoreCase)` over the list markup, and elsewhere selects with
|
||
`Find("button:contains('Create API Key')")`. So **button label text is load-bearing too** — a
|
||
sweep that "improves" wording breaks a suite exactly as a deleted class does. This binds §1c as
|
||
much as it binds this step; see the note there.
|
||
|
||
Corollary, learned the hard way twice: **a dead-code claim derived from grepping `src/` alone is
|
||
not a dead-code claim.** ScadaBridge nearly deleted `Shared/DataTable.razor` as an orphan —
|
||
`DataTablePagerTests` renders it. Same mistake as the `console-*` near-deletion, different
|
||
clothes.
|
||
2. **Is the scoped-CSS bundle linked?** If any `*.razor.css` exists, the build emits
|
||
`<App>.bundle.scp.css` — but nothing links it automatically when the head is hand-authored.
|
||
Compare the `<link>` list in `App.razor` against `obj/…/scopedcss/bundle/`. OtOpcUa's
|
||
`ContextMenu.razor.css` built, served 200, and was referenced by **nothing** — every context menu
|
||
rendered in-flow (no `position:fixed`, no backdrop) for the component's whole life.
|
||
3. **Phantom CSS custom properties.** Grep every `var(--…)` in markup/inline styles against the
|
||
tokens actually defined in the theme + app CSS. An undefined token makes the declaration invalid
|
||
*at computed-value time* — silently dropped, no console error. A fallback argument
|
||
(`var(--x, #fff)`) hides the defect. (OtOpcUa: `var(--alert)` ×13 — the theme's red family is
|
||
`--bad*` — so intended red error tints stayed amber; `var(--surface-2)` likewise dead.)
|
||
4. **Button sizing.** Theme 0.3.1 ships **no `.btn` rule**, so Bootstrap's hard-coded
|
||
`--bs-btn-font-size: 1rem` applies against the theme's 0.9rem body. Count how many buttons carry
|
||
`btn-sm` vs full-size; modal feet and shared form shells are where full-size accumulates.
|
||
(ScadaBridge: 50 full-size + 13 `py-0 px-*` hacks.)
|
||
|
||
Two **inverted** instances, where the standard §2 `.btn` block would not have helped:
|
||
|
||
- **A literal `font-size` in the app stylesheet** (mxgw `site.css:187`, `font-size: 0.82rem`).
|
||
Bootstrap sizes `.btn-sm`/`.btn-group-sm` *purely* by redefining `--bs-btn-font-size`, so a
|
||
literal at equal specificity loaded later flattens every button to one size — `btn-sm` becomes
|
||
a font-size no-op app-wide and differs only in padding. Tell: `btn-sm` present in quantity but
|
||
visually indistinguishable.
|
||
- **A chassis class standing in for the button system** (HistorianGateway: all 13 buttons were
|
||
`.rail-btn`, the *side-rail* class, and 11 carried inline `style="width:auto"` to undo the
|
||
`width:100%` a local override imposed). The `.btn` block would have applied to nothing. Fix is
|
||
the block **plus** moving page buttons onto real `.btn`; the width hacks then delete
|
||
themselves. Tell: inline sizing hacks clustered on one non-`.btn` class.
|
||
5. **Dark scheme: determine whether the app has one.** Theme 0.3.1 itself is light-only (no
|
||
`prefers-color-scheme` / `data-bs-theme`), and the router, OtOpcUa, mxgw and HistorianGateway
|
||
are all light-only — for those, don't chase it, and fix any comment claiming otherwise.
|
||
**ScadaBridge is the exception**: it implements its own `[data-bs-theme="dark"]` token block
|
||
plus a pre-paint localStorage script. Where a second scheme exists this step **inverts** —
|
||
hard-coded colours and inline token overrides are actively worse, not merely moot, because they
|
||
are invisible in the scheme you're looking at and broken in the other.
|
||
6. **Per-app token overrides delivered as inline styles.** Check how the app passes brand tokens to
|
||
`ThemeShell`. The kit emits `Accent="…"` as an inline `style="--accent: …"` **on the shell
|
||
root** — a descendant of `<html>` — so it **beats every scheme-level `:root` override below it**.
|
||
ScadaBridge's `Accent="#2f5fd0"` killed the dark accent app-wide (brand mark, active rail-link
|
||
border, `.rail-btn`) while being a no-op in light, because that hex already *is* theme.css's
|
||
light default: invisible in both directions.
|
||
|
||
The general rule: **a per-app token override delivered as an inline style on a shell element
|
||
silently defeats every scheme-level `:root` override.** Declare the token in the app
|
||
stylesheet's `:root` instead — it then loses to the dark block by source order and wins over
|
||
theme.css by sheet order.
|
||
|
||
Worked example of why to check even when nothing looks wrong: **OtOpcUa passes the identical
|
||
`Accent="#2f5fd0"`** (`MainLayout.razor:11`) and has no dark scheme at all, so the override is
|
||
**inert today and latent forever** — it does nothing now and silently kills the dark accent the
|
||
day someone adds a dark block. That future session will see "dark mode doesn't work" with no
|
||
reason to suspect a layout parameter set years earlier. ignitionoee passes no `Accent` and is
|
||
unaffected.
|
||
|
||
### 1b. Button inventory
|
||
|
||
Quick tells: `grep -rn "btn-group" Components/` returning **zero** means nothing is grouped
|
||
(both apps' idiom was `d-flex gap-2`); also grep for `·` separators between actions,
|
||
`btn-link p-0` on things that *do* rather than *navigate*, `py-0`/inline `width:` sizing hacks.
|
||
|
||
Inventory, with file:line:
|
||
|
||
- **Adjacent related buttons** that should be one `btn-group`: page-head action pairs, modal/panel
|
||
feet (Save/Cancel, Cancel/Confirm), pagers, browse-toolbar pairs, inline confirm pairs.
|
||
- **Row actions styled as links** (`<a href="#">` or `btn-link` text-links) → real
|
||
`btn btn-sm btn-outline-secondary`, destructive → `btn-outline-danger`, grouped when ≥2 in a cell.
|
||
**Distinguish navigation from action**: a link that routes somewhere stays a link; a link that
|
||
mutates state becomes a button. Both directions can coexist in one row (OtOpcUa's UnsTree: keep
|
||
`⚙ settings`/`Open` as links, convert `+ Area`/`Edit`/`Delete` to buttons).
|
||
- **Arm→confirm flows** (click Delete → Confirm/Cancel appears): list them; the fix is restyle-only
|
||
and must not collapse the two-step.
|
||
- **Size mismatches** within one visual group (`btn-sm` beside full-size).
|
||
|
||
### 1c. Prose inventory (DELETE / KEEP / RELOCATE rubric — binding)
|
||
|
||
- **DELETE**: unconditional documentation blocks — always-rendered banners explaining how a feature
|
||
works, config-mode walkthroughs, "how X is decided" sections, column-meaning tables. These belong
|
||
in `docs/`, not on the page. Verify each fact is already docs-covered (grep for distinctive
|
||
phrases); if it is, plain delete.
|
||
- **RELOCATE-then-delete**: facts stated **nowhere else**. Land them in the right doc file *first*,
|
||
then delete the page copy — same ordering both sweeps used (foundation/doc task before page
|
||
batches).
|
||
- **KEEP**: state banners (empty/error/truncation/live status), form hints (`form-text`, field
|
||
hints — they render only in form context), visual-encoding **legends** that are the only decoder
|
||
of a chip/graphic, workflow cross-links, expander-gated notes.
|
||
- **Before rewording anything, re-read §1a-1(e).** In a repo with bUnit, **visible text is a
|
||
selector**: `Find("button:contains('Create API Key')")` makes a button's label load-bearing, and
|
||
`DoesNotContain("secret", …, OrdinalIgnoreCase)` makes an *absent* substring load-bearing. Prose
|
||
work is the activity most likely to trip this, because improving wording is the whole point of
|
||
it. Grep the test sources for any string you are about to change.
|
||
- **The wrong document may be the docs, not the page.** ScadaBridge inverted this section entirely:
|
||
zero DELETEs, zero RELOCATEs — its pages carried no unconditional documentation at all — but its
|
||
*docs* were wrong. `Component-Transport.md` claimed notification lists pull their SMTP config
|
||
(they don't) and listed API keys as exportable (deliberately not; SMS configs were missing
|
||
instead), plus stale ClusterClient claims in three files. When the page and the doc disagree,
|
||
establish which one the code supports before assuming the page is the stale one.
|
||
- **Hunt stale claims while you're in there** — every sweep found prose that was actively *wrong*,
|
||
not just verbose: milestone labels for shipped features ("F8/F9 pending", "(F3)", "arriving in
|
||
Batch 2/3"), architecture claims superseded by later phases, dead hyperlinks to repo paths the
|
||
app doesn't serve, and **config keys that don't exist** (prose said `Authentication:Ldap`, the
|
||
code's `SectionName` said `Security:Ldap` — always verify cited key names against the options
|
||
class). Wrong facts outrank style: fix or delete them even if you skip everything else.
|
||
|
||
### 1d. Density / layout scan (found post-sweep, on OtOpcUa's `/hosts`)
|
||
|
||
The four scans above can all pass while a page is still unreadable — the sweep left `/hosts`
|
||
structurally correct and visually overloaded ("text slams together, the table is overloaded, no
|
||
separation between sections"). Walk each *page* (not each file) with realistic data and look for:
|
||
|
||
- **Sections running together.** One `panel` wrapping several groups, each with its own nested
|
||
`panel-head` — three headings stack flush with no card boundaries. Tell: `panel-head` appearing
|
||
more than once inside one `<section class="panel">`.
|
||
- **Identity slam.** A cell rendering name + id + status chips inline in one run
|
||
(`gate-modbus-renamedDRV-ce2fb5217f5d`). Two identifiers in one cell need two lines.
|
||
- **Unbounded free-text columns.** Any column bound to text the app doesn't control — error
|
||
messages, exception text, paths, config values — with no truncation. One long driver exception
|
||
widened the table until the actions column scrolled out of reach. Tell: `@d.SomeError`,
|
||
`@x.Message` etc. rendered bare in a `<td>`.
|
||
- **Inline full-width expander/detail rows** inside an already-dense table (a `colspan` row hosting
|
||
a whole panel). They destroy scannability exactly when the operator most needs it.
|
||
- **Realistic data matters.** On a healthy rig every error cell reads "—" and the defect is
|
||
invisible — the live gate below must be run with at least one faulted/erroring row on screen.
|
||
|
||
### 1e. Tree tables — check the data shape before porting the component
|
||
|
||
The router built a generic `TreeTable` + `TreeVisibility` (depth-ordered flat `ITreeRow` list,
|
||
forward-scan visibility — reference implementation in
|
||
`../ignitionoee/router/src/ZB.MOM.Oee.Ui/Components/Shared/TreeTable.razor` + `ViewModels/TreeRows.cs`)
|
||
and adopted it on two pages. OtOpcUa **rejected it entirely** after the scan. The rubric:
|
||
|
||
- **Candidates**: tables flattening real hierarchy through hacks — a repeated `"SITE / AREA / LINE"`
|
||
path-prefix column, hand-rolled `rowspan` grouping, indent-by-padding rows, faked full-width
|
||
group-header rows.
|
||
- **Not candidates**: rank-ordered lists and fact/audit logs (a tree destroys load-bearing
|
||
ordering), tables already grouped per parent (one table per group), rows that are all roots.
|
||
- **Nav/picker trees are a different animal.** Lazy-loading navigation trees with context menus or
|
||
selection (OtOpcUa has four) don't fit the flat-list TreeTable model (no columns, lazy expansion)
|
||
— leave them alone. Duplicated expander scaffolding across nav trees is not, by itself, reason to
|
||
unify in a repo with no component test harness.
|
||
|
||
If no adoption justifies the component, don't port it — an unused component is an orphan.
|
||
|
||
---
|
||
|
||
## 2. How to fix
|
||
|
||
### Foundation first (one task, own files, everything else references it by name)
|
||
|
||
CSS class names aren't compile-checked, so batch agents can reference foundation classes before the
|
||
foundation branch merges — all branches build independently and compose at merge.
|
||
|
||
1. **App stylesheet** loaded **after** `<ThemeHead />` so it wins the cascade (create one if the app
|
||
lacks it; the router created `app.css`, OtOpcUa extended `site.css`).
|
||
2. **Button sizing as Bootstrap CSS-variable overrides** — never redefine box properties wholesale
|
||
(`btn-group` seam/radius machinery depends on them). The block both apps now carry:
|
||
|
||
```css
|
||
.btn { --bs-btn-font-size: .85rem; --bs-btn-padding-y: .35rem; --bs-btn-padding-x: .8rem; --bs-btn-line-height: 1.4; }
|
||
.btn-group-sm > .btn, .btn-sm { --bs-btn-font-size: .78rem; --bs-btn-padding-y: .25rem; --bs-btn-padding-x: .6rem; }
|
||
```
|
||
|
||
This normalizes every unsized button app-wide without touching call sites. **Upstreamed in
|
||
`ZB.MOM.WW.Theme` 0.4.0** — the kit now ships this block in `layout.css`, after all five apps had
|
||
independently written it. On bump to 0.4.0+ it arrives automatically, so remove the local copy in
|
||
the same change or the rule is declared twice.
|
||
|
||
**Trim the local block, don't delete it wholesale.** The upstreamed rule covers *sizing* only —
|
||
the four `--bs-btn-*` variables and the `.btn-group-sm > .btn, .btn-sm` rule. A local `.btn` may
|
||
also carry app-specific *shape* declarations that predate the sweep and that the kit does not
|
||
ship; mxgw's carried `border-radius: 5px; font-weight: 500; white-space: nowrap`, and a wholesale
|
||
delete would have silently dropped all three. Diff your block against the kit's before removing
|
||
**And check whether your app has one at all — with TWO greps, because one is misleading.**
|
||
"Every app carries this block" was false (HistorianGateway has none). But the obvious correction —
|
||
"then the kit now supplies sizing it was missing, so the bump restyles it" — was **also false**, and
|
||
is the more instructive error. Two independent questions:
|
||
|
||
```bash
|
||
grep -rl bs-btn-font-size src | grep -v lib/bootstrap # (1) declares its own sizing?
|
||
grep -rhoE 'class="[^"]*"' <components> \
|
||
| grep -cE '(^|["[:space:]])btn([[:space:]]|"|-)' # (2) uses Bootstrap buttons?
|
||
```
|
||
|
||
They come apart exactly when an app doesn't use Bootstrap buttons. HistorianGateway answers no to
|
||
both — all 15 buttons carry the kit's `.rail-btn` chassis class — so the kit's `.btn` rule matches
|
||
nothing and the bump is invisible there. Zero call sites ⇒ invisible regardless of the block;
|
||
non-zero and no block ⇒ the restyle warning holds. Ask (2) before telling anyone their UI will
|
||
change, and note that a "Bootstrap-free" app is precisely the profile that yields zero call sites.
|
||
|
||
anything: four of the five apps' blocks were pure sizing and safe to delete outright, one was
|
||
not, and the difference is invisible unless you look. Two of the four that were safe deleted
|
||
without diffing first and said so — they were lucky, not careful, and that is the same shape of
|
||
error as the ghost-class deletion §1a-1 guards against. **When a family-wide instruction lands,
|
||
check mxgw by hand first**: it is the member whose local CSS predates the kit by the most, so it
|
||
is the most likely to be the exception to any "all the apps have X" premise — it was the outlier
|
||
here *and* the one wrongly exempted from the scadaproj#2 modal sweep. Note also that a retained `border-radius`
|
||
should stay a **literal** rather than becoming `--bs-btn-border-radius` — `.btn-sm` redefines
|
||
that variable, so the switch would shrink small buttons to Bootstrap's small radius.
|
||
3. **Phantom tokens → a real class**, not a token alias. OtOpcUa's pattern:
|
||
`.notice-alert { color: var(--bad); background: var(--bad-bg); border-color: var(--bad-border); }`
|
||
then migrate the inline `style="border-color:var(--alert)"` hacks to the class in the page
|
||
batches. Mirror how the theme's own variant classes (e.g. `.chip-alert`) use the tokens.
|
||
4. **Link the scoped bundle** if 1a-2 found it dead:
|
||
`<link rel="stylesheet" href="_content/<AppNamespace>/<AppNamespace>.bundle.scp.css"/>` — verify
|
||
the exact asset name from the build output, not by guessing.
|
||
5. **Shared class for repeated inline sizing** (e.g. `.tree-twist { flex: 0 0 18px; width: 18px; }`
|
||
replacing four trees' `style="width:18px"` expanders).
|
||
6. **Relocate the orphaned prose facts into docs** (from 1c) in this same task, so page deletions
|
||
never race the relocation.
|
||
|
||
### Button conventions (page batches)
|
||
|
||
- `<div class="btn-group" role="group">` (`btn-group btn-group-sm` when members are small); fold
|
||
per-button `btn-sm` into the group class; drop `ms-2`/`me-1` spacers between members.
|
||
**Budget for the fold breaking test locators**: any selector pinning `button.btn-*.btn-sm` stops
|
||
matching the moment `btn-sm` moves onto the group. ScadaBridge updated 16 selectors for exactly
|
||
this and it is a mechanical, expected cost — not a signal that the fold was wrong. Three sweeps
|
||
hit the fold and only the one with a dense suite noticed, which is why it is written down here.
|
||
- Mixed `<a class="btn">` + `<button>` members are valid Bootstrap — group them (keep `download`
|
||
etc. attributes verbatim).
|
||
- A group whose conditional member doesn't render is fine — keep `@if` inside the group.
|
||
- Deliberately separated members stay **outside** the group (the `ms-auto` Delete in a form foot);
|
||
inputs interleaved in a run split it into two groups (never put a `form-control` inside).
|
||
- Destructive actions → `btn-outline-danger`; the armed/confirm state may go solid `btn-danger`.
|
||
- **Preserve behavior byte-for-byte**: every `@onclick`, `disabled`, binding, `AuthorizeView` gate,
|
||
and arm→confirm flow unchanged — wrappers, classes, and prose only. Grep the diff for
|
||
`@onclick|disabled=|href=|type="submit"` appearing identically on both sides.
|
||
- Respect existing "buttons-not-anchors" comments (Blazor's `blazor.web.js` navigation interceptor
|
||
is why some tree labels are buttons) — never convert those to links.
|
||
|
||
### Density fixes (the `/hosts` rework, applicable anywhere 1d fires)
|
||
|
||
- **One panel per group.** Replace panel-wrapping-groups with one `section.panel` per group; hoist
|
||
the group's metadata (node/host chips, counts) into that panel's head so it stops being a
|
||
paragraph between heading and table.
|
||
- **Stack identities.** Primary name on the first line; the machine id on its own
|
||
`<div class="mono small text-muted">` beneath. Chips stay with the name line.
|
||
- **Truncate unbounded text with a safe helper, never a raw slice.** OtOpcUa's
|
||
`DisplayText.Abbreviate(value, 60)` + the full text on `title` — a raw `@x[..60]` 500s the whole
|
||
page when the value is shorter (OtOpcUa #504). Keep the full text reachable (tooltip + the
|
||
row's detail dialog). Every app that hit this needed its own helper (HistorianGateway added
|
||
`Dashboard/DisplayText.cs` with 7 tests pinning the short-value case); ScadaBridge also found
|
||
three unguarded `[..8]` slices in `Topology.razor` that were a latent
|
||
`ArgumentOutOfRangeException` on the deploy path, so grep for raw range operators generally, not
|
||
only in the cells you're truncating.
|
||
- **CSS clipping is the wrong tool for user-resizable grid columns.** `nowrap` + `max-width` raises
|
||
the column's min-content width and breaks drag-to-resize. Cap the *string* there instead, and
|
||
keep CSS clamping for fixed-width columns.
|
||
- **Inline expander rows → a dialog.** Host the same detail component in a Bootstrap modal
|
||
(`modal-backdrop fade show` + `modal fade show d-block`, `modal-lg`), one at a time. Lifecycle
|
||
cautions that made the OtOpcUa version correct:
|
||
- Hold only the **id** of the open row on the page component; **re-resolve the row from current
|
||
data every render** — auto-refresh replaces row records, and a dialog caching one goes stale or
|
||
strands. A row that disappears ⇒ the dialog renders nothing (self-closes).
|
||
- Preserve any `@key` recycle discipline the inline version carried (OtOpcUa keys the panel on
|
||
`id + Enabled` so a config flip disposes and remounts it, keeping store subscriptions honest).
|
||
- Expect **source-scan guard tests** to pin the old markup (no bUnit ⇒ repos guard wiring by
|
||
grepping the `.razor` source); re-pin them to the dialog markup in the same commit.
|
||
|
||
### Execution pattern (what actually worked, twice)
|
||
|
||
- Discovery scans → plan doc with the full inventories → **one foundation agent + N page-batch
|
||
agents on disjoint file sets**, all parallel; shared components (form shells, collection editors)
|
||
are the biggest lever — one file can fix a dozen forms.
|
||
- Briefs must carry exact `file:line` inventories **and the rubric that produced them**, not just
|
||
the line numbers — plus the instruction *"verify by reading before editing; refuse items the
|
||
markup contradicts."* An agent holding only a line list can't tell a mis-derived item from a
|
||
correct one. Every sweep so far has had agents correctly refuse brief items (buttons in mutually
|
||
exclusive `@if` branches can't be grouped; toolbars the inventory over-counted), four of
|
||
ScadaBridge's seven batches did, and one there caught a brief instruction that would have failed
|
||
the test suite. Treat refusals as the mechanism working, not as friction.
|
||
- Merge foundation first, then batches; run the **full build + test suite on the merged tree** even
|
||
if every branch was green in isolation (parallel branches compose only at the full build).
|
||
- **Baseline any failure you intend to call pre-existing — measure it, don't assert it.** Stash to a
|
||
clean tree and run it more than once; a test that fails 2-in-3 on the untouched base is not your
|
||
regression, and you can only know that by running it. Related trap: **a suspiciously fast green
|
||
is not a pass.** HistorianGateway's `RedundantWritePathLiveTests` "passed in isolation" in 504 ms
|
||
while the real code path requires a live historian round trip — it was taking an early-return
|
||
guard and passing *vacuously*, and the full-suite run that failed was the honest one. This
|
||
inverts the usual reading of "passes in isolation." An unguarded environment-dependent test that
|
||
fails rather than skips on a dev box is a hygiene defect worth filing on its own, because it
|
||
trains people to ignore a red suite.
|
||
- Verification greps on the merged tree: zero remaining phantom-token usages, zero remaining
|
||
`·` action separators / inline `width:` hacks, `btn-group` count roughly matching the
|
||
inventory, the bundle link present.
|
||
|
||
### Live browser gate (mandatory in every app, bUnit or not)
|
||
|
||
bUnit cannot see computed styles, cascade order, or whether an element is actually reachable by a
|
||
pointer — which is precisely what this sweep changes. Two probes worth stealing:
|
||
**hit-testability**, not just visibility (`document.elementFromPoint` at a confirm button's centre
|
||
must return the button, not the backdrop — mxgw's modal gate), and **realistic-length injected
|
||
data** (ScadaBridge injected a 150-char OPC UA node id; HistorianGateway created a key with an
|
||
88-char display name, which rendered as 40 chars + ellipsis with Revoke still in the viewport).
|
||
|
||
**The recurring failure of this whole playbook is a property asserted about a set after checking
|
||
the convenient part of it.** It happened three times in one round, in three different guises, and
|
||
each time the claim was made in good faith by someone who had genuinely looked at *something*:
|
||
|
||
- "All five apps carry a local `.btn` block, so the bump is a cleanup" — inferred from the apps that
|
||
had one. HistorianGateway had none, making its bump a visible restyle.
|
||
- "The sweep is behavior-preserving" — asserted about a 76-file diff after reviewing the wrapper and
|
||
class changes it was *supposed* to contain. Four files carried an unrequested state-model refactor
|
||
that closes a modal when a background refresh drops its row.
|
||
- "mxgw is Bootstrap-free so the modal defect can't affect it" — an exemption from a family-wide gate,
|
||
granted on a property nobody re-derived. It was a Bootstrap host and *was* affected.
|
||
|
||
The shape is identical every time: a universal claim, a partial check, and a conclusion that happens
|
||
to be convenient. The fix is not more caution, it is a **cheap per-member check that actually runs**,
|
||
plus stating the claim's scope honestly ("verified in these three; not checked in the other two")
|
||
when it hasn't. All three were caught by someone re-examining their own earlier assertion, never by
|
||
the assertion being challenged.
|
||
|
||
**Enumerate untracked paths BEFORE staging, not after.** A sweep touches many files, so the landing
|
||
commit is where unrelated local state gets swept in — and `git status` counts mislead: one repo's
|
||
"78 modified" was 76 modified + 2 untracked, and one of the untracked entries was `.claude/`,
|
||
containing that session's `settings.local.json`. A literal `git add -A`/`git add .` following a
|
||
"commit the sweep" instruction would have put local permission config into a tracked commit feeding a
|
||
production deploy, invisible inside a 76-file diff. Run `git status --porcelain` and read the `??`
|
||
lines specifically; decide each one before staging anything. Two repos in this round had an exclusion
|
||
that had to be made deliberately (`.claude/`, and a `docker-compose.yml` carrying an undecided
|
||
`DisableLogin` override) — in both cases the exclusion was agreed before the commit existed, which is
|
||
the only point at which it is cheap.
|
||
|
||
**Audit the commit's CONTENT, not just its file list — they catch disjoint failures.** A filename
|
||
audit finds the wrong-file problem (`.claude/`, a stray `.db`, a `.pfx`). It cannot find the
|
||
right-file-wrong-content problem: a sweep whose live gate mints an API key, a token or a connection
|
||
string can paste it into a *legitimately tracked* plan doc, where a `--name-only` check sees a file
|
||
that is supposed to be there. So run both:
|
||
|
||
```bash
|
||
git diff --name-only <base>..<sha> # (a) wrong-file
|
||
git show <base>..<sha> | grep -inE '(api[_-]?key|password|secret|token|bearer|connectionstring)[\"'"'"' :=]+[A-Za-z0-9+/_-]{12,}'
|
||
git show <base>..<sha> | grep '^+' \
|
||
| grep -oE '[A-Fa-f0-9]{24,}|[A-Za-z0-9+/]{32,}={0,2}' | grep -v / # (c) unlabeled entropy
|
||
git show <base>..<sha> | grep -c 'BEGIN [A-Z ]*PRIVATE KEY\|BEGIN CERTIFICATE'
|
||
```
|
||
|
||
**(b) is keyword-anchored and therefore blind to unlabeled material** — a minted key id like a bare
|
||
32-hex string has no keyword near it and will not match; verified. Hence (c), and hence the PEM
|
||
check, since `-----BEGIN` is labeled but by none of those six words. The prefix list is also
|
||
**repo-specific and goes stale** (`otua_` is OtOpcUa's, `histgw_` HistorianGateway's) — extend it
|
||
per repo rather than treating the line as fixed.
|
||
|
||
**The reliable check is none of the above: if your verification gate MINTED anything — an API key,
|
||
a token, a test account — grep the commit for the literal values it produced.** Generic patterns
|
||
are a net for what you did not anticipate; the literal grep is the check for what you *did*, and
|
||
only it is dependable, because you know the exact string and nobody else does. A clean generic scan
|
||
is weak evidence when a gate minted credentials that the scan structurally cannot see.
|
||
|
||
Verified 2026-08-11 across this family: **`.claude/` is gitignored in OtOpcUa (`.gitignore:32`) and
|
||
HistorianGateway (`.gitignore:51`), but NOT in MxAccessGateway or ScadaBridge.** In those two the
|
||
enumerate-first discipline is the *only* thing between a "commit the sweep" instruction and local
|
||
permission config in a production commit — and ScadaBridge is exactly the repo where it nearly
|
||
happened. Adding the ignore line to the two that lack it converts a discipline into a guarantee.
|
||
|
||
**When every hypothesis is falsified, test the SHARED PREMISE before adding a new one.** Two
|
||
investigations into a 0-byte log file eliminated, between them, config levels, env vars in five
|
||
scopes, ACLs, stale files and a two-stage bootstrap — and deadlocked. The unexamined premise both
|
||
sides had inherited was *"the file exists, therefore the sink was constructed, therefore no events
|
||
flowed."* A ten-minute local experiment against the pinned sink version showed it creates **no file
|
||
at construction and no file when events are filtered** — the file appears only on the first emitted
|
||
event. So the file's existence proved the opposite of what both investigations assumed, and every
|
||
hypothesis either side had ranked was answering the wrong question. Nobody had checked which
|
||
behaviours actually produce the observed artifact.
|
||
|
||
**Eliminate at the right granularity.** The same investigation "killed" the write-failure hypothesis
|
||
by observing a *second* log file writing bytes in the *same directory* — which proves the DIRECTORY
|
||
is writable, not that THIS file HANDLE is. Separate sinks hold separate handles and fail separately.
|
||
A proof about the container is not a proof about the member, and that substitution is invisible
|
||
because the evidence is real and the reasoning feels airtight. Ask explicitly: *does my evidence
|
||
range over exactly the thing I am ruling out, or over something that contains it?*
|
||
|
||
**Re-read the comments inside your own diff — a change that invalidates prose usually ships the
|
||
prose.** A commit that removed a modal's self-closing behaviour left two comments in the same file,
|
||
in the same commit, still describing the modal as self-closing. The code and its explanation
|
||
contradicted each other inside one change, and the diff read as coherent because a reviewer checks
|
||
whether the *code* is right, not whether the surrounding sentences survived it. Grep your own diff
|
||
for comments in the touched regions and ask what each one asserts. This is the most common way a
|
||
correct fix ships a false claim, and it is invisible afterwards: the next reader trusts the comment.
|
||
|
||
**When a modal misbehaves on refresh, the property to check is whether it holds any state of its
|
||
own.** A modal that is a *pure projection of a list* cannot distinguish "paged out of view" from
|
||
"deleted", so any list change is indistinguishable from the record vanishing. That — not the presence
|
||
of a particular field — is the test. In one sweep, four detail surfaces split exactly on it: the two
|
||
that fetch detail by id survive a page change with usable content, the two that project from the list
|
||
are page-scoped whether or not anyone intended them to be, and only the latter two need their held id
|
||
cleared on paging. Checking for the *field* would have found the same two here by luck; checking for
|
||
*state of its own* is what generalises.
|
||
|
||
**A "what I could not establish" section confers unearned credibility on everything above it.**
|
||
Listing your limitations is good practice and this playbook demands it — but be aware of what it does
|
||
to a reader: the presence of a limitations section is taken as evidence that the rest was held to
|
||
that same standard. It usually wasn't. A real example from this round: a `pending.md` entry recorded
|
||
"do not fix this service" on evidence that actually supported only the narrower "a real shutdown does
|
||
not drop events," and its own Not-established section listed two statements that **cannot both be
|
||
true** (the executing task is awaited on stop; the loop never ran before stop returned) without
|
||
flagging the contradiction. Adjacent bullets under an honest heading read as thoroughness and
|
||
function as a dead end for whoever picks it up.
|
||
|
||
Two habits follow. **State what each piece of evidence licenses, not just what you believe** — if the
|
||
evidence covers a narrower claim than the one you are writing, write the narrower one. And **when two
|
||
items in your own limitations list conflict, say so explicitly**; a contradiction is a lead, and
|
||
burying it in a list of unknowns converts the most informative thing you have into noise.
|
||
|
||
**A correction is not automatically safer than the claim it replaces.** The first item above was
|
||
corrected to "HistorianGateway has no block, so the bump restyles it" — which was *also* wrong, for a
|
||
new reason: it had no Bootstrap button call sites either, so the kit's rule matches nothing. The
|
||
correction inherited the original's flaw (one grep, one inference) while feeling rigorous because it
|
||
was a retraction. When you correct a set-wide claim, the replacement needs the *same* evidentiary bar
|
||
you now know the original lacked — and a wrong correction is worse than the wrong claim, because it
|
||
arrives with the authority of a self-caught mistake.
|
||
|
||
**Record what you could not exercise rather than rounding it up.** HistorianGateway proved 2 of its
|
||
4 truncation sites live and said so: `sf.Error` needs a genuine drain failure, which needs a
|
||
historian that accepts a connection and *then* rejects the write, and a box with nothing listening
|
||
dies at setup and never enqueues. That is a rig capability ceiling — don't manufacture it, and
|
||
don't claim the sites you couldn't reach.
|
||
|
||
Rebuild/redeploy the app's dev rig (**all** instances behind the load balancer — round-robin serves
|
||
you the stale one otherwise), then **hard-reload** and walk the checks below.
|
||
|
||
**The cache bypass is not optional, and skipping it inverts the result.** After a kit-CSS change
|
||
(a `ZB.MOM.WW.Theme` bump, a new rule in `layout.css`), a normal reload can serve the *previous*
|
||
version of `layout.css` from cache while the server is serving the new one correctly. The symptom
|
||
is precisely the signature of a failed change — HistorianGateway saw `.btn` computing at
|
||
Bootstrap's 16px, exactly what an unreached override looks like — so the natural next move is to
|
||
"fix" it by restoring the local rule you just deleted, entrenching the duplication the change
|
||
removed. Hard-reload first; only then trust a computed-style probe.
|
||
|
||
**A hard reload is not sufficient, and there are three caches, not one.** HistorianGateway hit a
|
||
second browser-side failure that survived the hard reload entirely: after running CSSOM experiments
|
||
on a page (deleting rules to preview a modifier), the tab's **DOM** kept the pre-experiment markup —
|
||
no `rail-btn-block` class on the element — while `curl` showed the server rendering
|
||
`class="rail-btn rail-btn-block"` correctly. It came within one report of calling a working fix
|
||
broken. A fresh tab settled it. There is also a third, non-browser cache: **NuGet's HTTP cache**,
|
||
which made `dotnet add package` resolve a just-published version as "not found, nearest is <prior>"
|
||
while the feed's registration index already listed it (`dotnet nuget locals http-cache --clear`).
|
||
|
||
So the rule is stronger than "hard-reload": **confirm against the server's own output before
|
||
believing the browser.** One `curl` of the page and one of the stylesheet distinguishes all three —
|
||
stale CSS, stale DOM, and a genuinely broken change — and **none of them are distinguishable from
|
||
each other in the viewport.** Every one of the three presents as "the change didn't work," and the
|
||
tempting response to each is to revert a change that was correct.
|
||
|
||
1. Computed-style probes (one `javascript` eval): a `.btn`'s `font-size` vs body; the phantom-token
|
||
replacement class resolving to real colors; the scoped bundle present in `document.styleSheets`.
|
||
**Measure behaviour rather than reading a computed value when the two can disagree.** Chrome
|
||
serializes `-webkit-box` as `display: flow-root`, so a working line-clamp *reads* like a silent
|
||
failure; ScadaBridge measured instead (40px clamped vs 500px unclamped) and confirmed it works.
|
||
Reading the value alone would have produced a "fix" for something that was never broken.
|
||
If the app has a second colour scheme, probe **both** — flip it and confirm the accent tokens
|
||
move together (ScadaBridge: `rgb(47,95,208)` → `rgb(77,127,232)`, with `--accent-deep` following
|
||
instead of desyncing).
|
||
2. Every page leads with data — no unconditional documentation above the fold.
|
||
3. Seamed `btn-group`s where the inventory says; destructive members red.
|
||
4. One arm→confirm flow per style: arm → confirm UI appears → **Cancel** (never complete a real
|
||
write on shared rigs); verify disarm.
|
||
5. Anything the scoped bundle resurrected (context menus float at the cursor, backdrops cover).
|
||
6. Legends/hints/empty states from the KEEP list still render.
|
||
7. **Density, with realistic data**: at least one faulted/erroring row on screen (a healthy rig
|
||
renders "—" in every free-text cell and hides the unbounded-column defect entirely); sections
|
||
read as separate cards; no horizontal scroll; any detail dialog opens with live data and closes
|
||
cleanly.
|
||
|
||
Record PASS/FAIL per check in the plan or a recipe doc; CSS-only fixes may land in-session, anything
|
||
structural gets recorded instead.
|