Knowledge Wiki Orchestrata Ultimate

Orchestrata Ultimate › Plans › rename-engine-briefs › BRIEF-B1

BRIEF-B1 — LSP client + type-resolved symbol-rename core

Source: plans/rename-engine-briefs/BRIEF-B1.md at 8a2cb42ad · last changed 2026-07-27 · first committed 2026-07-27 · 297 lines.

Prose pending.

Plan text

BRIEF-B1 — LSP client + type-resolved symbol-rename core

Spec: specs/109-rename-engine/spec.md · Design: plans/rename-engine-design.md Branch: feat/rename-engine · Agent: Sonnet builder Depends on: nothing. B2 and B3 code against the models you own — land first.

1. Context

We are building a deterministic rename engine because LLM agents half-apply mechanical renames (a gw→slot rename once missed four Redis slot parsers and produced 54 test failures). The engine renames by type resolution, driving a real basedpyright-langserver over stdio JSON-RPC, and refuses rather than guessing.

A live spike already proved this works — read plans/rename-engine-design.md §2 before writing a line. Three of its findings are non-negotiable and you will get silently-wrong behaviour if you ignore them:

basedpyright is already a dev dependency on this branch (uv add --dev basedpyright, version 1.39.9). basedpyright-langserver is on .venv/bin.

Existing assets:

2. Files you own (create these; touch nothing else)

src/orchestrata_ultimate/refactor/rename/__init__.py
src/orchestrata_ultimate/refactor/rename/models.py
src/orchestrata_ultimate/refactor/rename/lsp_client.py
src/orchestrata_ultimate/refactor/rename/workspace.py
src/orchestrata_ultimate/refactor/rename/symbol_rename.py
tests/refactor/rename/__init__.py
tests/refactor/rename/conftest.py
tests/refactor/rename/test_lsp_transport.py
tests/refactor/rename/test_workspace_config.py
tests/refactor/rename/test_symbol_rename.py
tests/refactor/rename/test_resolution_preflight.py

refactor/rename/__init__.py exports only what you own; B2 and B3 will append.

3. Contracts

3.1 models.py — contract-frozen by the spec, implement verbatim

Copy the model definitions from specs/109-rename-engine/spec.md § Data Models exactly: ResidueReason, RenameStatus, SymbolTarget, TextEdit, FileEdit, WorkspaceEditPlan, ResidueItem, RenameOutcome. All frozen dataclasses with slots=True; enums are StrEnum. B2 and B3 import these — do not change a field name or type. Add to_dict() on ResidueItem and RenameOutcome returning JSON-safe primitives (Path → str).

3.2 lsp_client.py

@dataclass(frozen=True, slots=True)
class LspServerConfig:
    workspace_root: Path          # the ENGINE-OWNED temp config root, not the repo
    repo_root: Path
    python_path: Path
    server_cmd: tuple[str, ...] = ("basedpyright-langserver", "--stdio")
    request_timeout: float = 120.0
    settle_timeout: float = 60.0

class LspTransport:
    def __init__(self, cmd: Sequence[str], cwd: Path,
                 env: dict[str, str] | None = None,
                 tracer: Tracer | None = None) -> None: ...
    def request(self, method: str, params: dict, timeout: float = 120.0) -> dict: ...
    def notify(self, method: str, params: dict) -> None: ...
    def close(self) -> None: ...

class PyrightLangServer:
    def __init__(self, config: LspServerConfig, tracer: Tracer | None = None) -> None: ...
    def __enter__(self) -> PyrightLangServer: ...
    def __exit__(self, *exc: object) -> None: ...
    def open_document(self, path: Path) -> None: ...
    def prepare_rename(self, path: Path, line: int, column: int) -> dict | None: ...
    def rename(self, path: Path, line: int, column: int, new_name: str) -> WorkspaceEditPlan: ...
    def definition(self, path: Path, line: int, column: int) -> list[dict]: ...
    def wait_until_settled(self, quiet_seconds: float = 8.0) -> None: ...

Transport requirements:

initialize params must declare workspace.configuration=true, workspace.workspaceEdit.documentChanges=true, and textDocument.rename.prepareSupport=true.

rename() normalizes both WorkspaceEdit shapes into a WorkspaceEditPlan: result["changes"] (uri → edits map) and result["documentChanges"] (array of {textDocument:{uri,version}, edits:[...]}). version is null for unopened files — not an error. Positions are 0-based, end exclusive.

3.3 workspace.py

class WorkspaceConfigBuilder:
    DEFAULT_INCLUDE = ("src", "tests", "scripts")
    DEFAULT_EXCLUDE = ("**/__pycache__", "**/*_pb2*.py", "**/*_pb2*.pyi")
    def __init__(self, tracer: Tracer | None = None) -> None: ...
    def build(self, repo_root: Path,
              include: Sequence[str] = DEFAULT_INCLUDE,
              exclude: Sequence[str] = DEFAULT_EXCLUDE) -> Path: ...
    def cleanup(self) -> None: ...

build() creates a temp dir, writes pyrightconfig.json into it, returns the dir. The config uses absolute paths into repo_root:

{
  "include":  ["<repo>/src", "<repo>/tests", "<repo>/scripts"],
  "exclude":  ["**/__pycache__", "**/*_pb2*.py", "**/*_pb2*.pyi"],
  "extraPaths": ["<repo>/src"],
  "venvPath": "<repo>", "venv": ".venv",
  "typeCheckingMode": "off",
  "useLibraryCodeForTypes": true
}

typeCheckingMode: "off" is required (REQ-109-007 / SR-109-003): we want references, not diagnostics, and it keeps basedpyright "based extras" out of the picture so scripts/pyright_ratchet.py --max 283 is unaffected. Never write a basedpyright-specific key into the repo's own pyrightconfig.json — never touch that file at all.

class ResolutionPreflight:
    def __init__(self, tracer: Tracer | None = None) -> None: ...
    def check(self, server: PyrightLangServer, target: SymbolTarget,
              candidates: Sequence[ResidueItem]) -> tuple[bool, tuple[str, ...]]: ...

Returns (ok, reasons). Three checks, all must pass:

  1. REQ-109-009 — for a candidate file that imports target.module, issue textDocument/definition on that import name; the resolved path must be the same on-disk file the engine opened as the definition. A mismatch returns the two paths in reasons (this is spike S2).
  2. REQ-109-010 — if a candidate file's import of target.module resolves to nothing, that is UNRESOLVED_IMPORTS.
  3. REQ-109-011 — if candidates exist in files other than the defining file but the rename plan touches only the defining file, that is the S2 signature.

check() is pure diagnosis — it never writes and never raises for a normal "no" answer.

3.4 symbol_rename.py

class SymbolRenamer:
    def __init__(self, server: PyrightLangServer,
                 preflight: ResolutionPreflight,
                 tracer: Tracer | None = None) -> None: ...
    def plan(self, target: SymbolTarget) -> WorkspaceEditPlan: ...

plan():

  1. Resolve target.module to a file under repo_root (dotted path → path under the source root). Not found ⇒ raise SymbolNotFoundError listing near misses (REQ-109-105).
  2. ast.parse that file, locate the definition of target.name at module scope (or in the named class for kind="method"). Compute the 0-based line/col of the identifier, not the class/def keyword.
  3. server.open_document(path); server.wait_until_settled().
  4. prepare_rename — None ⇒ raise RenameRefusedError with the position (REQ-109-013).
  5. server.rename(...) → WorkspaceEditPlan.
  6. Assert no edit lands in a file that defines a different symbol of the same name (REQ-109-016).

Preflight is invoked by the caller (B3's executor) with the candidate set, since B1 does not own the scanner. SymbolRenamer holds the reference so the executor can pass it through.

3.5 Exceptions (define in models.py)

RenameEngineError (base) → LspTimeoutError, LspServerError, SymbolNotFoundError, RenameRefusedError, ResolutionError.

4. Test plan (TDD — write these first, watch them fail, then implement)

Real tests only. Every LSP test spawns a real basedpyright-langserver against a real fixture package on tmp_path. No mocks, no monkeypatch, no unittest.mock, no skips. Function-scope fixtures only. Never xdist.

conftest.py provides a function-scope fixture_package(tmp_path) factory that writes a small package and returns its root, plus a lang_server factory that builds a config root via WorkspaceConfigBuilder and yields a started PyrightLangServer (torn down in the fixture's finalizer).

Gate the LSP tests on shutil.which("basedpyright-langserver") being present — but do not pytest.skip. It is a declared dev dependency; if it is missing the test must fail with a clear message. (Skips are forbidden suite-wide.)

Test ID Behaviour
test_framing_roundtrip_request_response TST-109-001 a real initialize returns capabilities.renameProvider.prepareProvider is True
test_transport_answers_workspace_configuration TST-109-001 the handshake completes and ≥1 workspace/configuration server request was answered
test_request_timeout_raises TST-109-001 a bogus method with timeout=0.1 raises LspTimeoutError
test_close_reaps_process TST-109-001 after close(), proc.poll() is not None; a second close() is a no-op
test_generated_config_has_absolute_paths TST-109-002 generated pyrightconfig.json include/extraPaths are absolute and land inside repo_root; typeCheckingMode == "off"
test_repo_pyrightconfig_untouched TST-109-002 the real repo pyrightconfig.json bytes are identical before/after a full build+cleanup
test_cleanup_removes_temp_root TST-109-002 the temp root does not exist after cleanup(), and cleanup is idempotent
test_rename_disambiguates_same_named_twin TST-109-003 the motivating case. Fixture: mod_a.Neo4jConfig, mod_b.Neo4jConfig, consumer_a imports A, consumer_b imports B. Rename A → plan touches mod_a + consumer_a only; mod_b/consumer_b absent
test_rename_requires_open_document TST-109-003 renaming without open_document yields ≤1 file; with it, ≥2 (asserts spike S1 explicitly)
test_prepare_rename_null_refuses TST-109-004 a position on a comment/blank line raises RenameRefusedError; no file on disk changed
test_symbol_not_found_lists_near_misses TST-109-004 unknown symbol raises SymbolNotFoundError whose message names a near miss
test_preflight_detects_foreign_resolution TST-109-005 fixture where the package also exists at a second path injected via a .pth-equivalent extraPaths ordering — check() returns (False, ...) naming both paths; nothing written
test_preflight_detects_defining_file_only_plan TST-109-005 candidates in 3 files, plan touches 1 ⇒ (False, ...)
test_tracer_injection_no_stdout TST-109-015 with an injected TraceConfig, capsys shows no stdout from engine components

Namespace any temp artefacts with a test_id (uuid4 hex) if you touch shared paths. Everything here is tmp_path-scoped, so no infra isolation is needed.

5. Verification

cd /home/mohamed/workspace/wt-ENGINE
export PYTHONPATH=$(pwd)/src

# your tests, live-observed (mandatory — never run bare pytest)
.venv/bin/python -m pytest tests/refactor/rename/ -p orchestrata_ultimate.core.threading.pytest_live_observer -v

uv run black src/orchestrata_ultimate/refactor/rename/ tests/refactor/rename/
uv run ruff check src/orchestrata_ultimate/refactor/rename/ tests/refactor/rename/
.venv/bin/python scripts/serialize_codebase_ast.py check-deps --include-tests
.venv/bin/python scripts/serialize_codebase_ast.py standards
.venv/bin/python scripts/serialize_codebase_ast.py check-traces
.venv/bin/python scripts/test-review.py tests/refactor/rename/

# the ratchet MUST still pass unchanged — this is SR-109-003
.venv/bin/python scripts/pyright_ratchet.py --max 283

6. Hard rules