fix(ui): correct a stale boot-order comment in the analytics test (RIG-2874) - #1081
Merged
Merged
Conversation
…G-2874) T5 record hygiene. The test comment asserted that boot builds the transport before analytics, which the T4 reorder inverted, and it contradicted the matching comment on the traceId getter (analytics.ts:53). The test behaviour is unchanged and still correct: the getter picks up a trace id that arrives after createAnalytics. The module header (analytics.ts:1-16) needed no change — "built once at boot from the resolved AnalyticsConfig" is still accurate after the reorder. Co-authored-by: Matt Wilkinson <matt@rigel.build>
|
😎 Merged successfully - details. |
|
Compass engineering docs preview: https://compass-ui-rig-2874-t5-recor.compass-eng-docs.pages.dev Deployed from |
Review nit on #1081: the comment sat exactly on the four-line ceiling, and "the slot is still empty here" described the test local rather than the production sink it stands in for. Same fact, two lines, no conflation. Co-authored-by: Matt Wilkinson <matt@rigel.build>
mattwilkinsonn
approved these changes
Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the last task of RIG-2874's UI lane: T5 — record hygiene (design record
compass/docs/designs/ui/compass-outbound-session-header/design.md:746).The record's T5 was conditional — update the module-header prose if the T4 boot reorder changed what it asserts, and verify no other comment states the old order as a constraint. Both clauses resolved:
analytics.ts:1-16AnalyticsConfig" survives the reorder intactThe defect
analytics.test.ts:266-267asserted that boot builds the transport before analytics — precisely the order T4 inverted. It also contradicted the matching comment on thetraceIdgetter atanalytics.ts:53, so the file and its test disagreed in prose about the same invariant.Corrected to state the real order: analytics is built before the transport (
index.tsx:121createAnalytics→:125createLiveClients), so the trace-id slot is still empty at that point and the first id lands later.Why this was invisible to every gate
The test's behaviour was correct and green — a trace id arriving after
createAnalyticsis picked up, which is the regression this test exists to catch. Only its stated reason was stale. No typecheck, lint, or test run can fail on a wrong comment above a passing assertion, and review naturally skims the prose of a green test. That is the shape worth noting: the assertion never lied, the explanation did, and the explanation is what the next reader trusts when deciding whether a failure here is real.Change
Comment-only. One file, 3 insertions / 2 deletions. No behaviour change, no assertion changed, no production code touched.
Gates
bun test apps/ui/src/analytics/analytics.test.ts— 16 pass / 0 failbunx tsc --noEmit(apps/ui) — rc=0 (bun testdoes not typecheck, so this ran separately)bunx biome checkon the changed file — rc=0All three re-run after rebasing onto merged
main(8d26d723), since #1055 landing put the T1–T4 code underneath this commit.Sweep evidence
Confirming the fix is complete rather than pattern-local, with a live positive control so the zero is not a dead query:
analytics BEFORE the transport→ 2 sites (the corrected test comment + the getter doc atanalytics.ts:53) — the control firestransport … before … analytic) → 0 sitesProvenance
Authored on top of #1055 (RIG-2874 outbound session header, merged
76c85dae) rather than amended into it. #1055 was already approved, and on this repo an approval is not re-checked on a later push, so amending would have put an unreviewed head in front of the merge queue.Co-authored-by: Matt Wilkinson matt@rigel.build