Repository navigation
fix(pdf): release export captures; record the architecture audit - #121
Conversation
Run the improve-codebase-architecture process against the reviewed baseline and record the result in the audit: four candidates with deletion-test verdicts, the size of each focused slice, findings the first pass missed, and corrections to the first pass's own wording. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P2GhoLCcxJsj66fwofnxw9
Make the Markdown the standalone record, remove the drifting HTML copy and the structural-check JSON, and narrow the publication, loader, PDF, and engine conclusions to what the reviewed source supports.
State that the publication work starts from those guarantees rather than a class merger.
leoafarias
left a comment
leoafarias
left a comment
There was a problem hiding this comment.
Documentation source review: accepted
Carrying forward the source-review outcome from #120 to this replacement PR. The head remains the exact reviewed commit, d0f7bcce7fe8b472fcba201f8d99d30a6ecc23d8. I rechecked the replacement PR's head and changed-file patch: it adds only the standalone Markdown audit. No new factual corrections are requested.
This is a review comment, not a merge action or permission to merge. Keep this PR open for the maintainer's decision.
Implementation remains separate from this documentation PR. Start with the ownership regressions in section 1: saves must not mix Markdown, artwork, and theme from different decks; a newer saved-deck selection must survive older generation completion; malformed opening and failed or abandoned result application must leave the previous deck coherent. Treat refresh() operation ownership independently. Follow with memory reload terminal completion and focused PDF entry/terminal cleanup.
A class merger, late-subscriber replay, and a new public PDF export-session module remain unproven design choices, not accepted implementation decisions. No builds, application runs, or tests were performed for this review, and these implementation scenarios remain unexecuted.
…, save, and open Opening a saved deck now withdraws an in-flight generation's automatic publication; the retained result can still be shown or saved explicitly, and a save writes that result's Markdown, artwork, and theme together. The open saved deck is released only when a generated deck is accepted. Overlapping save, open, and refresh calls can no longer overwrite newer state, an open switches only after its Markdown decodes, and MemoryDeckLoader.reload re-emits the last terminal result.
Only one export runs at a time. Captured image bytes are released when an export fails, is cancelled, or the controller is disposed, and the dialog ignores a completion from a controller it has replaced or disposed. The dialog accepts a capture service for tests.
Resolve against #122 and #123 without restoring what they removed: - Keep DeckLibraryController.openDeck; the ownership fix now reads it in production. - Serialize retained generations with SlideSerializer instead of reviving DeckMarkdownCodec.encode. - Drop the PDF dialog's disposed check rather than restoring PdfController.disposed; the dialog disposes a controller only when it replaces it or unmounts. - Point imports and audit references at features/ai/generation, and state the renderer, assembly, and Mermaid decisions inline now that their notes are gone.
|
Visit the preview URL for this PR (updated for commit ff533c2): https://superdeck-dev--pr121-docs-architecture-ad-rdit4vgv.web.app (expires Fri, 23 Oct 2026 02:02:56 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: bd68fc230762285849207e7e120aaf87cd4ca2f9 |
The saved-deck library is being replaced by a one-shot export, which removes the concurrent saved and generated decks this fix coordinated. The MemoryDeckLoader.reload fix moves to that change.
leoafarias
changed the title
2 participants
Intent
Two things, relative to
main:superdeck_pdf).PdfController.export()runs one export at a time, and ignores calls afterdispose().SlideCaptureService, which the lifecycle tests use. It isn't part of the plugin's public exports.docs/maintainers/architecture-review/superdeck_architecture_adversarial_audit.mdrestores the corrected audit from docs: record corrected SuperDeck architecture audit #120, which was merged and then reverted. It now opens with a status note: refactor(playground)!: replace the saved-deck library with deck export #124 replaces the saved-deck library it examines, so its publication, saving and ownership findings no longer apply.History on this branch, for context: a saved-deck ownership fix was added and then reverted here, because #124 removes the library that fix coordinated. That fix's
MemoryDeckLoader.reloadchange moved to #124. The branch leaves no playground changes.Impacted packages
superdeck_pdf(code, tests, changelog) and docs. No other package changes.Commands run
fvm dart analyze --fatal-infosinpackages/plugins/pdf: no issuesfvm flutter testinpackages/plugins/pdf: 39 passing, including the newpdf_export_lifecycle_test.dartmain: analysis clean in all 7 packages; playground tests passing