Skip to content

fix(sdk): repair dead docs links and in-package link routing#875

Open
mlennie wants to merge 7 commits into
mainfrom
monty/fix-publisher-ui-links
Open

fix(sdk): repair dead docs links and in-package link routing#875
mlennie wants to merge 7 commits into
mainfrom
monty/fix-publisher-ui-links

Conversation

@mlennie

@mlennie mlennie commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

What

Fixes several developer-experience link/navigation bugs in the Publisher web UI.

Dead "Learn more" docs links

The Home screen feature cards, the empty-state button, and the Add-Package dialog linked to README anchors (#ad-hoc-data-analysis, #notebook-based-dashboards, #mcp-based-ai-data-agents, #server-configuration, #architecture-overview) that no longer exist after the README was restructured to link out to docs.malloydata.dev. GitHub silently loads the README top, so every card was a dead link on the app's first screen. They now point at the live docs pages (Explorer, MCP for AI Agents, Publish Your Models) through a shared DOC_LINKS module that the sidebar also uses, so a future docs move needs one edit.

In-package navigation dropped the package segment

Two paths built URLs relative to the current page and dropped the package segment, landing on /{env}/{file} which resolves the filename as a package and 404s with a misleading "Package manifest ... does not exist":

  1. Links inside a rendered notebook or README (for example an imdb package README's table of contents, [Steven Spielberg](spielberg.malloynb)) were plain browser anchors resolved against the current page URL. They now route through the SPA, resolved against the source file's own directory so nested notebooks keep their subdirectory, while external, hash, and absolute links stay as normal anchors with a safe-scheme guard for hrefs in untrusted package READMEs. Notebook/Package stay router-agnostic: the host threads an optional onNavigate handler (the app supplies react-router), and without one, links fall back to plain absolute anchors, so embedding the SDK still needs no <Router> context.

  2. The package page's file-list rows (notebooks, models, pages) navigated with a bare relative path and relied on react-router relative resolution, which drops the package segment depending on trailing-slash and route state. They now use absolute paths, matching the sibling "Back" and "Materializations" links that were already absolute.

Testing

New Playwright regression tests: clicking a README table-of-contents link and clicking a package file-list row both keep the package segment and render, instead of 404ing. The README case uses README.malloynb, which shares no name overlap with the package, so a dropped segment is caught (the pre-existing imdb.malloynb test could not catch it).

bun run typecheck, bun run lint, and bun run test (226 pass) are green, and every new docs URL was verified to return HTTP 200.

The Home screen and Add-Package dialog linked "Learn more" cards to
README anchors that no longer exist after the README was restructured to
point at docs.malloydata.dev, so every card was a dead link on the app's
first screen. Repoint them at the live docs pages through a shared
DOC_LINKS module (also consumed by the sidebar), so a future docs move
needs one edit rather than hunting down hardcoded links.

Two navigation paths inside a package also dropped the package segment
from the URL, 404ing with a misleading "Package manifest ... does not
exist":

- Links inside a rendered notebook or README (for example an imdb package
  README's table of contents) were plain browser anchors resolved against
  the current page URL. Route them through the SPA, resolved against the
  source file's own directory so nested notebooks keep their subdirectory,
  while external, hash, and absolute links stay as normal anchors with a
  safe-scheme guard. Notebook/Package stay router-agnostic: the host
  threads an optional navigation handler (the app passes react-router),
  and without one, links fall back to plain absolute anchors, so embedding
  the SDK needs no Router context.
- The package page's file-list rows (notebooks, models, pages) navigated
  with a bare relative path and relied on react-router relative
  resolution, which drops the package segment depending on trailing-slash
  and route state. Make them absolute, matching the sibling "Back" and
  "Materializations" links.

Add Playwright regression tests for both the README-link and file-list
paths.

Signed-off-by: Monty Lennie <[email protected]>
@mlennie
mlennie force-pushed the monty/fix-publisher-ui-links branch from 46d2e4c to 17fc596 Compare July 8, 2026 23:58

@jswir jswir left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm!

mlennie and others added 5 commits July 9, 2026 14:20
…le packages

The two regression specs this PR added were written against the old
malloy-samples fixture set (the imdb package and its README.malloynb).
Main has since replaced the served samples with the examples packages
(storefront, governed-analytics, html-data-app), so after merging main
both specs failed in CI before asserting anything: openPackage could
not find an "imdb" entry on the environment page.

Port them to the current fixtures without weakening what they pin:

- notebook-readme-links: no bundled example ships a README.malloynb, so
  the spec now writes its own README.malloynb plus a target notebook
  into the storefront package, reloads the package, and cleans up after
  (the same fixture pattern as notebook-givens.spec.ts). The target
  name (linked_tour.malloynb) shares nothing with the package name, so
  a dropped package segment cannot accidentally still resolve.

- package-notebooks "keeps the package segment": governed-analytics's
  orders.malloynb replaces imdb's README.malloynb as the no-name-overlap
  case; a dropped segment lands on /examples/orders.malloynb and 404s,
  which the storefront.malloynb test above cannot catch because its URL
  matches either way.

Full Playwright suite is green locally against a fresh start:init
server: 49 passed, 0 failed.

Signed-off-by: Kyle Nesbit <[email protected]>
Co-authored-by: Cursor <[email protected]>
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