Skip to content

Stop a note under a recovered table row being glued into that row - #3595

Merged
BigSimmo merged 5 commits into
mainfrom
claude/ingestion-review-lbw6rf
Oct 17, 2026
Merged

BigSimmo merged 5 commits into
mainfrom
claude/ingestion-review-lbw6rf

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

User description

Summary

  • Before: when the PDF reader recovers a table row that find_tables cut off, any short line just under that row was merged into one of its cells. A note such as "See note" under "Clozapine" became "Clozapine See note" in the table, the accessible table text and the table facts.
  • After: a lone line joins the row above only when that cell actually wrapped. The cell's text must reach closer to the column's right edge than the width of the next line's first word. Otherwise the line is treated as a note under the table and left out.
  • Ledger #ZWJCK4, rechecked on current main: its main complaint (prose admitted by position alone) was already fixed in feat: wire PR #3538 components, fix clinical math/Tall Man/RAG latency/auth bugs, and record Josh Simpson clinical sign-offs #3571 and is pinned by tests/pdf-table-extraction.test.ts. This PR fixes the leftover. The update request downgrades it to P2 and records what is still open: text laid out in the table's own columns directly beneath it (for example an abbreviation key) can still become a row when a drawing extends the crop over it.

Areas touched: Source intake and indexing, Knowledge and records
RAG impact: no retrieval behaviour change — extractor output only changes for documents ingested or re-indexed after merge; live chunks, embeddings and ranking are untouched until then.

Verification

  • Checks run:
    • New test "keeps a short note under a recovered row out of that row's cells" fails with the fix removed (Tests 1 failed | 4 passed (5)) and passes with it.
    • npx vitest run tests/pdf-table-extraction.test.ts tests/pdf-extractor.test.ts: Tests 10 passed | 1 skipped (11).
    • python3 -m unittest discover -p "test_extract_pdf_assets*.py" in worker/python: Ran 53 tests ... OK.
    • npm run test:focused, eslint --max-warnings 0, docs:check-links passed.

Risk and rollout

  • Risk: low. One merge rule in the edge-strip row recovery. A genuinely wrapped cell still joins (tested). A wrapped cell in a column whose recorded edge sits well right of the real wrap point could now stay split as a dropped line.
  • Rollback: revert this PR.
  • Provider or production effects: none on merge. Takes effect for new uploads or a re-index.

Notes

  • Draft until the owner marks it ready.

Generated by Claude Code


Summary by cubic

Stops a short note under a recovered table row being merged into that row's cells. A lone line now joins the row above only when that cell actually wrapped (free space is counted on both sides, covering right-aligned and centred cells) or when the line starts at the cell's left edge with a bullet, lower-case word, or number (a dose under a drug name); otherwise it stays out of the table.

Bug Fixes

  • Wrapped cells, deliberate line breaks, and doses under a drug name still join; a lone line also fills a still-empty cell even when it starts on a lower staggered baseline, as it did before. Notes under short right-aligned values stay out, as do star and numbered footnotes such as * Off-label use and 1 Not PBS listed. Tests in tests/pdf-table-extraction.test.ts cover all cases.
  • Updates ledger #ZWJCK4 to P2; its original fault was already fixed in feat: wire PR #3538 components, fix clinical math/Tall Man/RAG latency/auth bugs, and record Josh Simpson clinical sign-offs #3571, this fixes the leftover.
  • Still open: text laid out in the table's own columns directly beneath it can become a row when a drawing extends the crop over it.
  • Output changes only for documents ingested or re-indexed after merge.

Written for commit d5dfe96. Summary will update on new commits.

View guided diff Turn on auto-fix


CodeAnt-AI Description

Keep notes and footnotes out of recovered PDF table rows

What Changed

  • A line below a recovered table row is no longer added to a cell just because it is nearby; it joins only when it appears to continue that cell’s text.
  • Wrapped text, doses, and bulleted instructions remain in their cells, while short notes and footnotes stay outside the row.

Impact

✅ Fewer notes shown inside PDF table cells
✅ Preserved multi-line doses and instructions in extracted tables
✅ Cleaner table text in newly indexed PDFs

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

A lone line beneath a row recovered from a table's edge strip was merged into
the row's cell whenever it sat within 16pt. A short note such as 'See note'
under 'Clozapine' became 'Clozapine See note'. Now the line joins the row only
when the cell above actually wrapped: its text must reach closer to the
column's right edge than the width of the next line's first word.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M6gRtN6r4vZDgDfzMtjs95
@supabase

supabase Bot commented Oct 17, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

Next included review available in 30 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 106 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: BigSimmo/PsychSift/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Team
  • Run ID: b35641c7-40da-4702-b32c-5c5adcf1e859
📥 Commits

Reviewing files that changed from the base of the PR and between 99507eb and d5dfe96.

📒 Files selected for processing: 3
  • docs/outstanding-issues-inbox/99b5370e-305b-47f4-b0b5-5c81c37f39d3.json
  • tests/pdf-table-extraction.test.ts
  • worker/python/extract_pdf_assets.py
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@codeant-ai

codeant-ai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

🏁 CodeAnt Quality Gate Results

Commit: d5dfe961
Scan Time: 2026-10-17 03:52:08 UTC

✅ Overall Status: PASSED

Quality Gate Details

Quality Gate Status Details
Secrets ✅ PASSED 0 secrets found
Duplicate Code ✅ PASSED 0.0% duplicated
SAST ✅ PASSED No security issues
Bugs ✅ PASSED Rating S: No bugs
IAC ✅ PASSED No IAC issues

View Full Results

Comment thread worker/python/extract_pdf_assets.py Resolved
A dose written under a drug name, or a bulleted step, does not reach the
column's right edge, so the wrap check dropped it. A line that starts at the
cell's own left edge with a bullet, a lower-case word or a number now joins
the cell too. A capitalised note such as 'See note' still stays out.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M6gRtN6r4vZDgDfzMtjs95
Comment thread worker/python/extract_pdf_assets.py Resolved
A footnote flush with the first cell, such as '* Off-label use' or
'1 Not PBS listed', matched the deliberate-break rule and was glued into the
row. '*' no longer counts as a continuation marker, and a leading number counts
only when the next word is not capitalised, so doses like '1 to 2 mg' still join.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M6gRtN6r4vZDgDfzMtjs95
gitar-bot[bot]
gitar-bot Bot previously approved these changes Oct 17, 2026

@gitar-bot gitar-bot Bot 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.

Gitar has auto-approved this PR (configure)

@BigSimmo
BigSimmo marked this pull request as ready for review October 17, 2026 03:25

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

BigSimmo has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@BigSimmo
BigSimmo enabled auto-merge (squash) October 17, 2026 03:25
@codeant-ai

codeant-ai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Reviewed your PR 44657fe Oct 17, 2026 · 03:25 03:29

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-17T03:27:26.040932Z 44657fe Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@codeant-ai

codeant-ai Bot commented Oct 17, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@claude

claude Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Oct 17, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Prevent notes below recovered PDF table rows from merging into cells

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Require evidence of a wrapped or deliberate cell continuation before merging a lone line.
• Keep short notes and footnotes out of recovered rows while preserving genuine multiline cells.
• Add regression tests and record the remaining column-aligned text issue.
Diagram

graph TD
  A["PDF edge strip"] --> B["Column alignment"] --> C{"Multi-column band?"}
  C -- Yes --> D["Recovered row"]
  C -- No --> E{"Cell continuation?"}
  E -- Yes --> F["Append to cell"] --> D
  E -- No --> G["Exclude note"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Tighten table crop boundaries
  • ➕ Could also reject column-aligned text beneath the table, which this fix does not address.
  • ➖ PDF boundary geometry is unreliable; a tighter crop could lose genuine truncated rows.

Recommendation: Use the scoped continuation check for this bug: it preserves edge-strip row recovery while preventing nearby notes from joining cells. Address column-aligned text beneath a table separately, since the continuation rule cannot distinguish it from a genuine new row.

Files changed (3) +125 / -0

Bug fix (1) +44 / -0
extract_pdf_assets.pyGate lone-line merging on cell continuation evidence +44/-0

Gate lone-line merging on cell continuation evidence

• Tracks the preceding line's horizontal extent per column and checks whether a nearby lone line plausibly wraps or deliberately continues that cell. Lines that fail the check no longer merge into a recovered row; the deliberate-break rule also distinguishes tested footnote forms.

worker/python/extract_pdf_assets.py

Tests (1) +68 / -0
pdf-table-extraction.test.tsCover notes and multiline recovered table cells +68/-0

Cover notes and multiline recovered table cells

• Adds PDF-based regression cases for a short note, wrapped text, deliberate dose and bullet continuations, and star or numbered footnotes. The tests check recovered row contents through the Python extractor.

tests/pdf-table-extraction.test.ts

Documentation (1) +13 / -0
99b5370e-305b-47f4-b0b5-5c81c37f39d3.jsonRecord the narrowed outstanding PDF table issue +13/-0

Record the narrowed outstanding PDF table issue

• Adds a ledger update downgrading #ZWJCK4 to P2. It records that full-width prose rejection was already fixed, the note-merging case is addressed here, and column-aligned text beneath an extended crop remains unresolved.

docs/outstanding-issues-inbox/99b5370e-305b-47f4-b0b5-5c81c37f39d3.json

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44657fe61d

ℹ️ 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".

Comment thread worker/python/extract_pdf_assets.py Resolved
A short right-aligned value (a score of 5) ends near the column's right
edge without having wrapped, so a note under it was still joined. The
wrap test now counts the free space on either side of the line above.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M6gRtN6r4vZDgDfzMtjs95
@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo Fixer

No findings are available for this PR yet. Findings appear here once Qodo has reviewed the PR.

@codeant-ai

codeant-ai Bot commented Oct 17, 2026

Copy link
Copy Markdown

CodeAnt Nitpicks

1 code suggestion

1. A lowercase note aligned with the preceding cell is always joined, even when that cell's text ends far from the column edge, corrupting the extracted table.

Incorrect condition logic · worker/python/extract_pdf_assets.py:760-763

gitar-bot[bot]
gitar-bot Bot previously approved these changes Oct 17, 2026

@gitar-bot gitar-bot Bot 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.

Gitar has auto-approved this PR (configure)

@codeant-ai

codeant-ai Bot commented Oct 17, 2026

Copy link
Copy Markdown

CodeAnt PR Risk: Medium Risk

  • The PR needs attention before merging because some text below a table can still be extracted as an extra row.
  • This can occur when text aligns with the table columns and a drawing below extends the extraction crop, potentially adding incorrect content to search results.
  • The new tests cover short notes and wrapped cell text, but do not cover this remaining case.

Assessed commit: acc233883ef7

@qodo-code-review

qodo-code-review Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

No findings for this group


Remediation recommended

No findings for this group


Informational

No findings for this group

Grey Divider


Resolved findings

1. Clinical notes vanish from page text ✗ Dismissed
Description
collect_rows discards a rejected note without passing it to the table-text replacement path, which
can remove the native text block containing that note. When a PDF groups the note with table text in
a block mostly contained by the detected table, the note is absent from searchable page text and
structured table output even though it remains visible in the crop.
Code

worker/python/extract_pdf_assets.py[R850-852]

+                    # A short note or caption under the table, not the rest of a wrapped cell.
+                    prev_band_y1 = None
+                    continue
Relevance

●●● Strong

Dropping rejected notes can remove searchable clinical content, violating conservative ingestion and
accessibility behavior.

PR-#2810
PR-#3021

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The clinical skill requires conservative failure behavior for ingestion. The changed branch drops
the note from recovered rows; the page-text assembler removes a mostly table-contained native block
and substitutes markdown made from accepted rows, so a note in that block has no machine-readable
replacement.

worker/python/extract_pdf_assets.py[845-852]
worker/python/extract_pdf_assets.py[1587-1604]
worker/python/extract_pdf_assets.py[1887-1905]
Skill: clinical

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A note rejected from a recovered table row can also disappear from searchable page text when table replacement removes its shared native text block.

## Fix Focus Areas
- worker/python/extract_pdf_assets.py[845-852]
- worker/python/extract_pdf_assets.py[1587-1604]
- tests/pdf-table-extraction.test.ts[120-127]

## Recommended Fix
Keep rejected edge-strip text outside the structured table, but preserve it when replacing table-contained native text blocks. Add a full page-text regression using a synthetic PDF whose table and adjacent note share a native text block; assert that the note remains searchable and does not enter the table cells.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Lowercase notes still enter table cells ✗ Dismissed
Description
_continues_wrapped_cell() accepts a line at the previous cell’s left edge whenever it starts with
a lowercase letter, bullet, or most numbers, without checking whether that cell wrapped. A note such
as see note or • Reference note directly beneath a recovered row is appended to the cell and
reaches the accessible table text.
Code

worker/python/extract_pdf_assets.py[R761-764]

+    if abs(sx0 - prev_line_start) <= 3.0 and (
+        cleaned[0] in "•·-–" or cleaned[0].islower() or (cleaned[0].isdigit() and not numbered_footnote)
+    ):
+        return True
Relevance

●●● Strong

Directly conflicts with the PR’s note-removal intent; lowercase and bullet notes need regression
coverage and correction.

PR-#2810

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new prefix branch returns true for these notes solely from their starting character and
x-position. The caller then appends an accepted singleton band to the preceding row; the note tests
cover title-case, starred, and capitalized-numbered examples but not lowercase or bulleted ones.

worker/python/extract_pdf_assets.py[760-764]
worker/python/extract_pdf_assets.py[845-861]
tests/pdf-table-extraction.test.ts[120-126]
tests/pdf-table-extraction.test.ts[162-175]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Lowercase and bulleted notes aligned with a recovered cell are unconditionally treated as continuations and enter table output.
## Fix Focus Areas
- worker/python/extract_pdf_assets.py[760-767]
- tests/pdf-table-extraction.test.ts[120-175]
## Recommended Fix
Remove the unconditional prefix-based acceptance or require additional evidence that the line belongs to the cell. Add regression cases for lowercase and bulleted notes beneath a recovered row while preserving deliberate in-cell breaks.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Staggered table cells lose their text ✓ Resolved
Description
_recover_table_rows_split() passes None to _continues_wrapped_cell() when a singleton line
appears in a column absent from the preceding text band, so the new check discards it. A recovered
row whose cells begin on staggered baselines loses the later-starting cell even when its line is
within the existing 16-point row threshold.
Code

worker/python/extract_pdf_assets.py[R846-852]

+                col_idx = next(iter(first_segment))
+                if not _continues_wrapped_cell(
+                    prev_row_line_ends.get(col_idx), first_segment[col_idx], effective_ranges[col_idx]
+                ):
+                    # A short note or caption under the table, not the rest of a wrapped cell.
+                    prev_band_y1 = None
+                    continue
Relevance

●●● Strong

Missing prior geometry can drop staggered cell text, a concrete regression in hard-table recovery.

PR-#3021

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Line geometry is recorded only for columns containing text. Once a band with at least two populated
columns becomes a row, a nearby line in its empty column has no geometry entry; the new function
returns false for that entry and the caller skips the line. Existing table fixtures include empty
cells and staggered multiline text.

worker/python/extract_pdf_assets.py[752-753]
worker/python/extract_pdf_assets.py[838-852]
worker/python/extract_pdf_assets.py[865-870]
worker/python/test_extract_pdf_assets_hard_tables.py[32-52]
worker/python/test_extract_pdf_assets_hard_tables.py[94-105]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A singleton in a column with no preceding-band text is dropped instead of joining a recovered row with staggered cell baselines.
## Fix Focus Areas
- worker/python/extract_pdf_assets.py[838-852]
- tests/pdf-table-extraction.test.ts[97-118]
## Recommended Fix
Handle a missing prior line separately from a failed wrap check: allow a nearby singleton to fill an empty column of the current recovered row, while retaining the note guard for columns that already contain text. Add a staggered-baseline regression test.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View resolved (1)
4. Some recovered cell lines disappear ✗ Dismissed
Description
_continues_wrapped_cell() rejects a deliberately broken cell line that starts with a capitalized
word when the preceding short line leaves more space than the estimated first word needs. For a cell
containing Lorazepam followed by Further monitoring required, the second line is skipped rather
than included in the recovered row and its table text.
Code

worker/python/extract_pdf_assets.py[R761-767]

+    if abs(sx0 - prev_line_start) <= 3.0 and (
+        cleaned[0] in "•·-–" or cleaned[0].islower() or (cleaned[0].isdigit() and not numbered_footnote)
+    ):
+        return True
+    first_word_width = (sx1 - sx0) * (len(words[0]) / max(1, len(cleaned)))
+    free_space = (column_range[1] - prev_line_end) + max(0.0, prev_line_start - column_range[0])
+    return free_space <= first_word_width + slack
Relevance

●● Moderate

Capitalized continuations are intentionally treated as notes, but the claimed wrapped-cell loss is a
plausible correctness gap.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Capitalized text matches none of the new deliberate-break prefixes, and a short preceding line makes
the free-space comparison false. The caller discards that band; the deliberate-break test exercises
only a numeric line and a bulleted line.

worker/python/extract_pdf_assets.py[761-767]
worker/python/extract_pdf_assets.py[845-852]
tests/pdf-table-extraction.test.ts[152-160]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The continuation heuristic drops capitalized deliberate line breaks after short cell lines.
## Fix Focus Areas
- worker/python/extract_pdf_assets.py[761-767]
- worker/python/extract_pdf_assets.py[845-852]
- tests/pdf-table-extraction.test.ts[141-160]
## Recommended Fix
Distinguish a deliberate in-cell break from a note without relying solely on the next line’s capitalization or available wrap space. Add a recovered-row test with a short first line and a capitalized second line.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
❌ Tickets: failed to load — Ticket collection could not establish complete evidence
✅ Compliance rules (platform): 9 rules
✅ Cross-repo context — repo relationships
✅ REVIEW.md
Review mode: Auto: 🚀 Fast: Localized extractor rule with contained, low-risk behavioral impact.
✅ Fix-loop round: fix loop

Grey Divider

Tip of the day
💡 Did you know, you can see how risky a PR is at a glance with blast radius labels

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit d5dfe96

Results up to commit acc2338 ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required

No findings for this group


Remediation recommended

No findings for this group


Informational

No findings for this group


Resolved findings

1. Clinical notes vanish from page text ✗ Dismissed
Description
collect_rows discards a rejected note without passing it to the table-text replacement path, which
can remove the native text block containing that note. When a PDF groups the note with table text in
a block mostly contained by the detected table, the note is absent from searchable page text and
structured table output even though it remains visible in the crop.
Code

worker/python/extract_pdf_assets.py[R850-852]

+                    # A short note or caption under the table, not the rest of a wrapped cell.
+                    prev_band_y1 = None
+                    continue
Relevance

●●● Strong

Dropping rejected notes can remove searchable clinical content, violating conservative ingestion and
accessibility behavior.

PR-#2810
PR-#3021

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The clinical skill requires conservative failure behavior for ingestion. The changed branch drops
the note from recovered rows; the page-text assembler removes a mostly table-contained native block
and substitutes markdown made from accepted rows, so a note in that block has no machine-readable
replacement.

worker/python/extract_pdf_assets.py[845-852]
worker/python/extract_pdf_assets.py[1587-1604]
worker/python/extract_pdf_assets.py[1887-1905]
Skill: clinical

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A note rejected from a recovered table row can also disappear from searchable page text when table replacement removes its shared native text block.

## Fix Focus Areas
- worker/python/extract_pdf_assets.py[845-852]
- worker/python/extract_pdf_assets.py[1587-1604]
- tests/pdf-table-extraction.test.ts[120-127]

## Recommended Fix
Keep rejected edge-strip text outside the structured table, but preserve it when replacing table-contained native text blocks. Add a full page-text regression using a synthetic PDF whose table and adjacent note share a native text block; assert that the note remains searchable and does not enter the table cells.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Staggered table cells lose their text ✓ Resolved
Description
_recover_table_rows_split() passes None to _continues_wrapped_cell() when a singleton line
appears in a column absent from the preceding text band, so the new check discards it. A recovered
row whose cells begin on staggered baselines loses the later-starting cell even when its line is
within the existing 16-point row threshold.
Code

worker/python/extract_pdf_assets.py[R846-852]

+                col_idx = next(iter(first_segment))
+                if not _continues_wrapped_cell(
+                    prev_row_line_ends.get(col_idx), first_segment[col_idx], effective_ranges[col_idx]
+                ):
+                    # A short note or caption under the table, not the rest of a wrapped cell.
+                    prev_band_y1 = None
+                    continue
Relevance

●●● Strong

Missing prior geometry can drop staggered cell text, a concrete regression in hard-table recovery.

PR-#3021

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Line geometry is recorded only for columns containing text. Once a band with at least two populated
columns becomes a row, a nearby line in its empty column has no geometry entry; the new function
returns false for that entry and the caller skips the line. Existing table fixtures include empty
cells and staggered multiline text.

worker/python/extract_pdf_assets.py[752-753]
worker/python/extract_pdf_assets.py[838-852]
worker/python/extract_pdf_assets.py[865-870]
worker/python/test_extract_pdf_assets_hard_tables.py[32-52]
worker/python/test_extract_pdf_assets_hard_tables.py[94-105]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A singleton in a column with no preceding-band text is dropped instead of joining a recovered row with staggered cell baselines.
## Fix Focus Areas
- worker/python/extract_pdf_assets.py[838-852]
- tests/pdf-table-extraction.test.ts[97-118]
## Recommended Fix
Handle a missing prior line separately from a failed wrap check: allow a nearby singleton to fill an empty column of the current recovered row, while retaining the note guard for columns that already contain text. Add a staggered-baseline regression test.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Lowercase notes still enter table cells ✗ Dismissed
Description
_continues_wrapped_cell() accepts a line at the previous cell’s left edge whenever it starts with
a lowercase letter, bullet, or most numbers, without checking whether that cell wrapped. A note such
as see note or • Reference note directly beneath a recovered row is appended to the cell and
reaches the accessible table text.
Code

worker/python/extract_pdf_assets.py[R761-764]

+    if abs(sx0 - prev_line_start) <= 3.0 and (
+        cleaned[0] in "•·-–" or cleaned[0].islower() or (cleaned[0].isdigit() and not numbered_footnote)
+    ):
+        return True
Relevance

●●● Strong

Directly conflicts with the PR’s note-removal intent; lowercase and bullet notes need regression
coverage and correction.

PR-#2810

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new prefix branch returns true for these notes solely from their starting character and
x-position. The caller then appends an accepted singleton band to the preceding row; the note tests
cover title-case, starred, and capitalized-numbered examples but not lowercase or bulleted ones.

worker/python/extract_pdf_assets.py[760-764]
worker/python/extract_pdf_assets.py[845-861]
tests/pdf-table-extraction.test.ts[120-126]
tests/pdf-table-extraction.test.ts[162-175]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Lowercase and bulleted notes aligned with a recovered cell are unconditionally treated as continuations and enter table output.
## Fix Focus Areas
- worker/python/extract_pdf_assets.py[760-767]
- tests/pdf-table-extraction.test.ts[120-175]
## Recommended Fix
Remove the unconditional prefix-based acceptance or require additional evidence that the line belongs to the cell. Add regression cases for lowercase and bulleted notes beneath a recovered row while preserving deliberate in-cell breaks.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View resolved (1)
4. Some recovered cell lines disappear ✗ Dismissed
Description
_continues_wrapped_cell() rejects a deliberately broken cell line that starts with a capitalized
word when the preceding short line leaves more space than the estimated first word needs. For a cell
containing Lorazepam followed by Further monitoring required, the second line is skipped rather
than included in the recovered row and its table text.
Code

worker/python/extract_pdf_assets.py[R761-767]

+    if abs(sx0 - prev_line_start) <= 3.0 and (
+        cleaned[0] in "•·-–" or cleaned[0].islower() or (cleaned[0].isdigit() and not numbered_footnote)
+    ):
+        return True
+    first_word_width = (sx1 - sx0) * (len(words[0]) / max(1, len(cleaned)))
+    free_space = (column_range[1] - prev_line_end) + max(0.0, prev_line_start - column_range[0])
+    return free_space <= first_word_width + slack
Relevance

●● Moderate

Capitalized continuations are intentionally treated as notes, but the claimed wrapped-cell loss is a
plausible correctness gap.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Capitalized text matches none of the new deliberate-break prefixes, and a short preceding line makes
the free-space comparison false. The caller discards that band; the deliberate-break test exercises
only a numeric line and a bulleted line.

worker/python/extract_pdf_assets.py[761-767]
worker/python/extract_pdf_assets.py[845-852]
tests/pdf-table-extraction.test.ts[152-160]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The continuation heuristic drops capitalized deliberate line breaks after short cell lines.
## Fix Focus Areas
- worker/python/extract_pdf_assets.py[761-767]
- worker/python/extract_pdf_assets.py[845-852]
- tests/pdf-table-extraction.test.ts[141-160]
## Recommended Fix
Distinguish a deliberate in-cell break from a note without relying solely on the next line’s capitalization or available wrap space. Add a recovered-row test with a short first line and a capitalized second line.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread worker/python/extract_pdf_assets.py Resolved
Comment thread worker/python/extract_pdf_assets.py Resolved
Comment thread worker/python/extract_pdf_assets.py Resolved
Comment thread worker/python/extract_pdf_assets.py Resolved
A row whose cells start on staggered baselines had its later cell
dropped by the new note check, because that column had no line above
it to compare against. A lone line now fills a still-empty cell as it
did before; the note check applies only to cells that already hold text.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M6gRtN6r4vZDgDfzMtjs95

Copy link
Copy Markdown
Owner Author

Replies to the Qodo review on acc2338:

  1. Notes vanish from page text (High): not reachable through this change. Recovered edge-strip rows feed only the table crop's metadata (recovered_rows near line 1891). The page-text rebuild (around line 1587) uses the find_tables candidate's own markdown and rect, and neither changes in this PR. A native block that holds both the table and a note was already replaced by that same markdown on main, so this PR does not change what page text keeps. Leaving it as is.
  2. Lower-case or bulleted notes still join: this is deliberate. It was added at Gitar's request so that a dose or bullet step under a drug name stays in its cell (Lorazepam / 1 to 2 mg orally / - repeat once). A note has to start flush with the cell's text and in lower case or with a bullet to slip in. That is rarer and less harmful than losing a dose line. Leaving it as is.
  3. Staggered cells lose their text: agreed, and it's a real regression. Fixed in d5dfe96, which I'm pushing now. A lone line in a column that is still empty in the row above now fills that cell, as it did before this PR. The note check applies only to cells that already hold text. New test: "still fills an empty cell whose text starts a line lower than the rest of the row". It fails without the fix.
  4. Capitalised deliberate break after a short line: this is the trade-off Gitar accepted earlier in this PR. Lorazepam / Oral tablet cannot be told apart from Lorazepam / See note by layout alone. Keeping notes out is the conservative choice. Leaving it as is.

Generated by Claude Code

@gitar-bot gitar-bot Bot 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.

Gitar has auto-approved this PR (configure)

@gitar-bot

gitar-bot Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor
Code Review ✅ Approved 2 closed / 2 findings

🟡 Medium risk · Changing recovered-row continuation heuristics could omit legitimate wrapped cell text or retain notes in extracted tables for newly indexed PDFs.

Stops short notes under a recovered table row from being merged into that row's cells, so a line joins the row above only when that cell actually wrapped or starts with a bullet, lower-case word, or number. No open issues remain. Hard line breaks inside cells are now dropped from recovered rows, and footnotes starting with '*' or a digit are no longer glued back into the row.

✅ 2 closed
Closed: Edge Case: Hard line breaks inside cells are now dropped from recovered rows

📄 worker/python/extract_pdf_assets.py:741-754 📄 worker/python/extract_pdf_assets.py:831-839
_continues_wrapped_cell only accepts a lone line when the line above ran almost to the column's right edge. Clinical tables often break a cell on purpose: a drug name with the dose on the next line (Lorazepam / 1–2 mg PO), or bulleted steps (Call registrar / • Give oral medication). In those cells the first line ends far from the edge, so the check fails and the continuation is thrown away. Because prev_band_y1 is then set to None, every later line of that cell is lost too. Before this PR those lines were merged. Now the dose or step is missing from the recovered row's accessible table text and table facts, so it is no longer attached to its row. The PR's risk note only covers columns whose recorded edge sits too far right; deliberate breaks are a separate case with no test.

Suggested fix: keep the width rule, but also accept the line when it clearly continues the cell. That means it starts at the cell's own left edge and begins with a bullet or list marker, a lowercase letter, or a digit (a dose). Add a test with a two-line drug/dose cell.

Closed: Bug: Footnotes starting with '*' or a digit get glued back into the row

📄 worker/python/extract_pdf_assets.py:757-760
The new deliberate-break shortcut returns True whenever a lone line starts within 3pt of the cell's left edge and its first character is in •·-–*, is lower-case, or is a digit. Table footnotes usually look exactly like that: they sit flush with the table's left edge, which is also the first column's left edge, and start with * or a footnote number (* Off-label use, 1 Not PBS listed). They also usually fall within the 16pt gap. So a note such as * Off-label under the recovered row "5 | Highly aroused | ..." becomes "5 * Off-label". That is the same gluing the previous commit set out to stop, and it skips the wrap-width check completely. Two fixes: drop * and bare digits from the shortcut (keep bullets, en dashes and lower-case), or only allow the shortcut when the column is not the first one and the line is not flush with the table's left edge. Either way, add a test with a *-prefixed footnote under the first column.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Approved

For more verification details, comment gitar display:verbose.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Important

Your trial ends in 4 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

Was this helpful? React with 👍 / 👎 | Gitar

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit d5dfe96

Copy link
Copy Markdown
Owner Author

Comment audit against head d5dfe96. All 7 review threads are resolved with replies. Dispositions for the items outside threads:

  • CodeAnt nitpick (lower-case note aligned with the cell is always joined): intentional. It is the same trade-off as the resolved Qodo thread. A flush, lower-case or bulleted line is treated as a deliberate in-cell break so a dose line is not lost.
  • CodeAnt risk summary (text laid out in the table's own columns under a table, plus a drawing that extends the crop, can still become an extra row): known and recorded as low risk in ledger item #ZWJCK4 (docs/outstanding-issues-inbox/99b5370e-...json). Geometry alone cannot tell such a key from a real row. No change in this PR.
  • Gitar's last note (a footnote like 1 see note below with a lower-case word after the number would still join): accepted limitation, covered by the same trade-off.
  • Codex P2 (right-aligned short cell): fixed in acc2338 and present at head (free_space counts both sides of the line above).
  • Qodo staggered-cell finding: fixed in d5dfe96 (fills_empty_cell), with a regression test.
  • Greptile credit-limit notice, CodeRabbit review-limit notice, and the Claude Code Review job error are status notices, not findings.

Note for the owner: this PR changes worker/python/extract_pdf_assets.py, a retrieval-input producer under the RAG ranking protection rule.


Generated by Claude Code

@BigSimmo
BigSimmo merged commit cd47af2 into main Oct 17, 2026
52 of 53 checks passed
@BigSimmo
BigSimmo deleted the claude/ingestion-review-lbw6rf branch October 17, 2026 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🟠 Medium blast radius size:L This PR changes 100-499 lines, ignoring generated files 🕐 20-40 Minutes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants