Skip to content

fix(console): draw resource IDs without ligatures - #3019

Merged
freeqaz merged 2 commits into
mainfrom
flake-2114
Oct 20, 2026
Merged

freeqaz merged 2 commits into
mainfrom
flake-2114

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

tests/browser/console-agents-list-return.test.mjs "On a phone the Agents list stacks each row..." failed about 1 run in 13 at 2 CPUs on a dev host, always with 360: row N ID fits one line (finding 2114).

Cause: not timing. The host's fontconfig monospace is Nimbus Mono PS, which has ligatures, and Chromium applies them by default, so ff in an Agent ID is drawn as one glyph. A 40-character Agent ID is 288 px in the 279 px ID line of a 360 px stacked row, so it wraps after a hyphen. An ID with two ff pairs drew 2 characters narrower (273.6 px), fit one line, and failed the check. Agent IDs are random UUIDs, so whether a run hit this was random. A 4x CPU throttle did not change the rate.

It is a product issue as well as a flake: an ID should read character by character, and a ligature merges two characters into one glyph. On this host agt_ffffffff-ffff-4fff-bfff-ffffffffffff drew 187 px wide against 288 px for an ID with no ff.

Change

  • console.css: .resource-id sets font-variant-ligatures: none. This covers every resource ID in the console (Agents list, Agent page, shell, access bindings).
  • The phone-layout test also checks, at every width, that an ID with ff draws as wide as one without. It measures a probe span in the row's ID style.
  • Storybook: new story Pages/Agents → Agent ID with ff (phone) (agentsIdLigaturesPhone) lists an Agent whose ID is agt_ffffffff-ffff-4fff-bfff-ffffffffffff. The fixture Agent takes an optional scenario agentId. The story's description says how to see the difference: view it where the monospace font has ligatures.

Verification

All runs below use taskset to pin each run to 2 CPUs, 4 runs at a time.

Head Runs Failures
main 75ebcc2 80 6 (360: row 1/2/3/4/6/8 ID fits one line)
main, 4x CPU throttle (CDP) 40 2 (same failure)
this branch 40 0
this branch, 4x CPU throttle 40 0

