[repository-quality] Repository Quality Improvement Report — Output Stream Abstraction Gap (2026-07-22)

[github-actions[bot]]

🎯 Repository Quality Improvement Report — Output Stream Abstraction Gap

Analysis Date: 2026-07-22
Focus Area: Output Stream Abstraction Gap — Hardcoded os.Stdout in Render Functions
Strategy Type: Custom
Custom Area: Yes — this repo has a growing family of render* formatter functions in pkg/cli/ that bypass all output injection and write directly to os.Stdout, making unit testing awkward (requiring OS-pipe surgery via captureOutput) and making output redirection impossible for callers.

Executive Summary

The pkg/cli/ package contains 167 hardcoded os.Stdout references across production files, with the highest concentration in the log-rendering family (logs_format_tsv.go: 12 refs, logs_format_compact.go: 39 refs, audit_cross_run_render.go: 22 refs). Six core render* functions — renderLogsTSV, renderLogsTSVVerbose, renderLogsCompact, renderLogsCompactVerbose, renderLogsJSON, and renderLogsConsole — accept only a LogsData struct and write directly to os.Stdout, with no io.Writer parameter.

A captureOutput test helper in checks_command_test.go works around this by swapping os.Stdout at the OS level, annotated with a comment warning against t.Parallel(). This anti-pattern propagates to all five test files that use it. Meanwhile, audit_cross_run_render.go demonstrates the correct pattern — it defines renderCrossRunReportMarkdownToWriter(w io.Writer, ...) alongside a thin renderCrossRunReportMarkdown wrapper — but this pattern has not been applied to the TSV, compact, JSON, or console formatters.

The fix is straightforward: add an io.Writer parameter to each affected function, update callers to pass cmd.OutOrStdout() or os.Stdout, and update tests to use bytes.Buffer instead of OS-pipe surgery. This eliminates the captureOutput mutex, enables t.Parallel() in formatter tests, and aligns with the existing audit_cross_run_render.go convention.

Full Analysis Report

Focus Area: Output Stream Abstraction Gap

Current State Assessment

The pkg/cli/ log rendering subsystem hardcodes os.Stdout in all six primary render functions. Only audit_cross_run_render.go has adopted the ...ToWriter(w io.Writer, ...) convention — and only for its Markdown sub-functions; its JSON and Pretty top-level functions still hardcode os.Stdout.

Metrics Collected:

Metric	Value	Status
os.Stdout refs in pkg/cli/ (non-test)	167	❌
render* functions with hardcoded os.Stdout	27	❌
Functions already accepting io.Writer in pkg/cli/	11	⚠️
Test files relying on captureOutput OS-swap	5	⚠️
renderLogs* functions with io.Writer parameter	0 / 6	❌
assert.NoError calls (should be require.NoError)	251	⚠️

Findings

Strengths

• audit_cross_run_render.go already exports ...ToWriter variants demonstrating the correct pattern.
• audit_cross_run_render.go's renderCrossRunReportMarkdown is a thin wrapper — the split is already there.
• The captureOutput test helper centralises the OS-swap workaround rather than duplicating it.

Areas for Improvement

• High: renderLogsTSV / renderLogsTSVVerbose (39 stdout writes) — highest impact for testability.
• High: renderLogsCompact / renderLogsCompactVerbose (39 stdout writes across 39 lines) — same pattern.
• Medium: renderLogsJSON / renderLogsConsole and renderCrossRunReportJSON / renderCrossRunReportPretty still hardcode.
• Medium: captureOutput serialises via a package-level mutex and blocks t.Parallel().
• Low: 251 assert.NoError calls (not require.NoError) mean that nil-pointer dereferences after a missed error can produce confusing test output instead of a clean failure.

Detailed Analysis

Core problematic signatures (all in pkg/cli/):

func renderLogsTSV(data LogsData)                          // logs_format_tsv.go:31
func renderLogsTSVVerbose(data LogsData)                   // logs_format_tsv.go:115
func renderLogsCompact(data LogsData)                      // logs_format_compact.go:56
func renderLogsCompactVerbose(data LogsData)               // logs_format_compact.go:225
func renderLogsJSON(data LogsData, verbose bool) error     // logs_report.go:526
func renderLogsConsole(data LogsData)                      // logs_report.go:615

Callsite in logs_orchestrator_render.go:

renderLogsTSVVerbose(logsData)   // line 56
renderLogsTSV(logsData)          // line 58
renderLogsCompactVerbose(logsData) // line 140
renderLogsCompact(logsData)      // line 142

The captureOutput helper (file-level mutex, OS-pipe swap) is used in 5 test files and carries this comment:

// Do not call this helper from tests using t.Parallel; it manipulates
// process-global stdout/stderr and must be serialized.

The correct pattern already exists in the same package:

// audit_cross_run_render.go — the right way
func renderCrossRunReportMarkdown(report *CrossRunAuditReport) {
    renderCrossRunReportMarkdownToWriter(os.Stdout, report)
}
func renderCrossRunReportMarkdownToWriter(w io.Writer, report *CrossRunAuditReport) {
    // ... writes to w
}

---

🤖 Tasks for Copilot Agent

NOTE TO PLANNER AGENT: Split the following tasks into individual work items.

Improvement Tasks

Task 1: Add io.Writer parameter to renderLogsTSV and renderLogsTSVVerbose

Priority: High
Estimated Effort: Small
Focus Area: Output Stream Abstraction

Description: Refactor renderLogsTSV(data LogsData) and renderLogsTSVVerbose(data LogsData) in pkg/cli/logs_format_tsv.go to accept an io.Writer as their first parameter. Replace all fmt.Fprintf(os.Stdout, ...) and fmt.Fprintln(os.Stdout, ...) calls inside those functions with equivalent calls using the passed writer. Update the two call sites in pkg/cli/logs_orchestrator_render.go to pass the appropriate writer (from renderLogsOutputOptions or os.Stdout as fallback). Update pkg/cli/logs_format_tsv_test.go to pass bytes.Buffer instead of relying on captureOutput.

Acceptance Criteria:

• renderLogsTSV(w io.Writer, data LogsData) and renderLogsTSVVerbose(w io.Writer, data LogsData) are the new signatures
• Zero os.Stdout references remain in logs_format_tsv.go
• Tests in logs_format_tsv_test.go use &bytes.Buffer{} and no longer call captureOutput
• make fmt passes with no diff

Code Region: pkg/cli/logs_format_tsv.go, pkg/cli/logs_orchestrator_render.go, pkg/cli/logs_format_tsv_test.go

Refactor `renderLogsTSV` and `renderLogsTSVVerbose` in `pkg/cli/logs_format_tsv.go` to accept an `io.Writer` as their first argument instead of hardcoding `os.Stdout`. Replace all `fmt.Fprintf(os.Stdout, ...)` / `fmt.Fprintln(os.Stdout, ...)` calls with equivalent calls on the passed writer. Update callers in `pkg/cli/logs_orchestrator_render.go` to pass an appropriate writer. Update existing tests in `pkg/cli/logs_format_tsv_test.go` to pass `&bytes.Buffer{}` directly instead of using the `captureOutput` OS-pipe workaround. Run `make fmt` and `make test-unit` before finishing.

---

Task 2: Add io.Writer parameter to renderLogsCompact and renderLogsCompactVerbose

Priority: High
Estimated Effort: Small
Focus Area: Output Stream Abstraction

Description: Refactor renderLogsCompact(data LogsData) and renderLogsCompactVerbose(data LogsData) in pkg/cli/logs_format_compact.go to accept an io.Writer as their first parameter. This file has 39 hardcoded os.Stdout references — the highest count of any single file. Replace all with the passed writer. Pay special attention to tabwriter.NewWriter(os.Stdout, ...) on line 108 which must become tabwriter.NewWriter(w, ...). Update the callers in pkg/cli/logs_orchestrator_render.go.

Acceptance Criteria:

• renderLogsCompact(w io.Writer, data LogsData) and renderLogsCompactVerbose(w io.Writer, data LogsData) are the new signatures
• Zero os.Stdout references remain in logs_format_compact.go
• tabwriter.NewWriter uses the passed w, not os.Stdout
• Callers in logs_orchestrator_render.go updated
• make fmt passes with no diff

Code Region: pkg/cli/logs_format_compact.go, pkg/cli/logs_orchestrator_render.go

Refactor `renderLogsCompact` and `renderLogsCompactVerbose` in `pkg/cli/logs_format_compact.go` to accept an `io.Writer` as their first argument. Replace all 39 `fmt.Fprintf(os.Stdout, ...)` / `fmt.Fprintln(os.Stdout, ...)` / `tabwriter.NewWriter(os.Stdout, ...)` calls to use the passed writer. Update callers in `pkg/cli/logs_orchestrator_render.go`. Run `make fmt` and `make test-unit` before finishing.

---

Task 3: Add io.Writer parameter to renderLogsJSON, renderLogsConsole, renderCrossRunReportJSON, and renderCrossRunReportPretty

Priority: Medium
Estimated Effort: Small
Focus Area: Output Stream Abstraction

Description: Four more top-level render functions still hardcode os.Stdout:

• renderLogsJSON(data LogsData, verbose bool) error in pkg/cli/logs_report.go:526 — uses json.NewEncoder(os.Stdout)
• renderLogsConsole(data LogsData) in pkg/cli/logs_report.go:615
• renderCrossRunReportJSON(report *CrossRunAuditReport) error in pkg/cli/audit_cross_run_render.go:20
• renderCrossRunReportPretty(report *CrossRunAuditReport) in pkg/cli/audit_cross_run_render.go:210

Add w io.Writer as the first parameter to each. For renderLogsJSON, change json.NewEncoder(os.Stdout) to json.NewEncoder(w). For renderCrossRunReportJSON, same pattern. Update all callers.

Acceptance Criteria:

• All four functions accept io.Writer as first param
• No os.Stdout references remain in those four functions
• All callers updated (pass os.Stdout or a real writer)
• make fmt passes with no diff

Code Region: pkg/cli/logs_report.go, pkg/cli/audit_cross_run_render.go

Refactor `renderLogsJSON`, `renderLogsConsole` in `pkg/cli/logs_report.go` and `renderCrossRunReportJSON`, `renderCrossRunReportPretty` in `pkg/cli/audit_cross_run_render.go` to accept `io.Writer` as their first parameter. Replace all hardcoded `os.Stdout` writes inside those functions with the passed writer. Update all callers. Run `make fmt` and `make test-unit` before finishing.

---

Task 4: Expose renderLogsOutputOptions.output io.Writer and thread it through renderLogsOutput

Priority: Medium
Estimated Effort: Small
Focus Area: Output Stream Abstraction

Description: renderLogsOutput in pkg/cli/logs_orchestrator_render.go is the single dispatch function that calls all renderLogs* formatters. After Tasks 1–3, all format functions will accept io.Writer. Add an output io.Writer field to renderLogsOutputOptions (defaulting to os.Stdout when nil at call sites). Pass opts.output (or os.Stdout if nil) through to each renderLogs* call. This enables future callers (e.g., tests) to inject a bytes.Buffer without using captureOutput.

Acceptance Criteria:

• renderLogsOutputOptions has an output io.Writer field
• renderLogsOutput uses opts.output (nil-defaults to os.Stdout) for all stdout output
• Existing callers of renderLogsOutput compile without changes (nil field falls back to os.Stdout)
• At least one test constructs renderLogsOutputOptions{output: &bytes.Buffer{}} and verifies output without captureOutput
• make fmt and make test-unit pass

Code Region: pkg/cli/logs_orchestrator_render.go

Add an `output io.Writer` field to `renderLogsOutputOptions` in `pkg/cli/logs_orchestrator_render.go`. Inside `renderLogsOutput`, resolve the writer as `w := opts.output; if w == nil { w = os.Stdout }` and pass `w` to every `renderLogs*` call (which will accept `io.Writer` after Tasks 1–3). Existing callers that leave `output` as the zero value will continue to use `os.Stdout`. Add at least one test that sets `output: &bytes.Buffer{}` to verify output without relying on `captureOutput`. Run `make fmt` and `make test-unit` before finishing.

---

Task 5: Replace assert.NoError with require.NoError where result is immediately used

Priority: Low
Estimated Effort: Medium
Focus Area: Test Reliability

Description: The codebase has 251 assert.NoError calls vs 5,735 require.NoError calls. The assert variant does not stop test execution on failure, so when the subsequent line dereferences a nil pointer returned by the failing call, the test panics with a confusing stack trace instead of a clear "unexpected error" message. The highest-count files are pkg/cli/audit_diff_test.go (289 total assert.* calls), pkg/workflow/checkout_manager_test.go (284), and pkg/workflow/compiler_orchestrator_workflow_test.go (272). Scan each test file for the pattern assert.NoError(t, err) immediately followed by code that uses the return value of the same call, and replace those specific assert.NoError calls with require.NoError.

Acceptance Criteria:

• All assert.NoError calls in pkg/cli/audit_diff_test.go where the result is used on the next line are replaced with require.NoError
• All assert.NoError calls in pkg/workflow/checkout_manager_test.go that gate subsequent value usage are replaced
• assert.NoError calls where the error is truly optional (no immediate result usage) are left as-is
• make test-unit passes with no new failures

Code Region: pkg/cli/audit_diff_test.go, pkg/workflow/checkout_manager_test.go, pkg/workflow/compiler_orchestrator_workflow_test.go

Scan `pkg/cli/audit_diff_test.go`, `pkg/workflow/checkout_manager_test.go`, and `pkg/workflow/compiler_orchestrator_workflow_test.go` for `assert.NoError(t, err)` calls that are immediately followed by code using the value returned by the same statement (e.g., `result, err := foo(); assert.NoError(t, err); result.Bar()`). Replace those specific `assert.NoError` calls with `require.NoError` so the test stops immediately on failure rather than panicking with a nil dereference. Leave standalone `assert.NoError` calls where no subsequent code depends on the success. Run `make test-unit` before finishing.

---

📊 Historical Context

Previous Focus Areas

Date	Focus Area	Type	Custom	Key Outcomes
2026-07-17	Error Chain Transparency & Non-Wrapping fmt.Errorf Gap	Custom	Y	788 non-wrapping fmt.Errorf; 5 tasks
2026-07-16	Options Struct Pattern Adoption for High-Arity Functions	Custom	Y	2 over-9-param functions; 5 tasks
2026-07-15	Compile-Time Interface Satisfaction Assertions	Custom	Y	Zero var _ I = (*T)(nil) guards; 4 tasks
2026-07-14	Logger Name Collision & Instantiation Hygiene	Custom	Y	3 duplicate logger names; 4 tasks
2026-07-08	Deprecated API Alias Proliferation & Zombie Callsite Cleanup	Custom	Y	3 GetGeminiAPITarget callsites; 4 tasks

---

🎯 Recommendations

Immediate Actions (This Week)

1. Add io.Writer to renderLogsTSV / renderLogsTSVVerbose — Priority: High (Task 1)
2. Add io.Writer to renderLogsCompact / renderLogsCompactVerbose — Priority: High (Task 2)

Short-term Actions (This Month)

1. Add io.Writer to renderLogsJSON, renderLogsConsole, cross-run report functions — Priority: Medium (Task 3)
2. Thread output io.Writer through renderLogsOutputOptions — Priority: Medium (Task 4)

Long-term Actions (This Quarter)

1. Replace assert.NoError with require.NoError where results are used — Priority: Low (Task 5)
2. Consider a custom outputstream linter that flags os.Stdout direct writes outside main.go and cmd/

---

📈 Success Metrics

• os.Stdout refs in pkg/cli/ (non-test): 167 → < 20 (cmd wiring only)
• render* functions with hardcoded os.Stdout: 27 → 0
• Test files using captureOutput mutex: 5 → 0
• renderLogs* functions with io.Writer param: 0/6 → 6/6

---

Next Steps

1. Review and prioritise the tasks above
2. Assign tasks 1–4 to Copilot coding agent via planner agent
3. Track progress on improvement items
4. Re-evaluate output abstraction completeness in 4 weeks

---

References:

• §29923186852

Generated by Repository Quality Improvement Agent

Generated by ⚡ Repository Quality Improvement Agent · sonnet46 108 AIC · ⌖ 6.51 AIC · ⊞ 6.7K · ◷

• expires on Jul 23, 2026, 5:26 AM UTC-08:00

[github-actions[bot]]

Smoke ogre tap drum here.
Latest discussion found.
Comment path blocked by workflow permission.

📰 BREAKING: Report filed by Smoke Copilot · gpt54 87.5 AIC · ⌖ 8.14 AIC · ⊞ 19.4K · ◷
_{Comment /smoke-copilot to run again
Add label smoke to run again}

[github-actions[bot]]

This discussion was automatically closed because it expired on 2026-07-23T13:26:59.545Z.

Closed by Workflow