Skip to content

Link search title/page matches to the page top instead of a section anchor (RND-11916) - #4434

Closed
zenoachtig wants to merge 1 commit into
mainfrom
claude/tender-allen-a8m5gm
Closed

Link search title/page matches to the page top instead of a section anchor (RND-11916)#4434
zenoachtig wants to merge 1 commit into
mainfrom
claude/tender-allen-a8m5gm

Conversation

@zenoachtig

Copy link
Copy Markdown
Contributor

Night-shift draft — not ready to merge, prepared for Zeno. Please review in the morning; nothing here has been human-reviewed, and I've listed the judgment calls below.

Proposed changes

Report: RND-11916 (Vectra AI / Tom Bilen). When a search result matched the page title, clicking it dropped the user into a mid-page section anchor (#heading) instead of the top of the article. Expected: title/page matches should go to the top of the page; section-level matches should keep their heading anchor.

Root cause: In packages/gitbook/src/components/Search/SearchPageResultItem.tsx, page results carry an optional bestSection (the highest-scoring section for that page). The link logic used bestSection.href (the #anchor) whenever a section snippet existed — regardless of whether the query actually matched the page title or a section. So a title match still linked to an arbitrary section anchor.

Fix: There is no server-provided title-vs-section flag on the search API response, so we infer a title match the same way title scoring already does in reciprocalRankFusion: every whitespace-split query word is a case-insensitive substring of the title. Added a small pure helper isPageTitleMatch.ts for this. The link now uses the section anchor only when a section snippet is shown and the query is not a title match; otherwise it links to the top of the page. Section-level matches are unchanged and keep their heading anchor.

This reproduces the approach from the earlier draft PR #4400 (closed only because its ephemeral branch was cleaned up — no review, no rejection) on the current stable branch.

Files changed:

  • packages/gitbook/src/components/Search/isPageTitleMatch.ts (new) — pure helper.
  • packages/gitbook/src/components/Search/isPageTitleMatch.test.ts (new) — unit tests, incl. the customer's examples (EDR Integrations → top; selecting crowdstrike URL → keeps anchor).
  • packages/gitbook/src/components/Search/SearchPageResultItem.tsx — link logic.
  • .changeset/search-title-match-page-top.md (new).

Changelog

  • [Fix] Search results that match a page title now link to the top of the page instead of a mid-page section anchor; section-level matches keep their heading anchor.

For Zeno

Judgment calls to sanity-check:

  • The title-match heuristic is substring-based, matching the existing reciprocalRankFusion title-scoring notion. This means short query words can match inside unrelated title words — e.g. api is a substring of Rapid, so a query "api" against a page titled "Rapid start" would be treated as a title match and link to the top. I kept it consistent with the existing scoring logic rather than introducing a stricter word-boundary rule; flag if you'd prefer word-boundary matching.
  • I did not suppress the section snippet on a title match — the result still shows the bestSection body preview text; only the link target changes. Arguably on a title match we could hide the section snippet entirely for consistency, but that's a bigger UX change and I left the visible snippet as-is. Let me know if you want the snippet hidden too.
  • No Playwright test added. The change is a pure link-target tweak covered by the unit test on the helper; a full browser test for search navigation felt like more than this fix warrants. Happy to add one if you want end-to-end coverage.

Generated by Claude Code

…nchor

Search results render as page items with an optional bestSection. The link
used the section anchor whenever a section snippet existed, so title/page
matches dropped users into a mid-page heading instead of the top of the article.

Link to the section anchor only when a section snippet is shown AND the query
is not a title match (every whitespace-split query word is a case-insensitive
substring of the title, mirroring the title-scoring notion in
reciprocalRankFusion). Section-level matches keep their heading anchor.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T4BRKDCFacoNMqgP5QyDPu
@linear-code

linear-code Bot commented Jul 23, 2026

Copy link
Copy Markdown

RND-11916

@changeset-bot

changeset-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: a1f8078

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
gitbook Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@argos-ci

argos-ci Bot commented Jul 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
customers-v2-cloudflare (Inspect) ⚠️ Changes detected (Review) 16 changed, 2 ignored Jul 23, 2026, 12:24 AM
customers-v2-vercel (Inspect) ⚠️ Changes detected (Review) 4 changed, 4 ignored Jul 23, 2026, 12:19 AM
v2-cloudflare (Inspect) ⚠️ Changes detected (Review) 3 changed, 3 ignored Jul 23, 2026, 12:22 AM
v2-vercel (Inspect) ✅ No changes detected 2 ignored Jul 23, 2026, 12:21 AM

Copy link
Copy Markdown
Contributor Author

Night-shift note on the Argos customers-v2-vercel failure (4 changed / 4 ignored, "waiting for your decision"):

This is a visual-diff review gate, not a code or test failure. The diff here only changes a search-result link target (SearchPageResultItem.tsx) plus a pure helper, its unit test, and a changeset — nothing that alters rendered pixels. So the 4 flagged screenshots are almost certainly baseline noise on that suite (the earlier draft of this same change, #4400, showed the same suite flaking). Test, Format, Build, Typecheck, and both preview deploys are green.

Leaving the Argos baseline decision to you since it's a human review call and this is a draft that isn't meant to merge yet. Worth a quick glance at the 4 diffs to confirm they're unrelated before this goes ready.


Generated by Claude Code

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.

2 participants