docs: custom autosummary template + residual triage (PR 2.5)#410
docs: custom autosummary template + residual triage (PR 2.5)#410
Conversation
Closes the gap PR #407 left: 614 of 635 PR #407-residual warnings were member-level duplicate-object descriptions because :no-index: on page-level autoclass directives doesn't propagate to autodoc-generated members. PR 2.5 adds a custom autosummary class.rst template with :no-members: on the stub autoclass, and a function.rst template with :no-index: on the stub autofunction. All 50 class stubs and 48 function stubs are auto-regenerated on the next clean build (autosummary_generate_overwrite=True default). Architecture: - Class cross-refs resolve to stub URL (per PR #407 page-level :no-index:) - Member cross-refs resolve to page-level URL (only place still registered) - Function cross-refs resolve to page-level URL (stub now :no-index:) - Page-level pages keep current full inline member rendering (UX preserved) - Stubs become "header + autosummary tables linking to page-level" Also addresses 18 non-duplicate residuals so make html now warns 0: - 4 footnote-not-referenced + 1 def-list blank-line in source docstrings - 4 :doc: refs to .md files (myst-parser absent) converted to literal text - 4 broken markdown links to REGISTRY.html in T18/T19 notebooks - 3 orphan benchmarks.rst citations removed - T19 + T20 added to Tutorials: Business Applications toctree - 2 stale autosummary entries fixed (DiDResults.ci, CallawaySantAnnaResults.aggregate) - 3 page-vs-page function dups (chaisemartin_dhaultfoeuille, stacked_did, trop) resolved by adding :no-index: to page-level autofunction directives and switching 3 path-qualified autoclass directives to top-level paths Result: Sphinx make html went 2,170 (PR #407 start) -> 0 warnings (PR 2.5 end). This unblocks PR 3 (queued: -W enforcement in docs-tests.yml). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Looks goodExecutive Summary
MethodologyNo P0/P1 findings. Affected methods checked:
Code QualityNo P0/P1/P2 findings. Pattern-wide inference grep found no new inline inference computation in the changed Python hunks. Existing project-wide hits are outside this PR’s changed code. PerformanceNo findings. The PR is documentation/template focused and does not alter runtime estimator paths. MaintainabilityNo P0/P1 findings. P3 — Informational: tracked docs tech debt addressed Impact: The autosummary refresh and custom templates directly address existing Sphinx duplicate/stale-stub debt already tracked in Concrete fix: No action required. Tech DebtNo untracked blocker-level tech debt found. The remaining limitation around documentation inventory below is not statistical correctness and does not block approval. SecurityNo findings. I grepped the non-notebook diff for common secret patterns and did not find credentials or private keys. Documentation/TestsP2 — Shadowed convenience functions lose Python-domain targets Impact: The three page-level convenience functions that shadow module names are now documented with Concrete fix: Keep exactly one indexed target for each shadowed function. A clean shape is to replace the page P3 — Validation not locally reproduced Impact: I did not run Concrete fix: Rely on CI’s Sphinx build for final confirmation, or run |
Reviewer flagged that page-level :no-index: on .. autofunction:: (added
in initial PR 2.5 commit to resolve 3 page-vs-page duplicates) made
diff_diff.{trop,stacked_did,chaisemartin_dhaultfoeuille} convenience
functions absent from the py:function inventory entirely - so :py:func:
cross-refs and intersphinx pointers to those public functions wouldn't
resolve.
Empirical investigation surfaced the actual conflict source: each of the 3
files had a top-level ``.. module:: diff_diff.<name>`` directive that
registered the module FQN, colliding with the function FQN of the same
name. Removing the .. module:: directive lets .. autofunction:: register
the function as the canonical (and only) :py:function entry. The
.. autoclass:: directives on those pages were already converted to
top-level paths (diff_diff.X form) in the initial PR 2.5 commit and
don't depend on the implicit module context.
Verified: make html shows 0 ERRORs / 0 WARNINGs / 0 duplicates; all 3
convenience functions present in objects.inv as py:function entries.
Doc-snippet test parity preserved (111 passed, 4 skipped).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/ai-review |
|
🔁 AI review rerun (requested by @igerber) Head SHA: ✅ Looks goodExecutive Summary
MethodologyNo P0/P1 findings. Severity: P3-informational Concrete fix: No action required. Severity: P3-informational Concrete fix: No action required. Code QualityNo P0/P1/P2 findings. Pattern-wide grep for inline inference anti-patterns ( PerformanceNo findings. The PR is documentation and Sphinx-template focused; no runtime estimator path changes were introduced. MaintainabilityNo P0/P1 findings. Severity: P3-informational Concrete fix: No action required. Tech DebtNo unmitigated blocker-level tech debt found. The docs-warning cleanup is already covered by SecurityNo findings. Secret-pattern grep over changed non-notebook source/docs found no obvious credentials, private keys, or tokens. Documentation/TestsSeverity: P2 Concrete fix: Add indexed page-level directives for the three functions, likely Severity: P3-informational Concrete fix: Rely on CI’s Sphinx build, or run |
R2 reviewer flagged 3 plotting helpers (plot_bacon, plot_power_curve, plot_pretrends_power) as having :no-index: stubs without page-level indexed replacements. Added page-level .. autofunction:: directives in visualization.rst, power.rst, and pretrends.rst. Proactive sweep (per feedback_ci_reviewer_pattern_checks) surfaced 7 sibling dataset loaders (load_card_krueger, load_castle_doctrine, load_divorce_laws, load_mpdta, load_dataset, list_datasets, clear_cache) with the same class of issue: docs/api/datasets.rst used bare names under .. module:: diff_diff.datasets, registering them at diff_diff.datasets.X instead of the canonical diff_diff.X export FQN. Removed the .. module:: directive and switched to fully-qualified diff_diff.X names so they appear at the same FQN as their autosummary listings. Verified: full coverage sweep finds 51/51 functions and 59/59 classes in py:function / py:class inventory under their canonical diff_diff.X FQN. make html shows 0 ERRORs / 0 WARNINGs / 0 duplicates. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/ai-review |
|
🔁 AI review rerun (requested by @igerber) Head SHA: ✅ Looks goodExecutive Summary
MethodologySeverity: P3-informational Severity: P3-informational Code QualityNo P0/P1/P2 findings. Pattern-wide grep for inline inference anti-patterns found no new affected Python hunks. The only Python changes are docstrings. PerformanceNo findings. The PR is documentation/Sphinx infrastructure only; no runtime estimator path changes were introduced. MaintainabilityNo P0/P1/P2 findings. Static mirror checks passed: all class stubs have Workflow transitive-dependency audit: no workflow or pytest-selection changes were in the supplied diff. Existing docs workflow paths already include docs, package code, Tech DebtSeverity: P3-informational SecurityNo findings. Secret-pattern scan over changed Python/RST/Markdown surfaces, excluding the notebook carve-out paths, found no obvious credentials or private keys. Documentation/TestsNo P0/P1/P2 findings. Severity: P3-informational Severity: P3-informational |
Per PR #410 R3 reviewer P3-informational: TODO.md still listed Sphinx duplicate-object, autodoc-import-failure, EDiDBootstrapResults cross-ref ambiguity, and stale-autosummary-stubs entries that this PR resolves. Removed: - L134 ~1583 duplicate object description Sphinx warnings (now 0) - L137 Sphinx autodoc fails to import DiDResults.ci, MultiPeriodDiDResults.att, CallawaySantAnnaResults.aggregate (typo fixes + template change) - L138 EDiDBootstrapResults cross-reference ambiguous (no "more than one target" warnings remain in build log) - L139 Tracked autosummary stubs stale + 6 untracked (PR 2.5 regenerates all from the new templates and commits the previously-untracked ones) Kept L135 (.txt AI guides outside CI) and L136 (doc-deps.yaml integrity) since neither is in PR 2.5 scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/ai-review |
|
🔁 AI review rerun (requested by @igerber) Head SHA: ✅ Looks goodExecutive Summary
MethodologyNo P0/P1/P2 findings. Severity: P3-informational Severity: P3-informational Code QualitySeverity: P3 PerformanceNo findings. The PR changes documentation infrastructure and docstrings only. MaintainabilityNo P0/P1/P2 findings. Static mirror checks passed: class stubs have Tech DebtNo findings. The prior Sphinx/autosummary TODO rows are gone; remaining review debt starts at SecurityNo findings. Secret-pattern scan over changed text surfaces, excluding notebook carve-outs, found no credentials or private keys. Documentation/TestsSeverity: P2 Severity: P3-informational |
R3 findings:
- P2: plot_bacon section was inserted before plot_group_time_heatmap's
Example block, so the heatmap example rendered under the plot_bacon
heading. Reordered to put plot_bacon AFTER the heatmap example, and
added a real plot_bacon example.
- P3: docs/_templates/autosummary/class.rst had {% block %} tags with
3-space indent that produced trailing-whitespace blank lines in every
generated stub. Switched the block tags to column 0 (no indent) so
Jinja control flow doesn't leak indentation into the rendered output.
Regenerated all 50 class stubs - now whitespace-clean.
Verified: 0 ERRORs / 0 WARNINGs, 0 stubs with trailing whitespace,
git diff --check passes, page-level rendering preserved (286 <dt count
on api/results.html, 1 class header on stub).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/ai-review |
|
🔁 AI review rerun (requested by @igerber) Head SHA: ✅ Looks goodExecutive Summary
MethodologySeverity: P3-informational Severity: P3-informational Code QualityNo P0/P1/P2 findings. Pattern-wide grep found no new inline inference anti-pattern in modified code. Existing matches are PerformanceNo findings. The PR changes documentation infrastructure, generated RST stubs, and docstrings only. MaintainabilityNo P0/P1/P2 findings. The autosummary mirror is consistent: class stubs use Reciprocal checks also passed: page-level Tech DebtNo findings. The prior Sphinx duplicate/stale-stub TODO rows have been removed; the remaining docs/testing debt starts at SecurityNo findings. Secret-pattern scan over changed text surfaces, excluding notebook carve-outs, found no credentials, tokens, or private keys. Documentation/TestsSeverity: P3-informational |
CI's "Validate RST code snippets" job failed with 2 errors after the plot_bacon section was inserted in docs/api/visualization.rst: 1. block8 (new plot_bacon example) used wrong API: BaconDecomposition.fit() takes first_treat=, not treatment=. Fixed to first_treat='first_treat'. 2. block9 (Plotly Backend, was block8 before insertion) raised NameError on 'results' because it relies on context from earlier blocks. The _CONTEXT_DEPENDENT_SNIPPETS allowlist had api_visualization:block8 which now points to my new plot_bacon block (which is standalone and does NOT need leniency); the actual context-dependent block moved to index 9. Updated allowlist entry to api_visualization:block9. Verified locally: both blocks now pass under PYTHONPATH=. DIFF_DIFF_BACKEND=python pytest Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/ai-review |
|
🔁 AI review rerun (requested by @igerber) Head SHA: ✅ Looks goodExecutive Summary
MethodologySeverity: P3-informational Severity: P3-informational Code QualityNo findings. Pattern-wide grep found no new inline inference computation in modified code. Existing matches are PerformanceNo findings. The PR changes documentation infrastructure, generated RST stubs, and docstrings only. MaintainabilityNo findings. Autosummary symmetry checks passed: class stubs use Reciprocal indexing checks passed: page-level Tech DebtNo findings. The stale Sphinx TODO rows are gone; the remaining Testing/Docs debt starts at SecurityNo findings. Secret-pattern scan over changed text surfaces, excluding the explicitly carved-out notebook paths, found no credentials, tokens, or private keys. Documentation/TestsSeverity: P3-informational The |
Adds a sphinx-build job to .github/workflows/docs-tests.yml that runs make -C docs html SPHINXOPTS="-W" so warnings become build failures. This locks in the 0-warning state PR #410 achieved and blocks any future warning from sneaking in. The job mirrors the RTD build (.readthedocs.yaml): Python 3.11, pandoc apt package for nbsphinx, and the same docs extras pinned in pyproject.toml [project.optional-dependencies.docs]. Also adds tutorials/21_had_pretest_workflow to the Tutorials: Business Applications toctree at docs/index.rst:84. PR #409 landed this notebook without a toctree entry; -W enforcement caught it on the local smoke test before push. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
Closes the Sphinx WARNING gap PR #407 left. PR #407 dropped warnings from 2,170 to 635 (71%) by adding `:no-index:` to all 78 page-level `.. autoclass::` directives. The empirical spike on `bacon.rst` revealed that `:no-index:` on autoclass suppresses CLASS-level cross-reference registration only — it doesn't propagate to autodoc-generated members when `:members:` is implicit (via `autodoc_default_options["members"] = True` in `docs/conf.py:39-44`). 614 of those 635 residuals were member-level duplicates.
PR 2.5 closes both that gap and the 21 non-duplicate residuals so PR 3 (queued: `-W` enforcement in `docs-tests.yml`) can land cleanly.
Result: `make html` is now 0 ERRORs / 0 WARNINGs (down from 2,170 at PR #407 start; 635 at PR #407 merge).
Architecture
User-decided this session: custom autosummary class template that adds `:no-members:` to the autoclass in stubs (Option A). Effect:
Sphinx's `autosummary_generate_overwrite=True` default means a single clean build regenerates all 50 class stubs + 48 function stubs from the new templates — no hand-edit per stub.
Spike-before-bulk discipline
Per `feedback_spike_before_bulk_mechanical_edit` (lesson saved from PR #407): the plan included an explicit empirical spike of just the class.rst template. The spike caught that 41 of 44 remaining residuals were FUNCTION duplicates (not class), surfacing the need for a function.rst template too. The remaining 3 page-vs-page were pinned to specific module-name conflicts (`diff_diff.chaisemartin_dhaultfoeuille`, `diff_diff.stacked_did`, `diff_diff.trop` — function names that shadow module names) and resolved with targeted page-level `:no-index:` on the autofunction. Without the spike, the bulk pass would have committed to a 1-template architecture that left ~44 function dups unhandled.
Constructor visibility note
The new class template removes the explicit `.. automethod:: init` line from stubs (would re-register `init` and defeat the `:no-members:` suppression). The constructor signature still appears on page-level via `autodoc_class_signature = "separated"` (`docs/conf.py:46`). Stubs lose the per-class `init` rendering; page-level keeps it.
Residual triage (Cat A–E)
Five categories of non-duplicate warnings, all fixed:
Plan-reviewer found that the original Cat E plan (suggested `suppress_warnings = ["autosummary"]` based on "Rust backend not built") was wrong: Sphinx emits autosummary import-failure warnings without `type=`, so the suppress filter is a no-op. The actual fixes were RST typos.
Page-vs-page function fix (Cat F surprise)
Spike rebuild surfaced 3 final duplicates from `chaisemartin_dhaultfoeuille`, `stacked_did`, `trop` — function names that shadow their module names. The conflict was Python's same-namespace ambiguity: `from .X import X` shadows the module, but Sphinx still registers both. Fix: `:no-index:` on the page-level autofunction (3 sites). Also switched 3 path-qualified autoclass directives to top-level paths (`diff_diff.X.Y` → `diff_diff.Y`) for consistency with project convention (PR #406 export pattern).
Methodology references
Validation
Security / privacy