Skip to content

add search function for docs & about!#974

Open
emmahodcroft wants to merge 10 commits into
mainfrom
try-docs-search
Open

add search function for docs & about!#974
emmahodcroft wants to merge 10 commits into
mainfrom
try-docs-search

Conversation

@emmahodcroft
Copy link
Copy Markdown
Member

@emmahodcroft emmahodcroft commented May 22, 2026

Watch out everyone - I'm out here being a dev thanks to Claude! 👊 💥 pow, pow!

It's quite hard to find things in our docs - the menu on the left is quite overwhelming to read through. If you know exactly what you want, it's ok, but for a newcomer I imagine it's quite intimidating.

So let's fix it! Claude and I did it together. Here's his take on this PR. I tested it locally and it seems to work 😁

That'll do pig. That'll do.

🚀 Preview: https://preview-try-docs-search.pathoplexus.org

Comment thread monorepo/website/src/pages/search-index.json.ts Outdated
Comment thread monorepo/website/src/utils/buildSearchIndex.ts Outdated
Comment thread monorepo/website/src/utils/buildSearchIndex.ts Outdated
Copy link
Copy Markdown
Contributor

@corneliusroemer corneliusroemer left a comment

Choose a reason for hiding this comment

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

Need to add minisearch to deps that's why image build fails - do we allow adding new packages in the overlay? Might be tricky.

Overriding whole package.json would be messy. So for now would have to add the package on Loculus side

@emmahodcroft
Copy link
Copy Markdown
Member Author

No this is me not knowing about dev things. Theo says we should add minisearch (and now it would like to add yaml to address some of Theo's comments) to Loculus

@theosanderson
Copy link
Copy Markdown
Member

@theosanderson-agent - try to address all the points above. you can push straight to Emma's branch. You need to make a PR to loculus to add the necessary dependencies, e.g. minisearch, yaml. Then you can update the loculusVersion in this PR to point to the commit hash from the Loculus PR and check CI passes here.

claude added 2 commits May 22, 2026 15:44
- search-index.json.ts: send Cache-Control: public, max-age=3600 and
  a sha1 ETag derived from the index JSON; 304 on if-none-match. Closes
  the caching & cache-key feedback from @theosanderson and @corneliusroemer.
- buildSearchIndex.ts: parse the MDX frontmatter with the yaml lib
  instead of regex matching (per @theosanderson), drop the dead
  index-page skip, and cache the built index + ETag in memory.
- DocsSearch.tsx: fix the SearchResult type cast, type the fetched
  response, swap the clear button for the wrapped Button component,
  and remove console.error/floating-promise warnings flagged by lint.
- bump loculusVersion to 46b6cf6f (the head of the Loculus
  add-search-deps PR that adds minisearch + yaml deps).
Switches buildSearchIndex.ts to use import.meta.glob with ?raw so Vite
bundles the MDX content into the server bundle. The index payload is
JSON.stringify-ed once at module load (when the Node server boots),
and the route just hands back the precomputed string.

With the work moved out of the request path there is no benefit to
ETag / Cache-Control / 304 handling, so the route is now a one-line
new Response(...).
@theosanderson-agent
Copy link
Copy Markdown
Collaborator

Took a pass at the review here:

  • Caching / hash cache key — Moved the index build to build time in 9fbe198 using import.meta.glob with ?raw; Vite bundles the MDX content into the server bundle and the payload is JSON.stringify-ed once when the Node server boots. The route is now a one-line new Response(...). With the work out of the request path the ETag / Cache-Control / 304 plumbing from the first round (c609ad2) became redundant and is gone.
  • Properly parse the yaml — Frontmatter is now parsed with the yaml library (c609ad2).
  • Only mdx files / redundant index skip — Confirmed parseUrlFromPath already strips trailing /index, so the if (!url.endsWith("index")) guard was dead code. Removed in c609ad2; index pages now get included.

Other things along the way:

  • Loculus PR feat(website): add docs search loculus-project/loculus#6496 (deps) needed a package-lock.json regen — the first attempt regenerated the whole lockfile and accidentally marked vite's nested platform @esbuild/* binaries as dev: true, which broke npm ci --omit=dev in the website Docker build with an esbuild postinstall mismatch. Reverted to a surgical 8-line diff that only adds the minisearch package entry (46b6cf6).
  • loculusVersion bumped to that commit on this PR.
  • Small cleanups in DocsSearch.tsx: fixed the SearchResult cast, typed the fetched response, swapped the clear button for the wrapped Button from common/, and removed lint-flagged console.error / floating promise issues.

Build is green on both PRs.

# Conflicts:
#	pathoplexus_app/values.yaml
theosanderson-agent pushed a commit to loculus-project/loculus that referenced this pull request May 22, 2026
Adds a MiniSearch-powered client-side search that's exposed as a bar
at the top of the docs and about menu.

- buildSearchIndex.ts pulls MDX content at build time via
  import.meta.glob('?raw') so Vite bundles the source into the
  server output. The {options, documents} payload is JSON.stringify-ed
  once when the Node module loads; the HTTP route just hands the
  precomputed string back, so there's no per-request work and no
  need for Cache-Control / ETag headers.
- Frontmatter titles are parsed with the yaml lib.
- DocLayout.astro and AboutLayout.astro render <DocsSearch client:load />
  above the existing DocsMenu.

This lands a feature that was originally drafted in pathoplexus PR
pathoplexus/pathoplexus#974; moving the impl to Loculus so downstream
consumers automatically get the same behaviour without overlay files.
claude added 6 commits May 22, 2026 16:14
The DocsSearch component, buildSearchIndex util and search-index.json
route all moved upstream in loculus-project/loculus#6496, which also
wires the search bar into DocLayout and AboutLayout. Drop the
overlaid copies here and bump loculusVersion to the new tip
(b773fe7ab) so the sync pulls everything in.
Picks up c4bd4a01f from add-search-deps which fixes
- "Untitled" rows in the docs search index for mdx files with CRLF
  line endings (e.g. /about/governance/data-submission)
- the layout shift caused by the "Loading search..." text appearing
  briefly under the search input on load
@@ -1,3 +1,3 @@
loculusVersion: 30f3557c5c7066a1e1bfdb7d586f4cca3f4c64c9
loculusVersion: 3117e2f5e27a46f5197eda78fda2f19f6ef12285
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

revert

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants