Skip to content

[codex] Add Astro Starlight site#23

Merged
plx merged 3 commits into
mainfrom
codex/astro-starlight-site
May 18, 2026
Merged

[codex] Add Astro Starlight site#23
plx merged 3 commits into
mainfrom
codex/astro-starlight-site

Conversation

@plx
Copy link
Copy Markdown
Owner

@plx plx commented May 13, 2026

Summary

  • adds an Astro/Starlight static site under site/ using the approved landing-page concept
  • adds Starlight docs pages for overview, commands, guide format, and CI usage
  • adds a GitHub Pages deployment workflow configured for plx.github.io/agentic-navigation-guide

Validation

  • npm run build from site/
  • parsed .github/workflows/pages.yml as YAML
  • git diff --check

Note: landing page copy is intentionally still draft-ready for follow-up edits.

@claude
Copy link
Copy Markdown

claude Bot commented May 13, 2026

Code Review — PR #23: Add Astro Starlight site

Overall this is a well-structured addition. The CI workflow is solid, the design system approach (design tokens + component CSS) is thoughtful, and the Starlight docs pages cover the right topics. The PR is still DRAFT, but a few issues should be addressed before merge.

Bugs

--space-scale-5 is used but never defined in tokens.css

tokens.css defines --space-scale-4: 16px and --space-scale-6: 24px but skips scale step 5. var(--space-scale-5) silently resolves to nothing (the property takes its initial/inherited value, effectively 0 for length properties) in at least five places:

File Property affected
cards-composites.components.css .mq-cta-band padding
cards-composites.components.css .mq-footer__inner gap
landing.css .section__header margin-block-end
landing.css .workflow padding
landing.css mobile hero gap (@media (max-width: 760px))

Fix: add --space-scale-5: 20px; to tokens.css (consistent with the 4 px base scale).

Accessibility

aria-current="page" is hardcoded on the "Features" anchor link

