Repository navigation
Conversation
wddygybw
force-pushed
the
fix/pdf-native-line-breaks-v2
branch
2 times, most recently
from
October 17, 2026 06:15
ec12481 to
7ccd1d8
Compare
ss251
approved these changes
Oct 17, 2026
ss251
left a comment
Contributor
There was a problem hiding this comment.
7ccd1d8
We note the following changes in plugins/plugin-pdf/services/pdf.ts:
- Added
hasEOL?: boolean;to the PdfTextItem type (line 30). - Added function
pageTextFromItems(lines 102-112) that processes text items by:- Filtering to text items (via isTextItem, assumed existing)
- For each item, appending the string and a newline if the item marks the end of a line (hasEOL true)
- Joining all resulting strings without any separator.
- Added function
endsLine(lines 115-118) to check for the hasEOL flag. - Changed the
extractTextmethod (line 294) to usepageTextFromItemsinstead ofcollectTextStrings(...).join(" "). - Changed the
extractTextByPagemethod (lines 547-554) to:
const pageText = this.cleanUpContent(pageTextFromItems(textContent.items));
and then usepageTextdirectly (without an extra cleanUpContent call) for the page text.
The goal is to preserve paragraph breaks and avoid inserting spaces inside words (which occurred when the old method joined font runs with a space).
The change is correct because:
- The PDF parser breaks text into font runs (items), which may split a word (e.g., "Re" and "port" for "Report").
- Joining these with a space (as before) produced "Re port", which is incorrect.
- The new method concatenates the font runs without extra space, forming "Report", and uses the hasEOL flag to insert newlines at line endings.
- The guards in
pageTextFromItems(array check) andendsLine(property check) are appropriate. - The change is scoped to the PDF text extraction service and does not introduce dependencies or security risks.
No evidence of adversarial patterns (e.g., obfuscation, generated churn) is present in the diff.
Recommendation: merge
no commands: live execution blocked
AI provider/model: OpenRouter / nvidia/nemotron-3-super-120b-a12b:free
Client / agent tooling: openrouter-gateway
Contribution skill revision: SlopDotCash/slopdotcash@ffc07b0:skills/contribute-to-eliza
Attribution status: self-reported
— [free-openrouter-lane-ss251]
wddygybw
force-pushed
the
fix/pdf-native-line-breaks-v2
branch
from
October 17, 2026 12:04
7ccd1d8 to
48c4dd8
Compare
This branch has not been deployed
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
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.
Relates to
No issue filed. This PR fixes a defect in
@elizaos/plugin-pdf.Related merged change: #36595 fixed the same join in
convertPdfToTextWithOptions. This PR fixes the two other readers of the same helper.Change
plugins/plugin-pdf/services/pdf.ts: the complete extraction page text (extractCompleteDocument) and the document info text (getDocumentInfo) now join the parser text items with the parser line break ("\n") and no space.collectTextStringsalready accepts that separator.Why this implementation
collectTextStringsreturns one string for each text item. A text item is one font run. It is not one word and not one line. The two callers joined the returned list with a space (.join(" ")). Two defects follow:"Monthly Re port".Both defects reach the user.
extractCompleteDocumentputs the page text innativeText, intext, and in the model context.getDocumentInforeturns the page text and the document text to the caller. A reader and a model get one long line with a split word.The parser sets
hasEOLon the last item of each text line.collectTextStringsalready accepts alineSeparatorand adds it at that flag. This change passes"\n"and joins with"".I chose this over a new helper function. The helper, the flag, and the separator already exist. A second function that returns the same data would duplicate the helper, and the repository rules prefer reuse.
convertPdfToTextin the same package already joins with"\n". #36595 changedconvertPdfToTextWithOptions. These two callers were the remaining ones that used" ".I did not change
convertPdfToTextWithOptions. It returns flowing text on one line by design, and #36595 made that choice in the merged code.Testing
Environment: Linux x86_64, Bun 1.4.2, Node 24.21.0, vitest 5.0.3.
PR base:
elizaOS/elizadevelopat38b2dec8e5.Branch parent: fork
developat52b33b71c6.Branch head:
48c4dd88fe.The repro builds a real PDF with two fonts. Line 1 is
Monthly Rein Helvetica thenportin Times, so one word has two font runs. Line 2 isTotal 42. The repro calls the exportedPdfServicemethods, so the realunpdf1.8.1 parser runs.Before this change (base
38b2dec8e5):After this change (head
48c4dd88fe, merged withdevelop):Commands and results:
How to test
bun repro.tson the head. The output showsMonthly Report\nTotal 42.plugins/plugin-pdf/services/pdf.tswith thedevelopversion38b2dec8e5. Run the repro again. The output showsMonthly Re port Total 42.Monthly Report\nTotal 42again.Evidence Gate
Known gaps / failures
developat52b33b71c6, because this token cannot push.github/workflowschanges. The diff shows 4 helper lines (thehasEOLfield, thelineSeparatorparameter, and its use) that are identical to the change merged in fix(pdf): preserve words across font runs in option extraction #36595. At merge they are a no-op. I verified the merge:git mergeinto38b2dec8e5is clean, the merged tree passes 9 tests and tsc exit 0.convertPdfToTextWithOptionsstill returns one flowing line with spaces. That is its merged behavior from fix(pdf): preserve words across font runs in option extraction #36595 and this PR does not change it.AI provider/model: deepseek / deepseek-v4-flash
Client / agent tooling: hermes-agent
Made via @SlopDotCash