Repository navigation
fix: unified-tree follow-ups in expand, intros and standard indexing - #554
Merged
Merged
Conversation
Flash expand priced a candidate level without the intro attach_children then adds. An intro sharing its first child's page could make the committed node cost exactly its span, so the next merge folded it back after its summary was final, and the whole index raised "dropped or changed after it was marked final" (PRML 5.1, the 2023 annual report). expand_cost now prices the level with its intro. heading_at_page_start took any first line containing the heading as the heading opening its page. A running header repeating the section title cut intros and the Preface a page early, so the opening text above the real heading was in no node. The first line must now start with the heading, no later line may start with it, and a heading with no Latin letter is never placed. An uncertain page is shared. Expand on an intro or the Preface could list the heading printed on its shared last page (the next node's) or atop it (its parent's) as a new child. A proposal whose page and title, leading number aside, match a node already in the tree is dropped. The local get_document_structure tool read the stored tree through LocalAPI.raw_tree and missed get_tree's end < start clamp. raw_tree is gone; both modes read client.get_tree. A test runs the standard tree_parser path end to end: intro insertion before and after the large-node split, and the covering pass on the default tail.
50caf15 dropped a proposed child whose page and title, leading number aside, matched any node in the live tree. Sibling expansions run concurrently, so the result hung on reply order: when Methods' reply landed before Results', Methods kept "3.1 Discussion" from their shared page and Results lost its own. Stripping every leading number also took "3.2 Results" for its parent "3 Results" and dropped it. A proposal is now dropped when it already is a node, in the tree as expand found it or made by the node's own ancestors, which no concurrent branch can change; or when, on a page the node shares, it is printed above the node's heading or at and below the next node's. A heading not found on its page decides nothing. A leading number is ignored only when one of the two titles lacks it. The check runs before the empty-reply retry, and the tree is read once per pass instead of once per node. A summary reply counted as answered whenever it was non-empty, so a model returning {"summary": ""} everywhere stored a document of fallback summaries without the all-failed error. It now counts only when a summary parses out of it. post_processing ended a TOC item a page before its start when the next item opened the same page; the end is now at least the start.
heading_at_page_start never places a heading without a Latin letter, so a bare "2" (often a page number) shares its page; nothing tested that. The add_intro_nodes idempotency check compared the tree with itself, the same object the call returns, so it could not fail; it now compares a second pass over a copy with the first.
The ownership filter ran inside the try that absorbs failed model calls, so a bug in it would be logged as a model error, retried, and leave the node silently unexpanded. It now runs after the call, where an error surfaces. The neighbor test now also pins the retry: a reply the filter empties is asked again. own_children drops a guard for lines expand always passes.
A mutation pass over the filter found 13 of 21 mutants surviving the suite. Each part now has a test that fails without it: the snapshot of known nodes (Results' run-in heading leaves only the tree to recognize its subsection), the lineage of nodes this pass made, the ancestors and next node each new child is handed, filtering of cached headings, the occurrence each anchor reads (last line of a repeated next heading, last line of a child mentioned above), `>=` for a variant printed on the next heading's own line, key_items for a same-page fusion, and the clamp for a TOC item listed out of order.
…ading normalize keeps only Latin letters and digits, so "1. 概要" reads as "1" and "1.1 背景" as "1 1". same_heading took the bare "1" for an unnumbered title and matched the pair: a CJK chapter dropped its first subsection as already a node, a "1.2" heading on the page where "2." opens was dropped the same way, and a "2.2" line passed for the next node's "2." heading, so the next node's "2.1", printed above it, stayed. Ignoring a leading number only one title prints now needs a Latin letter to compare. On a node's shared first page, the cut for "printed above the node's heading" took the first line matching the node or any ancestor that starts there. With the parent's heading printed above the node's, what sits between (the parent's opening, or a sibling's subsection) was kept as the node's child. The cut now reads the nearest heading found on the page: the node's own, else its parent's, and so on up.
load_pages, the reader behind optimize_tree(pdf_path) and the
tree_optimize CLI, sorted each page's lines by height across columns.
On a two-column page a subsection low in the left column landed after
the next section's heading at the top of the right column, and expand's
line cuts dropped it: 29 of 694 subsections on generated two-column
papers. It now takes flash's own page text, read in layout order, so
both entry points see the same lines. pymupdf's raw stream order was
not enough: the 9/11 report draws a mid-page heading before the text
above it. Reading a 1,100-page book takes about 30 s (5 s before), and
the CLI no longer needs pymupdf.
On a page a node shares with the next one, the cut anchored only on the
next node's own heading and kept everything when that heading was not
found (an unprinted bookmark title, a heading split by extraction). It
now falls back to the heading of the next node's first descendant that
starts on that page. In Murphy's ML book "23.2.1 Using the cdf" no
longer hangs under "22.6.5".
process_large_node_recursively could split the same pages forever when
the model listed another heading above the section's own ("PART II"
above "Chapter 3"): the rebuilt copy was again a large leaf. A leaf
whose range equals the range last split above it now stays a leaf, as
in compute.
Local standard indexing asked page_index_main for node text that nothing
reads any more and that the store strips before saving. It no longer
builds it; the stored tree and every prompt are unchanged.
build_pdf in tests/conftest.py also takes a page as (x, y, size, text)
lines, so the two-column test runs without pymupdf.
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. |
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.
These are follow-ups to #541 (the unified document tree). There are seven commits; please squash them on merge.
Fixes
An index could fail while its summaries were being frozen (50caf15)
Expand priced a candidate level without the intro that
attach_childrenthen adds. When an intro shared its first child's page, the committed node could cost exactly its span, so the next merge folded it back after its summary was final. The whole index then raised "dropped or changed after it was marked final".expand_costnow prices the level together with its intro.Running headers cut intros and the Preface a page early (50caf15)
heading_at_page_startaccepted any first line containing the heading. Now the first line must start with the heading, and no later line may start with it. A heading with no Latin letter is never placed, and an uncertain page is shared.Expand only keeps the subsections printed inside the node (50caf15, ff4cfd0, 2873883, c74e192)
Expand used to adopt headings that belong elsewhere: the next node's heading on a shared last page, the parent's heading printed above, or a neighbour's subsection. Now a proposal is dropped in two cases:
A heading that isn't found on its page decides nothing. A leading number is ignored only when exactly one of the two titles prints it, and only when the titles contain Latin letters. This stops CJK chapters from losing their first subsection.
The ownership filter now runs outside the model-error handler (c29ffa7)
Before, a bug in the filter would have been logged as a model error and the node would have been left silently unexpanded.
optimize_tree(pdf_path)and thetree_optimizeCLI now read pages the way flash does (c74e192)Lines used to be sorted by height across columns, so on two-column pages subsections fell after the next section's heading and were cut. Pages now come from flash's own text in layout order. Reading a 1,100-page book takes about 30 s instead of 5 s, and the CLI no longer needs pymupdf.
Standard indexing
post_processingno longer ends a TOC item before its start (ff4cfd0).Smaller fixes
{"summary": ""}everywhere now hits the all-failed error (ff4cfd0).get_document_structuretool reads throughclient.get_tree, so it gets theend < startclamp (50caf15).Tests
tree_parserpath end to end (50caf15).build_pdfintests/conftest.pynow also accepts(x, y, size, text)lines.Known issues, not fixed here
#553 also edits
build_pdfintests/conftest.py, so it will be rebased after this lands.