MBTI-PDF-RESULT-RENDER-CLEANUP-03: clean result-page PDF print surface#1492
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7078983480
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| '[data-site-header="true"]', | ||
| '[data-site-footer="true"]', |
There was a problem hiding this comment.
Limit PDF blocker scan to the printable surface
On localized result/print pages, SiteChrome still renders SiteHeader and SiteFooter around this client, and this checker scans the whole document with document.querySelector. Because these newly-added header/footer selectors are always present in the DOM even when CSS hides them for print, hasMbtiPdfReadySections() never becomes true, so the fermat-pdf-ready marker and window.__FERMAT_PDF_READY__ are never set for otherwise valid MBTI PDFs.
Useful? React with 👍 / 👎.
| import ResultClient from "./ResultClient"; | ||
|
|
||
| const RESULT_PAGE_PDF_SURFACE = "mbti.result_page_export.v1"; | ||
| const RESULT_PAGE_PDF_SURFACE = "mbti.result_page_export.v2"; |
There was a problem hiding this comment.
Use one PDF surface version for token checks
This route now only enters PDF mode for surface=mbti.result_page_export.v2, but verifyResultPagePdfToken() still accepts only payloads whose surface is mbti.result_page_export.v1. Any signed v2 link reaches this branch and then 404s during verification, while old v1 links no longer enable pdfMode, so the result-page PDF entry point is effectively broken until the verifier/token issuer and route agree on the same surface value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a5d86e3ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const lines = cleanText(section.bodyMd) | ||
| .split(/\n+|(?:\s+-\s+)/) | ||
| .map((line) => cleanText(line)) | ||
| .filter(Boolean); |
There was a problem hiding this comment.
Render payload-backed projection copy in the PDF
When mbti_public_projection_v1.sections use payload-backed renderers such as bullets, letters_intro, trait_dimension_grid, or preferred_role_list, the user-visible copy is in section.payload, not bodyMd (the existing MBTI projection fixture has many such sections). This helper only reads bodyMd, so those cards render as title-only blocks while the PDF is still marked ready, producing incomplete result-page PDFs for otherwise valid reports.
Useful? React with 👍 / 👎.
| return [ | ||
| { | ||
| key: "personality-traits", | ||
| title: "Personality Traits", |
There was a problem hiding this comment.
Localize the PDF section headings
For locale === "zh", this new PDF shell still uses English top-level headings because the section titles are hard-coded here while the component otherwise receives locale and localizes other copy. Chinese result-page PDF exports will therefore be mixed-language even when the projection content is localized.
Useful? React with 👍 / 👎.
What changed
Why
The result-page PDF export should render the actual four core result modules without site chrome, tools, placeholders, or interactive UI contaminating the PDF body.
Validation
pnpm vitest run tests/contracts/result-gotenberg-print-route.contract.test.ts tests/contracts/result-client-view-state.contract.test.tsxpnpm typecheckgit diff --checkIntentionally deferred