fix(export_document): letterhead logo-only for composed reports (stag… - #902
Merged
Conversation
…ing smoke)
The staging re-smoke showed the render approach (Slice 1.1) no longer leaked raw
Jinja, but produced tenant "None"/partial junk ("g", "(Demo)", "None") in the
header: a Letter Head is a document-BOUND template (company address/contact keyed
off the printed doc) and a composed report has no such doc — an unwinnable render.
Pivot to LOGO-ONLY for composed reports: resolve_letterhead now extracts only the
Letter Head's <img> logo(s) (bounded first, then inlined to permission-checked
base64 + the nh3 data-only gate) and DROPS the doc-bound Jinja/text block. No
render, so no raw tags and no "None"/junk — deterministic and clean. A tenant that
wants text branding on a report passes header=/footer= explicitly (the agent
already supplies its own report title/footer). Removed _render_letterhead /
_letterhead_context / _JINJA_RE (no longer rendering the letterhead).
Tests: replaced the render-template cases with _letterhead_logos (extract img,
drop text+Jinja, size-cap) and updated the resolve tests (logo kept, text/Jinja
dropped, remote img still stripped by the inline gate). 165 site-free tests + 151
subtests green; ruff clean.
⚠️ Still render-behavior — confirm on the staging re-smoke: header shows the logo
(or nothing for a text-only letterhead), never raw template or "None" junk.
Co-Authored-By: Claude Opus 4.8 <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.
…ing smoke)
The staging re-smoke showed the render approach (Slice 1.1) no longer leaked raw Jinja, but produced tenant "None"/partial junk ("g", "(Demo)", "None") in the header: a Letter Head is a document-BOUND template (company address/contact keyed off the printed doc) and a composed report has no such doc — an unwinnable render.
Pivot to LOGO-ONLY for composed reports: resolve_letterhead now extracts only the Letter Head's![]()
logo(s) (bounded first, then inlined to permission-checked base64 + the nh3 data-only gate) and DROPS the doc-bound Jinja/text block. No render, so no raw tags and no "None"/junk — deterministic and clean. A tenant that wants text branding on a report passes header=/footer= explicitly (the agent already supplies its own report title/footer). Removed _render_letterhead / _letterhead_context / _JINJA_RE (no longer rendering the letterhead).
Tests: replaced the render-template cases with _letterhead_logos (extract img, drop text+Jinja, size-cap) and updated the resolve tests (logo kept, text/Jinja dropped, remote img still stripped by the inline gate). 165 site-free tests + 151 subtests green; ruff clean.
Summary
Pre-merge checklist
testscheck on this PR passes (never merge on ❌)main(so it is tested against the latest code)