feat(context): surface LSP diagnostics — context --check diagnostics (closes #253) - #259
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…closes #253) Gap vs Serena MCP: Serena surfaces language-server diagnostics (lint/ errors/warnings) per file so an agent can find bugs without shelling out to a linter. CodeLens had all the LSP plumbing — lsp_client.py even registered the publishDiagnostics client capability at init — but only ever issued definition/references/hover, using LSP purely to verify its own findings. The diagnostics themselves were never exposed. - New LSPClient.get_diagnostics(): opens the file, polls the reader loop's existing _notification_list for a matching publishDiagnostics notification, returns the latest. Does NOT drop already-collected diagnostics first (many servers only push on change, not re-open, so drop-and-wait would return empty for an already-analyzed file). Reader loop now appends notifications under the lock so this can't race. - New commands/diagnostics.py: enables LSP internally (diagnostics have no non-LSP fallback), maps raw LSP diagnostics to findings (severity 1..4 -> error/warning/info/hint, 0->1-indexed lines), degrades to lsp_available:false + note when no server is installed. - Exposed as `context --check diagnostics --file <path>` (opt-in, command count stays 12). Verification: 8 unit tests (notification filtering + finding transform + graceful degradation). Graceful-degradation path verified end-to-end via real CLI (.ts file, no ts-language-server -> lsp_available:false, no hang, valid JSON). Happy path is mock-covered because the only LSP server installed here (rust-analyzer) does not respond to `initialize` within 60s on this machine — a pre-existing rust-analyzer startup issue, unrelated to this code (initialize() is untouched). Full rationale + honest verification limits in docs/design/0253-lsp-diagnostics.md.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #253. First of the Serena-gap issues (#253-#258).
Summary
Surfaces language-server diagnostics (lint/errors/warnings) per file — the "contextual diagnostics" capability Serena has and CodeLens lacked, despite already having the LSP plumbing.
LSPClient.get_diagnostics()— reuses the reader loop's existing_notification_list(which already capturedpublishDiagnostics), no new transport. Reader now appends notifications under the lock so the diagnostics reader can't race.commands/diagnostics.py— enables LSP internally (no non-LSP fallback for diagnostics), maps raw LSP → findings (severity 1..4 → error/warning/info/hint, 0→1-indexed lines), degrades tolsp_available:false+ note when no server installed.context --check diagnostics --file <path>(opt-in, command count stays 12).Test plan
pytest tests/test_diagnostics_command.py— 8/8 (notification filtering: URI match/other-file-ignored/latest-wins/not-initialized; command: missing-file/file-not-found/lsp-unavailable/raw→finding transform)pytest tests/test_command_registry.py tests/test_issue195_consolidation.py tests/test_command_count.py— pass (diagnostics added to impl-module allowlist)--command-count→ 12; design-doc CI check passes; sync clean.ts, no ts-language-server →lsp_available:false, no hang, valid JSON, exit 0)Honest verification limit
The happy path (diagnostics actually returned) is mock-covered, not live — rust-analyzer (the only LSP server installed in the dev env) doesn't respond to
initializewithin 60s on this machine (pre-existing rust-analyzer startup issue;initialize()is untouched by this PR). The mocked tests exercise the exact_notification_listcapture the other LSP features (find_references/go_to_definition) already use in production. Documented in the design doc.