chore(UNTRACKED): fix notion bugs#740
Open
potter-potter wants to merge 6 commits into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 29 files
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 12 files (changes from recent commits).
Shadow auto-approve: would require human review. Introduces a shared init_from_dict helper and applies it across block/rich-text types, with refactors and substantial test coverage. The changes touch core parsing infrastructure and require human review.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would require human review. Large refactor replacing bare dataclass construction with a custom init_from_dict across the Notion connector. The change touches critical parsing and mapping logic, so it warrants human review.
Re-trigger cubic
potter-potter
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the Notion connector’s dataclass parsing so that additive Notion API fields (unknown keys) don’t break ingestion, and fixes relation-property parsing so one-way relations no longer abort database extraction. It introduces a shared init_from_dict helper used across several Notion types, adjusts a few schemas to match observed optionality, and adds unit/regression coverage for the hardened behaviors.
Changes:
- Add
init_from_dicthelper to instantiate dataclasses while ignoring unknown payload keys, and adopt it across many Notion block / rich-text / file / relation types. - Fix Notion database
relationproperty parsing to acceptsingle_propertyand tolerate unknown future relation types without raising. - Add comprehensive unit tests covering the helper, rich-text parsing hardening, file parsing hardening, relation variants, and block dispatch/hardening.
Reviewed changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| unstructured_ingest/processes/connectors/notion/interfaces.py | Adds init_from_dict helper to safely instantiate dataclasses from Notion payloads while dropping unknown keys. |
| unstructured_ingest/processes/connectors/notion/types/rich_text.py | Switches multiple rich-text leaf parsers to init_from_dict and makes RichText.from_dict tolerate missing annotations. |
| unstructured_ingest/processes/connectors/notion/types/file.py | Uses init_from_dict for file objects and makes File.expiry_time optional. |
| unstructured_ingest/processes/connectors/notion/types/database_properties/relation.py | Makes dual_property optional and prevents single_property/unknown relation types from raising. |
| unstructured_ingest/processes/connectors/notion/types/blocks/toggle.py | Uses init_from_dict to ignore unknown keys while still parsing rich text and icon. |
| unstructured_ingest/processes/connectors/notion/types/blocks/todo.py | Uses init_from_dict to ignore unknown keys while still parsing rich text and icon. |
| unstructured_ingest/processes/connectors/notion/types/blocks/template.py | Uses init_from_dict to ignore unknown keys while still parsing rich text and icon. |
| unstructured_ingest/processes/connectors/notion/types/blocks/table.py | Uses init_from_dict for Table to ignore unknown keys. |
| unstructured_ingest/processes/connectors/notion/types/blocks/table_of_contents.py | Uses init_from_dict to ignore unknown keys (including optional icon). |
| unstructured_ingest/processes/connectors/notion/types/blocks/quote.py | Uses init_from_dict to ignore unknown keys while still parsing rich text and icon. |
| unstructured_ingest/processes/connectors/notion/types/blocks/paragraph.py | Uses init_from_dict to ignore unknown keys while still parsing rich text and icon. |
| unstructured_ingest/processes/connectors/notion/types/blocks/numbered_list.py | Adds list_format/list_start_index fields and uses init_from_dict to prevent ordered-list metadata from breaking parsing. |
| unstructured_ingest/processes/connectors/notion/types/blocks/link_to_page.py | Uses init_from_dict so unmodeled variants (e.g., comment_id) don’t raise. |
| unstructured_ingest/processes/connectors/notion/types/blocks/link_preview.py | Uses init_from_dict to ignore unknown keys. |
| unstructured_ingest/processes/connectors/notion/types/blocks/heading.py | Uses init_from_dict to ignore unknown keys while still parsing rich text and icon. |
| unstructured_ingest/processes/connectors/notion/types/blocks/equation.py | Uses init_from_dict to ignore unknown keys. |
| unstructured_ingest/processes/connectors/notion/types/blocks/embed.py | Uses init_from_dict and avoids mutating the input payload when parsing captions. |
| unstructured_ingest/processes/connectors/notion/types/blocks/child_page.py | Uses init_from_dict to ignore unknown keys. |
| unstructured_ingest/processes/connectors/notion/types/blocks/child_database.py | Uses init_from_dict to ignore unknown keys. |
| unstructured_ingest/processes/connectors/notion/types/blocks/callout.py | Uses init_from_dict for callout icon subtypes to ignore unknown keys in those payloads. |
| unstructured_ingest/version.py | Bumps version to 1.6.21. |
| test/unit/connectors/notion/types/test_rich_text.py | Adds regression tests for rich-text parsing hardening and rendering behavior. |
| test/unit/connectors/notion/types/test_interfaces.py | Adds unit tests for init_from_dict. |
| test/unit/connectors/notion/types/test_file.py | Adds unit tests for file-object parsing hardening (unknown keys, optional expiry). |
| test/unit/connectors/notion/types/test_block.py | Adds unit tests for block dispatcher mapping and regression around ordered-list metadata. |
| test/unit/connectors/notion/types/database_properties/test_relation.py | Adds regression tests for single/dual/unknown relation variants and relation cell parsing. |
| test/unit/connectors/notion/types/blocks/test_blocks.py | Adds broad unit coverage for block from_dict/get_html/hardening behavior across block types. |
| CHANGELOG.md | Adds release notes for Notion hardening and relation parsing fix (but currently drops 1.6.20 section). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
10
| ## [1.6.21] | ||
|
|
||
| ### Fixes | ||
|
|
||
| - **fix(notion): parse one-way (`single_property`) relation columns instead of aborting the whole database.** `RelationProp.from_dict` only handled two-way (`dual_property`) relations and `raise`d `ValueError("single_property type not recognized")` for anything else. A normal one-way relation column — the Notion default — therefore failed to map, and because the error propagates up to database retrieval it aborted extraction of the *entire* database (every property and row), not just the relation field. `dual_property` is now optional and only parsed when present; one-way relations map with `dual_property=None`, and unmodeled future relation `type`s no longer raise. Adds regression tests for single/dual/unknown relation variants and the relation cell. | ||
| - **harden(notion): drop unknown keys across the page-block extraction path so additive Notion API fields can't break mapping.** Notion ships new, backwards-compatible keys on object payloads (e.g. ordered-list `list_format`/`list_start_index` on `numbered_list_item`), but the connector mapped most types with a bare `cls(**data)` that raises `TypeError` the moment an unmodeled key appears — which had already taken down `numbered_list_item` downloads. A shared `init_from_dict` helper (in the notion `interfaces` module) now instantiates each dataclass from only its declared fields, dropping unknown keys while still surfacing genuinely-missing required fields. It is applied across the block layer (`paragraph`, `heading`, `quote`, `toggle`, `to_do`, `template`, `table_of_contents`, `numbered_list_item`, `child_database`, `child_page`, `embed`, `equation`, `link_preview`, `link_to_page`, `table`, and the `callout` icon sub-types), the rich-text leaves parsed inside nearly every block (`Annotations`, `Text`, `RichText`, the `Mention*` types, `Equation`; `RichText` also tolerates a missing `annotations` key), the file objects backing image/video/file/PDF blocks (`External`, `File` — `expiry_time` is now optional), and the `relation` database property. `link_to_page` variants the dataclass doesn't model (e.g. `comment_id`) now map cleanly and simply render nothing instead of raising. Adds comprehensive unit coverage for every block type plus the helper, rich-text, file, and relation hardening. | ||
|
|
||
| - **test(stager): de-flake the stager bounded-memory tests.** `test_process_whole_peak_memory_is_flat_as_input_grows` and `test_blob_store_stager_peak_memory_is_flat_as_input_grows` asserted a near-flat `tracemalloc` peak ratio between two input sizes. `tracemalloc`'s peak includes not-yet-collected per-element garbage, so the ratio varies with GC timing across environments and the blob-store test failed on CI even though the streamed peak (~3.8 MB) was a small fraction of the ~47 MB input. The tests now assert the meaningful streaming-vs-whole-file bound — peak stays below the input file size (a whole-file load materializes the parsed list at several times the JSON text) — which is robust to GC timing while still catching a regression to whole-file loading. | ||
|
|
||
| ## [1.6.19] |
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would auto-approve. Hardens Notion connector parsing to ignore unknown API fields, adds comprehensive unit tests, and updates changelog/version. No business logic changes.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Shadow auto-approve: would auto-approve. Adds defensive unknown-key handling to Notion parser and comprehensive unit tests. Low risk, well-tested, no logic changes to core functionality.
Re-trigger cubic
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.
Uh oh!
There was an error while loading. Please reload this page.