fix(wukong): rework event-log conversion for correct GenAI traces - #170
fix(wukong): rework event-log conversion for correct GenAI traces#170Snssn wants to merge 3 commits into
Conversation
Wukong sessions produced malformed traces. Rework the AGUI -> event-log conversion in WukongInput to fix four issues verified against real data: - Truncated answer / zero tokens: tighten the completeness gate (require isComplete + RUN_FINISHED/RUN_ERROR + USAGE + a closing TEXT_MESSAGE_END) so a still-streaming message is deferred instead of emitted truncated, and never advance the cursor past it. - Missing `other` event: emit an `other` user-prompt event at each turn start so the turn no longer begins with tool.call. - Wrong step order: segment steps by LLM decision (an assistant utterance plus the tools it triggers), attribute USAGE per step, and sort all records by time_unix_nano. - Token parsing: read real prompt/completion/total/cached tokens from USAGE. Also merge REASONING + text into a single llm.response (multi-part), use a unique per-utterance response id (not the shared runId) so the converter does not collapse distinct steps, and drop redundant agent.* extension fields (source/message_id/conversation_id/ttft/e2e_ttft/run_duration). Add a real-session test fixture and end-to-end assertions.
🔍 Code Review Summary
Lifecycle Verdict
状态恢复 FAIL 证据:
总体结论方向正确:other-first、答案不截断、真实 token、时间序、reasoning+text 合并单条 response、per-utterance response.id 等均与 但重写在删除既有能力/权威信号时引入回归,建议合入前修复 3 个 High:
Medium:activity 结果状态判定回归(失败被标 success、 Highlights(正向实践)
评审报告详见: |
…mplete, honor STEP_STARTED Addresses the code-review findings on the wukong rework: - Restore full ACTIVITY_SNAPSHOT extraction for FILE_READ/SEARCH/SKILL/ARTIFACT (plus FILE_WRITE file_path and DIRECTORY_LIST files/total_count fallbacks) via compactObject, so their arguments/results are no longer dropped into `default`. - Restore resolveActivityResultStatus (status string / error_message / cancelled) and emit error.message, instead of judging failure by exit_code alone. - Completeness gate now trusts the API's authoritative `isComplete` flag (isComplete===1 -> complete; ===0 -> defer; heuristic only when absent), so a settled message can never permanently block the rest of the session. - Honor STEP_STARTED/STEP_FINISHED as authoritative step boundaries when present; otherwise split sequential (non-overlapping) tools into separate steps while keeping overlapping/parallel tools together. - Accumulate per-step USAGE instead of overwriting. - Skip PERMISSION (HITL) and metadata-only/invisible auxiliary messages so they don't fragment the turn or emit empty tool/LLM pairs. Tests: assert FILE_READ args/result in the e2e case; add unit tests for FILE_READ/SEARCH/SKILL/ARTIFACT payload extraction, status-from-error_message, REASONING+text single-response merge, and PERMISSION/aux-message skipping.
|
感谢细致的 review 🙏 已在 [High] #1 ACTIVITY_SNAPSHOT 抽取回退丢数据 — 已修复。恢复 FILE_READ / SEARCH / SKILL / ARTIFACT 的专用抽取,并补回 FILE_WRITE 的 [Medium] #2 activity 结果状态判定回归 — 已修复。恢复 [High] #3 完成门禁覆盖 API 权威标志 — 已修复。 [High] #4 step 分段欠切分 — 已修复。存在 [Medium] #5 多条 USAGE 双算/少算 — 已改为按 step 累加(不再覆盖)。 [Medium] #6 测试盲区 — 已补:e2e 断言 FILE_READ 的 args(path)/result(content);新增 FILE_READ/SEARCH/SKILL/ARTIFACT 载荷抽取、status-from-error_message、REASONING+TEXT 合并单条 另外修复了一个连带问题: 已用真实 wukong 会话复核: |
There was a problem hiding this comment.
Review: fix(wukong): rework event-log conversion for correct GenAI traces
Summary
Focused 3-file refactor (−62 net lines) that reworks the AGUI → event-log conversion in WukongInput to fix malformed GenAI traces. Four issues addressed, all verified against real wukong-cli data.
Key Changes Analysis
1. Tightened completeness gate
- A message is only emitted when it has fully settled:
isComplete+RUN_FINISHED/RUN_ERROR+USAGE+ closingTEXT_MESSAGE_END - Still-streaming messages are deferred to next poll — prevents truncated answers and zero-token emission
- Cursor is not advanced past incomplete messages — correct behavior
2. New StepAcc accumulator interface
- Properly segments one assistant utterance (reasoning + text) plus triggered tools
- Per-utterance
responseIdensures uniquegen_ai.response.idper LLM call (run-levelrunIdis shared and must not be reused) - Clear documentation: "one step == one LLM decision (spec §2.3)"
3. Step segmentation
- Uses explicit
STEP_STARTED/STEP_FINISHEDboundaries when available (authoritative) - Falls back to heuristic segmentation when not present — good defensive design
4. Defensive timestamp sanitization
evsmap sanitizes AGUI timestamps once upfront with fallback tomsg.createdAt- Handles external data gracefully
5. Metadata-only message filtering
MEANINGFUL_EVENTSset skips messages with onlyCUSTOM/FIRST_TOKENevents- Prevents spurious empty LLM pairs — good edge case handling
6. other event per turn
- Each turn now begins with an
otherevent for the user prompt - Uses earliest user message timestamp for correct temporal ordering
Test Coverage
wukong-input.test.tssignificantly updated (+238 −177) — tests verify the new conversion logic- New fixture
leetcode-session.json(+80) — real test data for end-to-end verification - CI all green: build-and-test (Node 18/20/22) ✅
Minor Observations
-
ACTIVITY_TYPE_TO_TOOL_NAMEchanges: RemovedFILE_READandSEARCHmappings. If these activity types still appear in real AGUI data, they'll fall through to thedefaultcase. Worth confirming these are truly unused. -
stepMessageIdformat: Changed from(evt.messageId as string) ?? 'step-${stepIndex}'to just's${stepIndex}'. This simplifies the ID but loses the AGUI messageId correlation. If downstream consumers relied on this correlation, it could break.
Verdict
Clean, well-documented refactor that fixes real trace correctness issues. Good test coverage with real fixtures. CI green. Approving.
Automated review by github-manager-bot
linrunqi08
left a comment
There was a problem hiding this comment.
🔍 Code Review Round 2 — 修复验证
PR #170 · head
59999a48(round-1 head8474be05)· 增量验证 commit59999a48
round-1 全部 6 条 inline 评论(3 High + 3 Medium)均已修复,逐条已回复并 resolve。作者另主动修复 PERMISSION/invisible 消息碎片 trace 连带问题,核对不引入回归。
| 项 | 级别 | 结论 |
|---|---|---|
| #1 ACTIVITY_SNAPSHOT 抽取回退 | High | ✅ fixed |
| #2 结果状态判定回归 | Medium | ✅ fixed |
| #3 完成门禁覆盖 isComplete | High | ✅ fixed |
| #4 step 分段欠切分 | High | ✅ fixed |
| #5 USAGE 双算/少算 | Medium | ✅ fixed |
| #6 测试覆盖盲区 | Medium | ✅ fixed |
Lifecycle Verdict
| Check | round-1 | round-2 |
|---|---|---|
| 资源释放 | PASS | PASS |
| 死锁/卡死风险 | PASS | PASS |
| 状态恢复正确性 | FAIL | PASS(H2 根因已修) |
Merge Gate(合入门禁)
APPROVE-READY ✅ — 无任何 open 阻断项(Critical/High/Medium 全部 fixed,Lifecycle 三项全 PASS)。仅剩 2 项 Low(otherTs 未钳制、stepMessageId 死字段)不阻断合入。
验证
npx vitest run tests/unit/inputs/wukong-input.test.ts→ 41/41 pass(node v22)。
✅ Medium/High 及以上问题已全部解决,本轮评审通过(approved)。
评审报告详见: code-review/pr-170/final-report-round2.md
Generated by LoongSuite-Pilot Code Review Agent
|
This PR has conflicts with the git fetch origin
git checkout fix/wukong-trace-quality
git rebase origin/main
# resolve conflicts, then:
git push --force-with-leaseThis is a one-time reminder. Feel free to @mention me for a re-review after conflicts are resolved. Automated notification by github-manager-bot |
…ality # Conflicts: # tests/unit/inputs/wukong-input.test.ts
ralf0131
left a comment
There was a problem hiding this comment.
Summary
Reworks the AGUI → event-log conversion in WukongInput to fix four issues with Wukong session GenAI traces: truncated answers/zero tokens, incorrect step segmentation, missing response IDs, and orphan user messages.
Overall: LGTM. The refactoring significantly improves code clarity and correctness.
Highlights
StepAccaccumulator pattern — replaces the old mutableflushStepLlmclosure pattern with a clean, explicit accumulator per step. Each step now collects reasoning, text, tool calls, and usage independently, making the code much easier to reason about- Run-level / step-level separation —
RUN_STARTED/RUN_FINISHED/RUN_ERRORare now scanned in a dedicated pre-pass, cleanly decoupled from step segmentation logic - Completeness gate hardening — the new
isCompletefield fromget_spark_agui_messagesprovides an explicit signal; fallback to event-based detection for older payloads maintains backward compatibility - Metadata-only message filtering —
MEANINGFUL_EVENTSset prevents spurious empty LLM pairs from invisiblesession_context_statsmessages - Per-utterance response IDs — each LLM call in a run now gets a unique
gen_ai.response.id, fixing the previous issue where all steps shared the run-level ID - User prompt timestamp —
userPromptTscorrectly timestamps theotherevent before the run starts
Minor Observations (non-blocking)
- Tool name mapping cleanup —
FILE_READandSEARCHremoved fromACTIVITY_TYPE_TO_TOOL_NAME. If any legacy Wukong data still emits these activity types, they will be silently dropped. Consider logging a warning for unmapped types in a follow-up openSteptimestamp — uses the triggering event timestamp, which is correct. If the event timestamp is somehow missing/invalid, the fallback tomsg.createdAtin the event sanitization pass handles it
Automated review by github-manager-bot
ralf0131
left a comment
There was a problem hiding this comment.
Summary
Complete rework of Wukong event-log conversion for correct GenAI trace generation. Replaces the monolithic transformation with a clean step-based accumulator model (StepAcc) that correctly segments multi-step conversations.
Highlights
- Correct step segmentation: One step = one LLM decision (reasoning + text + tools). New step opens when fresh utterance arrives after tools emitted
- Completeness gate:
isCompleteflag + event-based fallback prevents truncated answers and 0-token emissions - Defensive timestamp handling: Sanitizes timestamps up front, handles metadata-only messages correctly
- Per-utterance response IDs: Each LLM call gets a unique
responseId(not shared run-level id) - Clean separation: Run-level scan (RUN_STARTED/FINISHED/ERROR) separated from step-level accumulation
Comprehensive test coverage with real fixture data.
Automated review by github-manager-bot
Summary
Wukong sessions produced malformed GenAI traces. This reworks the AGUI → event-log conversion in
WukongInputto fix four issues, all verified end-to-end against realwukong-clidata:isComplete+RUN_FINISHED/RUN_ERROR+USAGE+ a closingTEXT_MESSAGE_END). A still-streaming message is deferred to the next poll instead of being emitted truncated, and the cursor is not advanced past it.otherevent — each turn now begins with anotheruser-prompt event, so the stream no longer starts withtool.call.USAGEis attributed per step, and all records are sorted bytime_unix_nano(other → llm.request → llm.response → tool.call → tool.result → …).prompt/completion/total/cachedtokens fromUSAGE.Additional cleanups:
REASONING+ text into a singlellm.response(multi-part).runId) so the converter does not collapse distinct steps.agent.*extension fields (source/message_id/conversation_id/ttft/e2e_ttft/run_duration) that duplicated standard fields.Test plan
npx vitest run tests/unit/inputs/wukong-input.test.ts— 37/37 passtests/fixtures/wukong/leetcode-session.json) + end-to-end assertions (other-first, full answer, real tokens,STEP == LLM, chronological order)wukong-clidata:field-coverage.mjs --agents wukong= 100% for all event types;validate-trace.mjs= 0 errors