feat(core): add portable document ingestion contracts - #1178
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5810e4dca
ℹ️ 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".
e5810e4 to
5a0ad61
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a0ad615fe
ℹ️ 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".
phernandez
changed the title
5a0ad61 to
cbd903c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbd903c254
ℹ️ 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".
cbd903c to
fca36b5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fca36b555a
ℹ️ 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".
|
@codex review |
fca36b5 to
fa57ffe
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa57ffe030
ℹ️ 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".
|
@codex review |
fa57ffe to
bc13b3d
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc13b3d444
ℹ️ 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".
| schema_ref: Literal["schema/document-extraction"] = Field( | ||
| default="schema/document-extraction", | ||
| validation_alias="schema", | ||
| serialization_alias="schema", |
There was a problem hiding this comment.
When downstream code serializes either frontmatter model with the standard model_dump() and reconstructs it with model_validate(), the dump contains schema_ref, but this validation_alias accepts only schema; because extras are forbidden, the model rejects its own serialized payload. The parallel ingestion-run field has the same problem, so enable validation by field name or accept both names.
Useful? React with 👍 / 👎.
| schema_version: Literal["1"] = "1" | ||
| title: NonEmptyText | ||
| body: StrictStr | ||
| tags: tuple[NonEmptyText, ...] = () |
There was a problem hiding this comment.
When an agent returns a structured tag such as "finance,secret" or "#hidden", this tuple preserves that literal value in the document contract, but EntityParser later runs parse_tags and indexes it as finance/secret or hidden. The accepted Markdown contract and its indexed metadata therefore disagree; validate tags with a canonical single-tag grammar or normalize them before merging into frontmatter.
AGENTS.md reference: AGENTS.md:L251-L251
Useful? React with 👍 / 👎.
Signed-off-by: phernandez <paul@basicmachines.co>
bc13b3d to
61d7b0d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61d7b0dd55
ℹ️ 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".
| validation_alias=AliasChoices("schema", "schema_ref"), | ||
| serialization_alias="schema", | ||
| ) | ||
| tags: tuple[NonEmptyText, ...] = ("document",) |
There was a problem hiding this comment.
Fresh evidence beyond the prior agent-tag report is that raw/trusted DocumentNoteFrontmatterV1.tags still accepts any nonempty text: values such as finance,secret or #hidden survive this contract and are propagated by enrichment, but EntityParser later turns them into finance/secret and hidden. The accepted document contract therefore disagrees with the metadata indexed from its canonical Markdown; apply the same single-tag grammar used for agent tags here.
AGENTS.md reference: AGENTS.md:L251-L251
Useful? React with 👍 / 👎.
| elif self.ingestion.stage is DocumentIngestionStage.failed: | ||
| if self.failure is None: | ||
| raise ValueError("failed ingestion run requires structured failure details") |
There was a problem hiding this comment.
When a post-extraction failure such as an agent timeout carries output.raw but omits extraction, this branch accepts the run even though a raw extraction revision was already produced. Fresh evidence beyond the earlier run-ID report is that this post-extraction failure shape bypasses _require_deterministic_run_id, so it can record an arbitrary UUID while losing the extractor engine, profile, version, and options that shaped the output; require extraction whenever a failed run has output.
Useful? React with 👍 / 👎.
| raise ValueError("successful ingestion run requires extraction metadata") | ||
| if self.output is None or self.output.current is None: | ||
| raise ValueError("successful ingestion run requires its current document revision") | ||
| if self.output.current.kind is DocumentRevisionKind.raw_extraction: |
There was a problem hiding this comment.
When a ready or needs-review worker changes current.kind but accidentally reuses raw.checksum, this validation accepts the audit record because it checks only the kind. These checksums represent exact Markdown revisions, and a successful document necessarily changes at least the ingestion stage and bm_parse_semantics, so the same checksum cannot identify both revisions; otherwise history consumers fetch raw bytes labeled as enriched or human-edited.
AGENTS.md reference: AGENTS.md:L355-L359
Useful? React with 👍 / 👎.
1 participant
Why
Hosted document ingestion needs a portable contract before Cloud can safely compose PDF extraction, accepted note writes, and later agent enrichment. Without a shared schema, provenance fields, run identity, lifecycle rules, and raw semantic parsing behavior would drift between runtimes.
This is the Core dependency for basicmachines-co/basic-memory-cloud#1648 and records the parser-neutral portion of #1006. Historical revision indexing remains separate in #1177.
What Changed
documentanddocument_ingestion_runPydantic contracts.pdf.mdsidecar paths, canonical Markdown assembly/parsing, and SHA-256 helpersbm_parse_semantics: falsewhile retaining searchable body contentdocument_ingestionas an explicit accepted-note object sourceImplementation Details
src/basic_memory/schemas/document.pyowns parser-neutral source, extraction, ingestion, revision, run-note, and agent-output models.source,extraction,ingestion, anddocumentfrontmatter is strict, frozen, extra-forbidden, and fail-fast.enrich_document_markdownaccepts only a raw note, requires its exact canonical Markdown checksum, and reconstructs trusted provenance instead of accepting it from the agent.entity_parserlazily. Public schema exports therefore remain stable without pulling the heavy Markdown stack into lightweight CLI registration.EntityParseronly suppresses semantic parsing for an explicit scalar false opt-out. Malformed list/dict settings no longer crash arbitrary Markdown parsing.Testing
Automated
just fast-check: passed Ruff fix/check, formatting, andtytype checking.uv run pytest -q tests/schemas/test_document.py tests/markdown tests/test_runtime.py::TestRuntimeContracts::test_note_object_metadata_parses_safe_values_only: 125 passed;src/basic_memory/schemas/document.pyreached 100% statement coverage.uv run pytest -q --no-cov tests/cli/test_cli_exit.py::test_bm_cli_import_does_not_load_heavy_stack: 1 passed.git diff --check: passed before the amend.Manual
Risks / Follow-ups
.pdf.mdpath.unknown_keybecause the available public key is registered for authentication but not as a GitHub signing key.