You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Astro-Han (via GitHub)" <gi...@apache.org> on 2026/09/07 13:25:10 UTC
[PR] fix(desktop): show expandable failed-turn diagnostics [maka]
Astro-Han opened a new pull request, #4986:
URL: https://github.com/apache/maka/pull/4986
## Summary
Failed turns show the failure category but hide the bounded diagnostic already stored on their terminal RuntimeEvent. This makes a rate limit's provider code, message, status and request ID impossible to inspect. The same banner can also recommend resending while the recorded retry decision warns against repeating tool activity.
Fixes #4502. Builds on #4951.
- Carry the existing terminal error text through the owner Turn projection, bounded to the existing 2 KiB limit with existing best-effort redaction. No new storage or error classifier. Shared transcript projections continue to omit the diagnostic.
- Reuse Astryx Banner's default collapsed disclosure to show the available text, with wrapping for long diagnostics. Missing details are stated without an empty disclosure. Do not infer additional provider fields from the text.
- Keep the recorded retry reason and existing Host-controlled continuation action visible. Remove unconditional resend advice from provider failure summaries. A sandbox tool failure cannot hide a terminal diagnostic, and a completed turn's tool error does not become a Turn failure.
## Compatibility and overlap
Host compatibility epoch advances from 124 to 125 for the optional `failureMessage` field in the Turn contribution codec. Historical records without details remain readable. Retry budgets, recovery conditions, permissions and execution actions are unchanged.
The first commit only forwards the diagnostic through the existing `runtime-event-read-model.ts`, `session.ts`, `session-turns.ts` and UI materialization seams. These files may overlap #4879; its inspected head `a1db59a0` does not add this field. This PR does not migrate transcript storage or change its read path. Rebase the later merge across this optional field.
## Verification
- 142 focused Runtime, Host, Desktop and UI tests pass. Coverage includes terminal-to-cold decoding, UTF-8 bounds, shared bootstrap/paging exclusion, incremental view updates, tool/Turn separation and existing continuation actions.
- The terminal projection test fails on main because the diagnostic is absent. Removing UI materialization also makes its refresh regression fail; restoring it passes. Reused Banner disclosure and existing test seams; no new interaction abstraction or retry path.
- Core, Storage, Runtime, Host and UI builds; Desktop main/renderer builds and all Desktop typechecks; format, lint, ASF headers and Astryx inventory pass. The renderer architecture check passes against `2f5f04ea`, including 101 checker tests; the affected module's token count decreases.
- 28 Chromium renderer checks via CDP cover light/dark at 1280 and 720 widths: category failures, tool failures, long text, missing details, partial output, live arrival and restart continuation. Checked DOM, computed styles, geometry, focus and console. Reload returns the same diagnostic behind a collapsed disclosure. No real provider request or native process-restart exercise was run.
Same main/branch stories at 1280 × 900; top row rate limit, bottom row truncated stream. Details are expanded in AFTER.
## AI use
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: Codex traced the failure path, implemented the projection and UI changes, updated regression coverage and performed local/CDP verification.
## Checklist
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [x] Yes — described under Summary above
- [ ] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): show expandable failed-turn diagnostics [maka]
Posted by "jackwener (via GitHub)" <gi...@apache.org>.
jackwener commented on code in PR #4986:
URL: https://github.com/apache/maka/pull/4986#discussion_r3951530253
##########
packages/runtime/src/runtime-event-read-model.ts:
##########
@@ -1239,6 +1241,11 @@ function projectTerminalTurnState(
...(status === 'aborted' ? { abortedAt: event.ts } : {}),
...(abortSource ? { abortSource } : {}),
...(status === 'failed' ? { errorClass: failureClass ?? 'unknown' } : {}),
+ ...(status === 'failed' && event.content?.kind === 'error' && event.content.message
+ ? {
+ failureMessage: truncateUtf8(event.content.message, MODEL_FAILURE_MESSAGE_MAX_BYTES, '…'),
Review Comment:
This closes the gap the earlier heads had, and it is worth recording why, because the fix arrived from two directions at once.
On the earlier head the diagnostic could be derived here but never reached the transcript the Desktop reads: the terminal write passed only `ts`, `errorClass` and `retry` to the separate `turn_state` append, so a refreshed session showed "no diagnostic detail available" for an ordinary provider failure.
**Two things removed that.** Merging the transcript-from-RuntimeEvents work deleted the separate write that was omitting the field — there is no longer a second projection to fall out of sync. And this line now carries the message into the terminal projection under the same byte budget. **The gap closed because the second authority disappeared, not only because a field was added**, which is the more durable of the two reasons.
**One naming hazard is still live and worth a comment or a rename.** `agent-run.ts` holds a private `failureMessage` for run-level failure tracking, distinct from the transcript field of the same name. Between two of these heads that private field was changed — its trace redaction swapped for byte truncation — while the transcript field remained unpopulated. **The same identifier meaning two things in one file is how "we added failureMessage" can feel complete while the transcript one is still empty.** It cost a round here; it will cost the next reader too.
<details><summary>简体中文</summary>
**这一行补上了先前几个 head 上的缺口,而缘由值得记下来,因为修复是从两个方向同时到达的。**
在先前的 head 上,诊断可以在这里派生,**却从未到达 Desktop 实际读取的那份 transcript**:终态写入只把 `ts`、`errorClass` 与 `retry` 交给那次独立的 `turn_state` 追加,**于是一次普通的 provider 失败,在刷新后显示的是「无可用诊断详情」。**
**有两件事消除了它。** 合入「transcript 由 RuntimeEvent 派生」的工作**删掉了那次遗漏该字段的独立写入** —— **不再有第二份投影可以失步**;而本行现在把 message 以同一字节预算带进终态投影。**缺口之所以关闭,是因为第二个权威消失了,而不只是因为补了一个字段** —— 两个原因里,前者更持久。
**有一处命名隐患仍然存在,值得加注释或改名。** `agent-run.ts` 里有一个用于 run 级失败追踪的私有 `failureMessage`,**与同名的 transcript 字段是两回事**。在其中两个 head 之间,**那个私有字段被改动过**(把 trace 脱敏换成了字节截断),**而 transcript 那个字段仍未被填充**。**同一个标识符在同一文件里指两件事,正是「我们已经加了 failureMessage」这种感觉得以成立、而 transcript 那个仍为空的原因。** 它在这里花掉了一轮;**对下一个读代码的人也会。**
</details>
---
> **Automated review notice:** This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): show expandable failed-turn diagnostics [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4986:
URL: https://github.com/apache/maka/pull/4986#discussion_r3951637522
##########
packages/runtime/src/runtime-event-read-model.ts:
##########
@@ -1239,6 +1241,11 @@ function projectTerminalTurnState(
...(status === 'aborted' ? { abortedAt: event.ts } : {}),
...(abortSource ? { abortSource } : {}),
...(status === 'failed' ? { errorClass: failureClass ?? 'unknown' } : {}),
+ ...(status === 'failed' && event.content?.kind === 'error' && event.content.message
+ ? {
+ failureMessage: truncateUtf8(event.content.message, MODEL_FAILURE_MESSAGE_MAX_BYTES, '…'),
Review Comment:
Confirmed on `33eaaa9d2`: the earlier missing-diagnostic path is resolved by the RuntimeEvent-backed transcript and the terminal projection. Your overall review already marks that P1 resolved.
For the remaining naming suggestion, we will leave the code unchanged. AgentRun's private `failureMessage` records execution failure text and supplies terminal-event finalization/fallback; the transcript's `failureMessage` is derived from that durable event. These are stages of the same diagnostic, not independently maintained transcript authorities. The earlier omission establishes a missing write/projection, but does not establish naming as its cause. A rename/comment is at most an optional P3 readability improvement, not a remaining P1 or merge blocker.
The related terminal-ledger suite passes all 44 tests on this head.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): show expandable failed-turn diagnostics [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #4986:
URL: https://github.com/apache/maka/pull/4986
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org