Unify the document tree across local and cloud - #541
Merged
Merged
Conversation
get_tree returns the same node shape in both modes: start_index and end_index (no page_index) and summary (no prefix_summary). The SDK only maps field names on the way out. A stored tree is never restructured, so older documents keep their own ranges and summaries. New local indexes, standard and flash, are built in the unified shape: - a parent whose first child starts on a later page gets a first child "<parent title> (intro)" holding those pages - a parent's range covers its whole subtree, and its summary is written from its children's summaries, deepest first - a node the model leaves unsummarized falls back to its subsection titles or its opening text; a run with no answer at all still fails - the standard large-node split acts on leaves only, so it no longer replaces a parent's existing subsections A page cut that cannot tell where a heading sits gives the page to both sides. A parent's text runs onto its first child's page, and is empty when its intro holds those pages. The flash Preface takes the first section's page unless that heading opens it. A heading with nothing to match counts as not at the top of its page.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
rejojer
added a commit
that referenced
this pull request
Oct 1, 2026
#541 makes the same change (a node's own start_index/end_index win) with its own test, and the two versions conflict on merge. The node helpers here don't depend on it.
rejojer
added a commit
that referenced
this pull request
Oct 1, 2026
create_node_mapping walked children with tree.get('nodes', []), so a
node carrying nodes: None raised TypeError while get_node and
get_node_path, which use `or []`, handled the same tree. Use `or []`
in the shared walker so every create_node_mapping caller is covered.
#541 does not touch this line; a trial merge with its head 79d88e8 is
clean.
A parent's text runs onto the page its first child starts on, and is empty when its intro holds those pages. LocalAPI and the standard builder each applied this rule themselves. utils.own_pages now holds it, and add_node_text and add_node_text_with_labels use it, so both get the same text as before.
Standard mode ran its section summaries level by level through a second, slower copy of what summarize_tree already does for flash. It now calls summarize_tree: a parent waits only for its own children, calls run deepest first under the concurrency cap, short leaves keep their raw text, and the prompts are flash's. generate_summaries_for_structure is back to its main version.
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
1 participant
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.
What changes
get_treereturns one node shape in local and cloud mode:{title, node_id, start_index, end_index, summary, text, nodes}.page_indexandprefix_summaryno longer appear. Breaking for code that reads them.The SDK only renames fields on the way out and never restructures a stored tree. A document indexed before this change keeps its own ranges and summaries, so every node's summary still matches its range.
New local indexes (standard and flash) are built in the unified shape:
"<parent title> (intro)"that holds those pages. A parent with no title gets"Intro".summarize_tree, as flash does. A parent waits only for its own children, and calls run deepest first under the concurrency cap. Short leaves keep their raw text, and the prompts are flash's.Page boundaries: when a cut can't tell where a heading sits on a page, the page goes to both sides.
add_node_textandadd_node_text_with_labelscut text by the same rule.When the cloud API returns a tree without
end_index, the SDK fills each node's end from the next node's start. This costs one extra metadata request.Tests
tests/test_tree_format.py: 10 new tests. Each one fails on main.test_local_chat, whose failures here come from an httpx environment issue and also fail on main. 601 passed and 218 skipped without frameworks.get_treereturns it unchanged.Follow-ups (not in this PR)
get_treein the SDK documents page andSKILL.md.get_treechange.