feat: search now fully functinonal - #4
Merged
Merged
Conversation
Signed-off-by: AlexMikhalev <alex@metacortex.engineer>
AlexMikhalev
added a commit
that referenced
this pull request
Aug 8, 2026
AlexMikhalev
added a commit
that referenced
this pull request
Aug 8, 2026
…errors Addresses review findings 1, 2 and 5, and adds the missing output-contract regression guards. Finding 1/2 — a bare `md-book` outside a book directory exited 0 having built an empty book, and the e2e test asserting the old "required args" failure was red. Path resolution stays pure; BookPaths::validate_for_build is the I/O boundary check and names both the expected path and the recovery. The e2e test now asserts the real contract, plus its counterpart (inside a book directory, no flags needed). Finding 5 — SummaryError::PrefixAfterNumbered was never constructed. A bare link after numbered chapters is a suffix chapter by definition, so the case the variant was meant to catch is a list item *after* a suffix chapter, which was being silently absorbed into the suffix list and losing its section number. Renamed to NumberedAfterSuffix and made reachable. The new guards caught a real defect: increment D1 made static asset URLs relative but left every navigation link root-absolute, so output still could not deploy under a sub-path or open over file://. Fixed at all four sources — sidebar hrefs (to_nav now takes path_to_root), prev/next, the header home link and logo, and the index card grid. Legacy `sections` paths become root-relative with templates prefixing path_to_root. doc-toc.js reuses the document's resolved stylesheet URL, since a copied component cannot see Tera's path_to_root. Six tests added: no absolute asset paths (scanning all emitted HTML and JS), no unexpected external URLs (Shoelace allowlisted pending D2), stable heading IDs across rebuilds, sidebar nesting and ARIA, asset pass-through with orphan exclusion, and create-missing idempotence. Refs #2, Refs #4 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AlexMikhalev
added a commit
that referenced
this pull request
Aug 8, 2026
Closes increment D's offline gate. Shoelace was loaded from jsDelivr, so generated books needed the network to render. Vendoring the full distribution was not an option: it is 14MB over 2,920 files (8.4MB of that the icon set, which sl-icon fetches at runtime) and copy_static_assets writes templates into *every* built book. Instead the transitive import closure of the five components actually used — button, icon, input, spinner, card — is vendored with the ten referenced icons and the light/dark themes: 43 JS files, 356KB. shoelace-local.js replaces the CDN autoloader and derives setBasePath from its own module URL, so it resolves at a domain root, under a sub-path, or over file://. Shadow-DOM components cannot see Tera variables, so doc-toc, doc-sidebar and simple-block now resolve their stylesheets from import.meta.url. The dead <script> tag injected via innerHTML in doc-sidebar (innerHTML never executes scripts) is removed. Verified against the 30-page corpus served under /docs/: no external URLs, every referenced asset resolves, and all 44 files in the Shoelace module graph load — a missing chunk would have broken component upgrade silently. Separately, this exposed a pre-existing defect that the sub-path check made visible: css/, js/ and img/ were only emitted when a templates directory happened to exist on disk, so an installed md-book produced books with no styles.css, no search JS, no mermaid and no logo. The repo's own builds masked it (book.toml points at src/templates) and TestBook has no templates dir either, so nothing caught it. Default css/js/img/components are now embedded with include_dir and a templates directory overrides them per file. copy_static_assets loses its pile of include_str! calls in the process. Adds test_default_assets_emitted_without_templates_dir, which asserts every local URL a page references was actually emitted, and tightens test_output_has_no_external_urls by deleting the CDN allowlist. Refs #4 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AlexMikhalev
added a commit
that referenced
this pull request
Aug 8, 2026
Vendoring the full Shoelace distribution was rejected once measured (14MB, 2,920 files, copied into every book); the plan now records what shipped instead. include_dir was not anticipated by the plan and is documented with its justification. Refs #4 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AlexMikhalev
added a commit
that referenced
this pull request
Aug 8, 2026
Browser verification of E7 (agent-browser against a served build) confirmed
the mermaid gating and turned up three defects a DOM check would not have
found from the HTML alone.
E7 verified: the diagram page fetches mermaid.min.js (2867KB) and renders
svg#mermaid-… with nodes Start/Middle/End, the SVG replacing the raw fence
inside its <code> element; the plain page fetches nothing mermaid-related
and window.mermaid is undefined. No console errors on either.
Defects found and fixed:
- Config defaults were never applied. `#[serde(default = "…")]` does not
survive twelf's layering (absent keys become empty strings), and
`#[serde(default)]` on a container field constructs it with
`Default::default()`, bypassing the per-field defaults inside. A book with
no book.toml rendered an empty <title>, and a logo with src="" that the
browser resolved to the page itself. Book, Rust, Paths and SearchConfig
now have hand-written Default impls using the same default_* functions,
and load_config fills unset scalars from them.
- The page and index templates emitted a bare <html> with no lang, while
404 and print hard-coded lang="en". All four now use
config.book.language, which the defaults fix makes non-empty.
- Three config tests asserted `x.is_empty() || !x.is_empty()` — true for any
value — which is why every default being empty went unnoticed. They now
assert the actual documented defaults.
Also corrects the research document: canonical URLs, meta descriptions and
the skip link were recorded as "Have", but were read from a dirty working
tree and live only in stash@{0}. main has none of them. Merging that stash
will now conflict with increments C-E, which rewrote the same templates.
Refs #4, Refs #5
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AlexMikhalev
added a commit
that referenced
this pull request
Aug 8, 2026
stash@{0} (canonical URL, meta description, skip link) cannot be popped: its
src/core.rs hunks patch functions increment A dissolved, and all three
templates it touches were rewritten by C-E. It is a source to salvage from,
not a change to replay.
Scoped as increment G with four steps, since two of its items (the lang
attribute and the header aria-labels) have already landed independently:
- G1 per-page description, deriving text from the parsed mdast rather than
the stash's `if c.is_ascii()` filter, which turns "café" into "caf "
- G2 canonical URLs, reconciling the two competing keys: mdBook's
output.html.site-url wins, book.base_url becomes a deprecated alias, and
disagreement between them warns
- G3 skip link plus the article id and the visually-hidden style
- G4 search UI gated on a Pagefind index actually existing — currently a
search box ships even when indexing was skipped
Eight tests specified, including one guarding the non-ASCII defect. The gate
records that stash@{0} can be dropped once G lands.
Refs #4
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.