fix(export): save the note that's on screen as PDF, not always the Standard one - #426
Conversation
…andard one
Reported while testing: with a generated template report open, "Save notes as
PDF…" exported the Standard structured note instead of the report on screen.
Root cause: saveNotesPdf called buildNotesHtml(noteSections) and never passed
activeReport — the builder's input type had no slot for one. It was a deliberate
scoping decision ("the branded template is structured-note shaped"), but it left
the two adjacent menu actions disagreeing: stenolabs#318 taught "Copy notes" to follow
the screen and the PDF path was never given the same treatment.
A report is free-form markdown, so the fix serialises it with the SAME renderer
the detail view uses (react-markdown via renderToStaticMarkup) and drops the
result into the branded shell. Reusing the view's renderer is the point — a
second markdown implementation inside notesPdf.ts would drift from what the
user sees.
- buildNotesHtml takes an optional pre-rendered report; when present it REPLACES
the Standard sections, mirroring buildNotesCopyText's short-circuit.
- The branded shell moved into renderDocument() so both paths share chrome,
fonts and layout and can't drift.
- CSS for the element types markdown brings that the structured note never
emits: body headings, ordered lists, emphasis, code, quotes, tables.
- canExportNotesPdf counts an open report on its own, so a transcript-only note
with a generated report can be exported at all (it could not before).
The report HTML is injected unescaped by necessity; notesPdf.ts documents why
that is safe (react-markdown emits no raw HTML without rehype-raw, which is not
enabled) and the document CSP stays the second line of defence.
Tests: four unit cases in notesPdf.test.ts (three failed before the fix) and a
notes-pdf-export.t1 case driving the real UI — open the seeded report, export,
assert the file carries the RENDERED report, the template name, no Standard
sections, and no leaked reasoning. Verified the e2e case fails without the fix.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/renderer/src/lib/notesPdf.ts">
<violation number="1" location="app/renderer/src/lib/notesPdf.ts:284">
P3: Links in generated reports fall back to the browser's default visited/blue link styling in the PDF, unlike the on-screen report and the paper/ink design. Add a scoped anchor rule so exported report links remain readable and branded.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| border: 1px solid var(--rule); padding: 5px 7px; text-align: left; vertical-align: top; | ||
| } | ||
| .report th { background: var(--paper-1); font-weight: 600; } | ||
| .report hr { border: none; border-top: 1px solid var(--rule); margin: 14px 0; } |
There was a problem hiding this comment.
P3: Links in generated reports fall back to the browser's default visited/blue link styling in the PDF, unlike the on-screen report and the paper/ink design. Add a scoped anchor rule so exported report links remain readable and branded.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/renderer/src/lib/notesPdf.ts, line 284:
<comment>Links in generated reports fall back to the browser's default visited/blue link styling in the PDF, unlike the on-screen report and the paper/ink design. Add a scoped anchor rule so exported report links remain readable and branded.</comment>
<file context>
@@ -192,6 +238,50 @@ ${listItems(input.actionItems)}
+ border: 1px solid var(--rule); padding: 5px 7px; text-align: left; vertical-align: top;
+ }
+ .report th { background: var(--paper-1); font-weight: 600; }
+ .report hr { border: none; border-top: 1px solid var(--rule); margin: 14px 0; }
footer {
margin-top: 30px; padding: 10px var(--inset) 0; border-top: 1px solid var(--rule);
</file context>
| .report hr { border: none; border-top: 1px solid var(--rule); margin: 14px 0; } | |
| .report a { color: var(--ink-900); text-decoration: underline; } | |
| .report hr { border: none; border-top: 1px solid var(--rule); margin: 14px 0; } |
Merge order note: this overlaps #448Flagging an overlap rather than proposing a decision - the order is yours. This PR and #448 (Share menu) touch the same four files, and two of the overlaps are semantic rather than textual: 1. #448 adds two new entries ( 2. 3. Suggested order: this one first, then rebase #448. It is the smaller and older change, it resolves #448's open caveat rather than colliding with it, and the reverse order would mean pulling this PR through #448's menu restructuring. Happy to do the #448 rebase as soon as this lands. |
One conflict, in app/package.json: main's stenolabs#440 appended update-error-copy.test.js to test:unit while this branch appended share-temp.test.js. Both kept — the lists are additive and both files exist. The overlap flagged in the PR's merge-order note is now resolved by main itself: stenolabs#426 landed, so "Save notes as PDF" already follows the note on screen. The semantic half of that integration is the commit that follows this one.
Integrating main after stenolabs#426 landed. Git merged both sides cleanly, and that is the problem: stenolabs#426 taught "Save notes as PDF" to export whichever note is on screen, but "Share notes as PDF" was written on this branch in parallel and still called buildNotesHtml(noteSections). Merged, the two adjacent entries in the same menu disagreed about the same document - with a template report open, sharing sent the Standard structured note. That is the exact bug stenolabs#426 was filed for, one entry lower. Both PDF surfaces now go through one buildNotesPdfHtml(), so they cannot drift apart again. It is a function rather than a memo for the reason stenolabs#426 gives: the branded shell is ~45KB with its base64 font, so it is built on click. Also repoints notes-pdf-export.t1's report case at the Share menu. This branch moves the save entries out of the "..." menu, so the spec's 'More options' click opened a menu that no longer holds "Save notes as PDF" - it timed out on the merge, not on a product fault. Its sibling case in the same file already used the Share fixture. Tests: a share-menu.t1 case asserting the SHARED payload carries the rendered report, the template name, no Standard sections and no leaked reasoning. It needs the whole payload, not the log's 200-char head - a branded PDF's first 200 chars are doctype and font CSS - so the share mock gains STENOAI_E2E_SHARE_PAYLOAD_PATH, the seam STENOAI_E2E_EXPORT_PATH already gives the save path. Verified the case fails without the fix (the shared HTML has no report content) and passes with it; full T1 suite green, 68 passed.
2 participants
Found while testing a build: with a generated template report open, "Save
notes as PDF…" exported the Standard structured note instead of the report on
screen.
Root cause
saveNotesPdfcalledbuildNotesHtml(noteSections)and never passedactiveReport— the builder's input type had no slot for one. That was adeliberate scoping decision ("the branded template is structured-note
shaped"), but it left the two adjacent menu actions disagreeing: #318 taught
Copy notes to follow the screen, and the PDF path was never given the same
treatment.
Fix
A report is free-form markdown, so it is serialised with the same renderer the
detail view uses (
react-markdownviarenderToStaticMarkup) and dropped intothe branded shell. Reusing the view's renderer is the point: a second markdown
implementation inside
notesPdf.tswould drift from what the user is looking at.buildNotesHtmltakes an optional pre-rendered report; when present itreplaces the Standard sections, mirroring
buildNotesCopyText'sshort-circuit.
renderDocument()so both paths share chrome,fonts and layout and cannot drift.
emits: body headings, ordered lists, emphasis, code, quotes, tables. Without
it they inherited the structured note's list rules and lost their numbering.
canExportNotesPdfnow counts an open report on its own. Atranscript-only note (auto-summarise off) with a generated report had the
action disabled outright — the report could not be exported at all.
The report HTML is injected unescaped by necessity.
notesPdf.tsdocuments whythat is safe:
react-markdownemits no raw HTML from its input unlessrehype-rawis enabled (it is not), and the document's CSP(
default-src 'none') remains the second line of defence.Testing
notesPdf.test.ts— three failed before the fix, so thebug was pinned by tests before any code changed.
notes-pdf-export.t1case driving the real UI: open the seeded report,export, then assert the written file carries the rendered report, the
template name, no Standard sections, and no leaked
<think>reasoning.Typecheck clean, lint 0 errors, unit 255 node:test + 124 vitest, T1 suite 50/50.
Summary by cubic
Fixes “Save notes as PDF…” exporting the Standard note when a template report is open. The PDF now matches what’s on screen and can export generated reports even when the structured note is empty.
react-markdownviarenderToStaticMarkup) so the PDF matches the UI.buildNotesHtmlaccepts an optional pre-rendered report and replaces the Standard sections when present.renderDocument()and added CSS for markdown elements (headings, ordered lists, code, tables) to keep numbering and layout correct.canExportNotesPdfnow treats an open report as exportable on its own, enabling export for transcript-only notes.Written for commit 6ce64de. Summary will update on new commits.