Skip to content

Conversation

@MatthewMckee4
Copy link
Contributor

Summary

Make documentation markdown for completions, which, in zed, moves the documentation to the right of the completions panel, rather than inside it.

Also add documentation for class and function definitions

image

Test Plan

e2e test.

@astral-sh-bot
Copy link

astral-sh-bot bot commented Dec 2, 2025

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@astral-sh-bot
Copy link

astral-sh-bot bot commented Dec 2, 2025

mypy_primer results

Changes were detected when running on open source projects
beartype (https://github.com/beartype/beartype)
- beartype/claw/_package/clawpkgtrie.py:66:29: warning[unsupported-base] Unsupported class base with type `<class 'dict[str, PackagesTrieBlacklist]'> | <class 'dict[str, Divergent]'>`
- beartype/claw/_package/clawpkgtrie.py:247:29: warning[unsupported-base] Unsupported class base with type `<class 'dict[str, PackagesTrieWhitelist]'> | <class 'dict[str, Divergent]'>`
- Found 498 diagnostics
+ Found 496 diagnostics

scikit-build-core (https://github.com/scikit-build/scikit-build-core)
+ src/scikit_build_core/build/_pathutil.py:25:38: error[invalid-argument-type] Argument to function `__new__` is incorrect: Expected `str | PathLike[str]`, found `DirEntry[Path]`
+ src/scikit_build_core/build/_pathutil.py:27:24: error[invalid-argument-type] Argument to function `__new__` is incorrect: Expected `str | PathLike[str]`, found `DirEntry[Path]`
+ src/scikit_build_core/build/wheel.py:98:20: error[no-matching-overload] No overload of bound method `__init__` matches arguments
- Found 41 diagnostics
+ Found 44 diagnostics

pandas-stubs (https://github.com/pandas-dev/pandas-stubs)
+ pandas-stubs/_typing.pyi:1207:16: warning[unused-ignore-comment] Unused blanket `type: ignore` directive
- Found 5814 diagnostics
+ Found 5815 diagnostics

No memory usage changes detected ✅

Gankra
Gankra previously requested changes Dec 2, 2025
@AlexWaygood AlexWaygood added server Related to the LSP server ty Multi-file analysis & type inference labels Dec 2, 2025
kind: SymbolKind::Class,
name_range: class_def.name.range(),
full_range: stmt.range(),
documentation: class_def.definition(&self.model).docstring(self.model.db()),
Copy link
Member

@BurntSushi BurntSushi Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty worried about doing this eagerly for every single symbol in an environment. I did some very ad hoc testing to compare perf here. On main with this pyproject.toml:

[project]
name = "ex003-reexport-simple"
version = "0.1.0"
description = "Add your description here"
readme = "README.md"
requires-python = ">=3.12"
dependencies = [
    "matplotlib>=3.10.7",
    "numpy>=2.3.4",
    "pandas>=2.3.3",
    "scikit-learn>=1.7.2",
    "scipy>=1.16.3",
    "whenever>=0.9.3",
]

And this Python source file:

array

With my cursor right after array.

I get this in the trace logs after requesting completions at my cursor position a few times:

2025-12-02 09:59:31.888839440 DEBUG request{id=13 method="textDocument/completion"}: Completions request returned 1582 suggestions in 177.106611ms
2025-12-02 09:59:34.297544175 DEBUG request{id=15 method="textDocument/completion"}: Completions request returned 1582 suggestions in 56.358747ms
2025-12-02 09:59:36.489098862 DEBUG request{id=17 method="textDocument/completion"}: Completions request returned 1582 suggestions in 57.155421ms
2025-12-02 09:59:38.932666244 DEBUG request{id=20 method="textDocument/completion"}: Completions request returned 1582 suggestions in 57.014757ms
2025-12-02 09:59:42.342361845 DEBUG request{id=22 method="textDocument/completion"}: Completions request returned 1582 suggestions in 58.887699ms
2025-12-02 09:59:45.683431792 DEBUG request{id=24 method="textDocument/completion"}: Completions request returned 1582 suggestions in 57.7364ms

Now with this PR, I do the same test and I get:

2025-12-02 10:02:13.280166325 DEBUG request{id=13 method="textDocument/completion"}: Completions request returned 1582 suggestions in 139.561616ms
2025-12-02 10:02:16.136079062 DEBUG request{id=15 method="textDocument/completion"}: Completions request returned 1582 suggestions in 73.513749ms
2025-12-02 10:02:18.525222384 DEBUG request{id=17 method="textDocument/completion"}: Completions request returned 1582 suggestions in 71.187676ms
2025-12-02 10:02:20.888572941 DEBUG request{id=19 method="textDocument/completion"}: Completions request returned 1582 suggestions in 71.481457ms
2025-12-02 10:02:23.408312653 DEBUG request{id=21 method="textDocument/completion"}: Completions request returned 1582 suggestions in 70.057387ms
2025-12-02 10:02:26.422923788 DEBUG request{id=23 method="textDocument/completion"}: Completions request returned 1582 suggestions in 72.955201ms

Which seems like a noticeable dip to me. And this probably isn't that big of a Python project either.

So I think I'd prefer to hold off on doing this eagerly with auto-import specifically. I think it looks like that should be done via a resolve request.

An alternative that still involves eagerly getting the docstring is to:

  1. Only do it for symbols that match the query given.
  2. Perhaps only do it for the first N such symbols.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah okay, resolve request sounds a lot better.

I can revert these changes and add them to another PR adding resolve completion. Do you think we should defer the auto import too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! What do you mean by defer the auto import too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I mean defer generating the edit for unimported symbols.

I'm not sure how expensive this is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave that for now personally. It isn't obviously slow to me, and I wrote it to amortize most work so that the unit of work done per completion is pretty small (but still not fully optimal from a quick glance).

@AlexWaygood AlexWaygood removed their request for review December 2, 2025 15:15
@Gankra Gankra dismissed their stale review December 2, 2025 15:40

My issues are resolved.

@MatthewMckee4 MatthewMckee4 changed the title Use markdown for completions docstring [ty] Use markdown for completions documentation Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

server Related to the LSP server ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants