Sign up
View stargazersSign up

fix(web): PR links in expanded tables no longer split mid-number#24584

closed2
main
fix/issue-24147
opened2d agobyJJulius Marminge
1 files changed
View on GitHubTwo-way sync enabledPR last imported 2h ago

Problem

In an expanded Markdown table, explicit PR links such as [#363](https://github.com/example/repo/pull/363) could split the number into #36 and 3. Reproduced in the real web app with a wide chat lane and prose competing for table-column width.

Change

Only labels whose suffix contains a dot or slash receive per-character break opportunities. Short reference labels now use normal word boundaries, while URL-shaped labels retain their existing wrapping. This changes one shared web/desktop Markdown helper.

Scope and approval

Fixes https://github.com/pingdotgg/t3code/issues/24146

GitCafe: https://git.cafe/pingdotgg/t3code/issues/24147

Triaged bug via the automated issue flow; GitCafe mirror #24147. No duplicate found among open issues through the 90-day cutoff. Existing contributor #24163 is untouched.

Verification

  • Targeted formatting and lint passed for MarkdownExternalLink.tsx; web typecheck and git diff --check passed on final head.
  • Purely visual fix: no implementation-mirroring tests added. Two isolated code reviewers returned MERGE.
  • Real web dev-server reproduction showed split digits before; both roles, header/data cells, themes and wide/narrow samples were captured. Long URL wrapping, long ordinary labels, Files and attachment previews, table toggles/panning and the plan preview were checked.
  • Captured candidate d4d5ad61 against eea98953. Main was merged into final head e8058c32; the renderer, table CSS and ChatMarkdown paths are identical to the captured candidate. The final-head integrated browser smoke again showed each reference on one line: Final-head smoke

Evidence: fixture matrix, 19-pair index, recordings and public seed/driver files, 10-pair supplement and eight recordings, independent core proof audit. The core audit verified all original images/diffs/recordings and independently rebuilt the candidate geometry. The supplemental audit verified all additional captures, recordings, 20 diff maps, 20 unchanged repeats and 68 public media hashes. It confirms the material S4/C10/P5 and cached C11/P6 gaps are resolved, with no confirmed regression. The original full matrix remains incomplete for compact reasoning C4/P7 and reasoning round-trip R3; other secondary limitations are listed below. Its attempted final-head HTTP check occurred after the lead stopped the runtime; the lead’s completed final-head smoke is shown above.

Environment: T3 collaborative browser, real isolated web dev server, CSS viewports 1440×1000 and 390×1000, DPR 2, system fonts at default scale, light/dark primary samples. Desktop snapshots are downscaled by the preview to 1280×889; narrow snapshots are 780×2000. Loaded-font endpoint captures have pixel-identical repeats 500 ms apart. Motion was suppressed with CSS; native reduced-motion emulation was unavailable. Diff0 is absolute RGB difference amplified ×8; DiffT highlights max-channel differences >16/255. Supplemental pointer highlights/scrollbar visibility are explicitly annotated and are not renderer evidence.

What else changed: ordinary labels without a suffix dot/slash regain natural wrapping throughout shared Markdown. Long labels can widen a column, narrow adjacent prose and change row height; narrow paragraph/list labels can move to a fresh line. The primary wide table becomes 19.5 CSS px shorter. These visible effects are included in the captures.

Unverified: live PR retrieval failed with “Could not reach GitCafe”; the production PR renderer was exercised with explicitly synthetic local cached data, including generated reference/mention/commit controls. Compact thoughtLine/reasoning, italic inspector, event detail, pending-question and review/context-comment callers, live PR comments/media splits, loaded favicon/rich-image controls and Unicode glyphs remain partial or unverified. No Electron-shell run was performed; desktop shares this helper. Native mobile uses a separate renderer and is unaffected.

Before/after tables below show captured base eea98953 and candidate d4d5ad61. Full fixture conditions, repeats, geometry and narration are in the linked indexes.

FixtureBefore · eea98953After · d4d5ad61Diff0 ×8DiffT >16/255
outsideBefore outsideAfter outsideDiff0 outsideDiffT outside
plan-previewBefore plan-previewAfter plan-previewDiff0 plan-previewDiffT plan-preview
primary-assistant-1440-darkBefore primary-assistant-1440-darkAfter primary-assistant-1440-darkDiff0 primary-assistant-1440-darkDiffT primary-assistant-1440-dark
primary-assistant-1440-lightBefore primary-assistant-1440-lightAfter primary-assistant-1440-lightDiff0 primary-assistant-1440-lightDiffT primary-assistant-1440-light
primary-assistant-390-darkBefore primary-assistant-390-darkAfter primary-assistant-390-darkDiff0 primary-assistant-390-darkDiffT primary-assistant-390-dark
primary-assistant-390-lightBefore primary-assistant-390-lightAfter primary-assistant-390-lightDiff0 primary-assistant-390-lightDiffT primary-assistant-390-light
primary-user-1440-darkBefore primary-user-1440-darkAfter primary-user-1440-darkDiff0 primary-user-1440-darkDiffT primary-user-1440-dark
primary-user-1440-lightBefore primary-user-1440-lightAfter primary-user-1440-lightDiff0 primary-user-1440-lightDiffT primary-user-1440-light
primary-user-390-darkBefore primary-user-390-darkAfter primary-user-390-darkDiff0 primary-user-390-darkDiffT primary-user-390-dark
primary-user-390-lightBefore primary-user-390-lightAfter primary-user-390-lightDiff0 primary-user-390-lightDiffT primary-user-390-light
reasoning-openBefore reasoning-openAfter reasoning-openDiff0 reasoning-openDiffT reasoning-open
shapes-01-03Before shapes-01-03After shapes-01-03Diff0 shapes-01-03DiffT shapes-01-03
shapes-04-06Before shapes-04-06After shapes-04-06Diff0 shapes-04-06DiffT shapes-04-06
shapes-07-09Before shapes-07-09After shapes-07-09Diff0 shapes-07-09DiffT shapes-07-09
shapes-10-12Before shapes-10-12After shapes-10-12Diff0 shapes-10-12DiffT shapes-10-12
shapes-13-15Before shapes-13-15After shapes-13-15Diff0 shapes-13-15DiffT shapes-13-15
shapes-16-18Before shapes-16-18After shapes-16-18Diff0 shapes-16-18DiffT shapes-16-18
shapes-19-21Before shapes-19-21After shapes-19-21Diff0 shapes-19-21DiffT shapes-19-21
shapes-22Before shapes-22After shapes-22Diff0 shapes-22DiffT shapes-22
FixtureBefore · eea98953After · d4d5ad61Diff0 ×8DiffT >16/255
attachment-1440Before attachment-1440After attachment-1440Diff0 attachment-1440DiffT attachment-1440
attachment-390Before attachment-390After attachment-390Diff0 attachment-390DiffT attachment-390
chat-assistant-token-390Before chat-assistant-token-390After chat-assistant-token-390Diff0 chat-assistant-token-390DiffT chat-assistant-token-390
chat-user-leading-1440Before chat-user-leading-1440After chat-user-leading-1440Diff0 chat-user-leading-1440DiffT chat-user-leading-1440
chat-user-shapes-1440Before chat-user-shapes-1440After chat-user-shapes-1440Diff0 chat-user-shapes-1440DiffT chat-user-shapes-1440
chat-user-token-390Before chat-user-token-390After chat-user-token-390Diff0 chat-user-token-390DiffT chat-user-token-390
file-1440Before file-1440After file-1440Diff0 file-1440DiffT file-1440
file-390Before file-390After file-390Diff0 file-390DiffT file-390
pr-1440Before pr-1440After pr-1440Diff0 pr-1440DiffT pr-1440
pr-390Before pr-390After pr-390Diff0 pr-390DiffT pr-390

Recordings: before table toggle/pan, after table toggle/pan. Matching scripted steps and timestamps are in the indexes; supplemental clips cover Files/attachment source/rendered round trips and long-token panning.

Implemented with Claude Opus 5.5; reviewed and verified with GPT-6.1-Sol and Claude Opus 5.5 through T3 Code (Codex and Claude harnesses).

Checks

24 successful · 15 skipped
All checks passed
e8058c3

Added 2 commits

  • fix(web): pull request links in expanded tables no longer split mid-numberd4d5ad6
  • Merge main into fix/issue-24147e8058c3
MmacroscopeappBot2d ago
Synced from GitHub

Approvability

Verdict: Approved at e8058c3

Macroscope's review found this PR approvable — This one-file fix narrowly adjusts external-link text wrapping so short labels such as PR numbers remain intact while URL-shaped labels retain their existing break opportunities. It introduces no new capability, schema change, default, or static-analysis override.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot2d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 3cc31ed27c8956b36cee35a50509fb7c6869592b and e8058c32bac198a471de72f678e570efc0ba38e5.

External image Files selected for processing: 1
  • apps/web/src/components/chat/MarkdownExternalLink.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

Walkthrough

The link-label helper now leaves labels without periods or slashes intact. Labels containing either character retain per-character break opportunities.

Changes: External link label wrapping

Layer / File(s)Summary
Link label break opportunities
apps/web/src/components/chat/MarkdownExternalLink.tsx		breakableExternalLinkText returns labels without inserted break opportunities when they contain neither . nor /.

Priority: External image Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

  • pingdotgg/t3code#24163: This PR addresses the same mid-number wrapping issue in expanded Markdown tables by hiding <wbr> elements.

Suggested reviewers: macroscopeapp

Merge Risk: External image Minimal · up to e8058

No concrete merge-blocking risk remains from this focused wrapping change.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the primary fix: preventing pull-request links in expanded web tables from splitting mid-number.
Description checkPassedThe description is complete and follows the required sections. It explains the problem, implementation, scope, verification steps, screenshots, known limitations, and affected environments.
Linked Issues checkPassedDirectly linked issue #24146 requires PR link chips such as #363 to stay on one line in expanded Markdown tables. Reviewed head e8058c32 shows breakableExternalLinkText in `apps/web/src/componen…
Out of Scope Changes checkPassedThe raw summary and PR description identify one changed file, apps/web/src/components/chat/MarkdownExternalLink.tsx, with +4/-0 lines. The change adds a predicate guard to `breakableExternalLinkText…
Full details: Linked Issues check

Explanation

Directly linked issue #24146 requires PR link chips such as #363 to stay on one line in expanded Markdown tables. Reviewed head e8058c32 shows breakableExternalLinkText in apps/web/src/components/chat/MarkdownExternalLink.tsx. It returns the label as a single text node when the label contains neither . nor /. Only URL-shaped labels receive a &lt;wbr /&gt; after each character. For #363, the leading # and the favicon sit in a whitespace-nowrap span. The remaining 363 has no per-character break opportunities, so the digits cannot split at #36 and 3. Under CSS Text rules, overflow-wrap: break-word does not reduce min-content width, so the column keeps at least the width of the word. This matches the issue's expected behavior. The PR reports before/after captures for expanded tables at 1440 and 390 CSS px, in light and dark themes. I could not view those images, and the base revision file could not be read, so the before/after claim rests on the reported evidence plus the head code. Table CSS and ChatMarkdown are reported unchanged. Automated tests were not added. The issue requests a visual fix and the PR states that no implementation-mirroring tests were added, so this does not block PASS.

Full details: Out of Scope Changes check

Explanation

The raw summary and PR description identify one changed file, apps/web/src/components/chat/MarkdownExternalLink.tsx, with +4/-0 lines. The change adds a predicate guard to breakableExternalLinkText and a comment. It does not alter declarations or other files. The whole-PR text diff could not be read, so the one-file scope rests on the supplied summary and PR description. The PR also states that ordinary labels without a dot or slash now wrap by normal word boundaries. This side effect comes from the same helper predicate. It applies to labels in the same link-rendering path that the linked issue concerns, and the PR discloses and captures it. It is not an unrelated change.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

JJulius Marminge4h ago

Closing in favour of #24163 by @fixfon, which fixes the same split-number bug (#24146) with one CSS rule scoped to expanded tables. This PR changed the shared Markdown link helper, which also changes wrapping for ordinary labels outside tables. The evidence gathered here stays linked from this PR for reference. Thanks @fixfon for the precise fix.