refactor as per manual code reviews

11 files changed, 644 insertions, 596 deletions

diff --git a/cmd/hexai-lsp/main.go b/cmd/hexai-lsp/main.go
index a473ad7..5ff79fa 100644
--- a/cmd/hexai-lsp/main.go
+++ b/cmd/hexai-lsp/main.go
@@ -1,5 +1,4 @@
 // Summary: Hexai LSP entrypoint; parses flags and delegates to internal/hexailsp.
-// Not yet reviewed by a human
 package main
 
 import (
diff --git a/internal/appconfig/config.go b/internal/appconfig/config.go
index 7076862..c5166a0 100644
--- a/internal/appconfig/config.go
+++ b/internal/appconfig/config.go
@@ -13,14 +13,14 @@ import (
 
 // App holds user-configurable settings read from ~/.config/hexai/config.json.
 type App struct {
-	MaxTokens          int      `json:"max_tokens"`
-	ContextMode        string   `json:"context_mode"`
-	ContextWindowLines int      `json:"context_window_lines"`
-	MaxContextTokens   int      `json:"max_context_tokens"`
-	LogPreviewLimit    int      `json:"log_preview_limit"`
-	
-	TriggerCharacters  []string `json:"trigger_characters"`
-	Provider           string   `json:"provider"`
+	MaxTokens          int    `json:"max_tokens"`
+	ContextMode        string `json:"context_mode"`
+	ContextWindowLines int    `json:"context_window_lines"`
+	MaxContextTokens   int    `json:"max_context_tokens"`
+	LogPreviewLimit    int    `json:"log_preview_limit"`
+
+	TriggerCharacters []string `json:"trigger_characters"`
+	Provider          string   `json:"provider"`
 
 	// Provider-specific options
 	OpenAIBaseURL  string `json:"openai_base_url"`
diff --git a/internal/hexaicli/run_test.go b/internal/hexaicli/run_test.go
index f9c8443..377b224 100644
--- a/internal/hexaicli/run_test.go
+++ b/internal/hexaicli/run_test.go
@@ -1,5 +1,4 @@
 // Summary: Unit tests for Hexai CLI helpers and run flow (input parsing, messages, streaming).
-// Not yet reviewed by a human
 package hexaicli
 
 import (
@@ -9,8 +8,6 @@ import (
 	"testing"
 )
 
-// helpers moved to testhelpers_test.go
-
 func TestReadInput_ArgsOnly(t *testing.T) {
 	restore, f := setStdin(t, "")
 	defer restore()
diff --git a/internal/hexaicli/testhelpers_test.go b/internal/hexaicli/testhelpers_test.go
index 4a25ff1..bd3c3dd 100644
--- a/internal/hexaicli/testhelpers_test.go
+++ b/internal/hexaicli/testhelpers_test.go
@@ -1,63 +1,62 @@
 // Summary: Test helpers for Hexai CLI tests (stdin swapping and fake LLM clients/streamers).
-// Not yet reviewed by a human
 package hexaicli
 
 import (
-    "context"
-    "os"
-    "path/filepath"
-    "testing"
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
 
-    "hexai/internal/llm"
+	"hexai/internal/llm"
 )
 
 // setStdin sets os.Stdin from a string and returns a restore func and reader.
 func setStdin(t *testing.T, content string) (func(), *os.File) {
-    t.Helper()
-    tmpDir := t.TempDir()
-    fpath := filepath.Join(tmpDir, "stdin.txt")
-    if err := os.WriteFile(fpath, []byte(content), 0o600); err != nil {
-        t.Fatalf("write temp stdin: %v", err)
-    }
-    f, err := os.Open(fpath)
-    if err != nil {
-        t.Fatalf("open temp stdin: %v", err)
-    }
-    old := os.Stdin
-    os.Stdin = f
-    restore := func() {
-        f.Close()
-        os.Stdin = old
-    }
-    return restore, f
+	t.Helper()
+	tmpDir := t.TempDir()
+	fpath := filepath.Join(tmpDir, "stdin.txt")
+	if err := os.WriteFile(fpath, []byte(content), 0o600); err != nil {
+		t.Fatalf("write temp stdin: %v", err)
+	}
+	f, err := os.Open(fpath)
+	if err != nil {
+		t.Fatalf("open temp stdin: %v", err)
+	}
+	old := os.Stdin
+	os.Stdin = f
+	restore := func() {
+		f.Close()
+		os.Stdin = old
+	}
+	return restore, f
 }
 
 // fakeClient implements llm.Client for tests.
 type fakeClient struct {
-    name    string
-    model   string
-    resp    string
-    gotMsgs []llm.Message
+	name    string
+	model   string
+	resp    string
+	gotMsgs []llm.Message
 }
 
 func (f *fakeClient) Chat(ctx context.Context, messages []llm.Message, opts ...llm.RequestOption) (string, error) {
-    f.gotMsgs = append([]llm.Message{}, messages...)
-    return f.resp, nil
+	f.gotMsgs = append([]llm.Message{}, messages...)
+	return f.resp, nil
 }
-func (f fakeClient) Name() string        { return f.name }
+func (f fakeClient) Name() string         { return f.name }
 func (f fakeClient) DefaultModel() string { return f.model }
 
 // fakeStreamer implements llm.Streamer over fakeClient.
 type fakeStreamer struct {
-    fakeClient
-    chunks []string
-    sMsgs  []llm.Message
+	fakeClient
+	chunks []string
+	sMsgs  []llm.Message
 }
 
 func (s *fakeStreamer) ChatStream(ctx context.Context, messages []llm.Message, onDelta func(string), opts ...llm.RequestOption) error {
-    s.sMsgs = append([]llm.Message{}, messages...)
-    for _, c := range s.chunks {
-        onDelta(c)
-    }
-    return nil
+	s.sMsgs = append([]llm.Message{}, messages...)
+	for _, c := range s.chunks {
+		onDelta(c)
+	}
+	return nil
 }
diff --git a/internal/hexailsp/run.go b/internal/hexailsp/run.go
index 5a0ab4a..8a79b12 100644
--- a/internal/hexailsp/run.go
+++ b/internal/hexailsp/run.go
@@ -1,18 +1,17 @@
 // Summary: Hexai LSP runner; configures logging, loads config, builds the LLM client,
 // and constructs/runs the LSP server (with injectable factory for tests).
-// Not yet reviewed by a human
 package hexailsp
 
 import (
-    "log"
-    "os"
-    "strings"
-    "io"
+	"io"
+	"log"
+	"os"
+	"strings"
 
-    "hexai/internal/appconfig"
-    "hexai/internal/llm"
-    "hexai/internal/logging"
-    "hexai/internal/lsp"
+	"hexai/internal/appconfig"
+	"hexai/internal/llm"
+	"hexai/internal/logging"
+	"hexai/internal/lsp"
 )
 
 // ServerRunner is the minimal interface satisfied by lsp.Server.
@@ -24,68 +23,68 @@ type ServerFactory func(r io.Reader, w io.Writer, logger *log.Logger, opts lsp.S
 // Run configures logging, loads config, builds the LLM client and runs the LSP server.
 // It is thin and delegates to RunWithFactory for testability.
 func Run(logPath string, stdin io.Reader, stdout io.Writer, stderr io.Writer) error {
-    logger := log.New(stderr, "hexai-lsp ", log.LstdFlags|log.Lmsgprefix)
-    if strings.TrimSpace(logPath) != "" {
-        f, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)
-        if err != nil {
-            logger.Fatalf("failed to open log file: %v", err)
-        }
-        defer f.Close()
-        logger.SetOutput(f)
-    }
-    logging.Bind(logger)
-    cfg := appconfig.Load(logger)
-    return RunWithFactory(logPath, stdin, stdout, logger, cfg, nil, nil)
+	logger := log.New(stderr, "hexai-lsp ", log.LstdFlags|log.Lmsgprefix)
+	if strings.TrimSpace(logPath) != "" {
+		f, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)
+		if err != nil {
+			logger.Fatalf("failed to open log file: %v", err)
+		}
+		defer f.Close()
+		logger.SetOutput(f)
+	}
+	logging.Bind(logger)
+	cfg := appconfig.Load(logger)
+	return RunWithFactory(logPath, stdin, stdout, logger, cfg, nil, nil)
 }
 
 // RunWithFactory is the testable entrypoint. When client is nil, it is built from cfg+env.
 // When factory is nil, lsp.NewServer is used.
 func RunWithFactory(logPath string, stdin io.Reader, stdout io.Writer, logger *log.Logger, cfg appconfig.App, client llm.Client, factory ServerFactory) error {
-    // Normalize and apply logging config
-    cfg.ContextMode = strings.ToLower(strings.TrimSpace(cfg.ContextMode))
-    if cfg.LogPreviewLimit >= 0 {
-        logging.SetLogPreviewLimit(cfg.LogPreviewLimit)
-    }
+	// Normalize and apply logging config
+	cfg.ContextMode = strings.ToLower(strings.TrimSpace(cfg.ContextMode))
+	if cfg.LogPreviewLimit >= 0 {
+		logging.SetLogPreviewLimit(cfg.LogPreviewLimit)
+	}
 
-    // Build LLM client if not provided
-    if client == nil {
-        llmCfg := llm.Config{
-            Provider:       cfg.Provider,
-            OpenAIBaseURL:  cfg.OpenAIBaseURL,
-            OpenAIModel:    cfg.OpenAIModel,
-            OllamaBaseURL:  cfg.OllamaBaseURL,
-            OllamaModel:    cfg.OllamaModel,
-            CopilotBaseURL: cfg.CopilotBaseURL,
-            CopilotModel:   cfg.CopilotModel,
-        }
-        oaKey := os.Getenv("OPENAI_API_KEY")
-        cpKey := os.Getenv("COPILOT_API_KEY")
-        if c, err := llm.NewFromConfig(llmCfg, oaKey, cpKey); err != nil {
-            logging.Logf("lsp ", "llm disabled: %v", err)
-        } else {
-            client = c
-            logging.Logf("lsp ", "llm enabled provider=%s model=%s", c.Name(), c.DefaultModel())
-        }
-    }
+	// Build LLM client if not provided
+	if client == nil {
+		llmCfg := llm.Config{
+			Provider:       cfg.Provider,
+			OpenAIBaseURL:  cfg.OpenAIBaseURL,
+			OpenAIModel:    cfg.OpenAIModel,
+			OllamaBaseURL:  cfg.OllamaBaseURL,
+			OllamaModel:    cfg.OllamaModel,
+			CopilotBaseURL: cfg.CopilotBaseURL,
+			CopilotModel:   cfg.CopilotModel,
+		}
+		oaKey := os.Getenv("OPENAI_API_KEY")
+		cpKey := os.Getenv("COPILOT_API_KEY")
+		if c, err := llm.NewFromConfig(llmCfg, oaKey, cpKey); err != nil {
+			logging.Logf("lsp ", "llm disabled: %v", err)
+		} else {
+			client = c
+			logging.Logf("lsp ", "llm enabled provider=%s model=%s", c.Name(), c.DefaultModel())
+		}
+	}
 
-    if factory == nil {
-        factory = func(r io.Reader, w io.Writer, logger *log.Logger, opts lsp.ServerOptions) ServerRunner {
-            return lsp.NewServer(r, w, logger, opts)
-        }
-    }
+	if factory == nil {
+		factory = func(r io.Reader, w io.Writer, logger *log.Logger, opts lsp.ServerOptions) ServerRunner {
+			return lsp.NewServer(r, w, logger, opts)
+		}
+	}
 
-    server := factory(stdin, stdout, logger, lsp.ServerOptions{
-        LogContext:        strings.TrimSpace(logPath) != "",
-        MaxTokens:         cfg.MaxTokens,
-        ContextMode:       cfg.ContextMode,
-        WindowLines:       cfg.ContextWindowLines,
-        MaxContextTokens:  cfg.MaxContextTokens,
-        
-        Client:            client,
-        TriggerCharacters: cfg.TriggerCharacters,
-    })
-    if err := server.Run(); err != nil {
-        logger.Fatalf("server error: %v", err)
-    }
-    return nil
+	server := factory(stdin, stdout, logger, lsp.ServerOptions{
+		LogContext:       strings.TrimSpace(logPath) != "",
+		MaxTokens:        cfg.MaxTokens,
+		ContextMode:      cfg.ContextMode,
+		WindowLines:      cfg.ContextWindowLines,
+		MaxContextTokens: cfg.MaxContextTokens,
+
+		Client:            client,
+		TriggerCharacters: cfg.TriggerCharacters,
+	})
+	if err := server.Run(); err != nil {
+		logger.Fatalf("server error: %v", err)
+	}
+	return nil
 }
diff --git a/internal/hexailsp/run_test.go b/internal/hexailsp/run_test.go
index 7af9cb8..df810e2 100644
--- a/internal/hexailsp/run_test.go
+++ b/internal/hexailsp/run_test.go
@@ -1,145 +1,145 @@
 // Summary: Tests for the Hexai LSP runner using a fake server factory and environment keys.
-// Not yet reviewed by a human
 package hexailsp
 
 import (
-    "bytes"
-    "log"
-    "io"
-    "os"
-    "path/filepath"
-    "testing"
+	"bytes"
+	"io"
+	"log"
+	"os"
+	"path/filepath"
+	"testing"
 
-    "hexai/internal/appconfig"
-    "hexai/internal/llm"
-    "hexai/internal/lsp"
-    "hexai/internal/logging"
+	"hexai/internal/appconfig"
+	"hexai/internal/llm"
+	"hexai/internal/logging"
+	"hexai/internal/lsp"
 )
 
 // fake server capturing options and recording run calls
-type fakeServer struct{
-    ran bool
-    opts lsp.ServerOptions
+type fakeServer struct {
+	ran  bool
+	opts lsp.ServerOptions
 }
+
 func (f *fakeServer) Run() error { f.ran = true; return nil }
 
 func TestRunWithFactory_UsesDefaultsAndCallsServer(t *testing.T) {
-    old := os.Getenv("OPENAI_API_KEY")
-    t.Cleanup(func(){ _ = os.Setenv("OPENAI_API_KEY", old) })
-    _ = os.Setenv("OPENAI_API_KEY", "")
+	old := os.Getenv("OPENAI_API_KEY")
+	t.Cleanup(func() { _ = os.Setenv("OPENAI_API_KEY", old) })
+	_ = os.Setenv("OPENAI_API_KEY", "")
+
+	var stderr bytes.Buffer
+	logger := log.New(&stderr, "hexai-lsp ", 0)
+	cfg := appconfig.Load(nil) // defaults
+	var gotOpts lsp.ServerOptions
+	factory := func(r io.Reader, w io.Writer, logger *log.Logger, opts lsp.ServerOptions) ServerRunner {
+		gotOpts = opts
+		return &fakeServer{opts: opts}
+	}
+	if err := RunWithFactory("", bytes.NewBuffer(nil), bytes.NewBuffer(nil), logger, cfg, nil, factory); err != nil {
+		t.Fatalf("RunWithFactory error: %v", err)
+	}
+	if gotOpts.MaxTokens != cfg.MaxTokens {
+		t.Fatalf("MaxTokens want %d got %d", cfg.MaxTokens, gotOpts.MaxTokens)
+	}
+	if gotOpts.ContextMode != cfg.ContextMode {
+		t.Fatalf("ContextMode want %q got %q", cfg.ContextMode, gotOpts.ContextMode)
+	}
+	if gotOpts.WindowLines != cfg.ContextWindowLines {
+		t.Fatalf("WindowLines want %d got %d", cfg.ContextWindowLines, gotOpts.WindowLines)
+	}
+	if gotOpts.MaxContextTokens != cfg.MaxContextTokens {
+		t.Fatalf("MaxContextTokens want %d got %d", cfg.MaxContextTokens, gotOpts.MaxContextTokens)
+	}
 
-    var stderr bytes.Buffer
-    logger := log.New(&stderr, "hexai-lsp ", 0)
-    cfg := appconfig.Load(nil) // defaults
-    var gotOpts lsp.ServerOptions
-    factory := func(r io.Reader, w io.Writer, logger *log.Logger, opts lsp.ServerOptions) ServerRunner {
-        gotOpts = opts
-        return &fakeServer{opts: opts}
-    }
-    if err := RunWithFactory("", bytes.NewBuffer(nil), bytes.NewBuffer(nil), logger, cfg, nil, factory); err != nil {
-        t.Fatalf("RunWithFactory error: %v", err)
-    }
-    if gotOpts.MaxTokens != cfg.MaxTokens {
-        t.Fatalf("MaxTokens want %d got %d", cfg.MaxTokens, gotOpts.MaxTokens)
-    }
-    if gotOpts.ContextMode != cfg.ContextMode {
-        t.Fatalf("ContextMode want %q got %q", cfg.ContextMode, gotOpts.ContextMode)
-    }
-    if gotOpts.WindowLines != cfg.ContextWindowLines {
-        t.Fatalf("WindowLines want %d got %d", cfg.ContextWindowLines, gotOpts.WindowLines)
-    }
-    if gotOpts.MaxContextTokens != cfg.MaxContextTokens {
-        t.Fatalf("MaxContextTokens want %d got %d", cfg.MaxContextTokens, gotOpts.MaxContextTokens)
-    }
-    
-    if gotOpts.Client != nil { // with no env, openai client fails to build
-        t.Fatalf("expected nil client when API key missing")
-    }
+	if gotOpts.Client != nil { // with no env, openai client fails to build
+		t.Fatalf("expected nil client when API key missing")
+	}
 }
 
 func TestRunWithFactory_BuildsClientWhenKeysPresent(t *testing.T) {
-    // Set a dummy OpenAI key to allow client creation
-    old := os.Getenv("OPENAI_API_KEY")
-    t.Cleanup(func(){ _ = os.Setenv("OPENAI_API_KEY", old) })
-    _ = os.Setenv("OPENAI_API_KEY", "dummy")
+	// Set a dummy OpenAI key to allow client creation
+	old := os.Getenv("OPENAI_API_KEY")
+	t.Cleanup(func() { _ = os.Setenv("OPENAI_API_KEY", old) })
+	_ = os.Setenv("OPENAI_API_KEY", "dummy")
 
-    var stderr bytes.Buffer
-    logger := log.New(&stderr, "hexai-lsp ", 0)
-    cfg := appconfig.Load(nil) // defaults, provider=openai by default
-    var got llm.Client
-    factory := func(r io.Reader, w io.Writer, logger *log.Logger, opts lsp.ServerOptions) ServerRunner {
-        got = opts.Client
-        return &fakeServer{opts: opts}
-    }
-    if err := RunWithFactory("", bytes.NewBuffer(nil), bytes.NewBuffer(nil), logger, cfg, nil, factory); err != nil {
-        t.Fatalf("RunWithFactory error: %v", err)
-    }
-    if got == nil {
-        t.Fatalf("expected non-nil client when OPENAI_API_KEY is set")
-    }
+	var stderr bytes.Buffer
+	logger := log.New(&stderr, "hexai-lsp ", 0)
+	cfg := appconfig.Load(nil) // defaults, provider=openai by default
+	var got llm.Client
+	factory := func(r io.Reader, w io.Writer, logger *log.Logger, opts lsp.ServerOptions) ServerRunner {
+		got = opts.Client
+		return &fakeServer{opts: opts}
+	}
+	if err := RunWithFactory("", bytes.NewBuffer(nil), bytes.NewBuffer(nil), logger, cfg, nil, factory); err != nil {
+		t.Fatalf("RunWithFactory error: %v", err)
+	}
+	if got == nil {
+		t.Fatalf("expected non-nil client when OPENAI_API_KEY is set")
+	}
 }
 
 func TestRun_RespectsLogPathFlag(t *testing.T) {
-    tmp := t.TempDir()
-    logFile := filepath.Join(tmp, "hexai-lsp.log")
-    // Run with real Run but nil env key so client disabled; ensure no panic and file created
-    if err := Run(logFile, bytes.NewBuffer(nil), bytes.NewBuffer(nil), bytes.NewBuffer(nil)); err != nil {
-        t.Fatalf("Run error: %v", err)
-    }
-    if _, err := os.Stat(logFile); err != nil {
-        t.Fatalf("expected log file to be created: %v", err)
-    }
+	tmp := t.TempDir()
+	logFile := filepath.Join(tmp, "hexai-lsp.log")
+	// Run with real Run but nil env key so client disabled; ensure no panic and file created
+	if err := Run(logFile, bytes.NewBuffer(nil), bytes.NewBuffer(nil), bytes.NewBuffer(nil)); err != nil {
+		t.Fatalf("Run error: %v", err)
+	}
+	if _, err := os.Stat(logFile); err != nil {
+		t.Fatalf("expected log file to be created: %v", err)
+	}
 }
 
 func TestRunWithFactory_NormalizesContextMode_AndSetsPreviewLimit(t *testing.T) {
-    t.Cleanup(func(){ logging.SetLogPreviewLimit(0) })
-    var stderr bytes.Buffer
-    logger := log.New(&stderr, "hexai-lsp ", 0)
-    cfg := appconfig.App{
-        ContextMode:     "  File-On-New-Func  ",
-        LogPreviewLimit: 3,
-    }
-    var gotOpts lsp.ServerOptions
-    factory := func(r io.Reader, w io.Writer, logger *log.Logger, opts lsp.ServerOptions) ServerRunner {
-        gotOpts = opts
-        return &fakeServer{opts: opts}
-    }
-    if err := RunWithFactory("", bytes.NewBuffer(nil), bytes.NewBuffer(nil), logger, cfg, nil, factory); err != nil {
-        t.Fatalf("RunWithFactory error: %v", err)
-    }
-    if gotOpts.ContextMode != "file-on-new-func" {
-        t.Fatalf("ContextMode not normalized: %q", gotOpts.ContextMode)
-    }
-    if logging.PreviewForLog("abcdef") != "abc…" {
-        t.Fatalf("PreviewForLog not respecting limit: %q", logging.PreviewForLog("abcdef"))
-    }
+	t.Cleanup(func() { logging.SetLogPreviewLimit(0) })
+	var stderr bytes.Buffer
+	logger := log.New(&stderr, "hexai-lsp ", 0)
+	cfg := appconfig.App{
+		ContextMode:     "  File-On-New-Func  ",
+		LogPreviewLimit: 3,
+	}
+	var gotOpts lsp.ServerOptions
+	factory := func(r io.Reader, w io.Writer, logger *log.Logger, opts lsp.ServerOptions) ServerRunner {
+		gotOpts = opts
+		return &fakeServer{opts: opts}
+	}
+	if err := RunWithFactory("", bytes.NewBuffer(nil), bytes.NewBuffer(nil), logger, cfg, nil, factory); err != nil {
+		t.Fatalf("RunWithFactory error: %v", err)
+	}
+	if gotOpts.ContextMode != "file-on-new-func" {
+		t.Fatalf("ContextMode not normalized: %q", gotOpts.ContextMode)
+	}
+	if logging.PreviewForLog("abcdef") != "abc…" {
+		t.Fatalf("PreviewForLog not respecting limit: %q", logging.PreviewForLog("abcdef"))
+	}
 }
 
 func TestRunWithFactory_LogContextFlag(t *testing.T) {
-    var stderr bytes.Buffer
-    logger := log.New(&stderr, "hexai-lsp ", 0)
-    cfg := appconfig.App{}
-    var got1, got2 lsp.ServerOptions
-    first := true
-    factory := func(r io.Reader, w io.Writer, logger *log.Logger, opts lsp.ServerOptions) ServerRunner {
-        if first {
-            got1 = opts
-            first = false
-        } else {
-            got2 = opts
-        }
-        return &fakeServer{opts: opts}
-    }
-    if err := RunWithFactory("/tmp/some.log", bytes.NewBuffer(nil), bytes.NewBuffer(nil), logger, cfg, nil, factory); err != nil {
-        t.Fatalf("RunWithFactory error: %v", err)
-    }
-    if !got1.LogContext {
-        t.Fatalf("expected LogContext true when logPath is non-empty")
-    }
-    if err := RunWithFactory("", bytes.NewBuffer(nil), bytes.NewBuffer(nil), logger, cfg, nil, factory); err != nil {
-        t.Fatalf("RunWithFactory error: %v", err)
-    }
-    if got2.LogContext {
-        t.Fatalf("expected LogContext false when logPath is empty")
-    }
+	var stderr bytes.Buffer
+	logger := log.New(&stderr, "hexai-lsp ", 0)
+	cfg := appconfig.App{}
+	var got1, got2 lsp.ServerOptions
+	first := true
+	factory := func(r io.Reader, w io.Writer, logger *log.Logger, opts lsp.ServerOptions) ServerRunner {
+		if first {
+			got1 = opts
+			first = false
+		} else {
+			got2 = opts
+		}
+		return &fakeServer{opts: opts}
+	}
+	if err := RunWithFactory("/tmp/some.log", bytes.NewBuffer(nil), bytes.NewBuffer(nil), logger, cfg, nil, factory); err != nil {
+		t.Fatalf("RunWithFactory error: %v", err)
+	}
+	if !got1.LogContext {
+		t.Fatalf("expected LogContext true when logPath is non-empty")
+	}
+	if err := RunWithFactory("", bytes.NewBuffer(nil), bytes.NewBuffer(nil), logger, cfg, nil, factory); err != nil {
+		t.Fatalf("RunWithFactory error: %v", err)
+	}
+	if got2.LogContext {
+		t.Fatalf("expected LogContext false when logPath is empty")
+	}
 }
diff --git a/internal/llm/copilot.go b/internal/llm/copilot.go
index 1e36bb7..7cc0278 100644
--- a/internal/llm/copilot.go
+++ b/internal/llm/copilot.go
@@ -3,153 +3,160 @@
 package llm
 
 import (
-    "bytes"
-    "context"
-    "encoding/json"
-    "errors"
-    "fmt"
-    "net/http"
-    "strings"
-    "time"
+	"bytes"
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"net/http"
+	"strings"
+	"time"
 
-    "hexai/internal/logging"
+	"hexai/internal/logging"
 )
 
 // copilotClient implements Client against GitHub Copilot's Chat Completions API.
 type copilotClient struct {
-    httpClient   *http.Client
-    apiKey       string
-    baseURL      string
-    defaultModel string
+	httpClient   *http.Client
+	apiKey       string
+	baseURL      string
+	defaultModel string
+	chatLogger   *logging.ChatLogger
 }
 
 func newCopilot(baseURL, model, apiKey string) Client {
-    if strings.TrimSpace(baseURL) == "" {
-        baseURL = "https://api.githubcopilot.com"
-    }
-    if strings.TrimSpace(model) == "" {
-        model = "gpt-4.1"
-    }
-    return &copilotClient{
-        httpClient:   &http.Client{Timeout: 30 * time.Second},
-        apiKey:       apiKey,
-        baseURL:      strings.TrimRight(baseURL, "/"),
-        defaultModel: model,
-    }
+	if strings.TrimSpace(baseURL) == "" {
+		baseURL = "https://api.githubcopilot.com"
+	}
+	if strings.TrimSpace(model) == "" {
+		model = "gpt-4.1"
+	}
+	return &copilotClient{
+		httpClient:   &http.Client{Timeout: 30 * time.Second},
+		apiKey:       apiKey,
+		baseURL:      strings.TrimRight(baseURL, "/"),
+		defaultModel: model,
+		chatLogger:   logging.NewChatLogger("copilot"),
+	}
 }
 
 type copilotChatRequest struct {
-    Model       string           `json:"model"`
-    Messages    []copilotMessage `json:"messages"`
-    Temperature *float64         `json:"temperature,omitempty"`
-    MaxTokens   *int             `json:"max_tokens,omitempty"`
-    Stop        []string         `json:"stop,omitempty"`
+	Model       string           `json:"model"`
+	Messages    []copilotMessage `json:"messages"`
+	Temperature *float64         `json:"temperature,omitempty"`
+	MaxTokens   *int             `json:"max_tokens,omitempty"`
+	Stop        []string         `json:"stop,omitempty"`
 }
 
 type copilotMessage struct {
-    Role    string `json:"role"`
-    Content string `json:"content"`
+	Role    string `json:"role"`
+	Content string `json:"content"`
 }
 
 type copilotChatResponse struct {
-    Choices []struct {
-        Index   int `json:"index"`
-        Message struct {
-            Role    string `json:"role"`
-            Content string `json:"content"`
-        } `json:"message"`
-        FinishReason string `json:"finish_reason"`
-    } `json:"choices"`
-    Error *struct {
-        Message string `json:"message"`
-        Type    string `json:"type"`
-        Param   any    `json:"param"`
-        Code    any    `json:"code"`
-    } `json:"error,omitempty"`
+	Choices []struct {
+		Index   int `json:"index"`
+		Message struct {
+			Role    string `json:"role"`
+			Content string `json:"content"`
+		} `json:"message"`
+		FinishReason string `json:"finish_reason"`
+	} `json:"choices"`
+	Error *struct {
+		Message string `json:"message"`
+		Type    string `json:"type"`
+		Param   any    `json:"param"`
+		Code    any    `json:"code"`
+	} `json:"error,omitempty"`
 }
 
 func (c *copilotClient) Chat(ctx context.Context, messages []Message, opts ...RequestOption) (string, error) {
-    if strings.TrimSpace(c.apiKey) == "" {
-        return nilStringErr("missing Copilot API key")
-    }
-    o := Options{Model: c.defaultModel}
-    for _, opt := range opts {
-        opt(&o)
-    }
-    if o.Model == "" {
-        o.Model = c.defaultModel
-    }
+	if strings.TrimSpace(c.apiKey) == "" {
+		return nilStringErr("missing Copilot API key")
+	}
+	o := Options{Model: c.defaultModel}
+	for _, opt := range opts {
+		opt(&o)
+	}
+	if o.Model == "" {
+		o.Model = c.defaultModel
+	}
 
-    start := time.Now()
-    logging.Logf("llm/copilot ", "chat start model=%s temp=%.2f max_tokens=%d stop=%d messages=%d", o.Model, o.Temperature, o.MaxTokens, len(o.Stop), len(messages))
-    for i, m := range messages {
-        logging.Logf("llm/copilot ", "msg[%d] role=%s size=%d preview=%s%s%s", i, m.Role, len(m.Content), logging.AnsiCyan, logging.PreviewForLog(m.Content), logging.AnsiBase)
-    }
+	start := time.Now()
+	logMessages := make([]struct {
+		Role    string
+		Content string
+	}, len(messages))
+	for i, m := range messages {
+		logMessages[i] = struct {
+			Role    string
+			Content string
+		}{Role: m.Role, Content: m.Content}
+	}
+	c.chatLogger.LogStart(false, o.Model, o.Temperature, o.MaxTokens, o.Stop, logMessages)
 
-    req := copilotChatRequest{Model: o.Model}
-    req.Messages = make([]copilotMessage, len(messages))
-    for i, m := range messages {
-        req.Messages[i] = copilotMessage{Role: m.Role, Content: m.Content}
-    }
-    if o.Temperature != 0 {
-        req.Temperature = &o.Temperature
-    }
-    if o.MaxTokens > 0 {
-        req.MaxTokens = &o.MaxTokens
-    }
-    if len(o.Stop) > 0 {
-        req.Stop = o.Stop
-    }
+	req := copilotChatRequest{Model: o.Model}
+	req.Messages = make([]copilotMessage, len(messages))
+	for i, m := range messages {
+		req.Messages[i] = copilotMessage{Role: m.Role, Content: m.Content}
+	}
+	if o.Temperature != 0 {
+		req.Temperature = &o.Temperature
+	}
+	if o.MaxTokens > 0 {
+		req.MaxTokens = &o.MaxTokens
+	}
+	if len(o.Stop) > 0 {
+		req.Stop = o.Stop
+	}
 
-    body, err := json.Marshal(req)
-    if err != nil {
-        logging.Logf("llm/copilot ", "marshal error: %v", err)
-        return "", err
-    }
+	body, err := json.Marshal(req)
+	if err != nil {
+		logging.Logf("llm/copilot ", "marshal error: %v", err)
+		return "", err
+	}
 
-    endpoint := c.baseURL + "/chat/completions"
-    logging.Logf("llm/copilot ", "POST %s", endpoint)
-    httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body))
-    if err != nil {
-        logging.Logf("llm/copilot ", "new request error: %v", err)
-        return "", err
-    }
-    httpReq.Header.Set("Content-Type", "application/json")
-    httpReq.Header.Set("Authorization", "Bearer "+c.apiKey)
-    // Some Copilot deployments expect a version header; optional here.
-    // httpReq.Header.Set("X-GitHub-Api-Version", "2023-12-07")
+	endpoint := c.baseURL + "/chat/completions"
+	logging.Logf("llm/copilot ", "POST %s", endpoint)
+	httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body))
+	if err != nil {
+		logging.Logf("llm/copilot ", "new request error: %v", err)
+		return "", err
+	}
+	httpReq.Header.Set("Content-Type", "application/json")
+	httpReq.Header.Set("Authorization", "Bearer "+c.apiKey)
 
-    resp, err := c.httpClient.Do(httpReq)
-    if err != nil {
-        logging.Logf("llm/copilot ", "%shttp error after %s: %v%s", logging.AnsiRed, time.Since(start), err, logging.AnsiBase)
-        return "", err
-    }
-    defer resp.Body.Close()
-    if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-        var apiErr copilotChatResponse
-        _ = json.NewDecoder(resp.Body).Decode(&apiErr)
-        if apiErr.Error != nil && strings.TrimSpace(apiErr.Error.Message) != "" {
-            logging.Logf("llm/copilot ", "%sapi error status=%d type=%s msg=%s duration=%s%s", logging.AnsiRed, resp.StatusCode, apiErr.Error.Type, apiErr.Error.Message, time.Since(start), logging.AnsiBase)
-            return "", fmt.Errorf("copilot error: %s (status %d)", apiErr.Error.Message, resp.StatusCode)
-        }
-        logging.Logf("llm/copilot ", "%shttp non-2xx status=%d duration=%s%s", logging.AnsiRed, resp.StatusCode, time.Since(start), logging.AnsiBase)
-        return "", fmt.Errorf("copilot http error: status %d", resp.StatusCode)
-    }
+	resp, err := c.httpClient.Do(httpReq)
+	if err != nil {
+		logging.Logf("llm/copilot ", "%shttp error after %s: %v%s", logging.AnsiRed, time.Since(start), err, logging.AnsiBase)
+		return "", err
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		var apiErr copilotChatResponse
+		_ = json.NewDecoder(resp.Body).Decode(&apiErr)
+		if apiErr.Error != nil && strings.TrimSpace(apiErr.Error.Message) != "" {
+			logging.Logf("llm/copilot ", "%sapi error status=%d type=%s msg=%s duration=%s%s", logging.AnsiRed, resp.StatusCode, apiErr.Error.Type, apiErr.Error.Message, time.Since(start), logging.AnsiBase)
+			return "", fmt.Errorf("copilot error: %s (status %d)", apiErr.Error.Message, resp.StatusCode)
+		}
+		logging.Logf("llm/copilot ", "%shttp non-2xx status=%d duration=%s%s", logging.AnsiRed, resp.StatusCode, time.Since(start), logging.AnsiBase)
+		return "", fmt.Errorf("copilot http error: status %d", resp.StatusCode)
+	}
 
-    var out copilotChatResponse
-    if err := json.NewDecoder(resp.Body).Decode(&out); err != nil {
-        logging.Logf("llm/copilot ", "%sdecode error after %s: %v%s", logging.AnsiRed, time.Since(start), err, logging.AnsiBase)
-        return "", err
-    }
-    if len(out.Choices) == 0 {
-        logging.Logf("llm/copilot ", "%sno choices returned duration=%s%s", logging.AnsiRed, time.Since(start), logging.AnsiBase)
-        return "", errors.New("copilot: no choices returned")
-    }
-    content := out.Choices[0].Message.Content
-    logging.Logf("llm/copilot ", "success choice=0 finish=%s size=%d preview=%s%s%s duration=%s", out.Choices[0].FinishReason, len(content), logging.AnsiGreen, logging.PreviewForLog(content), logging.AnsiBase, time.Since(start))
-    return content, nil
+	var out copilotChatResponse
+	if err := json.NewDecoder(resp.Body).Decode(&out); err != nil {
+		logging.Logf("llm/copilot ", "%sdecode error after %s: %v%s", logging.AnsiRed, time.Since(start), err, logging.AnsiBase)
+		return "", err
+	}
+	if len(out.Choices) == 0 {
+		logging.Logf("llm/copilot ", "%sno choices returned duration=%s%s", logging.AnsiRed, time.Since(start), logging.AnsiBase)
+		return "", errors.New("copilot: no choices returned")
+	}
+	content := out.Choices[0].Message.Content
+	logging.Logf("llm/copilot ", "success choice=0 finish=%s size=%d preview=%s%s%s duration=%s", out.Choices[0].FinishReason, len(content), logging.AnsiGreen, logging.PreviewForLog(content), logging.AnsiBase, time.Since(start))
+	return content, nil
 }
 
 // Provider metadata
 func (c *copilotClient) Name() string         { return "copilot" }
-func (c *copilotClient) DefaultModel() string { return c.defaultModel }
+func (c *copilotClient) DefaultModel() string { return c.defaultModel }
\ No newline at end of file
diff --git a/internal/llm/ollama.go b/internal/llm/ollama.go
index 774eaf1..49adcb2 100644
--- a/internal/llm/ollama.go
+++ b/internal/llm/ollama.go
@@ -3,15 +3,15 @@
 package llm
 
 import (
-    "bytes"
-    "context"
-    "encoding/json"
-    "errors"
-    "fmt"
-    "io"
-    "net/http"
-    "strings"
-    "time"
+	"bytes"
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"net/http"
+	"strings"
+	"time"
 
 	"hexai/internal/logging"
 )
@@ -21,6 +21,7 @@ type ollamaClient struct {
 	httpClient   *http.Client
 	baseURL      string
 	defaultModel string
+	chatLogger   *logging.ChatLogger
 }
 
 func newOllama(baseURL, model string) Client {
@@ -34,14 +35,15 @@ func newOllama(baseURL, model string) Client {
 		httpClient:   &http.Client{Timeout: 30 * time.Second},
 		baseURL:      strings.TrimRight(baseURL, "/"),
 		defaultModel: model,
+		chatLogger:   logging.NewChatLogger("ollama"),
 	}
 }
 
 type ollamaChatRequest struct {
-    Model    string      `json:"model"`
-    Messages []oaMessage `json:"messages"`
-    Stream   bool        `json:"stream"`
-    Options  any         `json:"options,omitempty"`
+	Model    string      `json:"model"`
+	Messages []oaMessage `json:"messages"`
+	Stream   bool        `json:"stream"`
+	Options  any         `json:"options,omitempty"`
 }
 
 type ollamaChatResponse struct {
@@ -63,10 +65,17 @@ func (c *ollamaClient) Chat(ctx context.Context, messages []Message, opts ...Req
 	}
 
 	start := time.Now()
-	logging.Logf("llm/ollama ", "chat start model=%s temp=%.2f max_tokens=%d stop=%d messages=%d", o.Model, o.Temperature, o.MaxTokens, len(o.Stop), len(messages))
+	logMessages := make([]struct {
+		Role    string
+		Content string
+	}, len(messages))
 	for i, m := range messages {
-		logging.Logf("llm/ollama ", "msg[%d] role=%s size=%d preview=%s%s%s", i, m.Role, len(m.Content), logging.AnsiCyan, logging.PreviewForLog(m.Content), logging.AnsiBase)
+		logMessages[i] = struct {
+			Role    string
+			Content string
+		}{Role: m.Role, Content: m.Content}
 	}
+	c.chatLogger.LogStart(false, o.Model, o.Temperature, o.MaxTokens, o.Stop, logMessages)
 
 	req := ollamaChatRequest{Model: o.Model, Stream: false}
 	req.Messages = make([]oaMessage, len(messages))
@@ -139,91 +148,98 @@ func (c *ollamaClient) DefaultModel() string { return c.defaultModel }
 
 // Streaming support (optional)
 func (c *ollamaClient) ChatStream(ctx context.Context, messages []Message, onDelta func(string), opts ...RequestOption) error {
-    o := Options{Model: c.defaultModel}
-    for _, opt := range opts {
-        opt(&o)
-    }
-    if o.Model == "" {
-        o.Model = c.defaultModel
-    }
-
-    start := time.Now()
-    logging.Logf("llm/ollama ", "stream start model=%s temp=%.2f max_tokens=%d stop=%d messages=%d", o.Model, o.Temperature, o.MaxTokens, len(o.Stop), len(messages))
-    for i, m := range messages {
-        logging.Logf("llm/ollama ", "msg[%d] role=%s size=%d preview=%s%s%s", i, m.Role, len(m.Content), logging.AnsiCyan, logging.PreviewForLog(m.Content), logging.AnsiBase)
-    }
-
-    req := ollamaChatRequest{Model: o.Model, Stream: true}
-    req.Messages = make([]oaMessage, len(messages))
-    for i, m := range messages {
-        req.Messages[i] = oaMessage{Role: m.Role, Content: m.Content}
-    }
-    // Build options map
-    optsMap := map[string]any{}
-    if o.Temperature != 0 {
-        optsMap["temperature"] = o.Temperature
-    }
-    if o.MaxTokens > 0 {
-        optsMap["num_predict"] = o.MaxTokens
-    }
-    if len(o.Stop) > 0 {
-        optsMap["stop"] = o.Stop
-    }
-    if len(optsMap) > 0 {
-        req.Options = optsMap
-    }
-
-    body, err := json.Marshal(req)
-    if err != nil {
-        return err
-    }
-
-    endpoint := c.baseURL + "/api/chat"
-    logging.Logf("llm/ollama ", "POST %s (stream)", endpoint)
-    httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body))
-    if err != nil {
-        return err
-    }
-    httpReq.Header.Set("Content-Type", "application/json")
-
-    resp, err := c.httpClient.Do(httpReq)
-    if err != nil {
-        logging.Logf("llm/ollama ", "%shttp error after %s: %v%s", logging.AnsiRed, time.Since(start), err, logging.AnsiBase)
-        return err
-    }
-    defer resp.Body.Close()
-    if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-        var apiErr ollamaChatResponse
-        _ = json.NewDecoder(resp.Body).Decode(&apiErr)
-        if strings.TrimSpace(apiErr.Error) != "" {
-            logging.Logf("llm/ollama ", "%sapi error status=%d msg=%s duration=%s%s", logging.AnsiRed, resp.StatusCode, apiErr.Error, time.Since(start), logging.AnsiBase)
-            return fmt.Errorf("ollama error: %s (status %d)", apiErr.Error, resp.StatusCode)
-        }
-        logging.Logf("llm/ollama ", "%shttp non-2xx status=%d duration=%s%s", logging.AnsiRed, resp.StatusCode, time.Since(start), logging.AnsiBase)
-        return fmt.Errorf("ollama http error: status %d", resp.StatusCode)
-    }
-
-    dec := json.NewDecoder(resp.Body)
-    for {
-        var ev ollamaChatResponse
-        if err := dec.Decode(&ev); err != nil {
-            if errors.Is(err, io.EOF) {
-                break
-            }
-            logging.Logf("llm/ollama ", "%sdecode stream error after %s: %v%s", logging.AnsiRed, time.Since(start), err, logging.AnsiBase)
-            return err
-        }
-        if strings.TrimSpace(ev.Error) != "" {
-            logging.Logf("llm/ollama ", "%sstream event error: %s%s", logging.AnsiRed, ev.Error, logging.AnsiBase)
-            return fmt.Errorf("ollama stream error: %s", ev.Error)
-        }
-        if s := ev.Message.Content; strings.TrimSpace(s) != "" {
-            onDelta(s)
-        }
-        if ev.Done {
-            break
-        }
-    }
-    logging.Logf("llm/ollama ", "stream end duration=%s", time.Since(start))
-    return nil
-}
+	o := Options{Model: c.defaultModel}
+	for _, opt := range opts {
+		opt(&o)
+	}
+	if o.Model == "" {
+		o.Model = c.defaultModel
+	}
+
+	start := time.Now()
+	logMessages := make([]struct {
+		Role    string
+		Content string
+	}, len(messages))
+	for i, m := range messages {
+		logMessages[i] = struct {
+			Role    string
+			Content string
+		}{Role: m.Role, Content: m.Content}
+	}
+	c.chatLogger.LogStart(true, o.Model, o.Temperature, o.MaxTokens, o.Stop, logMessages)
+
+	req := ollamaChatRequest{Model: o.Model, Stream: true}
+	req.Messages = make([]oaMessage, len(messages))
+	for i, m := range messages {
+		req.Messages[i] = oaMessage{Role: m.Role, Content: m.Content}
+	}
+	// Build options map
+	optsMap := map[string]any{}
+	if o.Temperature != 0 {
+		optsMap["temperature"] = o.Temperature
+	}
+	if o.MaxTokens > 0 {
+		optsMap["num_predict"] = o.MaxTokens
+	}
+	if len(o.Stop) > 0 {
+		optsMap["stop"] = o.Stop
+	}
+	if len(optsMap) > 0 {
+		req.Options = optsMap
+	}
+
+	body, err := json.Marshal(req)
+	if err != nil {
+		return err
+	}
+
+	endpoint := c.baseURL + "/api/chat"
+	logging.Logf("llm/ollama ", "POST %s (stream)", endpoint)
+	httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body))
+	if err != nil {
+		return err
+	}
+	httpReq.Header.Set("Content-Type", "application/json")
+
+	resp, err := c.httpClient.Do(httpReq)
+	if err != nil {
+		logging.Logf("llm/ollama ", "%shttp error after %s: %v%s", logging.AnsiRed, time.Since(start), err, logging.AnsiBase)
+		return err
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		var apiErr ollamaChatResponse
+		_ = json.NewDecoder(resp.Body).Decode(&apiErr)
+		if strings.TrimSpace(apiErr.Error) != "" {
+			logging.Logf("llm/ollama ", "%sapi error status=%d msg=%s duration=%s%s", logging.AnsiRed, resp.StatusCode, apiErr.Error, time.Since(start), logging.AnsiBase)
+			return fmt.Errorf("ollama error: %s (status %d)", apiErr.Error, resp.StatusCode)
+		}
+		logging.Logf("llm/ollama ", "%shttp non-2xx status=%d duration=%s%s", logging.AnsiRed, resp.StatusCode, time.Since(start), logging.AnsiBase)
+		return fmt.Errorf("ollama http error: status %d", resp.StatusCode)
+	}
+
+	dec := json.NewDecoder(resp.Body)
+	for {
+		var ev ollamaChatResponse
+		if err := dec.Decode(&ev); err != nil {
+			if errors.Is(err, io.EOF) {
+				break
+			}
+			logging.Logf("llm/ollama ", "%sdecode stream error after %s: %v%s", logging.AnsiRed, time.Since(start), err, logging.AnsiBase)
+			return err
+		}
+		if strings.TrimSpace(ev.Error) != "" {
+			logging.Logf("llm/ollama ", "%sstream event error: %s%s", logging.AnsiRed, ev.Error, logging.AnsiBase)
+			return fmt.Errorf("ollama stream error: %s", ev.Error)
+		}
+		if s := ev.Message.Content; strings.TrimSpace(s) != "" {
+			onDelta(s)
+		}
+		if ev.Done {
+			break
+		}
+	}
+	logging.Logf("llm/ollama ", "stream end duration=%s", time.Since(start))
+	return nil
+}
\ No newline at end of file
diff --git a/internal/llm/openai.go b/internal/llm/openai.go
index 288622f..fe6705b 100644
--- a/internal/llm/openai.go
+++ b/internal/llm/openai.go
@@ -3,17 +3,17 @@
 package llm
 
 import (
-    "bufio"
-    "bytes"
-    "context"
-    "encoding/json"
-    "errors"
-    "fmt"
+	"bufio"
+	"bytes"
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
 	"net/http"
 	"strings"
-    "time"
+	"time"
 
-    "hexai/internal/logging"
+	"hexai/internal/logging"
 )
 
 // openAIClient implements Client against OpenAI's Chat Completions API.
@@ -22,10 +22,9 @@ type openAIClient struct {
 	apiKey       string
 	baseURL      string
 	defaultModel string
+	chatLogger   *logging.ChatLogger
 }
 
-// Colors and base styling are provided by logging.go
-
 // newOpenAI constructs an OpenAI client using explicit configuration values.
 // The apiKey may be empty; calls will fail until a valid key is supplied.
 func newOpenAI(baseURL, model, apiKey string) Client {
@@ -40,16 +39,17 @@ func newOpenAI(baseURL, model, apiKey string) Client {
 		apiKey:       apiKey,
 		baseURL:      baseURL,
 		defaultModel: model,
+		chatLogger:   logging.NewChatLogger("openai"),
 	}
 }
 
 type oaChatRequest struct {
-    Model       string      `json:"model"`
-    Messages    []oaMessage `json:"messages"`
-    Temperature *float64    `json:"temperature,omitempty"`
-    MaxTokens   *int        `json:"max_tokens,omitempty"`
-    Stop        []string    `json:"stop,omitempty"`
-    Stream      bool        `json:"stream,omitempty"`
+	Model       string      `json:"model"`
+	Messages    []oaMessage `json:"messages"`
+	Temperature *float64    `json:"temperature,omitempty"`
+	MaxTokens   *int        `json:"max_tokens,omitempty"`
+	Stop        []string    `json:"stop,omitempty"`
+	Stream      bool        `json:"stream,omitempty"`
 }
 
 type oaMessage struct {
@@ -86,11 +86,18 @@ func (c *openAIClient) Chat(ctx context.Context, messages []Message, opts ...Req
 		o.Model = c.defaultModel
 	}
 	start := time.Now()
-	logging.Logf("llm/openai ", "chat start model=%s temp=%.2f max_tokens=%d stop=%d messages=%d", o.Model, o.Temperature, o.MaxTokens, len(o.Stop), len(messages))
+	logMessages := make([]struct {
+		Role    string
+		Content string
+	}, len(messages))
 	for i, m := range messages {
-		// Sending context (cyan)
-		logging.Logf("llm/openai ", "msg[%d] role=%s size=%d preview=%s%s%s", i, m.Role, len(m.Content), logging.AnsiCyan, logging.PreviewForLog(m.Content), logging.AnsiBase)
+		logMessages[i] = struct {
+			Role    string
+			Content string
+		}{Role: m.Role, Content: m.Content}
 	}
+	c.chatLogger.LogStart(false, o.Model, o.Temperature, o.MaxTokens, o.Stop, logMessages)
+
 	req := oaChatRequest{Model: o.Model}
 	req.Messages = make([]oaMessage, len(messages))
 	for i, m := range messages {
@@ -152,138 +159,135 @@ func (c *openAIClient) Chat(ctx context.Context, messages []Message, opts ...Req
 	return content, nil
 }
 
-// small helper to keep return type consistent
-func nilStringErr(msg string) (string, error) { return "", errors.New(msg) }
-
 func (c *openAIClient) logf(format string, args ...any) { logging.Logf("llm/openai ", format, args...) }
 
-func trimPreview(s string, n int) string {
-	if n <= 0 || len(s) <= n {
-		return s
-	}
-	return s[:n] + "…"
-}
-
 // Provider metadata
 func (c *openAIClient) Name() string         { return "openai" }
 func (c *openAIClient) DefaultModel() string { return c.defaultModel }
 
 // Streaming support (optional)
 type oaStreamChunk struct {
-    Choices []struct {
-        Delta struct {
-            Content string `json:"content"`
-        } `json:"delta"`
-        FinishReason string `json:"finish_reason"`
-    } `json:"choices"`
-    Error *struct {
-        Message string `json:"message"`
-        Type    string `json:"type"`
-        Param   any    `json:"param"`
-        Code    any    `json:"code"`
-    } `json:"error,omitempty"`
+	Choices []struct {
+		Delta struct {
+			Content string `json:"content"`
+		} `json:"delta"`
+		FinishReason string `json:"finish_reason"`
+	} `json:"choices"`
+	Error *struct {
+		Message string `json:"message"`
+		Type    string `json:"type"`
+		Param   any    `json:"param"`
+		Code    any    `json:"code"`
+	} `json:"error,omitempty"`
 }
 
 func (c *openAIClient) ChatStream(ctx context.Context, messages []Message, onDelta func(string), opts ...RequestOption) error {
-    if c.apiKey == "" {
-        return errors.New("missing OpenAI API key")
-    }
-    o := Options{Model: c.defaultModel}
-    for _, opt := range opts {
-        opt(&o)
-    }
-    if o.Model == "" {
-        o.Model = c.defaultModel
-    }
-    start := time.Now()
-    logging.Logf("llm/openai ", "stream start model=%s temp=%.2f max_tokens=%d stop=%d messages=%d", o.Model, o.Temperature, o.MaxTokens, len(o.Stop), len(messages))
-    for i, m := range messages {
-        logging.Logf("llm/openai ", "msg[%d] role=%s size=%d preview=%s%s%s", i, m.Role, len(m.Content), logging.AnsiCyan, logging.PreviewForLog(m.Content), logging.AnsiBase)
-    }
+	if c.apiKey == "" {
+		return errors.New("missing OpenAI API key")
+	}
+	o := Options{Model: c.defaultModel}
+	for _, opt := range opts {
+		opt(&o)
+	}
+	if o.Model == "" {
+		o.Model = c.defaultModel
+	}
+	start := time.Now()
+	logMessages := make([]struct {
+		Role    string
+		Content string
+	}, len(messages))
+	for i, m := range messages {
+		logMessages[i] = struct {
+			Role    string
+			Content string
+		}{Role: m.Role, Content: m.Content}
+	}
+	c.chatLogger.LogStart(true, o.Model, o.Temperature, o.MaxTokens, o.Stop, logMessages)
 
-    req := oaChatRequest{Model: o.Model, Stream: true}
-    req.Messages = make([]oaMessage, len(messages))
-    for i, m := range messages {
-        req.Messages[i] = oaMessage{Role: m.Role, Content: m.Content}
-    }
-    if o.Temperature != 0 {
-        req.Temperature = &o.Temperature
-    }
-    if o.MaxTokens > 0 {
-        req.MaxTokens = &o.MaxTokens
-    }
-    if len(o.Stop) > 0 {
-        req.Stop = o.Stop
-    }
+	req := oaChatRequest{Model: o.Model, Stream: true}
+	req.Messages = make([]oaMessage, len(messages))
+	for i, m := range messages {
+		req.Messages[i] = oaMessage{Role: m.Role, Content: m.Content}
+	}
+	if o.Temperature != 0 {
+		req.Temperature = &o.Temperature
+	}
+	if o.MaxTokens > 0 {
+		req.MaxTokens = &o.MaxTokens
+	}
+	if len(o.Stop) > 0 {
+		req.Stop = o.Stop
+	}
 
-    body, err := json.Marshal(req)
-    if err != nil {
-        c.logf("marshal error: %v", err)
-        return err
-    }
-    endpoint := c.baseURL + "/chat/completions"
-    logging.Logf("llm/openai ", "POST %s (stream)", endpoint)
-    httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body))
-    if err != nil {
-        c.logf("new request error: %v", err)
-        return err
-    }
-    httpReq.Header.Set("Content-Type", "application/json")
-    httpReq.Header.Set("Authorization", "Bearer "+c.apiKey)
-    // Streaming uses SSE-style data lines
-    httpReq.Header.Set("Accept", "text/event-stream")
+	body, err := json.Marshal(req)
+	if err != nil {
+		c.logf("marshal error: %v", err)
+		return err
+	}
+	endpoint := c.baseURL + "/chat/completions"
+	logging.Logf("llm/openai ", "POST %s (stream)", endpoint)
+	httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body))
+	if err != nil {
+		c.logf("new request error: %v", err)
+		return err
+	}
+	httpReq.Header.Set("Content-Type", "application/json")
+	httpReq.Header.Set("Authorization", "Bearer "+c.apiKey)
+	// Streaming uses SSE-style data lines
+	httpReq.Header.Set("Accept", "text/event-stream")
 
