Replace logger raw ANSI namespace colors with Lipgloss adaptive styles

[Copilot]

✨ Enhancement

What does this improve?
The debug logger in pkg/logger/logger.go no longer hardcodes ANSI 256-color escape sequences; namespace coloring now uses Lipgloss styles aligned with the shared pkg/styles color system. This keeps hash-based namespace color assignment while moving color rendering to adaptive styling.

• Logger color model migration

  • Replaced raw \033[38;5;*m palette entries and reset code with a Lipgloss style palette.
  • Updated namespace color selection to return a rendered, styled namespace string instead of raw escape fragments.
  • Preserved deterministic namespace→color mapping via existing FNV-1a hash logic.

• Terminal/color behavior

  • Removed manual tty.IsStderrTerminal() gating from logger color selection.
  • Switched logger output writes to lipgloss.Fprintf(os.Stderr, ...) so color output follows Lipgloss/colorprofile behavior.
  • Kept DEBUG_COLORS=0 as an explicit opt-out path (plain namespace output).

• Coverage updates

  • Updated pkg/logger/logger_test.go assertions to reflect styled namespace rendering.
  • Added explicit verification that logger output contains no ANSI escapes when NO_COLOR=1.

// Before: manual ANSI composition
fmt.Fprintf(os.Stderr, "%s%s%s %s +%s\n", l.color, l.namespace, colorReset, message, ...)

// After: pre-rendered namespace + Lipgloss writer
lipgloss.Fprintf(os.Stderr, "%s %s +%s\n", l.color, message, ...)

[Copilot]

Pull request overview

Replaces the logger's hand-rolled ANSI 256-color palette and TTY gating with a Lipgloss-based adaptive style palette sourced from pkg/styles, and writes log lines via lipgloss.Fprintf so the colorprofile machinery governs whether colors are emitted.

Changes:

• Swap the raw \033[38;5;*m palette and colorReset for a []lipgloss.Style palette using styles.Color* adaptive colors.
• Drop the tty.IsStderrTerminal() check; rely on Lipgloss/colorprofile (including NO_COLOR) for color suppression, keeping DEBUG_COLORS=0 as an explicit opt-out.
• Update selectColor to return either the plain namespace or a pre-rendered styled namespace, and update tests (removing TestColorDisabling, adding TestNoColorEnvironment).

Show a summary per file

File	Description
pkg/logger/logger.go	Replaces raw ANSI palette with Lipgloss adaptive styles, removes TTY gating, and switches output to lipgloss.Fprintf.
pkg/logger/logger_test.go	Adapts color tests to the new rendered-namespace contract and adds a NO_COLOR=1 ANSI-absence check.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 2/2 changed files
• Comments generated: 3

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 97/100

✅ Excellent test quality

Metric	Value
New/modified tests analyzed	2
✅ Design tests (behavioral contracts)	2 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	2 (100%)
Duplicate test clusters	0
Test inflation detected	No
🚨 Coding-guideline violations	None

---

Test Classification Details

Test	File	Classification	Notes
TestColorSelection (modified)	pkg/logger/logger_test.go:283	✅ Design	Verifies observable output of selectColor: non-empty result, namespace text inclusion, ANSI presence, and plain fallback when disabled
TestNoColorEnvironment (renamed from TestColorDisabling)	pkg/logger/logger_test.go:323	✅ Design	End-to-end: creates real logger, captures stderr, asserts no ANSI codes when NO_COLOR=1

---

Quality Highlights

TestColorSelection was improved from an implementation-leaning test (checking against an internal colorPalette slice) to a behavioral contract: the new assertions verify what consumers observe — that the returned label embeds the namespace text and contains ANSI styling when colors are on, and returns the plain namespace string when disabled. The t.Cleanup ensures state is restored without relying on defer ordering.

TestNoColorEnvironment upgraded from white-box (isTTY field manipulation) to black-box: it constructs a real Logger, logs through it, and inspects captured stderr for the absence of ANSI escape sequences. This correctly models the user-observable contract: "when NO_COLOR=1, the logger emits no colour codes."

---

Language Support

Tests analyzed:

• 🐹 Go (*_test.go): 2 tests — unit (//go:build !integration)

---

Verdict

✅ Check passed. 0% of new/modified tests are implementation tests (threshold: 30%). Both changed tests verify behavioral contracts with error/edge-case paths covered.

---

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

• Assert on observable outputs, return values, or state changes
• Cover error paths and boundary conditions
• Would catch a behavioral regression if deleted
• Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

• Assert on internal function calls (mocking internals)
• Only test the happy path with typical inputs
• Break during legitimate refactoring even when behavior is correct
• Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · ● 6.2M · ◷

[github-actions]

✅ Test Quality Sentinel: 97/100. Test quality is excellent — 0% of new/modified tests are implementation tests (threshold: 30%). Both changes improve tests from white-box internal checks to black-box behavioral contracts.

[github-actions]

Skills-Based Review 🧠

Applied /zoom-out and /tdd — this is a clean refactor with accompanying test updates, both skills apply directly.

Key Themes

• Naming drift (zoom-out): the color field and selectColor function no longer describe what they hold/return after the migration. Both now carry the fully-rendered namespace label, not a raw color code. Small rename = big clarity gain.
• One stray raw color (zoom-out): lipgloss.Color("33") in the palette sits alongside 11 semantic styles.Color* entries — partially defeats the purpose of the migration.
• Test fragility (tdd): the \x1b[ assertion in TestColorSelection tests the internal encoding, not the behaviour; Lipgloss strips ANSI in no-TTY environments (CI), so this will fail there. The TestNoColorEnvironment pattern already shows the right approach.
• Test isolation bug (tdd): debugEnv is mutated in TestNoColorEnvironment without a corresponding restore — only debugColors is cleaned up.

Positive Highlights

• ✅ Removing tty.IsStderrTerminal() gating in favour of Lipgloss's own adaptive output is the right architectural move — centralises TTY detection.
• ✅ TestNoColorEnvironment is a well-structured end-to-end behavioural test (Arrange → Act → Assert through the public Printf API).
• ✅ The PR description is clear and the before/after code sample helps reviewers quickly understand the change.

Verdict

The core approach is solid. The naming issues and the \x1b[ test are the most important things to address before merge.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 6.9M

[pelikhan]

@copilot review comments

[Copilot]

@copilot review comments

Addressed in commit 1b3f80d.

Updated the logger palette to only use readable foreground-oriented adaptive colors (removed ColorBackground, ColorTableAltRow, and the raw lipgloss.Color("33") entry), and renamed the logger field/function naming to reflect that it stores a rendered namespace label (color -> label, selectColor -> selectNamespaceLabel). Tests were updated accordingly and validations pass.