|
| 1 | +# Implementation Plan: terraphim_lsp Foundation (Gitea #2668) |
| 2 | + |
| 3 | +**Status**: Draft |
| 4 | +**Research Doc**: `.docs/adf/2668/research.md` |
| 5 | +**Author**: AI Agent |
| 6 | +**Date**: 2026-06-13 |
| 7 | +**Issue**: terraphim/terraphim-ai#2668 |
| 8 | +**Epic**: terraphim/terraphim-ai#2667 |
| 9 | +**Estimated Effort**: 1 hour |
| 10 | + |
| 11 | +## Overview |
| 12 | + |
| 13 | +### Summary |
| 14 | +Restore `terraphim_lsp` to a compilable workspace member by deleting its orphaned `Cargo.lock`, aligning its `Cargo.toml` with the workspace, declaring minimal KG-focused dependencies, and replacing the placeholder `lib.rs` with working module declarations and a no-op `tower-lsp` server. |
| 15 | + |
| 16 | +### Approach |
| 17 | +Make the smallest possible set of file changes so that `cargo check -p terraphim_lsp` succeeds from the workspace root. Do not implement real handlers; reserve module slots for Step 2 (`kg_analysis`) and Step 3 (`server`). |
| 18 | + |
| 19 | +### Scope |
| 20 | + |
| 21 | +**In Scope:** |
| 22 | +- Root `Cargo.toml`: remove `crates/terraphim_lsp` from `exclude`. |
| 23 | +- Delete `crates/terraphim_lsp/Cargo.lock`. |
| 24 | +- Rewrite `crates/terraphim_lsp/Cargo.toml` (edition 2024, dependencies, metadata). |
| 25 | +- Rewrite `crates/terraphim_lsp/src/lib.rs` (module declarations, no-op LSP server). |
| 26 | +- Verification commands. |
| 27 | + |
| 28 | +**Out of Scope:** |
| 29 | +- Real hover/completion/diagnostics handlers (Step 3). |
| 30 | +- KG term matching implementation (Step 2). |
| 31 | +- LSP binary target (Step 3). |
| 32 | +- CI workflow changes (Step 11). |
| 33 | + |
| 34 | +**Avoid At All Cost:** |
| 35 | +- Re-introducing EDM/negative-contribution diagnostics (historical scope, replaced by KG focus). |
| 36 | +- Adding dependencies beyond the six listed in #2668. |
| 37 | +- Implementing handler logic under the guise of "foundation". |
| 38 | + |
| 39 | +## Architecture |
| 40 | + |
| 41 | +### Component Diagram |
| 42 | +``` |
| 43 | +Workspace |
| 44 | + └── crates/terraphim_lsp |
| 45 | + ├── Cargo.toml # workspace edition, minimal deps |
| 46 | + └── src/lib.rs |
| 47 | + ├── mod kg_analysis; # placeholder for Step 2 |
| 48 | + ├── mod server; # placeholder for Step 3 |
| 49 | + └── TerraphimLspServer # no-op tower-lsp impl |
| 50 | +``` |
| 51 | + |
| 52 | +### Data Flow |
| 53 | +Not applicable for foundation step. Future flow (Step 3): |
| 54 | +``` |
| 55 | +Editor LSP request → tower-lsp → TerraphimLspServer → kg_analysis (Step 2) → LSP response |
| 56 | +``` |
| 57 | + |
| 58 | +### Key Design Decisions |
| 59 | + |
| 60 | +| Decision | Rationale | Alternatives Rejected | |
| 61 | +|----------|-----------|----------------------| |
| 62 | +| Path dependencies for core crates | `terraphim_automata`, `terraphim_types`, `terraphim_rolegraph` are local workspace members in this branch | Registry deps (unnecessary indirection) | |
| 63 | +| No-op `LanguageServer` impl | Satisfies `tower-lsp` trait without implementing Step 2/3 logic | Partial handlers (scope creep) | |
| 64 | +| Module declarations without bodies | Provides compile-time structure for Steps 2 and 3 | Inline everything in `lib.rs` (harder to review) | |
| 65 | +| Remove `terraphim-lsp` binary feature gate | Step 3 will add the binary; Step 1 does not need it | Keep historical binary config (broken without server) | |
| 66 | + |
| 67 | +### Eliminated Options (Essentialism) |
| 68 | + |
| 69 | +| Option Rejected | Why Rejected | Risk of Including | |
| 70 | +|-----------------|--------------|-------------------| |
| 71 | +| Full EDM diagnostic server | Historical scope, replaced by KG analysis in epic #2667 | Reintroduces dead code and `terraphim_negative_contribution` dependency | |
| 72 | +| Registry dependencies for core crates | Adds registry auth complexity when local paths are available | Slower builds, potential version skew | |
| 73 | +| Implement `kg_analysis.rs` in Step 1 | Blurs Step 1/2 boundary; Step 2 has its own issue (#2669) | Larger, harder-to-review PR | |
| 74 | +| Add binary target now | Needs `server.rs` implementation first | Would not compile or would be empty | |
| 75 | + |
| 76 | +### Simplicity Check |
| 77 | + |
| 78 | +**What if this could be easy?** The easiest correct foundation is: un-exclude the crate, delete the stale lockfile, add six dependencies, and write a `lib.rs` that declares modules and implements the `tower-lsp` trait with empty methods. That is exactly this plan. |
| 79 | + |
| 80 | +**Senior Engineer Test**: A senior engineer would call this appropriately minimal for a foundation step. |
| 81 | + |
| 82 | +**Nothing Speculative Checklist**: |
| 83 | +- [x] No features the user didn't request |
| 84 | +- [x] No abstractions "in case we need them later" |
| 85 | +- [x] No flexibility "just in case" |
| 86 | +- [x] No error handling for scenarios that cannot occur |
| 87 | +- [x] No premature optimization |
| 88 | + |
| 89 | +## File Changes |
| 90 | + |
| 91 | +### New Files |
| 92 | +| File | Purpose | |
| 93 | +|------|---------| |
| 94 | +| `crates/terraphim_lsp/src/kg_analysis.rs` | Module placeholder for Step 2 (empty or minimal docs) | |
| 95 | +| `crates/terraphim_lsp/src/server.rs` | Module placeholder for Step 3 (no-op server struct) | |
| 96 | + |
| 97 | +### Modified Files |
| 98 | +| File | Changes | |
| 99 | +|------|---------| |
| 100 | +| `Cargo.toml` | Remove `"crates/terraphim_lsp"` from `exclude` | |
| 101 | +| `crates/terraphim_lsp/Cargo.toml` | Edition 2024, add dependencies, keep metadata | |
| 102 | +| `crates/terraphim_lsp/src/lib.rs` | Module declarations, re-exports, no-op server | |
| 103 | + |
| 104 | +### Deleted Files |
| 105 | +| File | Reason | |
| 106 | +|------|---------| |
| 107 | +| `crates/terraphim_lsp/Cargo.lock` | Orphaned; conflicts with workspace `Cargo.lock` | |
| 108 | + |
| 109 | +## API Design |
| 110 | + |
| 111 | +### Public Types |
| 112 | +```rust |
| 113 | +/// Placeholder LSP server implementing the tower-lsp LanguageServer trait. |
| 114 | +/// |
| 115 | +/// Step 3 will add document tracking, KG analysis, and handlers. |
| 116 | +#[derive(Debug)] |
| 117 | +pub struct TerraphimLspServer; |
| 118 | + |
| 119 | +impl TerraphimLspServer { |
| 120 | + pub fn new() -> Self { |
| 121 | + Self |
| 122 | + } |
| 123 | +} |
| 124 | +``` |
| 125 | + |
| 126 | +### Public Functions |
| 127 | +```rust |
| 128 | +/// Run the LSP server (placeholder for Step 3). |
| 129 | +pub async fn run_lsp_server() -> anyhow::Result<()> { |
| 130 | + Ok(()) |
| 131 | +} |
| 132 | +``` |
| 133 | + |
| 134 | +### Error Types |
| 135 | +No custom error types in Step 1; use `anyhow::Result` for the placeholder runner. |
| 136 | + |
| 137 | +## Test Strategy |
| 138 | + |
| 139 | +### Unit Tests |
| 140 | +| Test | Location | Purpose | |
| 141 | +|------|----------|---------| |
| 142 | +| `test_server_can_be_constructed` | `src/lib.rs` | Verify `TerraphimLspServer::new()` works | |
| 143 | + |
| 144 | +### Integration Tests |
| 145 | +None for Step 1; integration tests for LSP protocol will be added in Step 3. |
| 146 | + |
| 147 | +### Verification Commands |
| 148 | +```bash |
| 149 | +cargo check -p terraphim_lsp |
| 150 | +cargo check --workspace |
| 151 | +cargo clippy -p terraphim_lsp --all-targets -- -D warnings |
| 152 | +cargo fmt --all -- --check |
| 153 | +``` |
| 154 | + |
| 155 | +## Implementation Steps |
| 156 | + |
| 157 | +### Step 1: Un-exclude crate from workspace |
| 158 | +**Files:** `Cargo.toml` |
| 159 | +**Description:** Remove `"crates/terraphim_lsp"` from the `exclude` array. |
| 160 | +**Tests:** `cargo check -p terraphim_lsp` will start resolving the crate. |
| 161 | +**Estimated:** 5 minutes |
| 162 | + |
| 163 | +### Step 2: Delete orphaned Cargo.lock |
| 164 | +**Files:** `crates/terraphim_lsp/Cargo.lock` |
| 165 | +**Description:** Delete the file; workspace `Cargo.lock` will take over. |
| 166 | +**Tests:** `cargo check -p terraphim_lsp` uses workspace lockfile. |
| 167 | +**Estimated:** 2 minutes |
| 168 | + |
| 169 | +### Step 3: Fix Cargo.toml |
| 170 | +**Files:** `crates/terraphim_lsp/Cargo.toml` |
| 171 | +**Description:** |
| 172 | +- Set `edition.workspace = true`. |
| 173 | +- Add dependencies: `tower-lsp`, `tokio` (workspace), `serde_json` (workspace), `terraphim_automata`, `terraphim_types`, `terraphim_rolegraph`. |
| 174 | +- Keep existing package metadata. |
| 175 | +**Tests:** `cargo check -p terraphim_lsp` resolves dependencies. |
| 176 | +**Estimated:** 15 minutes |
| 177 | + |
| 178 | +### Step 4: Add module placeholders and no-op server |
| 179 | +**Files:** `crates/terraphim_lsp/src/lib.rs`, `crates/terraphim_lsp/src/kg_analysis.rs`, `crates/terraphim_lsp/src/server.rs` |
| 180 | +**Description:** |
| 181 | +- `src/lib.rs`: declare `pub mod kg_analysis; pub mod server;`, re-export `TerraphimLspServer`, add unit test. |
| 182 | +- `src/kg_analysis.rs`: empty module with module-level docs. |
| 183 | +- `src/server.rs`: define `TerraphimLspServer` and implement `tower_lsp::LanguageServer` with empty async methods. |
| 184 | +**Tests:** `cargo check -p terraphim_lsp` and `cargo test -p terraphim_lsp` pass. |
| 185 | +**Dependencies:** Step 3 |
| 186 | +**Estimated:** 25 minutes |
| 187 | + |
| 188 | +### Step 5: Verification |
| 189 | +**Description:** Run the verification commands and fix any fmt/clippy issues. |
| 190 | +**Tests:** All verification commands pass. |
| 191 | +**Dependencies:** Step 4 |
| 192 | +**Estimated:** 15 minutes |
| 193 | + |
| 194 | +## Rollback Plan |
| 195 | + |
| 196 | +If issues discovered: |
| 197 | +1. Restore `"crates/terraphim_lsp"` to `exclude` in root `Cargo.toml`. |
| 198 | +2. Revert `crates/terraphim_lsp/Cargo.toml` and `src/lib.rs`. |
| 199 | +3. Restore `crates/terraphim_lsp/Cargo.lock` if needed. |
| 200 | + |
| 201 | +## Dependencies |
| 202 | + |
| 203 | +### New Dependencies |
| 204 | +| Crate | Version | Justification | |
| 205 | +|-------|---------|---------------| |
| 206 | +| `tower-lsp` | 0.20 | LSP server framework | |
| 207 | +| `tokio` | workspace | Async runtime | |
| 208 | +| `serde_json` | workspace | LSP JSON-RPC | |
| 209 | +| `terraphim_automata` | path | Aho-Corasick term matching (Step 2) | |
| 210 | +| `terraphim_types` | path | Shared domain types | |
| 211 | +| `terraphim_rolegraph` | path | KG connectivity (Step 3) | |
| 212 | + |
| 213 | +### Dependency Updates |
| 214 | +None. |
| 215 | + |
| 216 | +## Performance Considerations |
| 217 | + |
| 218 | +### Expected Performance |
| 219 | +| Metric | Target | Measurement | |
| 220 | +|--------|--------|-------------| |
| 221 | +| `cargo check -p terraphim_lsp` | < 30s | Stopwatch | |
| 222 | +| `cargo check --workspace` | No regression | Compare before/after | |
| 223 | + |
| 224 | +### Benchmarks to Add |
| 225 | +None in Step 1. |
| 226 | + |
| 227 | +## Open Items |
| 228 | + |
| 229 | +| Item | Status | Owner | |
| 230 | +|------|--------|-------| |
| 231 | +| Confirm `tower-lsp` 0.20 resolves with workspace tokio | Pending | Implementer | |
| 232 | +| Decide whether to keep `readme = "../../README.md"` | Pending | Reviewer | |
| 233 | + |
| 234 | +## Approval |
| 235 | + |
| 236 | +- [ ] Technical review complete |
| 237 | +- [ ] Test strategy approved |
| 238 | +- [ ] Human approval received |
0 commit comments