Repository navigation
feat(llm): trace 报文内联 base64 媒体的单副本存储 - #304
Conversation
落库前对 request_text / response_text / response_raw_text 各跑一遍截断器,达到门槛的 base64 载荷写入内容寻址 blob 库,正文替换为 [qq:blob sha256=<hex> bytes=<n>] 占位符,替换后报文仍是合法 JSON。 llm_http_traces 新增 media_truncation 列记录 splice 清单与全文哈希;旧行 NULL 视作原样存储。读路径默认返回占位符文本,新增按 sha 的 blob 下载端点与 restore 参数。 每日留存清理之后按剩余行的引用集合 mark-sweep 无引用 blob,24h 保守带规避「blob 已写入、引用行尚未提交」的并发窗口。 由 LLM_TRACE_MEDIA_MIN_BYTES 控制,未设置或为 0 时原样存储,回滚即关开关。 Closes 3aKHP#291
There was a problem hiding this comment.
Bot Review — 💬 Comment
The final review stage did not produce a usable verdict; this automated comment is generated by the review pipeline.
Programmatic comment — the final adjudication result could not be used (reason: final_incomplete:terminal_tool_misuse). Reviewer findings below are shown as reported.
Findings
🔴 BLOCKING: 新增的 GET /logs/trace/blobs/{sha256} 以 Response(content=data, media_type=mime) 原样返回 blob 字节,其中 mime 来自 blob 载荷自带的 data:<type>/<subtype>;base64, 前缀(_DATA_URI_HEAD 仅限制字符集,text/html、image/svg+xml 均可通过),即 Content-Type 由内容作者决定。任何会进入 trace 的第三方内容(群用户消息、prompt-injection 让模型回显的 data URI)都可在越过 min_bytes 门槛后落成 blob,随后诱导已登录 admin 的浏览器直接打开该 URL:text/html 或含 <script> 的 image/svg+xml 会在 quickquip admin 同源下内联执行(仓库内无 CSP、无 X-Content-Type-Options: nosniff),等价于用被 trace 的内容对 admin 会话做存储型 XSS。现有 /logs/trace/calls/* 返回 JSON(application/json)不暴露此风险,本端点是新引入的攻击面。
File: src/quickquip/app/web/routes/logs.py:267
不信任 blob 自报的 MIME:下载端点强制 application/octet-stream + Content-Disposition: attachment; filename="<sha>.bin" + X-Content-Type-Options: nosniff;若前端图片预览需要可渲染类型,仅对白名单栅格格式(png/jpeg/gif/webp)回传 image/*,image/svg+xml、text/html 一律按 octet-stream 处理。例如:
_SAFE_IMAGE_MIMES = {"image/png", "image/jpeg", "image/gif", "image/webp"}
media_type = mime if mime in _SAFE_IMAGE_MIMES else "application/octet-stream"
return Response(content=data, media_type=media_type, headers={
"X-Content-Type-Options": "nosniff",
"Content-Disposition": f'attachment; filename="{sha256}.bin"',
})🟡 SHOULD-FIX: _BASE64_BODY = r"[A-Za-z0-9+/=\s]" 把空白并入 base64 字符类,配合 _BARE_BASE64_RE = re.compile(_BASE64_BODY + r"+$") 与 match() 的全串锚定,任何长度 ≥ min_bytes、且仅由字母/数字/空格/+///= 组成的 JSON 字符串都会被判为“裸 base64 载荷”:例如 LLM 生成的无标点词表、长 token 列表(调低 LLM_TRACE_MEDIA_MIN_BYTES 时门槛文本更常见)。这类普通文本被搬进媒体 blob 库、正文变成占位符,违反模块自述的「只截断 base64 载荷」纪律,下载时 mime 兜底为 octet-stream。JSON 字符串内的合法 base64 不可能含空白,\s 纯属过宽。
File: src/quickquip/llm/provider/media_truncator.py:46
裸 base64 判定去掉空白:_BARE_BASE64 = r"[A-Za-z0-9+/=]+"(data URI 分支同样收紧),并追加廉价校验,例如 len(body) % 4 == 0 与 base64.b64decode(body, validate=True) 成功(可吞 binascii.Error 后返回 None),确保判为载荷的字符串确实是合法 base64;同时补一条“≥门槛的无标点纯文本不被截断”的单测。
🟡 SHOULD-FIX: 本 PR 对 trace.py 的约 200 行改动——_merge_field_manifests 的跨调用合并(begin 落 request、finish 落 response/response_raw,注释明言“必须合并而非覆盖”)、finish_call 新增的先 SELECT 后 UPDATE/无行即 return 控制流、restore_call 的逐字段回拼与 ValueError 上抛、_cleanup_if_due 末尾的 mark-sweep 接线,以及 logs.py 新增的 restore 查询参数与 /logs/trace/blobs 路由——没有任何测试覆盖:新增测试只覆盖 media_truncator.py 纯函数,既有 tests/unit/test_trace_store.py、tests/unit/web/test_logs_routes.py 未扩展。恰好是最易回归的粘合逻辑(合并不覆盖、行缺失早退、restore 校验失败映射 409)处于未测状态。
File: src/quickquip/llm/provider/trace.py:474
在 test_trace_store.py 补集成用例:① begin_call + finish_call 后单行 media_truncation 同时含 request/response/response_raw 三个字段清单且同载荷同 sha;② 对不存在的 call_id 调 finish_call 不抛错、不留事件;③ restore_call 在 blob 被篡改/删除时抛 ValueError;④ min_bytes=0 时三列与 media_truncation 均为原样/NULL。test_logs_routes.py 补 blobs 端点的 422(非法 digest)、404 及 Content-Type 行为。
⚪ NIT: DEFAULT_MIN_BYTES = 65536 在全仓无任何引用(resolve_min_bytes 未设环境变量时返回 0,文档也写默认 0/关闭),是与实际默认行为矛盾的死常量,易误导后续维护者以为存在 64KB 的隐式默认门槛。
File: src/quickquip/llm/provider/media_truncator.py:38
删除该常量;若希望保留“推荐门槛”的语义,改为在 docs 的 LLM_TRACE_MEDIA_MIN_BYTES 行注明推荐值即可。
⚪ NIT: _parse_manifest(trace.py:53)与 media_truncator._load_manifest 是同一份“json.loads + 非 dict 返 None”的逻辑,两处重复。manifest 的形态判定本模块已是唯一权威方,双份实现一旦演进(如未来校验 v 版本)容易漂移。
File: src/quickquip/llm/provider/trace.py:53
在 media_truncator 导出 _load_manifest(更名为 load_manifest),trace.py 的 _parse_manifest 直接复用该函数。
🟡 SHOULD-FIX: _parse_manifest(trace.py:53-62)与 media_truncator._load_manifest(media_truncator.py:437-444)是逐字相同的实现,两处各持一份 manifest JSON 解析。更实质的是格式所有权被拆分:包装形态 {"v": 1, "request": {...}, "response": {...}} 由 trace.py 的 _merge_field_manifests(65-87 行)负责写出,却由 media_truncator.py 的 _iter_field_manifests(395-411 行)负责解开——后者的 docstring 明言它在兼容「trace 落库时把三个文本字段的清单合成一列后的形态」,即下层模块反向持有上层存储布局的知识。同一数据格式的读、写、解析分属两个文件,格式演进(如新增字段或版本号规则)必须锁步修改两个模块,且回归时难以定位 owner。
File: src/quickquip/llm/provider/trace.py:53
把 _merge_field_manifests 与一个公开的 parse_manifest(现有 _load_manifest 改公有)移入 media_truncator,作为 manifest 格式的唯一 owner;trace.py 已从该模块导入 collect_manifest_shas / find_manifest_mime,改为复用即可,_parse_manifest 删除。
🟡 SHOULD-FIX: 本 PR 在 trace.py 新增的集成不变量没有任何测试保护。_merge_field_manifests 的「三字段共用一个 media_truncation 列、finish_call 必须合并且不得覆盖 begin_call 已写入的 request 段」是整个变更中最微妙的跨写入路径不变量(docstring 自己也强调「必须合并而非覆盖」),restore_call 与 _collect_orphaned_blobs 挂钩同样是新代码;但 tests/unit/test_trace_store.py、tests/unit/web/test_logs_routes.py 一行未改(grep media|truncat|blob|restore 均无命中),新增测试只覆盖 media_truncator 模块自身。若日后有人把 finish_call 简化为直接写 manifest,request 的回拼会静默损坏,而现有测试全绿。
File: src/quickquip/llm/provider/trace.py:65
在 tests/unit/test_trace_store.py 补一条截断开启下的 begin_call→finish_call 链路测试:请求与 response_raw 中的同一载荷指向同一 blob sha;finish_call 之后 media_truncation 仍含 request 段(合并不覆盖);restore_call 对三个字段字节级还原。顺带覆盖 restore 参数路由行为。
⚪ NIT: _collect_orphaned_blobs 的 docstring 逐字重复了 MediaBlobStore.collect_orphaned(media_truncator.py)的设计理由段落(「选扫描式而非引用计数:对任意删除路径……免疫,不存在计数漂移。保守带规避……」)。同一设计理由两处维护,日后修改措辞或理由容易漏改一处造成文档漂移。
File: src/quickquip/llm/provider/trace.py:776
trace 侧 docstring 只保留本函数特有的部分(引用集合来自本库的 media_truncation 列、挂在每日留存清理之后),扫描式与保守带的理由指向 media_truncator 的 owner 文档,不重复。
⚪ NIT: find_blob_mime 用 WHERE media_truncation LIKE '%<sha>%' LIMIT 20 子串扫描序列化 JSON 列、再逐行 find_manifest_mime 解析。blob 下载端点的 Content-Type 隐式依赖「manifest 恰好以 JSON 文本形式包含该 sha 子串」和 LIMIT 20 启发式这两个未言明的契约;manifest 序列化格式若变(例如 sha 大小写、字段重命名),该查询会静默返回 None 而无测试或注释警示。
File: src/quickquip/llm/provider/trace.py:614
在方法 docstring 明注该 LIKE 扫描对 manifest 序列化文本形态的依赖与 LIMIT 20 的截断语义;若 mime 反查成为热路径,考虑把 (sha256, mime) 物化为独立列或侧表。
Review provenance
- General reviewer: complete — agent session submitted a review
- Style reviewer: complete — agent session submitted a review
- Coverage: complete
- Reviewed head:
ce6c4ca(base87a43ea)
Automated review by @KHPilot. Reply with @KHPilot to ask follow-up questions.
3aKHP
left a comment
There was a problem hiding this comment.
感谢这次贡献!这个 PR 承接了 #291 中 Trace 媒体重复存储的问题,将载荷抽取集中在 Trace 入库入口,并补上内容寻址存储、恢复清单、下载接口及文档同步。默认关闭、旧记录兼容和原文字节计数也都有考虑。
本次独立审查检查了载荷识别、Trace 写入与恢复、Web 下载响应和清理生命周期,并使用隔离存储与合成数据复核。当前建议 Request changes:合并前请先处理两项 Blocking,并补齐两项 Should-fix。具体证据和建议附在对应代码行。
| 编号 | 级别 | 需要处理的事项 |
|---|---|---|
| C1 | Blocking | GC 引用扫描与并发写入缺少协调,可能删除新记录刚引用的旧 blob,导致原文无法恢复 |
| C2 | Blocking | 下载响应直接采用载荷声明的 MIME,需要限制不可信内容在管理后台同源下的呈现方式 |
| C3 | Should-fix | 裸 base64 识别会将超过门槛的普通英文正文替换为媒体占位符 |
| C4 | Should-fix | 需要为三字段清单合并、真实入库与恢复、路由参数及错误转换补充集成回归测试 |
Verified claims
- Linux / Python 3.12 下,新增截断器测试、Trace Store 测试和日志路由测试共 32 passed;改动文件 Ruff 检查通过,self-docs 的 52 份引用同步检查通过。
- 合成数据验证了
begin_call → finish_call → restore_call:请求、重组响应与原始 SSE 中完全相同的载荷字符串共用一个 blob,三个正文均可逐字恢复。 - 下载路由继承了现有管理员鉴权。未登录请求返回 401;合成管理员身份下,非法摘要返回 422、缺失 blob 返回 404、恢复校验失败返回 409。
- 当前去重粒度是完全相同的载荷文本;哈希计算保留 data URI 前缀。该行为与按原文恢复的实现相符。
验证范围
本地核验使用了隔离目录、合成内容和模拟管理员身份。安全项确认了响应正文与响应头行为;浏览器脚本执行及其后续操作未实际执行。完整测试、多平台文件系统、崩溃恢复和性能基准未在本轮本地审查中执行。
|
感谢你为 Trace 媒体去重提供了完整的初始实现。维护者已决定继续通过这个 PR 完成交付,目标安排在 1.16.5。 接下来我们会协助处理评审中的并发清理、下载响应安全、载荷识别和集成测试,并在本分支追加维护者修复提交,保留你的原始提交与署名。历史数据回填和前端图片预览另行安排。 如果你希望继续负责其中某一部分,欢迎在这里说明,方便协调分工;暂时没有时间也没关系,我们会跟进剩余修复和复审,并在本 PR 更新进展。 |
Coordinate blob writes and GC across processes, enforce attachment downloads, and preserve ordinary text with protocol-aware extraction. Validate restoration manifests, bound parser memory, clear orphan media, and cover storage and HTTP integration paths.
|
维护者修复已推送,当前 head 为 最终 pre-push:3212 项 Python 测试、Ruff、前端 type-check、13 份配置示例及 ID 检查全部通过;另行完成前端 85 项测试/build、73 份 self-docs 检查。独立五透镜评审及交叉复核已收束,无剩余 Blocking / Should-fix。 @KHPilot 请针对该 head 复核此前评审事项及本轮维护者修复,重点检查跨进程 GC/引用事务、下载内容策略、媒体识别、恢复清单和清理生命周期。PR 描述已更新范围及验证结果。 |
3aKHP
left a comment
There was a problem hiding this comment.
维护者收口审查通过,核对 head 194ad33c1e184d59866ec943b06e66f54a5d9c47。
独立五透镜评审和发现交叉复核已完成,原 C1–C4 及补充发现均已修复,无剩余 Blocking / Should-fix。原评审线程已逐条记录处置。原作者提交和署名完整保留,维护者修复为追加提交。
Verified claims:跨进程 SQLite 事务覆盖媒体引用与 GC;下载采用管理员鉴权与附件策略;普通正文保持可读;三字段原文恢复、清单/文件损坏检测及清空生命周期均有回归。独立复核的解析峰值已收敛;POSIX 文件与目录发布同步已核对。
验证:最终推送钩子 3212 项后端测试全部通过,专项65项、前端85项/type-check/build、Ruff、13份示例、ID检查及73份self-docs检查通过。最新 GitHub CI 已批准运行,合并前核对其最终结果;Bot增量复核已请求。
Not verified:真实模型服务、Windows实机、浏览器下载交互及断电故障注入。
|
合并前核对:head 原有 Bot 评审发现已逐条复核、修复和回复;独立五透镜评审及交叉复核完成,无剩余 Blocking / Should-fix。此次额外请求的 Bot 增量复核尚未回复,记录为未完成;不将其计为通过。维护者依据已完成的独立审查、原 Bot 发现处置和最终 CI 接受当前候选,按授权保留历史合并。 感谢 @xiaoyikunle0710-zy 的初始实现!本次继续使用原 PR,保留原始提交与署名。该能力安排在 1.16.5,默认关闭;历史数据回填和前端媒体预览后续另行安排。 |
Trace 会在多轮对话和 SSE 中重复保存相同的内联媒体,导致抓包数据库快速膨胀。本 PR 在 Trace 入库时抽取达到门槛的媒体载荷,按内容摘要存储一份,并支持经校验的原文恢复。
初始实现由 @xiaoyikunle0710-zy 提供(
ce6c4ca)。维护者在保留原提交的基础上对齐当前dev,追加存储生命周期、安全与验证修复。目标为 1.16.5。行为与范围
LLM_TRACE_MEDIA_MIN_BYTES默认 0,设为正数后在进程启动时启用;推荐 65536。只处理新写入记录,发往上游的请求保持原样。restore=true恢复正文。损坏清单、文件缺失或哈希不符返回 409;旧记录及关闭开关后的恢复继续可用。历史数据回填与前端图片预览留待后续。
评审与验证
原 C1–C4 和相关 Bot 发现已处理。独立五透镜评审覆盖协议/不可信输入、工具与外部效果、持久化与恢复、Web/部署,以及架构职责;追加发现的清空生命周期、损坏清单与解析内存问题也已修复并交叉复核,最终无剩余 Blocking / Should-fix。
本轮未执行真实模型服务、Windows 实机、浏览器下载交互和断电故障注入。
Changelog 草稿
Trace 可按配置将内联媒体按内容去重存储,减少多轮对话重复图片造成的磁盘占用;管理员可恢复经校验的原始报文并下载媒体附件。默认关闭,启用后应将 Trace 数据库与媒体目录一并备份。感谢雾岛少帅(@xiaoyikunle0710-zy)提供初始实现。
Closes #291