Skip to content

fix(desktop): align agent message metadata - #6863

Open
matt2e wants to merge 3 commits into
mainfrom
jitter/fix-managed-by-baseline
Open

fix(desktop): align agent message metadata#6863
matt2e wants to merge 3 commits into
mainfrom
jitter/fix-managed-by-baseline

Conversation

@matt2e

@matt2e matt2e commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • use the responsive timestamp typography for agent-owner attribution so the author, managed by, owner, and timestamp share a baseline at every message font size
  • remove the icon's downward transform and baseline-align the Inbox message-header variant
  • cover Timeline and Inbox independently at Smaller, Default, and Larger after explicitly loading and verifying the rendered Inter Variable font

Testing

  • pnpm --dir desktop test (5,556 passed)
  • pnpm --dir desktop check
  • pnpm --dir desktop build:e2e
  • exact-build Playwright geometry tests repeated across 40 clean browser launches (20/20 Timeline and 20/20 Inbox)
  • mutation check: reverting only the Inbox header's outer baseline rule fails specifically at Inbox managedBy baseline at smaller font size by 1px

Before / After

Agent message header (alice · managed by bob · timestamp) captured from the mock-bridge E2E build at 3× so per-pixel baseline drift is legible. BEFORE is origin/main (e8172b5, the PR's merge base); AFTER is the production state introduced at 235bae0cc. Current head 348cf68cc changes only the regression test, so the rendered production state below is unchanged. Each pair covers both surfaces the change touches — the timeline row and the Inbox detail header — at all three message font sizes. The thin 50%-alpha cyan guide follows the alice text baseline in each panel.

Timeline

Font size Before → After
Smaller Timeline — smaller
Default Timeline — default
Larger Timeline — larger

Inbox

Font size Before → After
Smaller Inbox — smaller
Default Inbox — default
Larger Inbox — larger

In each BEFORE frame the bot glyph rides above the name's baseline and the managed by/timestamp run sits slightly high; AFTER, the icon is centered on the metadata line box and the author, managed by, owner, and timestamp share one baseline.

Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e
matt2e requested a review from a team as a code owner August 26, 2026 06:38

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Reviewed: e8172b5baccd0f4a711f5e19a3bc4313708f1c78..5b6e3737afd680d563de3793eb4871669d09281d (exact head 5b6e3737afd680d563de3793eb4871669d09281d)

Risk: medium — this is a bounded shared metadata/layout change across timeline, system-message, and Inbox renderers, with no relay, IPC, persistence, or identity-contract change.

Blocking finding

  • [P2] The regression test does not cover the claimed Inbox fix and is not deterministic — desktop/tests/e2e/messaging.spec.ts:393-469. The PR changes the Inbox header alignment at desktop/src/features/home/ui/InboxMessageRow.tsx:212,239, but the new geometry loop opens channel-general and measures timeline [data-testid=message-row] / [data-testid=message-timestamp]. It never renders InboxMessageRow or measures [data-testid=inbox-message-timestamp]. Scoped search under desktop/tests/e2e found no Inbox agent-owner geometry assertion. Reverting either Inbox items-baseline rule therefore leaves the committed test green by construction, so one of the PR's stated user-visible fixes is unprotected.

    The focused exact-head assertion is also unstable: clean, freshly built runs alternated between passing and failing at Smaller because the measured icon-center offset was 1.385040283203125px against a <0.01px threshold (messaging.spec.ts:443-467). Independent visible checks found the rendered states sound, which narrows this to the test/layout-readiness contract rather than an established production rendering defect—but a regression gate that alternates red and green on identical source is not trustworthy.

Author action: seed/render an agent message in Inbox; assert author/owner/timestamp geometry and icon centering across Smaller/Default/Larger; wait explicitly for fonts and stable layout before measuring; and mutation-prove that reverting each changed Inbox alignment rule fails the Inbox test. Keep the timeline assertions for the shared component.

Verification owner: author for causal, deterministic regression coverage; CI for repository gates.

Validation

At clean exact head 5b6e3737afd680d563de3793eb4871669d09281d:

  • PASS — just desktop-ci (full Desktop frontend and Tauri package gate).
  • PASS — independent visible browser captures at Smaller/Default/Larger, 150% zoom, narrow layout, long localization-style names, owner-unavailable state, and accessibility reading order.
  • MIXED/FAIL — focused Playwright geometry test: clean exact-head runs alternated pass/fail at the Smaller icon-center assertion.
  • CAUSAL FAIL — reverting the Inbox alignment rule did not affect the only new geometry test because that test exercises the timeline renderer.
  • GitHub exact-head macOS build, relay E2E, integration E2E, release-candidate, and all four smoke shards were green when reviewed; Desktop Core remained in progress.

Manual/native evidence: browser-visible renderer evidence only; native Tauri capture was not run. This is a confidence gap, not additional author action.

Residual risk: the production CSS appears visually correct in exercised renderer states, but the Inbox-specific contract lacks causal coverage and the new geometry gate can report false failures until layout readiness is stabilized.

Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e
matt2e requested a review from jedwards27 August 26, 2026 08:02

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Reviewed: e8172b5baccd0f4a711f5e19a3bc4313708f1c78..235bae0cc2f84b8704db21f1ff1ef908edabddb4 (exact head 235bae0cc2f84b8704db21f1ff1ef908edabddb4)

Risk: medium — user-visible shared metadata geometry and its Desktop regression gate.

Behavior/contracts traced: shared MessageAgentOwner composition in timeline, Inbox, and system rows; Inbox feed-item seeding/rendering; Smaller/Default/Larger geometry assertions; font/layout readiness; browser accessibility reading order; CI selection.

Blocking finding (P2): nondeterministic regression gate. desktop/tests/e2e/messaging.spec.ts:105-230 treats document.fonts.ready plus three identical animation-frame rectangles as proof that the intended webfont/layout is ready. That condition is insufficient. From a clean exact-head E2E build, the focused test passed five isolated launches, but a clean --repeat-each=20 run then passed only two repetitions before repeatedly failing the timeline Smaller icon-center assertion with 1.385040283203125px against the <=0.25px contract (repeats 4–15; later server/harness fallout excluded). Identical source/build bytes therefore produce both green and false-red outcomes.

This also prevents the combined timeline→Inbox test from causally protecting the Inbox fix: reverting only InboxMessageRow.tsx:212 from items-baseline to items-center failed first in the unchanged timeline assertion, so Inbox was never reached. An independently isolated Inbox mutation does fail the Inbox baseline assertion, confirming the production rule matters, but the shipped combined gate can mask that protection behind its flaky first surface.

Author action: make readiness discriminate the intended loaded/rendered font and prove repeated clean launches remain green. Split timeline and Inbox into independent tests (or otherwise ensure one surface cannot prevent the other's assertions), then demonstrate that reverting the Inbox outer baseline rule fails specifically at an Inbox assertion.

Verification owner: author for deterministic, causal regression coverage; :bot: Jude’s code review agent for fresh exact-head rerun; CI for required repository gates.

Validation at matching HEAD:

  • PASS: clean pnpm --dir desktop install --frozen-lockfile and pnpm --dir desktop build:e2e.
  • PASS: focused geometry test 5 isolated launches in one reviewer lane; PASS 10/10 in an independent lane after a fresh rebuild.
  • FAIL: clean focused --repeat-each=20 — 2 passed; repeats 4–15 reproducibly failed the Smaller timeline icon-center assertion at 1.385040283203125px; later failures excluded as harness/server fallout.
  • PASS: independently isolated Inbox outer-baseline mutation failed causally by 1px at the Inbox Smaller managed-by baseline.
  • PASS: pnpm --dir desktop test — 5,556/5,556 in the independent UI lane.
  • PASS observed at review time: macOS build, Desktop release candidate, relay E2E, and both Desktop integration shards. Remaining Desktop Core/smoke checks were still in progress and do not supersede the reproduced flake.

Manual/browser evidence: browser-rendered timeline and Inbox were exercised at 520px, dark theme, keyboard zoom, and all three font sizes. Accessibility snapshots preserved author → Agent managed by → owner → timestamp → body; decorative icon/middot stayed hidden. Six supplied before/after artifacts were inspected. No native Tauri/WKWebView journey was run.

Non-blocking residual risk: the shared non-membership system-row composition lacks equivalent geometry coverage; localized long-name wrapping and native Tauri AX remain unwitnessed. These are confidence gaps, not additional author-actionable defects.

Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e
matt2e requested a review from jedwards27 August 26, 2026 08:30

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Reviewed: e8172b5baccd0f4a711f5e19a3bc4313708f1c78..348cf68cc8968c5c67d39f1671ec886db91d3237 (exact head 348cf68cc8968c5c67d39f1671ec886db91d3237)

Risk: medium — user-visible shared metadata geometry and its Desktop regression gate.

Behavior/contracts traced: shared MessageAgentOwner composition in timeline, Inbox, and non-membership system rows; Inbox feed-item seeding/rendering; Smaller/Default/Larger geometry; intended-font readiness; independent surface causality; wrapping/zoom/browser accessibility; CI selection. No persistence, relay, IPC, identity, or native contract changed.

Findings: no unresolved author-actionable defect. The prior blocker is resolved:

  • desktop/tests/e2e/messaging.spec.ts:77-153 now verifies computed Inter Variable, explicitly loads each rendered style/weight/size, checks returned FontFace family/status plus document.fonts.check(), waits for document.fonts.ready, and only then samples stable geometry.
  • Timeline and Inbox are independent tests at desktop/tests/e2e/messaging.spec.ts:638 and :665, so one surface cannot mask the other.
  • Independent exact-head repetition passed 40/40 split tests (20 timeline + 20 Inbox), with the prior 1.385040283203125px Smaller failure absent.
  • Causal mutations died on their own surfaces: the timeline alignment mutation failed the timeline Smaller managed-by baseline while Inbox passed; the Inbox alignment mutation failed the Inbox Smaller geometry while timeline passed independently. A separate icon mutation consistently failed timeline icon-center geometry.

Author action: none.

Verification owner: CI for required repository gates; native reviewer only if additional WKWebView/macOS AX confidence is desired.

Validation at matching HEAD:

  • PASS: frozen install and fresh pnpm --dir desktop build:e2e in independent lanes.
  • PASS: canonical split geometry tests --repeat-each=20 — 40/40 in the product/UI lane.
  • PASS: an additional lane reached 22 geometry passes before its local Python server terminated; subsequent ERR_CONNECTION_REFUSED failures were harness/server termination, not geometry failures.
  • PASS: isolated timeline, Inbox, and icon mutation probes produced causal failures, followed by a clean exact-head rebuild and passing canonical tests.
  • PASS on prior production-equivalent head and unchanged production UI: full Desktop frontend suite, 5,556/5,556. This changed head modifies only the regression oracle.
  • PASS observed at submission: DCO, dead-token guard, release candidate, macOS build, relay E2E, and both Desktop integration shards. Desktop Core/smoke remained in progress; those external gates retain merge ownership.

Manual/browser evidence: narrow 760px, 125% zoom, long Japanese-style author text, owner-unavailable wrapping, and all three font sizes passed on timeline and Inbox. Browser semantic snapshots preserved author → agent/owner → timestamp/body order; decorative bot icon and separator stayed hidden.

Residual risk: native Tauri/WKWebView/macOS AX and system-row-specific geometry were not independently rendered. A trustworthy persisted dark-theme comparison was not produced. These are bounded confidence gaps, not code defects; production theme/UI code is unchanged by the final test-only commit.

@Chessing234 Chessing234 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the testing here is unusually thorough — waiting on document.fonts.load for the exact computed Inter Variable face before measuring, 40 clean launches, and a mutation check that names the test the reverted rule breaks. that's the right way to make a 1px geometry assertion trustworthy, and i'd be happy to see more of it.

my concern is the hunk the tests can't see.

MessageAgentOwner drops translate-y-[0.125em] from the Bot icon, and with it a comment that was specifically about something baselines don't measure:

self-center keeps the icon out of baseline alignment... Centred on the line box the glyph's ink still rides ~1.6px above the text's cap band, reading as a couple of pixels too high; 0.125em drops its optical centre onto that band.

self-center is still there, so the icon is still centred on the line box — the transform that corrected for the glyph's ink sitting high inside that box is what's gone. every new assertion is about the baselines of message-author, message-agent-owner and the timestamp, i.e. the text. the icon's optical position relative to the cap band is exactly the property that isn't asserted, before or after, so the suite would stay green whether or not the icon reads high again.

so: was the earlier reasoning wrong, or is it superseded by the switch to text-message-timestamp? if the line-height change moved the line box such that the correction is no longer needed, that's a great answer and worth one sentence in the pr, because the deleted comment is quite emphatic and the next person to notice a high-looking icon will re-derive it. if it's "we looked and it's fine now", a screenshot of the chip at all three sizes would settle it — the before/after images in the description are the natural place.

the other two hunks look right to me. items-centeritems-baseline on the Inbox message-header is the same rule the Timeline header already used, and swapping text-xs leading-4 for text-message-timestamp is what makes the owner chip track the timestamp at every font size instead of at one.

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.

3 participants