feat(core): add PDF page citations to document observations by phernandez · Pull Request #1490 · basicmachines-co/basic-memory · GitHub
Skip to content

feat(core): add PDF page citations to document observations - #1490

Merged
phernandez merged 2 commits into
mainfrom
feat/pdf-page-citations
Sep 6, 2026
Merged

feat(core): add PDF page citations to document observations#1490
phernandez merged 2 commits into
mainfrom
feat/pdf-page-citations

Conversation

@phernandez

@phernandez phernandez commented Sep 6, 2026

Copy link
Copy Markdown
Member

Why

Refs #1366. Document observations need to point back to the source PDF page so readers can inspect the evidence. This is the page-level slice agreed for v0.24.0, following the amended SPEC-89: the observation is the claim, and citations use OKF footnotes joined to stable sources[].id values.

What Changed

  • Added an optional typed PDF page locator to document-agent observations: one-based physical page, with an optional separate printed page_label.
  • Trusted enrichment generates page-specific OKF-compatible sources entries and Markdown footnotes. Agents provide page numbers, not destination URLs.
  • Preserved ordinary observation category, tags, context, and existing uncited document serialization.
  • Added real PDF extraction/enrichment/Markdown tests and an API → canonical read-back → on-disk file → database observation integration test, without mocks or parameterized cases.
  • Documented the portable representation and its limits in docs/DOCUMENT_CITATIONS.md.

Implementation Details

The page number is validated against the actual extraction page count. Citation resources use project/bundle-root-relative, percent-encoded file paths with standard #page=N fragments. IDs such as document-page-2 are scoped to the note's single trusted source PDF, not an array index; repeated citations share an entry and reordering does not retarget them. Printed labels never change the navigation page.

The existing singular source retains the original PDF checksum and storage-version provenance. The new plural sources supplies citation targets. Trusted-envelope validation checks those entries against the source PDF; generated footnote definitions are not agent-supplied. A separating space prevents trailing Markdown escapes in observation text from escaping the generated marker.

Specifications and prior art

Testing

  • uv run pytest test-int/test_document_page_citations.py --no-cov -q: 19 passed on SQLite.
  • BASIC_MEMORY_TEST_POSTGRES=1 uv run pytest test-int/test_document_page_citations.py --no-cov -q: 19 passed against real PostgreSQL via testcontainers.
  • uv run pytest tests/schemas/test_document.py tests/schemas/test_document_agent_temporal.py tests/document_ingestion --no-cov -q: 118 passed.
  • just fast-check: passed (lint, formatting, typecheck).
  • just doctor: passed the isolated file/API/index/search/status loop.
  • codex review --uncommitted: run before pushing; its trailing-backslash finding was fixed with a real Markdown-rendering regression test. The second pass reported no actionable defects (76 focused tests passed in the review).
  • An additional coverage-instrumented run failed during FastMCP/Pydantic settings initialization before collection; no coverage percentage is claimed. The normal targeted runs above pass.

Risks / Follow-ups

Review follow-up: generated citation references as well as definitions are reserved across agent-controlled Markdown fields when locators are present. Omitted page labels are compatible with an explicit label in either observation order; only distinct explicit labels conflict. Explicit tests also cover duplicate source IDs and non-PDF sources. The follow-up pre-push review found no actionable regressions (19 citation tests passed).

  • This is the Core portable contract and assembly layer. Cloud prompt adoption and UI/viewer resource routing need separate integration; a project-root-relative link is not a new Cloud HTTP route.
  • The link opens the current resource in supporting viewers; the retained checksum/version records extraction provenance but does not implement historical-version retrieval or automatic stale-citation detection.
  • Extraction offset/page maps, quote selectors/highlighting, evidence watermarks, contradiction findings, and generic OKF conformance remain out of scope. Optional source locator on document observations (page → context, resolvable against the extraction page map) #1366 stays open to track its remaining precision/integration work.
  • No migrations, changes to canonical file-service behavior, or additional concurrency machinery.

Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez phernandez added this to the v0.24.0 milestone Sep 6, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 2026

Copy link
Copy Markdown

@phernandez

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 11c7334898

ℹ️ 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".

Comment thread src/basic_memory/schemas/document.py Outdated
Comment thread test-int/test_document_page_citations.py
Comment thread src/basic_memory/schemas/document.py Outdated
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

@phernandez
phernandez merged commit 9cd8169 into main Sep 6, 2026
58 of 59 checks passed
@phernandez
phernandez deleted the feat/pdf-page-citations branch September 6, 2026 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant