fix(spec): open search results on the correct PDF page - #12
Conversation
Search found the right sections but clicking one usually opened the wrong page. Two independent ingest faults, both measured against the published PDFs and the live API. Printed vs physical pages ------------------------- extract.py read the page number printed in the running header; PdfViewer fed that straight into `#page=`, which addresses physical sheets. Nothing converted between them, so every link landed in the front matter's worth of pages early - measured at -11 in 52 of 69 sampled Part 1 sections and -10 in the other 17. Never correct. `spec_content.page_number` keeps storing the printed page (the number a reader sees and cites). PdfViewer now adds the part's front-matter offset when building the fragment. The four hardcoded page counts in PDF_CONFIG were also wrong - Part 4 claimed 4031 pages against an actual 1548, which broke the scrubber outright. extract.py now extracts with `page_chunks=True`, so a line's physical page is known exactly instead of inferred from stray digits, and measures the offset from the running headers (unanimous across all four parts). It prints the totalPages/pageOffset pair to copy into PDF_CONFIG when a PDF is replaced. Table-of-contents rows in the corpus ------------------------------------ Headings were matched by bold styling, and the contents listing is bold too, so every TOC line became its own section - title, section ID, leader dots, and the page it points at. The guard meant to catch this was anchored to a leading digit while the lines start with `**`, so it never fired. Across 12 representative queries, 55% of results were TOC rows: excellent at matching (pure title text, so they outrank real prose) and useless at locating. Headings are now matched on structure, accepting both the bold and ATX forms current pymupdf4llm emits, and rejecting leader-dot titles. Migration 0006 clears the rows already stored; its predicate was verified in Postgres against 539 production rows - 256 deleted, no prose caught, none missed. Chunk-level pages ----------------- chunk.ts declared `const currentPage = pageStart` and never advanced it, so every chunk of a section pointed at the section's opening page. Sections now carry inline page markers and each chunk records the page it falls on: 17.3.1.12 goes from three chunks all claiming 218 to 219/221/223. Verified: 19/19 extracted section pages exact against the PDF, zero TOC leaks, typecheck and build clean. Against the existing corpus the viewer now lands within one page; a re-ingest makes it exact. Removes fix-page-numbers.py, which duplicated the buggy page logic and would reintroduce wrong numbers if run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Annex pages repeat a bare "Annex A" as their running header. The annex pattern accepted lines with no heading markup and allowed an empty title, so each of those headers opened a new section - Part 3 produced four "Annex A" entries, three of them empty, swallowing the annex body. Require heading markup (# or **) and a non-empty title, and normalise the captured letter so the ID is always "Annex A". Part 3 now yields exactly one Annex A (page 33) and one Annex B (page 38), matching its contents listing. Caught while ingesting Parts 2-4 locally; Part 1's 19/19 page check is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
@caio-pizzol @caiopizzol The search was broken due to contents pages not being part of the page numbers but being pages in the PDF. Please look at this PR that Claude wrote with a fix. If possible, merging this would be fantastic since we like your tool |
![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: d74ad3181d
ℹ️ 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".
|
|
||
| # No leader dots, but still "Title 1047" - a contents entry whose title | ||
| # ran long enough to swallow the dots. | ||
| return bool(TOC_TRAILING_PAGE_RE.search(title)) |
There was a problem hiding this comment.
When a legitimate section title ends with a standalone numeric token, such as Version 1 or XML Schema 1.0, this predicate classifies it as a TOC entry even without leader dots. match_heading then returns None, so the section is omitted and its text is folded into the preceding section, corrupting both ooxml_search and ooxml_section; TOC detection needs additional context rather than rejecting every numeric-ending title.
AGENTS.md reference: AGENTS.md:L109-L113
Useful? React with 👍 / 👎.
|
|
||
| annex = ANNEX_PATTERN.match(stripped) | ||
| if annex and has_heading_markup: | ||
| title = annex.group(2).strip().strip("*").strip() |
There was a problem hiding this comment.
When pymupdf4llm emits the supported form **Annex A** **(normative)**, the capture begins with ** **, and this strip("*") call stops at the intervening space; the resulting title is **(normative) rather than (normative). That malformed title is subsequently stored in spec_content and returned by the prose tools, so the annex markup should be parsed or normalized before persisting it.
AGENTS.md reference: AGENTS.md:L109-L113
Useful? React with 👍 / 👎.
|
This little Claude Artifact might help make sense of the issues that this resolves: |
|
Hey, thanks for digging into this—the main fix looks right. I verified the page offsets, TOC issue, and chunk page numbers. There are two things to fix before merging:
Could you fix those and add a couple of focused tests? Then we can take another look. The 4th/5th edition source mismatch is separate. We’ll handle that before the next production ingest. |
upload.ts inserted unconditionally, so re-running the pipeline for a part added a second copy of every chunk and semantic search returned the same passage twice. Uploads now delete the part's existing rows and insert the new ones in one transaction, so a failed upload also leaves the previous ingest intact. --append keeps the old behaviour for uploading a part in slices. Migration 0006 clears duplicates out of databases ingested before this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The annex regex captured the "(normative)" qualifier as the title, so "Annex A (normative) Namespaces" was stored with the title "normative", and any leftover bold run kept its ** markers. It also only matched raw bold runs, not the #-prefixed headings newer pymupdf4llm releases emit. Heading detection now strips the heading prefix and emphasis markers before matching, so no marker can survive into a title, and lives in one module shared by extract.py and fix-page-numbers.py rather than in two copies that drifted apart. An annex name on the line after its qualifier is picked up as part of the title. Tests run with `bun run pdf:test` - no PDF or database needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ge-navigation Brings in the two ingest fixes and resolves them against the page-number work already on this branch: * Re-ingesting a part appended, so search returned every passage twice. upload.ts now replaces the part - delete plus insert in one transaction, with --append kept for uploading a part in slices - and migration 0007 (renumbered around 0006_remove_toc_chunks) clears duplicates already in the database. Covered by tests/db/spec-content.test.ts, including that a failed replace leaves the previous ingest intact. * Annex headings kept `**` in the stored title and mistook the qualifier for the name. Emphasis is now stripped before the annex line is matched, so no marker arrangement any supported pymupdf4llm release emits can leak a marker through, and an annex name on the line after its qualifier is picked up. The heading detection stays in extract.py rather than moving to the shared section_headings.py the merged branch added: fix-page-numbers.py is gone on this branch, so extract.py is the only consumer. extract.py keeps this branch's defences against running headers, contents listings and table rows; tests/ingest-pdf/test_headings.py pins all of it (bun run pdf:test - no PDF or database needed). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 participants
Search finds the right sections, but clicking one usually opens the wrong page. Two independent ingest faults, both measured against the published PDFs and the live API.
1. Page numbers are stored in the book's coordinates, then used as the file's
These PDFs carry two numbering systems: the number printed in the running header, and the physical sheet index that
#page=Naddresses.extract.pyread the printed number;PdfViewerfed it straight into#page=. Nothing converted between them —grep -r offsetreturned nothing.Sampling 70 random Part 1 sections and resolving each one's true page by scanning the PDF: the error was −11 in 52 of 69 sections and −10 in the other 17. Never zero, never positive.
spec_content.page_numberkeeps storing the printed page — the number a reader sees and cites.PdfViewernow adds the part's front-matter offset when building the fragment.The four hardcoded page counts were also wrong:
Part 4 claimed 4,031 pages against an actual 1,548, so the scrubber addressed 2,483 positions that don't exist.
extract.pynow extracts withpage_chunks=True, so a line's physical page is known exactly rather than inferred from stray digits in the text, and measures the offset from the running headers (unanimous across all four parts: 5016/5016, 129/129, 38/38, 1532/1532). Each run prints thetotalPages/pageOffsetpair to copy intoPDF_CONFIGwhen a PDF is replaced.2. Half the corpus is the table of contents
Headings were matched by bold styling, and the contents listing is bold too — so every TOC line became its own section, carrying a title, a section ID, leader dots, and the page it points at. The guard meant to catch this is anchored to a leading digit (
^\d+) while the lines start with**, so it never fired.Across 12 representative queries, 66 of 120 results (55%) were TOC rows. They are excellent at matching — pure title text, so they outrank real prose — and useless at locating, since they carry the contents listing's own page, typically 50–100 pages adrift. §17.3.1.12 has four rows in production; the junk one stores page 166 while literally containing the right answer (219) after its leader dots.
Headings are now matched on structure, accepting both the bold and ATX (
#### 17.3.1.12 Title) forms — current pymupdf4llm emits the latter, so this was version-fragile — and rejecting leader-dot titles.Migration
0006clears the rows already stored. The predicate was verified in Postgres against 539 production rows: 256 deleted, zero real prose caught, zero TOC rows left behind (identical to the Python predicate it was developed against).3. Every chunk of a section claimed the section's first page
chunk.tsdeclaredconst currentPage = pageStartand never advanced it. Sections now carry inline<!--page:N-->markers and each chunk records the page it falls on — §17.3.1.12 goes from three chunks all claiming 218 to 219 / 221 / 223. Markers are stripped from stored content and embedding text.Verification
main— they need a seededTEST_DATABASE_URL)Against the existing corpus the viewer now lands within one page — the residual ±1 is the old extractor's tracking lag baked into stored data. A re-ingest makes it exact. I could not run one: it needs embedding credits.
Notes for the reviewer
SELECTin the header comment.fix-page-numbers.py, which duplicated the buggy page logic and would reintroduce wrong numbers if run.data/sources.jsondeclares 5th edition (2015–2021) with sha256 hashes of the 5th-edition zips. Section numbering and front-matter length differ between editions, so a re-ingest following the manifest would reintroduce exactly this class of bug. Worth deciding which edition the site should serve before the next ingest.🤖 Generated with Claude Code