Skip to content

Read nested Notion blocks in page order - #1085

Merged
WaylandYang merged 2 commits into
deeplethe:devfrom
Floating-Y:fix/notion-nested-body
Oct 5, 2026
Merged

WaylandYang merged 2 commits into
deeplethe:devfrom
Floating-Y:fix/notion-nested-body

Conversation

@Floating-Y

Copy link
Copy Markdown
Contributor

Why

Notion's children endpoint returns one level at a time. The connector only read top-level blocks, omitting paragraphs inside toggles and columns. This fixes a traversal gap in the original connector, not a regression from the rate-limit fix in #269.

What changes

  • Read nested blocks in parent/child/sibling order, with separate pagination cursors for each parent.
  • Share one Paced client and one MAX_BLOCKS_PER_PAGE budget across the entire page, retaining request spacing, 429 retries, and Retry-After handling. Stop making requests when the budget is exhausted.
  • Traverse containers without renderable text while preserving child_page and child_database boundaries.
  • Preserve Markdown rendering, page identity, title, last-edited metadata, and ingestion. Child-request errors propagate to the existing warning and title-only fallback; this does not change the synchronization error policy.
  • Add wiremock regressions against the production traversal and an isolated PostgreSQL resync test. The API address seam is private; there are no new dependencies or user-facing settings.

How it was checked

  • Before the traversal fix: the regression run had 9 failures. After the fix: all 18 Notion tests passed, with database execution required.
  • Coverage includes toggles, columns, multiple nesting levels and sibling order, independent top-level/child pagination, containers without text, the shared block limit with no extra requests, independent content boundaries, child-request failures, pacing/429 retries, and unchanged flat-page output.
  • Resync with mocked Notion responses and real isolated PostgreSQL: the same external key and unchanged last-edited timestamp update an existing top-level-only document with the missing body, create one new version, and produce Unchanged on the next identical sync.
  • Live Notion synthetic fixture: production body fetching and page search matched all 10 expected text lines exactly across 13 blocks, including nested toggles, columns, sibling order, identity, and metadata. A dedicated live acceptance test passed (1 passed, 0 ignored). This validates API reads; resync through the running application against real Notion remains unverified.
  • cargo fmt --all --check and cargo clippy --locked --workspace --all-targets -- -D warnings passed.
  • The initial parallel workspace test run hit two existing chat-registry deadline failures. All five chat-registry tests passed on serial retry, and the complete workspace run with --test-threads=1 passed with database and PDF checks required. The existing real-workspace smoke test was filtered out of that run; the dedicated live test above was run separately.
  • A separate workspace build compiled and linked, but its final replacement of the running server executable failed because Windows held the file open. The running service was left untouched.
  • Frontend checks were not run because no frontend files changed.

Before review

  • Every commit is signed off (git commit -s).
  • cargo fmt --all --check passed.
  • cargo clippy --locked --workspace --all-targets -- -D warnings passed.
  • The complete serial workspace test run passed; the parallel-run failures and live-test scope are disclosed above.

Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.com>

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @Floating-Y. Text inside toggles and columns was simply not read, so this is a correctness fix and it lands. The traversal order, the shared block budget and stopping at child pages and databases are right.

One cost to know about, not to change here: every block with children is now one more paced request, and the sync reads every page's body on every run, whether or not the page changed. A workspace of pages with thirty toggles each goes from about three minutes a sync to well over an hour. The cure is not in the traversal: the search result already carries last_edited_time, so a page whose time matches the stored document's need not have its blocks read at all. That would make syncs cheaper than they were before this PR. If you want it, it is yours; otherwise I will open an issue for it.

@WaylandYang
WaylandYang merged commit 18bd2dd into deeplethe:dev Oct 5, 2026
6 checks passed
@WaylandYang WaylandYang mentioned this pull request Oct 9, 2026
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.

2 participants