UI media (simulated Storybook fixture, this host's Nimbus Mono PS)

Story Pages/Agents → Agent ID with ff (phone). Before uses main's console.css and after uses this head (033fe95); both render the same fixture from prepare-assets.mjs output. Before, the ID is 187.2 px on one line at 360 px. After, it is 288.0 px and wraps after a hyphen. At 390 px it is 288.0 px on one line.

Before After
360 px before-360-row after-360-row
390 px before-390-row after-390-row

Full 360 px page after: after-360

Walkthrough at 360 px: scroll the list, then select the ID (after-360-select.webm).

Not verified: the Storybook manager build itself (CI's Console Storybook workflow builds it), and fonts other than Nimbus Mono PS.

Risks

Low. This is one CSS property on monospace IDs, and it changes rendering only where the monospace font has ligatures. With a font that has no ff ligature, such as the usual CI fonts, the new assertion passes on main too. It guards the regression only on hosts like this one, and the visual change shows only there.

A monospace font with ligatures (Nimbus Mono PS, the fontconfig default on some
Linux hosts) drew "ff" in an ID as one glyph. An Agent ID with two "ff" pairs was
then 14.4 px narrower and fit one line in the 360 px stacked Agents list, so the
phone-layout test failed about 1 run in 13 on such a host (finding 2114). IDs
read character by character, so .resource-id turns ligatures off, and the test
checks that an ID with "ff" draws as wide as one without.
@freeqaz
freeqaz requested a review from a team as a code owner October 20, 2026 07:46
@clawsweeper

clawsweeper Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 20, 2026
@clawsweeper

clawsweeper Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR disables ligatures for console resource IDs and adds browser coverage and a matching Storybook scenario.

Example: View agt_ffffffff-ffff-4fff-bfff-ffffffffffff at 360 px using Nimbus Mono PS.

  • Before: Repeated f characters merge into narrower glyphs and the ID fits on one line.
  • After: Each character retains its monospace width and the ID wraps after a hyphen.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch is focused and the visual comparison supports the repair; repository-required native media publication remains incomplete.
Proof confidence 🦐 gold shrimp (3/6) Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Some monospace fonts merge adjacent resource-ID characters, making IDs harder to read and their phone wrapping dependent on UUID contents.
Reason: The maintainer-authored direction repairs demonstrated identifier rendering with one shared CSS property and preserves existing resource behavior.

Merge readiness

✅ Ready for maintainer review

This PR remains useful: current main still permits ligatures in resource IDs, and the focused repair introduces no identified code defect; the previous Storybook finding is resolved.

Priority: P3
Reviewed head: 033fe958739c1347c628a380b176b25522f0cef5

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: Publish the existing final-head screenshots and selection walkthrough as native GitHub attachments to satisfy the repository's UI evidence requirement.
Agent review details

How this fits together

The controller serves console modules that render API resource IDs with the shared resource-id CSS class; the browser turns those strings into selectable text.

flowchart TB
  A[Resource API responses] --> B[Console renderers]
  B --> C[Shared resource ID style]
  C --> D[Browser font shaping]
  D --> E[Readable selectable IDs]
Loading

Technical review

Best possible solution:

Keep the rendering fix in the existing shared resource-ID rule and retain the focused browser and Storybook coverage.

Do we have a high-confidence way to reproduce the issue?

Current main leaves ligatures enabled for monospace resource IDs, and the supplied before screenshots demonstrate merged f glyphs; the author also reports base failure and head success on Nimbus Mono PS, but no target code was run during this review.

Is this the best way to solve the issue?

Disabling ligatures at the shared ID style fixes the rendering cause across existing consumers without changing identifiers, font selection, or wrapping rules.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 6aedac546aad.

Provenance checked

  • Shared resource-ID rendering changes intended behavior with a stated reason (3474bb7: The original console presents resource IDs as selectable monospace text; its linked PR records no requirement to retain ligatures.)
  • Phone Agents layout regression coverage keeps the original intent (24ea793: The responsive Agents work deliberately checks ID wrapping at separators and requires wrapping at 360 px; this PR preserves those checks while correcting font-dependent width.)
  • Storybook Agent fixture keeps the original intent (f2b1431: Storybook provides isolated production-console previews against simulated API responses; the optional fixture ID preserves the existing default.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Likely related people:

  • Free Wortley: Raw commit 24ea793 adds tests/browser/console-agents-list-return.test.mjs:682 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 24ea7934df16; files: tests/browser/console-agents-list-return.test.mjs)

Review metrics

Metric Value Why it matters
Production and regression coverage Production +3 lines; browser test +22 lines One shared CSS property repairs rendering, while the existing browser case gains a distinct font-dependent width assertion.

Labels

Label changes:

  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P3: This is a limited font-dependent readability and test-reliability repair without blocked resource operations.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Attach the existing final-head screenshots and short selection walkthrough directly to the PR's Verification section.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (1 earlier review cycle)
  • reviewed 2026-10-20T08:13:29.559Z sha 1b86fcb :: needs changes before merge. :: [P3] Extend the Agents Storybook scenario for ligature-prone IDs

Reviewed October 20, 2026, 4:22 AM ET / 08:22 UTC (Revision 2).

Pages/Agents "Agent ID with ff (phone)" lists an Agent whose ID is
agt_ffffffff-ffff-4fff-bfff-ffffffffffff, so the ligature fix can be inspected:
where the monospace font has ligatures (Nimbus Mono PS), each "ff" used to draw
as one glyph. The fixture Agent takes an optional scenario agentId.
@freeqaz

freeqaz commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. labels Oct 20, 2026
@freeqaz
freeqaz merged commit 59d9934 into main Oct 20, 2026
73 checks passed
@freeqaz
freeqaz deleted the flake-2114 branch October 20, 2026 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant