feat: PDF export via headless Chrome with Playwright fallback - #15
Merged
Conversation
Add `investo analyze --pdf [FILE]`, rendering the research note to PDF with no new required dependency. investo.export shells out to a system Chrome/Edge/Chromium/Brave when one is present (the common case), falls back to a Playwright-managed Chromium if that package is installed, and otherwise raises with a message naming all three fixes -- while the .html is always written first, so a failed export still leaves a usable report on disk. Four headless-Chrome details that were each a real bug are handled and commented at their site: a TemporaryDirectory rather than a still-open NamedTemporaryFile (unreopenable by path on Windows), resolve().as_uri() for a correctly escaped file:// URL, a mandatory throwaway --user-data-dir so headless never attaches to a running profile, and verifying the output file exists and is non-empty rather than trusting Chrome's exit code. Rework the analyze output flags to compose: --json, --html and --pdf each do one thing and combine cleanly, where --html previously suppressed --json with no signal. Bare --html/--pdf write investo-<SYMBOL>-<date>.<ext> (ticker sanitised, since M&M.NS is real), parents are created, and a PDF-engine failure exits 2 with the sidecar retained and the reason on stderr. Adds the pdf optional extra, INVESTO_CHROME / INVESTO_PDF_TIMEOUT / INVESTO_EXPORT_DIR config, and offline tests that stub the browser -- covering the exit-zero-but-no-file case, the Windows file-URI escaping, flag composition and the exit-2 contract -- none of which launch a real browser. Verified by generating a 7-page A4 PDF from live KPIT data through headless Chrome.
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
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.
Adds
investo analyze --pdf, rendering the RFC 2 research note to PDF. Third of four RFCs; depends on #14 (merged).No new required dependency
PDF is genuinely optional at the dependency level.
investo.export.html_to_pdftries, in order:PATH, orINVESTO_CHROME) — the common case, zero setup.pip install 'investo[pdf]'andplaywright install chromiumhave been run.PdfExportErrornaming all three remedies — while the.htmlis still written, so a failure leaves a usable report, not an empty hand.Four headless-Chrome details that were each a real bug
Commented at their site in
_chrome_pdf, because every one is a silent failure:TemporaryDirectory+ a file inside it, notNamedTemporaryFile— on Windows a still-open NamedTemporaryFile can't be reopened by path, which is exactly what Chrome must do.resolve().as_uri()for the URL — yieldsfile:///C:/…with spaces percent-escaped;"file://" + str(path)gets both wrong.--user-data-dir— without it, headless can attach to an already-running browser and silently produce nothing (the docs: add Cursor Directory listing (badge + one-click install) #1 "works in CI, not locally" cause).--print-to-pdfcan exit 0 having written nothing. There's a dedicated test for this case.CLI flags now compose
--json,--html,--pdfeach do one thing and combine — previously--htmlsilently suppressed--json. Bare--html/--pdfwriteinvesto-<SYMBOL>-<date>.<ext>(ticker sanitised —M&M.NSis real), parents are created, and a PDF failure exits 2 with the sidecar kept and the reason on stderr.Verification
ruff+mypyclean; 223 tests pass (was 198). The newtest_export.pyandtest_cli.pynever launch a real browser — every test stubssubprocess.runor Playwright.The real proof, though, is a live run:
investo analyze "KPIT Technologies" --pdfproduced a valid 7-page A4 PDF (%PDF-1.4, MediaBox 595×842pt, embedded fonts, clean%%EOF) through headless Chrome on this machine, with the.htmlsidecar alongside.--print-to-pdfhas no@pagemargin-box support, so there are no CSS page numbers — running header/footer only (viaposition: fixed). That's a documented engine limit, not an omission.No breaking changes
--pdfis new; thepdfextra is optional; new config fields have defaults.