From 0c2994f0065090a4884b28dc27eb760db2dfaab3 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Fri, 29 Aug 2025 00:22:39 +0300 Subject: lsp: refactor dispatch to handler map; split handlers into feature files (completion, codeaction, init, document); decompose completion logic into small helpers; update review checklist --- REVIEWRESULT.md | 70 +++ internal/lsp/chat_trigger_suppression_test.go | 23 +- internal/lsp/codeaction_test.go | 129 ++-- internal/lsp/completion_cache_test.go | 64 +- internal/lsp/completion_codex_path_test.go | 92 +-- internal/lsp/completion_prefix_strip_test.go | 210 ++++--- internal/lsp/handlers.go | 848 ++------------------------ internal/lsp/handlers_codeaction.go | 214 +++++++ internal/lsp/handlers_completion.go | 306 ++++++++++ internal/lsp/handlers_document.go | 273 +++++++++ internal/lsp/handlers_helpers_test.go | 148 ++--- internal/lsp/handlers_init.go | 40 ++ internal/lsp/handlers_test.go | 270 ++++---- internal/lsp/llm_busy_test.go | 41 +- internal/lsp/server.go | 28 +- internal/lsp/testfakes_test.go | 9 +- internal/lsp/types.go | 24 +- 17 files changed, 1542 insertions(+), 1247 deletions(-) create mode 100644 REVIEWRESULT.md create mode 100644 internal/lsp/handlers_codeaction.go create mode 100644 internal/lsp/handlers_completion.go create mode 100644 internal/lsp/handlers_document.go create mode 100644 internal/lsp/handlers_init.go diff --git a/REVIEWRESULT.md b/REVIEWRESULT.md new file mode 100644 index 0000000..f89250e --- /dev/null +++ b/REVIEWRESULT.md @@ -0,0 +1,70 @@ +# Codebase Review Findings + +This document outlines the results of a codebase review for the `hexai` project, focusing on readability, maintainability, Go best practices, and adherence to the guidelines in `AGENTS.md`. + +## 1. Executive Summary + +The `hexai` codebase is well-structured, with a clear separation of concerns between the LSP server, LLM providers, and CLI components. Test coverage appears to be good for the core LLM provider logic. + +However, several key areas require attention to improve maintainability and adhere to the project's coding standards. The most critical issues are: + +- **Large, complex functions:** Several functions, particularly within the LSP message handling logic, significantly exceed the 50-line limit. This makes them difficult to read, understand, and maintain. +- **Large source files:** The primary LSP handler file (`internal/lsp/handlers.go`) has grown too large, violating the 1000-line limit. +- **Centralized request handling:** The main request loop in `internal/lsp/server.go` is a large monolithic function that dispatches all LSP messages. + +Addressing these issues by refactoring large functions and splitting up large files will significantly improve the long-term health of the codebase. + +## 2. File and Function Size Violations + +The following files and functions violate the size constraints defined in `AGENTS.md`. + +### 2.1. Files Exceeding 1000 Lines + +- **`internal/lsp/handlers.go`**: This file is significantly over the 1000-line limit. It contains the logic for many different LSP requests. + - **Recommendation**: Split this file into multiple smaller files, each responsible for a specific set of related LSP features (e.g., `handlers_completion.go`, `handlers_codeaction.go`, `handlers_commands.go`). + +### 2.2. Functions Exceeding 50 Lines + +- **`internal/lsp/server.go`**: + - `Serve()`: This function is the main request loop and is very long. + - **Recommendation**: Refactor this method. Instead of a single large `switch` statement, use a map of method names to handler functions (e.g., `map[string]func(*jsonrpc2.Request) error`). This is a common pattern in LSP servers and will make the code much cleaner and more extensible. + +- **`internal/lsp/handlers.go`**: + - `handleTextDocumentCompletion()`: This function is extremely large and complex. It handles completion requests, interacts with the LLM, manages caching, and formats the response. + - `handleCodeAction()`: This function is also very large and contains complex logic for determining available code actions. + - `handleExecuteCommand()`: This function has a large `switch` statement for dispatching different commands. + - **Recommendation**: Break down these functions into smaller, more focused helper functions. For example, `handleTextDocumentCompletion` could be split into functions for: + 1. Checking if a completion should be triggered. + 2. Fetching results from the cache. + 3. Preparing the request for the LLM. + 4. Calling the LLM and handling its response. + 5. Formatting the completion items. + +- **`Magefile.go`**: + - Several build functions are slightly over the 50-line limit. + - **Recommendation**: While less critical than the application code, consider breaking down the larger Mage functions into smaller, reusable helper functions. + +## 3. Readability and Maintainability + +- **Complex Conditionals**: Functions like `handleTextDocumentCompletion` have deeply nested `if` and `switch` statements. This makes the logic flow very difficult to follow. Refactoring into smaller functions will help flatten these conditionals. +- **Lack of Comments for Complex Logic**: While the code is generally clean, some of the more complex parts of the LSP logic (e.g., position calculations, completion context) could benefit from comments explaining the *why* behind the code. + +## 4. Testing + +- **Good Coverage for LLM Providers**: The `internal/llm` package has a good set of tests for each provider. This is excellent. +- **`handlers_test.go` is Large**: Similar to `handlers.go`, the corresponding test file is also very large. Splitting the handlers into smaller files should be mirrored in the tests. +- **Main Packages Not Tested**: The `main` functions in `cmd/hexai-lsp/main.go` and `cmd/hexai/main.go` contain some logic that is not unit tested. + - **Recommendation**: Extract the core application logic from the `main` functions into separate functions (e.g., `run() error`) in a different file/package so that it can be tested. The `internal/hexaicli/run.go` and `internal/hexailsp/run.go` files seem to be a good step in this direction. + +## 5. Go Best Practices & Conventions + +- **Error Handling**: The project follows Go's error handling conventions well. +- **Variable Naming**: The code generally uses descriptive variable names, avoiding single-letter identifiers except in idiomatic cases (e.g., loop counters). + +## 6. Summary of Recommendations + +1. [x] Refactor JSON-RPC dispatch: replace the large `switch` with a handler map. Implemented via `Server.handlers` and `handle` now dispatches through the map. +2. [x] Split `internal/lsp/handlers.go`: Extracted feature-specific files `internal/lsp/handlers_codeaction.go` and `internal/lsp/handlers_completion.go`. +3. [x] Refactor large handler functions: `handleTextDocumentCompletion` split into focused helpers (prefix heuristics, cache, provider-native path, chat path, post-processing). `handleCodeAction` already small; no `handleExecuteCommand` present. +4. [x] Mirror test structure: Feature-specific tests already exist (`codeaction_test.go`, `completion_*_test.go`); no changes needed. +5. [x] Extract logic from `main`: Entrypoints already delegate to `internal/hexailsp.Run` and `internal/hexaicli.Run`, both tested. diff --git a/internal/lsp/chat_trigger_suppression_test.go b/internal/lsp/chat_trigger_suppression_test.go index 197fbfb..55a5245 100644 --- a/internal/lsp/chat_trigger_suppression_test.go +++ b/internal/lsp/chat_trigger_suppression_test.go @@ -4,14 +4,17 @@ import "testing" // Ensure completion is suppressed when a chat trigger is at EOL (?>,!>,:>,;>) func TestCompletionSuppressedOnChatTriggerEOL(t *testing.T) { - s := &Server{ maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string) } - 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, "") - if !ok { t.Fatalf("case %d: expected ok=true", i) } - if len(items) != 0 { t.Fatalf("case %d: expected no completion items for EOL chat trigger", i) } - } + s := &Server{maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string)} + 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, "") + if !ok { + t.Fatalf("case %d: expected ok=true", i) + } + if len(items) != 0 { + t.Fatalf("case %d: expected no completion items for EOL chat trigger", i) + } + } } - diff --git a/internal/lsp/codeaction_test.go b/internal/lsp/codeaction_test.go index 59b16d8..f5abbbf 100644 --- a/internal/lsp/codeaction_test.go +++ b/internal/lsp/codeaction_test.go @@ -1,71 +1,100 @@ package lsp import ( - "context" - "encoding/json" - "testing" - "hexai/internal/llm" + "context" + "encoding/json" + "hexai/internal/llm" + "testing" ) -type fakeLLM struct{ resp string; err error } +type fakeLLM struct { + resp string + err error +} func (f fakeLLM) Chat(_ context.Context, _ []llm.Message, _ ...llm.RequestOption) (string, error) { - return f.resp, f.err + return f.resp, f.err } -func (f fakeLLM) Name() string { return "fake" } +func (f fakeLLM) Name() string { return "fake" } func (f fakeLLM) DefaultModel() string { return "fake-model" } func TestBuildRewriteCodeAction_LazyAndResolves(t *testing.T) { - s := newTestServer() - s.llmClient = fakeLLM{resp: "REWRITTEN"} - p := CodeActionParams{TextDocument: TextDocumentIdentifier{URI: "file:///t.go"}, Range: Range{Start: Position{Line: 1, Character: 2}, End: Position{Line: 3, Character: 4}}} - sel := ";rewrite;\nold code" - ca := s.buildRewriteCodeAction(p, sel) - if ca == nil { t.Fatalf("expected code action") } - // Should be lazy (no edit yet) - if ca.Edit != nil { t.Fatalf("expected nil Edit before resolve") } - if len(ca.Data) == 0 { t.Fatalf("expected data payload for lazy resolve") } - // Resolve now - resolved, ok := s.resolveCodeAction(*ca) - if !ok || resolved.Edit == nil { t.Fatalf("expected resolve to produce edit") } - edits := resolved.Edit.Changes[p.TextDocument.URI] - if len(edits) != 1 { t.Fatalf("expected 1 edit, got %d", len(edits)) } - if edits[0].Range != p.Range { t.Fatalf("edit range mismatch: got %+v want %+v", edits[0].Range, p.Range) } - if edits[0].NewText == "" { t.Fatalf("expected non-empty replacement text") } + s := newTestServer() + s.llmClient = fakeLLM{resp: "REWRITTEN"} + p := CodeActionParams{TextDocument: TextDocumentIdentifier{URI: "file:///t.go"}, Range: Range{Start: Position{Line: 1, Character: 2}, End: Position{Line: 3, Character: 4}}} + sel := ";rewrite;\nold code" + ca := s.buildRewriteCodeAction(p, sel) + if ca == nil { + t.Fatalf("expected code action") + } + // Should be lazy (no edit yet) + if ca.Edit != nil { + t.Fatalf("expected nil Edit before resolve") + } + if len(ca.Data) == 0 { + t.Fatalf("expected data payload for lazy resolve") + } + // Resolve now + resolved, ok := s.resolveCodeAction(*ca) + if !ok || resolved.Edit == nil { + t.Fatalf("expected resolve to produce edit") + } + edits := resolved.Edit.Changes[p.TextDocument.URI] + if len(edits) != 1 { + t.Fatalf("expected 1 edit, got %d", len(edits)) + } + if edits[0].Range != p.Range { + t.Fatalf("edit range mismatch: got %+v want %+v", edits[0].Range, p.Range) + } + if edits[0].NewText == "" { + t.Fatalf("expected non-empty replacement text") + } } func TestBuildRewriteCodeAction_NoInstruction(t *testing.T) { - s := newTestServer() - s.llmClient = fakeLLM{resp: "IGNORED"} - p := CodeActionParams{TextDocument: TextDocumentIdentifier{URI: "file:///t.go"}, Range: Range{}} - sel := "no instruction here" - if ca := s.buildRewriteCodeAction(p, sel); ca != nil { t.Fatalf("expected nil action when no instruction present") } + s := newTestServer() + s.llmClient = fakeLLM{resp: "IGNORED"} + p := CodeActionParams{TextDocument: TextDocumentIdentifier{URI: "file:///t.go"}, Range: Range{}} + sel := "no instruction here" + if ca := s.buildRewriteCodeAction(p, sel); ca != nil { + t.Fatalf("expected nil action when no instruction present") + } } func TestBuildDiagnosticsCodeAction_LazyAndResolves(t *testing.T) { - s := newTestServer() - s.llmClient = fakeLLM{resp: "FIXED"} - p := CodeActionParams{TextDocument: TextDocumentIdentifier{URI: "file:///t.go"}, Range: Range{Start: Position{Line: 10}, End: Position{Line: 12, Character: 5}}} - ctx := CodeActionContext{Diagnostics: []Diagnostic{ - {Range: Range{Start: Position{Line: 11}, End: Position{Line: 11, Character: 10}}, Message: "inside"}, - {Range: Range{Start: Position{Line: 2}, End: Position{Line: 3}}, Message: "outside"}, - }} - raw, _ := json.Marshal(ctx) - p.Context = json.RawMessage(raw) - sel := "some selected code" - ca := s.buildDiagnosticsCodeAction(p, sel) - if ca == nil { t.Fatalf("expected diagnostics code action") } - if ca.Edit != nil { t.Fatalf("expected lazy action without edit") } - if len(ca.Data) == 0 { t.Fatalf("expected data payload for lazy diagnostics action") } - resolved, ok := s.resolveCodeAction(*ca) - if !ok || resolved.Edit == nil { t.Fatalf("expected resolve to produce edit") } + s := newTestServer() + s.llmClient = fakeLLM{resp: "FIXED"} + p := CodeActionParams{TextDocument: TextDocumentIdentifier{URI: "file:///t.go"}, Range: Range{Start: Position{Line: 10}, End: Position{Line: 12, Character: 5}}} + ctx := CodeActionContext{Diagnostics: []Diagnostic{ + {Range: Range{Start: Position{Line: 11}, End: Position{Line: 11, Character: 10}}, Message: "inside"}, + {Range: Range{Start: Position{Line: 2}, End: Position{Line: 3}}, Message: "outside"}, + }} + raw, _ := json.Marshal(ctx) + p.Context = json.RawMessage(raw) + sel := "some selected code" + ca := s.buildDiagnosticsCodeAction(p, sel) + if ca == nil { + t.Fatalf("expected diagnostics code action") + } + if ca.Edit != nil { + t.Fatalf("expected lazy action without edit") + } + if len(ca.Data) == 0 { + t.Fatalf("expected data payload for lazy diagnostics action") + } + resolved, ok := s.resolveCodeAction(*ca) + if !ok || resolved.Edit == nil { + t.Fatalf("expected resolve to produce edit") + } } func TestBuildDiagnosticsCodeAction_NoDiagnostics(t *testing.T) { - s := newTestServer() - s.llmClient = fakeLLM{resp: "FIXED"} - p := CodeActionParams{TextDocument: TextDocumentIdentifier{URI: "file:///t.go"}, Range: Range{}} - // empty context - p.Context = json.RawMessage(nil) - if ca := s.buildDiagnosticsCodeAction(p, "sel"); ca != nil { t.Fatalf("expected nil action when no diagnostics") } + s := newTestServer() + s.llmClient = fakeLLM{resp: "FIXED"} + p := CodeActionParams{TextDocument: TextDocumentIdentifier{URI: "file:///t.go"}, Range: Range{}} + // empty context + p.Context = json.RawMessage(nil) + if ca := s.buildDiagnosticsCodeAction(p, "sel"); ca != nil { + t.Fatalf("expected nil action when no diagnostics") + } } diff --git a/internal/lsp/completion_cache_test.go b/internal/lsp/completion_cache_test.go index a350281..779f89d 100644 --- a/internal/lsp/completion_cache_test.go +++ b/internal/lsp/completion_cache_test.go @@ -1,42 +1,42 @@ package lsp import ( - "bytes" - "log" - "strings" - "testing" + "bytes" + "log" + "strings" + "testing" - "hexai/internal/logging" + "hexai/internal/logging" ) func TestCompletionCache_IgnoresWhitespaceBeforeCursor(t *testing.T) { - var buf bytes.Buffer - logger := log.New(&buf, "", 0) - s := NewServer(bytes.NewBuffer(nil), &buf, logger, ServerOptions{}) - logging.Bind(logger) - s.triggerChars = []string{" ", "."} - fake := &countingLLM{} - s.llmClient = fake + var buf bytes.Buffer + logger := log.New(&buf, "", 0) + s := NewServer(bytes.NewBuffer(nil), &buf, logger, ServerOptions{}) + logging.Bind(logger) + s.triggerChars = []string{" ", "."} + fake := &countingLLM{} + s.llmClient = fake - // 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, "") - 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) - } + // 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, "") + 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) + } - // 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, "") - if !ok2 || len(items2) == 0 { - t.Fatalf("expected cache hit to still return items") - } - if fake.calls != 1 { - t.Fatalf("expected cache hit to avoid LLM call; calls=%d", fake.calls) - } - if !strings.Contains(buf.String(), "completion cache hit") { - t.Fatalf("expected log to contain cache hit message, got: %s", buf.String()) - } + // 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, "") + if !ok2 || len(items2) == 0 { + t.Fatalf("expected cache hit to still return items") + } + if fake.calls != 1 { + t.Fatalf("expected cache hit to avoid LLM call; calls=%d", fake.calls) + } + if !strings.Contains(buf.String(), "completion cache hit") { + t.Fatalf("expected log to contain cache hit message, got: %s", buf.String()) + } } diff --git a/internal/lsp/completion_codex_path_test.go b/internal/lsp/completion_codex_path_test.go index 65ab75a..c8ce912 100644 --- a/internal/lsp/completion_codex_path_test.go +++ b/internal/lsp/completion_codex_path_test.go @@ -1,58 +1,78 @@ package lsp import ( - "context" - "errors" - "testing" + "context" + "errors" + "testing" - "hexai/internal/llm" + "hexai/internal/llm" ) // fakeCodeLLM implements both llm.Client and llm.CodeCompleter. -type fakeCodeLLM struct{ - codeCalls int - chatCalls int - result string - codeErr error +type fakeCodeLLM struct { + codeCalls int + chatCalls int + result string + codeErr error } func (f *fakeCodeLLM) CodeCompletion(_ context.Context, _ string, _ string, n int, _ string, _ float64) ([]string, error) { - f.codeCalls++ - if f.codeErr != nil { return nil, f.codeErr } - if n <= 0 { n = 1 } - out := make([]string, n) - for i := 0; i < n; i++ { out[i] = f.result } - return out, nil + f.codeCalls++ + if f.codeErr != nil { + return nil, f.codeErr + } + if n <= 0 { + n = 1 + } + out := make([]string, n) + for i := 0; i < n; i++ { + out[i] = f.result + } + return out, nil } func (f *fakeCodeLLM) Chat(_ context.Context, _ []llm.Message, _ ...llm.RequestOption) (string, error) { - f.chatCalls++ - return "chat", nil + f.chatCalls++ + return "chat", nil } func (f *fakeCodeLLM) Name() string { return "fake" } func (f *fakeCodeLLM) DefaultModel() string { return "m" } func TestTryLLMCompletion_PrefersCodeCompleterOverChat(t *testing.T) { - s := &Server{ maxTokens: 32, triggerChars: []string{"."}, compCache: make(map[string]string) } - 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, "") - if !ok || len(items) == 0 { t.Fatalf("expected completion items via CodeCompleter path") } - if fake.codeCalls == 0 { t.Fatalf("expected CodeCompletion to be called") } - if fake.chatCalls != 0 { t.Fatalf("did not expect Chat fallback when CodeCompletion succeeds") } + s := &Server{maxTokens: 32, triggerChars: []string{"."}, compCache: make(map[string]string)} + 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, "") + if !ok || len(items) == 0 { + t.Fatalf("expected completion items via CodeCompleter path") + } + if fake.codeCalls == 0 { + t.Fatalf("expected CodeCompletion to be called") + } + if fake.chatCalls != 0 { + t.Fatalf("did not expect Chat fallback when CodeCompletion succeeds") + } } func TestTryLLMCompletion_FallsBackToChatOnCodeCompleterError(t *testing.T) { - s := &Server{ maxTokens: 32, triggerChars: []string{"."}, compCache: make(map[string]string) } - 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, "") - if !ok { t.Fatalf("expected ok=true even on fallback path") } - if len(items) == 0 { t.Fatalf("expected some items from Chat fallback") } - if fake.codeCalls == 0 { t.Fatalf("expected CodeCompletion to be attempted first") } - if fake.chatCalls == 0 { t.Fatalf("expected Chat fallback to be called when CodeCompletion errors") } + s := &Server{maxTokens: 32, triggerChars: []string{"."}, compCache: make(map[string]string)} + 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, "") + if !ok { + t.Fatalf("expected ok=true even on fallback path") + } + if len(items) == 0 { + t.Fatalf("expected some items from Chat fallback") + } + if fake.codeCalls == 0 { + t.Fatalf("expected CodeCompletion to be attempted first") + } + if fake.chatCalls == 0 { + t.Fatalf("expected Chat fallback to be called when CodeCompletion errors") + } } diff --git a/internal/lsp/completion_prefix_strip_test.go b/internal/lsp/completion_prefix_strip_test.go index 9953714..64cca49 100644 --- a/internal/lsp/completion_prefix_strip_test.go +++ b/internal/lsp/completion_prefix_strip_test.go @@ -1,120 +1,158 @@ package lsp import ( - "encoding/json" - "testing" + "encoding/json" + "testing" ) func TestStripDuplicateGeneralPrefix_ExactOverlap(t *testing.T) { - prefix := "func New " - sugg := "func New() *CustData" - got := stripDuplicateGeneralPrefix(prefix, sugg) - // We expect the already typed prefix to be removed from the suggestion. - if got == sugg { - t.Fatalf("expected duplicate prefix to be stripped; got unchanged: %q", got) - } - if got != "() *CustData" { - t.Fatalf("got %q want %q", got, "() *CustData") - } + prefix := "func New " + sugg := "func New() *CustData" + got := stripDuplicateGeneralPrefix(prefix, sugg) + // We expect the already typed prefix to be removed from the suggestion. + if got == sugg { + t.Fatalf("expected duplicate prefix to be stripped; got unchanged: %q", got) + } + if got != "() *CustData" { + t.Fatalf("got %q want %q", got, "() *CustData") + } } func TestStripDuplicateGeneralPrefix_TokenBoundarySuffix(t *testing.T) { - prefix := "db." - sugg := "db.Query()" - got := stripDuplicateGeneralPrefix(prefix, sugg) - if got != "Query()" { - t.Fatalf("got %q want %q", got, "Query()") - } + prefix := "db." + sugg := "db.Query()" + got := stripDuplicateGeneralPrefix(prefix, sugg) + if got != "Query()" { + t.Fatalf("got %q want %q", got, "Query()") + } } func TestStripDuplicateAssignmentPrefix_AssignAndWalrus(t *testing.T) { - // walrus - if out := stripDuplicateAssignmentPrefix("name := ", "name := compute()" ); out != "compute()" { - t.Fatalf(":= expected compute(), got %q", out) - } - // equals - if out := stripDuplicateAssignmentPrefix("x = ", "x = y+1" ); out != "y+1" { - t.Fatalf("= expected y+1, got %q", out) - } + // walrus + if out := stripDuplicateAssignmentPrefix("name := ", "name := compute()"); out != "compute()" { + t.Fatalf(":= expected compute(), got %q", out) + } + // equals + if out := stripDuplicateAssignmentPrefix("x = ", "x = y+1"); out != "y+1" { + t.Fatalf("= expected y+1, got %q", out) + } } func TestTryLLMCompletion_ManualInvokeAfterWhitespace_Allows(t *testing.T) { - s := &Server{ maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string) } - s.llmClient = fakeLLM{resp: "() *CustData"} - line := "func fib(i int) " // cursor after space - 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, "") - if !ok { t.Fatalf("expected ok=true for manual invoke after whitespace") } - if len(items) == 0 { t.Fatalf("expected at least one completion item") } + s := &Server{maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string)} + s.llmClient = fakeLLM{resp: "() *CustData"} + line := "func fib(i int) " // cursor after space + 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, "") + if !ok { + t.Fatalf("expected ok=true for manual invoke after whitespace") + } + if len(items) == 0 { + t.Fatalf("expected at least one completion item") + } } func TestTryLLMCompletion_InlineSemicolonPromptAlwaysTriggers(t *testing.T) { - s := &Server{ maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string) } - s.llmClient = fakeLLM{resp: "replacement"} - 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, "") - if !ok || len(items) == 0 { t.Fatalf("expected completion to trigger on inline ;text; prompt") } + s := &Server{maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string)} + s.llmClient = fakeLLM{resp: "replacement"} + 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, "") + if !ok || len(items) == 0 { + t.Fatalf("expected completion to trigger on inline ;text; prompt") + } } func TestTryLLMCompletion_DoubleSemicolonEmpty_DoesNotAutoTrigger(t *testing.T) { - s := &Server{ maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string) } - fake := &countingLLM{} - s.llmClient = fake - line := ";; " // empty content after ';;' 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, "") - if !ok { t.Fatalf("expected ok=true for non-trigger path") } - if len(items) != 0 { t.Fatalf("expected no items when inline ';;' is empty") } - if fake.calls != 0 { t.Fatalf("LLM should not be called; calls=%d", fake.calls) } + s := &Server{maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string)} + fake := &countingLLM{} + s.llmClient = fake + line := ";; " // empty content after ';;' 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, "") + if !ok { + t.Fatalf("expected ok=true for non-trigger path") + } + if len(items) != 0 { + t.Fatalf("expected no items when inline ';;' is empty") + } + if fake.calls != 0 { + t.Fatalf("LLM should not be called; calls=%d", fake.calls) + } } func TestHasDoubleSemicolonTrigger_Variants(t *testing.T) { - if hasDoubleSemicolonTrigger(";;") { t.Fatalf("bare ';;' should not trigger") } - if hasDoubleSemicolonTrigger(";; ;") { t.Fatalf("';;' followed by space should not trigger") } - if hasDoubleSemicolonTrigger(";;;") { t.Fatalf("';;;' should not trigger (no content)") } - if !hasDoubleSemicolonTrigger(";;x;") { t.Fatalf("expected trigger for ';;x;' pattern") } + if hasDoubleSemicolonTrigger(";;") { + t.Fatalf("bare ';;' should not trigger") + } + if hasDoubleSemicolonTrigger(";; ;") { + t.Fatalf("';;' followed by space should not trigger") + } + if hasDoubleSemicolonTrigger(";;;") { + t.Fatalf("';;;' should not trigger (no content)") + } + if !hasDoubleSemicolonTrigger(";;x;") { + t.Fatalf("expected trigger for ';;x;' pattern") + } } func TestBareDoubleSemicolonPreventsAutoTriggerEvenWithOtherTriggers(t *testing.T) { - s := &Server{ maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string) } - fake := &countingLLM{} - s.llmClient = fake - // Place a '.' earlier but also include bare ';;' 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, "") - if !ok { t.Fatalf("expected ok=true (handled), but not auto-triggering") } - if len(items) != 0 { t.Fatalf("expected no items due to bare ';;'") } - if fake.calls != 0 { t.Fatalf("LLM should not be called; calls=%d", fake.calls) } + s := &Server{maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string)} + fake := &countingLLM{} + s.llmClient = fake + // Place a '.' earlier but also include bare ';;' 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, "") + if !ok { + t.Fatalf("expected ok=true (handled), but not auto-triggering") + } + if len(items) != 0 { + t.Fatalf("expected no items due to bare ';;'") + } + if fake.calls != 0 { + t.Fatalf("LLM should not be called; calls=%d", fake.calls) + } } func TestBareDoubleSemicolonOnNextLine_PreventsAutoTrigger(t *testing.T) { - s := &Server{ maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string) } - fake := &countingLLM{} - s.llmClient = fake - 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, "") - if !ok { t.Fatalf("expected ok=true handled") } - if len(items) != 0 { t.Fatalf("expected no items due to bare ';;' on next line") } - if fake.calls != 0 { t.Fatalf("LLM should not be called; calls=%d", fake.calls) } + s := &Server{maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string)} + fake := &countingLLM{} + s.llmClient = fake + 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, "") + if !ok { + t.Fatalf("expected ok=true handled") + } + if len(items) != 0 { + t.Fatalf("expected no items due to bare ';;' on next line") + } + if fake.calls != 0 { + t.Fatalf("LLM should not be called; calls=%d", fake.calls) + } } func TestBareDoubleSemicolonPreventsManualInvoke(t *testing.T) { - s := &Server{ maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string) } - fake := &countingLLM{} - s.llmClient = fake - line := ";;" - 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, "") - if !ok { t.Fatalf("expected ok=true (handled)") } - if len(items) != 0 { t.Fatalf("expected no items for bare ';;' even with manual invoke") } - if fake.calls != 0 { t.Fatalf("LLM should not be called; calls=%d", fake.calls) } + s := &Server{maxTokens: 32, triggerChars: []string{".", ":", "/", "_"}, compCache: make(map[string]string)} + fake := &countingLLM{} + s.llmClient = fake + line := ";;" + 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, "") + if !ok { + t.Fatalf("expected ok=true (handled)") + } + if len(items) != 0 { + t.Fatalf("expected no items for bare ';;' even with manual invoke") + } + if fake.calls != 0 { + t.Fatalf("LLM should not be called; calls=%d", fake.calls) + } } diff --git a/internal/lsp/handlers.go b/internal/lsp/handlers.go index 332344a..774a94a 100644 --- a/internal/lsp/handlers.go +++ b/internal/lsp/handlers.go @@ -3,209 +3,25 @@ package lsp import ( - "context" "encoding/json" "fmt" - "hexai/internal" "hexai/internal/llm" "hexai/internal/logging" - "os" "strings" "time" ) func (s *Server) handle(req Request) { - switch req.Method { - case "initialize": - s.handleInitialize(req) - case "initialized": - s.handleInitialized() - case "shutdown": - s.handleShutdown(req) - case "exit": - s.handleExit() - case "textDocument/didOpen": - s.handleDidOpen(req) - case "textDocument/didChange": - s.handleDidChange(req) - case "textDocument/didClose": - s.handleDidClose(req) - case "textDocument/completion": - s.handleCompletion(req) - case "textDocument/codeAction": - s.handleCodeAction(req) - case "codeAction/resolve": - s.handleCodeActionResolve(req) - default: - if len(req.ID) != 0 { - s.reply(req.ID, nil, &RespError{Code: -32601, Message: fmt.Sprintf("method not found: %s", req.Method)}) - } - } -} - -func (s *Server) handleInitialize(req Request) { - version := internal.Version - if s.llmClient != nil { - version = version + " [" + s.llmClient.Name() + ":" + s.llmClient.DefaultModel() + "]" - } - res := InitializeResult{ - Capabilities: ServerCapabilities{ - TextDocumentSync: 1, // 1 = TextDocumentSyncKindFull - CompletionProvider: &CompletionOptions{ - ResolveProvider: false, - TriggerCharacters: s.triggerChars, - }, - CodeActionProvider: CodeActionOptions{ResolveProvider: true}, - }, - ServerInfo: &ServerInfo{Name: "hexai", Version: version}, - } - s.reply(req.ID, res, nil) -} - -func (s *Server) handleCodeAction(req Request) { - var p CodeActionParams - if err := json.Unmarshal(req.Params, &p); err != nil { - if len(req.ID) != 0 { - s.reply(req.ID, []CodeAction{}, nil) - } - return - } - d := s.getDocument(p.TextDocument.URI) - if d == nil || len(d.lines) == 0 || s.llmClient == nil { - if len(req.ID) != 0 { - s.reply(req.ID, []CodeAction{}, nil) - } + if h, ok := s.handlers[req.Method]; ok { + h(req) return } - sel := extractRangeText(d, p.Range) - if strings.TrimSpace(sel) == "" { - if len(req.ID) != 0 { - s.reply(req.ID, []CodeAction{}, nil) - } - return - } - - actions := make([]CodeAction, 0, 2) - if a := s.buildRewriteCodeAction(p, sel); a != nil { - actions = append(actions, *a) - } - if a := s.buildDiagnosticsCodeAction(p, sel); a != nil { - actions = append(actions, *a) - } if len(req.ID) != 0 { - s.reply(req.ID, actions, nil) - } -} - -func (s *Server) buildRewriteCodeAction(p CodeActionParams, sel string) *CodeAction { - if instr, cleaned := instructionFromSelection(sel); strings.TrimSpace(instr) != "" { - payload := struct { - Type string `json:"type"` - URI string `json:"uri"` - Range Range `json:"range"` - Instruction string `json:"instruction"` - Selection string `json:"selection"` - }{Type: "rewrite", URI: p.TextDocument.URI, Range: p.Range, Instruction: instr, Selection: cleaned} - raw, _ := json.Marshal(payload) - ca := CodeAction{Title: "Hexai: rewrite selection", Kind: "refactor.rewrite", Data: raw} - return &ca - } - return nil -} - -func (s *Server) buildDiagnosticsCodeAction(p CodeActionParams, sel string) *CodeAction { - diags := s.diagnosticsInRange(p.Context, p.Range) - if len(diags) == 0 { - return nil + s.reply(req.ID, nil, &RespError{Code: -32601, Message: fmt.Sprintf("method not found: %s", req.Method)}) } - payload := struct { - Type string `json:"type"` - URI string `json:"uri"` - Range Range `json:"range"` - Selection string `json:"selection"` - Diagnostics []Diagnostic `json:"diagnostics"` - }{Type: "diagnostics", URI: p.TextDocument.URI, Range: p.Range, Selection: sel, Diagnostics: diags} - raw, _ := json.Marshal(payload) - ca := CodeAction{Title: "Hexai: resolve diagnostics", Kind: "quickfix", Data: raw} - return &ca } -func (s *Server) resolveCodeAction(ca CodeAction) (CodeAction, bool) { - if s.llmClient == nil || len(ca.Data) == 0 { - return ca, false - } - var payload struct { - Type string `json:"type"` - URI string `json:"uri"` - Range Range `json:"range"` - Instruction string `json:"instruction,omitempty"` - Selection string `json:"selection"` - Diagnostics []Diagnostic `json:"diagnostics,omitempty"` - } - if err := json.Unmarshal(ca.Data, &payload); err != nil { - return ca, false - } - switch payload.Type { - case "rewrite": - sys := "You are a precise code refactoring engine. Rewrite the given code strictly according to the instruction. Return only the updated code with no prose or backticks. Preserve formatting where reasonable." - user := fmt.Sprintf("Instruction: %s\n\nSelected code to transform:\n%s", payload.Instruction, payload.Selection) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - messages := []llm.Message{{Role: "system", Content: sys}, {Role: "user", Content: user}} - opts := s.llmRequestOpts() - if text, err := s.llmClient.Chat(ctx, messages, opts...); err == nil { - if out := stripCodeFences(strings.TrimSpace(text)); out != "" { - edit := WorkspaceEdit{Changes: map[string][]TextEdit{payload.URI: {{Range: payload.Range, NewText: out}}}} - ca.Edit = &edit - return ca, true - } - } else { - logging.Logf("lsp ", "codeAction rewrite llm error: %v", err) - } - case "diagnostics": - sys := "You are a precise code fixer. Resolve the given diagnostics by editing only the selected code. Return only the corrected code with no prose or backticks. Keep behavior and style, and avoid unrelated changes." - var b strings.Builder - b.WriteString("Diagnostics to resolve (selection only):\n") - for i, dgn := range payload.Diagnostics { - if dgn.Source != "" { - fmt.Fprintf(&b, "%d. [%s] %s\n", i+1, dgn.Source, dgn.Message) - } else { - fmt.Fprintf(&b, "%d. %s\n", i+1, dgn.Message) - } - } - b.WriteString("\nSelected code:\n") - b.WriteString(payload.Selection) - ctx, cancel := context.WithTimeout(context.Background(), 12*time.Second) - defer cancel() - messages := []llm.Message{{Role: "system", Content: sys}, {Role: "user", Content: b.String()}} - opts := s.llmRequestOpts() - if text, err := s.llmClient.Chat(ctx, messages, opts...); err == nil { - if out := stripCodeFences(strings.TrimSpace(text)); out != "" { - edit := WorkspaceEdit{Changes: map[string][]TextEdit{payload.URI: {{Range: payload.Range, NewText: out}}}} - ca.Edit = &edit - return ca, true - } - } else { - logging.Logf("lsp ", "codeAction diagnostics llm error: %v", err) - } - } - return ca, false -} - -func (s *Server) handleCodeActionResolve(req Request) { - var ca CodeAction - if err := json.Unmarshal(req.Params, &ca); err != nil { - if len(req.ID) != 0 { - s.reply(req.ID, ca, nil) - } - return - } - if resolved, ok := s.resolveCodeAction(ca); ok { - s.reply(req.ID, resolved, nil) - return - } - s.reply(req.ID, ca, nil) -} +// handleInitialize moved to handlers_init.go func (s *Server) llmRequestOpts() []llm.RequestOption { opts := []llm.RequestOption{llm.WithMaxTokens(s.maxTokens)} @@ -327,59 +143,7 @@ func findStrictSemicolonTag(line string) (string, int, int, bool) { // diagnosticsInRange parses the CodeAction context and returns diagnostics // that overlap the given selection range. If the context is missing or does // not contain diagnostics, returns an empty slice. -func (s *Server) diagnosticsInRange(ctxRaw json.RawMessage, sel Range) []Diagnostic { - if len(ctxRaw) == 0 { - return nil - } - var ctx CodeActionContext - if err := json.Unmarshal(ctxRaw, &ctx); err != nil { - return nil - } - if len(ctx.Diagnostics) == 0 { - return nil - } - out := make([]Diagnostic, 0, len(ctx.Diagnostics)) - for _, d := range ctx.Diagnostics { - if rangesOverlap(d.Range, sel) { - out = append(out, d) - } - } - return out -} - -// rangesOverlap reports whether two LSP ranges overlap at all. -func rangesOverlap(a, b Range) bool { - // Normalize ordering - if greaterPos(a.Start, a.End) { - a.Start, a.End = a.End, a.Start - } - if greaterPos(b.Start, b.End) { - b.Start, b.End = b.End, b.Start - } - // a ends before b starts - if lessPos(a.End, b.Start) { - return false - } - // b ends before a starts - if lessPos(b.End, a.Start) { - return false - } - return true -} - -func lessPos(p, q Position) bool { - if p.Line != q.Line { - return p.Line < q.Line - } - return p.Character < q.Character -} - -func greaterPos(p, q Position) bool { - if p.Line != q.Line { - return p.Line > q.Line - } - return p.Character > q.Character -} +// CodeAction-related handlers and helpers moved to handlers_codeaction.go // extractRangeText returns the exact text within the given document range. func extractRangeText(d *document, r Range) string { @@ -426,122 +190,33 @@ func extractRangeText(d *document, r Range) string { return b.String() } -func (s *Server) handleInitialized() { - logging.Logf("lsp ", "client initialized") -} +// handleInitialized moved to handlers_init.go -func (s *Server) handleShutdown(req Request) { - s.reply(req.ID, nil, nil) -} +// handleShutdown moved to handlers_init.go -func (s *Server) handleExit() { - s.exited = true - os.Exit(0) -} +// handleExit moved to handlers_init.go -func (s *Server) handleDidOpen(req Request) { - var p DidOpenTextDocumentParams - if err := json.Unmarshal(req.Params, &p); err == nil { - s.setDocument(p.TextDocument.URI, p.TextDocument.Text) - s.markActivity() - } -} +// handleDidOpen moved to handlers_document.go -func (s *Server) handleDidChange(req Request) { - var p DidChangeTextDocumentParams - if err := json.Unmarshal(req.Params, &p); err == nil { - if len(p.ContentChanges) > 0 { - s.setDocument(p.TextDocument.URI, p.ContentChanges[len(p.ContentChanges)-1].Text) - } - s.markActivity() - // Detect in-editor chat trigger lines and respond inline. - s.detectAndHandleChat(p.TextDocument.URI) - } -} +// handleDidChange moved to handlers_document.go -func (s *Server) handleDidClose(req Request) { - var p DidCloseTextDocumentParams - if err := json.Unmarshal(req.Params, &p); err == nil { - s.deleteDocument(p.TextDocument.URI) - s.markActivity() - } -} +// handleDidClose moved to handlers_document.go -func (s *Server) handleCompletion(req Request) { - var p CompletionParams - var docStr string - if err := json.Unmarshal(req.Params, &p); err == nil { - // Log trigger information for every completion request from client - tk, tch := extractTriggerInfo(p) - 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) - if s.logContext { - s.logCompletionContext(p, above, current, below, funcCtx) - } - if s.llmClient != nil { - newFunc := s.isDefiningNewFunction(p.TextDocument.URI, p.Position) - extra, has := s.buildAdditionalContext(newFunc, p.TextDocument.URI, p.Position) - items, ok := s.tryLLMCompletion(p, above, current, below, funcCtx, docStr, has, extra) - if ok { - s.reply(req.ID, CompletionList{IsIncomplete: false, Items: items}, nil) - return - } - } - } - items := s.fallbackCompletionItems(docStr) - s.reply(req.ID, CompletionList{IsIncomplete: false, Items: items}, nil) -} +// handleCompletion moved to handlers_completion.go func (s *Server) reply(id json.RawMessage, result any, err *RespError) { - resp := Response{JSONRPC: "2.0", ID: id, Result: result, Error: err} - s.writeMessage(resp) + resp := Response{JSONRPC: "2.0", ID: id, Result: result, Error: err} + s.writeMessage(resp) } // docBeforeAfter returns the full document text split at the given position. // The returned strings are the text before the cursor (inclusive of anything // left of the position) and the text after the cursor. -func (s *Server) docBeforeAfter(uri string, pos Position) (string, string) { - d := s.getDocument(uri) - if d == nil { return "", "" } - // Clamp indices - line := pos.Line - if line < 0 { line = 0 } - if line >= len(d.lines) { line = len(d.lines) - 1 } - col := pos.Character - if col < 0 { col = 0 } - if col > len(d.lines[line]) { col = len(d.lines[line]) } - // Build before - var b strings.Builder - for i := 0; i < line; i++ { b.WriteString(d.lines[i]); b.WriteByte('\n') } - b.WriteString(d.lines[line][:col]) - before := b.String() - // Build after - var a strings.Builder - a.WriteString(d.lines[line][col:]) - for i := line + 1; i < len(d.lines); i++ { a.WriteByte('\n'); a.WriteString(d.lines[i]) } - return before, a.String() -} +// docBeforeAfter moved to handlers_document.go // extractTriggerInfo returns the LSP completion TriggerKind and TriggerCharacter // if provided by the client; when absent it returns zeros. -func extractTriggerInfo(p CompletionParams) (kind int, ch string) { - if p.Context == nil { - return 0, "" - } - var ctx struct { - TriggerKind int `json:"triggerKind"` - TriggerCharacter string `json:"triggerCharacter,omitempty"` - } - if raw, ok := p.Context.(json.RawMessage); ok { - _ = json.Unmarshal(raw, &ctx) - } else { - b, _ := json.Marshal(p.Context) - _ = json.Unmarshal(b, &ctx) - } - return ctx.TriggerKind, ctx.TriggerCharacter -} +// extractTriggerInfo moved to handlers_completion.go // --- in-editor chat (";C ...") --- @@ -550,483 +225,86 @@ func extractTriggerInfo(p CompletionParams) (kind int, ch string) { // and no non-empty answer line yet). If found, it asks the LLM and inserts the // answer below the blank line, leaving exactly one empty line between prompt // and response. -func (s *Server) detectAndHandleChat(uri string) { - if s.llmClient == nil { - return - } - d := s.getDocument(uri) - if d == nil || len(d.lines) == 0 { - return - } - for i, raw := range d.lines { - // Find last non-space character index - j := len(raw) - 1 - for j >= 0 { - if raw[j] == ' ' || raw[j] == '\t' { - j-- - continue - } - break - } - if j < 1 { // need at least two chars for pattern like '?>' - continue - } - pair := raw[j-1 : j+1] - isTrigger := pair == "?>" || pair == "!>" || pair == ":>" || pair == ";>" - if !isTrigger { - continue - } - // Avoid double-answering: if the next non-empty line starts with '>' we skip. - k := i + 1 - for k < len(d.lines) && strings.TrimSpace(d.lines[k]) == "" { - k++ - } - if k < len(d.lines) && strings.HasPrefix(strings.TrimSpace(d.lines[k]), ">") { - continue - } - // Derive prompt by removing only the trailing '>' - removeCount := 1 - base := raw[:j+1-removeCount] - prompt := strings.TrimSpace(base) - if prompt == "" { - continue - } - lineIdx := i - lastIdx := j - go func(prompt string, remove int) { - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) - defer cancel() - sys := "You are a helpful coding assistant. Answer concisely and clearly." - // Build short conversation history from the document above this line - history := s.buildChatHistory(uri, lineIdx, prompt) - msgs := append([]llm.Message{{Role: "system", Content: sys}}, history...) - opts := s.llmRequestOpts() - logging.Logf("lsp ", "chat llm=requesting model=%s", s.llmClient.DefaultModel()) - text, err := s.llmClient.Chat(ctx, msgs, opts...) - if err != nil { - logging.Logf("lsp ", "chat llm error: %v", err) - return - } - out := strings.TrimSpace(stripCodeFences(text)) - if out == "" { - return - } - s.applyChatEdits(uri, lineIdx, lastIdx, remove, "> "+out) - }(prompt, removeCount) - // Only handle one per change tick to avoid flooding - break - } -} +// detectAndHandleChat moved to handlers_document.go // applyChatEdits removes the triggering punctuation at end of the line and // inserts two newlines followed by a new line with the response prefixed. -func (s *Server) applyChatEdits(uri string, lineIdx int, lastNonSpace int, removeCount int, response string) { - 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 - // newline so there is exactly one blank line after the reply - 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) -} +// applyChatEdits moved to handlers_document.go // buildChatHistory walks upwards from the current line to collect the most recent // Q/A pairs in the in-editor transcript. It returns messages in chronological order // ending with the current user prompt. Limits to a small number of pairs to control tokens. -func (s *Server) buildChatHistory(uri string, lineIdx int, currentPrompt string) []llm.Message { - d := s.getDocument(uri) - if d == nil { - return []llm.Message{{Role: "user", Content: currentPrompt}} - } - type pair struct { - q string - a string - } - pairs := []pair{} - i := lineIdx - 1 - // Collect up to 3 recent pairs - for i >= 0 && len(pairs) < 3 { - // Skip blank lines - for i >= 0 && strings.TrimSpace(d.lines[i]) == "" { - i-- - } - if i < 0 { - break - } - // Expect assistant reply lines starting with ">" - if !strings.HasPrefix(strings.TrimSpace(d.lines[i]), ">") { - break - } - // Collect contiguous reply block - 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 - } - // Skip a single blank line that should separate Q from A - for i >= 0 && strings.TrimSpace(d.lines[i]) == "" { - i-- - } - if i < 0 { - break - } - // Take the question as the non-empty line above - q := strings.TrimSpace(d.lines[i]) - // Remove any lingering trigger pair at end if present - q = stripTrailingTrigger(q) - pairs = append([]pair{{q: q, a: strings.Join(replyLines, "\n")}}, pairs...) - i-- - // Continue to find older pairs - } - // Build messages - 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 -} +// buildChatHistory moved to handlers_document.go // stripTrailingTrigger removes a single trailing punctuation from the set // [?,!,:] or both semicolons if present at end, mirroring the inline trigger rules. -func stripTrailingTrigger(sx string) string { - s := strings.TrimRight(sx, " \t") - // New chat triggers use a trailing '>' paired with one of ? ! : ; - if len(s) >= 2 && s[len(s)-1] == '>' { - prev := s[len(s)-2] - if prev == '?' || prev == '!' || prev == ':' || prev == ';' { - return strings.TrimRight(s[:len(s)-1], " \t") - } - } - if strings.HasSuffix(s, ";;") { // keep legacy inline ';;' cleanup used in history building - return strings.TrimRight(strings.TrimSuffix(s, ";;"), " \t") - } - if len(s) == 0 { - return sx - } - last := s[len(s)-1] - switch last { // legacy: remove one trailing punctuation for old-chat triggers - case '?', '!', ':': - return strings.TrimRight(s[:len(s)-1], " \t") - default: - return sx - } -} +// stripTrailingTrigger moved to handlers_document.go // clientApplyEdit sends a workspace/applyEdit request to the client. -func (s *Server) clientApplyEdit(label string, edit WorkspaceEdit) { - params := ApplyWorkspaceEditParams{Label: label, Edit: edit} - // Build a JSON-RPC request with a fresh id - id := s.nextReqID() - req := Request{JSONRPC: "2.0", ID: id, Method: "workspace/applyEdit"} - // marshal params separately to avoid changing Request type - b, _ := json.Marshal(params) - req.Params = b - s.writeMessage(req) -} +// clientApplyEdit moved to handlers_document.go // nextReqID returns a unique json.RawMessage id for server-initiated requests. -func (s *Server) nextReqID() json.RawMessage { - s.mu.Lock() - s.nextID++ - idNum := s.nextID - s.mu.Unlock() - b, _ := json.Marshal(idNum) - return b -} +// nextReqID moved to handlers_document.go // --- completion helpers --- -func (s *Server) 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)) -} +// buildDocString moved to handlers_completion.go -func (s *Server) 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)) -} +// logCompletionContext moved to handlers_completion.go -func (s *Server) tryLLMCompletion(p CompletionParams, above, current, below, funcCtx, docStr string, hasExtra bool, extraText string) ([]CompletionItem, bool) { - ctx, cancel := context.WithTimeout(context.Background(), 6*time.Second) - defer cancel() - // Track if we've already acquired the LLM busy lock during this call - locked := false - - // Inline prompt markers (strict ;text; or double-; patterns) explicitly allow triggering. - inlinePrompt := lineHasInlinePrompt(current) - // Only invoke LLM when triggered by our characters, manual invoke, or inline prompt markers. - if !inlinePrompt && !s.isTriggerEvent(p, current) { - 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 []CompletionItem{}, true - } - - // Suppress code completion when an in-editor chat trigger is at EOL. - // New triggers: ?> !> :> ;> (trim trailing whitespace before checking). - if t := strings.TrimRight(current, " \t"); len(t) >= 2 && t[len(t)-1] == '>' { - prev := t[len(t)-2] - if prev == '?' || prev == '!' || prev == ':' || prev == ';' { - logging.Logf("lsp ", "completion skip=chat-trigger-eol uri=%s line=%d", p.TextDocument.URI, p.Position.Line) - return []CompletionItem{}, true - } - } - - inParams := inParamList(current, p.Position.Character) - - // Detect manual invoke so we can relax prefix heuristics when user pressed completion key. - manualInvoke := false - if p.Context != nil { - var c struct { - TriggerKind int `json:"triggerKind"` - } - if raw, ok := p.Context.(json.RawMessage); ok { - _ = json.Unmarshal(raw, &c) - } else { - b, _ := json.Marshal(p.Context) - _ = json.Unmarshal(b, &c) - } - if c.TriggerKind == 1 { // Invoked - manualInvoke = true - } - } +// tryLLMCompletion moved to handlers_completion.go - // Build a cache key for this completion context (ignore trailing whitespace - // before the cursor when forming the key) and try cache before any LLM call. - key := s.completionCacheKey(p, above, current, below, funcCtx, inParams, hasExtra, extraText) - if cleaned, ok := s.completionCacheGet(key); ok && strings.TrimSpace(cleaned) != "" { - logging.Logf("lsp ", "completion cache hit uri=%s line=%d char=%d preview=%s%s%s", - p.TextDocument.URI, p.Position.Line, p.Position.Character, - logging.AnsiGreen, logging.PreviewForLog(cleaned), logging.AnsiBase) - return s.makeCompletionItems(cleaned, inParams, current, p, docStr), true - } - // If there is a bare ';;' on the current or next line (no valid ';;text;'), - // do not auto-trigger unless it was a manual invoke. - if (isBareDoubleSemicolon(current) || isBareDoubleSemicolon(below)) && !manualInvoke { - 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 []CompletionItem{}, true - } - - // Heuristic 1: Require a minimal typed identifier prefix to avoid early triggers, - // but allow immediate completion after structural trigger chars like '.', ':', '/'. - if !inParams { - // Determine the effective cursor index within current line, clamped, and - // skip over trailing spaces/tabs to support cases like "type Matrix| " - // where the cursor is after a space following an identifier. - idx := p.Position.Character - if idx > len(current) { - idx = len(current) - } - // Structural triggers allow no prefix - allowNoPrefix := false - if inlinePrompt { - allowNoPrefix = true - } - if idx > 0 { - ch := current[idx-1] - if ch == '.' || ch == ':' || ch == '/' || ch == '_' || ch == ')' { - allowNoPrefix = true - } - } - if !allowNoPrefix { - // Walk left over whitespace - j := idx - for j > 0 { - c := current[j-1] - if c == ' ' || c == '\t' { - j-- - continue - } - break - } - start := computeWordStart(current, j) - // For manual invoke, require a configurable minimum prefix length - min := 1 - if manualInvoke && s.manualInvokeMinPrefix >= 0 { - min = s.manualInvokeMinPrefix - } - if j-start < min { // require at least min identifier chars - 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 []CompletionItem{}, true - } - } - } - // Prefer provider-native code completion when available (e.g., Copilot Codex) - if cc, ok := s.llmClient.(llm.CodeCompleter); ok { - before, after := s.docBeforeAfter(p.TextDocument.URI, p.Position) - // Construct prompt/suffix similar to helix-gpt - path := strings.TrimPrefix(p.TextDocument.URI, "file://") - prompt := "// Path: " + path + "\n" + before - lang := "" - temp := 0.0 - if s.codingTemperature != nil { temp = *s.codingTemperature } - prov := "" - if s.llmClient != nil { prov = s.llmClient.Name() } - logging.Logf("lsp ", "completion path=codex provider=%s uri=%s", prov, path) - ctx2, cancel2 := context.WithTimeout(context.Background(), 8*time.Second) - defer cancel2() - // Concurrency guard - if s.isLLMBusy() { - return []CompletionItem{s.busyCompletionItem()}, true - } - s.setLLMBusy(true) - defer s.setLLMBusy(false) - locked = true - - suggestions, err := cc.CodeCompletion(ctx2, prompt, after, 1, lang, temp) - if err == nil && len(suggestions) > 0 { - cleaned := strings.TrimSpace(suggestions[0]) - if cleaned != "" { - cleaned = stripDuplicateAssignmentPrefix(current[:p.Position.Character], cleaned) - if cleaned != "" { cleaned = stripDuplicateGeneralPrefix(current[:p.Position.Character], cleaned) } - if cleaned != "" && hasDoubleSemicolonTrigger(current) { - indent := leadingIndent(current) - if indent != "" { cleaned = applyIndent(indent, cleaned) } - } - if strings.TrimSpace(cleaned) != "" { - key := s.completionCacheKey(p, above, current, below, funcCtx, inParams, hasExtra, extraText) - s.completionCachePut(key, cleaned) - return s.makeCompletionItems(cleaned, inParams, current, p, docStr), true - } - } - } else if err != nil { - logging.Logf("lsp ", "completion path=codex error=%v (falling back to chat)", err) - } - // If provider-native path failed, fall back to chat below. - } - - sysPrompt, userPrompt := buildPrompts(inParams, p, above, current, below, funcCtx) - messages := []llm.Message{ - {Role: "system", Content: sysPrompt}, - {Role: "user", Content: userPrompt}, - } - if hasExtra && extraText != "" { - messages = append(messages, llm.Message{Role: "user", Content: "Additional context:\n" + extraText}) - } - - // If an inline prompt marker is present, make the instruction stricter: code only. - if inlinePrompt { -