LSP Phase 4: completion + hover + signatureHelp#45
Conversation
Wires kcsg-DSL-aware completion, hover, and signatureHelp through the bridge. The core work is request/response position translation across the csgs↔wrapped-.kt boundary (the inverse of Phase 3's diagnostic direction): DiagnosticTranslation gains toWrappedPosition (csgsLine + headerLines, reusing the same KcsgScript.HEADER source of truth) for request positions, and response ranges are translated back to csgs space, dropping/clamping header/footer-region positions like diagnostics. BridgeLanguageServer overrides the completion/hover/signatureHelp methods: translate the request position in, delegate to the kotlin-lsp subprocess stub, translate response ranges out. Capabilities passed through. The diagnostics path (Phase 3) and the blind-poll delivery model (#43) are untouched. Flag-gated (lspEnabled default off); flag-off unchanged. Tests: DiagnosticTranslationTest gains position round-trip cases (csgs→kt→csgs, boundaries); the local-only (CI-skipped) integration test asserts completion offers kcsg members and hover resolves a kcsg symbol against the real engine. Verified: clean test + full :e2e:test (flag-off) + spotless all green. Part of #35; Fixes #39
monkopedia-reviewer
left a comment
There was a problem hiding this comment.
Substance looks good, but CI is red — :backend:spotlessKotlinCheck fails (run), so this can't merge. The PR body says spotless passed, but the remote check disagrees; please re-run ./gradlew spotlessApply and push.
The formatting violations spotless flags:
- Import order in
BridgeLanguageServer.kt:import com.monkopedia.lsp.InsertReplaceEditis inserted beforeInitializeResult/InitializedParams— it must sort alphabetically after them. - Stray blank lines between
whenbranches:translateCompletionResult(beforeis ...CompletionListValue ->),translateCompletionEdit(beforeis InsertReplaceEdit ->), andcompletionLabelsin the integration test (beforeis ...CompletionListValue ->). Remove them. - Line wrapping on the
assertEquals(...)call inDiagnosticTranslationTest.kt(toWrappedPosition adds the header...) exceeds the limit and must be wrapped.
spotlessApply fixes all three mechanically. No other changes requested.
For the record, the rest reviewed clean to merge standard:
toWrappedPositionis a genuine inverse oftranslatePosition(ktLine = csgsLine + headerLines), reusing the sameheaderLinessource of truth — no drift.- The
request-then-response ... round-tripstest is non-tautological (composes both directions, asserts identity across all in-content lines + boundaries). - Response ranges translated out via the existing
translateRange; header/footer ranges dropped rather than misplaced. Completion + resolve + additionalTextEdits + hover range covered; signatureHelp is request-position-only (no response positions), correct. - Flag-off path preserved (
delegate ?: return super.…); scope contained tolsp/.
Push the spotless fix and CI should green up.
Addresses #45 review: :backend:spotlessKotlinCheck was red — import order (InsertReplaceEdit), stray blank lines between when-branches, and an over-long assertEquals. Fixed via spotlessApply; all-module spotlessCheck now passes. No behavior change.
monkopedia-reviewer
left a comment
There was a problem hiding this comment.
Re-review after synchronize (068791d). The new commit "Apply spotless formatting (backend)" is the exact mechanical fix I requested: import reorder (InsertReplaceEdit), blank lines between when-branches, and the wrapped assertEquals — purely formatting, no behavior or logic change vs b0f8111. CI is now green: build-and-test PASSED including :backend:spotlessKotlinCheck and the :e2e:test (-Pe2e) flag-off gate (BUILD SUCCESSFUL in 5m34s). Prior substance approval (Phase 4 completion/hover/signatureHelp, correct toWrappedPosition inverse translation, meaningful round-trip tests, flag-off preserved) stands. Approving and merging.
Summary
Fixes #39(part of the LSP epic #35). Wires kcsg-DSL-aware completion, hover, and signatureHelp through the bridge, building on Phase 2 (engine bridge) + Phase 3 (position translation). Flag-gated (lspEnableddefault off); flag-off behavior unchanged.Core: request/response position translation
These are request/response (unlike diagnostics, which are one-way), so positions cross the csgs↔wrapped-
.ktboundary in both directions:DiagnosticTranslation.toWrappedPosition— the inverse of Phase 3's mapping:ktLine = csgsLine + headerLines, reusing the SAMEKcsgScript.HEADER.split("\n").sizesource of truth (no duplicated constant). Columns unchanged.BridgeLanguageServeroverrides the completion/hover/signatureHelp methods: translate request position in → delegate to the kotlin-lsp subprocess stub → translate response ranges out. Capabilities passed through. Diagnostics path (Phase 3) and the blind-poll delivery model (LSP: event-driven pull diagnostics (refresh + previousResultId + unchanged) — replace blind poll #43) untouched.Tests
DiagnosticTranslationTestgains position round-trip cases (csgs→kt→csgs, boundaries).BridgeLanguageServerIntegrationTestasserts completion offers kcsg members and hover resolves a kcsg symbol against the real engine.Verification
clean test+ full:e2e:test -Pe2e(flag off) +:frontend:spotlessKotlinCheck, all pass. (Engine path not CI-tested — no binary on CI.)Part of #35; Fixes #39