-
Notifications
You must be signed in to change notification settings - Fork 448
Remove misleading total_tokens: 0 from logs summary
#42722
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
31569f1
3cd425f
4107b58
21f74f1
56e8946
1bbbb96
ddd6ce3
d16e86c
53fa705
e70416f
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,55 @@ | ||
| //go:build !integration | ||
|
|
||
| package cli | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestRenderLogsTSVSummaryPreservesTokenField(t *testing.T) { | ||
| output, _ := captureOutput(t, func() error { | ||
| renderLogsTSV(LogsData{ | ||
| Summary: LogsSummary{ | ||
| TotalRuns: 2, | ||
| TotalDuration: "8m0s", | ||
| TotalTurns: 5, | ||
| TotalErrors: 1, | ||
| }, | ||
| }) | ||
| return nil | ||
| }) | ||
|
|
||
| lines := strings.Split(strings.TrimSpace(output), "\n") | ||
| if len(lines) == 0 { | ||
| t.Fatal("Expected TSV output") | ||
| } | ||
| if got, want := lines[0], "# 2 runs | 8m0s duration | n/a tokens | 5 turns | 1 errors"; got != want { | ||
| t.Fatalf("Unexpected TSV summary line:\n got: %q\nwant: %q", got, want) | ||
| } | ||
| } | ||
|
|
||
| func TestRenderLogsTSVVerboseSummaryPreservesTokenField(t *testing.T) { | ||
| output, _ := captureOutput(t, func() error { | ||
| renderLogsTSVVerbose(LogsData{ | ||
| Summary: LogsSummary{ | ||
| TotalRuns: 2, | ||
| TotalDuration: "8m0s", | ||
| TotalTokens: 1500, | ||
| TotalTurns: 5, | ||
| TotalErrors: 1, | ||
| TotalMissingTools: 2, | ||
| TotalGitHubAPICalls: 3, | ||
| }, | ||
| }) | ||
| return nil | ||
| }) | ||
|
|
||
| lines := strings.Split(strings.TrimSpace(output), "\n") | ||
| if len(lines) == 0 { | ||
| t.Fatal("Expected TSV verbose output") | ||
| } | ||
| if got, want := lines[0], "# 2 runs | 8m0s duration | 1500 tokens | 5 turns | 1 errors | 2 missing_tools | 3 github_api_calls"; got != want { | ||
| t.Fatalf("Unexpected TSV verbose summary line:\n got: %q\nwant: %q", got, want) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,8 +56,8 @@ type ContinuationData struct { | |
| type LogsSummary struct { | ||
| TotalRuns int `json:"total_runs" console:"header:Total Runs"` | ||
| TotalDuration string `json:"total_duration" console:"header:Total Duration"` | ||
| TotalTokens int `json:"total_tokens" console:"header:Total Tokens,format:number"` | ||
| TotalAIC float64 `json:"total_aic,omitempty"` | ||
| TotalTokens int `json:"total_tokens,omitempty" console:"header:Total Tokens,format:number,omitempty"` | ||
| TotalActionMinutes float64 `json:"total_action_minutes" console:"header:Total Action Minutes"` | ||
| TotalTurns int `json:"total_turns" console:"header:Total Turns"` | ||
| TotalSteeringEvents int `json:"total_steering_events,omitempty" console:"header:Total Steering Events,format:number,omitempty"` | ||
|
|
@@ -157,8 +157,8 @@ func buildLogsData(processedRuns []ProcessedRun, outputDir string, continuation | |
|
|
||
| // Build summary | ||
| var totalDuration time.Duration | ||
| var totalTokens int | ||
| var totalAIC float64 | ||
| var totalTokens int | ||
| var totalActionMinutes float64 | ||
| var totalTurns int | ||
| var totalSteeringEvents int | ||
|
|
@@ -191,10 +191,10 @@ func buildLogsData(processedRuns []ProcessedRun, outputDir string, continuation | |
| if run.Duration > 0 { | ||
|
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.
💡 Suggested fixThe PR's stated rationale is that the field is "always zero without A less destructive fix: keep the field but suppress it when zero: // In LogsSummary struct:
TotalTokens int `json:"total_tokens,omitempty" console:"header:Total Tokens,format:number,omitempty"`This makes |
||
| totalDuration += run.Duration | ||
| } | ||
| totalTokens += run.TokenUsage | ||
| if pr.TokenUsage != nil { | ||
| totalAIC += pr.TokenUsage.TotalAIC | ||
| } | ||
| totalTokens += run.TokenUsage | ||
| totalActionMinutes += run.ActionMinutes | ||
| totalTurns += run.Turns | ||
| if pr.TokenUsage != nil { | ||
|
|
@@ -347,8 +347,8 @@ func buildLogsData(processedRuns []ProcessedRun, outputDir string, continuation | |
| summary := LogsSummary{ | ||
| TotalRuns: len(processedRuns), | ||
| TotalDuration: timeutil.FormatDuration(totalDuration), | ||
| TotalTokens: totalTokens, | ||
| TotalAIC: totalAIC, | ||
| TotalTokens: totalTokens, | ||
| TotalActionMinutes: totalActionMinutes, | ||
| TotalTurns: totalTurns, | ||
| TotalSteeringEvents: totalSteeringEvents, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.