test: add fast unit coverage for the composed --update-baseline summary text

[Tgenz1213]

Summary

Adds unit coverage for the exact composed "Baseline scan complete: ..." and "Baseline written to ..." strings by extracting them into two small, pure functions (formatBaselineScanSummary, formatBaselineWrittenSummary) and testing those directly with table-driven cases.

Related issue

Closes #121

In scope

• internal/cli/cli.go: extract the two --update-baseline summary Printf calls into named functions (output is byte-identical; pure refactor)
• internal/cli/cli_test.go: TestFormatBaselineScanSummary (table-driven: zero counts, nonzero ADR-check-skip count, all-distinct-nonzero counts) and TestFormatBaselineWrittenSummary

Out of scope

• Any behavioral change to the printed output itself

Architectural notes

None — no new invariant; the extraction is a pure refactor with identical output, verified by keeping the exact same format strings.

Follow-ups

None open. Note on process: this PR went through two design iterations before landing here, both driven by code review.

First iteration (original commit) called runCheck directly through a full temp-git-repo + mocked-provider integration setup, per the issue's suggested cli_test.go-level approach. Review of that version surfaced:

1. It paid llm.AnalyzeDrift's real ~14s exponential backoff on every test run, since runCheck hardcodes context.Background() with no injection point.
2. It substantially duplicated a scenario already covered instantly (0.00s) by internal/analysis/analysis_test.go's TestRun_UpdateBaselineMode_ReportsSkippedADRCheckCount — the only genuinely new value was pinning the CLI's exact composed string, which didn't require re-exercising the whole engine/backoff pipeline.
3. Several comments exceeded CLAUDE.md's 2-line cap.
4. The second summary line ("Baseline written to ...") was never asserted.

Second iteration (current) replaces that integration test with the extraction described above, which resolves all four points at once: 0.00s instead of ~14s, no duplicated coverage, no oversized comments, and both summary lines now covered directly.

Checklist

• Title follows Conventional Commits (feat:, fix:, docs:, refactor:, build(deps):, etc.)
• All acceptance criteria from the linked issue are met
• go test -race -cover ./... passes
• golangci-lint run --timeout=5m is clean
• CLAUDE.md updated if this changes build/test commands, adds or renames a top-level package, changes a cross-package interface, or adds a footgun (N/A)
• A new ADR added under docs/arch/ if this embodies an architecturally-significant decision (N/A)
• Comments follow the 2-line-max, WHY-only convention

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

No unresolved review issues remain.

Pull request overview

Adds CLI-level regression coverage for the --update-baseline summary, including skipped ADR checks.

Changes:

• Creates temporary Git repositories and ADR fixtures.
• Uses mocked embedding and chat providers.
• Asserts the exact composed summary text.

File summaries

File	Description
internal/cli/cli_test.go	Adds direct runCheck coverage for baseline summary output.

Review details

Suppressed comments (4)

internal/cli/cli_test.go:529

• This three-line comment exceeds the repository's two-line maximum for comments. The setup rationale can be stated in two lines without restating the implementation flow.

	// Pre-build the index the way `archguard index` would -- runCheck's
	// store.Load is a no-op (not a rebuild trigger) when indexFile doesn't
	// exist yet.

internal/cli/cli_test.go:483

• This three-line comment exceeds the repository's two-line maximum for comments. Keep the rationale concise so the test remains readable without narrating the setup.

	// One ADR whose chat call succeeds, one whose chat call fails -- the
	// failure must be counted in SkippedADRChecks without blocking the
	// other ADR's violation from being recorded.

internal/cli/cli_test.go:545

• This four-line comment exceeds the repository's two-line maximum and includes detailed timing/implementation narration. Keep only the short rationale for why the test invokes the retry path.

	// runCheck hardcodes context.Background() internally (no injection
	// point), and the issue forbids behavioral changes to add one, so this
	// pays llm.AnalyzeDrift's real ~14s backoff (2s+4s+8s, 3 retries) for
	// the one bad ADR's Chat failure -- a one-time cost, not a per-ADR one.

internal/cli/cli_test.go:513

• This failure path makes every go test/CI run sleep for the full 2s+4s+8s retry backoff before SkippedADRChecks can be asserted, adding a fixed ~14s to the suite. Since this test only needs a terminal analysis failure and must not change production behavior, return backoff.Permanent(errors.New("simulated LLM failure")) from the mock (as an intentionally non-retryable error) so AnalyzeDrift fails immediately; the existing engine-level test already cancels context to avoid this delay.

				return "", errors.New("simulated LLM failure")

• Files reviewed: 1/1 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Tgenz1213]

What is the point of a helper function for one print statement?

[Tgenz1213]

Fair question. Without extracting it, the only way to test the exact composed string is to run the whole runCheck → Engine.Run → llm.AnalyzeDrift pipeline for real, which is what the first version of this PR did — and that's exactly what paid the 14s backoff tax and duplicated coverage internal/analysis already has. This isn't meant as a general-purpose abstraction, just a minimal seam so the Printf's exact wording is callable in isolation. Happy to revert to the integration-style test (eating the 14s) or drop this test angle entirely if you'd rather not have the extra functions — your call.

[Tgenz1213]

This is kind of silly.