Skip to content

fix: surface BusinessReport appendix render failures#574

Merged
igerber merged 1 commit into
igerber:mainfrom
shawcharles:fix/business-report-appendix-warning
Jun 29, 2026
Merged

fix: surface BusinessReport appendix render failures#574
igerber merged 1 commit into
igerber:mainfrom
shawcharles:fix/business-report-appendix-warning

Conversation

@shawcharles

@shawcharles shawcharles commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Closes #573.

Summary

This PR makes BusinessReport.full_report(include_appendix=True) visibly report when the estimator-native appendix cannot be rendered.

Previously, if the underlying estimator's summary() raised, full_report() caught the exception and silently omitted the Technical Appendix. The report could therefore look complete while missing the expected technical section.

The new behaviour keeps full_report() resilient while adding a bounded appendix-unavailable note. It covers failures while obtaining the estimator summary and while converting that appendix content to markdown text. It includes the exception class name, but intentionally does not include raw exception messages or tracebacks, since those may expose local paths, formulas, data values, or other internals.

Changes

  • Preserve existing code-fenced Technical Appendix output when estimator summary() succeeds.
  • Add a visible ## Technical Appendix fallback when appendix rendering fails.
  • Add regression coverage for visible failure reporting, stringification failures, and non-disclosure of raw exception details.

Validation

  • PYTHONPATH=. DIFF_DIFF_BACKEND=python pytest tests/test_business_report.py::TestAppendix -q
  • PYTHONPATH=. DIFF_DIFF_BACKEND=python pytest tests/test_business_report.py -q
  • PYTHONPATH=. DIFF_DIFF_BACKEND=python .venv-py39/bin/python -m pytest tests/test_business_report.py::TestAppendix -q
  • python -m ruff check diff_diff/business_report.py tests/test_business_report.py
  • git diff --check

@shawcharles shawcharles force-pushed the fix/business-report-appendix-warning branch from d2cd9a3 to f64b652 Compare June 28, 2026 21:03
@igerber

igerber commented Jun 29, 2026

Copy link
Copy Markdown
Owner

@shawcharles this looks like a worthwhile contribution and solves a real issue you surfaced. Is it ready for review from your POV?

@shawcharles shawcharles force-pushed the fix/business-report-appendix-warning branch from f64b652 to 8fe2470 Compare June 29, 2026 20:31
@shawcharles shawcharles marked this pull request as ready for review June 29, 2026 20:31
@shawcharles

Copy link
Copy Markdown
Contributor Author

Yes, ready for review from my side.

I rebased the branch onto current upstream/main and reran the relevant checks:

  • PYTHONPATH=. DIFF_DIFF_BACKEND=python pytest tests/test_business_report.py::TestAppendix -q
  • PYTHONPATH=. DIFF_DIFF_BACKEND=python pytest tests/test_business_report.py -q
  • PYTHONPATH=. DIFF_DIFF_BACKEND=python .venv-py39/bin/python -m pytest tests/test_business_report.py::TestAppendix -q
  • git diff --check upstream/main...HEAD

I also moved the PR out of draft.

@shawcharles

Copy link
Copy Markdown
Contributor Author

Follow-up: the current CI Gate failure is only the repository label gate:

The ready-for-ci label is required to run CI tests.

I do not have permission to apply that label, so from my side this is ready for maintainer review/CI whenever you want to label it.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jun 29, 2026
@igerber igerber merged commit 8b91688 into igerber:main Jun 29, 2026
30 of 31 checks passed
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.

Make BusinessReport appendix summary failures visible

2 participants