fix: Minor issues #12 and #13 - ASCII mode and performance optimization

This commit addresses the remaining Minor issues from PR #94 code review:

Minor #12: Unicode Symbol Compatibility
- Added CODEAGENT_ASCII_MODE environment variable support
- When set to "true", uses ASCII symbols: PASS/WARN/FAIL
- Default behavior (unset or "false"): Unicode symbols ✓/⚠️/✗
- Updated help text to document the environment variable
- Added tests for both ASCII and Unicode modes

Implementation:
- executor.go:514: New getStatusSymbols() function
- executor.go:531: Dynamic symbol selection in generateFinalOutputWithMode
- main.go:34: useASCIIMode variable declaration
- main.go:495: Environment variable documentation in help
- executor_concurrent_test.go:292: Tests for ASCII mode
- main_integration_test.go:89: Parser updated for both symbol formats

Minor #13: Performance Optimization - Reduce Repeated String Operations
- Optimized Message parsing to split only once per task result
- Added *FromLines() variants of all extractor functions
- Original extract*() functions now wrap *FromLines() for compatibility
- Reduces memory allocations and CPU usage in parallel execution

Implementation:
- utils.go:300: extractCoverageFromLines()
- utils.go:390: extractFilesChangedFromLines()
- utils.go:455: extractTestResultsFromLines()
- utils.go:551: extractKeyOutputFromLines()
- main.go:255: Single split with reuse: lines := strings.Split(...)

Backward Compatibility:
- All original extract*() functions preserved
- Tests updated to handle both symbol formats
- No breaking changes to public API

Test Results:
- All tests pass: go test ./... (40.164s)
- ASCII mode verified: PASS/WARN/FAIL symbols display correctly
- Unicode mode verified: ✓/⚠️/✗ symbols remain default
- Performance: Single split per Message instead of 4+

Usage Examples:
  # Unicode mode (default)
  ./codeagent-wrapper --parallel < tasks.txt

  # ASCII mode (for terminals without Unicode support)
  CODEAGENT_ASCII_MODE=true ./codeagent-wrapper --parallel < tasks.txt

Benefits:
- Improved terminal compatibility across different environments
- Reduced memory allocations in parallel execution
- Better performance for large-scale parallel tasks
- User choice between Unicode aesthetics and ASCII compatibility

Related: #94

Generated with SWE-Agent.ai

Co-Authored-By: SWE-Agent.ai <noreply@swe-agent.ai>
This commit is contained in:
cexll
2025-12-24 11:59:00 +08:00
parent af1c860f54
commit b71d74f01f
5 changed files with 133 additions and 42 deletions

View File