Both the desktop nav and the mobile panel hard-code aria-current="page" on <a href="#features">. aria-current="page" signals to assistive technologies that the link represents the currently loaded page — it should not be set statically on a section anchor. Either remove it entirely (anchor links don't need aria-current), or drive it dynamically with a scrollspy observer. Having it hard-coded means screen-reader users are incorrectly told that "#features" is the current page on every visit.

Project convention

AGENTIC_NAVIGATION_GUIDE.md not updated for the new site/ subtree

The guide currently ends with ... (catch-all placeholder) so this won't cause a verification failure, but per project convention (CLAUDE.md → Documentation Alignment) the site/ directory and its key files should be added to the guide when a new top-level directory is introduced. At a minimum, site/ with a short comment would keep the guide useful for agents navigating this repo.

Minor / nitpicks

Duplicate nav markup is a maintenance risk
The desktop nav links and the mobile panel links are copy-pasted HTML with identical href values. Any future nav change needs to be made in two places. This is common in static sites, but a shared Astro component or <template> pattern would eliminate the risk.

docsRsUrl points to docs.rs for a CLI binary
https://docs.rs/agentic-navigation-guide will work once published, but since the crate is primarily a binary the docs.rs page will be sparse (only public API surface visible to library consumers). Worth a note or link text like "API docs" rather than "docs.rs" if the audience is mostly CLI users.

No web font loading
The design tokens specify "IBM Plex Sans Condensed", "JetBrains Mono", and "Inter" as preferred fonts, but the <head> has no font loading (<link rel="stylesheet"> or @import). The site will render entirely in system fallbacks. If this is intentional (zero network fonts = best LCP / no flash of invisible text) then it's a valid tradeoff — but the heading personality described in the token comments ("condensed, sturdy, field-manual-like") depends on IBM Plex Sans Condensed actually loading.

What's working well

  • CI workflow: pinned to full commit SHAs (supply-chain safe), minimal permissions (contents: read, pages: write, id-token: write only), cancel-in-progress: false on the Pages concurrency group (correct — canceling a deploy would leave the live site in a half-deployed state), and workflow_dispatch for manual re-deploys.
  • Design token architecture: the semantic/palette layering in tokens.css is clean and will make theming or dark-mode easier later.
  • Starlight docs content: the four pages (Overview, Commands, Guide Format, CI) cover the right surface area and are accurate relative to the tool's actual behavior.
  • sitePath() helper: correctly normalizes the BASE_URL for the GitHub Pages subpath — this is easy to get wrong with Astro's base URL handling.
  • Reduced-motion support: @media (prefers-reduced-motion: reduce) is present and correctly disables decorative transitions.

@plx plx force-pushed the codex/astro-starlight-site branch from 7b1e3b2 to d2de40c Compare May 16, 2026 18:11
@claude
Copy link
Copy Markdown

claude Bot commented May 18, 2026

Code Review — PR #23: Add Astro Starlight site

Status: Draft · Good foundation with a few items worth addressing before merge.

Overview

This PR adds an Astro/Starlight static documentation site under site/ with a custom landing page, four docs pages (overview, commands, guide format, CI), a design token system, and a GitHub Pages deploy workflow. The scope is self-contained (no changes to the Rust crate), and the overall architecture is clean.

Strengths

  • All GitHub Actions are SHA-pinned in pages.yml — excellent supply chain hygiene.
  • sitePath helper in index.astro handles base URL normalization correctly across dev and production.
  • Mobile navigation JS is accessible: aria-expanded, aria-controls, Escape key handler, and focus management are all present.
  • Clipboard API is properly guarded in try/catch with correct ARIA label reset.
  • CSS architecture is well-organised — design tokens in tokens.css, semantic aliases, and BEM-style .mq-* component classes.

Issues

Bug / Correctness

aria-current="page" misused on all nav links (site/src/pages/index.astro, nav section)

<a class="mq-site-nav__link" href="#features" aria-current="page">Features</a>
<a class="mq-site-nav__link" href="#cli">CLI</a>

aria-current="page" should only appear on the link that represents the current page/section. Putting it on #features unconditionally means screen readers will always announce "Features (current page)" regardless of scroll position. For a single-page landing, either remove aria-current from all links and manage it dynamically, or drop it entirely.

Documentation Accuracy

Hardcoded version number will drift (site/src/pages/index.astro, line ~6834)

<span class="mq-code-panel__meta">agentic-navigation-guide 0.1.4</span>

This will be stale after the next release. Consider pulling this from package.json/an env var, or replacing it with something static like "CLI".

Unpinned action in docs example (site/src/content/docs/docs/ci.md)

- uses: actions/checkout@v4

The repo's own pages.yml pins every action to a SHA. The example in the docs doesn't have to match exactly, but showing the unpinned form teaches users a weaker pattern. A small note like # pin to a SHA for production or just using the SHA would be consistent with what the project already demonstrates.

Navigation Guide not updated

The AGENTIC_NAVIGATION_GUIDE.md at the repo root doesn't include the new site/ directory. Given that this is specifically a tool for keeping navigation guides honest, it's worth adding site/ — even as a high-level stub entry — before merging. If site/ is intentionally omitted (e.g. to keep the guide Rust-only), documenting that choice under "Known Intentional Divergences" per CLAUDE.md would be appropriate.

Minor / Nit

tokens.css ships documentation strings as CSS variables

Properties like --typography-usage-body, --typography-font-strategy-heading-personality, etc. carry long human-readable documentation strings that are never consumed by any CSS rule. These appear to be exported verbatim from a design-system JSON. They add ~2 KB to every page load for no runtime benefit. Worth stripping them before the site goes public.

No PR-time build check

pages.yml triggers only on push to main. A PR build job (without the deploy step) would catch Astro build failures before they reach main. Low priority since this is a draft, but worth adding before the workflow goes live.

Summary

No blocking correctness issues in the Rust crate. The site itself is well-structured. The main items before marking ready: fix aria-current on nav links, update or stub AGENTIC_NAVIGATION_GUIDE.md for the site/ subtree, and strip the unused documentation tokens from tokens.css. The hardcoded version and unpinned CI example are smaller polish items.

@claude
Copy link
Copy Markdown

claude Bot commented May 18, 2026

Code Review

Overall: Well-structured addition. The workflow, accessibility patterns, and design token approach are all solid. A few things worth addressing before merge.

CI / Workflow (.github/workflows/pages.yml)

Strengths:

  • All third-party actions are pinned to full commit SHAs — the right call for supply-chain safety.
  • Path filters prevent spurious runs when only Rust source changes.
  • cancel-in-progress: false on the pages concurrency group is correct; you don't want to cancel a nearly-finished deploy.
  • Upload/deploy steps are correctly gated on github.event_name != 'pull_request', so PRs get a build check without triggering a deploy.

Suggestions:

  • The concurrency.group is a bare "pages" string with no branch component. This is fine today (only main pushes reach the deploy path), but worth noting if the repo ever needs staging deploys from other branches.
  • node-version: 22 is pinned as a major, not a patch. Fine for now, but aligns with the @astrojs/mdx and @astrojs/prism engine constraint of >=22.12.0.

Astro Config (site/astro.config.mjs)

Minor:

  • crossorigin: "" on the gstatic.com preconnect link is technically valid but "anonymous" is more explicit and easier to read.
  • The Google Fonts URL is duplicated — once in astro.config.mjs (for Starlight pages) and again inline in index.astro (for the landing page). A single source of truth (e.g., a shared constant or Starlight's head field driving both) would prevent drift if the font set changes.

Landing Page (site/src/pages/index.astro)

Strengths:

  • Semantic HTML throughout (<article>, <nav>, <header>, <footer>, <main>).
  • ARIA labels are present on interactive controls and decorative SVGs are correctly marked aria-hidden.
  • sitePath() correctly handles base URL normalization for both trailing-slash and non-trailing-slash BASE_URL values.
  • The copy-button JS handles the success/failure states cleanly and uses aria-label updates for screen readers.

Issues:

  • No <link rel="icon"> in the landing page <head>. Starlight-generated pages get a default favicon from the framework, but this custom page does not. Browsers will silently 404 on favicon.ico.
  • The mobile nav toggle uses aria-controls="landing-mobile-nav" and the panel id="landing-mobile-nav" — correctly wired. But the Escape key handler is added to window without cleanup. In an Astro MPA this is fine (full navigations discard the handler), but worth noting if this component is ever extracted.

Design Tokens (site/src/styles/tokens.css)

Issue — no dark mode:
tokens.css defines only light-mode values with no @media (prefers-color-scheme: dark) block. Starlight pages get dark mode via Starlight's built-in theming (the starlight.css overrides feed into its [data-theme] system), but the custom landing page bypasses that and will always render in light mode regardless of system preference.

This may be intentional for the initial draft, but should be a known gap before the PR leaves draft status.

The starlight.css only defines light-mode overrides as well — Starlight's own dark-mode variant would need a [data-theme="dark"] :root { ... } block to pick up the custom palette there too.

Documentation Content

Accurate and concise. The four docs pages (index.md, commands.md, guide-format.md, ci.md) faithfully describe the current CLI behavior. One note:

  • ci.md recommends cargo install agentic-navigation-guide in the example workflow. If the crate isn't yet published to crates.io, first-time users who copy that snippet will hit an error. Worth adding a note or linking to the install section until the crate is live.

File Structure

The content lives at site/src/content/docs/docs/ (double docs/), which produces routes like /docs/commands. This works and the sidebar slugs are correct, but the on-disk layout is slightly surprising. A comment in astro.config.mjs or a brief note in the site README would help future contributors understand why the nesting exists.

Summary

Area Status
CI workflow ✅ Ready
Accessibility ✅ Ready
Responsive layout / reduced-motion ✅ Ready
Dark mode (landing page) ⚠️ Not implemented — acceptable for draft
Favicon ⚠️ Missing
Duplicate font URL ⚠️ Minor debt
Docs accuracy ✅ Accurate

No blocking issues for a draft PR. The dark mode gap and missing favicon are the two things I'd want resolved before this goes to final review.

@plx plx marked this pull request as ready for review May 18, 2026 11:10
@plx plx merged commit ce071d2 into main May 18, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@plx