Skip to content

fix(ui-tui): skip generic path segments when deriving slug fallback labels (#25606) - #25619

Closed
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/ui-tui-slug-generic-fallback-25606
Closed

fix(ui-tui): skip generic path segments when deriving slug fallback labels (#25606)#25619
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/ui-tui-slug-generic-fallback-25606

Conversation

@briandevans

@briandevans briandevans commented May 14, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

TUI link labels collapsed to a single generic word ("Status", "Auth", …) when title resolution fell back to urlSlugTitleLabel(), masking which destination was linked. Extends the same-shape skip used for numeric-only segments to cover a small set of generic single-word path segments, so the loop continues to a more informative segment or falls through to hostPathLabel.

PR #24013 introduced ResolvedLink + useLinkTitle() in ui-tui/src/components/markdown.tsx. When the async <title> fetch fails or is skipped (local/private hosts, non-HTML responses, errors), the display falls back to defaultLinkLabelurlSlugTitleLabel(url). urlSlugTitleLabel() iterates path segments in reverse, already skipping pure-numeric segments, but accepts the first segment with a letter and length ≥ 4, including obviously generic trailing words like status, auth, item. The Web UI shows the full clickable URL in the same case.

After the fix: https://x.com/OpenAI/status/1234567890OpenAI; https://accounts.google.com/o/oauth2/authaccounts.google.com/o/oauth2/auth; https://news.ycombinator.com/itemnews.ycombinator.com/item. Multi-word and domain-specific segments (issues, releases, pulls, commit, …) are left alone so existing readable slugs keep working.

Related Issue

Fixes #25606

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • ui-tui/src/lib/externalLink.ts — add GENERIC_PATH_SEGMENT_RE regex (status/auth/oauth/signin/signup/login/page/index/home/post/tweet/view/show/detail/comment/share/embed/dashboard/profile/account/settings/item/callback/redirect…) and a continue skip sibling to the existing numeric-only skip in urlSlugTitleLabel().
  • ui-tui/src/__tests__/externalLink.test.ts — 2 new cases covering the three URL classes above.

How to Test

  1. From ui-tui/: npx vitest run src/__tests__/externalLink.test.ts — 12/12 pass.
  2. Full ui-tui suite: npx vitest run (after building packages/hermes-ink) — 722/722 pass across 65 files.
  3. Type check: npm run type-check — clean.
  4. Regression guard: revert the production line; the two new tests fail with "Status" and "Auth". Restore — green.
  5. Existing fixture urlSlugTitleLabel('https://www.getyourguide.com/.../from-fajardo-icacos-island-full-day-catamaran-trip-t19891/') still returns From Fajardo Icacos Island Full Day Catamaran Trip — readable multi-word slug unaffected.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run focused tests for the touched code and all pass (722/722 + type-check clean)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15.x

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — TypeScript regex, platform-independent
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Related / Positioning

Sibling code paths that may need the same fix: ui-tui/src/lib/externalLink.ts:hostPathLabel returns the bare host+path for the final fallback, which is what we want — no widening needed there. If anything ever calls urlSlugTitleLabel() from outside the markdown renderer, this guard applies uniformly. Happy to widen if preferred.

Copilot AI review requested due to automatic review settings May 14, 2026 10:12

Copilot AI 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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds filtering of generic path segments (e.g., status, auth, login) when deriving a human-readable title from a URL slug, so that meaningful preceding segments surface instead of uninformative ones.

Changes:

  • Introduce GENERIC_PATH_SEGMENT_RE to match common generic URL path segments.
  • Skip these segments in urlSlugTitleLabel, falling back to host+path when no useful slug is found.
  • Add tests covering the new behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
ui-tui/src/lib/externalLink.ts Adds generic-segment regex and skip logic in urlSlugTitleLabel.
ui-tui/src/tests/externalLink.test.ts Adds tests for generic-segment skipping and host+path fallback.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) javascript labels May 14, 2026
@briandevans
briandevans force-pushed the fix/ui-tui-slug-generic-fallback-25606 branch from 9566468 to 39d11ef Compare May 15, 2026 12:14
@briandevans
briandevans force-pushed the fix/ui-tui-slug-generic-fallback-25606 branch from 39d11ef to 8094104 Compare May 19, 2026 03:54
@briandevans

Copy link
Copy Markdown
Contributor Author

Rebased onto current main. Dropped the x.com/OpenAI/status/NOpenAI test case because main's 7321b3c2d (fix(tui): keep x status citation fallbacks link-like) intentionally short-circuits x.com/status permalinks to hostPathLabel before any generic-skip logic runs — that's a deliberate design choice. The remaining tests (accounts.google.com/o/oauth2/auth, news.ycombinator.com/item) still exercise the generic-skip path. All 12 tests pass locally.

@briandevans
briandevans force-pushed the fix/ui-tui-slug-generic-fallback-25606 branch 2 times, most recently from 006181d to 0728cd2 Compare May 25, 2026 04:12
…abels (NousResearch#25606)

`urlSlugTitleLabel()` (introduced with PR NousResearch#24013's `ResolvedLink` / `useLinkTitle`
flow) iterates URL path segments and title-cases the first one with a letter. The
numeric-only filter two lines above already skips IDs like `12345`, but a generic
trailing path word such as `status`, `auth`, or `item` slips through and becomes
the entire user-facing label.

The reported symptom: every `https://x.com/<user>/status/<id>` link in the TUI
rendered as the literal word "Status", hiding which user and tweet was linked.
`https://accounts.google.com/o/oauth2/auth` rendered as "Auth".

Add a sibling skip for generic single-word segments (status, auth, oauth/oauth2,
signin/signup/login/logout, page/pages, home/index, post/posts, tweet/tweets,
view/show, comment/embed, dashboard/profile/account/settings, item, callback,
redirect, …). When skipped, the loop either reaches a more meaningful preceding
segment (`x.com/OpenAI/status/...` → "OpenAI") or falls through to the existing
`hostPathLabel` fallback (`accounts.google.com/o/oauth2/auth`,
`news.ycombinator.com/item`) so the destination stays visible instead of being
masked by a one-word generic label.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@briandevans
briandevans force-pushed the fix/ui-tui-slug-generic-fallback-25606 branch from 0728cd2 to bec3207 Compare May 28, 2026 18:10
@briandevans

Copy link
Copy Markdown
Contributor Author

Housekeeping: closing to keep my open-PR set focused on actively-reviewed work. This has been open ~18d without maintainer review and the surrounding code has continued to move, so it's unlikely to land as-is. The underlying fix still stands — happy to reopen and rebase if it would be useful. Thanks!

@briandevans briandevans closed this Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: TUI shows slug text instead of original URL

3 participants