From bc361ad56acbd7e3ae09efd3492875eab20207e0 Mon Sep 17 00:00:00 2001 From: vitali87 Date: Sun, 20 Sep 2026 22:56:51 +0100 Subject: [PATCH 1/3] feat: expose change-signature via API, CLI, and MCP --- codebase_rag/cli.py | 63 ++++++++++- codebase_rag/cli_help.py | 17 +++ codebase_rag/editing/__init__.py | 12 +++ codebase_rag/editing/signature.py | 132 ++++++++++++++++++++++++ codebase_rag/mcp/tools.py | 108 +++++++++++++++++++ codebase_rag/tools/tool_descriptions.py | 18 ++++ 6 files changed, 349 insertions(+), 1 deletion(-) create mode 100644 codebase_rag/editing/signature.py diff --git a/codebase_rag/cli.py b/codebase_rag/cli.py index dc5be3c945..6a2a0d2afa 100644 --- a/codebase_rag/cli.py +++ b/codebase_rag/cli.py @@ -1214,7 +1214,68 @@ def rename_command( payload = dict(report._asdict()) payload[cs.KEY_SITES] = sites_for(report.sites) payload[cs.KEY_AMBIGUOUS] = sites_for(report.ambiguous) - payload["verdict"] = report.verdict._asdict() if report.verdict else None + if report.verdict is not None: + payload[cs.KEY_VERDICT] = report.verdict._asdict() + typer.echo(json.dumps(payload, indent=cs.MCP_JSON_INDENT, sort_keys=True)) + if not report.applied and not dry_run: + raise typer.Exit(code=1) + + +@app.command( + name=ch.CLICommandName.CHANGE_SIGNATURE, + help=ch.CMD_CHANGE_SIGNATURE, + short_help=ch.CMD_CHANGE_SIGNATURE, + epilog=ch.EXAMPLES_CHANGE_SIGNATURE, + rich_help_panel=ch.PANEL_USE, +) +def change_signature_command( + qualified_name: str = typer.Argument(..., help=ch.HELP_SIGNATURE_QN), + param: list[str] = typer.Option([], "--param", "-p", help=ch.HELP_SIGNATURE_PARAM), + repo_path: Path = typer.Option( + Path(cs.MCP_DEFAULT_DIRECTORY), + "--repo-path", + exists=True, + file_okay=False, + help=ch.HELP_GRAPH_REPO_PATH, + ), + project: str | None = typer.Option(None, "--project", help=ch.HELP_GRAPH_PROJECT), + allow_heuristic: bool = typer.Option( + False, "--allow-heuristic", help=ch.HELP_RENAME_ALLOW_HEURISTIC + ), + dry_run: bool = typer.Option(False, "--dry-run", help=ch.HELP_RENAME_DRY_RUN), +) -> None: + from .editing.signature import SignatureRefused, change_signature, sites_for + from .graph_cli import _project_and_fetch + + name, fetch_all, ingestor = _project_and_fetch(project, repo_path) + with ingestor: # type: ignore[attr-defined] + parsers, queries = load_parsers() + updater = GraphUpdater( + ingestor=ingestor, # type: ignore[arg-type] + repo_path=repo_path.resolve(), + parsers=parsers, + queries=queries, + project_name=name, + ) + try: + report = change_signature( + repo_path.resolve(), + fetch_all, + name, + qualified_name, + param, + allow_heuristic=allow_heuristic, + dry_run=dry_run, + reingest=updater.reingest, + ) + except SignatureRefused as refused: + typer.echo(str(refused), err=True) + raise typer.Exit(code=1) from refused + payload = dict(report._asdict()) + payload[cs.KEY_SITES] = sites_for(report.sites) + payload[cs.KEY_UNMAPPED] = sites_for(report.unmapped) + if report.verdict is not None: + payload[cs.KEY_VERDICT] = report.verdict._asdict() typer.echo(json.dumps(payload, indent=cs.MCP_JSON_INDENT, sort_keys=True)) if not report.applied and not dry_run: raise typer.Exit(code=1) diff --git a/codebase_rag/cli_help.py b/codebase_rag/cli_help.py index bca9bec57e..e8c2f2f2fb 100644 --- a/codebase_rag/cli_help.py +++ b/codebase_rag/cli_help.py @@ -21,6 +21,7 @@ class CLICommandName(StrEnum): GRAPH = "graph" CHECK = "check" RENAME = "rename" + CHANGE_SIGNATURE = "change-signature" STOP = "stop" STATUS = "status" HELP = "help" @@ -145,6 +146,21 @@ class CLICommandName(StrEnum): "Rewrite through heuristic, overload and trace-only sites as well." ) HELP_RENAME_DRY_RUN = "Print the plan and diff without writing anything." +CMD_CHANGE_SIGNATURE = ( + "Change a definition's parameter list and rewrite every call site the " + "graph knows per an explicit mapping; unmappable sites are listed." +) +EXAMPLES_CHANGE_SIGNATURE = ( + "Examples:\n cgr change-signature myproj.pkg.util.helper -p a@0 -p n:int=1 -p b@1\n" + " cgr change-signature myproj.pkg.Store.get -p key@key -p default=None --dry-run" +) +HELP_SIGNATURE_QN = "Qualified name of the function or method whose signature changes." +HELP_SIGNATURE_PARAM = ( + "A new parameter, in order: name\\[:annotation]\\[=default]\\[@source] where " + "@2 maps from old index 2, @old from the old parameter old, =literal " + "without a source is inserted where a site passes nothing, and a bare " + "name is unmapped." +) CMD_TRACE_INGEST = "Resolve a trace file against a project and write dynamic edges" CMD_TRACE_CONVERT = "Convert a V8 .cpuprofile (node --cpu-prof) to a trace file" @@ -466,6 +482,7 @@ class CLICommandName(StrEnum): CLICommandName.GRAPH: CMD_GRAPH, CLICommandName.CHECK: CMD_CHECK, CLICommandName.RENAME: CMD_RENAME, + CLICommandName.CHANGE_SIGNATURE: CMD_CHANGE_SIGNATURE, CLICommandName.WORKSPACE: CMD_WORKSPACE, CLICommandName.STOP: CMD_STOP, CLICommandName.STATUS: CMD_STATUS, diff --git a/codebase_rag/editing/__init__.py b/codebase_rag/editing/__init__.py index 3786e8b039..c1c80f48d6 100644 --- a/codebase_rag/editing/__init__.py +++ b/codebase_rag/editing/__init__.py @@ -21,6 +21,13 @@ formatter_check, line_col_to_byte, ) +from .signature import ( + ParamSpec, + SignatureRefused, + SignatureReport, + change_signature, + parse_param_spec, +) from .transaction import ( EditTransaction, StagedFile, @@ -36,6 +43,11 @@ __all__ = [ "Expectation", + "ParamSpec", + "SignatureRefused", + "SignatureReport", + "change_signature", + "parse_param_spec", "Verdict", "change_signature_expectation", "measure", diff --git a/codebase_rag/editing/signature.py b/codebase_rag/editing/signature.py new file mode 100644 index 0000000000..62f97ed5e4 --- /dev/null +++ b/codebase_rag/editing/signature.py @@ -0,0 +1,132 @@ +"""`change_signature(qn, new_params)`: edit-algebra operation 2 (issue #1533). + +The definition changes, its callers in other packages do not: that is the +most-cited cross-package breakage. The graph knows every call site with +its argument shape (issue #1522), how each edge was resolved (issue #1526) +and the declared parameter types (issue #1527), so every site can be +rewritten per an explicit mapping or listed as unmapped. + +`new_params` describes the new parameter list in order. Each entry says +where its value comes from at a call site: an old positional index +(`from_index`, receiver excluded), an old parameter name (`from_name`), +a default literal (`literal`, inserted where the site passes nothing), or +nothing at all (unmapped: sites that pass no value are left untouched and +listed). Every definition in the override hierarchy is rewritten too. +""" + +from __future__ import annotations + +from collections.abc import Callable, Iterable +from pathlib import Path + +from .. import constants as cs +from .contract import Reingest, change_signature_expectation, measure, verify +from .signature_planner import SignaturePlanner +from .signature_types import ( + ParamSpec, + RewrittenSite, + SignatureRefused, + SignatureReport, + UnmappedSite, + parse_param_spec, + sites_for, +) +from .transaction import EditTransaction, StagedTree, VerificationResult, undo_last + + +class SignatureChanger(SignaturePlanner): + def apply( + self, qn: str, specs: Iterable[ParamSpec], allow_heuristic: bool = False + ) -> SignatureReport: + # The contract must know the caller waived the guess check, or it + # rolls back every rewrite of a heuristic site it was asked for. + self._allow_heuristic = allow_heuristic + report, patcher = self.plan(qn, specs, allow_heuristic) + tx = EditTransaction(self.repo_root) + results = patcher.stage_into(tx) + broken = [key for key, result in results.items() if result.parses is False] + if broken: + tx.rollback() + return report._replace( + files=tuple(sorted(results)), + message=cs.SIGNATURE_PARSE_FAILED.format(files=", ".join(broken)), + ) + + def verifier(tree: StagedTree) -> VerificationResult | bool | None: + return self.verify(tree) if self.verify is not None else True + + outcome = tx.commit(verifier) + report = report._replace( + applied=outcome.applied, + transaction_id=outcome.transaction_id, + files=outcome.files, + diff=outcome.diff, + message=outcome.message, + ) + if outcome.applied and self.reingest is not None: + report = self._enforce_contract(report) + return report + + def _enforce_contract(self, report: SignatureReport) -> SignatureReport: + assert self.reingest is not None + delta = measure( + self.fetch_all, self.project, self.repo_root, report.files, self.reingest + ) + verdict = verify( + change_signature_expectation( + (f"{u.path}:{u.line}" for u in report.unmapped), + heuristic_allowed=self._allow_heuristic, + ), + delta, + rewritten=[ + (f"{s.path}:{s.line}", s.resolution) + for s in report.sites + if s.before != s.after + ], + ) + if verdict.ok: + return report._replace(verdict=verdict) + undo_last(self.repo_root) + self.reingest(list(report.files)) + return report._replace( + applied=False, + verdict=verdict, + message=cs.SIGNATURE_CONTRACT_FAILED.format( + reasons="; ".join(verdict.failures) + ), + ) + + +def change_signature( + repo_root: Path, + fetch_all: Callable[[str, dict[str, object]], list[dict[str, object]]], + project_name: str, + qualified_name: str, + new_params: Iterable[ParamSpec | str], + allow_heuristic: bool = False, + dry_run: bool = False, + verify: Callable[[StagedTree], VerificationResult | bool | None] | None = None, + reingest: Reingest | None = None, +) -> SignatureReport: + """The op: plan (rewrites definitions and mapped sites) or plan and apply.""" + specs = [parse_param_spec(p) if isinstance(p, str) else p for p in new_params] + changer = SignatureChanger( + repo_root, fetch_all, project_name, verify=verify, reingest=reingest + ) + if dry_run: + report, _patcher = changer.plan(qualified_name, specs, allow_heuristic) + return report + return changer.apply(qualified_name, specs, allow_heuristic) + + +__all__ = [ + "ParamSpec", + "RewrittenSite", + "SignatureChanger", + "SignatureRefused", + "SignatureReport", + "UnmappedSite", + "change_signature", + "parse_param_spec", + "sites_for", +] diff --git a/codebase_rag/mcp/tools.py b/codebase_rag/mcp/tools.py index 44cd2bfc4e..208a071898 100644 --- a/codebase_rag/mcp/tools.py +++ b/codebase_rag/mcp/tools.py @@ -461,6 +461,7 @@ def __init__( self.tests_reaching, ), cs.MCPToolName.RENAME: self._rename_tool(), + cs.MCPToolName.CHANGE_SIGNATURE: self._change_signature_tool(), cs.MCPToolName.QUERY_CODE_GRAPH: ToolMetadata( name=cs.MCPToolName.QUERY_CODE_GRAPH, description=td.MCP_TOOLS[cs.MCPToolName.QUERY_CODE_GRAPH], @@ -2495,6 +2496,113 @@ def prop(kind: cs.MCPSchemaType, description: str) -> MCPInputSchemaProperty: returns_json=True, ) + def _change_signature_tool(self) -> ToolMetadata: + def prop(kind: cs.MCPSchemaType, description: str) -> MCPInputSchemaProperty: + return MCPInputSchemaProperty(type=kind, description=description) + + return ToolMetadata( + name=cs.MCPToolName.CHANGE_SIGNATURE, + description=td.MCP_TOOLS[cs.MCPToolName.CHANGE_SIGNATURE], + input_schema=MCPInputSchema( + type=cs.MCPSchemaType.OBJECT, + properties={ + cs.MCPParamName.QUALIFIED_NAME: prop( + cs.MCPSchemaType.STRING, td.MCP_PARAM_QUALIFIED_NAME + ), + cs.MCPParamName.NEW_PARAMS: MCPInputSchemaProperty( + type=cs.MCPSchemaType.ARRAY, + description=td.MCP_PARAM_NEW_PARAMS, + items={cs.MCPSchemaField.TYPE: cs.MCPSchemaType.STRING}, + ), + cs.MCPParamName.ALLOW_HEURISTIC: prop( + cs.MCPSchemaType.BOOLEAN, td.MCP_PARAM_ALLOW_HEURISTIC + ), + cs.MCPParamName.DRY_RUN: prop( + cs.MCPSchemaType.BOOLEAN, td.MCP_PARAM_RENAME_DRY_RUN + ), + cs.MCPParamName.PROJECT: prop( + cs.MCPSchemaType.STRING, td.MCP_PARAM_PROJECT + ), + }, + required=[cs.MCPParamName.QUALIFIED_NAME, cs.MCPParamName.NEW_PARAMS], + ), + handler=self.change_signature, + returns_json=True, + ) + + async def change_signature( + self, + qualified_name: str, + new_params: list[str], + allow_heuristic: bool = False, + dry_run: bool = False, + project: str | None = None, + ) -> object: + # Same lock discipline as rename: the read, the write and the + # contract's re-ingest see one generation of the graph. + return await self._graph_query( + cs.MCPToolName.CHANGE_SIGNATURE, + project, + lambda name: self._run_change_signature( + name, qualified_name, list(new_params), allow_heuristic, dry_run + ), + ) + + def _run_change_signature( + self, + project_name: str, + qualified_name: str, + new_params: list[str], + allow_heuristic: bool, + dry_run: bool, + ) -> object: + from codebase_rag.editing.signature import ( + SignatureRefused, + change_signature, + sites_for, + ) + + root = graph_query.source_root_for( + self.ingestor.fetch_all, project_name, Path(self.project_root) + ) + if root is None: + return { + cs.DICT_KEY_ERROR: cs.RENAME_WRONG_ROOT.format(project=project_name) + } + try: + report = change_signature( + root, + self.ingestor.fetch_all, + project_name, + qualified_name, + new_params, + allow_heuristic=allow_heuristic, + dry_run=dry_run, + reingest=self._reingest_for_contract(project_name), + ) + except SignatureRefused as refused: + return {cs.DICT_KEY_ERROR: str(refused)} + payload = dict(report._asdict()) + payload[cs.KEY_SITES] = sites_for(report.sites) + payload[cs.KEY_UNMAPPED] = sites_for(report.unmapped) + if report.verdict is not None: + payload[cs.KEY_VERDICT] = report.verdict._asdict() + return payload + + def _reingest_for_contract( + self, project_name: str + ) -> Callable[[list[str]], ReingestReport] | None: + """The live updater's re-ingest for an edit's postcondition contract. + + A project that is not indexed has no graph to measure against and + the operation runs without the contract (issue #1531). + """ + if self._live_updater is not None or ( + project_name in self.ingestor.list_projects() + ): + return self._updater_for_reingest(project_name).reingest + return None + async def rename( self, qualified_name: str, diff --git a/codebase_rag/tools/tool_descriptions.py b/codebase_rag/tools/tool_descriptions.py index e41f4c2c96..93706cc42e 100644 --- a/codebase_rag/tools/tool_descriptions.py +++ b/codebase_rag/tools/tool_descriptions.py @@ -249,6 +249,23 @@ class AgenticToolName(StrEnum): "Rewrite through heuristic, overload and trace-only sites too (default false)." ) MCP_PARAM_RENAME_DRY_RUN = "Plan only: report the sites and diff, write nothing." +MCP_CHANGE_SIGNATURE = ( + "Change a function's or method's parameter list and rewrite every call " + "site the graph knows per an explicit mapping. `new_params` lists the " + "new parameters in order as `name[:annotation][=default][@source]`: " + "`@2` takes the value from old positional index 2, `@old` from the old " + "parameter `old`, `=literal` without a source inserts that literal where " + "a site passes nothing (and becomes the definition's default), and a " + "bare name is unmapped: sites passing no value for it are left untouched " + "and listed. Overrides in the hierarchy are rewritten too; sites resolved " + "by guesswork are listed as unmapped unless `allow_heuristic`. A default " + "literal that does not fit the declared type is refused. Applied through " + "one transaction and held to the postcondition contract; set `dry_run` to " + "see the plan." +) +MCP_PARAM_NEW_PARAMS = ( + "The new parameter list, in order, as `name[:annotation][=default][@source]` specs." +) MCP_QUERY_CODE_GRAPH = ( "Prefer the deterministic tools (resolve, definition, callers, callees, " @@ -433,6 +450,7 @@ class AgenticToolName(StrEnum): MCPToolName.IMPORTERS: MCP_IMPORTERS, MCPToolName.TESTS_REACHING: MCP_TESTS_REACHING, MCPToolName.RENAME: MCP_RENAME, + MCPToolName.CHANGE_SIGNATURE: MCP_CHANGE_SIGNATURE, MCPToolName.QUERY_CODE_GRAPH: MCP_QUERY_CODE_GRAPH, MCPToolName.GET_CODE_SNIPPET: MCP_GET_CODE_SNIPPET, MCPToolName.SURGICAL_REPLACE_CODE: MCP_SURGICAL_REPLACE_CODE, From bcc4c7df41d8b63ddc4e9435c80a7f5cc2d1ea43 Mon Sep 17 00:00:00 2001 From: vitali87 Date: Sun, 20 Sep 2026 23:08:06 +0100 Subject: [PATCH 2/3] fix(mcp): guard and classify change_signature after the rebase onto main Two defects the rebase surfaced, both from the branch being written against a much older `mcp/tools.py`. `_reingest_for_contract` called `_updater_for_reingest`, which no longer exists, and passed the re-ingest callback RAW. Main replaced that with `_guarded_rename_reingest`, which wraps the callback in the incomplete-run marker: mark before the first write, clear after, refuse rather than start if the marker cannot be written. Passing it raw is the failure Greptile caught on PR #1547 -- a process death part way through leaves a partial graph a restarted server reads as complete. Now delegates to it. `change_signature` was a graph reader that never refused on a partial graph. `_NOT_GRAPH_READERS` is deliberately fail-closed (PR #1547): a new tool counts as a reader until classified, and `test_every_graph_reader_is_guarded` fails until it is guarded or listed. It is an EDIT, like RENAME which sits in that set already -- it runs its own re-ingest behind the marker and holds the result to the postcondition contract, so refusing up front on a partial graph would block the operation that repairs it. Verified: 343 passed across test_mcp_project_scoping, test_rename_op, test_edit_contract and test_change_signature. Deleting the classification line reddens test_every_graph_reader_is_guarded naming change_signature. --- codebase_rag/mcp/tools.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/codebase_rag/mcp/tools.py b/codebase_rag/mcp/tools.py index 208a071898..c6a725f9e8 100644 --- a/codebase_rag/mcp/tools.py +++ b/codebase_rag/mcp/tools.py @@ -119,6 +119,11 @@ def _read_file_slice(full_path: Path, start: int, limit: int | None) -> str: cs.MCPToolName.UPDATE_REPOSITORY, cs.MCPToolName.REINGEST, cs.MCPToolName.RENAME, + # Same shape as RENAME: a graph-driven EDIT, not a reader. It runs + # its own re-ingest behind the incomplete-run marker and holds the + # result to the postcondition contract, so refusing up front on a + # partial graph would block the operation that repairs it. + cs.MCPToolName.CHANGE_SIGNATURE, cs.MCPToolName.SURGICAL_REPLACE_CODE, cs.MCPToolName.READ_FILE, cs.MCPToolName.WRITE_FILE, From 04d6eeedabc0d25b8b03d72c0fa3be23bd86bd81 Mon Sep 17 00:00:00 2001 From: vitali87 Date: Tue, 22 Sep 2026 22:57:22 +0100 Subject: [PATCH 3/3] fix(mcp): delegate the contract re-ingest to the guarded helper, and type fetch_all as QueryFn `_reingest_for_contract` still called `_updater_for_reingest`, which the base deleted under #1783, so every call raised AttributeError and ty reported it unresolved. It now delegates to `_guarded_rename_reingest`, as the owner's e5e9f1c1 does on the source branch: that helper returns None for an unindexed project and wraps the callback in the incomplete-run marker. `change_signature` annotated `fetch_all` with a hand-written Callable that matched neither `MemgraphIngestor.fetch_all` nor `SignaturePlanner`, which takes `QueryFn`. That gave three invalid-argument-type errors, and `QueryFn` clears all three. --- codebase_rag/editing/signature.py | 3 ++- codebase_rag/mcp/tools.py | 18 +++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/codebase_rag/editing/signature.py b/codebase_rag/editing/signature.py index 62f97ed5e4..e681356b56 100644 --- a/codebase_rag/editing/signature.py +++ b/codebase_rag/editing/signature.py @@ -20,6 +20,7 @@ from pathlib import Path from .. import constants as cs +from ..graph_query import QueryFn from .contract import Reingest, change_signature_expectation, measure, verify from .signature_planner import SignaturePlanner from .signature_types import ( @@ -99,7 +100,7 @@ def _enforce_contract(self, report: SignatureReport) -> SignatureReport: def change_signature( repo_root: Path, - fetch_all: Callable[[str, dict[str, object]], list[dict[str, object]]], + fetch_all: QueryFn, project_name: str, qualified_name: str, new_params: Iterable[ParamSpec | str], diff --git a/codebase_rag/mcp/tools.py b/codebase_rag/mcp/tools.py index c6a725f9e8..c485be20d1 100644 --- a/codebase_rag/mcp/tools.py +++ b/codebase_rag/mcp/tools.py @@ -2597,16 +2597,16 @@ def _run_change_signature( def _reingest_for_contract( self, project_name: str ) -> Callable[[list[str]], ReingestReport] | None: - """The live updater's re-ingest for an edit's postcondition contract. - - A project that is not indexed has no graph to measure against and - the operation runs without the contract (issue #1531). + """The re-ingest an edit's postcondition contract measures against. + + Delegates to `_guarded_rename_reingest`, which returns None for a + project with no graph to measure (issue #1531) and wraps the callback + in the incomplete-run marker. Passing the raw callback here would + reopen the failure Greptile caught on PR #1547: a process death part + way through the re-ingest leaves a partial graph that a restarted + server reads as complete. """ - if self._live_updater is not None or ( - project_name in self.ingestor.list_projects() - ): - return self._updater_for_reingest(project_name).reingest - return None + return self._guarded_rename_reingest(project_name) async def rename( self,