Migrate compact logs run tables from tabwriter to console.RenderTable

docs/adr/50850-migrate-tabwriter-to-console-rendertable.md

@@ -0,0 +1,43 @@
+# ADR-50850: Migrate CLI Table Rendering from tabwriter to console.RenderTable
+
+**Date**: 2026-08-06
+**Status**: Draft
+**Deciders**: pelikhan (PR author), copilot-swe-agent (implementation)
+
+---
+
+### Context
+
+`pkg/cli/logs_format_compact.go` rendered its two `[runs]` tables using `text/tabwriter`, writing tab-delimited lines directly to the output writer. Every other table in the CLI used the centralized `console.RenderTable` / `console.TableConfig` API, which provides lipgloss-based borders, automatic TTY detection, and consistent plain-text degradation on non-TTY writers (pipes, CI logs). The divergence meant the compact logs formatter had different visual output, bypassed the project's shared styling machinery, and required manual flush-error handling (`tw.Flush()`) not present elsewhere.
+
+### Decision
+
+We will migrate both tabwriter-backed tables in `renderLogsCompactToWriter` and `renderLogsCompactVerboseToWriter` to `console.RenderTable(console.TableConfig{...})`. All column headers, ordering, fallback values (`-` for empty fields), filtering of `skipped`/`cancelled` runs, and numeric formatting are preserved; only the alignment mechanism changes from tab-padding to lipgloss borders.
+
+### Alternatives Considered
+
+#### Alternative 1: Keep text/tabwriter
+
+The existing tabwriter approach is simple and has no runtime dependencies beyond the standard library. However, it produces plain tab-aligned output that does not adapt to TTY vs. non-TTY contexts the same way `console.RenderTable` does, creates visible inconsistency with every other CLI table, and requires manual flush-error handling. Accepting this inconsistency indefinitely is a maintenance burden and makes the codebase harder to reason about.
+
+#### Alternative 2: Write a Custom Formatter Matching console.RenderTable Behavior
+
+A custom formatter could in theory produce identical output without the `console` package dependency. However, this would duplicate the TTY-detection and border-rendering logic already centralized in `console.RenderTable`, creating two sources of truth for the same behavior and increasing the risk of divergence over time. No material benefit justifies the extra code.
+
+### Consequences
+
+#### Positive
+- Compact logs tables now use the same bordered rendering style as all other CLI tables, eliminating a visible inconsistency in the tool's output.
+- TTY-detection and plain-text degradation are handled automatically by `console.RenderTable`; no manual flush-or-error pattern is needed.
+- The `text/tabwriter` import and both `tw.Flush()` error-logging branches are removed, reducing code surface area.
+
+#### Negative
+- Output format changes from whitespace-aligned columns to lipgloss box-drawing characters (`╭─┬─╮`, etc.), which is a breaking change for any downstream consumer parsing the raw text of these tables.
+- Box-drawing border glyphs increase per-row character count, raising token density when this output is consumed by LLMs or agents that read compact logs. The PR body explicitly calls this out as a deliberate tradeoff worth monitoring.
+
+#### Neutral
+- A new test file `logs_format_compact_test.go` is added to cover bordered rendering, non-TTY ANSI-escape degradation, and `skipped`/`cancelled` run exclusion — behavior that was previously untested.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/cli/data/agentic_workflows_fallback_aw_files.json

@@ -15,6 +15,7 @@
   "debug-agentic-workflow.md",
   "dependabot.md",
   "deployment-status.md",
+  "designer-mappings.md",
   "designer.md",
   "evals.md",
   "experiments.md",

pkg/cli/logs_format_compact.go

@@ -6,8 +6,8 @@ import (
 	"os"
 	"strconv"
 	"strings"
-	"text/tabwriter"
 
+	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/stringutil"
 )
@@ -108,10 +108,9 @@ func renderLogsCompactToWriter(w io.Writer, data LogsData) {
 		return
 	}
 
-	// [runs] aligned table using tabwriter
+	// [runs] aligned table
 	fmt.Fprintln(w, "[runs]")
-	tw := tabwriter.NewWriter(w, 0, 0, 2, ' ', 0)
-	fmt.Fprintln(tw, "RUNID\tWORKFLOW\tENGINE\tSTATUS\tDUR\tTOKENS\tAIC\tTURNS\tERR\tEVENT\tACTOR\tBRANCH")
+	rows := make([][]string, 0, len(data.Runs))
 
 	for _, r := range data.Runs {
 		status := r.Conclusion
@@ -132,14 +131,18 @@ func renderLogsCompactToWriter(w io.Writer, data LogsData) {
 		}
 		wfID := workflowIDFromRun(r.WorkflowPath, r.WorkflowName)
 
-		fmt.Fprintf(tw, "%d\t%s\t%s\t%s\t%s\t%d\t%s\t%d\t%d\t%s\t%s\t%s\n",
-			r.RunID, wfID, r.EngineID, status, dur,
-			r.TokenUsage, formatCompactAIC(r.AIC), r.Turns, r.ErrorCount,
-			r.Event, actor, branch)
-	}
-	if err := tw.Flush(); err != nil {
-		logsCompactLog.Printf("flush error: %v", err)
+		rows = append(rows, []string{
+			strconv.FormatInt(r.RunID, 10), wfID, r.EngineID, status, dur,
+			strconv.Itoa(r.TokenUsage), formatCompactAIC(r.AIC),
+			strconv.Itoa(r.Turns), strconv.Itoa(r.ErrorCount),
+			r.Event, actor, branch,
+		})
 	}
+	// RenderTable appends a trailing newline, so following section headers remain separated.
+	fmt.Fprint(w, console.RenderTable(console.TableConfig{
+		Headers: []string{"RUNID", "WORKFLOW", "ENGINE", "STATUS", "DUR", "TOKENS", "AIC", "TURNS", "ERR", "EVENT", "ACTOR", "BRANCH"},
+		Rows:    rows,
+	}))
 
 	// [errors] — aggregated error/warning messages
 	if len(data.ErrorsAndWarnings) > 0 {
@@ -283,8 +286,7 @@ func renderLogsCompactVerboseToWriter(w io.Writer, data LogsData) {
 
 	// [runs] verbose aligned table
 	fmt.Fprintln(w, "[runs]")
-	tw := tabwriter.NewWriter(w, 0, 0, 2, ' ', 0)
-	fmt.Fprintln(tw, "RUNID\tWORKFLOW\tENGINE\tSTATUS\tDUR\tTOKENS\tAIC\tTURNS\tERR\tWARN\tEVENT\tACTOR\tTBT\tCLASS\tCREATED\tBRANCH")
+	rows := make([][]string, 0, len(data.Runs))
 
 	for _, r := range data.Runs {
 		status := r.Conclusion
@@ -312,16 +314,19 @@ func renderLogsCompactVerboseToWriter(w io.Writer, data LogsData) {
 		}
 		wfID := workflowIDFromRun(r.WorkflowPath, r.WorkflowName)
 
-		fmt.Fprintf(tw, "%d\t%s\t%s\t%s\t%s\t%d\t%s\t%d\t%d\t%d\t%s\t%s\t%s\t%s\t%s\t%s\n",
-			r.RunID, wfID, r.EngineID, status, dur,
-			r.TokenUsage, formatCompactAIC(r.AIC),
-			r.Turns, r.ErrorCount, r.WarningCount,
+		rows = append(rows, []string{
+			strconv.FormatInt(r.RunID, 10), wfID, r.EngineID, status, dur,
+			strconv.Itoa(r.TokenUsage), formatCompactAIC(r.AIC),
+			strconv.Itoa(r.Turns), strconv.Itoa(r.ErrorCount), strconv.Itoa(r.WarningCount),
 			r.Event, actor, tbt, classification,
-			r.CreatedAt.Format("01-02 15:04"), r.Branch)
-	}
-	if err := tw.Flush(); err != nil {
-		logsCompactLog.Printf("flush error: %v", err)
-	}
+			r.CreatedAt.Format("01-02 15:04"), r.Branch,
+		})
+	}
+	// RenderTable appends a trailing newline, so following section headers remain separated.
+	fmt.Fprint(w, console.RenderTable(console.TableConfig{
+		Headers: []string{"RUNID", "WORKFLOW", "ENGINE", "STATUS", "DUR", "TOKENS", "AIC", "TURNS", "ERR", "WARN", "EVENT", "ACTOR", "TBT", "CLASS", "CREATED", "BRANCH"},
+		Rows:    rows,
+	}))
 
 	// [errors]
 	if len(data.ErrorsAndWarnings) > 0 {

pkg/cli/logs_format_compact_test.go

@@ -0,0 +1,81 @@
+//go:build !integration
+
+package cli
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func compactTestData() LogsData {
+	return LogsData{
+		Summary: LogsSummary{TotalRuns: 1},
+		Runs: []RunData{{
+			RunID:        1234567,
+			WorkflowName: "Logs",
+			WorkflowPath: ".github/workflows/logs.lock.yml",
+			EngineID:     "copilot",
+			Status:       "completed",
+			Conclusion:   "success",
+			Duration:     "1m2s",
+			TokenUsage:   1200,
+			Turns:        4,
+			Event:        "push",
+			Actor:        "octocat",
+			Branch:       "main",
+			CreatedAt:    time.Now(),
+		}},
+	}
+}
+
+func TestRenderLogsCompactRendersRunsTableWithBorders(t *testing.T) {
+	t.Parallel()
+	var buf bytes.Buffer
+	renderLogsCompactToWriter(&buf, compactTestData())
+	out := buf.String()
+
+	assert.Contains(t, out, "[runs]")
+	assert.Contains(t, out, "╭")
+	assert.Contains(t, out, "RUNID")
+	assert.Contains(t, out, "1234567")
+	assert.Contains(t, out, "logs")
+	assert.NotContains(t, out, "\x1b[", "non-TTY output should degrade to plain text")
+}
+
+func TestRenderLogsCompactVerboseRendersRunsTableWithBorders(t *testing.T) {
+	t.Parallel()
+	var buf bytes.Buffer
+	renderLogsCompactVerboseToWriter(&buf, compactTestData())
+	out := buf.String()
+
+	assert.Contains(t, out, "[runs]")
+	assert.Contains(t, out, "╭")
+	assert.Contains(t, out, "CLASS")
+	assert.Contains(t, out, "1234567")
+	assert.NotContains(t, out, "\x1b[", "non-TTY output should degrade to plain text")
+}
+
+func TestRenderLogsCompactSkipsSkippedAndCancelledRuns(t *testing.T) {
+	t.Parallel()
+	data := compactTestData()
+	data.Runs = append(data.Runs,
+		RunData{RunID: 222, WorkflowName: "skipped-wf", Status: "skipped", CreatedAt: time.Now()},
+		RunData{RunID: 333, WorkflowName: "cancelled-wf", Status: "cancelled", CreatedAt: time.Now()},
+		RunData{RunID: 444, WorkflowName: "skipped-c", Conclusion: "skipped", CreatedAt: time.Now()},
+		RunData{RunID: 555, WorkflowName: "cancelled-c", Conclusion: "cancelled", CreatedAt: time.Now()},
+	)
+
+	var buf bytes.Buffer
+	renderLogsCompactToWriter(&buf, data)
+	out := buf.String()
+
+	assert.NotContains(t, out, "222")
+	assert.NotContains(t, out, "333")
+	assert.NotContains(t, out, "444")
+	assert.NotContains(t, out, "555")
+	assert.Equal(t, 1, strings.Count(out, "1234567"))
+}