Repository navigation
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| const timingTooltip = entry.result | ||
| ? formatTimingTooltip( | ||
| { | ||
| serverTimeMs: entry.result.server_time_ms, |
There was a problem hiding this comment.
WARNING: totalTimeMs uses client-side entry.executionTime which may not equal server_time_ms + fetch_time_ms
The tooltip passes entry.executionTime (client-measured total) as totalTimeMs, but server_time_ms and fetch_time_ms are server-measured. Network latency and client processing can cause these to not add up (e.g., server=30ms + fetch=15ms ≠ client=50ms). Consider computing total as (serverTimeMs ?? 0) + (fetchTimeMs ?? 0) when both server and fetch are present, or omit the total line when breakdown is shown.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| const timingTooltip = entry.result | ||
| ? formatTimingTooltip( | ||
| { | ||
| serverTimeMs: entry.result.server_time_ms, |
There was a problem hiding this comment.
WARNING: totalTimeMs uses client-side entry.executionTime which may not equal server_time_ms + fetch_time_ms
Same issue as in ResultEntryContent: the tooltip passes entry.executionTime (client-measured) as totalTimeMs, but server_time_ms/fetch_time_ms are server-measured. The parts may not add up to the total shown. Consider computing total from server+fetch when both are available.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| activeTab.result | ||
| ? formatTimingTooltip( | ||
| { | ||
| serverTimeMs: activeTab.result.server_time_ms, |
There was a problem hiding this comment.
WARNING: totalTimeMs uses client-side activeTab.executionTime which may not equal server_time_ms + fetch_time_ms
Same issue: activeTab.executionTime is client-measured total time, while server_time_ms/fetch_time_ms are server-measured. The tooltip may show confusing numbers where Execution + Fetch ≠ Total. Consider using (serverTimeMs ?? 0) + (fetchTimeMs ?? 0) for the total when breakdown is available.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| truncated: set.truncated, | ||
| pagination: None, | ||
| additional_results: None, | ||
| server_time_ms: None, |
There was a problem hiding this comment.
SUGGESTION: Additional result sets from multi-result statements lack timing info
The additional_results (from stored procedures with multiple SELECTs) have server_time_ms: None and fetch_time_ms: None. Consider propagating timing per result set if feasible, or document this limitation. The primary result set correctly includes timing.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous Review Summary (commit 532373f)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 532373f)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (23 files)
Reviewed by free · Input: 161.8K · Output: 3.7K · Cached: 82.1K |
….ts, and ignore volatile timing in golden captures
Preview buildThe preview build of commit |
This branch has not been deployed
1 participant
Closes #990
Summary
Splits the query duration under query results into server/execution time (until the first row is received) and fetch time (transferring and reading the remaining rows), surfaced on hover via a breakdown tooltip.
Verification