-
Notifications
You must be signed in to change notification settings - Fork 85
Feat/tool call timestamps #913
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
base: main
Are you sure you want to change the base?
Changes from all commits
d51fcf2
c6e3758
612a111
a024d9d
cb587c4
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,7 @@ import { | |||||
| For, | ||||||
| Match, | ||||||
| on, | ||||||
| onCleanup, | ||||||
| onMount, | ||||||
| Show, | ||||||
| Switch, | ||||||
|
|
@@ -33,6 +34,7 @@ import { Prompt, type PromptRef } from "@tui/component/prompt" | |||||
| import type { AssistantMessage, Part, ToolPart, UserMessage, TextPart, ReasoningPart } from "@opencode-ai/sdk/v2" | ||||||
| import { useLocal } from "@tui/context/local" | ||||||
| import { Locale } from "@/util/locale" | ||||||
| import { SLOW_TOOL_MS, formatElapsed, type Elapsed } from "./timing" | ||||||
| import type { Tool } from "@/tool/tool" | ||||||
| import type { ReadTool } from "@/tool/read" | ||||||
| import type { WriteTool } from "@/tool/write" | ||||||
|
|
@@ -152,7 +154,7 @@ export function Session() { | |||||
| const [sidebarOpen, setSidebarOpen] = createSignal(false) | ||||||
| const [conceal, setConceal] = createSignal(true) | ||||||
| const [showThinking, setShowThinking] = kv.signal("thinking_visibility", true) | ||||||
| const [timestamps, setTimestamps] = kv.signal<"hide" | "show">("timestamps", "hide") | ||||||
| const [timestamps, setTimestamps] = kv.signal<"hide" | "show">("timestamps", "show") | ||||||
| const [showDetails, setShowDetails] = kv.signal("tool_details_visibility", true) | ||||||
| const [showAssistantMetadata, setShowAssistantMetadata] = kv.signal("assistant_metadata_visibility", true) | ||||||
| const [showScrollbar, setShowScrollbar] = kv.signal("scrollbar_visible", true) | ||||||
|
|
@@ -615,12 +617,12 @@ export function Session() { | |||||
| }, | ||||||
| }, | ||||||
| { | ||||||
| title: showTimestamps() ? "Hide timestamps" : "Show timestamps", | ||||||
| title: showTimestamps() ? "Hide tool timing" : "Show tool timing", | ||||||
| value: "session.toggle.timestamps", | ||||||
| category: "Session", | ||||||
| slash: { | ||||||
| name: "timestamps", | ||||||
| aliases: ["toggle-timestamps"], | ||||||
| aliases: ["toggle-timestamps", "timing", "toggle-timing"], | ||||||
| }, | ||||||
| onSelect: (dialog) => { | ||||||
| setTimestamps((prev) => (prev === "show" ? "hide" : "show")) | ||||||
|
|
@@ -1690,6 +1692,25 @@ function ToolTitle(props: { fallback: string; when: any; icon: string; children: | |||||
| ) | ||||||
| } | ||||||
|
|
||||||
| // Elapsed time (ms) for a tool part. Returns undefined for "pending" (no start yet), | ||||||
| // ticks at 1Hz while "running", freezes to end-start on "completed"/"error". | ||||||
| function useElapsed(part: () => ToolPart | undefined) { | ||||||
| const [now, setNow] = createSignal(Date.now()) | ||||||
| createEffect(() => { | ||||||
| if (part()?.state.status !== "running") return | ||||||
| const id = setInterval(() => setNow(Date.now()), 1000) | ||||||
| onCleanup(() => clearInterval(id)) | ||||||
| }) | ||||||
| return createMemo<Elapsed | undefined>(() => { | ||||||
| const p = part() | ||||||
| if (!p) return undefined | ||||||
| const s = p.state | ||||||
| if (s.status === "pending") return undefined | ||||||
| if (s.status === "running") return { start: s.time.start, ms: now() - s.time.start, running: true } | ||||||
|
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. P2: Clamp elapsed to zero to prevent negative duration on first render. The Prompt for AI agents
Suggested change
|
||||||
| return { start: s.time.start, ms: s.time.end - s.time.start, running: false } | ||||||
|
Comment on lines
+1698
to
+1710
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. Prevent negative elapsed values on pending→running transition.
Suggested fix function useElapsed(part: () => ToolPart | undefined) {
const [now, setNow] = createSignal(Date.now())
createEffect(() => {
if (part()?.state.status !== "running") return
+ setNow(Date.now())
const id = setInterval(() => setNow(Date.now()), 1000)
onCleanup(() => clearInterval(id))
})
return createMemo<Elapsed | undefined>(() => {
const p = part()
if (!p) return undefined
const s = p.state
if (s.status === "pending") return undefined
- if (s.status === "running") return { start: s.time.start, ms: now() - s.time.start, running: true }
- return { start: s.time.start, ms: s.time.end - s.time.start, running: false }
+ if (s.status === "running") return { start: s.time.start, ms: Math.max(0, now() - s.time.start), running: true }
+ return { start: s.time.start, ms: Math.max(0, s.time.end - s.time.start), running: false }
})
}🤖 Prompt for AI Agents |
||||||
| }) | ||||||
| } | ||||||
|
|
||||||
| function InlineTool(props: { | ||||||
| icon: string | ||||||
| iconColor?: RGBA | ||||||
|
|
@@ -1729,6 +1750,14 @@ function InlineTool(props: { | |||||
| error()?.includes("user 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. [LOW · code-reviewer] The useElapsed hook calls clearInterval(id) on cleanup, but does not check if id is defined before clearing. If the effect runs and immediately exits (e.g., if part() is undefined), id may be undefined, leading to a potential runtime warning or error in some environments. 💡 Suggestion: Wrap clearInterval(id) in a conditional: if (id) clearInterval(id). Confidence: 85/100 |
||||||
| ) | ||||||
|
|
||||||
| const elapsed = useElapsed(() => props.part) | ||||||
|
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. P2: InlineTool spinner branch omits elapsed timing display Prompt for AI agents |
||||||
| const timingVisible = createMemo(() => ctx.showTimestamps() && !!elapsed()) | ||||||
| const timingColor = createMemo(() => { | ||||||
| const e = elapsed() | ||||||
| if (e?.running && e.ms >= SLOW_TOOL_MS) return theme.warning | ||||||
| return theme.textMuted | ||||||
| }) | ||||||
|
|
||||||
| return ( | ||||||
| <box | ||||||
| marginTop={margin()} | ||||||
|
|
@@ -1771,6 +1800,12 @@ function InlineTool(props: { | |||||
| <Show fallback={<>~ {props.pending}</>} when={props.complete}> | ||||||
| <span style={{ fg: props.iconColor }}>{props.icon}</span> {props.children} | ||||||
| </Show> | ||||||
| <Show when={timingVisible()}> | ||||||
| <span style={{ fg: timingColor() }}> | ||||||
| {" · "} | ||||||
| {formatElapsed(elapsed()!)} | ||||||
| </span> | ||||||
| </Show> | ||||||
| </text> | ||||||
| </Match> | ||||||
| </Switch> | ||||||
|
|
@@ -1788,10 +1823,18 @@ function BlockTool(props: { | |||||
| part?: ToolPart | ||||||
| spinner?: boolean | ||||||
| }) { | ||||||
| const ctx = use() | ||||||
| const { theme } = useTheme() | ||||||
| const renderer = useRenderer() | ||||||
| const [hover, setHover] = createSignal(false) | ||||||
| const error = createMemo(() => (props.part?.state.status === "error" ? props.part.state.error : undefined)) | ||||||
| const elapsed = useElapsed(() => props.part) | ||||||
| const timingVisible = createMemo(() => ctx.showTimestamps() && !!elapsed()) | ||||||
| const timingColor = createMemo(() => { | ||||||
| const e = elapsed() | ||||||
| if (e?.running && e.ms >= SLOW_TOOL_MS) return theme.warning | ||||||
| return theme.textMuted | ||||||
| }) | ||||||
| return ( | ||||||
| <box | ||||||
| border={["left"]} | ||||||
|
|
@@ -1814,11 +1857,19 @@ function BlockTool(props: { | |||||
| when={props.spinner} | ||||||
| fallback={ | ||||||
| <text paddingLeft={3} fg={theme.textMuted}> | ||||||
| {props.title} | ||||||
| <span>{props.title}</span> | ||||||
| <Show when={timingVisible()}> | ||||||
| <span style={{ fg: timingColor() }}> | ||||||
| {" "} | ||||||
| {formatElapsed(elapsed()!)} | ||||||
| </span> | ||||||
| </Show> | ||||||
| </text> | ||||||
| } | ||||||
| > | ||||||
| <Spinner color={theme.textMuted}>{props.title.replace(/^# /, "")}</Spinner> | ||||||
| <Spinner color={theme.textMuted}> | ||||||
| {props.title.replace(/^# /, "") + (timingVisible() ? ` ${formatElapsed(elapsed()!)}` : "")} | ||||||
| </Spinner> | ||||||
| </Show> | ||||||
| {props.children} | ||||||
| <Show when={error()}> | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,10 +1,10 @@ | ||||||||||||||||||||||||||||||||||||||||||
| import { useSync } from "@tui/context/sync" | ||||||||||||||||||||||||||||||||||||||||||
| import { createMemo, For, Show, Switch, Match } from "solid-js" | ||||||||||||||||||||||||||||||||||||||||||
| import { createEffect, createMemo, createSignal, For, onCleanup, Show, Switch, Match } from "solid-js" | ||||||||||||||||||||||||||||||||||||||||||
| import { createStore } from "solid-js/store" | ||||||||||||||||||||||||||||||||||||||||||
| import { useTheme } from "../../context/theme" | ||||||||||||||||||||||||||||||||||||||||||
| import { Locale } from "@/util/locale" | ||||||||||||||||||||||||||||||||||||||||||
| import path from "path" | ||||||||||||||||||||||||||||||||||||||||||
| import type { AssistantMessage } from "@opencode-ai/sdk/v2" | ||||||||||||||||||||||||||||||||||||||||||
| import type { AssistantMessage, ToolPart } from "@opencode-ai/sdk/v2" | ||||||||||||||||||||||||||||||||||||||||||
| import { Global } from "@/global" | ||||||||||||||||||||||||||||||||||||||||||
| import { Installation } from "@/installation" | ||||||||||||||||||||||||||||||||||||||||||
| import { useKeybind } from "../../context/keybind" | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -62,6 +62,39 @@ export function Sidebar(props: { sessionID: string; overlay?: boolean }) { | |||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // altimate_change start - total tool time | ||||||||||||||||||||||||||||||||||||||||||
| // Flatten every tool part in this session so we can sum execution time across | ||||||||||||||||||||||||||||||||||||||||||
| // completed/error tools and live-tick any tool that's still running. | ||||||||||||||||||||||||||||||||||||||||||
| const toolParts = createMemo<ToolPart[]>(() => { | ||||||||||||||||||||||||||||||||||||||||||
| const out: ToolPart[] = [] | ||||||||||||||||||||||||||||||||||||||||||
| for (const message of messages()) { | ||||||||||||||||||||||||||||||||||||||||||
| const parts = sync.data.part[message.id] | ||||||||||||||||||||||||||||||||||||||||||
| if (!parts) continue | ||||||||||||||||||||||||||||||||||||||||||
| for (const part of parts) { | ||||||||||||||||||||||||||||||||||||||||||
| if (part.type === "tool") out.push(part) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| return out | ||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||
| const hasRunningTool = createMemo(() => toolParts().some((p) => p.state.status === "running")) | ||||||||||||||||||||||||||||||||||||||||||
| const [nowToolTime, setNowToolTime] = createSignal(Date.now()) | ||||||||||||||||||||||||||||||||||||||||||
| createEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||
| if (!hasRunningTool()) return | ||||||||||||||||||||||||||||||||||||||||||
| const id = setInterval(() => setNowToolTime(Date.now()), 1000) | ||||||||||||||||||||||||||||||||||||||||||
| onCleanup(() => clearInterval(id)) | ||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||
| const toolTime = createMemo(() => { | ||||||||||||||||||||||||||||||||||||||||||
| let ms = 0 | ||||||||||||||||||||||||||||||||||||||||||
| const tick = nowToolTime() | ||||||||||||||||||||||||||||||||||||||||||
| for (const part of toolParts()) { | ||||||||||||||||||||||||||||||||||||||||||
| const s = part.state | ||||||||||||||||||||||||||||||||||||||||||
| if (s.status === "running") ms += tick - s.time.start | ||||||||||||||||||||||||||||||||||||||||||
|
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. P2: Clamp to zero to prevent transient negative tool-time contribution. Prompt for AI agents
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
| else if (s.status === "completed" || s.status === "error") ms += s.time.end - s.time.start | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+80
to
+93
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. Seed the running-time ticker immediately to avoid stale/negative totals. When a tool starts after sidebar mount, Suggested fix const [nowToolTime, setNowToolTime] = createSignal(Date.now())
createEffect(() => {
if (!hasRunningTool()) return
+ setNowToolTime(Date.now())
const id = setInterval(() => setNowToolTime(Date.now()), 1000)
onCleanup(() => clearInterval(id))
})
const toolTime = createMemo(() => {
let ms = 0
const tick = nowToolTime()
for (const part of toolParts()) {
const s = part.state
- if (s.status === "running") ms += tick - s.time.start
- else if (s.status === "completed" || s.status === "error") ms += s.time.end - s.time.start
+ if (s.status === "running") ms += Math.max(0, tick - s.time.start)
+ else if (s.status === "completed" || s.status === "error") ms += Math.max(0, s.time.end - s.time.start)
}
return ms
})🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
| return ms | ||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+86
to
+95
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. [🟠 MEDIUM] If the sidebar is mounted significantly earlier than a tool starts executing, the This will either temporarily hide the tool time from the UI (if total time drops ≤ 0) or display an incorrect value for the first second of execution. We can fix this by ensuring Suggested change:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
| // altimate_change end | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const directory = useDirectory() | ||||||||||||||||||||||||||||||||||||||||||
| const kv = useKV() | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -107,6 +140,13 @@ export function Sidebar(props: { sessionID: string; overlay?: boolean }) { | |||||||||||||||||||||||||||||||||||||||||
| <text fg={theme.textMuted}>{context()?.tokens ?? 0} tokens</text> | ||||||||||||||||||||||||||||||||||||||||||
|
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. [CRITICAL · code-reviewer] The toolTime calculation uses Date.now() as a live tick for running tools, but does not account for the possibility that multiple tools may be running simultaneously and their start times may be from different epochs. This could lead to incorrect cumulative timing if tools start at different times and the live tick advances uniformly. 💡 Suggestion: Instead of using a single nowToolTime signal for all tools, compute elapsed time per tool individually using their own start time and the current time, avoiding shared tick logic that assumes synchronized timing. Confidence: 90/100 |
||||||||||||||||||||||||||||||||||||||||||
| <text fg={theme.textMuted}>{context()?.percentage ?? 0}% used</text> | ||||||||||||||||||||||||||||||||||||||||||
| <text fg={theme.textMuted}>{cost()} spent</text> | ||||||||||||||||||||||||||||||||||||||||||
| {/* altimate_change start - total tool time */} | ||||||||||||||||||||||||||||||||||||||||||
| <Show when={toolTime() > 0}> | ||||||||||||||||||||||||||||||||||||||||||
| <text fg={theme.textMuted}> | ||||||||||||||||||||||||||||||||||||||||||
| {Locale.duration(toolTime())} in tools{hasRunningTool() ? " (running)" : ""} | ||||||||||||||||||||||||||||||||||||||||||
| </text> | ||||||||||||||||||||||||||||||||||||||||||
| </Show> | ||||||||||||||||||||||||||||||||||||||||||
| {/* altimate_change end */} | ||||||||||||||||||||||||||||||||||||||||||
| </box> | ||||||||||||||||||||||||||||||||||||||||||
| {/* altimate_change start - trace section */} | ||||||||||||||||||||||||||||||||||||||||||
| <box> | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import { Locale } from "@/util/locale" | ||
|
|
||
| // Threshold above which a still-running tool is rendered in the warning color. | ||
| export const SLOW_TOOL_MS = 30_000 | ||
|
|
||
| export interface Elapsed { | ||
| start: number | ||
| ms: number | ||
| running: boolean | ||
| } | ||
|
|
||
| // Single source of truth for the "HH:MM:SS · running 12.3s" suffix shown | ||
| // next to a tool call. Separator/prefix lives at the call site because it | ||
| // differs by surface (inline vs block). | ||
| export function formatElapsed(e: Elapsed): string { | ||
| return `${Locale.clockTime(e.start)} · ${e.running ? "running " : ""}${Locale.duration(e.ms)}` | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,11 @@ export namespace Locale { | |
| return date.toLocaleTimeString(undefined, { timeStyle: "short" }) | ||
| } | ||
|
|
||
| export function clockTime(input: number): string { | ||
| const date = new Date(input) | ||
| return date.toLocaleTimeString(undefined, { hour12: false, hour: "2-digit", minute: "2-digit", second: "2-digit" }) | ||
|
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. P1: Prompt for AI agents |
||
| } | ||
|
|
||
| export function datetime(input: number): string { | ||
| const date = new Date(input) | ||
| const localTime = time(input) | ||
|
|
||
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.
[HIGH · code-reviewer] The default value for 'timestamps' was changed from 'hide' to 'show'. While the PR claims this is intentional and user preferences persist, the CLAUDE.md in the project root (if present) may explicitly require backward-compatible defaults for UI settings unless explicitly approved by product. This change alters user experience without a documented product decision.
💡 Suggestion: Verify that a product decision or CLAUDE.md rule explicitly permits flipping the default from hide to show. If not, revert to 'hide' and document the change in a follow-up PR with product sign-off.
Confidence: 70/100