Orchestrata Ultimate › Plans › rename-engine-design

Rename Engine — Architecture and Spike Findings (SPEC-109)

Source: plans/rename-engine-design.md at 8a2cb42ad · last changed 2026-07-27 · first committed 2026-07-27 · 510 lines.

Prose pending.

Plan text

Rename Engine — Architecture and Spike Findings (SPEC-109)

Status: design ratified by a passing live spike, 2026-07-27 Spec: specs/109-rename-engine/spec.md Branch: feat/rename-engine Builder briefs: plans/rename-engine-briefs/BRIEF-B{1,2,3}.md

1. Motivation

LLM agents half-apply mechanical renames. The live incident that triggered this work: a gw → slot label rename missed four Redis slot parsers and produced 54 test failures. The rename looked complete — nothing errored.

The failure class is precise: a rename that silently under-applies is indistinguishable from a rename that succeeded. Every design decision below exists to make that state impossible to reach unnoticed.

IDEs solve this with type resolution. Pylance does it well and is proprietary, which the open-source-only ruling forbids. basedpyright (MIT, self-contained PyPI wheel, bundles its own Node runtime via nodejs-wheel-binaries) exposes the same pyright language server we can drive ourselves.

2. Spike — live de-risk, run before the spec was written

Environment: basedpyright 1.39.9 (based on pyright 1.1.411), Python 3.14t, probes run from /home/mohamed/workspace/wt-ENGINE. Probe scripts were throwaway (scratchpad, not committed).

2.1 Probe 1 — synthetic fixture, the motivating case

Fixture: pkg/graph_config.py and pkg/legacy_config.py each define a class named Neo4jConfig; pkg/consumer.py imports from the first, pkg/decoy_consumer.py from the second.

Handshake actually exchanged:

-> initialize            {rootUri, workspaceFolders, capabilities:{workspace:{configuration:true,
                          workspaceEdit:{documentChanges:true}}, textDocument:{rename:{prepareSupport:true}}}}
<- result                capabilities.renameProvider = {"prepareProvider": true, "workDoneProgress": true}
-> initialized           {}
-> workspace/didChangeConfiguration
-> textDocument/didOpen  {uri, languageId:"python", version:1, text}
<- workspace/configuration   (x6 — server asks the CLIENT; must be answered or the handshake stalls)
-> textDocument/prepareRename {position:{line:3,character:6}}
<- result                {"start":{"line":3,"character":6},"end":{"line":3,"character":17}}
-> textDocument/rename   {position, newName:"GraphDbConfig"}
<- result                {"documentChanges":[{textDocument:{uri,version:null}, edits:[...]}]}

Result:

pkg/graph_config.py: 3 edits
pkg/consumer.py:     4 edits
VERDICT: PASS — type-resolved rename works

legacy_config.py and decoy_consumer.py — same class name, different definition — were not touched. Same-name disambiguation confirmed.

2.2 Probe 2 — the real 122K-LOC tree, naive settings → silent under-rename

Target: the real twin pair src/orchestrata_ultimate/knowledge_graph/config.py:50 class Neo4jConfig vs src/orchestrata_ultimate/orchestrator/neo4j_audit.py:81 class Neo4jConfig.

Without didOpen:

prepareRename -> null
rename        -> 1 file, 3 edits   (0.9s total — no workspace search happened)

With didOpen and analysis settled (7.7s):

prepareRename -> {"start":{"line":49,"character":6},"end":{"line":49,"character":17}}
rename        -> 1 file, 3 edits          <-- STILL WRONG

The missed files are unambiguously genuine references:

src/orchestrata_ultimate/knowledge_graph/neo4j_client.py:18
    from orchestrata_ultimate.knowledge_graph.config import Neo4jConfig
src/orchestrata_ultimate/orchestrator/impact/dependency_analyzer.py:11   (same import)
src/orchestrata_ultimate/agents/volks/architect/architect_context.py:57  (same import)
src/orchestrata_ultimate/kernel/pipeline_cli.py:194                      (same import)

No error was raised. This is precisely the 54-test-failure signature.

2.3 Probe 3 — root cause

$ cat .venv/lib/python3.14t/site-packages/_editable_impl_orchestrata_ultimate.pth
/home/mohamed/workspace/orchestrata-ultimate-architecture/src

Pyright reads .pth files in site-packages. In a git worktree, the editable install still points at the main checkout, so orchestrata_ultimate.knowledge_graph.config resolved to a file in a different tree than the one opened. Cross-file references therefore matched nothing.

Supplying python.analysis.extraPaths / basedpyright.analysis.extraPaths over the LSP workspace/configuration channel did not fix it — a pyrightconfig.json at the workspace root overrides language-server settings.

Control experiment: the identical probe against the main checkout, where the .pth does match:

=== WorkspaceEdit: 5 files, 15 edits ===
  agents/volks/architect/architect_context.py: 4
  kernel/pipeline_cli.py: 2
  knowledge_graph/config.py: 3
  knowledge_graph/neo4j_client.py: 3
  orchestrator/impact/dependency_analyzer.py: 3
twin NOT edited: PASS

15 edits = exactly the 18 textual src/ occurrences minus the twin's 3. Zero false positives, zero misses.

Second silent-under-rename axis, same probe: tests/ got 0 rewrites, because the repo's pyrightconfig.json pins include: ["src"]. 24 test files reference the symbol.

2.4 Probe 4 — the fix: an engine-owned config root

Rather than mutate the repo's pyrightconfig.json, the engine generates its own in a temp directory and uses that directory as the LSP workspace root, with absolute paths pointing into the real repo:

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

Result on the real repo:

analysis settled after 13.3s
rename took 0.3s
=== WorkspaceEdit: 25 files, 70 edits ===
src/  files rewritten : 5   (expected 5)
tests/ files rewritten: 20  (textual candidates: 24)
twin edited: False
VERDICT: PASS — engine-owned temp config root controls the file set

The 4 test files not rewritten were verified by hand — every one imports from orchestrator.neo4j_audit, i.e. they reference the twin and correctly must not change:

tests/generated_orchestrator/test_neo4j_audit_generated.py
tests/orchestrator/test_engine_behavioral.py
tests/orchestrator/test_intent_lifecycle_behavioral.py
tests/orchestrator/test_neo4j_audit_behavioral.py

2.5 Spike verdict

PASS. basedpyright is adopted. rope is not needed and is not a dependency. Four findings are normative and are cited by REQ id in the spec:

Non-obvious gotchas worth keeping:

• The server issues workspace/configuration requests to the client (6 of them). An unanswered request stalls the handshake — the client must be bidirectional, not request/response only.
• documentChanges[].textDocument.version is null for files never opened; do not treat that as an error.
• Positions are 0-based for both line and character; end is exclusive.
• Cold analysis on 122K LOC settles in 8–14s; the rename itself is 0.1–0.3s. Cost is per-session, so a plan with many entries must reuse one server session.
• A suspiciously fast rename (<0.5s with no prior settle) is the S2 signature, not good performance.

3. Architecture

3.1 Core idea — two independent oracles

residue = candidates − plan. Every residue item must be explained. A residue item that is a resolvable code reference is classified UNRESOLVED and aborts the run (REQ-109-020). This single rule converts the entire silent-under-rename failure class into a loud, pre-write abort.

The deliberately over-matching name-based machinery — the very thing SPEC-109 supersedes for execution — is retained as the safety net. That is the design's central move.

3.2 Component diagram

Composition only — every collaborator is constructor-injected. The single inheritance edge in the whole design is RefactorRenameTool extending the SPEC-072 BaseTool ABC, which is the framework's required boundary.

3.3 Execution pipeline

load plan  ──▶ validate (all entries, before anything executes)
           ──▶ WorkspaceConfigBuilder.build()   # engine-owned temp pyrightconfig
           ──▶ PyrightLangServer start + initialize (ONE session, reused)
  per entry:
           ──▶ CandidateScanner.scan()          # over-matching oracle
           ──▶ ResolutionPreflight.check()      # S2 guard — abort before writing
           ──▶ SymbolRenamer.plan() / ModuleMover.plan() / StringRefRewriter.plan()
           ──▶ ResidueClassifier.classify()     # candidates − plan
           ──▶ any UNRESOLVED residue?  ──▶ ABORT (exit 1), nothing written
  all entries planned:
           ──▶ dry run?  ──▶ print diff + residue, exit 0
           ──▶ snapshot every target file
           ──▶ EditApplier.apply() for every plan
           ──▶ ast.parse() every touched file    ──▶ failure ⇒ restore snapshot, exit 1
           ──▶ ResidueReporter.render()
           ──▶ cleanup temp config root

Ordering rule: symbols before modules before strings. Symbol renames are computed against the pre-move tree, so moving a file first would invalidate every cached position.

4. Decisions

D1 — basedpyright over rope/jedi

Ratified before the spike, confirmed by it. MIT, self-contained wheel (no system Node required — nodejs-wheel-binaries is pulled in automatically), drop-in with the pyright the repo already gates on. rope was the fallback if the spike failed; it did not, so rope is not a dependency and LGPL never enters the chain.

D2 — "Based extras" stay OFF, and the ratchet is protected

scripts/pyright_ratchet.py (pinned --max 283 in both .pre-commit-config.yaml:84 and .github/workflows/ci.yml:68) and scripts/check_async_await.py both invoke the literal pyright console script. basedpyright installs separate entry points (basedpyright, basedpyright-langserver) and does not shadow pyright, so adding it as a dev dependency changes nothing for those gates.

Three rules keep it that way (SR-109-003):

1. The engine invokes only basedpyright-langserver, never basedpyright or pyright as a checker.
2. The generated config sets typeCheckingMode: "off" — the engine wants references, not diagnostics, so based-extras (reportAny, reportExplicitAny, reportUnreachable, and the recommended default) are irrelevant and unset.
3. No basedpyright-specific key is ever added to the repo's pyrightconfig.json, which vanilla pyright would warn about. The engine's config lives in its own temp root.

Standing hazard to record: the repo's pyrightconfig.json has no typeCheckingMode key. pyright therefore defaults to standard; basedpyright would default to recommended. If anyone ever repoints the ratchet scripts at the basedpyright binary, the 283 baseline explodes. Do not.

D3 — Engine-owned config root, not a repo config edit

Forced by spike S3. Also the right call independently: the rename engine's analysis scope (src + tests + scripts) is deliberately wider than the type gate's (src only), and the two should not be coupled. Non-invasive — no repo file is touched, and it works from a git worktree whose editable .pth points elsewhere.

D4 — Fail-closed, generalized from existing precedent

_type_scoped_method_rename in serialize_codebase_ast.py already implements exactly the right contract (blocked=True ⇒ --apply refused) but only for rename --kind method --class C. SPEC-109 makes fail-closed the default for every kind, with a real type checker behind it.

D5 — Reuse from tools/lsp_tool.py, reject its symbol index

Reuse the wire dataclasses (Position, Range, LSPLocation), the URI helpers (_uri_to_path, _path_to_uri), and the LSPError → ToolResult bridge. Reject PythonSymbolIndex entirely — ast.walk name-string matching is the defect being replaced. Note that lsp_tool.py contains no JSON-RPC layer; its own docstring records that a real LSP client was rejected for its MVP. B1 writes the transport from scratch.

D6 — scripts/serialize_codebase_ast.py is not modified

It stays as-is. Its refs machinery is conceptually reused (re-implemented, not imported) as the CandidateScanner. Both tools ship; a later step may deprecate rename in favour of rename_engine.py.

D7 — One langserver session per run

Cold analysis is 8–14s; renames are 0.1–0.3s. A 20-entry plan reusing one session costs ~15s total instead of ~4 minutes. The session is a context manager so the process is always reaped.

5. Wiring plan (AGENTS.md Hard Rule 8 — built is not done, WIRED is done)

This is a deliberate, declared gap. The constraint for this branch is NEW FILES ONLY (six parallel rename branches merge before it), so the two edits that would make the tool reachable from a volk — agents/tool_registry.py and refactor/__init__.py — cannot be made here. The CLI is a real production seam and ships wired; the volk tool ships built-and-tested but unregistered. Registration is a ~6-line follow-up once the parallel branches land.

6. Builder split

Disjoint file ownership, dependency-ordered. models.py is contract-frozen in the spec so B2 and B3 can code against it without waiting for B1.

No brief edits scripts/serialize_codebase_ast.py, tools/registry.py, agents/tool_registry.py, or any existing __init__.py.

7. Open risks

Related

• Plans
• Orchestrata Ultimate — project index
• Workflow
• Tasks