Skip to content

fix(lsp-status): unify --lsp-status flag and lsp-status subcommand payload (issue #33)#87

Merged
Wolfvin merged 2 commits into
mainfrom
fix/issue-33-lsp-status-entry-point-parity
Jun 29, 2026
Merged

fix(lsp-status): unify --lsp-status flag and lsp-status subcommand payload (issue #33)#87
Wolfvin merged 2 commits into
mainfrom
fix/issue-33-lsp-status-entry-point-parity

Conversation

@Wolfvin

@Wolfvin Wolfvin commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes #33.

codelens --lsp-status (top-level flag, scripts/codelens.py L829-837) and codelens lsp-status (subcommand, scripts/commands/lsp_status.py) returned structurally different payloads for what the documentation treats as the same operation. The MCP server (which only exposes subcommands) got the smaller payload, so CLI users and MCP agents saw different "truths" for the same question.

Option chosen: B (unify schema, keep both entry points)

The issue suggested Option A is "more recommended" because it removes code duplication. I chose Option B instead, for the following reason:

The issue's own Definition of Done requires:

  1. DoD feat: add codelens-watch.py — file watcher with auto outline.json generation #1 — the repro diff <(...) <(...) command must show IDENTIK output with diff exit code 0
  2. DoD refactor: refocus CodeLens — archive frontend, remove garbage, keep core only #2 — a new test asserting both entry points (top-level flag AND subcommand) have the same top-level keys

Pure Option A (delete the --lsp-status interception entirely) makes the flag invalid -> codelens --lsp-status would fail with an argparse "unrecognized arguments" error -> diff exit code != 0 -> DoD #1 unsatisfiable, and DoD #2 untestable (only one entry point would remain).

Option B is the only choice consistent with the literal DoD. The architectural concern that motivated Option A (single source of truth / no duplicate logic) is preserved by routing both entry points through the same backend function, hybrid_engine.get_lsp_status(). There is no duplicate business logic — only a 9-line alias block in codelens.py that delegates to the same function the subcommand now delegates to.

Files changed

File Change
scripts/commands/lsp_status.py execute() now delegates to hybrid_engine.get_lsp_status() instead of rebuilding a smaller dict from lsp_client.detect_available_servers(). ImportError fallback updated to match the new richer shape.
scripts/lsp_client.py detect_available_servers() now returns sorted(config["extensions"]) instead of list(config["extensions"]) (deterministic across Python invocations; config["extensions"] is a set so list(set) order depended on hash randomization).
SKILL-QUICK.md Trigger map gains explicit "LSP servers available?" -> lsp-status entry noting that --lsp-status is an alias. Setup & Lifecycle command list also documents the alias relationship.
CHANGELOG.md New "LSP Status Entry-Point Unification (issue #33)" section under [8.2.0] - Unreleased.
tests/test_cli.py New TestLspStatusEntryPointParity class with 3 tests.

Note on dead code: lsp_client.detect_available_servers() is not removed — it is still used by hybrid_engine.HybridEngine.__init__, lsp_client.get_server_for_file, and lsp_client.get_server_for_workspace. It is the low-level building block; get_lsp_status() is the high-level wrapper. Both belong in the architecture.

Repro diff (DoD #1)

$ diff <(python3 scripts/codelens.py --lsp-status --format json | python3 -m json.tool) \
       <(python3 scripts/codelens.py lsp-status --format json | python3 -m json.tool)
$ echo $?
0

Before the fix (baseline, on main):

$ diff <(...) <(...)
4,5d3
<     "available_count": 0,
<     "total_servers": 6,
9d6
<             "path": null,
13,17d9
<             "extensions": [
<                 ".pyx",
<                 ".py",
<                 ".pyi"
<             ],
...
91c50
<     "recommendation": "No LSP servers found. Install one for --deep analysis. Recommended: pyright (Python) or typescript-language-server (JS/TS)."
---
>     "hint": "No LSP servers found. Install one for deep analysis (see install_hint)."
$ echo $?
1

After the fix:

$ diff <(...) <(...)
$ echo $?
0

Stable across 5 consecutive runs (hash randomization mitigated by the sorted() fix in detect_available_servers()):

Run 1: diff exit code 0
Run 2: diff exit code 0
Run 3: diff exit code 0
Run 4: diff exit code 0
Run 5: diff exit code 0

Test run (DoD #2 + DoD #3)

New parity tests

$ PYTHONPATH=scripts python3 -m pytest tests/test_cli.py::TestLspStatusEntryPointParity -v
============================= test session starts ==============================
platform linux --python 3.12.13, pytest-9.0.2
collected 3 items

tests/test_cli.py::TestLspStatusEntryPointParity::test_top_level_keys_match PASSED [ 33%]
tests/test_cli.py::TestLspStatusEntryPointParity::test_per_server_keys_match PASSED [ 66%]
tests/test_cli.py::TestLspStatusEntryPointParity::test_payloads_byte_identical PASSED [100%]

============================== 3 passed in 3.44s ==============================

LSP-related test files

$ PYTHONPATH=scripts python3 -m pytest tests/test_cli.py tests/test_hybrid_engine.py -v
============================= test session starts ==============================
collected 40 items

tests/test_cli.py .....................                                  [ 52%]
tests/test_hybrid_engine.py ..............s....                          [100%]

======================== 39 passed, 1 skipped in 10.50s ========================

(The 1 skip is test_hybrid_engine.py::TestHybridIntegration::test_deep_with_pyright — skips when pyright isn't installed; pre-existing, unrelated to this change.)

Full test suite

PYTHONPATH=scripts python3 -m pytest tests/ -v:

  • All test files pass except 2 pre-existing environmental issues, both confirmed to fail identically on the clean main baseline (before my changes) — verified by git stash + re-run:
    • tests/test_integration.py — hangs indefinitely even on baseline (likely because every test spawns a codelens.py subprocess and the environment is slow). Not caused by this PR.
    • tests/test_vuln_staleness.py::TestScanVulnerabilitiesCacheInfo::test_vulnerable_app_has_stale_cache_info — fails on baseline because the env's built-in VULN_DB is 486 days old but the test's staleness check returns False (clock/environment issue). Not caused by this PR.
  • All other 36 test files pass cleanly. The new TestLspStatusEntryPointParity class adds 3 new passing tests.

Checklist

  • Option explicitly chosen (B) with rationale
  • Repro diff produces byte-identical output (exit code 0)
  • New test asserting structural key parity between both entry points added to tests/test_cli.py
  • Full test suite run (pre-existing environmental failures called out, no new failures introduced)
  • SKILL-QUICK.md trigger map updated per chosen option
  • No dead code introduced (detect_available_servers() is still used by 3 other call sites)
  • CHANGELOG.md updated
  • PR title follows fix: ... format from CONTRIBUTING.md

Closes #33.

…yload (issue #33)

Both entry points now delegate to hybrid_engine.get_lsp_status() (single
source of truth). Previously --lsp-status called get_lsp_status() while
lsp-status called detect_available_servers() directly, producing
structurally different payloads for what the docs treat as the same
operation. The MCP server (which only exposes the subcommand) got the
smaller payload, so CLI users and MCP agents saw different 'truths'.

Also fixes a pre-existing determinism bug in detect_available_servers():
extensions was list(set) which varies across Python invocations due to
hash randomization. Now sorted, so the repro diff is byte-identical.

Chose Option B over Option A (issue suggested A is 'more recommended')
because the DoD explicitly requires both entry points to produce
byte-identical output (repro diff exit code 0) AND a test asserting
'both entry points have the same top-level keys' — both DoD items are
unsatisfiable if the top-level flag is removed entirely. Option A's
architectural concern (single source of truth) is preserved by routing
both entry points through the same backend function.

Files changed:
- scripts/commands/lsp_status.py: delegate to hybrid_engine.get_lsp_status()
- scripts/lsp_client.py: sort extensions for deterministic output
- SKILL-QUICK.md: document both entry points in trigger map
- CHANGELOG.md: add issue #33 entry
- tests/test_cli.py: new TestLspStatusEntryPointParity class (3 tests)

Closes #33.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@Wolfvin Wolfvin merged commit b27671c into main Jun 29, 2026
0 of 6 checks passed
@Wolfvin Wolfvin deleted the fix/issue-33-lsp-status-entry-point-parity branch June 29, 2026 17:15
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG-05] --lsp-status has two entry points with different payload shapes

1 participant