Skip to content

docs: fix search-result excerpts (CSS-special anchors) + enable search-as-you-type#713

Merged
igerber merged 1 commit into
mainfrom
docs-search-anchor-fix
Jul 20, 2026
Merged

docs: fix search-result excerpts (CSS-special anchors) + enable search-as-you-type#713
igerber merged 1 commit into
mainfrom
docs-search-anchor-fix

Conversation

@igerber

@igerber igerber commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Summary

  • Fix the Sphinx search-excerpt crash: notebook section anchors containing CSS-special characters (leading digits, apostrophes, parentheses/commas, colons) made searchtools.js throw SyntaxError in querySelector and silently drop result summaries. docs/_static/searchtools-css-escape.js wraps Search.htmlToText with CSS.escape() (deferred to DOMContentLoaded - search.html emits custom JS before searchtools.js).
  • Strip numeric prefixes from 185 tutorial headings across 22 notebooks via a fence-aware transform with a byte-identity JSON round-trip gate (the 2 digit-leading python comments inside notebook 26's code fence are untouched). Ordering is conveyed by the grouped tutorials index (docs: restructure RTD site around 5 top-level sections (IA overhaul) #703).
  • Reword 50 self-referential "Section N" / "§N" prose references to heading names, across markdown AND code cells; paper-section citations (Butts §3.1/§4, ADH §2.4, Sant'Anna & Zhao Section 4, de Chaisemartin et al. Section 4.2/4.3, Athey-Imbens Section 4, Lumley §2.2) and quoted library-output verdict strings are kept verbatim.
  • Keep old numbered deep links working: docs/_static/legacy-fragment-redirect.js rewrites a no-longer-existing #3.-Fit-Event-Study-style hash to its unnumbered target via history.replaceState + scroll. Page paths are unchanged.
  • Enable the pydata theme's search_as_you_type live-filtering search dialog (deliberately sequenced after the crash fix).
  • Notebook hygiene in 10_trop.ipynb: removed invalid outputs/execution_count keys 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).
  • CI hardening in the sphinx-build job: built-HTML anchor tripwires (no digit-leading ids, shim emitted, htmlToText hook still present) plus a jsdom behavioral smoke test (.github/scripts/search_excerpt_smoke.mjs) that drives the built searchtools.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-pinned actions/setup-node).

Methodology references (required if estimator / math changes)

  • Method name(s): N/A - no methodology changes (documentation navigation/search only; estimator math, assumptions, weighting, defaults, and inference are unchanged)
  • Paper / source link(s): N/A
  • Any intentional deviations from the source (and why): None

Validation

  • Tests added/updated: .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 in docs-tests.yml
  • Backtest / simulation / notebook evidence (if applicable): -W build 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-Estimator link 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

  • Confirm no secrets/PII in this PR: Yes

@github-actions

Copy link
Copy Markdown

Overall Assessment: ✅ Looks good

Executive Summary

  • No estimator, mathematical, weighting, variance, assumption, or default behavior changes.
  • Notebook edits are limited to heading prefixes, textual cross-references, and comments/display labels.
  • No P0/P1 findings.
  • One P2 test-coverage gap exists around script load order.
  • No security or performance concerns found.

Methodology

No findings. diff_diff/ and docs/methodology/REGISTRY.md are unchanged; estimator outputs and executable methodology remain intact.

Code Quality

No findings. Both shims are defensive and syntax-valid.

Performance

No findings. Runtime overhead is negligible; jsdom runs only in CI.

Maintainability

No findings. The Sphinx hook is guarded and checked for upstream drift.

Tech Debt

No findings. No new methodological or correctness debt is introduced.

Security

No findings. Dependencies are lockfile-pinned, install scripts are disabled, actions use commit SHAs, and no secrets were detected.

Documentation/Tests

P2 — Smoke test does not reproduce production load order

  • Location: .github/scripts/search_excerpt_smoke.mjs:L60-L71, docs/_static/searchtools-css-escape.js:L15-L21
  • Impact: Production deliberately loads the shim before searchtools.js, when Search is undefined. The test instead evaluates searchtools.js before the shim and calls htmlToText only after DOMContentLoaded. It could therefore pass if a future template change causes the real search runner to execute before the wrapper.
  • Concrete fix: Construct the jsdom page with separate classic <script> tags in production order—doctools.js, shim, then searchtools.js—and drive the initial search query. Also assert their emitted order in search.html.

@github-actions

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: c0d5b1c990910ff82c9425bae67ae5871ddf9226


Overall Assessment: ✅ Looks good

