fix(extractor): skip comment stripping inside inline image data

[deepanshupal]

What

Fixes the in-repo half of #617: strip_pdf_comments no longer strips inside inline image data. Bytes between the ID operator's whitespace separator and the terminating EI are raw image content, not content-stream tokens - a % there is a pixel value, not a comment, and a ( or < must not move the string state.

Why

On 1.25.2, an unfiltered inline image whose data contains a % byte loses everything from that byte to the next end-of-line in the pre-pass; the image data then comes up short, lopdf rejects the content stream, and whole-document calls (extractPagesMarkdown and friends) fail for every page, including the readable ones. A ( or < byte in the data similarly corrupts the string state for the rest of the stream.

How

The pre-pass tracks inline-image state: it enters on the ID operator token (whitespace-delimited, outside string and hex-string context) and copies bytes verbatim until EI delimited by whitespace - the same delimiter rule lopdf uses to locate EI, so the pre-pass stays aligned with the parser it feeds.

The reporter offered to pick this up 5 days ago and no PR has landed, so here's a scoped fix for component 3 only; happy to close this if they'd rather take it.

Scope, per the issue's three components:

• This PR is component 3 (strip_pdf_comments). With it, the issue's "image data contains a % byte" repro row extracts all 3 pages on pages="all".
• Component 2 (lopdf consuming every whitespace byte after ID) needs the upstream fix in fix: read one white-space character after ID in inline images J-F-Liu/lopdf#582 and a lopdf bump; the "image data is three spaces" row still depends on that.
• Component 1 (degrading a failed page to needs_ocr instead of failing the call) is a behavior/API decision and is left alone here.

Tests

• test_strip_pdf_comments_skips_inline_image_data (unit): % in image data preserved, ( in data does not shield a real comment after EI, ID inside a string literal stays text, multiple images in one stream stay verbatim.
• test_inline_image_percent_byte_does_not_fail_document (integration): rebuilds the issue's repro PDF in Rust; fails on main exactly as the issue's matrix shows (whole-document call throws, pages=[0,2] subset still extracts), passes with the fix.
• test_inline_image_gray_control_still_extracts (integration): the issue's control row, guarding against over-skipping.

cargo test: 1954 passed, 0 failed. cargo clippy -- -D warnings and cargo fmt --check clean (AGENTS.md gates).

---

---

Summary by cubic

Fixes strip_pdf_comments so %, (, and < bytes inside unfiltered inline image data are preserved instead of being stripped as comments or shifting string state, which was truncating image data and failing whole-document extraction on PDFs that contain such images.

• The pre-pass copies bytes verbatim between the ID operator and the whitespace-delimited EI, using the same delimiter rule as lopdf; inline-image mode only opens after a BI operator, so a bare ID or the name /ID stays regular text, and ID adjacent to a closing delimiter (e.g. [/Fl]ID) is still recognized.
• This addresses only component 3 of issue One page whose content stream does not decode fails extraction of the whole document #617; component 2 still needs the upstream lopdf fix, and the needs_ocr degradation behavior (component 1) is unchanged.

^{Written for commit 207b42b. Summary will update on new commits.}

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Confidence score: 4/5

• src/extractor/content_stream.rs rejects ID when it is followed directly by a delimiter, despite documenting that boundary; inline-image streams in that form may not tokenize correctly. Update the follower check to accept the documented delimiter.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/extractor/content_stream.rs">

<violation number="1" location="src/extractor/content_stream.rs:113">
P3: The new `ends_token` doc claims inline images may run one token into the next (`BI/W`, `ID<`), but the `ID` branch's follower check only accepts whitespace or end-of-stream. So when `ID` is followed directly by a delimiter byte (e.g. image data starting with `<` or `(`), `in_image_data` is never set, `bi_open` stays true, and `%` bytes later in the data region are still stripped as comments — the corruption this PR exists to prevent. A missed `ID` also leaves `bi_open` set, so a later bare `ID` (the `cm ID`-style marker the PR deliberately tolerates) then wrongly opens verbatim mode and shields real comments. Widen the follower to `ends_token` (a strict superset: whitespace/EOF stay accepted) to match the documented and feedback-grounded token rule.</violation>
</file>

<sub>Shadow auto-approve: would auto-approve with 1 open P3 issue. Fixes strip_pdf_comments to treat bytes between ID and EI as raw inline image data, preventing % bytes from being stripped as comments; bounded to the content-stream pre-pass and pinned by unit and integration tests.

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The new ends_token doc claims inline images may run one token into the next (BI/W, ID<), but the ID branch's follower check only accepts whitespace or end-of-stream. So when ID is followed directly by a delimiter byte (e.g. image data starting with < or (), in_image_data is never set, bi_open stays true, and % bytes later in the data region are still stripped as comments — the corruption this PR exists to prevent. A missed ID also leaves bi_open set, so a later bare ID (the cm ID-style marker the PR deliberately tolerates) then wrongly opens verbatim mode and shields real comments. Widen the follower to ends_token (a strict superset: whitespace/EOF stay accepted) to match the documented and feedback-grounded token rule.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/extractor/content_stream.rs, line 113:

<comment>The new `ends_token` doc claims inline images may run one token into the next (`BI/W`, `ID<`), but the `ID` branch's follower check only accepts whitespace or end-of-stream. So when `ID` is followed directly by a delimiter byte (e.g. image data starting with `<` or `(`), `in_image_data` is never set, `bi_open` stays true, and `%` bytes later in the data region are still stripped as comments — the corruption this PR exists to prevent. A missed `ID` also leaves `bi_open` set, so a later bare `ID` (the `cm ID`-style marker the PR deliberately tolerates) then wrongly opens verbatim mode and shields real comments. Widen the follower to `ends_token` (a strict superset: whitespace/EOF stay accepted) to match the documented and feedback-grounded token rule.</comment>

<file context>
@@ -76,14 +93,29 @@ fn strip_pdf_comments(data: &[u8]) -> Vec<u8> {
-            && data.get(i + 2).is_some_and(|&n| is_pdf_whitespace(n))
-            && (i == 0 || is_pdf_whitespace(data[i - 1]))
+            && after_token_break(data, i)
+            && data.get(i + 2).is_none_or(|&n| is_pdf_whitespace(n))
         {
             result.push(b'I');
</file context>

Suggested change

	&& data.get(i + 2).is_none_or(|&n| is_pdf_whitespace(n))
	&& data.get(i + 2).is_none_or(|&n| ends_token(n))

[deepanshupal]

Replies to the three cubic findings (native per-thread replies aren't offered on the resolved threads, so answering here with the thread refs):

r4236543730 (bare ID as opener) - fixed in 0426cc6. Image-data mode now only opens after a BI opener: a pending-BI flag is set when a BI token is seen and consumed by ID, the same rule the detector's content mask (src/detector/content_mask.rs) uses. A bare ID (or the name /ID) is left alone, so a comment following it is still stripped. Regression test: test_strip_pdf_comments_bare_id_does_not_open_image_data.

r4236543732 (ID after a closing delimiter) - fixed in 0426cc6. The token-break guard now mirrors after_token_break in the content mask (stream start, whitespace, or a closing delimiter ), ], >), and BI/ID termination uses the same ends_token set, so [/Fl]ID with no whitespace is recognized. Regression test: test_strip_pdf_comments_id_after_closing_delimiter.

r4236568238 (ID followed directly by a delimiter) - not a behavior bug; doc corrected in the latest commit. The follower check intentionally requires whitespace after ID: the spec gives ID a single whitespace separator before the image data, and lopdf parses inline images with pair(tag("ID"), content_space), so a stream in ID<data form is not a parseable inline image for the parser this pre-pass feeds. Accepting a delimiter there would misalign the pre-pass from lopdf rather than fix a real case. What was genuinely wrong was my ends_token doc example (ID<), now corrected with the reasoning recorded next to it.

Full cargo test: 1956 passed, 0 failed. cargo clippy -- -D warnings and cargo fmt --check clean.

[cubic-dev-ai]

0 issues found across 1 file (changes from recent commits).

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

<sub>Shadow auto-approve: would auto-approve with 1 open P3 issue. Fixes strip_pdf_comments to preserve raw bytes between ID and EI so %/(/< in inline image data no longer breaks extraction; the change is scoped to the content-stream pre-pass and pinned by unit and integration tests.

View guided diff | Re-trigger cubic</sub>