Skip to content

fix(ui): trim the boot comments after the composeBoot extraction (RIG-2874) - #1104

Merged
trunk-io[bot] merged 1 commit into
compass-ui/rig-2874-record-direct-seamfrom
compass-ui/rig-2874-comment-trim
Sep 11, 2026
Merged

fix(ui): trim the boot comments after the composeBoot extraction (RIG-2874)#1104
trunk-io[bot] merged 1 commit into
compass-ui/rig-2874-record-direct-seamfrom
compass-ui/rig-2874-comment-trim

Conversation

@rigel-mintaka

@rigel-mintaka rigel-mintaka commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Review fixes for #1096, landed additively on top rather than amended into it
(pushed commits are immutable here — immutable_heads() includes
remote_bookmarks() — and review fixes are additive by house rule, so an
interdiff stays visible).

What the review found

Medium — the index.tsx comment block went stale on extraction. #1096
moved the analytics/clients construction into composeBoot, but left the
~30-line block that narrates it in main(). After the move there is no getter
literal and no two ordered statements at that location, so the sentence

`clients` is a forward reference from inside this getter, which is safe
because the getter only runs once an event is captured — long after the
next statement binds it.

described code that is no longer there: the "next statement" is now a single
destructuring call. It also duplicated the same forward-reference invariant
already carried in compose-boot.ts, so changing the order in one file would
silently falsify the other.

Trimmed to the call-site facts a reader needs there (analytics off by default;
correlation best-effort in both directions) with a one-line pointer to
composeBoot for the construction order. 30 lines → 6. compose-boot.ts is
now the single owner of the forward-reference explanation.

Low — two module headers over the comment ceiling. The house ceiling is 4
physical lines; compose-boot.ts:9-14 and compose-boot.test.ts:14-19 were
both 6. Each cut to 3, keeping the why (a wrong order fails only at the
network door, so injectable factories are the only way to pin it) and dropping
the restated mechanics.

Not changed, and why

The review also flagged expect(built.analytics).toBe(analytics) /
expect(built.clients).toBe(clients) as near-padding, on the grounds that the
getter-resolution assertions already prove the wiring. Keeping both: they are
the only assertions that catch a swapped or dropped return, which the
getter assertions do not — those resolve through the captured closures and
pass even if composeBoot returned the wrong pair.

Gates

  • bun test compose-boot + analytics + live/client — 30 pass / 0 fail
  • Order inversion re-verified after the edits: inverting the two construction
    lines still gives 0 pass / 1 fail; restored file byte-identical (cmp)
  • bunx tsc --noEmit (apps/ui) — rc=0
  • bunx biome check (3 files) — rc=0

RIG-2874

…-2874)

The index.tsx block still narrated construction mechanics that moved into
composeBoot, and two module headers ran 6 lines against a 4-line ceiling.
@linear-code

linear-code Bot commented Sep 11, 2026

Copy link
Copy Markdown

RIG-2874

@github-actions

Copy link
Copy Markdown

Compass engineering docs preview: https://compass-ui-rig-2874-comment.compass-eng-docs.pages.dev

Deployed from compass-ui/rig-2874-comment-trim at d3a8db9.

@trunk-io
trunk-io Bot merged commit ca6cbbf into main Sep 11, 2026
16 checks passed
@trunk-io
trunk-io Bot deleted the compass-ui/rig-2874-comment-trim branch September 11, 2026 19:37
@trunk-io

trunk-io Bot commented Sep 11, 2026

Copy link
Copy Markdown

This pull request was merged into main as part of stacked PR 1105.

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.

2 participants