feat: add observable progress updates - #4270
Conversation
Normalize model-authored progress and final-answer text across provider, persistence, Runtime Host, CLI, TUI, and UI boundaries. Infer phases for providers without native support and preserve explicit OpenAI Responses phases. Generated-by: OpenAI Codex
|
Closing this draft for now while local end-to-end validation is completed. The branch remains available and the proposal continues in Discussion #4268. |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for building this out end to end. I reviewed e39eeb3, and the implementation helped make the protocol differences and edge cases concrete.
After discussing the product boundary, I think we can solve the user problem with a much smaller architecture:
flowchart LR
A[Responses / Chat Completions / Anthropic] -->|ordinary assistant text| B[Existing text stream and timeline]
B --> C{Runtime Turn}
C -->|active| D[Progress visible and interruptible]
C -->|terminal| E[Last settled text is the final reply]
F[Responses native phase] -->|provider metadata only| G[Adapter and durable replay]
What users need is straightforward: while the Turn is active, the model should occasionally explain its progress; when the Turn finishes, it should give a final reply. All three protocols can already do that through ordinary assistant text, so the common fix can be the progress-update prompt plus the existing text pipeline.
Only Responses has a native phase. Keeping it as provider-owned replay metadata avoids inventing an equivalent fact for Chat Completions and Anthropic. It also avoids having a normalized top-level phase and the original provider phase disagree, or having to propagate the new field through storage, Runtime Host, CLI, and UI before there is a concrete presentation requirement.
My suggestion is therefore to keep:
- the progress-update prompt and cross-provider behavior tests;
- lossless Responses phase/item-boundary preservation at the adapter and replay boundary.
And defer the provider-neutral stored phase, phase inference, Runtime Host epoch change, phase-aware consumers, and commentary-only continuation.
I left one P1 and two P2 inline for the concrete risks in the current implementation. The broader work was still useful—it showed us exactly where the complexity would spread. I am very open to pushback if there is a current product consumer or provider constraint that needs a stable cross-provider phase.
AI-assisted review using OpenAI Codex; I verified the exact head, provider paths, persistence/replay boundary, and continuation behavior.
中文架构建议
建议先用更小的架构解决当前问题：
- 三种协议都通过普通 assistant text 输出工作进展；
- Runtime Turn 的进行中和终态负责区分“进展”和“最终回复”；
- Responses 原生
phase只作为 provider metadata 无损保存和回放； - 暂不为其他协议推断 phase，也不扩展 Runtime Host、CLI 和 UI 协议。
这样已经能让用户看到进展并及时打断，同时避免出现两个 phase 权威不一致的问题。以后有独立样式或 answer-only export 等具体需求时，再增加 Maka 自有语义也不迟。
| ts: number; | ||
| text: string; | ||
| /** User-visible role of this model-authored text. */ | ||
| phase?: AssistantTextPhase; |
There was a problem hiding this comment.
P1 — Category ② (reasonable provider irregularity): keep one durable authority for assistant phase
This adds a second persisted semantic for text that already retains its native Responses phase in providerOptions.
On a reachable Responses path where the provider labels text as final_answer but the same step later returns a client tool call, normalizedAssistantTextPhase() stores message.phase = "commentary" while providerOptions.openai.phase remains "final_answer". The UI, CLI, copy behavior, and child summaries consume the new top-level field, while provider replay consumes providerOptions. The same durable assistant row therefore has two contradictory identities, and reconstruction depends on which consumer reads it.
Could we avoid persisting a second top-level phase authority? My suggested boundary is:
- keep the original Responses phase losslessly in provider metadata;
- preserve native text-item boundaries where replay requires them;
- let Runtime's active/terminal Turn lifecycle and chronological text position own the product distinction between progress and the settled reply.
That would also remove the need to propagate inferred phase through AssistantMessage, RuntimeEvent, Runtime Host continuity, and the public wire protocol.
There was a problem hiding this comment.
Thanks, this distinction is now explicit in the implementation and PR description. AssistantMessage.phase is the sole Maka product authority consumed by UI, CLI, copy, summaries, and continuity. Provider options remain opaque replay evidence and are not consulted for product classification after normalization. A malformed native Responses step can therefore replay its original provider payload while Maka consistently treats text followed by tool work as commentary. The adapter-capability gate and the new reconnect/read-model tests make that ownership boundary explicit.
| // Responses supplies phase directly. Chat Completions and Anthropic Messages | ||
| // do not, so their response topology is the portable signal: text before a | ||
| // client tool call is progress, and completed text-only output is terminal. | ||
| if (input.hasClientToolCall) return 'commentary'; |
There was a problem hiding this comment.
P2 — Category ① (supported provider-tool path): phase-less text around server-executed tools is classified as final
This inference only considers client-executed tool calls. Maka also supports provider-executed tools, including Anthropic and OpenAI native web search; those events follow the provider-tool-input / provider-tool-result path and never set stepHasClientToolCall.
Consequently, phase-less assistant text emitted before a provider-executed tool reaches flushStep(completedStep = true) with hasClientToolCall = false and is labeled final_answer, even though tool activity followed and the work was still in progress. That value can then drive copy behavior, CLI output, memory extraction, or child summaries as though it were the settled answer.
If normalized phase remains, the topology would need to account for every supported form of tool activity and preserve the relevant text/tool boundaries. I think the smaller fix is to avoid cross-provider phase inference entirely and let the Turn lifecycle plus chronology own this distinction.
There was a problem hiding this comment.
Fixed in b1b8b1a2c. Pending assistant content is now flushed as commentary when provider tool input or a provider-executed call begins, then the assistant message identity rotates so post-tool text can settle independently as the final answer. The regression test covers commentary -> provider WebSearch -> final_answer, and the existing OpenAI/Anthropic native web-search tests still pass.
| currentStepMessageId = this.newId(); | ||
| continue agentLoop; | ||
| } | ||
| throw { |
There was a problem hiding this comment.
P2 — Category ② (reasonable model noncompliance): do not add a paid retry and hard failure without demonstrated need
When a provider completes a response with explicit commentary and no tool call, this changes the existing terminal behavior in two ways: Maka silently sends another provider request, and a repeated commentary response becomes a non-retryable Turn failure.
That changes request count, cost, latency, and the final Turn status for a recoverable model-output problem. The evidence in Discussion #4268 demonstrates that models currently fail to produce progress updates; it does not demonstrate that commentary-only completion is occurring in production or that converting it into a failed Turn is the desired product policy.
Could we remove the bounded continuation and throw from this slice? The prompt can instruct the model not to stop after a progress update. If real telemetry later shows premature commentary-only completion, we can design recovery from that concrete failure mode without coupling it to the basic observability feature.
There was a problem hiding this comment.
I kept the bounded continuation, but made its scope and cost explicit. It applies only when the provider explicitly labels the terminal text as commentary, because final-only CLI/copy/child-summary consumers cannot safely treat that semantic as a successful answer. Maka performs at most one additional request; a repeated violation becomes a visible non-retryable failure instead of silently exporting commentary as the answer. Phase-less inferred commentary does not trigger this terminal guard.
Add a provider-neutral ProgressUpdate transport for phase-less model APIs, hide its implementation-detail activity, and present commentary, reasoning, and tool work as a Codex-style log that folds when the final answer begins. Preserve native Responses phases and keep CLI final-output selection phase-aware. Generated-by: OpenAI Codex
…y-phase # Conflicts: # packages/runtime-host/src/protocol/index.ts
hqhq1025
changed the title
UI comparisonThe original baseline comparison remains at the same persisted Turn, application state, content, zoom, and Flat baseline
Completed work collapsed
Current revisionThe current
|
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed exact head b1b8b1a2cd0154b82770c0c4fdb9b2c7901f0dcf. This remains NO-GO due to the two inline P1 findings: the compatibility epoch collides with a different current-main wire change, and the new raw disclosure buttons introduce an Astryx blocker that already fails the hosted test job. The provider-executed tool split and the focused phase, replay, Runtime Host, UI, and CLI paths otherwise passed local review.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
| // Increment when the same protocol version no longer guarantees safe Client-Host | ||
| // interoperability. Mismatches are rejected before domain commands are admitted. | ||
| export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 78 as const; | ||
| export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 79 as const; |
There was a problem hiding this comment.
P1 — allocate a fresh compatibility epoch after rebasing
Current main is already at epoch 80 and assigns epoch 79 to the queued Skill-outcome wire change. This branch assigns 79 to assistant text phases. A conflict resolution that keeps this value would allow peers with different closed wire shapes to pass the same epoch handshake. Preserve main's 79/80 history and assign this change the next epoch (currently 81), with the protocol test updated accordingly.
| data-collapsible={props.collapsed ? 'true' : undefined} | ||
| > | ||
| {props.collapsed && ( | ||
| <button |
There was a problem hiding this comment.
P1 — use the Astryx disclosure/button primitive for both new toggles
This file introduces raw <button> controls here and again in ProcessingBlock (line 1435). The repository's generated Astryx inventory classifies that as a blocker: npm run astryx:surface-inventory fails on this head, and regenerating changes this file from aligned to raw <button (API Use-the-System) | blocker. Replace both controls with the available Astryx Button/Collapsible boundary and regenerate the inventory rather than committing the blocker state.
|
Left detailed feedback on #4268 (discussion) rather than here, since the questions are about the shape rather than the code: Not blocking with a formal request for changes yet — I'd rather settle the shape on the discussion first. The collapse hierarchy and the Codex import fix look good to me independently of that. |
|
Updated the branch to The implementation now follows the smaller architecture proposed in review and Discussion #4268:
The previous CI failure was in Desktop E2E. The stable regression was the accessibility scenario still trying to reach a tool result through the retired flat layout. It now follows the real keyboard path through the collapsed outer work log and inner processing summary before opening the tool result. The second reported transcript-width failure passed after syncing current Validation on this revision:
A single long local Desktop E2E run passed 90 tests before an Electron teardown timeout caused cascading closed-page/fixture-start failures. Every earliest and representative failure from that cascade passed in isolation, including all tests related to this PR. Hosted CI has been retriggered by the push. |
|
Hosted CI is now green on exact head The first attempt reached Storybook smoke after Desktop E2E had passed, then intermittently missed the post-navigation focus assertion in the unchanged Attempt 2 passed the complete workflow, including:
Review has been re-requested from @Astro-Han and @M4n5ter. The PR still requires an independent human approval before it is merge-ready. |
Review follow-up on
|
Before (main) |
After (fc96cfe48) |
|---|---|
 |
 |
ProviderRetrying
Before (main) |
After (fc96cfe48) |
|---|---|
 |
 |
ComputerUseObservability
Before (main) |
After (fc96cfe48) |
|---|---|
 |
 |
The retry story is intentionally unchanged visually; its fix is in retry eligibility and is covered by the new Runtime regression.
Review re-requested from @Astro-Han and @M4n5ter. The PR still requires an independent human approval before merge.
CI follow-up on
|
Standard-test follow-up on
|
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
NO-GO on exact head 88f4eb5855a209099d114464d73d66bb95f987a0.
The Responses retry and WorkLog remount findings from the prior round are closed. Two current issues remain: legacy persisted tool rows lose their semantic category and deterministically break the Desktop accessibility path, and the new “Worked for” label displays time-to-last-assistant as though it were total work duration.
Validation on this head: clean production build and full typecheck; 361 focused UI/Runtime/Storage/Desktop tests; changed-file lint, format, ASF headers, Astryx inventory, renderer architecture, and git diff --check; plus the same focused tests and full typecheck on a clean synthetic merge onto current main c86da40d7. Hosted CI attempt 1 failed at Desktop E2E; the accessibility failure reproduced locally twice under Xvfb with the actual accessible name 调用 1 个工具. The separate transcript-scroll failure passed in isolation and is not included as a finding. CI attempt 2 is currently running.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
| let failed = 0; | ||
| let interrupted = 0; | ||
| for (const tool of tools) { | ||
| const kind = tool.activityKind ?? 'tool'; |
There was a problem hiding this comment.
P2 — Preserve tool categories for legacy rows
activityKind is optional by contract (ToolCallMessage explicitly says it is absent on legacy rows), and materializeTools() passes that absence through. Replacing the name-based compatibility classifier with the generic fallback therefore changes persisted Bash, Read, Edit, and similar rows to a generic tool summary. The checked-in legacy E2E fixture has exactly such a Bash call: on this exact head the accessible button is 调用 1 个工具, so the required Desktop E2E times out waiting for 运行 1 条命令; I reproduced the same failure twice locally under Xvfb. Please keep a compatibility classifier at the materialization/summary boundary, or backfill legacy rows before removing the fallback.
There was a problem hiding this comment.
Fixed in 588447b7d. The summary now treats an explicit activityKind as authoritative and applies the previous name-based classifier only when legacy persisted rows omit that field. The checked-in legacy Bash fixture therefore exposes 运行 1 条命令 again without overriding modern runtime semantics. Added a focused legacy-category regression; the original Desktop accessibility scenario now passes twice after a clean renderer rebuild.
| const label = | ||
| durationMs === undefined | ||
| ? copy.workLog | ||
| : copy.workLogDuration(formatTurnDuration(durationMs)); |
There was a problem hiding this comment.
P2 — Do not label time-to-last-text as total work duration
turn.durationMs still means “user send to latest assistant message” (materialize.ts:759-767). This new label says “Worked for …”, but failed, aborted, and tool-ending Turns can keep working long after their last text. The new regression itself has assistant text at ts=2, a failed tool result at 120000, and terminal state at 120001, yet asserts durationMs === 1; rendering that projection produces Worked for 0s for a two-minute failed Turn. Use a terminal/work-span metric for this label, or omit the duration when only the time-to-text metric exists.
There was a problem hiding this comment.
Fixed in 588447b7d. turn.durationMs keeps its existing time-to-latest-assistant meaning. The work-log label now consumes a separate UI projection field, workDurationMs, derived from the recorded terminal turn_state timestamp when available and from the last settled durable event for legacy terminal turns. The two-minute failed-turn regression now projects a two-minute work span while preserving durationMs === 1, and running turns still use the live capture path.
Review follow-up on
|
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed exact head 588447b7d90adc3b45a62a85af12997558806cd4. I found no new P0-P3 code issues in the six-file follow-up.
Both prior P2 findings are closed:
packages/ui/src/processing-summary.ts:161-193restores the prior name-based classifier only when persisted rows lackactivityKind, while keeping explicit runtime metadata authoritative. The legacyBashaccessibility scenario now exposes运行 1 条命令again.packages/ui/src/materialize.ts:676-680,828-834adds a separateworkDurationMsbased on the terminalturn_statetimestamp, with a last-durable-event fallback for legacy Turns;packages/ui/src/chat-turn.tsx:709-712uses that metric without changing the existing time-to-answerdurationMscontract.
Validation on this head: clean production build, full typecheck, 339 UI tests, lint, format, ASF headers, renderer architecture, Astryx theme/inventory, and git diff --check; the two affected Electron E2E scenarios passed, and the previously flaky transcript-scroll scenario passed five additional repetitions with four workers. A clean synthetic merge onto current main f9d3d7e031e9d19b15fcea99cbdede0b78a20e19 also passed build, full typecheck, 351 UI tests, static checks, and both affected E2E scenarios.
This branch is not merge-ready yet because the required hosted test check is red. Attempt 3 checked out stale merge commit 2bed13fd9 (588447b7d merged into c86da40d7) and failed only at the CLI production dependency audit. The exact branch reports two production advisories; the current-main synthetic merge reports zero because f9d3d7e already contains the dependency fix. Please refresh/rebase the branch and rerun the required check against current main. I did not classify that stale-base gate as a defect in this six-file follow-up.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
|
Revision update for
Please re-review the current head when convenient. |
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
NO-GO on exact head 5a293b6807354fe98eabb2558d443ddfb31c75b4 due to one P2 test-reliability finding in the two-file follow-up. The prior product fixes remain unchanged: this head adds only Electron E2E stabilization changes on top of the already reviewed tree.
The hosted required test run 33675695252 passed, including one four-worker Desktop E2E execution. However, the changed code-scroll test still fails nondeterministically under the same four-worker concurrency: two independent exact-head runs failed 1/5 and 1/10 repetitions at the new selection auto-scroll assertion after the overflow precondition had already passed. transcript-measure passed 5/5. Clean install and production build passed on this head; the previously reviewed production tree remains byte-identical apart from these two test files. A clean merge-tree onto current main eacfb46aa7ec93273bf468335f4270bba62d35a5 also succeeds.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
| }); | ||
| await expect.poll( | ||
| () => viewport.evaluate((element) => (element as HTMLElement).scrollLeft), | ||
| ).toBeGreaterThan(0); |
There was a problem hiding this comment.
P2: This still leaves the required E2E flaky under CI's four-worker concurrency. On this exact head, --workers=4 --repeat-each=5 failed 1/5 and a separate 10-repeat run failed 1/10 here, both after the new overflow precondition had passed. The failure screenshot shows a genuinely overflowed code block with its scrollbar still at zero: moving the selection drag earlier does not make Chromium's edge auto-scroll deterministic while parallel Electron workers are busy. Because the required job uses four workers, this can continue to block unrelated changes randomly even though today's hosted run happened to pass. Please make this interaction deterministic, or isolate the selection auto-scroll check from the overloaded multi-worker path and pin the intended execution mode with repeated coverage.
Review follow-up on
|
Hosted CI follow-up on
|
|
Thanks for sticking with this one, and for the screenshots along the way — they made the remaining question easy to look at directly. I owe you a correction first. In my 8/30 review I said the collapse hierarchy looked good independently, and I should have pushed on it then instead of now. Having built storybook on both sides and put the same stories side by side, I'd like to land the runtime and prompt work and hold the presentation change back to Why. The fold assumes a Turn shaped like Codex's: progress text, then tools, then trailing final text. When a Turn doesn't end that way, everything the model said disappears behind a duration header. This is your own
Reachability is category ① — the Stop button is an ordinary control, and Anthropic / Chat Completions Turns land here without any interruption at all, since they carry no The second thing is that the flat timeline was already doing the job.
A tool result now takes three disclosures instead of one, and the full-width grey band and left rail are elements the rest of the transcript doesn't use. What I'd suggest keeping: the None of that work is lost. Once the prompt has been on One more finding that has no line in this diff to hang on: P1, category ① — Six more inline, rated, in the review alongside this comment. The first is the only blocker: two findings come from a single block, and deleting it costs nothing. How I checked: storybook built on 简体中文先认个错：8/30 我说折叠层级独立成立，当时就该追问。两边构建 storybook 把同一批 story 并排看过之后，建议保留 runtime 和 prompt 部分，展示层退回 原因是折叠假设了 Codex 形状的 Turn。不按这个形状结束时，模型说过的话整体消失在一个时长折叠头后面——上图就是你自己的 另一点是平铺本来就够用： 建议保留 prompt 片段、text-end 与 item 边界、"等待模型输出"替换、Copy 单一权威；暂缓 另有一条 P1 挂不到本次 diff 的行上：side-chat 面板的 |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Six rated findings inline. The presentation argument, the side-by-side storybook images, and one more P1 that has no line in this diff to hang on are in the comment just above.
| stepTextPartStartOffset = stepText.length; | ||
| if (nextProviderOptions !== undefined) { |
There was a problem hiding this comment.
Could we drop the providerOptions merge here and keep only providerItemBoundary? Two findings trace back to this one block, and removing it doesn't cost any behavior this PR asserts — with it deleted the full ai-sdk-backend suite is 223/223, including preserves native Responses text item boundaries, merges citation metadata across multiple provider text items, and does not carry metadata from an empty Responses text item. Responses emits text-end at response.output_item.done carrying the same itemId, phase and annotations, so nothing is lost.
P1, category ② (provider stall → idle-watchdog recovery). On main, stepTextProviderOptions had one writer (text-metadata), which also set attemptSawContinuationMetadata — "step text has providerOptions" and "this attempt is retryable" were mutually exclusive by construction. Writing it here without the flag (correctly, per the last round) breaks that pairing, and the retry branch only flushes when stepThinkingParts is non-empty. Your own retries an idle watchdog timeout after an unstarted Responses text item shows it; one added line:
assert.equal(recovered?.providerOptions, undefined);fails with {openai:{itemId:'message-item-1', phase:'commentary'}} on a durable message whose text is 'recovered' — the abandoned item's identity, plus commentary stamped on what is actually the final answer. It's persisted, so it goes back to the provider on every later request in that session.
P1, category ① (normal path on Google and Anthropic connections). model-adapter.ts:984 attaches text-start metadata for every provider, not only Responses. @ai-sdk/anthropic sends text-start with {anthropic:{type:'compaction'}}; @ai-sdk/google sends {google:{thoughtSignature}}, which its request transform passes straight back. Since mergeTextProviderOptions folds a step's text parts into one message-level object, the last part's signature lands on the whole concatenated text: two parts carrying sig-part-1 and sig-part-2 persist as text: 'first part second part' with sig-part-2. main dropped text-start metadata entirely, so this is new, and Gemini 3 validates those signatures. Neither path has coverage — the new adapter tests only exercise openai and openai-compatible.
| } | ||
|
|
||
| export function finalAssistantReplyText(turn: TurnViewModel): string { | ||
| if (turn.status !== "completed") return ""; |
There was a problem hiding this comment.
P1, category ① (Stop button). if (turn.status !== "completed") return ""; is what disables Copy on aborted and failed Turns whose text is visible, and pressing Stop is an ordinary action.
My read is that "is this Turn settled enough to fold" and "does this Turn have a final reply" are two different questions, and only the first belongs to status. Whether an aborted Turn should copy its partial text is a product call — I'd say yes, it's text the user can see — but either way the render path and the Copy gate need to agree.
| @@ -916,7 +957,7 @@ function TurnFooterActions(props: { | |||
| } | |||
|
|
|||
| async function copyAssistantText() { | |||
| if (!props.assistantText || copyPendingRef.current) return; | |||
| if (copyPendingRef.current) return; | |||
There was a problem hiding this comment.
P2, category ① (in combination with the side-chat gate noted in the comment above). Could we keep the !props.assistantText guard that used to lead this function? On its own it's defense in depth — it only matters when a caller's enabled disagrees with what the button copies. Cheap to keep even once that authority is settled, and it's the difference between a no-op and a cleared clipboard.
| ): Record<string, unknown> | undefined { | ||
| const phase = record?.phase; | ||
| if (phase !== 'commentary' && phase !== 'final_answer') return undefined; | ||
| const itemId = stringField(record, 'id'); |
There was a problem hiding this comment.
P2, category ② (an imported Codex session continued against OpenAI). Would it be safer to import phase only and leave id behind? phase is a descriptive label and round-trips harmlessly. itemId, though, is replayed to the Responses API as a pointer into Codex's own response store — an item created by another product under another account. Nothing in the repo reads phase today either (the UI's commentary treatment is positional), so itemId is currently the only field with an effect.
Also worth noting the event_msg branch at :388 takes id from a payload where it isn't defined as an OpenAI item id, and the fixture has no id, so that branch is uncovered.
| Avoid empty narration such as "I will take a look", "Working on it", "Continuing", or announcing a routine tool choice. Describe useful intent, findings, decisions, or changed direction instead. | ||
| Do not expose hidden reasoning or repeat commands, tool names, counts, durations, or other raw activity that the interface already shows. | ||
| Skip progress updates only when no tool is needed or exactly one obvious, quick tool call answers the whole request. | ||
| Checking several requested facts is a multi-step task even when those checks could be combined into one shell command. |
There was a problem hiding this comment.
P3, category ① (simple factual questions). This line pulls against ## Response format's "Keep simple answers simple" — "what branch am I on and what node version" would now get a progress update first. The cost is verbosity and tokens rather than correctness, hence P3, but could it come out, or the fragment be scoped per lane? It currently applies to Side Chat, Deep Research and Plan alike.
(Sub-agents are clean — I checked that childInstruction bypasses the assembler.)
| await page.mouse.move(metrics.rect.x + metrics.rect.width + 50, textY, { | ||
| steps: 20, | ||
| }); | ||
| await expect.poll( |
There was a problem hiding this comment.
P2, test reliability. This one looks unrelated to the feature, and main has fde3253a7 test(desktop): deselect-proof the code-scroll selection drag going the other way on the same file — the selection drag moves earlier here and removeAllRanges() is gone, so the later wheel assertions now run on top of a live selection. A flaky required job blocks unrelated changes, which is the same reason your own run flagged it. Worth rebasing onto that commit and seeing whether it clears on its own.
| timeout: 20_000, | ||
| }); | ||
| const answer = page | ||
| .locator('.maka-turn') |
There was a problem hiding this comment.
P2, test reliability. Also unrelated to the feature as far as I can tell: this trades a semantic query (getByRole('article')) for a structural one (.maka-turn), which couples the test to markup this PR happens to change. If the scoping is needed for the measurement, could the outer locator stay role-based?
hqhq1025
changed the title
Review follow-up on
|
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
All seven findings are closed on ca59da1b6, and closed by deletion — the commit is −1749/+92 and the branch is now 19 files, +562/−84 against main.
I re-verified rather than read:
- runtime suite 216/216, with the
assert.equal(recovered?.providerOptions, undefined)line now living in your own idle-watchdog test; - the Google
thoughtSignaturepath, which previously stamped the last part's signature onto the whole concatenated text, now persiststext: 'first part second part'with noproviderOptionsat all; - an aborted Turn renders its text and copies it —
clipboard writes: ["partial answer the user can see"], and no work-log wrapper in the rendered output; - storybook rebuilt at this head:
InterruptedToolAfterTurnAbortandRunningStatusDuringToolRunare pixel-identical tomainapart from the status label.
Thanks for taking the rollback in good humour — it was a lot of work to remove, and what's left is the part that actually needed to exist. Merging.
简体中文
七条在 ca59da1b6 全部关闭，而且是靠删除关闭的：该 commit 净 −1749/+92，分支相对 main 只剩 19 个文件、+562/−84。
我重新实测而非只读：runtime 216/216；Google thoughtSignature 那条路径不再附着任何 providerOptions；中断 Turn 正常显示并能复制；在此 head 重新构建 storybook，两个 story 与 main 除状态文案外像素一致。
辛苦把这部分删干净，留下的正是真正需要存在的那部分。这就合并。
2 participants
Summary
thinking/tools/textorder for provider-executed tool steps so live and replayed transcripts agreeWaiting for model output…state while the provider has emitted no semantic progressfinalAssistantReplyTextconsistently for the Copy payload and Copy availability in the main transcript and Side Chatphaseas descriptive metadata without replaying foreign Codex item IDsRefs #4268
Architecture
All supported protocols can emit ordinary assistant text around tool calls, so Maka uses the existing text stream and flat timeline for progress and final output:
flowchart LR A[Responses / Chat Completions / Anthropic] -->|ordinary assistant text| B[Existing text stream] B --> C[Flat chronological transcript] C --> D[Visible progress, reasoning, tools, and final output] E[Responses native item metadata] -->|adapter and replay only| F[Provider options and item boundaries] G[Runtime tool events] -->|derived UI only| CThis PR deliberately does not add:
The presentation experiment is deferred until production transcripts provide evidence for a fold that works across providers, aborted and failed Turns, and Turns that end with tool activity.
Behavior
text-startitem to contaminate retry output.Verification
npm run typechecknpm run lintnpm run format:checknpm run check:asf-headersnpm run astryx:surface-inventoryorigin/mainAI use
Select exactly one:
Tool(s) and scope: OpenAI Codex investigated the provider/runtime/UI contracts, implemented and tested the progress behavior, incorporated reviewer feedback, performed adversarial self-review, and prepared this revision. The human contributor remains responsible for review and submission.
Checklist
mainDoes this PR entail a change in behavior?