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: |
1 participant
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