[wip] refactor python language server #10908

austin3dickey · 2025-12-03T18:12:24Z

Release Notes

New Features

N/A

Bug Fixes

N/A

QA Notes

@:console @:help @:notebooks @:positron-notebooks @:outline @:problems @:references @:web @:win

github-actions · 2025-12-03T18:12:42Z

E2E Tests 🚀
This PR will run tests tagged with: @:critical @:console @:help @:notebooks @:positron-notebooks @:outline @:problems @:references @:web @:win

^readme ^{valid tags}

Copilot

Pull request overview

This PR refactors the Python language server implementation by replacing the jedi-language-server dependency with a custom lightweight LSP server implementation. The new server maintains Positron-specific features (namespace-aware completions, DataFrame/Series column completions, magic commands, help topics, syntax diagnostics) while delegating static analysis features to third-party extensions like Pylance.

Key changes:

Removed jedi-language-server, jedi, and parso dependencies and vendored code
Created new positron_lsp.py implementing a minimal LSP server using pygls directly
Removed complex Jedi patches and integration code
Updated dependency versions (pygls 2.0.0, lsprotocol 2025.0.0)

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`scripts/patches/*.patch`	Removed patch files for jedi, parso, and jedi-language-server
`python_files/run-jedi-language-server.py`	Added type ignore comment for import
`python_files/positron_requirements/requirements.txt`	Updated dependencies, removed jedi-language-server, jedi, parso
`python_files/positron_requirements/requirements.in`	Removed jedi-language-server, upgraded pygls to 2.0.0
`python_files/posit/positron_language_server.py`	Updated import from positron_jedilsp to positron_lsp
`python_files/posit/positron/tests/test_positron_lsp.py`	New test file for the refactored LSP server
`python_files/posit/positron/tests/test_positron_jedilsp.py`	Removed old test file
`python_files/posit/positron/positron_lsp.py`	New LSP server implementation
`python_files/posit/positron/positron_jedilsp.py`	Removed old LSP server implementation
`python_files/posit/positron/lsp.py`	Updated import from positron_jedilsp to positron_lsp
`python_files/posit/positron/jedi.py`	Removed Jedi patches and integration code

extensions/positron-python/python_files/posit/positron/positron_lsp.py

Copilot · 2025-12-04T05:18:25Z

extensions/positron-python/python_files/posit/positron/positron_lsp.py

+        elif inspect.ismethod(obj) or inspect.isfunction(obj):
+            return types.CompletionItemKind.Function


Lines 782 and 784 return the same value. The elif branch checking for methods/functions is redundant since the else branch will also return CompletionItemKind.Function. Consider simplifying by removing the elif or add a comment explaining why this distinction exists in the code structure.

Suggested change

elif inspect.ismethod(obj) or inspect.isfunction(obj):

return types.CompletionItemKind.Function

Copilot · 2025-12-04T05:18:25Z

extensions/positron-python/python_files/posit/positron/positron_lsp.py

+        # Fall back to the expression itself
+        topic = expr
+
+    logger.info(f"Help topic found: {topic}")


Using f-string formatting in logging calls can cause performance issues since the string is always formatted even if the logging level would skip it. Consider using lazy formatting: logger.info('Help topic found: %s', topic)

Suggested change

logger.info(f"Help topic found: {topic}")

logger.info("Help topic found: %s", topic)

Copilot · 2025-12-04T05:18:25Z

extensions/positron-python/python_files/posit/positron/positron_lsp.py

+    try:
+        _publish_diagnostics(server, uri)
+    except Exception:
+        logger.exception(f"Failed to publish diagnostics for {uri}", exc_info=True)


logger.exception() already includes exc_info=True by default, so passing it explicitly is redundant. Consider removing the exc_info=True parameter.

Suggested change

logger.exception(f"Failed to publish diagnostics for {uri}", exc_info=True)

logger.exception(f"Failed to publish diagnostics for {uri}")

seeM

I took a look at the code and it looks like a great start!

seeM · 2025-12-04T13:00:24Z

extensions/positron-python/python_files/run-jedi-language-server.py



-from jedi_language_server.cli import cli  # noqa: E402
+from jedi_language_server.cli import cli  # type: ignore # noqa: E402


Isn't this an upstream file? If so can we ignore in the pyproject.toml instead?

Yeah I could add this to our pyproject.toml!

seeM · 2025-12-04T13:03:52Z

extensions/positron-python/python_files/posit/positron/positron_lsp.py

+        self._messages_to_handle: list[Any] = []
+
+    @lru_cache  # noqa: B019
+    def get_message_type(self, method: str) -> type | None:


Is this pygls' recommended way to add methods? I wonder if there's something neater now that we're not depending on JediLanguageServerProtocol

seeM · 2025-12-04T13:05:13Z

extensions/positron-python/python_files/posit/positron/positron_lsp.py

+            init_opts = PositronInitializationOptions()
+
+        # Store the working directory (using params.root_path since workspace may not be initialized yet)
+        server._working_directory = init_opts.working_directory or params.root_path  # noqa: SLF001


I also wonder if there's another pygls recommended way

seeM · 2025-12-04T13:05:42Z

extensions/positron-python/python_files/posit/positron/positron_lsp.py

+        # Yield to parent implementation which handles workspace setup
+        return (yield from super().lsp_initialize(params))
+
+    def _data_received(self, data: bytes) -> None:  # type: ignore[override]


I don't think we need this anymore, and may want to try to make our server async

seeM · 2025-12-04T13:07:05Z

extensions/positron-python/python_files/posit/positron/positron_lsp.py

+        # Cache for magic completions
+        self._magic_completions: dict[str, tuple] = {}
+
+    def start_tcp(self, host: str) -> None:


It's possible we don't need all of this in the latest pygls. A lot of this was because pygls didn't let you use your own loop, which we needed to do

seeM · 2025-12-04T13:11:49Z

extensions/positron-python/python_files/posit/positron/positron_lsp.py

+    elif trimmed_line.startswith((_LINE_MAGIC_PREFIX, _CELL_MAGIC_PREFIX)):
+        # Magic command completion only
+        pass  # Will add magics below
+    else:


I haven't checked, are we providing the right completions (if any) inside string literals? Do we provide path completions?

seeM · 2025-12-04T13:17:17Z

extensions/positron-python/python_files/posit/positron/positron_lsp.py

+
+    # Try to evaluate the expression
+    try:
+        obj = eval(expr, server.shell.user_ns)


This feels like a really dangerous thing to do!

seeM · 2025-12-04T13:18:25Z

extensions/positron-python/python_files/posit/positron/positron_lsp.py

+    # Get keys based on the type of object
+    if isinstance(obj, dict):
+        keys = [str(k) for k in obj if isinstance(k, str)]
+    elif _is_environ_like(obj):
+        # os.environ or similar
+        keys = list(os.environ.keys())
+    elif _is_dataframe_like(obj):
+        # pandas/polars DataFrame
+        with contextlib.suppress(Exception):
+            keys = list(obj.columns)
+    elif _is_series_like(obj):
+        # pandas Series with string index
+        with contextlib.suppress(Exception):
+            keys = [str(k) for k in obj.index if isinstance(k, str)]


Not sure yet if it's a good idea, but we could consider using the existing inspectors code to abstract this sort of thing

seeM · 2025-12-04T13:19:00Z

extensions/positron-python/python_files/posit/positron/positron_lsp.py

+
+    # Try to evaluate the expression in the namespace
+    try:
+        obj = eval(expr, server.shell.user_ns)


How do other libs handle safe evaluation?

Looks like parso doesn't eval code at all. I'll see if I can adopt a similar strategy (or just use parso)

seeM · 2025-12-04T13:20:50Z

extensions/positron-python/python_files/posit/positron/positron_lsp.py

+        logger.exception(f"Failed to publish diagnostics for {uri}", exc_info=True)
+
+
+def _publish_diagnostics(server: PositronLanguageServer, uri: str) -> None:


Does this only run in console?

juliasilge mentioned this pull request Dec 3, 2025

Do not print out the message about the native REPL in the Positron console #10909

Open

wip

82a7c52

austin3dickey force-pushed the aus/pls branch from e253979 to 82a7c52 Compare December 4, 2025 03:46

austin3dickey added 4 commits December 3, 2025 22:18

wip

98beb0e

oops pls

0be29bc

hm

5949d05

fixes

3caa0d0

austin3dickey requested a review from Copilot December 4, 2025 05:17

Copilot AI reviewed Dec 4, 2025

View reviewed changes

seeM reviewed Dec 4, 2025

View reviewed changes

		elif inspect.ismethod(obj) or inspect.isfunction(obj):
		return types.CompletionItemKind.Function

	logger.info(f"Help topic found: {topic}")
	logger.info("Help topic found: %s", topic)

	logger.exception(f"Failed to publish diagnostics for {uri}", exc_info=True)
	logger.exception(f"Failed to publish diagnostics for {uri}")



		from jedi_language_server.cli import cli # noqa: E402
		from jedi_language_server.cli import cli # type: ignore # noqa: E402

		logger.exception(f"Failed to publish diagnostics for {uri}", exc_info=True)


		def _publish_diagnostics(server: PositronLanguageServer, uri: str) -> None:

[wip] refactor python language server #10908

Are you sure you want to change the base?

[wip] refactor python language server #10908

Uh oh!

Conversation

austin3dickey commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release Notes

New Features

Bug Fixes

QA Notes

Uh oh!

github-actions bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

seeM left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

austin3dickey commented Dec 3, 2025 •

edited

Loading

github-actions bot commented Dec 3, 2025 •

edited

Loading