修复 OpenCode 输出解析失败的BUG,并优化该CLI生成过程的反馈#78
Conversation
|
嘿 @ZhongXiandou 👋 看了一下这个 PR 的范围 — 这块正在被多个 PR 同时改进中。简单交叉引用一下,避免重复工作: Related:
Suggestion: 你可能想和 #52/#50 的作者协调一下范围 — 这几个 PR 都在改同一个文件 ( 审查进行中 👀 |
| if (typeof obj.text === "string") out.push({ kind: "delta", text: obj.text }); | ||
| if (typeof obj.content === "string") out.push({ kind: "delta", text: obj.content }); | ||
| if (typeof obj.message === "string") out.push({ kind: "delta", text: obj.message }); | ||
| if (typeof part?.text === "string") out.push({ kind: "delta", text: part.text }); | ||
| if (typeof part?.content === "string") out.push({ kind: "delta", text: part.content }); | ||
| if (typeof part?.message === "string") out.push({ kind: "delta", text: part.message }); |
There was a problem hiding this comment.
These new OpenCode lines append every matching alias field (obj.text/content/message and part.text/content/message). If one event carries the same chunk both at the top level and inside part—which is exactly the compatibility case this branch is trying to tolerate—parseLine() will emit two delta entries for one payload. handleEvent() then appends every delta directly to task.html, so the preview/output becomes duplicated or corrupted for affected runs. Please pick one preferred source per event (for example part.text ?? part.content ?? part.message ?? obj.text ?? obj.content ?? obj.message) or dedupe identical strings before pushing, and add a regression test that covers a payload containing both top-level and nested text fields.
| const text = | ||
| typeof part?.text === "string" | ||
| ? part.text | ||
| : typeof part?.content === "string" | ||
| ? part.content | ||
| : typeof part?.message === "string" | ||
| ? part.message | ||
| : typeof obj.text === "string" | ||
| ? obj.text | ||
| : typeof obj.content === "string" | ||
| ? obj.content | ||
| : typeof obj.message === "string" | ||
| ? obj.message | ||
| : null; |
There was a problem hiding this comment.
This precedence chain stops at the first typed alias, even when that alias is an empty string. For example, parseLine("opencode", JSON.stringify({ type: "text", content: "<html>ok</html>", part: { type: "text", text: "" } })) currently returns only [{ kind: "delta", text: "" }]. handleEvent() then drops that empty delta, so the valid top-level HTML chunk is lost entirely. Because this branch is meant to tolerate mixed top-level and nested aliases, it should choose the first non-empty string (or otherwise skip empty aliases before falling back) rather than the first string-valued field. Please update the selector to ignore "" candidates and add a regression test for an empty nested alias plus a populated top-level alias.
| if (part?.tokens && typeof part.tokens === "object") { | ||
| const tokens = part.tokens as { | ||
| input?: number; | ||
| output?: number; | ||
| cache?: { read?: number; write?: number }; | ||
| }; | ||
| out.push({ | ||
| kind: "meta", | ||
| key: "usage", | ||
| value: { | ||
| input_tokens: tokens.input, | ||
| output_tokens: tokens.output, | ||
| cache_read_input_tokens: tokens.cache?.read, | ||
| cache_creation_input_tokens: tokens.cache?.write, | ||
| }, | ||
| }); | ||
| } | ||
| if (typeof part?.cost === "number") out.push({ kind: "meta", key: "cost_usd", value: part.cost }); |
There was a problem hiding this comment.
step_start / step_finish are step-scoped events, but this branch forwards every part.tokens / part.cost payload as the final usage / cost_usd meta update. In handleEvent() those keys overwrite the task stats instead of accumulating them, so any OpenCode run with multiple steps will end up showing only the last step's tokens and cost. That makes the new usage feature report incorrect totals even though earlier steps consumed tokens too. Please either accumulate these step-finish values before emitting usage / cost_usd, or emit separate partial keys and sum them client-side before patching the run stats.
nettee
left a comment
There was a problem hiding this comment.
@ZhongXiandou 我重新检查了这次 OpenCode 修复里的嵌套 part 文本提取、空字符串 fallback、多 step usage/cost 累加,以及等待态预览反馈这几处改动;当前 head 上这些问题链路已经对齐,新增的 argv 单测也覆盖了重复 delta、空嵌套字段回退和多步累加回归。这个 worktree 里未安装依赖,所以我没法在本地直接跑 Vitest,但从变更范围和现有测试覆盖看,这版实现是自洽的。辛苦了,这轮跟进修得很扎实。
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.解析 OpenCode 嵌套 part 输出,补充用量信息解析,减少重复 session 日志,并在等待生成时显示更准确的预览反馈。
OpenCode 的 JSON 事件可能同时携带顶层 text/content/message 字段和嵌套 part.text/content/message 字段。之前解析逻辑会把所有匹配字段都作为 delta 输出,导致同一 HTML 片段被重复追加,进而污染预览内容。 本次调整为每个事件只选择一个优先文本来源:part.text、part.content、part.message、obj.text、obj.content、obj.message。这样既保留兼容性,又避免重复 delta。 同时补充回归测试,覆盖顶层文本和嵌套文本同时存在时只输出一个 delta 的场景。
1. 修复空字符串阻断 fallback 的边界问题:当嵌套的 part.text 为空字符串时,不再阻断解析,而是跳过空文本并继续 fallback 到顶层的有效文本字段(如 obj.content),确保有效 HTML 数据不丢失。 2. 优化每个事件只输出唯一非空 delta:对 [part.text, part.content, part.message, obj.text, obj.content, obj.message] 队列进行过滤,只选择第一个值存在且长度大于 0 的有效文本源,彻底杜绝重复 HTML 的可能性。 3. 补充 falls back 测试用例:在测试中覆盖「嵌套字段为空,顶层字段有值」的回退逻辑,确保未来的代码重构不引入此回归问题。
因为 step_start/step_finish 事件是 Step 级别的,之前每次都直接发射单步值覆盖 handleEvent 里的全局 stats,导致多步骤任务在前端只能显示最后一步的 tokens 和 cost。 本次修改将多步 tokens 和 cost 的计算累加任务放在 makeParser 的生命周期状态(ParseState)中,每次遇到新 step 时均进行累计相加,然后再发射累积的总额,让前端 handleEvent 的覆盖机制正确显示至今为止的总统计。 同时补充了多步骤累加的回归单测,确保多步完成时数值不会被直接覆盖。
|
还不合并吗🤣 |
|
看到了 @ZhongXiandou 👀 这边已经同步值班 maintainer 做合并判断了。当前 PR 已有 @nettee approve,GitHub 也显示可合并;因为这个仓库没有 CI 信号,最后合并还需要 maintainer 手动确认一下。 |
|
Heads-up: #92 is now also open against this area — both PRs touch |
解析 OpenCode 嵌套 part 输出,补充用量信息解析,减少重复 session 日志,并在等待生成时显示更准确的预览反馈。