docs: fix search-result excerpts (CSS-special anchors) + enable search-as-you-type#713
Conversation
Overall Assessment: ✅ Looks goodExecutive Summary
MethodologyNo findings. Code QualityNo findings. Both shims are defensive and syntax-valid. PerformanceNo findings. Runtime overhead is negligible; jsdom runs only in CI. MaintainabilityNo findings. The Sphinx hook is guarded and checked for upstream drift. Tech DebtNo findings. No new methodological or correctness debt is introduced. SecurityNo findings. Dependencies are lockfile-pinned, install scripts are disabled, actions use commit SHAs, and no secrets were detected. Documentation/TestsP2 — Smoke test does not reproduce production load order
|
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment: ✅ Looks goodExecutive Summary
MethodologyNo findings. Estimator sources and Code QualityNo findings. The JavaScript files pass syntax checks and defensively handle missing APIs. PerformanceNo findings. Runtime overhead is negligible; jsdom is CI-only. MaintainabilityNo findings. Tech DebtNo findings. The P2 below is not currently tracked, but does not block approval. SecurityNo findings. Dependencies and actions are pinned, install scripts are disabled, and no secrets were introduced. Documentation/TestsP2 — [Newly identified] Legacy links collide after duplicate headings are de-numbered
The previous smoke-test finding is resolved by the production-order assertion and loading-state lifecycle harness in |
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment: ✅ Looks goodExecutive Summary
MethodologyNo findings. Estimator sources and Code QualityNo findings. The shims safely handle malformed fragments and unavailable browser APIs. PerformanceNo findings. Browser overhead is negligible; jsdom runs only in CI. MaintainabilityNo findings. Production script ordering and the Sphinx hook are explicitly guarded. Tech DebtNo findings. No relevant deferred work was introduced. SecurityNo findings. Node dependencies have integrity-pinned lock entries, install scripts are disabled, and the action is SHA-pinned. Documentation/TestsNo findings. The previous P2 is resolved: repeated titles were already unnumbered, so no legacy numbered link existed for their later occurrences. The invariant is documented in |
…h-as-you-type Notebook section anchors containing CSS-special characters crashed Sphinx's search-excerpt renderer (querySelector SyntaxError at searchtools.js htmlToText), silently dropping result summaries. Browser-verified root cause and fix: - docs/_static/searchtools-css-escape.js: CSS.escape() wrapper around Search.htmlToText - fixes ALL crash classes (leading digits, apostrophes, parens/commas, colons). Wraps at DOMContentLoaded because search.html emits custom js before searchtools.js. - Strip numeric prefixes from 185 tutorial headings across 22 notebooks (fence-aware transform, byte-identity JSON round-trip gate; ordering is conveyed by the grouped tutorials index). 50 self-referential "Section N" prose references reworded to heading names across markdown AND code cells (incl. section-sign form); paper-section citations and quoted library output kept verbatim. - docs/_static/legacy-fragment-redirect.js: pre-rename numbered deep links (#3.-Fit-Event-Study) redirect to their unnumbered targets via history.replaceState; page paths unchanged. - Enable pydata theme search_as_you_type (deliberately sequenced after the crash fix). - Notebook hygiene in 10_trop: markdown cell carried outputs/ execution_count keys; cell 27 source was stored as 940 single-character strings (hid one heading from every text tool). - CI (docs-tests sphinx-build job): built-HTML anchor tripwires + jsdom behavioral smoke test (.github/scripts/search_excerpt_smoke.mjs) driving the built searchtools.js + shims with every special-anchor class and the legacy-fragment redirect; reproduces the production script order (inline classic-script harness + emit-order pin) and pins per-page id uniqueness so redirect targets stay unambiguous; jsdom pinned via committed manifest+lockfile, npm ci --ignore-scripts, SHA-pinned setup-node. Verified: -W build clean from scratch, zero digit-leading ids in built HTML, all shims on all 292 built pages, browser end-to-end (search page + as-you-type dialog, zero console errors), 174 drift/IA/extract tests and 291 doc-snippet/deps tests pass, nbformat-valid notebooks.
7cf3266 to
e022d87
Compare
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment: ✅ Looks goodExecutive Summary
MethodologyNo findings. Code QualityNo findings. The shims safely guard unavailable APIs, malformed fragments, and nonexistent redirect targets ( PerformanceNo findings. Runtime overhead is negligible; jsdom is CI-only. MaintainabilityNo findings. Production script order and redirect-target uniqueness are explicitly tested ( Tech DebtNo findings. No new deferred limitation was introduced. SecurityNo findings. jsdom is integrity-locked, lifecycle scripts are disabled during installation, and GitHub actions use commit SHAs. Documentation/TestsNo findings. All changed notebooks are valid JSON; semantic comparison confirms only heading/reference text and the documented TROP serialization cleanup changed. JavaScript, Python configuration, and workflow syntax checks pass. |
Summary
searchtools.jsthrowSyntaxErrorinquerySelectorand silently drop result summaries.docs/_static/searchtools-css-escape.jswrapsSearch.htmlToTextwithCSS.escape()(deferred to DOMContentLoaded -search.htmlemits custom JS beforesearchtools.js).docs/_static/legacy-fragment-redirect.jsrewrites a no-longer-existing#3.-Fit-Event-Study-style hash to its unnumbered target viahistory.replaceState+ scroll. Page paths are unchanged.search_as_you_typelive-filtering search dialog (deliberately sequenced after the crash fix).10_trop.ipynb: removed invalidoutputs/execution_countkeys from a markdown cell; normalized cell 27's source from 940 single-character strings to standard line form (this malformation hid one numbered heading from every text tool).sphinx-buildjob: built-HTML anchor tripwires (no digit-leading ids, shim emitted,htmlToTexthook still present) plus a jsdom behavioral smoke test (.github/scripts/search_excerpt_smoke.mjs) that drives the builtsearchtools.js+ shims with every special-anchor class and both legacy-fragment redirect behaviors. jsdom is pinned by a committed manifest + lockfile (npm ci --ignore-scripts, SHA-pinnedactions/setup-node).Methodology references (required if estimator / math changes)
Validation
.github/scripts/search_excerpt_smoke.mjs(new, CI-wired jsdom behavioral test: section-scoped excerpts for apostrophe/parens/colon/legacy-digit anchors + legacy-fragment redirect assertions); built-HTML tripwire step indocs-tests.yml-Wbuild clean from scratch; zero digit-leading ids in built HTML; both shims present on all built pages; browser-verified end-to-end (crash reproduced on the pre-fix build, excerpts render post-fix on the search page and the as-you-type dialog with zero console errors; legacy#3.-Callaway-Sant'Anna-Estimatorlink redirects and scrolls). 174 tutorial-drift/IA/extract tests and 291 doc-snippet/deps tests pass; all 28 notebooks validate against the nbformat schema.Security / privacy