Executive Summary

  • The prior P2 load-order test gap is resolved.
  • No estimator code, methodology, assumptions, weighting, or inference changed.
  • No P0/P1 findings.
  • One P2 legacy-link collision remains for repeated heading names.
  • No performance or security concerns.

Methodology

No findings. Estimator sources and docs/methodology/REGISTRY.md are unchanged.

Code Quality

No findings. The JavaScript files pass syntax checks and defensively handle missing APIs.

Performance

No findings. Runtime overhead is negligible; jsdom is CI-only.

Maintainability

No findings.

Tech Debt

No findings. The P2 below is not currently tracked, but does not block approval.

Security

No findings. Dependencies and actions are pinned, install scripts are disabled, and no secrets were introduced.

Documentation/Tests

P2 — [Newly identified] Legacy links collide after duplicate headings are de-numbered

  • Location: docs/_static/legacy-fragment-redirect.js:L23-L29; docs/tutorials/06_power_analysis.ipynb:L476, L524; docs/tutorials/09_real_world_examples.ipynb:L77, L318, L611; docs/tutorials/10_trop.ipynb:L658, L675
  • Impact: Several headings now share the same title. The redirect strips the old numeric prefix and always looks up the bare ID, so old links to later occurrences of “Background,” “Inspecting the Search Path,” or “Summary” can land on the first occurrence instead.
  • Concrete fix: Give repeated headings unique descriptive names, add explicit mappings from their old fragments to the resulting IDs, and cover each collision in the legacy-fragment smoke test.

The previous smoke-test finding is resolved by the production-order assertion and loading-state lifecycle harness in .github/scripts/search_excerpt_smoke.mjs:L41-L140.

@github-actions

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 7cf3266d12d03912320c11712096a8e84a97dc38


Overall Assessment: ✅ Looks good

Executive Summary

  • The prior legacy-link collision concern is resolved.
  • No estimator, methodology, weighting, assumptions, defaults, or inference changed.
  • No P0/P1 findings.
  • Search behavior, load order, redirects, and ID uniqueness are covered by CI.
  • No security or performance concerns.

Methodology

No findings. Estimator sources and docs/methodology/REGISTRY.md are unchanged.

Code Quality

No findings. The shims safely handle malformed fragments and unavailable browser APIs.

Performance

No findings. Browser overhead is negligible; jsdom runs only in CI.

Maintainability

No findings. Production script ordering and the Sphinx hook are explicitly guarded.

Tech Debt

No findings. No relevant deferred work was introduced.

Security

No findings. Node dependencies have integrity-pinned lock entries, install scripts are disabled, and the action is SHA-pinned.

Documentation/Tests

No 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 docs/_static/legacy-fragment-redirect.js:L12-L16 and built-page ID uniqueness is enforced in .github/scripts/search_excerpt_smoke.mjs:L175-L195. The smoke test also verifies production load order and actual special-anchor excerpt behavior.

@igerber igerber added ready-for-ci Triggers CI test workflows and removed ready-for-ci Triggers CI test workflows labels Jul 20, 2026
…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.
@igerber
igerber force-pushed the docs-search-anchor-fix branch from 7cf3266 to e022d87 Compare July 20, 2026 14:13
@github-actions

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: e022d8766d4c2a354797f6dcc319f583e32967ec


Overall Assessment: ✅ Looks good

Executive Summary

  • No unmitigated P0/P1 findings.
  • No estimator code, methodology, assumptions, defaults, weighting, or inference changed.
  • Prior legacy-link collision concern remains resolved through ID-uniqueness testing.
  • Search escaping, load order, legacy redirects, and notebook serialization are adequately covered.
  • Dependencies are locked; install scripts and mutable action tags are avoided.

Methodology

No findings. diff_diff/ and docs/methodology/REGISTRY.md are unchanged. Notebook edits only remove heading numbers and replace self-referential section labels.

Code Quality

No findings. The shims safely guard unavailable APIs, malformed fragments, and nonexistent redirect targets (docs/_static/searchtools-css-escape.js:L25-L46, docs/_static/legacy-fragment-redirect.js:L19-L45).

Performance

No findings. Runtime overhead is negligible; jsdom is CI-only.

Maintainability

No findings. Production script order and redirect-target uniqueness are explicitly tested (.github/scripts/search_excerpt_smoke.mjs:L34-L202).

Tech Debt

No findings. No new deferred limitation was introduced.

Security

No findings. jsdom is integrity-locked, lifecycle scripts are disabled during installation, and GitHub actions use commit SHAs.

Documentation/Tests

No 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.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jul 20, 2026
@igerber
igerber merged commit 180c5d9 into main Jul 20, 2026
39 of 40 checks passed
@igerber
igerber deleted the docs-search-anchor-fix branch July 20, 2026 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant