Skip to content

fix: store inline images and apply AI budget in document order - #4121

Open
jacentsao wants to merge 3 commits into
Tencent:mainfrom
jacentsao:fix/strip-inline-image-data-uris
Open

jacentsao wants to merge 3 commits into
Tencent:mainfrom
jacentsao:fix/strip-inline-image-data-uris

Conversation

@jacentsao

@jacentsao jacentsao commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Description

Inline Base64 image data could remain in document content and enter Chunking/Embedding, making chunks carry large encoded payloads or causing ingestion to fail. Because images are replaced in reverse document order, the 30-image AI budget could also be assigned to trailing images.

Save valid inline images and replace their Base64 with serving URLs before Chunking, applying the independent 30-image AI budget to each syntax's leading images. Store up to 1,000 inline images / 100 MiB decoded inline bytes per document, independently of the AI budget. Malformed, unsupported, oversized, or over-budget images with a known payload boundary are replaced with [image omitted]. If a malformed bare multiline payload cannot be separated safely from surrounding text, preserve the original content and fail ingestion before Chunking/Embedding without retrying; storage failures remain retryable. Use one-pass Markdown substitutions to avoid quadratic copying. The final guard rejects only unresolved image-like payloads, before any remote-image downloads, and failed attempts clean up newly written unbound inline and ImageRef resources. Existing remote-image and PDF/Office processing retain their own behavior.

Review follow-up / compatibility notes

  • Literal Markdown inline/fenced code is excluded from inline-image resolution and the unresolved-payload guard.
  • Valid line-wrapped bare images are resolved to serving URLs. Truncated/invalid or over-limit payloads degrade to placeholders when their boundary is known; ambiguous continuations retain the original text and stop ingestion. Ordinary short prose examples remain indexable.
  • Inline raster decoding has fixed support limits: 16,777,216 pixels per image; GIFs additionally allow at most 256 frames / 33,554,432 aggregate frame pixels. A valid larger image is outside the supported decode budget, not necessarily corrupt. Complete over-budget containers can be omitted without full pixel decoding; they are never uploaded based on headers alone. Unknown MIME types are accepted only when the bytes identify a supported image format and pass validation.
  • Manual knowledge now processes HTML and bare Base64 as well as Markdown. An HTML <img src="data:..." width=... alt=...> becomes a Markdown image link, so HTML attributes may be lost. Publishing the same raw inline-image body again uploads new copies; this was not changed to content-addressed deduplication in this PR.
  • The 30-image AI budget remains per inline syntax, not a global 30-image cap; the independent storage caps protect aggregate upload cost. Added full-document, malformed-image, order, rollback, budget, and code-literal regression coverage.

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 💥 Breaking change
  • 📚 Documentation update
  • 🎨 Refactor
  • ⚡ Performance improvement
  • 🧪 Test
  • 🔧 Configuration / Build / CI

Related Issue

N/A

Testing

Regression coverage for the Base64 guard before Chunking, document-order per-syntax AI budgets, bounded image omission, unknown image data, ambiguous-content preservation/rollback, and decode resource limits. CI results must be assessed for the current pushed HEAD separately from local validation.

Checklist

  • git diff --check origin/main...HEAD passes
  • Changed source files are formatted
  • Targeted tests for the changed packages/components pass
  • Diff-scoped lint passes where applicable (for Go: golangci-lint run --new-from-rev=origin/main ./...)
  • Full-repository checks were run, or any unrelated/environment-dependent failures are documented above
  • Self-reviewed the code
  • Added/updated tests covering the change
  • Updated related documentation (README, website-docs/, Swagger annotations, etc.)
  • Breaking changes are clearly called out in the description above

Screenshots / Recordings

N/A — backend processing fix.

@jacentsao
jacentsao force-pushed the fix/strip-inline-image-data-uris branch 2 times, most recently from dd362bd to f4f4229 Compare October 10, 2026 06:47
@jacentsao jacentsao changed the title fix: reject unresolved inline image payloads fix: store inline images beyond legacy limit Oct 10, 2026
@jacentsao
jacentsao force-pushed the fix/strip-inline-image-data-uris branch from 9e5c925 to 6d6e2da Compare October 10, 2026 08:19
@jacentsao jacentsao changed the title fix: store inline images beyond legacy limit fix: store inline images and apply AI budget in document order Oct 10, 2026
@jacentsao
jacentsao marked this pull request as ready for review October 10, 2026 08:44
@lyingbug

Copy link
Copy Markdown
Collaborator

Review of 53284a8d5730.

[P2] Please exclude literal inline-code examples from the unresolved-image failure guard.

HasUnresolvedInlineImagePayload in internal/infrastructure/docparser/image_resolver.go removes fenced code only. A normal documentation sentence such as:

Use `data:image/png;base64,...` to illustrate a data URI.

returns true from the new guard. It is a literal code example, so the resolver has no real image to decode. The new checks in document processing and manual-content processing then mark the entire document failed and return SkipRetry.

A temporary regression using the actual helper reproduces this; the existing inline-image tests pass. Please recognize inline code (and other literal code contexts) before checking actual unresolved image payloads, and add a document-processing regression proving this example remains ingestible. Keep coverage for genuinely malformed image payloads.

Copy link
Copy Markdown
Contributor Author

Thanks @lyingbug for catching this. Fixed in 619a676 and added the requested document-processing regression in 7a94c49.

  • The unresolved-image guard now ignores fenced and inline Markdown code literals (including multi-backtick spans).
  • The inline-image resolvers use the same code-literal ranges so examples are neither uploaded nor rewritten.
  • Added a ProcessDocument regression with an inline-code data URI example alongside a real image: only the real image is stored, the code example remains intact, and chunking proceeds without marking the document failed.
  • Added a ProcessDocument regression confirming genuinely invalid image data URIs still fail before chunking/embedding, plus a benchmark for documents with many fenced code blocks.

Would you mind taking another look?

@lyingbug lyingbug left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

感谢修复!这个 PR 解决的三个问题都是真实存在的:超过 30 张的内联图以 Base64 原样进入切分/Embedding、AI 名额因倒序遍历落到文末图片、SaveBytes 失败被静默吞掉。全部存储 + 按文档顺序分配 SkipMultimodal 名额、失败可重试并回滚本次已存图片、代码块内 Base64 不处理,这些方向都很好。不过有一个阻塞问题需要先处理。

1. 【高】解析器主动跳过的图片会被 guard 判为整篇终态失败

内联解析器在以下情况会 continue 跳过且不返回错误,Base64 保留在正文中:

  • 解码失败(例如截断的 payload)
  • 单张解码后超过 10MB(maxRemoteImageSize)
  • 裸 data URI 按 76 列换行,凑不满 {100,} 的连续字符
  • 正文只是提到 data:image/png;base64,xxx(例如 PDF 抽取出的前端教程,没有反引号)

而 HasUnresolvedInlineImagePayload 的 unresolvedImageDataURI 只要求 payload ≥ 1 个字符,命中后 ProcessDocument / triggerManualProcessing 都直接标记 failed 并返回 asynq.SkipRetry。

我在 PR 分支上对这四种输入逐一验证:ResolveAndStore 均返回 err=nil, stored=0,随后 guard 均为 true。也就是说一份几百页的文档只要含一张 11MB 的内联图或一段损坏的 Base64,整篇就会永久失败,用户无法补救;而合入前这类文档是可以入库的。TestProcessDocumentBlocksUnresolvedImageDataURIBeforeChunking 也把「正文提及 data URI 即失败」固化成了预期行为。

建议:对解析器跳过的内联图,原地替换为占位符(如 [image omitted])或直接移除(与现有 icon 的处理一致),而不是让整篇失败;guard 仅作最后兜底,且至少要求 payload 能解码并像图片(与裸 Base64 分支的 looksLikeImageBase64 对齐)。

2. 【中】移除 30 张上限后,数量与改写开销都无上界

每次替换 markdown[:m[0]] + ... + markdown[m[1]:] 都会复制整篇正文,k 张图、n 字节正文是 O(n·k);例如 100MB 正文含 100 张图约复制 10GB。同时单文档可写入的对象存储/catalog 记录也不再有上限。建议改为单遍正向扫描 + strings.Builder,并为单文档内联图总数或总字节设硬上限,超出部分按第 1 点降级。

3. 【中】guard 失败时远程图片成为孤儿

guard 在 ResolveRemoteImages 之后执行,cleanupUnboundInlineImages 只清理 Inline=true 的图,已下载并上传的远程图片(以及 ImageRefs 路径存下的图片)既未绑定也未删除。建议把 guard 挪到 ResolveAndStore 之后、远程下载之前。

4. 【低】手动知识行为变化

手动路径从 ResolveDataURIImages 改为 ResolveAndStore,会顺带跑 HTML 与裸 Base64 解析器:用户手写的 <img src="data:..." width=... alt=...> 会被改写为 ![image](url),丢失属性;且每次重新发布都会重新上传全部内联图(原先最多 30 张)。建议在描述中说明。

5. 【低】可观测性不一致

手动路径的 guard 失败没有 beginStage / failStage,与 ProcessDocument 不一致,处理轨迹里看不到失败阶段。

测试缺口

  • 缺少第 1 点四类「解析器跳过」输入的回归用例。
  • ResolveBareBase64Content 内部两个解析器各有独立的 30 张名额(单文档内联图最多 120 张进入 AI 理解,与旧行为一致),但测试只覆盖了 Markdown 与 HTML 两种语法。

@jacentsao
jacentsao force-pushed the fix/strip-inline-image-data-uris branch 8 times, most recently from 6055c02 to 40fbf07 Compare October 12, 2026 09:34
@jacentsao
jacentsao force-pushed the fix/strip-inline-image-data-uris branch from 40fbf07 to 11dbdbf Compare October 12, 2026 09:45
@jacentsao

Copy link
Copy Markdown
Contributor Author

@lyingbug 感谢这轮 review。已针对上述意见继续修复,当前 HEAD 为 0bb1940e。逐项说明如下:

  1. 跳过图片与 guard:边界明确的损坏、超限图片替换为 [image omitted];短 data URI 教程示例、代码区域保持可入库。有效的跨行图片会正常保存并替换 URL。最新修复把“边界完整”和“允许保存”分开,避免超解码预算的完整图片因紧跟英文正文而误触发整篇失败,也去掉了裸 Base64 路径的重复完整解码。
  2. 数量与改写开销:内联图片按单文档最多 1,000 张、累计 100 MiB 解码字节限制,改写使用单遍 strings.Builder。30 张 AI 名额仍分别按 Markdown、HTML、裸 data URI、裸 base64, 四种语法的文档顺序分配,未改成全局 30 张。
  3. 资源回滚:guard 已移到远程图片下载之前;失败时清理本次新建的未绑定内联图片及 ImageRef 资源。SaveBytes 异常向上传播,保留重试和部分上传回滚。
  4. 手动知识兼容性:HTML 属性可能丢失、重新发布原始内联内容会重复上传,均已在 PR 说明中注明。
  5. 可观测性与测试:手动失败路径补齐 beginStage / failStage。补充四种语法的额度顺序、异常图片降级、正文保留、资源回滚及文件/手动入口回归。

还需明确一个保守边界:真正无法区分编码续行与正文时,会保留原文并在切分前终止入库,避免误删正文或让残余 Base64 进入 Embedding。中文开头正文或 Markdown 标题可作为明确边界;ASCII 开头的后续行仍按歧义保护。因此,部分损坏的裸图片仍会导致文档失败,这一点也请重点复核。另外,内嵌图片采用固定的 16,777,216 像素上限,GIF 另有帧数及累计像素上限;有效但超限的图片也会被省略,属于支持范围限制,已同步到 PR 说明。

本地验证通过:docparser 全量测试、12 项相关 service 回归、变更范围内 golangci-lint、git diff --check。原先“有效 6000×4000 PNG + 单换行英文正文”的失败用例也已通过,并覆盖两种裸语法及其换行形式;截断大图仍保持失败保护。CI 请以当前 HEAD 的检查结果为准。

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants