Skip to content

build(#577): UI-complexity instrument + S0/S1/S2 evaluation evidence - #580

Open
BorisTyshkevich wants to merge 6 commits into
mainfrom
docs/preact-shell-evaluation-577
Open

build(#577): UI-complexity instrument + S0/S1/S2 evaluation evidence#580
BorisTyshkevich wants to merge 6 commits into
mainfrom
docs/preact-shell-evaluation-577

Conversation

@BorisTyshkevich

@BorisTyshkevich BorisTyshkevich commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

What & why

Part of #577 — the measurement instrument and the first two states of the
Preact evaluation. This PR reaches no recommendation and adds no Preact
dependency.
It is the evidence foundation; the decision needs S2 (below).

#577 asks whether migrating the UI composition layer to Preact would make the
app materially simpler. Its decision principle is that a completed migration must
reduce, not merely reorganize, production UI code and lifecycle complexity.
That makes the instrument load-bearing: with the wrong metric the report reaches a
confident conclusion the evidence does not support.

What merges here

  • build/ui-complexity-lib.mjs — pure counting rules (41 unit tests).
  • build/ui-complexity-report.mjs — the runner (fs + esbuild + environment capture).
  • build/ui-complexity-manifest.json — the committed, auditable file set.
  • tests/unit/ui-complexity-report.test.js — including a sabotage suite.
  • docs/design/577/{s0,s1}/ — pinned measurements for the baseline and control.
  • docs/design/577/manifest-v2/all three states re-measured in one run over
    one 18-entry superset manifest, plus the S2 esbuild per-input attribution.
  • docs/design/577/S2-EVIDENCE.md — what the Preact arm cost, and what broke.
  • CHANGELOG.md [Unreleased].

Dev tooling only — nothing from it enters dist/sql.html (build/ is not in the
bundle graph; verified by rebuilding to a byte-identical artifact).

Why not just count LOC

Two obvious instruments both lie about this repository:

  • Physical LOC. The five shell-plumbing modules total 1791 physical lines
    but 709 lines of code
    — 989 are comment-only, because this repo documents
    each invariant next to the review bug that found it. An early draft of the plan
    set 1791 as the bar for the Preact arm; any arm written at normal comment
    density beats that while containing more code.
  • Regex counts of manual DOM mutation. Run over raw source they match the
    prose inside those comments. And three of the four families Evaluate Preact migration as an application-wide simplification #577 names
    (hidden, dataset, replaceChildren) go syntactically invisible under a
    vDOM
    whether or not the number of presentation decisions changed — a component
    arm scores ~0 on them by construction.

So every count runs over esbuild-transformed source: comments stripped and
formatting normalized by the repo's own build tool, which also means an arm cannot
win by collapsing statements onto fewer lines. esbuild handles what a hand-rolled
scanner does not — // inside a string, \/\/ inside a regex literal, a
*-prefixed line inside a template literal, a trailing code; // note.

Three design points worth reviewing specifically:

  1. Metric tiering is emitted in the JSON (METRIC_TIERS). Two independent plan
    reviews found that ten unranked numbers let almost any outcome support either
    recommendation. Each explanatory metric carries the reason it may not be voted
    with, so a demoted metric cannot be quietly promoted when the report is written.
  2. One canonical manifest across every state (--manifest), with an absent
    file reported rather than skipped or thrown on. This is load-bearing: the two
    most decision-relevant facts are a file appearing (the control adds the
    inspector) and a file disappearing (the treatment deletes the vanilla
    shell). Per-state manifests would erase both. For the same reason the manifest
    test pins the path set instead of asserting filesystem existence — an existence
    check fails on exactly the states where absence is the result.
  3. nonCodeLines is deliberately not called "comment-only lines." esbuild
    re-prints the code, so physical - blank - normalized is not a comment total;
    it is non-blank lines carrying no code after normalization (comments plus
    formatting-only lines). Naming it a comment count would be a measurement error
    in the metric the whole decision rests on.

The tests include a sabotage suite and a structural guard that the runner
counts stripped code rather than raw source — the lib is pure by design, so that
wiring is otherwise unguarded and swapping it back would leave every other test
green. Verified falsifiable: sabotaging it turns the suite red.

Measured so far

Both states measured over the one canonical 11-file manifest, from clean trees,
with an identical instrument (it lands complete in S0 for that reason).

S0 baseline S1 control Δ
code lines (plumbing) 599 742 +143
code lines (domain) 341 341 0
code lines (island) 199 203 +4
total 1139 1286 +147
artifact raw B 2126521 2130619 +4098
artifact gzip B 623736 624764 +1028

The vanilla control costs +147 code lines and +1.0 KB gzip — what a
#488-shaped right inspector costs under the current architecture, and the number
the Preact treatment has to beat on the same feature.

What is deliberately NOT in this PR

The evaluation states themselves. They are pushed refs, never merged:

  • 577/baseline / eval-577/s0-baselined319847 (identical to this PR's base commit)
  • 577/control / eval-577/s1-control7e7c8c4 — the vanilla right inspector

S1 must not land on main: it adds a persisted preference and a shell feature
that #488 owns, and merging it would start the implementation #577 lists as a
non-goal. Reviewers who want to review the control itself should use
s0…s1
— a docs-only PR means that code is not in this diff, so please review it there
rather than assume it was reviewed. Under-engineering the control biases the
whole evaluation toward "adopt", so control quality is as much a review target as
treatment quality will be.

Still to come (no decision is reachable yet)

S2 (Preact owns shell presentation, app-shell.ts deleted), the arm-agnostic
parity suite in Chromium + WebKit, the controlled-change experiment, the
whole-slice JSX sensitivity annex, then ADR-0004 with the recommendation and the
whole-application projection. The full handoff, including the decision rule fixed
before measuring, is in the ship log.

Reconciliation of #578 / #487 / #488 is deferred on purpose — all three depend
on a recommendation that does not exist yet.

Findings from this work that change how S2 must be built

  • Four gates silently do not apply to .tsx (all verified): the coverage glob
    is src/**/*.{js,ts}; tsconfig.json:18 includes only src/**/*.ts with no
    jsx option; both resolver shims are .js.ts and build/e2e-serve.mjs
    type-strips with loader:'ts', which does not parse JSX; and
    typography-contract.test.js:345 matches /class:\s*'([^']+)'/g, which JSX
    class="…" never matches — so a JSX arm could ship unstyled classes green.
    Hence h() for the measured arm, with JSX priced separately. Also
    build/check-boundaries.mjs:191 hardcodes src/ui/app-shell.ts, so S2 must
    update that list when the file goes. "The gate held" is only evidence if the
    gate ran.
  • The artifact carries a build stamp (build/build.mjs:34-48, plus -dirty).
    Measure only from a clean tree at a committed SHA — a dirty tree adds exactly 6
    bytes — and note that raw size is stable across commits while gzip varies
    ~±10 B purely from the embedded SHA text
    (two S0 builds: identical raw
    2126521, gzip 623736 vs 623743). Gzip deltas under ~20 B are not signal. Not a
    defect; Release build isn't reproducible: no lockfile lets transitive deps drift between local and CI builds #157 was a different, closed lockfile issue.

Amended in the issue, with the owner's approval

Acceptance criterion 7 ("the prototype is removed") now reads "absent from
product branches and shipped artifacts"
, with archival evaluation refs retained
as evidence — rather than being reinterpreted unilaterally in the ADR. The
original wording would have discarded the evidence exactly as the lost
spike/preact-schema branch did: it is gone from origin, and ADR-0001's Preact
addendum is now its only trace.

Checklist

S2 — the Preact treatment arm, measured

The third state now exists: eval-577/s2-treatment, tag 577/treatment
(commit 3f611b5e). Preact owns shell presentation and lifecycle;
src/ui/app-shell.ts (905 lines), ui/left-rail.ts and ui/right-inspector.ts
are deleted. mountAppShell's public seam is unchanged, so ui/app.ts
changes only an import path — the comparison stays about rendering, not
re-plumbing.

That branch is not merged and is not in this diff. Review it at
s1…s2.
Only the evidence merges here.

S0 S1 control S2 treatment S1 → S2
code lines — plumbing 599 742 1072 +330
code lines — domain 341 341 341 0
code lines — island 199 203 203 0
total 1139 1286 1616 +330
artifact gzip B 623 736 624 764 632 519 +7 755
repo-owned bytes in bundle 631 633 635 169 +3 536

Domain and island lines are identical across all three states — the
preservation constraint holding, and the check that the deviation below cost the
treatment nothing it was owed. Exactly one @preact/signals-core copy, proven by
per-package attribution. manifest-v2/s2/esbuild-inputs.json proves the vanilla
shell is absent from the artifact rather than shipped alongside.

Gates

check:schemas, check:examples, check:arch, check:types, 7 052 unit
tests
with per-file coverage, the single-file build, and Chromium + WebKit e2e
(417 passed). The two remaining tile-open-workbench failures fail
identically on the frozen S1 control, so they are scored against neither arm
— but they are not pre-existing: origin/main passes all 21 specs in that
file, and the failures come from the control's own inspector defaulting to 420px
and narrowing the Dashboard grid. Filed as #581 (inbox) for #488, which builds
the real inspector on a shipped surface.

What the arm turned up

Nine costs the control does not pay, in full in S2-EVIDENCE.md. The four worth
reading here:

  1. DOM updates became asynchronous relative to state writes. Eleven
    app.test.ts assertions needed an explicit flush, and showHost needed a
    flushSync escape hatch — app.showQuerySurface() immediately focuses the
    SQL editor, and a deferred render left it unfocusable because its host was
    still hidden.
  2. A defect class no gate can see. Importing @preact/signals-core instead
    of @preact/signals leaves the reactivity bridge unwired: the shell renders
    once and never repaints, with tsc, check-boundaries and every pure-signal
    test green. Only rendering a real component caught it.
  3. Derived state has no edge. A computed reports that a value changed; it
    cannot report that a gesture ended. navMode reaches its final value
    mid-drag, exactly where focus capture is correctly gated off — so the commit
    produced no dependency change and a pointer drag-fold silently rescued no
    focus. It had to be re-attached by hand.
  4. Two regressions found only in a real browser, with 6 995 unit tests green
    a live width measurement silently became push-based, and an intent-gated focus
    restore did nothing because a drag drops focus to <body> mid-gesture. Both
    fixed.

Seven genuine improvements are recorded too, including one defensive guard that
becomes structurally unreachable under the derived model.

The flagged weak premise — "one owner per subtree deletes the focus-rescue
category" — is refuted.
Preact has no before-the-DOM-changes hook, so the work
survives in full; only its location moves, and cost 3 above shows the relocation
introduced a failure the control does not have.

Deviation from the plan, recorded

The ship log said to delete "the geometry half of left-nav-separator.ts". Not
done, deliberately
— the manifest classifies that file as an island because
its ARIA/paint glue "is not separable at file granularity", and splitting it
would have handed the treatment a reduction it did not earn.

Reading against the precommitted rule

The rule was fixed before any measurement: adopt only on clear dominance in
both net repo-owned shell code and lifecycle/focus obligations plus
controlled-change amplification; any mixed result is RETAIN. On code the arm
is +330 lines (+26%) and +3 536 repo-owned bytes — not a reduction, so
that half fails outright rather than mixing.

ADR-0004 still owns the recommendation, and is not written here: the parity
suite, the controlled-change experiment and the JSX sensitivity annex are
outstanding.

Review notes

BorisTyshkevich and others added 3 commits July 30, 2026 20:37
The instrument #577's Preact decision rests on, plus the committed manifest
that makes acceptance criterion 4 (domain logic vs framework-like rendering
plumbing) auditable rather than asserted.

Counts run over esbuild-transformed source, never raw text. Two naive
instruments both lie about this repository:

- Physical LOC. The five shell-plumbing modules total 1791 physical lines but
  ~709 lines of code; 989 are comment-only, because each invariant is
  documented next to the review bug that found it. Any arm written at normal
  comment density "wins" on physical LOC while containing more code.
- Regex counts of manual DOM mutation. Run over raw source they match the
  prose *inside* those comments — a first pass reported 37 "mutation sites" in
  app-shell.ts, and a comment-only file scores above zero. They also go
  syntactically invisible under a vDOM, so a component arm scores ~0 by
  construction.

esbuild does the stripping because it is the repo's only build tool and it
handles what a hand-rolled scanner does not: `//` inside a string, `\/\/`
inside a regex literal, a `*`-prefixed line inside a template literal, a
trailing `code; // note`. Re-printing also normalizes formatting, so an arm
cannot win by collapsing statements onto fewer lines.

ONE canonical manifest is measured against every evaluation state, via
`--manifest`, and a manifest entry whose file is absent from the current
checkout is reported as `absent` rather than skipped or thrown on. This is
load-bearing, not a convenience: the two most decision-relevant facts in the
whole comparison are a file APPEARING (the control adds the inspector) and a
file DISAPPEARING (the treatment deletes the vanilla shell). Measuring each
state against its own manifest would make both vanish from the comparison
instead of showing up in it. For the same reason the manifest test pins the
path set instead of asserting filesystem existence — an existence check fails
on exactly the states where absence is the result.

Metric tiering is emitted in the JSON (METRIC_TIERS). Two independent plan
reviews found that ten unranked numbers let almost any outcome support either
recommendation, so each explanatory metric carries the reason it may not be
voted with: minified bytes ignore tree shaking and complexity moved into a
dependency; lcov BRF/FNF fall when mechanisms move into Preact, which is
externalized rather than eliminated complexity.

`nonCodeLines` is deliberately NOT called "comment-only lines": esbuild
re-prints the code, so physical - blank - normalized is not a comment total.
It is source non-blank lines carrying no code after normalization — comments
plus formatting-only lines. Naming it a comment count would be a measurement
error in the metric the decision rests on.

Unmatched lcov records report as null, never as a zero record: "no coverage
data" and "no branches" are different claims.

Tests include a sabotage suite, and a structural guard asserting the runner
counts stripped code rather than raw source — the lib is pure by design, so
that wiring is otherwise unguarded and swapping it back would leave every
other test green. Verified falsifiable: sabotaging it turns the suite red.

Part of #577.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XLuZUHRbTCFKDJf2Zf8nP1
Derived evidence committed on the branch that will merge, not left only on the
evaluation refs. ADR-0001's Preact addendum is currently the ONLY trace of the
`spike/preact-schema` branch it cites — that branch is gone from `origin` — and
this is the mistake that loses. Numbers a reader cannot re-derive are not
reproducible measurements (acceptance criterion 5).

Both states measured over the ONE canonical 11-file manifest committed here
alongside them, from clean trees, with the instrument identical in both (it
lands complete in S0 for exactly that reason). Each report embeds its own
environment capture: resolved commit SHA (never a tag name — tags can be
moved), lockfile hash, Node and esbuild versions, platform, and a clean/dirty
flag.

Headline: the vanilla control costs +147 code lines (plumbing +143, island +4,
domain 0) and +4098 raw / +1028 gzip artifact bytes. That is what a #488-shaped
right inspector costs under the current architecture, and it is the number the
Preact treatment has to beat on the same feature.

The esbuild metafiles are deliberately NOT committed yet (~360 KB each). They
land with the final report, where the S2 metafile's per-input attribution is
what actually PROVES the vanilla shell was deleted rather than shipped
alongside.

Part of #577.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XLuZUHRbTCFKDJf2Zf8nP1
Part of #577.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XLuZUHRbTCFKDJf2Zf8nP1
All three evaluation states re-measured in one run from clean worktrees at their
tagged SHAs, over one 18-entry superset manifest — the seven src/ui/shell/*
files report absent for S0/S1, and app-shell.ts / left-rail.ts /
right-inspector.ts report absent for S2. A treatment measured over a manifest
omitting its own new files would report a deletion with no matching cost.

Headline: the Preact arm is +330 code lines and +7.8 KB gzip against the vanilla
control for the same feature. Domain and island lines are identical across all
three states (the preservation constraint holding). Repo-owned bytes in the
artifact went UP 3,536 B.

manifest-v2/s2/esbuild-inputs.json is the per-input proof that the vanilla shell
is absent from the built artifact rather than shipped alongside.

Also records a measurement correction: S1's brotli figure differs from the one
pinned earlier while its raw and gzip match exactly, because brotli output is
not stable across Node versions. Re-measuring all three states in one run is
what makes that column comparable.

No recommendation is made here — ADR-0004 owns it, and the parity suite, the
controlled-change experiment and the JSX annex are still outstanding.

Part of #577

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XLuZUHRbTCFKDJf2Zf8nP1
@BorisTyshkevich BorisTyshkevich changed the title build(#577): UI-complexity measurement instrument + S0/S1 evaluation evidence build(#577): UI-complexity instrument + S0/S1/S2 evaluation evidence Jul 30, 2026
BorisTyshkevich and others added 2 commits July 31, 2026 00:18
…ures

An earlier draft called two tile-open-workbench failures pre-existing. They are
not: origin/main passes all 21 specs in that file from a clean worktree, and
both evaluation arms fail the same 2. The cause is the S1 control's own
inspector, which defaults to inspectorPx 420 and narrows the Dashboard grid
enough to break two tile-geometry assertions; S2 inherits the pane and the
failures.

They still do not differentiate the arms, so they remain unscored — but they are
a side effect of the evaluation SLICE, which is worth recording, and #577's
'passes every behavioural gate' wording is not cleanly met by either arm because
of them. Filed as #581 (inbox) for #488.

Part of #577

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XLuZUHRbTCFKDJf2Zf8nP1
The evidence said "Preact offers no before-the-DOM-changes hook". That is true
for FUNCTION components — which is what the treatment arm uses — but overbroad
as a statement about Preact: preact/src/diff/index.js calls componentWillUpdate
(:203) and getSnapshotBeforeUpdate (:250) before diffChildren (:259), gated on
isClassComponent.

The weak premise is still refuted, and the corrected reason is stronger:
getSnapshotBeforeUpdate IS a capture hook, so having it relocates the
capture/restore protocol into a lifecycle method rather than deleting it. The
component model changes where that code lives, never whether it exists.

Also records two open points from the same review: the superset manifest proves
nothing was omitted but does not prove no smaller Preact design exists (the
controlled-change experiment is where that should be settled), and the pinned
numbers have no re-verification path because the evaluation branches sit outside
CI.

The frozen 577/treatment branch keeps the original phrasing in its own source
comment; the tag pins measured evidence, and this document is the corrected
record.

Part of #577

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XLuZUHRbTCFKDJf2Zf8nP1
@BorisTyshkevich

Copy link
Copy Markdown
Collaborator Author

Third-party review (ChatGPT) — what I verified vs. dismissed

Run against this PR plus docs/design/577/S2-EVIDENCE.md. ChatGPT was reachable
and working but did not finish generating inside the time budget, so this is
what it reached. Both points it did produce were verified against the repo before
being accepted, and one of them corrects a load-bearing claim of mine.

ACCEPTED — the lifecycle claim was overbroad. The evidence said "Preact
offers no before-the-DOM-changes hook". Verified at
node_modules/preact/src/diff/index.js: componentWillUpdate (:203) and
getSnapshotBeforeUpdate (:250) are called before diffChildren (:259),
gated on isClassComponent. So the claim holds for function components —
what this arm uses — but not for Preact as a library. Corrected in f749fd1.

The weak premise is still refuted, and the corrected reason is stronger:
getSnapshotBeforeUpdate is a capture hook, so having it relocates the
capture/restore protocol into a lifecycle method rather than deleting it. The
component model changes where that code lives, never whether it exists.

ACCEPTED AS OPEN — "counted fairly" is not "no smaller design exists". The
superset manifest proves nothing was omitted; it cannot prove the Preact arm
could not have been written smaller. File-by-file the seven src/ui/shell/*
modules have distinct, non-overlapping responsibilities and no padding — but
shell-host.ts carries 636 of the 1072 plumbing lines and deliberately keeps
nine bare effect() calls beside the component tree. Whether a
hooks-throughout arm would be smaller or merely differently shaped is not
settled by this evidence, and the controlled-change experiment is where it
should be settled rather than by assertion. Recorded as an open point.

ADDED FROM VERIFICATION, not from ChatGPT — the pinned numbers have no
re-verification path. The evaluation branches are pushed refs outside CI, so
nothing re-runs the instrument against them; a reviewer wanting to trust the
table has to re-measure by hand (the reproduction recipe in S2-EVIDENCE.md is
exactly that). Either a scheduled check or an explicit "pinned, not gated"
caveat belongs in ADR-0004.

Also worth weighting down: cost 3 (importing @preact/signals-core instead
of @preact/signals, leaving the reactivity bridge unwired) is arguably a
footgun rather than an architectural property — two near-identically-named
packages, one of which silently does nothing — and is preventable with a lint
rule. A fair reading should weight it below the structural costs. Noted in the
evidence document.

Not dismissed, not reached: ChatGPT never got to the questions on whether any
of the nine costs are implementation mistakes, or on what a reviewer would still
need. Those stay open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant