fix(tui): 修复 /help 单页可选执行 + UTF-8 + 覆盖率修复#208
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…-emitter fix(tools): 重构 EmitChunk 错误传播与调用语义
|
/review |
Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
…t, rate-limit misclassification - runtime.go: decrement attempt before reactive-compact continue so the retry does not consume a MaxLoops slot (fixes max_loops=1 case) - compact.go: reset TokenInputTotal/TokenOutputTotal on session before saving after compact, preventing stale high totals from being restored on the next run - errors.go: guard text-based context_too_long override to only apply for generic client errors (ErrorCodeClient); 429 rate-limit errors whose message contains token-count fragments are no longer mis-routed into the reactive-compact path; same guard added in IsContextTooLong - errors_test.go: add regression cases for the 429-with-token-count-message scenarios in NewProviderErrorFromStatus and IsContextTooLong Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Yumiue <188874804+Yumiue@users.noreply.github.com>
…e-146-1775790664 feat(runtime): recover with reactive compact on context-too-long
…-ask-select-closure feat(tui/runtime): 补齐权限 ask 的选择式闭环(上下选择 + Enter确认)
Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: minorcell <120795714+minorcell@users.noreply.github.com>
Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com>
…e-213-1775793526 fix(runtime/tools): 收敛 EmitChunk 遗留 review 问题
Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: minorcell <120795714+minorcell@users.noreply.github.com>
|
/review -codex |
| line, err := reader.ReadLine() | ||
| // 每次读取前优先响应上下文取消,避免取消请求被误判为流中断。 | ||
| select { | ||
| case <-ctx.Done(): |
There was a problem hiding this comment.
Once done has been set after a [DONE] frame, this unconditional ctx.Done() check can still return context.Canceled before the trailing blank line/EOF is consumed. In that case a successfully completed stream is reported as canceled and message_done is never emitted. A regression test that cancels after [DONE] but before EOF would reproduce this.
| a.transcript.Height = max(6, lay.rightHeight-activityHeight-menuHeight-promptHeight) | ||
| availableHeight := lay.rightHeight - activityHeight - menuHeight - promptHeight | ||
| minTranscriptHeight := max(6, lay.rightHeight/2) | ||
| a.transcript.Height = max(minTranscriptHeight, availableHeight) |
There was a problem hiding this comment.
a.transcript.Height now uses max(lay.rightHeight/2, availableHeight), but renderWaterfall() still renders the transcript with max(6, height-activity-menu-prompt). When those diverge, the visual transcript/prompt positions no longer match the coordinates used by inputBounds() / activityBounds(), so mouse hit-testing and scrolling become offset on layouts where the activity pane or command menu is visible.
| a.transcript.Height = max(6, lay.rightHeight-activityHeight-menuHeight-promptHeight) | ||
| availableHeight := lay.rightHeight - activityHeight - menuHeight - promptHeight | ||
| minTranscriptHeight := max(6, lay.rightHeight/2) | ||
| a.transcript.Height = max(minTranscriptHeight, availableHeight) |
There was a problem hiding this comment.
a.transcript.Height is now forced to at least half of the right pane, even when the activity panel, command menu, and prompt leave less space than that. The new renderWaterfall() path recomputes a smaller visible transcript height, so the viewport's internal height diverges from what is actually rendered. On shorter terminals this throws off AtBottom/scroll calculations and can clip or jump the transcript. The viewport height should stay in sync with the rendered container instead of enforcing a larger minimum here.
| autoCompacted = true | ||
| var compactResult contextcompact.Result | ||
| session, compactResult, _ = s.runCompactForSession(ctx, input.RunID, session, cfg, false) | ||
| session, compactResult, _ = s.runCompactForSession(ctx, input.RunID, session, cfg, contextcompact.ModeManual, false) |
There was a problem hiding this comment.
This is the new token-threshold auto-compact branch, but it still calls runCompactForSession(..., contextcompact.ModeManual, ...). As a result every compact_* event from an automatic compact is reported as manual, so the UI/docs added in this PR cannot distinguish an auto-compact from an explicit /compact. If auto-compaction is now a first-class path, it needs its own trigger mode instead of reusing manual.
| "maximum context length", | ||
| "maximum prompt length", | ||
| "prompt is too long", | ||
| "requested too many tokens", |
There was a problem hiding this comment.
"requested too many tokens" / "too many tokens" are too broad for context_too_long detection. Providers also use those phrases for bad-request cases like oversized max_tokens / requested output, which compaction cannot fix. In those cases this PR will route the error into the new reactive-compact retry path instead of surfacing the real configuration problem.
| activityHeight := a.activityPreviewHeight() | ||
| menuHeight := a.commandMenuHeight(width) | ||
| promptHeight := lipgloss.Height(a.renderPrompt(width)) | ||
| transcriptHeight := max(6, height-activityHeight-menuHeight-promptHeight) |
There was a problem hiding this comment.
renderWaterfall now recomputes its own transcript height instead of using a.transcript.Height, but applyComponentLayout, inputBounds, and transcriptBounds still use a.transcript.Height. On short panes those values diverge (applyComponentLayout enforces a half-pane minimum, this code does not), which will misplace the rendered prompt/activity area relative to the viewport and break mouse hit-testing/scroll positioning.
| line, err := reader.ReadLine() | ||
| // 每次读取前优先响应上下文取消,避免取消请求被误判为流中断。 | ||
| select { | ||
| case <-ctx.Done(): |
There was a problem hiding this comment.
Once [DONE] has been seen, this pre-read ctx.Done() check can still turn a successfully completed stream into context.Canceled if the request is canceled before the trailing blank line/EOF is consumed. In that case message_done never fires even though the model already finished. The cancel fast-path should be skipped after done == true, or handled after the pending completion is flushed.
| @@ -251,12 +252,12 @@ func (s *Service) Run(ctx context.Context, input UserInput) error { | |||
| if builtContext.ShouldAutoCompact && !autoCompacted { | |||
| autoCompacted = true | |||
There was a problem hiding this comment.
autoCompacted is flipped before we know whether compacting actually succeeded. Because runCompactForSession(..., failOnError=false) swallows runner/save errors and can also return Applied=false, a failed or no-op auto-compact permanently disables further threshold-triggered compaction for the rest of the run. That leaves later oversized turns with no second chance to compact.
| line, err := reader.ReadLine() | ||
| // 每次读取前优先响应上下文取消,避免取消请求被误判为流中断。 | ||
| select { | ||
| case <-ctx.Done(): |
There was a problem hiding this comment.
If the provider has already sent [DONE], canceling the request before the trailing blank line / EOF now returns context.Canceled here instead of finishing the stream. That skips message_done, so the last assistant turn and token usage can be dropped during normal shutdown races. Once done is true, this pre-read cancel check should stop treating the stream as incomplete.
| "maximum context length", | ||
| "maximum prompt length", | ||
| "prompt is too long", | ||
| "requested too many tokens", |
There was a problem hiding this comment.
The new "requested too many tokens" / "too many tokens" fragments are broad enough to catch ordinary 400 validation errors that are not caused by prompt/context size (for example, asking for too many completion tokens). Those requests will now be upgraded to context_too_long and sent through the reactive-compact retry path even though compact cannot fix them.
| a.transcript.Height = max(6, lay.rightHeight-activityHeight-menuHeight-promptHeight) | ||
| availableHeight := lay.rightHeight - activityHeight - menuHeight - promptHeight | ||
| minTranscriptHeight := max(6, lay.rightHeight/2) | ||
| a.transcript.Height = max(minTranscriptHeight, availableHeight) |
There was a problem hiding this comment.
a.transcript.Height now uses max(minTranscriptHeight, availableHeight), but renderWaterfall still renders the transcript with just availableHeight. On short panes the viewport model thinks it is taller than the box actually drawn on screen, which throws off bottom detection/scroll math and causes clipping or jumpy scrolling.
|
@xgopilot 解决review提到的问题 |
已完成已修复 review 提到的几处回归,并已推送到当前 PR 分支,提交为 本次处理
验证
|
Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: creatang <165447160+creatang@users.noreply.github.com>
变更说明
/help改为与/model一致的可选择弹层展示/help列表改为单页展示(关闭分页并自适应高度)UTF-8 与覆盖率修复
update.go、view.go、copy_code.go、app.go相关测试路径refreshHelpPicker无效错误分支,降低不可达覆盖噪音验证
go test ./internal/tui/...