-    resp, err := c.httpClient.Do(httpReq)
-    if err != nil {
-        logging.Logf("llm/openai ", "%shttp error after %s: %v%s", logging.AnsiRed, time.Since(start), err, logging.AnsiBase)
-        return err
-    }
-    defer resp.Body.Close()
-    if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-        // try to decode body to surface message
-        var apiErr oaChatResponse
-        _ = json.NewDecoder(resp.Body).Decode(&apiErr)
-        if apiErr.Error != nil && apiErr.Error.Message != "" {
-            logging.Logf("llm/openai ", "%sapi error status=%d type=%s msg=%s duration=%s%s", logging.AnsiRed, resp.StatusCode, apiErr.Error.Type, apiErr.Error.Message, time.Since(start), logging.AnsiBase)
-            return fmt.Errorf("openai error: %s (status %d)", apiErr.Error.Message, resp.StatusCode)
-        }
-        logging.Logf("llm/openai ", "%shttp non-2xx status=%d duration=%s%s", logging.AnsiRed, resp.StatusCode, time.Since(start), logging.AnsiBase)
-        return fmt.Errorf("openai http error: status %d", resp.StatusCode)
-    }
+	resp, err := c.httpClient.Do(httpReq)
+	if err != nil {
+		logging.Logf("llm/openai ", "%shttp error after %s: %v%s", logging.AnsiRed, time.Since(start), err, logging.AnsiBase)
+		return err
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		// try to decode body to surface message
+		var apiErr oaChatResponse
+		_ = json.NewDecoder(resp.Body).Decode(&apiErr)
+		if apiErr.Error != nil && apiErr.Error.Message != "" {
+			logging.Logf("llm/openai ", "%sapi error status=%d type=%s msg=%s duration=%s%s", logging.AnsiRed, resp.StatusCode, apiErr.Error.Type, apiErr.Error.Message, time.Since(start), logging.AnsiBase)
+			return fmt.Errorf("openai error: %s (status %d)", apiErr.Error.Message, resp.StatusCode)
+		}
+		logging.Logf("llm/openai ", "%shttp non-2xx status=%d duration=%s%s", logging.AnsiRed, resp.StatusCode, time.Since(start), logging.AnsiBase)
+		return fmt.Errorf("openai http error: status %d", resp.StatusCode)
+	}
 
-    // Parse SSE: lines starting with "data: " containing JSON or [DONE]
-    scanner := bufio.NewScanner(resp.Body)
-    // Increase buffer for long lines
-    const maxBuf = 1024 * 1024
-    buf := make([]byte, 0, 64*1024)
-    scanner.Buffer(buf, maxBuf)
-    for scanner.Scan() {
-        line := scanner.Text()
-        if !strings.HasPrefix(line, "data: ") {
-            continue
-        }
-        payload := strings.TrimPrefix(line, "data: ")
-        if strings.TrimSpace(payload) == "[DONE]" {
-            break
-        }
-        var chunk oaStreamChunk
-        if err := json.Unmarshal([]byte(payload), &chunk); err != nil {
-            continue // skip malformed lines
-        }
-        if chunk.Error != nil && chunk.Error.Message != "" {
-            logging.Logf("llm/openai ", "%sstream error: %s%s", logging.AnsiRed, chunk.Error.Message, logging.AnsiBase)
-            return fmt.Errorf("openai stream error: %s", chunk.Error.Message)
-        }
-        for _, ch := range chunk.Choices {
-            if ch.Delta.Content != "" {
-                onDelta(ch.Delta.Content)
-            }
-        }
-    }
-    if err := scanner.Err(); err != nil {
-        logging.Logf("llm/openai ", "%sstream read error after %s: %v%s", logging.AnsiRed, time.Since(start), err, logging.AnsiBase)
-        return err
-    }
-    logging.Logf("llm/openai ", "stream end duration=%s", time.Since(start))
-    return nil
-}
+	// Parse SSE: lines starting with "data: " containing JSON or [DONE]
+	scanner := bufio.NewScanner(resp.Body)
+	// Increase buffer for long lines
+	const maxBuf = 1024 * 1024
+	buf := make([]byte, 0, 64*1024)
+	scanner.Buffer(buf, maxBuf)
+	for scanner.Scan() {
+		line := scanner.Text()
+		if !strings.HasPrefix(line, "data: ") {
+			continue
+		}
+		payload := strings.TrimPrefix(line, "data: ")
+		if strings.TrimSpace(payload) == "[DONE]" {
+			break
+		}
+		var chunk oaStreamChunk
+		if err := json.Unmarshal([]byte(payload), &chunk); err != nil {
+			continue // skip malformed lines
+		}
+		if chunk.Error != nil && chunk.Error.Message != "" {
+			logging.Logf("llm/openai ", "%sstream error: %s%s", logging.AnsiRed, chunk.Error.Message, logging.AnsiBase)
+			return fmt.Errorf("openai stream error: %s", chunk.Error.Message)
+		}
+		for _, ch := range chunk.Choices {
+			if ch.Delta.Content != "" {
+				onDelta(ch.Delta.Content)
+			}
+		}
+	}
+	if err := scanner.Err(); err != nil {
+		logging.Logf("llm/openai ", "%sstream read error after %s: %v%s", logging.AnsiRed, time.Since(start), err, logging.AnsiBase)
+		return err
+	}
+	logging.Logf("llm/openai ", "stream end duration=%s", time.Since(start))
+	return nil
+}
\ No newline at end of file
diff --git a/internal/logging/chatlogger.go b/internal/logging/chatlogger.go
new file mode 100644
index 0000000..b6b84a3
--- /dev/null
+++ b/internal/logging/chatlogger.go
@@ -0,0 +1,28 @@
+package logging
+
+// ChatLogger provides a structured way to log chat interactions.
+type ChatLogger struct {
+	Provider string
+}
+
+// NewChatLogger creates a new ChatLogger for a given provider.
+func NewChatLogger(provider string) *ChatLogger {
+	return &ChatLogger{Provider: provider}
+}
+
+// LogStart logs the beginning of a chat or stream interaction.
+func (cl *ChatLogger) LogStart(stream bool, model string, temp float64, maxTokens int, stop []string, messages []struct {
+	Role    string
+	Content string
+}) {
+	chatOrStream := "chat"
+	if stream {
+		chatOrStream = "stream"
+	}
+	Logf("llm/"+cl.Provider+" ", "%s start model=%s temp=%.2f max_tokens=%d stop=%d messages=%d",
+		chatOrStream, model, temp, maxTokens, len(stop), len(messages))
+	for i, m := range messages {
+		Logf("llm/"+cl.Provider+" ", "msg[%d] role=%s size=%d preview=%s%s%s",
+			i, m.Role, len(m.Content), AnsiCyan, PreviewForLog(m.Content), AnsiBase)
+	}
+}
diff --git a/internal/logging/logging.go b/internal/logging/logging.go
index b82ee99..2975c7a 100644
--- a/internal/logging/logging.go
+++ b/internal/logging/logging.go
@@ -1,5 +1,4 @@
 // Summary: ANSI-styled logging utilities with a bound standard logger and configurable preview truncation.
-// Not yet reviewed by a human
 package logging
 
 import (