Skip to content

PDF persistence — PR1 (storage primitives + config)

1. Issue reference

  • GitHub issue: PDF persistence epic (persist source PDFs to Supabase Storage)
  • Issue title: Persist source PDFs to self-hosted Supabase Storage — PR1 storage primitives
  • Issue type: feature
  • Milestone: TBD

2. Summary

  • What this issue changed: Added a new stateless storage_service module with the storage primitives (compute_sha256, derive_storage_path, is_archived, archive_pdf, signed_url_for) plus the resolve_pdf_bytes resolver (defined but not wired into ingestion), and two config settings (SUPABASE_PDF_BUCKET, PDF_ARCHIVE_ENABLED).
  • Why the change was needed: every ingest currently downloads-and-discards the source PDF. This is PR1 of 3 — the primitives + config + unit tests that PR2 (resolver wiring) and PR3 (backfill script) build on. No runtime behavior change in this PR: nothing calls the new code path yet.

3. Initial repo state

  • Relevant behavior before implementation: ingestion.py fetches fetch_pdf(ref["pdf_source"]) on every job and discards the bytes after parse/OCR. pdf_fetcher.validate_pdf_source_url already special-cases Supabase Storage presigned reads but rejects any path containing % or ...
  • Known constraints or gaps at start: the references storage columns (storage_path, content_sha256, file_size, archived_at) are owned by the migration/ project; not required for PR1 because the new code is not wired in.

4. Plan doc referenced

  • Plan doc path: docs/95_plans/2026-08-16-pdf-persistence.md
  • Plan status at implementation start: authored (design-only) and committed as part of PR1.
  • Was the plan updated during implementation?: No. One deviation from the plan's code snippet was necessary — see Decisions.

5. Decisions taken

Decision Reason Alternative rejected
_sanitize also collapses .. dot-segments (not just non-[A-Za-z0-9._-]) The plan's regex keeps ., so a record_key containing .. survives and the SSRF guard rejects it. Collapsing runs of 2+ dots keeps the derived key dot-segment-free while preserving lone dots (3.2, .pdf). Shipping the plan's regex verbatim (fails the SSRF invariant the function exists to protect)
Import our_logs from app.core.logger (not app.core.logging) our_logs lives in app/core/logger.py; app/core/logging.py is the JSON formatter module Following the plan's import path verbatim (ImportError)
archive_pdf tolerates a "resource already exists" / 409 as success Idempotent upsert; defense-in-depth if the SDK still 409s Special-casing nothing (a stray 409 would fail an otherwise-successful archive)
resolve_pdf_bytes defined here but NOT wired into ingestion.py PR1 must have zero runtime behavior change; PR2 owns the wiring Wiring the resolver now (would change ingestion behavior before columns confirmed live)

6. Files changed

File Change summary
app/services/storage_service.py New module: primitives + resolver (resolver unused this PR)
app/core/config.py New settings SUPABASE_PDF_BUCKET, PDF_ARCHIVE_ENABLED
tests/services/test_storage_service.py New — 24 tests (hashing, path derivation, SSRF, archive, resolver branches)
docs/95_plans/2026-08-16-pdf-persistence.md The design doc (authored earlier, committed here)
docs/96_implementation/2026-08-16-pdf-persistence-pr1.md This record
mkdocs.yml Nav entries for the plan + this record

7. Migrations / schema changes

  • Migration files: none (columns owned by migration/ project; not needed for PR1).
  • Schema changes: none.
  • Data backfill or manual steps: none (backfill is PR3).
  • Rollback notes: revert the PR; nothing persisted, nothing wired in.

8. API changes

  • None. No router or endpoint touched.

9. Tests added or updated

Test file or suite Change
tests/services/test_storage_service.py New — hashing determinism; path derivation incl. null/empty record_key fallback to id + unsafe-char sanitization; is_archived; archive_pdf upload args + duplicate-as-success + hard-failure reraise; signed_url_for returns signedURL; derived signed URL passes the SSRF guard; resolver branch selection (feature-off, archived, not-archived, stored-read fallback, archive-failure-non-blocking, zero-upload idempotency) — all storage/DB/httpx mocked

10. Risks / caveats

  • The resolver is dead code until PR2 wires it in — deliberately, to keep PR1 behavior-neutral.
  • Archive metadata write targets the references storage columns; if PR2 lands before those columns are live the write best-effort-fails and logs (does not crash) — see the plan.

11. Follow-up work

  • PR2: add resolve_pdf_bytes wiring at ingestion.py (fetch_pdf swap) + branch tests. Gate on the migration/ columns being live in prod.
  • PR3: scripts/backfill_pdf_archive.py to seed storage for the existing corpus.

12. Final repo state

  • Relevant behavior after implementation: storage primitives + resolver exist and are unit tested; ingestion is byte-for-byte unchanged; two new config knobs are available (defaults keep the feature dormant until wired).
  • Remaining limitations: nothing reads/writes Storage at runtime yet.

13. Docs updated

Doc path Update summary
docs/95_plans/2026-08-16-pdf-persistence.md Design doc committed
docs/96_implementation/2026-08-16-pdf-persistence-pr1.md This record
mkdocs.yml Nav entries for both