From 236f6543b093e54157f40ec0c021b35177e1713e Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Thu, 11 Jun 2026 00:16:27 +0300 Subject: Refactor lsp.Server God Object: extract chat and completion subsystems The Server type accumulated two large, tangled feature subsystems (in-editor chat and code completion) alongside its core LSP dispatch role. Pull them into cohesive types that own their state and logic while delegating shared infrastructure back to Server via a back-reference. - Add completionService (completion_service.go): owns the completion cache/throttle state (completionState) and all completion request-handling logic (handleCompletion, plan/jobs/execute, provider-native path, post-processing, message building, prefix heuristics). Methods moved off Server in handlers_completion.go to completionService. - Add chatService (chat_service.go + chat_handlers.go): owns the input-activity clock (lastInput, with its own mutex instead of Server.mu) and all in-editor chat logic (detection, transcript history, message building, edit application, inline prompts) plus the slash-command handlers (chat_commands.go). - Server now holds chat/completion fields, wires them in NewServer, and delegates (dispatch table, didChange, debounce gate) to them. Thin Server shims preserve the existing completion-state API for callers/tests. Pure refactor: no behavior or LSP protocol changes. Tests adjusted only to reach the relocated methods/state via the services. All tests pass with -race; coverage 86.1%. Co-Authored-By: Claude Opus 4.8 --- internal/lsp/build_prompts_table_test.go | 2 +- internal/lsp/chat_commands.go | 25 +- internal/lsp/chat_commands_test.go | 14 +- internal/lsp/chat_context_mode_test.go | 10 +- internal/lsp/chat_handlers.go | 337 ++++++++++++++++++++++ internal/lsp/chat_history_test.go | 12 +- internal/lsp/chat_no_double_answer_test.go | 2 +- internal/lsp/chat_prompt_test.go | 2 +- internal/lsp/chat_service.go | 52 ++++ internal/lsp/chat_trigger_suppression_test.go | 4 +- internal/lsp/completion_cache_test.go | 4 +- internal/lsp/completion_codex_path_test.go | 8 +- internal/lsp/completion_helpers_more_test.go | 10 +- internal/lsp/completion_messages_test.go | 16 +- internal/lsp/completion_prefix_strip_test.go | 24 +- internal/lsp/completion_provider_fallback_test.go | 2 +- internal/lsp/completion_service.go | 76 +++++ internal/lsp/completion_state.go | 28 -- internal/lsp/completion_toggle_test.go | 2 +- internal/lsp/debounce_throttle_more_test.go | 13 +- internal/lsp/debounce_throttle_test.go | 12 +- internal/lsp/document.go | 5 +- internal/lsp/document_test.go | 6 +- internal/lsp/handlers_completion.go | 90 +++--- internal/lsp/handlers_document.go | 322 +-------------------- internal/lsp/handlers_end_to_end_test.go | 2 +- internal/lsp/handlers_utils.go | 2 +- internal/lsp/helpers_inline_prompt_test.go | 2 +- internal/lsp/helpers_more_test.go | 2 +- internal/lsp/ignore_test.go | 3 +- internal/lsp/inline_prompt_completion_test.go | 2 +- internal/lsp/log_context_test.go | 2 +- internal/lsp/postprocess_indent_test.go | 2 +- internal/lsp/provider_native_success_test.go | 6 +- internal/lsp/server.go | 34 +-- internal/lsp/server_test.go | 4 +- internal/lsp/triggers_config_test.go | 8 +- 37 files changed, 644 insertions(+), 503 deletions(-) create mode 100644 internal/lsp/chat_handlers.go create mode 100644 internal/lsp/chat_service.go create mode 100644 internal/lsp/completion_service.go (limited to 'internal') diff --git a/internal/lsp/build_prompts_table_test.go b/internal/lsp/build_prompts_table_test.go index bc4f031..3c40f26 100644 --- a/internal/lsp/build_prompts_table_test.go +++ b/internal/lsp/build_prompts_table_test.go @@ -13,7 +13,7 @@ func TestBuildPrompts_Table(t *testing.T) { } for _, c := range cases { s := newTestServer() - msgs := s.buildCompletionMessages(false, false, "", c.inParams, p, "above", "current", "below", "func ctx") + msgs := s.completion.buildCompletionMessages(false, false, "", c.inParams, p, "above", "current", "below", "func ctx") if len(msgs) < 2 || msgs[0].Role != "system" || msgs[1].Role != "user" { t.Fatalf("%s: unexpected messages", c.name) } diff --git a/internal/lsp/chat_commands.go b/internal/lsp/chat_commands.go index 11e5a28..480643a 100644 --- a/internal/lsp/chat_commands.go +++ b/internal/lsp/chat_commands.go @@ -11,27 +11,27 @@ type chatCommandResult struct { message string } -func (s *Server) chatCommandResponse(uri string, lineIdx int, prompt string) (chatCommandResult, bool) { - trimmed := strings.TrimSpace(s.stripTrailingTrigger(prompt)) +func (c *chatService) chatCommandResponse(uri string, lineIdx int, prompt string) (chatCommandResult, bool) { + trimmed := strings.TrimSpace(c.stripTrailingTrigger(prompt)) if trimmed == "" || !strings.HasPrefix(trimmed, "/") { return chatCommandResult{}, false } switch { case strings.HasPrefix(trimmed, "/reload"): - return s.handleReloadCommand(), true + return c.handleReloadCommand(), true case strings.HasPrefix(trimmed, "/help"): - return s.handleHelpCommand(), true + return c.handleHelpCommand(), true case strings.HasPrefix(trimmed, "/disable"): - return s.handleDisableCompletionCommand(), true + return c.handleDisableCompletionCommand(), true case strings.HasPrefix(trimmed, "/enable"): - return s.handleEnableCompletionCommand(), true + return c.handleEnableCompletionCommand(), true default: return chatCommandResult{message: fmt.Sprintf("Unknown command %q. Try /help?>", trimmed)}, true } } -func (s *Server) handleHelpCommand() chatCommandResult { +func (c *chatService) handleHelpCommand() chatCommandResult { lines := []string{ "Available slash commands:", "- /reload?> reload configuration from file (ignores env overrides)", @@ -41,7 +41,8 @@ func (s *Server) handleHelpCommand() chatCommandResult { return chatCommandResult{message: strings.Join(lines, "\n")} } -func (s *Server) handleReloadCommand() chatCommandResult { +func (c *chatService) handleReloadCommand() chatCommandResult { + s := c.srv if s.configStore == nil { return chatCommandResult{message: "Reload unavailable: no config store"} } @@ -57,16 +58,16 @@ func (s *Server) handleReloadCommand() chatCommandResult { return chatCommandResult{message: summary} } -func (s *Server) handleDisableCompletionCommand() chatCommandResult { - prev := s.setCompletionsDisabled(true) +func (c *chatService) handleDisableCompletionCommand() chatCommandResult { + prev := c.srv.setCompletionsDisabled(true) if prev { return chatCommandResult{message: "Auto-completions were already disabled."} } return chatCommandResult{message: "Auto-completions disabled. Use /enable?> to restore."} } -func (s *Server) handleEnableCompletionCommand() chatCommandResult { - prev := s.setCompletionsDisabled(false) +func (c *chatService) handleEnableCompletionCommand() chatCommandResult { + prev := c.srv.setCompletionsDisabled(false) if !prev { return chatCommandResult{message: "Auto-completions are already enabled."} } diff --git a/internal/lsp/chat_commands_test.go b/internal/lsp/chat_commands_test.go index ffe31dd..15a3acf 100644 --- a/internal/lsp/chat_commands_test.go +++ b/internal/lsp/chat_commands_test.go @@ -28,7 +28,7 @@ func TestFormatReloadSummary(t *testing.T) { func TestHandleHelpCommandListsReload(t *testing.T) { s := newTestServer() - res := s.handleHelpCommand() + res := s.chatSvc().handleHelpCommand() if !strings.Contains(res.message, "/reload?>") { t.Fatalf("expected reload command in help output: %q", res.message) } @@ -70,7 +70,7 @@ func TestHandleReloadCommandReloadsStore(t *testing.T) { t.Fatalf("update config: %v", err) } - res := s.handleReloadCommand() + res := s.chatSvc().handleReloadCommand() if !strings.Contains(res.message, "Reloaded config (1 changes):") { t.Fatalf("unexpected reload summary: %q", res.message) } @@ -115,7 +115,7 @@ func TestDetectAndHandleChatExecutesSlashCommand(t *testing.T) { uri := "file:///cmd.go" s.setDocument(uri, "/reload>\n") - s.detectAndHandleChat(uri) + s.chatSvc().detectAndHandleChat(uri) outStr := out.String() if !strings.Contains(outStr, "Reloaded config") { @@ -132,7 +132,7 @@ func TestDisableEnableCommandsToggleCompletions(t *testing.T) { t.Fatalf("expected completions enabled initially") } - if res, ok := s.chatCommandResponse("file:///x", 0, "/disable>"); !ok { + if res, ok := s.chatSvc().chatCommandResponse("file:///x", 0, "/disable>"); !ok { t.Fatalf("expected disable command to be handled") } else if !strings.Contains(res.message, "disabled") { t.Fatalf("unexpected disable message: %q", res.message) @@ -141,13 +141,13 @@ func TestDisableEnableCommandsToggleCompletions(t *testing.T) { t.Fatalf("expected completions disabled after command") } - if res, ok := s.chatCommandResponse("file:///x", 0, "/disable>"); !ok { + if res, ok := s.chatSvc().chatCommandResponse("file:///x", 0, "/disable>"); !ok { t.Fatalf("expected repeated disable command to be handled") } else if !strings.Contains(res.message, "already disabled") { t.Fatalf("expected already-disabled message, got %q", res.message) } - if res, ok := s.chatCommandResponse("file:///x", 0, "/enable>"); !ok { + if res, ok := s.chatSvc().chatCommandResponse("file:///x", 0, "/enable>"); !ok { t.Fatalf("expected enable command to be handled") } else if !strings.Contains(res.message, "enabled") { t.Fatalf("unexpected enable message: %q", res.message) @@ -156,7 +156,7 @@ func TestDisableEnableCommandsToggleCompletions(t *testing.T) { t.Fatalf("expected completions enabled after command") } - if res, ok := s.chatCommandResponse("file:///x", 0, "/enable>"); !ok { + if res, ok := s.chatSvc().chatCommandResponse("file:///x", 0, "/enable>"); !ok { t.Fatalf("expected repeated enable command to be handled") } else if !strings.Contains(res.message, "already enabled") { t.Fatalf("expected already-enabled message, got %q", res.message) diff --git a/internal/lsp/chat_context_mode_test.go b/internal/lsp/chat_context_mode_test.go index 876f092..e6696f4 100644 --- a/internal/lsp/chat_context_mode_test.go +++ b/internal/lsp/chat_context_mode_test.go @@ -23,7 +23,7 @@ func TestChat_RespectsContextModeWindow(t *testing.T) { src := "package main\nline2 context\nwhat?>\n" s.setDocument(uri, src) - s.detectAndHandleChat(uri) + s.chatSvc().detectAndHandleChat(uri) s.inflight.Wait() if len(cap.msgs) == 0 { t.Fatalf("expected Chat to be called") @@ -59,7 +59,7 @@ func TestChat_ContextModeMinimal_NoExtra(t *testing.T) { uri := "file:///ctx2.go" s.setDocument(uri, "package main\nhelp?>\n") - s.detectAndHandleChat(uri) + s.chatSvc().detectAndHandleChat(uri) s.inflight.Wait() if len(cap.msgs) != 2 { @@ -81,7 +81,7 @@ func TestChat_ContextModeAlwaysFull_AddsExtra(t *testing.T) { uri := "file:///ctx3.go" s.setDocument(uri, "package main\nline2\nhelp?>\n") - s.detectAndHandleChat(uri) + s.chatSvc().detectAndHandleChat(uri) s.inflight.Wait() if len(cap.msgs) < 3 { @@ -109,7 +109,7 @@ func TestChat_ContextModeFileOnNewFunc_NoExtraWithoutSignature(t *testing.T) { uri := "file:///ctx4.go" s.setDocument(uri, "package main\nhelp?>\n") - s.detectAndHandleChat(uri) + s.chatSvc().detectAndHandleChat(uri) s.inflight.Wait() if len(cap.msgs) != 2 { @@ -130,7 +130,7 @@ func TestChat_ContextModeFileOnNewFunc_WithSignature_AddsExtra(t *testing.T) { // Signature without '{' yet; chat prompt appears before the body, so newFunc=true src := "package main\n\nfunc add(x int) int\nhelp?>\n" s.setDocument(uri, src) - s.detectAndHandleChat(uri) + s.chatSvc().detectAndHandleChat(uri) s.inflight.Wait() if len(cap.msgs) < 3 { diff --git a/internal/lsp/chat_handlers.go b/internal/lsp/chat_handlers.go new file mode 100644 index 0000000..6fffaca --- /dev/null +++ b/internal/lsp/chat_handlers.go @@ -0,0 +1,337 @@ +// In-editor chat handling for the LSP server. These are methods on chatService +// (the extracted in-editor chat subsystem), which detects chat/inline-prompt +// trigger lines, builds the rolling transcript history and request messages, +// and applies the LLM reply back into the document. It reaches into Server (via +// c.srv, aliased to s) for shared infrastructure such as LLM clients, document +// access, config and edit dispatch. +package lsp + +import ( + "strings" + "time" + + "codeberg.org/snonux/hexai/internal/llm" + "codeberg.org/snonux/hexai/internal/logging" +) + +// --- in-editor chat (";C ...") --- + +// detectAndHandleChat scans the current document for any line that starts with +// a new trigger pair (e.g., "?>" ",>" ":>" ";>") at EOL and inserts the LLM +// reply below. +func (c *chatService) detectAndHandleChat(uri string) { + s := c.srv + d := s.getDocument(uri) + if d == nil || len(d.lines) == 0 { + return + } + suffix, prefixes, _ := s.chatConfig() + openStr, _, openChar, closeChar := s.inlineMarkers() + for i, raw := range d.lines { + if c.maybeRunInlinePrompt(uri, i, raw, openStr, openChar, closeChar) { + continue + } + match, ok := parseChatPromptLine(raw, suffix, prefixes) + if !ok { + continue + } + if hasChatResponseBelow(d, i) { + continue + } + c.handleChatPrompt(uri, i, match) + // Only handle one per change tick to avoid flooding + break + } +} + +type chatPromptLine struct { + lastNonSpace int + removeCount int + prompt string +} + +func (c *chatService) maybeRunInlinePrompt(uri string, lineIdx int, raw string, openStr string, openChar byte, closeChar byte) bool { + s := c.srv + if !lineHasInlinePrompt(raw, openStr, openChar, closeChar) { + return false + } + if s.currentLLMClient() != nil { + pos := Position{Line: lineIdx, Character: len(raw)} + s.inflight.Add(1) + go func() { + defer s.inflight.Done() + c.runInlinePrompt(uri, pos) + }() + } + return true +} + +func parseChatPromptLine(raw string, suffix string, prefixes []string) (chatPromptLine, bool) { + if suffix == "" { + return chatPromptLine{}, false + } + last := findLastNonSpaceIndex(raw) + if last < 0 || string(raw[last]) != suffix { + return chatPromptLine{}, false + } + removeCount := len(suffix) + baseEnd := last + 1 - removeCount + if baseEnd < 0 { + return chatPromptLine{}, false + } + prompt := strings.TrimSpace(raw[:baseEnd]) + if prompt == "" { + return chatPromptLine{}, false + } + if !strings.HasPrefix(prompt, "/") && !hasTriggerPrefix(raw, last, prefixes) { + return chatPromptLine{}, false + } + return chatPromptLine{lastNonSpace: last, removeCount: removeCount, prompt: prompt}, true +} + +func findLastNonSpaceIndex(raw string) int { + for i := len(raw) - 1; i >= 0; i-- { + if raw[i] != ' ' && raw[i] != '\t' { + return i + } + } + return -1 +} + +func hasTriggerPrefix(raw string, suffixIdx int, prefixes []string) bool { + if suffixIdx < 1 { + return false + } + prev := string(raw[suffixIdx-1]) + for _, pfx := range prefixes { + if prev == pfx { + return true + } + } + return false +} + +func hasChatResponseBelow(d *document, lineIdx int) bool { + for i := lineIdx + 1; i < len(d.lines); i++ { + trimmed := strings.TrimSpace(d.lines[i]) + if trimmed == "" { + continue + } + return strings.HasPrefix(trimmed, ">") + } + return false +} + +func (c *chatService) handleChatPrompt(uri string, lineIdx int, match chatPromptLine) { + s := c.srv + if resp, ok := c.chatCommandResponse(uri, lineIdx, match.prompt); ok { + msg := strings.TrimSpace(resp.message) + if msg != "" { + c.applyChatEdits(uri, lineIdx, match.lastNonSpace, match.removeCount, "> "+msg) + } + return + } + s.inflight.Add(1) + go func() { + defer s.inflight.Done() + c.requestChatResponse(uri, lineIdx, match) + }() +} + +func (c *chatService) requestChatResponse(uri string, lineIdx int, match chatPromptLine) { + s := c.srv + ctx, cancel := s.requestTimeoutContext(25 * time.Second) + defer cancel() + pos := Position{Line: lineIdx, Character: match.lastNonSpace + 1} + msgs := c.buildChatMessages(uri, pos, match.prompt) + spec := s.buildRequestSpec(surfaceChat) + client := s.clientFor(spec) + if client == nil { + return + } + modelUsed := spec.effectiveModel(client.DefaultModel()) + logging.Logf("lsp ", "chat llm=requesting model=%s", modelUsed) + text, err := s.chatWithStats(ctx, surfaceChat, spec, msgs) + if err != nil { + logging.Logf("lsp ", "chat llm error: %v", err) + return + } + out := strings.TrimSpace(stripCodeFences(text)) + if out == "" { + return + } + c.applyChatEdits(uri, lineIdx, match.lastNonSpace, match.removeCount, "> "+out) +} + +// applyChatEdits removes the triggering punctuation at end of the line and +// inserts two newlines followed by a new line with the response prefixed. +func (c *chatService) applyChatEdits(uri string, lineIdx int, lastNonSpace int, removeCount int, response string) { + s := c.srv + d := s.getDocument(uri) + if d == nil { + return + } + // 1) Delete the trailing punctuation (1 or 2 chars) + delStart := Position{Line: lineIdx, Character: lastNonSpace + 1 - removeCount} + delEnd := Position{Line: lineIdx, Character: lastNonSpace + 1} + // 2) Insert two newlines and the response at end-of-line, then one extra blank line + insPos := Position{Line: lineIdx, Character: len(d.lines[lineIdx])} + resp := strings.TrimRight(response, "\n") + "\n" + insert := "\n\n" + resp + "\n" + edits := []TextEdit{ + {Range: Range{Start: delStart, End: delEnd}, NewText: ""}, + {Range: Range{Start: insPos, End: insPos}, NewText: insert}, + } + we := WorkspaceEdit{Changes: map[string][]TextEdit{uri: edits}} + s.clientApplyEdit("Hexai: insert chat response", we) +} + +func (c *chatService) runInlinePrompt(uri string, pos Position) { + s := c.srv + if s.currentLLMClient() == nil { + return + } + d := s.getDocument(uri) + if d == nil || pos.Line < 0 || pos.Line >= len(d.lines) { + return + } + line := d.lines[pos.Line] + openStr, _, openChar, closeChar := s.inlineMarkers() + if !lineHasInlinePrompt(line, openStr, openChar, closeChar) { + return + } + p := CompletionParams{TextDocument: TextDocumentIdentifier{URI: uri}, Position: Position{Line: pos.Line, Character: len(line)}} + p.Context = map[string]int{"triggerKind": 1} + above, current, below, funcCtx := s.lineContext(uri, p.Position) + docStr := s.completion.buildDocString(p, above, current, below, funcCtx) + newFunc := s.isDefiningNewFunction(uri, p.Position) + extra, hasExtra := s.buildAdditionalContext(newFunc, uri, p.Position) + items, ok, _ := s.completion.tryLLMCompletion(p, above, current, below, funcCtx, docStr, hasExtra, extra) + if !ok || len(items) == 0 { + return + } + c.applyInlineCompletion(uri, items[0]) +} + +func (c *chatService) applyInlineCompletion(uri string, item CompletionItem) { + var edits []TextEdit + if len(item.AdditionalTextEdits) > 0 { + edits = append(edits, item.AdditionalTextEdits...) + } + if item.TextEdit != nil { + edits = append(edits, *item.TextEdit) + } + if len(edits) == 0 { + return + } + we := WorkspaceEdit{Changes: map[string][]TextEdit{uri: edits}} + c.srv.clientApplyEdit("Hexai: inline prompt", we) +} + +// buildChatHistory walks upwards from the current line to collect the most recent +// Q/A pairs in the in-editor transcript. Returns messages ending with current prompt. +func (c *chatService) buildChatHistory(uri string, lineIdx int, currentPrompt string) []llm.Message { + s := c.srv + d := s.getDocument(uri) + if d == nil { + return []llm.Message{{Role: "user", Content: currentPrompt}} + } + type pair struct{ q, a string } + pairs := []pair{} + i := lineIdx - 1 + for i >= 0 && len(pairs) < 3 { + for i >= 0 && strings.TrimSpace(d.lines[i]) == "" { + i-- + } + if i < 0 { + break + } + if !strings.HasPrefix(strings.TrimSpace(d.lines[i]), ">") { + break + } + var replyLines []string + for i >= 0 { + line := strings.TrimSpace(d.lines[i]) + if strings.HasPrefix(line, ">") { + replyLines = append([]string{strings.TrimSpace(strings.TrimPrefix(line, ">"))}, replyLines...) + i-- + continue + } + break + } + for i >= 0 && strings.TrimSpace(d.lines[i]) == "" { + i-- + } + if i < 0 { + break + } + q := strings.TrimSpace(d.lines[i]) + q = c.stripTrailingTrigger(q) + pairs = append([]pair{{q: q, a: strings.Join(replyLines, "\n")}}, pairs...) + i-- + } + msgs := make([]llm.Message, 0, len(pairs)*2+1) + for _, p := range pairs { + if strings.TrimSpace(p.q) != "" { + msgs = append(msgs, llm.Message{Role: "user", Content: p.q}) + } + if strings.TrimSpace(p.a) != "" { + msgs = append(msgs, llm.Message{Role: "assistant", Content: p.a}) + } + } + msgs = append(msgs, llm.Message{Role: "user", Content: currentPrompt}) + return msgs +} + +// stripTrailingTrigger removes the trailing chat trigger punctuation from a line if present. +func (c *chatService) stripTrailingTrigger(sx string) string { + trim := strings.TrimRight(sx, " \t") + if len(trim) == 0 { + return sx + } + _, prefixes, suffixChar := c.srv.chatConfig() + if len(trim) >= 2 && suffixChar != 0 && trim[len(trim)-1] == suffixChar { + prev := string(trim[len(trim)-2]) + for _, pf := range prefixes { + if prev == pf { + return strings.TrimRight(trim[:len(trim)-1], " \t") + } + } + } + last := trim[len(trim)-1] + switch last { + case '?', '!', ':': + return strings.TrimRight(trim[:len(trim)-1], " \t") + default: + return sx + } +} + +// buildChatMessages assembles the chat request messages using: +// - system from prompts.chat.system +// - rolling in-editor history up to current prompt +// - optional extra context per general.context_mode (window/full-file/new-func) +func (c *chatService) buildChatMessages(uri string, pos Position, prompt string) []llm.Message { + s := c.srv + // Base system and history + cfg := s.currentConfig() + sys := cfg.PromptChatSystem + // Determine line index for history from position + lineIdx := pos.Line + history := c.buildChatHistory(uri, lineIdx, prompt) + // Start with system + msgs := []llm.Message{{Role: "system", Content: sys}} + // Optional additional context like completion path (insert before history so last remains the prompt) + newFunc := s.isDefiningNewFunction(uri, pos) + if extra, has := s.buildAdditionalContext(newFunc, uri, pos); has && strings.TrimSpace(extra) != "" { + // Reuse completion's extra header template to avoid duplication + header := renderTemplate(cfg.PromptCompletionExtraHeader, map[string]string{"context": extra}) + if strings.TrimSpace(header) == "" { + header = extra + } + msgs = append(msgs, llm.Message{Role: "user", Content: header}) + } + // Then add history (which ends with the current prompt) + msgs = append(msgs, history...) + return msgs +} diff --git a/internal/lsp/chat_history_test.go b/internal/lsp/chat_history_test.go index 70080f3..a6d6266 100644 --- a/internal/lsp/chat_history_test.go +++ b/internal/lsp/chat_history_test.go @@ -4,19 +4,19 @@ import "testing" func TestStripTrailingTrigger(t *testing.T) { s := newTestServer() - if got := s.stripTrailingTrigger("what?"); got != "what" { + if got := s.chatSvc().stripTrailingTrigger("what?"); got != "what" { t.Fatalf("should remove trailing ?") } - if got := s.stripTrailingTrigger("what?>"); got != "what?" { + if got := s.chatSvc().stripTrailingTrigger("what?>"); got != "what?" { t.Fatalf("should drop trailing > when preceded by ?") } - if got := s.stripTrailingTrigger("ok!>"); got != "ok!" { + if got := s.chatSvc().stripTrailingTrigger("ok!>"); got != "ok!" { t.Fatalf("should drop > after !") } - if got := s.stripTrailingTrigger("note:>"); got != "note:" { + if got := s.chatSvc().stripTrailingTrigger("note:>"); got != "note:" { t.Fatalf("should drop > after :") } - if got := s.stripTrailingTrigger("go;>"); got != "go;" { + if got := s.chatSvc().stripTrailingTrigger("go;>"); got != "go;" { t.Fatalf("should drop > after ;") } } @@ -27,7 +27,7 @@ func TestBuildChatHistory_OrderAndLimit(t *testing.T) { // Conversation: q1, > a1, blank, q2, > a2 lines, then current prompt doc := "q1\n> a1\n\nq2\n> a2\n\n" s.setDocument(uri, doc) - msgs := s.buildChatHistory(uri, 5, "q3") + msgs := s.chatSvc().buildChatHistory(uri, 5, "q3") // Expect: user q1, assistant a1, user q2, assistant a2, user q3 if len(msgs) != 5 || msgs[0].Role != "user" || msgs[1].Role != "assistant" || msgs[2].Role != "user" || msgs[3].Role != "assistant" || msgs[4].Role != "user" { t.Fatalf("unexpected roles: %+v", msgs) diff --git a/internal/lsp/chat_no_double_answer_test.go b/internal/lsp/chat_no_double_answer_test.go index 04196f8..68ad16f 100644 --- a/internal/lsp/chat_no_double_answer_test.go +++ b/internal/lsp/chat_no_double_answer_test.go @@ -15,7 +15,7 @@ func TestDetectAndHandleChat_NoDoubleAnswer(t *testing.T) { uri := "file:///x.go" // Question line with trigger, followed by an existing answer line starting with '>' s.setDocument(uri, "What?>\n> already answered\n") - s.detectAndHandleChat(uri) + s.chatSvc().detectAndHandleChat(uri) if out.Len() != 0 { t.Fatalf("expected no applyEdit request when answer exists; got %d bytes", out.Len()) } diff --git a/internal/lsp/chat_prompt_test.go b/internal/lsp/chat_prompt_test.go index 3a1146f..dbc6158 100644 --- a/internal/lsp/chat_prompt_test.go +++ b/internal/lsp/chat_prompt_test.go @@ -18,7 +18,7 @@ func TestDetectAndHandleChat_UsesConfiguredSystemPrompt(t *testing.T) { s.out = &out // Line that should trigger chat: ends with '>' and previous char in prefixes s.setDocument(uri, "help?>\n") - s.detectAndHandleChat(uri) + s.chatSvc().detectAndHandleChat(uri) // Wait for the background chat goroutine to finish. s.inflight.Wait() if len(cap.msgs) == 0 { diff --git a/internal/lsp/chat_service.go b/internal/lsp/chat_service.go new file mode 100644 index 0000000..fd39e14 --- /dev/null +++ b/internal/lsp/chat_service.go @@ -0,0 +1,52 @@ +package lsp + +import ( + "sync" + "time" +) + +// chatService owns the in-editor chat subsystem that used to be inlined on +// Server. It tracks editor input activity (used by the completion debounce +// gate) and reaches back into the Server (via srv) for shared infrastructure +// such as configuration, LLM clients, document access and edit dispatch. +// +// Pulling this out of Server keeps the chat detection/history/command logic +// cohesive and gives the input-activity clock its own small mutex instead of +// piggy-backing on Server.mu. +type chatService struct { + srv *Server + + activityMu sync.RWMutex + lastInput time.Time +} + +// newChatService constructs the chat subsystem bound to srv. +func newChatService(srv *Server) *chatService { + return &chatService{srv: srv} +} + +// markActivity records that the editor just sent input. The completion +// debounce gate uses this timestamp to decide how long to wait before issuing +// an LLM request. +func (c *chatService) markActivity() { + c.activityMu.Lock() + c.lastInput = time.Now() + c.activityMu.Unlock() +} + +// lastActivity returns the most recent input timestamp (zero if none yet). +func (c *chatService) lastActivity() time.Time { + c.activityMu.RLock() + defer c.activityMu.RUnlock() + return c.lastInput +} + +// chatSvc returns the chat subsystem, lazily constructing it for the bare +// Server literals used in some tests. Production code always has it wired up +// by NewServer. +func (s *Server) chatSvc() *chatService { + if s.chat == nil { + s.chat = newChatService(s) + } + return s.chat +} diff --git a/internal/lsp/chat_trigger_suppression_test.go b/internal/lsp/chat_trigger_suppression_test.go index 852f955..8cfd178 100644 --- a/internal/lsp/chat_trigger_suppression_test.go +++ b/internal/lsp/chat_trigger_suppression_test.go @@ -7,13 +7,13 @@ func TestCompletionSuppressedOnChatTriggerEOL(t *testing.T) { s := newTestServer() s.cfg.MaxTokens = 32 s.cfg.TriggerCharacters = []string{".", ":", "/", "_"} - s.compCache = make(map[string]string) + s.completion.compCache = make(map[string]string) initServerDefaults(s) s.llmClient = &countingLLM{} tests := []string{"What now?>", "Explain!>", "Refactor:>", "note ;>"} for i, line := range tests { p := CompletionParams{Position: Position{Line: 0, Character: len(line)}, TextDocument: TextDocumentIdentifier{URI: "file://chat-suppr.go"}} - items, ok, _ := s.tryLLMCompletion(p, "", line, "", "", "", false, "") + items, ok, _ := s.completion.tryLLMCompletion(p, "", line, "", "", "", false, "") if !ok { t.Fatalf("case %d: expected ok=true", i) } diff --git a/internal/lsp/completion_cache_test.go b/internal/lsp/completion_cache_test.go index ff85906..ee51016 100644 --- a/internal/lsp/completion_cache_test.go +++ b/internal/lsp/completion_cache_test.go @@ -25,7 +25,7 @@ func TestCompletionCache_IgnoresWhitespaceBeforeCursor(t *testing.T) { // First request with trailing spaces before cursor line := "foo " p := CompletionParams{Position: Position{Line: 0, Character: len(line)}, TextDocument: TextDocumentIdentifier{URI: "file://x.go"}} - items, ok, _ := s.tryLLMCompletion(p, "", line, "", "", "", false, "") + items, ok, _ := s.completion.tryLLMCompletion(p, "", line, "", "", "", false, "") if !ok || len(items) == 0 || fake.calls != 1 { t.Fatalf("expected first call to invoke LLM; ok=%v len=%d calls=%d", ok, len(items), fake.calls) } @@ -33,7 +33,7 @@ func TestCompletionCache_IgnoresWhitespaceBeforeCursor(t *testing.T) { // Same logical context but with a different amount of trailing whitespace line2 := "foo " p2 := CompletionParams{Position: Position{Line: 0, Character: len(line2)}, TextDocument: TextDocumentIdentifier{URI: "file://x.go"}} - items2, ok2, _ := s.tryLLMCompletion(p2, "", line2, "", "", "", false, "") + items2, ok2, _ := s.completion.tryLLMCompletion(p2, "", line2, "", "", "", false, "") if !ok2 || len(items2) == 0 { t.Fatalf("expected cache hit to still return items") } diff --git a/internal/lsp/completion_codex_path_test.go b/internal/lsp/completion_codex_path_test.go index 6ee8c97..a985528 100644 --- a/internal/lsp/completion_codex_path_test.go +++ b/internal/lsp/completion_codex_path_test.go @@ -42,13 +42,13 @@ func TestTryLLMCompletion_PrefersCodeCompleterOverChat(t *testing.T) { s := newTestServer() s.cfg.MaxTokens = 32 s.cfg.TriggerCharacters = []string{"."} - s.compCache = make(map[string]string) + s.completion.compCache = make(map[string]string) initServerDefaults(s) fake := &fakeCodeLLM{result: "DoThing()"} s.llmClient = fake line := "obj." p := CompletionParams{Position: Position{Line: 0, Character: len(line)}, TextDocument: TextDocumentIdentifier{URI: "file://x.go"}} - items, ok, _ := s.tryLLMCompletion(p, "", line, "", "", "", false, "") + items, ok, _ := s.completion.tryLLMCompletion(p, "", line, "", "", "", false, "") if !ok || len(items) == 0 { t.Fatalf("expected completion items via CodeCompleter path") } @@ -64,13 +64,13 @@ func TestTryLLMCompletion_FallsBackToChatOnCodeCompleterError(t *testing.T) { s := newTestServer() s.cfg.MaxTokens = 32 s.cfg.TriggerCharacters = []string{"."} - s.compCache = make(map[string]string) + s.completion.compCache = make(map[string]string) initServerDefaults(s) fake := &fakeCodeLLM{result: "DoThing()", codeErr: errors.New("boom")} s.llmClient = fake line := "obj." p := CompletionParams{Position: Position{Line: 0, Character: len(line)}, TextDocument: TextDocumentIdentifier{URI: "file://y.go"}} - items, ok, _ := s.tryLLMCompletion(p, "", line, "", "", "", false, "") + items, ok, _ := s.completion.tryLLMCompletion(p, "", line, "", "", "", false, "") if !ok { t.Fatalf("expected ok=true even on fallback path") } diff --git a/internal/lsp/completion_helpers_more_test.go b/internal/lsp/completion_helpers_more_test.go index 79d2523..9e4cdac 100644 --- a/internal/lsp/completion_helpers_more_test.go +++ b/internal/lsp/completion_helpers_more_test.go @@ -26,10 +26,10 @@ func TestShouldSuppressForChatTriggerEOL(t *testing.T) { s := newTestServer() p := CompletionParams{TextDocument: TextDocumentIdentifier{URI: "file:///x"}, Position: Position{Line: 0, Character: 10}} line := "say hi;>" - if !s.shouldSuppressForChatTriggerEOL(line, p) { + if !s.completion.shouldSuppressForChatTriggerEOL(line, p) { t.Fatalf("expected suppression when ;> at EOL") } - if s.shouldSuppressForChatTriggerEOL("plain>", p) { + if s.completion.shouldSuppressForChatTriggerEOL("plain>", p) { t.Fatalf("should not suppress for plain >") } } @@ -37,15 +37,15 @@ func TestShouldSuppressForChatTriggerEOL(t *testing.T) { func TestPrefixHeuristicAllows(t *testing.T) { s := newTestServer() // inline prompt allows zero prefix - if !s.prefixHeuristicAllows(true, "", CompletionParams{Position: Position{Line: 0, Character: 0}}, false) { + if !s.completion.prefixHeuristicAllows(true, "", CompletionParams{Position: Position{Line: 0, Character: 0}}, false) { t.Fatalf("inline prompt should allow") } // structural triggers like '.' allow without prefix - if !s.prefixHeuristicAllows(false, "fmt.", CompletionParams{Position: Position{Line: 0, Character: 4}}, false) { + if !s.completion.prefixHeuristicAllows(false, "fmt.", CompletionParams{Position: Position{Line: 0, Character: 4}}, false) { t.Fatalf("dot trigger should allow") } // otherwise need at least minimal prefix (default min=1) - if s.prefixHeuristicAllows(false, " ", CompletionParams{Position: Position{Line: 0, Character: 0}}, false) { + if s.completion.prefixHeuristicAllows(false, " ", CompletionParams{Position: Position{Line: 0, Character: 0}}, false) { t.Fatalf("should not allow with no prefix") } } diff --git a/internal/lsp/completion_messages_test.go b/internal/lsp/completion_messages_test.go index bc02645..cd42155 100644 --- a/internal/lsp/completion_messages_test.go +++ b/internal/lsp/completion_messages_test.go @@ -7,7 +7,7 @@ import ( func TestBuildCompletionMessages_InlinePromptOverridesSys(t *testing.T) { s := newTestServer() p := CompletionParams{TextDocument: TextDocumentIdentifier{URI: "file:///x"}, Position: Position{Line: 0, Character: 1}} - msgs := s.buildCompletionMessages(true, false, "", false, p, "above", "current", "below", "func f") + msgs := s.completion.buildCompletionMessages(true, false, "", false, p, "above", "current", "below", "func f") if len(msgs) < 2 { t.Fatalf("expected messages") } @@ -22,7 +22,7 @@ func TestBuildCompletionMessages_InlinePromptOverridesSys(t *testing.T) { func TestBuildCompletionMessages_ExtraContextIncluded(t *testing.T) { s := newTestServer() p := CompletionParams{TextDocument: TextDocumentIdentifier{URI: "file:///x"}, Position: Position{Line: 0, Character: 1}} - msgs := s.buildCompletionMessages(false, true, "EXTRA", false, p, "a", "b", "c", "f") + msgs := s.completion.buildCompletionMessages(false, true, "EXTRA", false, p, "a", "b", "c", "f") found := false for _, m := range msgs { if m.Role == "user" && contains(m.Content, "Additional context:") { @@ -40,11 +40,11 @@ func TestPrefixHeuristic_AllVariants(t *testing.T) { s.cfg.ManualInvokeMinPrefix = 2 cur := "a" p := CompletionParams{Position: Position{Line: 0, Character: 1}} - if s.prefixHeuristicAllows(false, cur, p, true) { + if s.completion.prefixHeuristicAllows(false, cur, p, true) { t.Fatalf("should require >=2 prefix on manual invoke") } // structural triggers allow without prefix - if !s.prefixHeuristicAllows(false, "fmt.", CompletionParams{Position: Position{Line: 0, Character: 4}}, false) { + if !s.completion.prefixHeuristicAllows(false, "fmt.", CompletionParams{Position: Position{Line: 0, Character: 4}}, false) { t.Fatalf("dot trigger should allow") } } @@ -52,7 +52,7 @@ func TestPrefixHeuristic_AllVariants(t *testing.T) { func TestBuildDocString_Contents(t *testing.T) { s := newTestServer() p := CompletionParams{TextDocument: TextDocumentIdentifier{URI: "file:///x"}, Position: Position{Line: 3, Character: 7}} - got := s.buildDocString(p, "above", "current", "below", "func ctx") + got := s.completion.buildDocString(p, "above", "current", "below", "func ctx") if !contains(got, "file: file:///x") || !contains(got, "line: 3") || !contains(got, "function: func ctx") { t.Fatalf("unexpected doc string: %q", got) } @@ -61,7 +61,7 @@ func TestBuildDocString_Contents(t *testing.T) { func TestBuildCompletionMessages_InParams_UsesParamPrompts(t *testing.T) { s := newTestServer() p := CompletionParams{TextDocument: TextDocumentIdentifier{URI: "file:///x"}, Position: Position{Line: 0, Character: 5}} - msgs := s.buildCompletionMessages(false, false, "", true, p, "a", "func f(x)", "c", "func f(x)") + msgs := s.completion.buildCompletionMessages(false, false, "", true, p, "a", "func f(x)", "c", "func f(x)") if len(msgs) < 2 || msgs[0].Role != "system" || msgs[1].Role != "user" { t.Fatalf("unexpected messages") } @@ -73,12 +73,12 @@ func TestBuildCompletionMessages_InParams_UsesParamPrompts(t *testing.T) { func TestPostProcessCompletion_CodeFencesAndDuplicates(t *testing.T) { s := newTestServer() // code fences - cleaned := s.postProcessCompletion("```go\nname := value\n```", "", "") + cleaned := s.completion.postProcessCompletion("```go\nname := value\n```", "", "") if cleaned == "" { t.Fatalf("expected non-empty after fence removal") } // duplicate assignment prefix strip - cleaned2 := s.postProcessCompletion("name := other", "name := ", "name := ") + cleaned2 := s.completion.postProcessCompletion("name := other", "name := ", "name := ") if cleaned2 == "" || cleaned2 == "name := other" { t.Fatalf("expected duplicate assignment prefix stripped: %q", cleaned2) } diff --git a/internal/lsp/completion_prefix_strip_test.go b/internal/lsp/completion_prefix_strip_test.go index c8e2bd7..33eef7b 100644 --- a/internal/lsp/completion_prefix_strip_test.go +++ b/internal/lsp/completion_prefix_strip_test.go @@ -42,7 +42,7 @@ func TestStripDuplicateAssignmentPrefix_AssignAndWalrus(t *testing.T) { func TestTryLLMCompletion_ManualInvokeAfterWhitespace_Allows(t *testing.T) { s := newTestServer() - s.compCache = make(map[string]string) + s.completion.compCache = make(map[string]string) cfg := s.cfg cfg.MaxTokens = 32 cfg.TriggerCharacters = []string{".", ":", "/", "_"} @@ -52,7 +52,7 @@ func TestTryLLMCompletion_ManualInvokeAfterWhitespace_Allows(t *testing.T) { p := CompletionParams{Position: Position{Line: 0, Character: len(line)}, TextDocument: TextDocumentIdentifier{URI: "file://x.go"}} // Simulate manual user invocation (TriggerKind=1) p.Context = json.RawMessage([]byte(`{"triggerKind":1}`)) - items, ok, _ := s.tryLLMCompletion(p, "", line, "", "", "", false, "") + items, ok, _ := s.completion.tryLLMCompletion(p, "", line, "", "", "", false, "") if !ok { t.Fatalf("expected ok=true for manual invoke after whitespace") } @@ -63,7 +63,7 @@ func TestTryLLMCompletion_ManualInvokeAfterWhitespace_Allows(t *testing.T) { func TestTryLLMCompletion_InlinePromptAlwaysTriggers(t *testing.T) { s := newTestServer() - s.compCache = make(map[string]string) + s.completion.compCache = make(map[string]string) cfg := s.cfg cfg.MaxTokens = 32 cfg.TriggerCharacters = []string{".", ":", "/", "_"} @@ -72,7 +72,7 @@ func TestTryLLMCompletion_InlinePromptAlwaysTriggers(t *testing.T) { line := "prefix >!do something> suffix" // No trigger char immediately before cursor; place cursor at end p := CompletionParams{Position: Position{Line: 0, Character: len(line)}, TextDocument: TextDocumentIdentifier{URI: "file://inline.go"}} - items, ok, _ := s.tryLLMCompletion(p, "", line, "", "", "", false, "") + items, ok, _ := s.completion.tryLLMCompletion(p, "", line, "", "", "", false, "") if !ok || len(items) == 0 { t.Fatalf("expected completion to trigger on inline >!text> prompt") } @@ -80,7 +80,7 @@ func TestTryLLMCompletion_InlinePromptAlwaysTriggers(t *testing.T) { func TestTryLLMCompletion_DoubleOpenEmpty_DoesNotAutoTrigger(t *testing.T) { s := newTestServer() - s.compCache = make(map[string]string) + s.completion.compCache = make(map[string]string) cfg := s.cfg cfg.MaxTokens = 32 cfg.TriggerCharacters = []string{".", ":", "/", "_"} @@ -89,7 +89,7 @@ func TestTryLLMCompletion_DoubleOpenEmpty_DoesNotAutoTrigger(t *testing.T) { s.llmClient = fake line := ">>! " // empty content after double-open should not force-trigger p := CompletionParams{Position: Position{Line: 0, Character: len(line)}, TextDocument: TextDocumentIdentifier{URI: "file://empty-inline.go"}} - items, ok, _ := s.tryLLMCompletion(p, "", line, "", "", "", false, "") + items, ok, _ := s.completion.tryLLMCompletion(p, "", line, "", "", "", false, "") if !ok { t.Fatalf("expected ok=true for non-trigger path") } @@ -118,7 +118,7 @@ func TestHasDoubleSemicolonTrigger_Variants(t *testing.T) { func TestBareDoubleOpenPreventsAutoTriggerEvenWithOtherTriggers(t *testing.T) { s := newTestServer() - s.compCache = make(map[string]string) + s.completion.compCache = make(map[string]string) cfg := s.cfg cfg.MaxTokens = 32 cfg.TriggerCharacters = []string{".", ":", "/", "_"} @@ -128,7 +128,7 @@ func TestBareDoubleOpenPreventsAutoTriggerEvenWithOtherTriggers(t *testing.T) { // Place a '.' earlier but also include bare double-open at end; should not auto-trigger line := "obj. call >>!" p := CompletionParams{Position: Position{Line: 0, Character: len(line)}, TextDocument: TextDocumentIdentifier{URI: "file://bare-ds.go"}} - items, ok, _ := s.tryLLMCompletion(p, "", line, "", "", "", false, "") + items, ok, _ := s.completion.tryLLMCompletion(p, "", line, "", "", "", false, "") if !ok { t.Fatalf("expected ok=true (handled), but not auto-triggering") } @@ -142,7 +142,7 @@ func TestBareDoubleOpenPreventsAutoTriggerEvenWithOtherTriggers(t *testing.T) { func TestBareDoubleOpenOnNextLine_PreventsAutoTrigger(t *testing.T) { s := newTestServer() - s.compCache = make(map[string]string) + s.completion.compCache = make(map[string]string) cfg := s.cfg cfg.MaxTokens = 32 cfg.TriggerCharacters = []string{".", ":", "/", "_"} @@ -152,7 +152,7 @@ func TestBareDoubleOpenOnNextLine_PreventsAutoTrigger(t *testing.T) { current := "expression := flag.String(\"expression\", \"\", \"Expression to evaluate\")" below := ">>!" p := CompletionParams{Position: Position{Line: 0, Character: len(current)}, TextDocument: TextDocumentIdentifier{URI: "file://nextline.go"}} - items, ok, _ := s.tryLLMCompletion(p, "", current, below, "", "", false, "") + items, ok, _ := s.completion.tryLLMCompletion(p, "", current, below, "", "", false, "") if !ok { t.Fatalf("expected ok=true handled") } @@ -166,7 +166,7 @@ func TestBareDoubleOpenOnNextLine_PreventsAutoTrigger(t *testing.T) { func TestBareDoubleOpenPreventsManualInvoke(t *testing.T) { s := newTestServer() - s.compCache = make(map[string]string) + s.completion.compCache = make(map[string]string) cfg := s.cfg cfg.MaxTokens = 32 cfg.TriggerCharacters = []string{".", ":", "/", "_"} @@ -177,7 +177,7 @@ func TestBareDoubleOpenPreventsManualInvoke(t *testing.T) { p := CompletionParams{Position: Position{Line: 0, Character: len(line)}, TextDocument: TextDocumentIdentifier{URI: "file://bare-ds-manual.go"}} // Simulate manual invoke p.Context = json.RawMessage([]byte(`{"triggerKind":1}`)) - items, ok, _ := s.tryLLMCompletion(p, "", line, "", "", "", false, "") + items, ok, _ := s.completion.tryLLMCompletion(p, "", line, "", "", "", false, "") if !ok { t.Fatalf("expected ok=true (handled)") } diff --git a/internal/lsp/completion_provider_fallback_test.go b/internal/lsp/completion_provider_fallback_test.go index 67dc78b..8e35108 100644 --- a/internal/lsp/completion_provider_fallback_test.go +++ b/internal/lsp/completion_provider_fallback_test.go @@ -41,7 +41,7 @@ func TestCompletion_FallbackOnProviderError(t *testing.T) { // Call handleCompletion and ensure it returns at least one item from chat fallback var buf nopWriter s.out = &buf - s.handleCompletion(Request{JSONRPC: "2.0", ID: json.RawMessage("6"), Method: "textDocument/completion", Params: mustJSON(p)}) + s.completion.handleCompletion(Request{JSONRPC: "2.0", ID: json.RawMessage("6"), Method: "textDocument/completion", Params: mustJSON(p)}) // No panic implies path executed; detailed decode not needed here } diff --git a/internal/lsp/completion_service.go b/internal/lsp/completion_service.go new file mode 100644 index 0000000..5bfa131 --- /dev/null +++ b/internal/lsp/completion_service.go @@ -0,0 +1,76 @@ +package lsp + +import ( + "context" +) + +// completionService owns the entire code-completion subsystem that used to be +// scattered across Server. It bundles the completion cache/throttle state +// (completionState) with the request-handling logic, and reaches back into the +// Server (via srv) for shared infrastructure such as configuration, LLM +// clients, document access and stats counters. +// +// Splitting this out of Server keeps the completion logic cohesive and lets the +// completionState mutex stay private to this subsystem, independent of +// Server.mu (the two locks are never held simultaneously). +type completionService struct { + srv *Server + completionState +} + +// newCompletionService constructs the completion subsystem bound to srv. +func newCompletionService(srv *Server) *completionService { + return &completionService{ + srv: srv, + completionState: newCompletionState(), + } +} + +// waitForThrottle gates completion (and chat) LLM calls using the configured +// throttle interval. It lives here because the throttle clock is part of the +// completion state. +func (cs *completionService) waitForThrottle(ctx context.Context) bool { + return cs.completionState.waitForThrottle(ctx, cs.srv.completionThrottle()) +} + +// completionSvc returns the completion subsystem, lazily constructing it for +// the bare Server literals used in some tests. Production code always has it +// wired up by NewServer. +func (s *Server) completionSvc() *completionService { + if s.completion == nil { + s.completion = newCompletionService(s) + } + return s.completion +} + +// --- Server delegation shims --------------------------------------------- +// These keep existing Server call sites (and tests) working while the real +// state and behavior live on completionService. + +func (s *Server) storePendingCompletion(key string, items []CompletionItem) { + s.completionSvc().storePendingCompletion(key, items) +} + +func (s *Server) setCompletionsDisabled(disabled bool) bool { + return s.completionSvc().setCompletionsDisabled(disabled) +} + +func (s *Server) completionDisabled() bool { + return s.completionSvc().completionDisabled() +} + +func (s *Server) takePendingCompletion(key string) []CompletionItem { + return s.completionSvc().takePendingCompletion(key) +} + +func (s *Server) completionCacheGet(key string) (string, bool) { + return s.completionSvc().cacheGet(key) +} + +func (s *Server) completionCachePut(key, value string) { + s.completionSvc().cachePut(key, value) +} + +func (s *Server) waitForThrottle(ctx context.Context) bool { + return s.completionSvc().waitForThrottle(ctx) +} diff --git a/internal/lsp/completion_state.go b/internal/lsp/completion_state.go index 5c2716f..04bab58 100644 --- a/internal/lsp/completion_state.go +++ b/internal/lsp/completion_state.go @@ -141,31 +141,3 @@ func (s *completionState) waitForThrottle(ctx context.Context, interval time.Dur return true } } - -func (s *Server) storePendingCompletion(key string, items []CompletionItem) { - s.completionState.storePendingCompletion(key, items) -} - -func (s *Server) setCompletionsDisabled(disabled bool) bool { - return s.completionState.setCompletionsDisabled(disabled) -} - -func (s *Server) completionDisabled() bool { - return s.completionState.completionDisabled() -} - -func (s *Server) takePendingCompletion(key string) []CompletionItem { - return s.completionState.takePendingCompletion(key) -} - -func (s *Server) completionCacheGet(key string) (string, bool) { - return s.completionState.cacheGet(key) -} - -func (s *Server) completionCachePut(key, value string) { - s.completionState.cachePut(key, value) -} - -func (s *Server) waitForThrottle(ctx context.Context) bool { - return s.completionState.waitForThrottle(ctx, s.completionThrottle()) -} diff --git a/internal/lsp/completion_toggle_test.go b/internal/lsp/completion_toggle_test.go index 57ee1fd..b0366ca 100644 --- a/internal/lsp/completion_toggle_test.go +++ b/internal/lsp/completion_toggle_test.go @@ -18,7 +18,7 @@ func TestHandleCompletionRespectsDisableCommand(t *testing.T) { params := CompletionParams{TextDocument: TextDocumentIdentifier{URI: "file:///test.go"}, Position: Position{Line: 0, Character: 0}} req := Request{JSONRPC: "2.0", ID: json.RawMessage("1"), Method: "textDocument/completion", Params: mustJSON(params)} - s.handleCompletion(req) + s.completion.handleCompletion(req) payload := buf.String() parts := strings.SplitN(payload, "\r\n\r\n", 2) diff --git a/internal/lsp/debounce_throttle_more_test.go b/internal/lsp/debounce_throttle_more_test.go index 22d1888..a3c4b11 100644 --- a/internal/lsp/debounce_throttle_more_test.go +++ b/internal/lsp/debounce_throttle_more_test.go @@ -11,11 +11,9 @@ func TestWaitForDebounce_WaitsRoughlyDebounce(t *testing.T) { cfg := s.cfg cfg.CompletionDebounceMs = 20 s.cfg = cfg - s.mu.Lock() - s.lastInput = time.Now() - s.mu.Unlock() + s.chatSvc().markActivity() start := time.Now() - s.waitForDebounce(context.Background()) + s.completionSvc().waitForDebounce(context.Background()) if elapsed := time.Since(start); elapsed < 15*time.Millisecond { t.Fatalf("debounce did not wait long enough: %v", elapsed) } @@ -26,9 +24,10 @@ func TestWaitForThrottle_WaitsRoughlyInterval(t *testing.T) { cfg := s.cfg cfg.CompletionThrottleMs = 20 s.cfg = cfg - s.stateMu.Lock() - s.lastLLMCall = time.Now() - s.stateMu.Unlock() + cs := s.completionSvc() + cs.stateMu.Lock() + cs.lastLLMCall = time.Now() + cs.stateMu.Unlock() start := time.Now() if !s.waitForThrottle(context.Background()) { t.Fatalf("waitForThrottle returned false") diff --git a/internal/lsp/debounce_throttle_test.go b/internal/lsp/debounce_throttle_test.go index 7efd439..d02bdb8 100644 --- a/internal/lsp/debounce_throttle_test.go +++ b/internal/lsp/debounce_throttle_test.go @@ -21,7 +21,7 @@ func (t *timeLLM) DefaultModel() string { return "m" } func TestCompletionDebounce_WaitsUntilQuiet(t *testing.T) { s := newTestServer() - s.compCache = make(map[string]string) + s.completion.compCache = make(map[string]string) cfg := s.cfg cfg.TriggerCharacters = []string{".", ":", "/", "_"} cfg.MaxTokens = 32 @@ -37,7 +37,7 @@ func TestCompletionDebounce_WaitsUntilQuiet(t *testing.T) { p.Context = json.RawMessage([]byte(`{"triggerKind":1}`)) start := time.Now() - _, ok, _ := s.tryLLMCompletion(p, "", line, "", "", "", false, "") + _, ok, _ := s.completion.tryLLMCompletion(p, "", line, "", "", "", false, "") if !ok { t.Fatalf("expected ok=true") } @@ -51,7 +51,7 @@ func TestCompletionDebounce_WaitsUntilQuiet(t *testing.T) { func TestCompletionThrottle_SerializesCalls(t *testing.T) { s := newTestServer() - s.compCache = make(map[string]string) + s.completion.compCache = make(map[string]string) cfg := s.cfg cfg.TriggerCharacters = []string{".", ":", "/", "_"} cfg.MaxTokens = 32 @@ -65,7 +65,7 @@ func TestCompletionThrottle_SerializesCalls(t *testing.T) { p := CompletionParams{Position: Position{Line: 0, Character: len(line)}, TextDocument: TextDocumentIdentifier{URI: "file://throttle.go"}} p.Context = json.RawMessage([]byte(`{"triggerKind":1}`)) start := time.Now() - if _, ok, _ := s.tryLLMCompletion(p, "", line, "", "", "", false, ""); !ok { + if _, ok, _ := s.completion.tryLLMCompletion(p, "", line, "", "", "", false, ""); !ok { t.Fatalf("first call expected ok=true") } if f1.t.IsZero() { @@ -74,10 +74,10 @@ func TestCompletionThrottle_SerializesCalls(t *testing.T) { // second call immediately after; should be delayed by ~interval. // Clear cache to ensure we actually call the LLM again. - s.compCache = make(map[string]string) + s.completion.compCache = make(map[string]string) f2 := &timeLLM{} s.llmClient = f2 - if _, ok, _ := s.tryLLMCompletion(p, "", line, "", "", "", false, ""); !ok { + if _, ok, _ := s.completion.tryLLMCompletion(p, "", line, "", "", "", false, ""); !ok { t.Fatalf("second call expected ok=true") } if f2.t.IsZero() { diff --git a/internal/lsp/document.go b/internal/lsp/document.go index f7d0bbe..4938925 100644 --- a/internal/lsp/document.go +++ b/internal/lsp/document.go @@ -3,7 +3,6 @@ package lsp import ( "strings" - "time" ) type document struct { @@ -25,9 +24,7 @@ func (s *Server) deleteDocument(uri string) { } func (s *Server) markActivity() { - s.mu.Lock() - s.lastInput = time.Now() - s.mu.Unlock() + s.chatSvc().markActivity() } func (s *Server) getDocument(uri string) *document { diff --git a/internal/lsp/document_test.go b/internal/lsp/document_test.go index e6fba40..849346d 100644 --- a/internal/lsp/document_test.go +++ b/internal/lsp/document_test.go @@ -38,15 +38,17 @@ func newTestServer() *Server { PromptCodeActionGoTestUser: "Function under test:\n{{function}}", }, } - return &Server{ + s := &Server{ logger: log.New(io.Discard, "", 0), docs: make(map[string]*document), cfg: cfg, codeActionSubsystem: codeActionSubsystem{ llmClientRegistry: llmClientRegistry{llmProvider: llmutils.CanonicalProvider(cfg.Provider)}, }, - completionSubsystem: completionSubsystem{completionState: completionState{}}, } + s.chat = newChatService(s) + s.completion = newCompletionService(s) + return s } func initServerDefaults(s *Server) { diff --git a/internal/lsp/handlers_completion.go b/internal/lsp/handlers_completion.go index e6d8951..fced629 100644 --- a/internal/lsp/handlers_completion.go +++ b/internal/lsp/handlers_completion.go @@ -1,4 +1,7 @@ -// Completion handlers split from handlers.go to reduce file size and isolate feature logic. +// Completion request handling for the LSP server. These are methods on +// completionService (the extracted code-completion subsystem), which owns the +// cache/throttle state and reaches back into Server (via cs.srv, aliased to s) +// for shared infrastructure such as LLM clients, document access and stats. package lsp import ( @@ -35,7 +38,8 @@ type completionJobResult struct { ok bool } -func (s *Server) handleCompletion(req Request) { +func (cs *completionService) handleCompletion(req Request) { + s := cs.srv if s.completionDisabled() { s.reply(req.ID, CompletionList{IsIncomplete: false, Items: nil}, nil) return @@ -60,14 +64,14 @@ func (s *Server) handleCompletion(req Request) { logging.Logf("lsp ", "completion trigger kind=%d char=%q uri=%s line=%d char=%d", tk, tch, p.TextDocument.URI, p.Position.Line, p.Position.Character) above, current, below, funcCtx := s.lineContext(p.TextDocument.URI, p.Position) - docStr = s.buildDocString(p, above, current, below, funcCtx) + docStr = cs.buildDocString(p, above, current, below, funcCtx) if s.logContext { - s.logCompletionContext(p, above, current, below, funcCtx) + cs.logCompletionContext(p, above, current, below, funcCtx) } if s.currentLLMClient() != nil { newFunc := s.isDefiningNewFunction(p.TextDocument.URI, p.Position) extra, has := s.buildAdditionalContext(newFunc, p.TextDocument.URI, p.Position) - items, ok, incomplete := s.tryLLMCompletion(p, above, current, below, funcCtx, docStr, has, extra) + items, ok, incomplete := cs.tryLLMCompletion(p, above, current, below, funcCtx, docStr, has, extra) if ok { s.reply(req.ID, CompletionList{IsIncomplete: incomplete, Items: items}, nil) return @@ -105,22 +109,23 @@ func extractTriggerInfo(p CompletionParams) (kind int, ch string) { // --- completion helpers --- -func (s *Server) buildDocString(p CompletionParams, above, current, below, funcCtx string) string { +func (cs *completionService) buildDocString(p CompletionParams, above, current, below, funcCtx string) string { return fmt.Sprintf("file: %s\nline: %d\nabove: %s\ncurrent: %s\nbelow: %s\nfunction: %s", p.TextDocument.URI, p.Position.Line, trimLen(above), trimLen(current), trimLen(below), trimLen(funcCtx)) } -func (s *Server) logCompletionContext(p CompletionParams, above, current, below, funcCtx string) { +func (cs *completionService) logCompletionContext(p CompletionParams, above, current, below, funcCtx string) { logging.Logf("lsp ", "completion ctx uri=%s line=%d char=%d above=%q current=%q below=%q function=%q", p.TextDocument.URI, p.Position.Line, p.Position.Character, trimLen(above), trimLen(current), trimLen(below), trimLen(funcCtx)) } -func (s *Server) tryLLMCompletion(p CompletionParams, above, current, below, funcCtx, docStr string, hasExtra bool, extraText string) ([]CompletionItem, bool, bool) { +func (cs *completionService) tryLLMCompletion(p CompletionParams, above, current, below, funcCtx, docStr string, hasExtra bool, extraText string) ([]CompletionItem, bool, bool) { + s := cs.srv ctx, cancel := s.requestTimeoutContext(12 * time.Second) var cancelOnce sync.Once end := func() { cancelOnce.Do(cancel) } - plan, items, handled := s.prepareCompletionPlan(p, above, current, below, funcCtx, docStr, hasExtra, extraText) + plan, items, handled := cs.prepareCompletionPlan(p, above, current, below, funcCtx, docStr, hasExtra, extraText) if handled { end() return items, true, false @@ -130,7 +135,7 @@ func (s *Server) tryLLMCompletion(p CompletionParams, above, current, below, fun end() return nil, false, false } - results, started, ok := s.startCompletionJobs(ctx, plan, specs) + results, started, ok := cs.startCompletionJobs(ctx, plan, specs) if !ok || started == 0 { end() return nil, false, false @@ -155,7 +160,7 @@ func (s *Server) tryLLMCompletion(p CompletionParams, above, current, below, fun return combined, true, false } - firstItems, ok := s.firstCompletionAndStore(results, plan.cacheKey, end) + firstItems, ok := cs.firstCompletionAndStore(results, plan.cacheKey, end) if !ok { end() return nil, false, false @@ -163,9 +168,10 @@ func (s *Server) tryLLMCompletion(p CompletionParams, above, current, below, fun return firstItems, true, true } -func (s *Server) startCompletionJobs(ctx context.Context, plan completionPlan, specs []requestSpec) (<-chan completionJobResult, int, bool) { +func (cs *completionService) startCompletionJobs(ctx context.Context, plan completionPlan, specs []requestSpec) (<-chan completionJobResult, int, bool) { + s := cs.srv results := make(chan completionJobResult, len(specs)) - s.waitForDebounce(ctx) + cs.waitForDebounce(ctx) if !s.waitForThrottle(ctx) { close(results) return results, 0, false @@ -182,7 +188,7 @@ func (s *Server) startCompletionJobs(ctx context.Context, plan completionPlan, s wg.Add(1) go func(spec requestSpec, client llm.Client) { defer wg.Done() - items, ok := s.runCompletionForSpec(ctx, plan, spec, client) + items, ok := cs.runCompletionForSpec(ctx, plan, spec, client) results <- completionJobResult{items: items, ok: ok} }(spec, client) } @@ -216,14 +222,15 @@ func collectCompletionResults(results <-chan completionJobResult) []CompletionIt return combined } -func (s *Server) firstCompletionAndStore(results <-chan completionJobResult, cacheKey string, end func()) ([]CompletionItem, bool) { +func (cs *completionService) firstCompletionAndStore(results <-chan completionJobResult, cacheKey string, end func()) ([]CompletionItem, bool) { + s := cs.srv firstCh := make(chan []CompletionItem, 1) // Track this goroutine in inflight so Run's deferred Wait() catches it // and prevents use-after-close writes on shutdown. s.inflight.Add(1) go func() { defer s.inflight.Done() - s.collectFirstCompletion(results, cacheKey, firstCh, end) + cs.collectFirstCompletion(results, cacheKey, firstCh, end) }() firstItems, ok := <-firstCh if !ok || len(firstItems) == 0 { @@ -232,7 +239,8 @@ func (s *Server) firstCompletionAndStore(results <-chan completionJobResult, cac return firstItems, true } -func (s *Server) collectFirstCompletion(results <-chan completionJobResult, cacheKey string, firstCh chan<- []CompletionItem, end func()) { +func (cs *completionService) collectFirstCompletion(results <-chan completionJobResult, cacheKey string, firstCh chan<- []CompletionItem, end func()) { + s := cs.srv defer end() combined := make([]CompletionItem, 0) firstSent := false @@ -254,7 +262,8 @@ func (s *Server) collectFirstCompletion(results <-chan completionJobResult, cach close(firstCh) } -func (s *Server) prepareCompletionPlan(p CompletionParams, above, current, below, funcCtx, docStr string, hasExtra bool, extraText string) (completionPlan, []CompletionItem, bool) { +func (cs *completionService) prepareCompletionPlan(p CompletionParams, above, current, below, funcCtx, docStr string, hasExtra bool, extraText string) (completionPlan, []CompletionItem, bool) { + s := cs.srv plan := completionPlan{ params: p, above: above, @@ -271,7 +280,7 @@ func (s *Server) prepareCompletionPlan(p CompletionParams, above, current, below logging.Logf("lsp ", "%scompletion skip=no-trigger line=%d char=%d current=%q%s", logging.AnsiYellow, p.Position.Line, p.Position.Character, trimLen(current), logging.AnsiBase) return plan, []CompletionItem{}, true } - if s.shouldSuppressForChatTriggerEOL(current, p) { + if cs.shouldSuppressForChatTriggerEOL(current, p) { return plan, []CompletionItem{}, true } plan.inParams = inParamList(current, p.Position.Character) @@ -284,14 +293,15 @@ func (s *Server) prepareCompletionPlan(p CompletionParams, above, current, below logging.Logf("lsp ", "%scompletion skip=empty-double-semicolon line=%d char=%d current=%q%s", logging.AnsiYellow, p.Position.Line, p.Position.Character, trimLen(current), logging.AnsiBase) return plan, []CompletionItem{}, true } - if !plan.inParams && !s.prefixHeuristicAllows(plan.inlinePrompt, current, p, plan.manualInvoke) { + if !plan.inParams && !cs.prefixHeuristicAllows(plan.inlinePrompt, current, p, plan.manualInvoke) { logging.Logf("lsp ", "%scompletion skip=short-prefix line=%d char=%d current=%q%s", logging.AnsiYellow, p.Position.Line, p.Position.Character, trimLen(current), logging.AnsiBase) return plan, []CompletionItem{}, true } return plan, nil, false } -func (s *Server) runCompletionForSpec(ctx context.Context, plan completionPlan, spec requestSpec, client llm.Client) ([]CompletionItem, bool) { +func (cs *completionService) runCompletionForSpec(ctx context.Context, plan completionPlan, spec requestSpec, client llm.Client) ([]CompletionItem, bool) { + s := cs.srv sortPrefix := fmt.Sprintf("%04d", spec.index) modelKey := spec.effectiveModel(client.DefaultModel()) providerKey := spec.provider @@ -307,14 +317,15 @@ func (s *Server) runCompletionForSpec(ctx context.Context, plan completionPlan, items := s.makeCompletionItems(cached, plan.inParams, plan.current, plan.params, plan.docStr, detail, sortPrefix) return items, true } - if items, ok := s.tryProviderNativeCompletion(ctx, plan, spec, client, sortPrefix); ok { + if items, ok := cs.tryProviderNativeCompletion(ctx, plan, spec, client, sortPrefix); ok { return items, true } - return s.executeChatCompletion(ctx, plan, spec, client, sortPrefix) + return cs.executeChatCompletion(ctx, plan, spec, client, sortPrefix) } -func (s *Server) executeChatCompletion(ctx context.Context, plan completionPlan, spec requestSpec, client llm.Client, sortPrefix string) ([]CompletionItem, bool) { - messages := s.buildCompletionMessages(plan.inlinePrompt, plan.hasExtra, plan.extraText, plan.inParams, plan.params, plan.above, plan.current, plan.below, plan.funcCtx) +func (cs *completionService) executeChatCompletion(ctx context.Context, plan completionPlan, spec requestSpec, client llm.Client, sortPrefix string) ([]CompletionItem, bool) { + s := cs.srv + messages := cs.buildCompletionMessages(plan.inlinePrompt, plan.hasExtra, plan.extraText, plan.inParams, plan.params, plan.above, plan.current, plan.below, plan.funcCtx) sentSize := 0 for _, m := range messages { sentSize += len(m.Content) @@ -334,7 +345,7 @@ func (s *Server) executeChatCompletion(ctx context.Cont