-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix: show dismissed question content in chat history #12043
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "kilo-code": patch | ||
| --- | ||
|
|
||
| Fixed dismissed question tool content not showing in chat history. Dismissed questions now render with a "Dismissed" label and "N dismissed" subtitle instead of being invisible. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2815,28 +2815,64 @@ function TodoWrite(props: ToolProps<typeof TodoWriteTool>) { | |
| function Question(props: ToolProps<typeof QuestionTool>) { | ||
| const { theme } = useTheme() | ||
| const count = createMemo(() => props.input.questions?.length ?? 0) | ||
| // kilocode_change start - show dismissed question content with toggle; | ||
| // use input.questions presence (not metadata) so dismissed/answered/error | ||
| // states all render content. Clicking the one-liner expands to the full | ||
| // block; clicking the block title collapses back. | ||
| const dismissed = createMemo( | ||
| () => | ||
| props.metadata.dismissed === true || | ||
| (props.part.state.status === "error" && String(props.part.state.error ?? "").includes("dismissed")), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION: Looser/inconsistent dismissal match vs. the webview renderer This matches the substring Reply with |
||
| ) | ||
| const [expanded, setExpanded] = createSignal(false) | ||
|
|
||
| function format(answer?: ReadonlyArray<string>) { | ||
| if (dismissed()) return "Dismissed" | ||
| if (!answer?.length) return "(no answer)" | ||
| return answer.join(", ") | ||
| } | ||
|
|
||
| const title = createMemo(() => (dismissed() ? "# Questions (dismissed)" : "# Questions")) | ||
| const subtitle = createMemo(() => { | ||
| if (dismissed()) return `${count()} dismissed` | ||
| if ((props.metadata.answers?.length ?? 0) > 0) return `${count()} answered` | ||
| return `${count()} question${count() !== 1 ? "s" : ""}` | ||
| }) | ||
| // kilocode_change end | ||
|
|
||
| return ( | ||
| <Switch> | ||
| <Match when={props.metadata.answers}> | ||
| <BlockTool title="# Questions" part={props.part}> | ||
| <box gap={1}> | ||
| <For each={props.input.questions ?? []}> | ||
| {(q, i) => ( | ||
| <box flexDirection="column"> | ||
| <text fg={theme.textMuted}>{q.question}</text> | ||
| <text fg={theme.text}>{format(props.metadata.answers?.[i()])}</text> | ||
| </box> | ||
| )} | ||
| </For> | ||
| </box> | ||
| </BlockTool> | ||
| {/* kilocode_change start - toggle between one-liner and full block */} | ||
| <Match when={count() > 0}> | ||
| <Show | ||
| when={expanded()} | ||
| fallback={ | ||
| <InlineTool | ||
| icon="→" | ||
| complete={count()} | ||
| pending="Asking questions..." | ||
| part={props.part} | ||
| onClick={() => setExpanded(true)} | ||
| > | ||
| {subtitle()} | ||
| </InlineTool> | ||
| } | ||
| > | ||
| <BlockTool title={title()} part={props.part} onClick={() => setExpanded(false)}> | ||
| <box gap={1}> | ||
| <For each={props.input.questions ?? []}> | ||
| {(q, i) => ( | ||
| <box flexDirection="column"> | ||
| <text fg={theme.textMuted}>{q.question}</text> | ||
| <text fg={theme.text}>{format(props.metadata.answers?.[i()])}</text> | ||
| </box> | ||
| )} | ||
| </For> | ||
| </box> | ||
| </BlockTool> | ||
| </Show> | ||
| </Match> | ||
| {/* kilocode_change end */} | ||
| <Match when={true}> | ||
| <InlineTool icon="→" pending="Asking questions..." complete={count()} part={props.part}> | ||
| Asked {count()} question{count() !== 1 ? "s" : ""} | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SUGGESTION: Dismissed detection relies on implicit caller-side gating
dismissed()treats anystatus === "error"as a dismissal without inspecting the error text. That's only safe today because the caller (ToolPartDisplayabove, viaisDismissedQuestionError()) only routes an error-status question part into this renderer when the message already contains"dismissed this question"— any other error takes theCardfallback and never reaches this component. If that caller-side gating ever changes, a genuine tool error could silently render as "Dismissed" here instead of as an error. Consider checking the error text directly (e.g. a shared helper) rather than relying onstatus === "error"alone, so this renderer is correct independent of the caller.Reply with
@kilocode-bot fix itto have Kilo Code address this issue.