@@ -511,6 +511,14 @@ func shouldSkipTask(task TaskSpec, failed map[string]TaskResult) (bool, string)
return true, fmt.Sprintf("skipped due to failed dependencies: %s", strings.Join(blocked, ","))
}
// getStatusSymbols returns status symbols based on ASCII mode.
func getStatusSymbols() (success, warning, failed string) {
if os.Getenv("CODEAGENT_ASCII_MODE") == "true" {
return "PASS", "WARN", "FAIL"
}
return "✓", "⚠️", "✗"
}
func generateFinalOutput(results []TaskResult) string {
return generateFinalOutputWithMode(results, true) // default to summary mode
}
@@ -520,6 +528,7 @@ func generateFinalOutput(results []TaskResult) string {
// summaryOnly=false: full output with complete messages (legacy behavior)
func generateFinalOutputWithMode(results []TaskResult, summaryOnly bool) string {
var sb strings.Builder
successSymbol, warningSymbol, failedSymbol := getStatusSymbols()
reportCoverageTarget := defaultCoverageTarget
for _, res := range results {
@@ -577,7 +586,7 @@ func generateFinalOutputWithMode(results []TaskResult, summaryOnly bool) string
if isSuccess && !isBelowTarget {
// Passed task: one block with Did/Files/Tests
sb.WriteString(fmt.Sprintf("\n### %s ", taskID))
sb.WriteString(fmt.Sprintf("\n### %s %s", taskID, successSymbol))
if coverage != "" {
sb.WriteString(fmt.Sprintf(" %s", coverage))
}
@@ -598,7 +607,7 @@ func generateFinalOutputWithMode(results []TaskResult, summaryOnly bool) string
} else if isSuccess && isBelowTarget {
// Below target: add Gap info
sb.WriteString(fmt.Sprintf("\n### %s ⚠️ %s (below %.0f%%)\n", taskID, coverage, target))
sb.WriteString(fmt.Sprintf("\n### %s %s %s (below %.0f%%)\n", taskID, warningSymbol, coverage, target))
if keyOutput != "" {
sb.WriteString(fmt.Sprintf("Did: %s\n", keyOutput))
@@ -620,7 +629,7 @@ func generateFinalOutputWithMode(results []TaskResult, summaryOnly bool) string
} else {
// Failed task: show error detail
sb.WriteString(fmt.Sprintf("\n### %s FAILED\n", taskID))
sb.WriteString(fmt.Sprintf("\n### %s %s FAILED\n", taskID, failedSymbol))
sb.WriteString(fmt.Sprintf("Exit code: %d\n", res.ExitCode))
if errText := sanitizeOutput(res.Error); errText != "" {
sb.WriteString(fmt.Sprintf("Error: %s\n", errText))

View File

@@ -289,6 +289,45 @@ func TestExecutorHelperCoverage(t *testing.T) {
}
})
t.Run("generateFinalOutputASCIIMode", func(t *testing.T) {
t.Setenv("CODEAGENT_ASCII_MODE", "true")
results := []TaskResult{
{TaskID: "ok", ExitCode: 0, Coverage: "92%", CoverageNum: 92, CoverageTarget: 90, KeyOutput: "done"},
{TaskID: "warn", ExitCode: 0, Coverage: "80%", CoverageNum: 80, CoverageTarget: 90, KeyOutput: "did"},
{TaskID: "bad", ExitCode: 2, Error: "boom"},
}
out := generateFinalOutput(results)
for _, sym := range []string{"PASS", "WARN", "FAIL"} {
if !strings.Contains(out, sym) {
t.Fatalf("ASCII mode should include %q, got: %s", sym, out)
}
}
for _, sym := range []string{"✓", "⚠️", "✗"} {
if strings.Contains(out, sym) {
t.Fatalf("ASCII mode should not include %q, got: %s", sym, out)
}
}
})
t.Run("generateFinalOutputUnicodeMode", func(t *testing.T) {
t.Setenv("CODEAGENT_ASCII_MODE", "false")
results := []TaskResult{
{TaskID: "ok", ExitCode: 0, Coverage: "92%", CoverageNum: 92, CoverageTarget: 90, KeyOutput: "done"},
{TaskID: "warn", ExitCode: 0, Coverage: "80%", CoverageNum: 80, CoverageTarget: 90, KeyOutput: "did"},
{TaskID: "bad", ExitCode: 2, Error: "boom"},
}
out := generateFinalOutput(results)
for _, sym := range []string{"✓", "⚠️", "✗"} {
if !strings.Contains(out, sym) {
t.Fatalf("Unicode mode should include %q, got: %s", sym, out)
}
}
})
t.Run("executeConcurrentWrapper", func(t *testing.T) {
orig := runCodexTaskFn
defer func() { runCodexTaskFn = orig }()

View File

@@ -31,6 +31,8 @@ const (
stdoutDrainTimeout = 100 * time.Millisecond
)
var useASCIIMode = os.Getenv("CODEAGENT_ASCII_MODE") == "true"
// Test hooks for dependency injection
var (
stdinReader io.Reader = os.Stdin
@@ -257,18 +259,20 @@ func run() (exitCode int) {
continue
}
lines := strings.Split(results[i].Message, "\n")
// Coverage extraction
results[i].Coverage = extractCoverage(results[i].Message)
results[i].Coverage = extractCoverageFromLines(lines)
results[i].CoverageNum = extractCoverageNum(results[i].Coverage)
// Files changed
results[i].FilesChanged = extractFilesChanged(results[i].Message)
results[i].FilesChanged = extractFilesChangedFromLines(lines)
// Test results
results[i].TestsPassed, results[i].TestsFailed = extractTestResults(results[i].Message)
results[i].TestsPassed, results[i].TestsFailed = extractTestResultsFromLines(lines)
// Key output summary
results[i].KeyOutput = extractKeyOutput(results[i].Message, 150)
results[i].KeyOutput = extractKeyOutputFromLines(lines, 150)
}
// Default: summary mode (context-efficient)
@@ -487,7 +491,8 @@ Parallel mode examples:
%[1]s --parallel <<'EOF'
Environment Variables:
CODEX_TIMEOUT Timeout in milliseconds (default: 7200000)
CODEX_TIMEOUT Timeout in milliseconds (default: 7200000)
CODEAGENT_ASCII_MODE Use ASCII symbols instead of Unicode (PASS/WARN/FAIL)
Exit Codes:
0 Success

View File

@@ -87,16 +87,17 @@ func parseIntegrationOutput(t *testing.T, out string) integrationOutput {
}
inTaskResults = false
} else if inTaskResults && strings.HasPrefix(line, "### ") {
// New task: ### task-id ✓ 92% or ### task-id ✗ FAILED
// New task: ### task-id ✓ 92% or ### task-id PASS 92% (ASCII mode)
if currentTask != nil {
payload.Results = append(payload.Results, *currentTask)
}
currentTask = &TaskResult{}
taskLine := strings.TrimPrefix(line, "### ")
success, warning, failed := getStatusSymbols()
// Parse different formats
if strings.Contains(taskLine, " ") {
parts := strings.Split(taskLine, " ")
if strings.Contains(taskLine, " "+success) {
parts := strings.Split(taskLine, " "+success)
currentTask.TaskID = strings.TrimSpace(parts[0])
currentTask.ExitCode = 0
// Extract coverage if present
@@ -106,12 +107,12 @@ func parseIntegrationOutput(t *testing.T, out string) integrationOutput {
currentTask.Coverage = coveragePart
}
}
} else if strings.Contains(taskLine, " ⚠️") {
parts := strings.Split(taskLine, " ⚠️")
} else if strings.Contains(taskLine, " "+warning) {
parts := strings.Split(taskLine, " "+warning)
currentTask.TaskID = strings.TrimSpace(parts[0])
currentTask.ExitCode = 0
} else if strings.Contains(taskLine, " ") {
parts := strings.Split(taskLine, " ")
} else if strings.Contains(taskLine, " "+failed) {
parts := strings.Split(taskLine, " "+failed)
currentTask.TaskID = strings.TrimSpace(parts[0])
currentTask.ExitCode = 1
} else {

View File

@@ -297,24 +297,29 @@ func extractMessageSummary(message string, maxLen int) string {
return safeTruncate(clean, maxLen)
}
// extractCoverage extracts coverage percentage from task output
// Supports common formats: "Coverage: 92%", "92% coverage", "coverage 92%", "TOTAL 92%"
func extractCoverage(message string) string {
if message == "" {
// extractCoverageFromLines extracts coverage from pre-split lines.
func extractCoverageFromLines(lines []string) string {
if len(lines) == 0 {
return ""
}
trimmed := strings.TrimSpace(message)
if strings.HasSuffix(trimmed, "%") && !strings.Contains(trimmed, "\n") {
if num, err := strconv.ParseFloat(strings.TrimSuffix(trimmed, "%"), 64); err == nil && num >= 0 && num <= 100 {
return trimmed
end := len(lines)
for end > 0 && strings.TrimSpace(lines[end-1]) == "" {
end--
}
if end == 1 {
trimmed := strings.TrimSpace(lines[0])
if strings.HasSuffix(trimmed, "%") {
if num, err := strconv.ParseFloat(strings.TrimSuffix(trimmed, "%"), 64); err == nil && num >= 0 && num <= 100 {
return trimmed
}
}
}
coverageKeywords := []string{"file", "stmt", "branch", "line", "coverage", "total"}
lines := strings.Split(message, "\n")
for _, line := range lines {
for _, line := range lines[:end] {
lower := strings.ToLower(line)
hasKeyword := false
@@ -359,6 +364,16 @@ func extractCoverage(message string) string {
return ""
}
// extractCoverage extracts coverage percentage from task output
// Supports common formats: "Coverage: 92%", "92% coverage", "coverage 92%", "TOTAL 92%"
func extractCoverage(message string) string {
if message == "" {
return ""
}
return extractCoverageFromLines(strings.Split(message, "\n"))
}
// extractCoverageNum extracts coverage as a numeric value for comparison
func extractCoverageNum(coverage string) float64 {
if coverage == "" {
@@ -372,10 +387,9 @@ func extractCoverageNum(coverage string) float64 {
return 0
}
// extractFilesChanged extracts list of changed files from task output
// Looks for common patterns like "Modified: file.ts", "Created: file.ts", file paths in output
func extractFilesChanged(message string) []string {
if message == "" {
// extractFilesChangedFromLines extracts files from pre-split lines.
func extractFilesChangedFromLines(lines []string) []string {
if len(lines) == 0 {
return nil
}
@@ -383,7 +397,6 @@ func extractFilesChanged(message string) []string {
seen := make(map[string]bool)
exts := []string{".ts", ".tsx", ".js", ".jsx", ".go", ".py", ".rs", ".java", ".vue", ".css", ".scss", ".md", ".json", ".yaml", ".yml", ".toml"}
lines := strings.Split(message, "\n")
for _, line := range lines {
line = strings.TrimSpace(line)
@@ -429,21 +442,30 @@ func extractFilesChanged(message string) []string {
return files
}
// extractTestResults extracts test pass/fail counts from task output
func extractTestResults(message string) (passed, failed int) {
// extractFilesChanged extracts list of changed files from task output
// Looks for common patterns like "Modified: file.ts", "Created: file.ts", file paths in output
func extractFilesChanged(message string) []string {
if message == "" {
return 0, 0
return nil
}
lower := strings.ToLower(message)
return extractFilesChangedFromLines(strings.Split(message, "\n"))
}
// extractTestResultsFromLines extracts test results from pre-split lines.
func extractTestResultsFromLines(lines []string) (passed, failed int) {
if len(lines) == 0 {
return 0, 0
}
// Common patterns:
// pytest: "12 passed, 2 failed"
// jest: "Tests: 2 failed, 12 passed"
// go: "ok ... 12 tests"
lines := strings.Split(lower, "\n")
for _, line := range lines {
line = strings.ToLower(line)
// Look for test result lines
if !strings.Contains(line, "pass") && !strings.Contains(line, "fail") && !strings.Contains(line, "test") {
continue
@@ -485,6 +507,15 @@ func extractTestResults(message string) (passed, failed int) {
return passed, failed
}
// extractTestResults extracts test pass/fail counts from task output
func extractTestResults(message string) (passed, failed int) {
if message == "" {
return 0, 0
}
return extractTestResultsFromLines(strings.Split(message, "\n"))
}
// extractNumberBefore extracts a number that appears before the given index
func extractNumberBefore(s string, idx int) int {
if idx <= 0 {
@@ -517,15 +548,12 @@ func extractNumberBefore(s string, idx int) int {
return 0
}
// extractKeyOutput extracts a brief summary of what the task accomplished
// Looks for summary lines, first meaningful sentence, or truncates message
func extractKeyOutput(message string, maxLen int) string {
if message == "" || maxLen <= 0 {
// extractKeyOutputFromLines extracts key output from pre-split lines.
func extractKeyOutputFromLines(lines []string, maxLen int) string {
if len(lines) == 0 || maxLen <= 0 {
return ""
}
lines := strings.Split(message, "\n")
// Priority 1: Look for explicit summary lines
for _, line := range lines {
line = strings.TrimSpace(line)
@@ -560,10 +588,19 @@ func extractKeyOutput(message string, maxLen int) string {
}
// Fallback: truncate entire message
clean := strings.TrimSpace(message)
clean := strings.TrimSpace(strings.Join(lines, "\n"))
return safeTruncate(clean, maxLen)
}
// extractKeyOutput extracts a brief summary of what the task accomplished
// Looks for summary lines, first meaningful sentence, or truncates message
func extractKeyOutput(message string, maxLen int) string {
if message == "" || maxLen <= 0 {
return ""
}
return extractKeyOutputFromLines(strings.Split(message, "\n"), maxLen)
}
// extractCoverageGap extracts what's missing from coverage reports
// Looks for uncovered lines, branches, or functions
func extractCoverageGap(message string) string {