Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions codebase_rag/constants/ast_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@
# tree-sitter parses comments as named children, so the flow walk filters them
# out before indexing arguments or reading a single sub-expression.
TS_COMMENT = "comment"
TS_SPREAD_ELEMENT = "spread_element"
# `(expr)` wraps its value in a parenthesized_expression; the flow walk unwraps
# it (like await) to reach the inner source/tainted expression.
TS_PARENTHESIZED_EXPRESSION = "parenthesized_expression"
Expand Down
3 changes: 3 additions & 0 deletions codebase_rag/constants/ast_python.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@
TS_PY_DEFAULT_PARAMETER = "default_parameter"
TS_PY_LIST_SPLAT_PATTERN = "list_splat_pattern"
TS_PY_DICTIONARY_SPLAT_PATTERN = "dictionary_splat_pattern"
# Call-site splats (`*args`, `**kwargs` in an argument list).
TS_PY_LIST_SPLAT = "list_splat"
TS_PY_DICTIONARY_SPLAT = "dictionary_splat"
TS_PY_POSITIONAL_SEPARATOR = "positional_separator"
TS_PY_KEYWORD_SEPARATOR = "keyword_separator"
TS_PY_SUBSCRIPT = "subscript"
Expand Down
45 changes: 45 additions & 0 deletions codebase_rag/constants/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,51 @@ class DiffMarker:
RENAME_CONTRACT_UNMEASURED = (
"Rename applied, but its postcondition could not be measured: {error}"
)
# change_signature (issue #1533).
SIGNATURE_BAD_SPEC = (
"Not a parameter spec: {spec!r} (expected name[:annotation][=default][@source])"
)
SIGNATURE_UNKNOWN = "No function or method named {qn} in the graph"
SIGNATURE_NO_GRAMMAR = "No grammar for {path}; the signature cannot be rewritten"
SIGNATURE_NO_DEFINITION_TOKEN = "Could not locate the definition of {qn} in {path}"
SIGNATURE_DUPLICATE_PARAM = "Parameter {name} is listed twice"
SIGNATURE_POSITIONAL_ONLY_MOVED = (
"The definition marks its leading parameters positional-only with `/`, "
"and this rewrite moves a parameter across that boundary. Rebuilding it "
"would change which arguments callers may pass by name. Rewrite the "
"parameters before `/` in place, or remove the marker first."
)
SIGNATURE_DUPLICATE_SOURCE = (
"Parameters {names} all map to {source}, which holds one value per "
"call site: mapping it to several would leave every caller short of "
"arguments. Map each new parameter to its own source, or give the "
"extras a literal default."
)
SIGNATURE_UNKNOWN_SOURCE = (
"No old parameter {source!r}; the old parameters are: {names}"
)
SIGNATURE_LITERAL_TYPE = (
"Default {literal} for {name} does not fit its declared type {annotation}"
)
SIGNATURE_PLANNED = "{count} call site(s) would be rewritten, {unmapped} left unmapped"
SIGNATURE_PARSE_FAILED = "Signature change rolled back: {files} would no longer parse"
SIGNATURE_CONTRACT_FAILED = (
"Signature change rolled back, postcondition failed: {reasons}"
)
SIGNATURE_UNLOCATABLE = "site cannot be located"
SIGNATURE_GUESSED = "resolved by {resolution}; pass allow_heuristic to rewrite"
SIGNATURE_MISSING_FILE = "file is missing"
SIGNATURE_NO_CALL = "no call with an argument list at the recorded site"
SIGNATURE_SPLAT = "the site spreads its arguments"
SIGNATURE_VARIADIC = (
"Cannot change the signature of {qn}: it takes *args or **kwargs, so its call "
"sites cannot be mapped one value per parameter"
)
SIGNATURE_UNKNOWN_KEYWORD = "the site passes an unknown keyword {name}"
SIGNATURE_NEEDS_KEYWORDS = "the mapping needs keyword arguments this language lacks"
SIGNATURE_UNMAPPED_PARAM = "the site passes no value for {name}"
SIGNATURE_SURPLUS_ARGS = "the site passes more arguments than the definition declares"
SIGNATURE_DEFAULT_ORDER = "Parameter {name} has no default but follows one that does"
# Postcondition contract (issue #1531).
CONTRACT_OP_RENAME = "rename"
CONTRACT_OP_CHANGE_SIGNATURE = "change_signature"
Expand Down
2 changes: 2 additions & 0 deletions codebase_rag/constants/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
KEY_SITES = "sites"
KEY_AMBIGUOUS = "ambiguous"
KEY_STRUCTURAL_DELTA = "structural_delta"
KEY_UNMAPPED = "unmapped"
KEY_VERDICT = "verdict"
KEY_DISPATCH_LITERAL = "dispatch_literal"


Expand Down
2 changes: 2 additions & 0 deletions codebase_rag/constants/mcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class MCPToolName(StrEnum):
TESTS_REACHING = "tests_reaching"
# Graph-driven edit operations (issue #1532).
RENAME = "rename"
CHANGE_SIGNATURE = "change_signature"
QUERY_CODE_GRAPH = "query_code_graph"
GET_CODE_SNIPPET = "get_code_snippet"
SURGICAL_REPLACE_CODE = "surgical_replace_code"
Expand Down Expand Up @@ -102,6 +103,7 @@ class MCPParamName(StrEnum):
MODULE_QN = "module_qualified_name"
NEW_NAME = "new_name"
ALLOW_HEURISTIC = "allow_heuristic"
NEW_PARAMS = "new_params"


# MCP server constants
Expand Down
6 changes: 6 additions & 0 deletions codebase_rag/cypher_queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,12 @@ def build_create_relationship_query(
AND t.qualified_name STARTS WITH $project_prefix
RETURN DISTINCT m.qualified_name AS from_qn, m.path AS from_path,
t.qualified_name AS to_qn"""
# The declared parameter shape of one definition (issues #1527, #1533).
CYPHER_GRAPH_SIGNATURE = """MATCH (n:Function|Method)
WHERE n.qualified_name = $qn AND n.qualified_name STARTS WITH $project_prefix
RETURN n.positional_params AS positional_params, n.param_types AS param_types,
n.return_type AS return_type
LIMIT 1"""
CYPHER_GRAPH_IMPORTERS = """MATCH (m:Module)-[r:IMPORTS]->(target)
WHERE target.qualified_name = $qn AND m.qualified_name STARTS WITH $project_prefix
RETURN m.qualified_name AS qualified_name, m.path AS path, r.line AS line,
Expand Down
5 changes: 4 additions & 1 deletion codebase_rag/editing/contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,15 @@ def rename_expectation(
)


def change_signature_expectation(unmapped: Iterable[str]) -> Expectation:
def change_signature_expectation(
unmapped: Iterable[str], heuristic_allowed: bool = False
) -> Expectation:
return Expectation(
operation=cs.CONTRACT_OP_CHANGE_SIGNATURE,
unmapped=tuple(sorted(set(unmapped))),
# A parameter added with a default changes no caller count.
caller_count_unchanged=True,
heuristic_allowed=heuristic_allowed,
)


Expand Down
136 changes: 136 additions & 0 deletions codebase_rag/editing/signature_calls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
"""Call-site argument mapping for change_signature."""

from __future__ import annotations

from tree_sitter import Node

from .. import constants as cs
from .. import graph_query
from ..parsers.call_processor import _find_call_arguments_node, _split_call_arguments
from ..types_defs import ResultRow
from .signature_params import _text
from .signature_types import ParamSpec

_SPLAT_TYPES = frozenset(
{
cs.TS_PY_LIST_SPLAT,
cs.TS_PY_DICTIONARY_SPLAT,
cs.TS_SPREAD_ELEMENT,
}
)


def _site_end(row: graph_query.CallSiteRow | ResultRow) -> tuple[int, int] | None:
"""The site's recorded end point as tree-sitter counts it, if recorded."""
end_line, end_col = row.get("end_line"), row.get("end_col")
if isinstance(end_line, int) and isinstance(end_col, int):
return (end_line - 1, end_col)
return None


def _call_at(
root: Node, line: int, col: int, end: tuple[int, int] | None = None
) -> Node | None:
"""The call node at (line, col) that carries arguments.

With the site's recorded end point the exact call is chosen, so a
chained `helper(2).upper()` rewrites `helper`'s arguments and not the
outer call's; without it the outermost call at that point is taken.
"""
stack = [root]
found: Node | None = None
while stack:
node = stack.pop()
if (
node.start_point == (line - 1, col)
and _find_call_arguments_node(node) is not None
):
if end is not None:
if node.end_point == end:
return node
elif found is None or node.end_byte > found.end_byte:
found = node
if node.start_point[0] <= line - 1 <= node.end_point[0]:
stack.extend(node.children)
return found


def _map_arguments(
args_node: Node,
old_names: list[str],
specs: list[ParamSpec],
language: cs.SupportedLanguage | None,
keyword_only: frozenset[str] = frozenset(),
) -> list[str] | str:
"""The site's new argument texts, or the reason it cannot be mapped.

A keyword naming a keyword-only parameter is not part of the positional
mapping and is carried through exactly as written.
"""
positional, keyword = _split_call_arguments(args_node)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Preserve C# named arguments

For F(y: 2, x: 1), the changed mapping receives 2 and 1 as positional arguments after the C# argument wrappers discard y: and x:. Reordering the declaration from (x, y) to (y, x) then emits F(1, 2), which reverses the original bindings and changes program behavior. Preserve the C# argument names during mapping, or refuse to rewrite named C# call sites. This must be fixed before merging.

Knowledge Base Used: Multi-language parsing (codebase_rag/parsers)

Artifacts

Evidence from the check

  • Authored harness extracts and executes the changed splitter and mapper against the C# named-argument AST shape, showing the binding-changing rewrite.

Evidence from the check

  • Command wrapper runs the authored C# named-argument harness and captures the before and after evidence logs.

Command output from the check

  • The executed Git command confirms the newly added signature-call module is absent from the parent commit, establishing the pre-change baseline.

Command output from the check

  • The executed harness shows C# `F(y: 2, x: 1)` split as positional `2, 1` and rewritten to `F(1, 2)`, confirming changed bindings.

Command output from the check

  • The captured command output contains the exact authored harness and wrapper used to produce the observed result, making the reproduction traceable.

View artifacts

T-Rex Ran code and verified through T-Rex

if any(child.type in _SPLAT_TYPES for child in args_node.named_children):
return cs.SIGNATURE_SPLAT
Comment on lines +70 to +72
values: dict[int, tuple[str, bool]] = {}
for index, node in enumerate(positional):
values[index] = (_text(node), False)
carried: list[str] = []
for name, node in keyword.items():
if name in keyword_only:
carried.append(f"{name}={_text(node)}")
continue
if name not in old_names:
return cs.SIGNATURE_UNKNOWN_KEYWORD.format(name=name)
values[old_names.index(name)] = (_text(node), True)
keywords_ok = language == cs.SupportedLanguage.PYTHON
if not positional and _keywords_already_fit(values, old_names, specs):
# A keyword-only site binds by name: reordering would change
# nothing but the spelling, so it is left exactly as written.
return [_text(child) for child in args_node.named_children]
out: list[str] = []
keyword_mode = False
for spec in specs:
if spec.from_index is not None:
found = values.pop(spec.from_index, None)
if found is None:
# The site relied on the old default; every later value
# must be spelled by name so positions do not shift.
keyword_mode = True
continue
text, was_keyword = found
if was_keyword or keyword_mode:
if not keywords_ok:
return cs.SIGNATURE_NEEDS_KEYWORDS
out.append(f"{spec.name}={text}")
keyword_mode = True
else:
out.append(text)
elif spec.literal is not None:
if keyword_mode:
if not keywords_ok:
return cs.SIGNATURE_NEEDS_KEYWORDS
out.append(f"{spec.name}={spec.literal}")
else:
out.append(spec.literal)
else:
return cs.SIGNATURE_UNMAPPED_PARAM.format(name=spec.name)
# Every spec consumed what it named; anything still in `values` is an
# argument the new signature has no home for. Rewriting would drop it
# from the caller's source silently -- and the contract cannot catch
# that, because the argument is gone from the file before the delta is
# measured, so the `too_many` arity check sees nothing.
if values:
return cs.SIGNATURE_SURPLUS_ARGS
return out + carried


def _keywords_already_fit(
values: dict[int, tuple[str, bool]], old_names: list[str], specs: list[ParamSpec]
) -> bool:
"""Every passed keyword keeps its name and nothing new must be inserted."""
kept = {spec.from_index: spec.name for spec in specs if spec.from_index is not None}
if any(index not in kept or kept[index] != old_names[index] for index in values):
return False
return all(spec.from_index is not None for spec in specs)


# --- literal type checks --------------------------------------------------------
55 changes: 55 additions & 0 deletions codebase_rag/editing/signature_literals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
"""Literal/type compatibility checks for change_signature."""

from __future__ import annotations

import ast
import re

_KIND_OF = {
bool: "bool",
int: "int",
float: "float",
str: "str",
list: "list",
dict: "dict",
set: "set",
tuple: "tuple",
type(None): "None",
}
_ACCEPTS: dict[str, frozenset[str]] = {
"int": frozenset({"int"}),
"float": frozenset({"int", "float"}),
"str": frozenset({"str"}),
"bool": frozenset({"bool"}),
"list": frozenset({"list"}),
"dict": frozenset({"dict"}),
"set": frozenset({"set"}),
"tuple": frozenset({"tuple"}),
"bytes": frozenset(),
}


def _literal_kind(literal: str) -> str | None:
try:
value = ast.literal_eval(literal)
except (ValueError, SyntaxError):
return None
return _KIND_OF.get(type(value))


def _compatible(kind: str, annotation: str) -> bool:
"""Whether a literal of `kind` fits `annotation` (builtin names only)."""
text = annotation.strip()
if kind == "None":
return "None" in text or text.startswith("Optional")
members = [m.strip() for m in re.split(r"\s*\|\s*", text)]
if len(members) > 1:
return any(_compatible(kind, member) for member in members)
base = re.match(r"^([A-Za-z_][A-Za-z0-9_]*)", text)
if base is None:
return True
accepted = _ACCEPTS.get(base.group(1))
if accepted is None:
# Not a builtin: the graph cannot tell, so do not accuse.
return True
return kind in accepted
Loading
Loading