diff --git a/codebase_rag/cli.py b/codebase_rag/cli.py index dc5be3c94..6a2a0d2af 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 bca9bec57..e8c2f2f2f 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 3786e8b03..c1c80f48d 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 000000000..e681356b5 --- /dev/null +++ b/codebase_rag/editing/signature.py @@ -0,0 +1,133 @@ +"""`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 ..graph_query import QueryFn +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: QueryFn, + 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 44cd2bfc4..c485be20d 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, @@ -461,6 +466,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 +2501,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 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. + """ + return self._guarded_rename_reingest(project_name) + async def rename( self, qualified_name: str, diff --git a/codebase_rag/tools/tool_descriptions.py b/codebase_rag/tools/tool_descriptions.py index e41f4c2c9..93706cc42 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,