LSP Phase 3: diagnostics through the header-line translation layer#44
Conversation
Phase 2 forwarded real kotlin-lsp diagnostics but their positions stayed
in WRAPPED-.kt space, so a squiggle for the user's csgs line N arrived on
line N+headerLines (off by the fixed 3-line header). This phase adds the
two-direction translation so diagnostics land on the correct csgs line.
Direction 1 (editor -> engine): didOpen/didChange now wrap the editor's
LIVE csgs content into the .kt form (reusing
KonstructionControllerImpl.copyContentToScript) and forward the wrapped
text to the engine via full-document sync, so the engine analyzes the
current edited content, not a stale stored snapshot.
Direction 2 (engine -> editor): publishDiagnostics ranges are translated
from .kt-space to csgs-space (csgsLine = ktLine - headerLines) in both
the Forwarder and the pull-poll push. headerLines reuses the same source
of truth as CompileTask (KcsgScript.HEADER.split("\n").size = 3).
Columns are unchanged. Header diagnostics (ktLine < headerLines) and
footer/beyond-content diagnostics are dropped, and ranges spilling into
the footer clamp their end inside the user's content -- mirroring
CompileTask's compile-error handling.
The translation lives in a new pure DiagnosticTranslation object inside
the lsp/ bridge. Completion/hover positions are untouched (Phase 4 #39);
the diagnostics delivery model (blind-poll) is untouched (#43).
Tests:
- DiagnosticTranslationTest (CI-runnable): a round-trip test that wraps a
known kcsg error via copyContentToScript, computes the wrapped line the
engine would report, translates it back, and asserts it lands on the
exact csgs line; plus user-line/header-drop/footer-drop/boundary/clamp
cases. 8 tests, all green.
- BridgeLanguageServerIntegrationTest (local real-engine, CI-skipped)
extended to assert the deliberate error lands on csgs line 1 (the
`val broken` line), not the wrapped line.
Fully flag-gated (lspEnabled default OFF + engine-gated); flag-off is
unchanged. Verified flag-off: ./gradlew test + spotlessCheck green, full
:e2e:test green (43 tests, 0 failures). Verified local real-engine: the
unresolved-reference diagnostic now reports on csgs line 1 (range 1..1),
not wrapped line 4.
Part of #35; Fixes #38.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
monkopedia-reviewer
left a comment
There was a problem hiding this comment.
Approving — LSP Phase 3, tier-1-clean (flag-gated LSP phase PR, per the standing waiver).
Translation correctness (the crux): csgsLine = ktLine - headerLines is provably correct against the actual wrapping. Verified KcsgScript.HEADER is 3 lines and copyContentToScript writes HEADER+"\n" then the csgs content, so user csgs line N lands at wrapped line N+3. headerLines reuses the SAME KcsgScript.HEADER.split("\\n").size that CompileTask uses (not a hardcoded 3) — the offset can't drift from the compiler's view. Header drop (ktLine < headerLines) and footer drop (csgsLine >= csgsLineCount) are correct; footer-spilling range ends clamp to start so ranges stay well-formed and inside user content — mirrors CompileTask's clamp/drop. Both directions (inbound wrap via copyContentToScript, outbound translate via DiagnosticTranslation) share the same header definition, so positions are symmetric.
Tests are meaningful, not tautological: the round-trip test wraps a known kcsg error through the real copyContentToScript, asserts the broken symbol physically lives on wrappedLines[csgsErrorLine + headerLines], then translates back and asserts the exact csgs line — grounded in the real wrapping, not assumed. Plus header-drop, footer-drop, boundary (last-header/first-user/last-user/footer), and footer-clamp. 8 CI-runnable tests. Real-engine round-trip extended in the integration test (correctly CI-skipped, no binary).
Flag-off preserved: gating is upstream (frontend lspEnabled default OFF) and the inert delegate ?: return guard; the new didChange opens with that same guard, so no-engine ⇒ no behavior change. Full-document sync re-wraps live editor content so the engine sees edits, not a stale file.
Scope: contained to lsp/ + tests; completion/hover untouched (Phase 4), delivery model untouched (#43); no unrelated edits.
CI: build-and-test PASSED (11m15s) — includes the new DiagnosticTranslationTest and the flag-off e2e suite (6 LSP-gated skips).
Quality: pure object, single-expression functions, mapNotNull drop, elvis defaults, @Volatile matching the existing pattern, copy() immutable updates, KDoc cross-refs to the mirror sites, Apache-2022 headers. Reads like it was always there.
What
Phase 3 of the LSP epic (#35). Phase 2 (#42) forwards real kotlin-lsp diagnostics, but their positions stayed in wrapped-
.ktspace, so a squiggle for the user's csgs line N arrived on line N+headerLines (off by the fixed 3-line header). This phase adds the two-direction translation so diagnostics land on the correct csgs line.Fully flag-gated (
lspEnableddefault OFF + engine-gated); flag-off ⇒ no behavior change. The change is contained to thelsp/bridge.The translation (both directions)
Direction 1 — document content IN (editor → engine).
textDocument/didOpenandtextDocument/didChangenow wrap the editor's LIVE csgs content into the.ktform before forwarding to the engine, so the engine analyzes what the user is currently editing rather than a stale stored snapshot.didChangedoes full-document sync for v1 (re-wrap the whole tiny doc, forward as a full-document change). Wrapping reuses konstructor's exactKonstructionControllerImpl.copyContentToScript(header swap + footer), so positions line up with the compiler.Direction 2 — diagnostic positions OUT (engine → editor). When forwarding the engine's diagnostics up as
publishDiagnostics(both in theForwarderpush path and the pull-poll push path), every range is translated from.kt-space to csgs-space:headerLinesreuses the same source of truth the backend already uses —KcsgScript.HEADER.split("\n").size(= 3), exactly asCompileTaskcomputes it (not hardcoded). Columns are unchanged (the header adds whole lines at column 0). Diagnostics whose line falls in the header (ktLine < headerLines) or the footer/beyond the user's content are dropped, and a range that spills into the footer clamps its end inside the user's content — mirroringCompileTask's existing compile-error handling (a wrapping artifact the user can't fix must never light a wrong line).The math lives in a new pure
DiagnosticTranslationobject inlsp/. Completion/hover positions are untouched (Phase 4 / #39); the diagnostics delivery model (blind-poll) is untouched (#43).The round-trip test (maintainer-flagged)
DiagnosticTranslationTest(CI-runnable, the guard): the core round-trip wraps a known kcsg error viacopyContentToScript, asserts the broken symbol actually lives on the computed wrapped line, then translates that engine range back and asserts it lands on the exact csgs error line (not N±offset). Plus: user-line subtraction, header-line drop, footer drop, boundary lines, and footer-clamp. 8 tests, all green.BridgeLanguageServerIntegrationTest(local real-engine, CI-skipped) extended to assert the deliberate error lands on csgs line 1 (theval brokenline), not the wrapped line.Verification
Flag-OFF / CI-equivalent — green:
./gradlew test(incl. the new 8 translation tests) ✅./gradlew spotlessCheck✅:e2e:testflag-off via the Mesa-EGL/xvfb env,clean, JDK 21 — 43 tests, 0 failures/errors (6 LSP-gated skips) ✅Real engine (LOCAL ONLY — CI has no binary): ran the integration test against
KONSTRUCTOR_KOTLIN_LSP=/tmp/kotlin-lsp-poc/server/kotlin-server-262.4739.0/bin/intellij-server. The unresolved-reference diagnostic (Unresolved reference 'thisSymbolDoesNotExist123') now reports on csgs line 1 (range 1..1).Before/after line math (real engine): the error is on csgs line 1; the engine reports it on wrapped line 4 (1 + 3 header lines); before this phase it surfaced on line 4, after it lands on line 1. ✅ (Engine path is not CI-tested — no binary in CI.)
Part of #35; Fixes #38.
🤖 Generated with Claude Code