Skip to content

Fix /doc placing Python docstrings before decorators#318571

Merged
ulugbekna merged 3 commits into
mainfrom
ulugbekna/agents/vscode-issue-283165-fix-plan
May 28, 2026
Merged

Fix /doc placing Python docstrings before decorators#318571
ulugbekna merged 3 commits into
mainfrom
ulugbekna/agents/vscode-issue-283165-fix-plan

Conversation

@ulugbekna
Copy link
Copy Markdown
Contributor

Fixes #283165.

Problem

Invoking /doc (or any docstring-generation flow that relies on the tree-sitter "node to document" identification) on a Python function/class with a decorator placed the docstring before the @decorator line instead of inside the function body. Pylance rightly flagged this as an error, and any users relying on the docstring being inside the function got broken code.

Repro from the issue:

import pytest

@pytest.fixture(scope="module")
def sample_data():
    return {"name": "John Doe", "age": 30, "email": "john.doe@example.com"}

Select the whole file → /doc → docstring lands above @pytest.fixture(...).

Root cause

In extensions/copilot/src/platform/parser/node/util.ts, isDocumentableNode had no Python-specific case and fell through to the default regex /definition|declaration|declarator/.

In Python's tree-sitter grammar, a decorated_definition node wraps the inner function_definition / class_definition:

decorated_definition
├── decorator: "@pytest.fixture(scope=...)"
└── function_definition: "def sample_data(): ..."

Both decorated_definition and function_definition match /definition/. When the user's selection covered the whole decorated unit, the weighting in _getNodeMatchingSelection preferred the outer decorated_definition — whose range starts at the @… line — so downstream consumers (LLM prompt context, docstring insertion anchor) treated the decorator line as the start of the function.

Fix

Add a Python case to isDocumentableNode that matches only function_definition and class_definition — not the wrapping decorated_definition. Decorators are metadata, and the documentable/symbol-bearing unit is the function/class itself.

case WASMLanguage.Python:
    return node.type.match(/^(function_definition|class_definition)$/);

_getNodeToDocument is also used by rename suggestions, the chat panel definition/references resolvers, test generation from source, and symbol-at-cursor context. All of these benefit from the parser identifying the inner declaration.

Tests

New extensions/copilot/src/platform/parser/test/node/getNodeToDocument.py.spec.ts with 14 tests:

  • Plain function / class definitions (cursor on def/class, in body).
  • Decorated function — cursor on def, in body, whole-file selection.
  • Multiple stacked decorators (@first / @second(arg=1) / @third).
  • Decorated class (@dataclass).
  • Plain method inside a class.
  • Decorated method inside a class (@staticmethod).
  • Decorated method inside a decorated class.
  • Classmethod with stacked decorators (@classmethod + @cached).
  • Exact pytest-fixture repro from /doc adds Python docstrings before function definition  #283165.

Verification

  • tsc --noEmit clean.
  • All 120 parser tests pass (14 new + 106 existing).
  • Wider src/extension/{prompt,context,intents} sweep: 982 passed, 0 failed.

In Python's tree-sitter grammar, a `decorated_definition` node wraps
`function_definition` / `class_definition`. The previous fall-through to
the default regex `/definition|declaration|declarator/` matched the
outer `decorated_definition`, so the documentable node's range started
at the `@decorator` line. Downstream consumers (LLM prompt context,
docstring insertion anchor) then placed docstrings *above* the
 a Pylance error and broken code.decorator

Match only the inner `function_definition` / `class_definition` for
Python. Add unit tests covering plain/decorated functions, classes, and
methods (including stacked decorators and decorated classes).

Fixes #283165

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 27, 2026 14:57
@ulugbekna ulugbekna enabled auto-merge (squash) May 27, 2026 14:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes Python /doc docstring placement when functions/classes are decorated by ensuring the parser selects the inner function_definition/class_definition node rather than the wrapping decorated_definition.

Changes:

  • Add a Python-specific isDocumentableNode rule to match only function_definition and class_definition.
  • Add a new Python-focused getNodeToDocument test suite covering decorated functions/classes/methods and the #283165 repro.
Show a summary per file
File Description
extensions/copilot/src/platform/parser/node/util.ts Adds Python-specific documentable-node matching to avoid selecting decorated_definition.
extensions/copilot/src/platform/parser/test/node/getNodeToDocument.py.spec.ts Adds regression and coverage tests for Python decorated definitions and selections.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 2

Comment thread extensions/copilot/src/platform/parser/node/util.ts Outdated
Address review feedback on #283165:

- Match `decorated_definition` in `isDocumentableNode` for Python so
  cursors/selections on the `@decorator` line no longer escape past it to
  the `module` root.
- Introduce `unwrapPythonDecoratedDefinition` and call it in both code
  paths of `_getNodeToDocument` so the returned range is always the inner
  `function_definition`/`class_ excluding the decorator definition`
  regardless of whether the node was found via selection match or via
  walk-up.
- Fix misleading test title for whole-method decorated selection (now
  picks the inner function_definition as advertised).
- Add 4 regression tests for cursor-on-decorator (incl. on `@` sign,
  selection of decorator-only, decorator on method inside class).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
roblourens
roblourens previously approved these changes May 27, 2026
…r too

Defensive consistency fix from subagent code review. While the Python
tree-sitter grammar makes `function_definition`/`class_definition` (not
`decorated_definition`) the direct parent of name  so thisidentifiers
branch can't trigger  applying the unwrap maintains thetoday
invariant: "the parser API never exposes a Python `decorated_definition`
externally". This protects against future grammar changes or callers
that pass ranges over decorator children.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ulugbekna ulugbekna merged commit c1b72a2 into main May 28, 2026
25 checks passed
@ulugbekna ulugbekna deleted the ulugbekna/agents/vscode-issue-283165-fix-plan branch May 28, 2026 13:27
@vs-code-engineering vs-code-engineering Bot added this to the 1.123.0 milestone May 28, 2026
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.

/doc adds Python docstrings before function definition

4 participants