feat(signature): contract expectation, shared call-site helpers and constants for change_signature (#1533, 1/9)

codebase_rag/constants/ast_python.py

@@ -51,6 +51,24 @@
 TS_PY_DICTIONARY_SPLAT_PATTERN = "dictionary_splat_pattern"
 TS_PY_POSITIONAL_SEPARATOR = "positional_separator"
 TS_PY_KEYWORD_SEPARATOR = "keyword_separator"
+# A definition's parameter list node, and the splats an argument list can
+# carry (`f(*args)`, `f(**opts)`); the `_pattern` forms above are the
+# definition-side spellings.
+TS_PY_PARAMETERS = "parameters"
+TS_PY_LIST_SPLAT = "list_splat"
+TS_PY_DICTIONARY_SPLAT = "dictionary_splat"
+# Scopes nested in a function body, where a parameter's name may be re-bound
+# rather than read, and the statements that re-bind a name in place.
+TS_PY_SET_COMPREHENSION = "set_comprehension"
+TS_PY_DICTIONARY_COMPREHENSION = "dictionary_comprehension"
+TS_PY_GENERATOR_EXPRESSION = "generator_expression"
+TS_PY_NONLOCAL_STATEMENT = "nonlocal_statement"
+# `typing` names a literal-versus-annotation check understands.
+PY_TYPING_OPTIONAL = "Optional"
+PY_TYPING_UNION = "Union"
+# The first parameter of a method that names the instance or class; a
+# signature change keeps it in place and never remaps it.
+PY_RECEIVER_NAMES = frozenset({"self", "cls"})
 TS_PY_SUBSCRIPT = "subscript"
 # The `subscript` node's index field (`os.environ["K"]` -> the `"K"` string).
 TS_PY_FIELD_SUBSCRIPT = "subscript"

codebase_rag/constants/cli.py

@@ -612,6 +612,109 @@ class DiffMarker:
 RENAME_CONTRACT_UNMEASURED = (
     "Rename applied, but its postcondition could not be measured: {error}"
 )
+# Change signature (issue #1533).
+SIGNATURE_PARAM_PROBE = "def _({text}): pass"
+SIGNATURE_NOT_PYTHON = (
+    "Cannot change the signature of {qn}: {path} is not Python, and only Python "
+    "definitions are supported for now (issue #{issue})"
+)
+SIGNATURE_DEFINITION_UNREADABLE = (
+    "Cannot change the signature of {qn}: its file {path} cannot be read "
+    "({error}). The index names a file the tree no longer has; re-index and retry."
+)
+SIGNATURE_NO_HEADER = "Could not locate the definition of {qn} in {path}"
+SIGNATURE_UNSUPPORTED_PARAMS = (
+    "Cannot change the signature of {qn}: `{text}` is not a plain "
+    "positional-or-keyword parameter, and only those can be remapped"
+)
+SIGNATURE_UNUSUAL_RECEIVER = (
+    "Cannot change the signature of {qn}: its first parameter `{name}` is not "
+    "self or cls, so the receiver cannot be told from the parameters; rename "
+    "it to self or cls first"
+)
+SIGNATURE_STAGE_FAILED = "Cannot stage the signature change: {error}"
+SIGNATURE_BAD_PARAM = (
+    "Not a parameter: `{text}` (expected `name`, `name: type`, `name = default` "
+    "or `name: type = default`)"
+)
+SIGNATURE_DUPLICATE_NEW = "Parameter {name} is listed twice"
+SIGNATURE_REQUIRED_AFTER_DEFAULT = (
+    "Parameter {name} has no default but comes after a defaulted parameter"
+)
+SIGNATURE_BODY_REBINDS = (
+    "Cannot rename parameter {old} of {qn} to {new}: `{old}` is used inside a "
+    "nested scope, or re-bound by a global, nonlocal or import statement, in "
+    "its body; rename it by hand first"
+)
+SIGNATURE_DEFAULT_REFERENCES_RENAMED = (
+    "Cannot rename parameter {old} of {qn} to {new}: the default of `{param}` "
+    "reads `{old}`, which is evaluated at definition time and would raise "
+    "NameError; rewrite that default by hand first"
+)
+SIGNATURE_DROPPED_STILL_READ = (
+    "Cannot drop parameter {name} of {qn}: its body still reads `{name}`, "
+    "which would raise NameError when called; remove that use by hand first"
+)
+SIGNATURE_BODY_NAME_TAKEN = (
+    "Cannot rename parameter {old} of {qn} to {new}: `{new}` is already used "
+    "in its body"
+)
+SIGNATURE_HIERARCHY_MISMATCH = (
+    "Cannot change the signature of {qn}: its override {member} declares "
+    "({theirs}) where {qn} declares ({ours}); make the hierarchy agree first"
+)
+SIGNATURE_MAPPING_UNKNOWN_NEW = (
+    "The mapping names {name}, which is not a new parameter ({names})"
+)
+SIGNATURE_MAPPING_UNKNOWN_OLD = (
+    "The mapping feeds {name} from `{source}`, which is neither an old parameter "
+    "({names}), an index into them, nor a `=literal`"
+)
+SIGNATURE_MAPPING_BAD_INDEX = (
+    "The mapping feeds {name} from index {index}, but only {count} old "
+    "parameter(s) exist"
+)
+SIGNATURE_MAPPING_EMPTY_LITERAL = "The mapping gives {name} an empty literal"
+SIGNATURE_MAPPING_DUPLICATE = (
+    "Old parameter {old} cannot feed both {first} and {second}"
+)
+SIGNATURE_LITERAL_MISMATCH = (
+    "{literal} does not fit the declared type of {name} ({annotation})"
+)
+SIGNATURE_BAD_MAP = "Mapping entries take the form NEW=SOURCE, got `{entry}`"
+SIGNATURE_SITE_NO_LOCATION = "site carries no location ({resolution})"
+SIGNATURE_SITE_GUESSED = (
+    "site was resolved by {resolution}; pass allow_heuristic to rewrite it"
+)
+SIGNATURE_SITE_NOT_PYTHON = "{path} is not Python"
+SIGNATURE_SITE_UNREADABLE_FILE = "file cannot be read ({error})"
+SIGNATURE_SITE_NO_CALL = "no call found at the recorded position"
+SIGNATURE_SITE_UNREADABLE = "arguments cannot be read positionally: `{text}`"
+SIGNATURE_SITE_UNKNOWN_KEYWORD = "passes keyword `{name}`, which is not declared"
+SIGNATURE_SITE_DUPLICATE = "passes `{name}` more than once"
+SIGNATURE_SITE_TOO_MANY = (
+    "passes {given} positional arguments but {declared} are declared"
+)
+SIGNATURE_SITE_NO_VALUE = "no value for `{name}`; map it or give it a default"
+SIGNATURE_SITE_OVERLAPS = "its arguments overlap another edit of this change"
+SIGNATURE_PLANNED = "{count} site(s) would be rewritten, {skipped} 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_ROLLBACK_REFUSED = (
+    "Signature change kept: its postcondition failed ({reasons}) but a later "
+    "edit was recorded on top of it, so it was not rolled back; undo the later "
+    "edit and rerun"
+)
+SIGNATURE_ROLLBACK_UNMEASURED = (
+    "Signature change rolled back after its postcondition failed ({reasons}), "
+    "but the graph could not be re-ingested afterwards ({error}); rebuild the "
+    "graph before the next graph-backed operation"
+)
+SIGNATURE_CONTRACT_UNMEASURED = (
+    "Signature change applied, but its postcondition could not be measured: {error}"
+)
 # Postcondition contract (issue #1531).
 CONTRACT_OP_RENAME = "rename"
 CONTRACT_OP_CHANGE_SIGNATURE = "change_signature"

codebase_rag/constants/core.py

@@ -106,6 +106,7 @@ class EventType(StrEnum):
 
 CHAR_SPACE = " "
 SEPARATOR_COMMA_SPACE = ", "
+SEPARATOR_SEMICOLON_SPACE = "; "
 PUNCTUATION_TYPES = (CHAR_PAREN_OPEN, CHAR_PAREN_CLOSE, CHAR_COMMA)
 
 REGEX_METHOD_CHAIN_SUFFIX = r"\)\.[^)]*$"

codebase_rag/constants/graph.py

@@ -60,6 +60,10 @@
 # Rename report fields (issue #1532).
 KEY_SITES = "sites"
 KEY_AMBIGUOUS = "ambiguous"
+# Change-signature report fields (issue #1533); `verdict` is shared
+# with rename.
+KEY_UNMAPPED = "unmapped"
+KEY_VERDICT = "verdict"
 KEY_STRUCTURAL_DELTA = "structural_delta"
 KEY_DISPATCH_LITERAL = "dispatch_literal"
 

codebase_rag/editing/__init__.py

@@ -1,5 +1,7 @@
 """Graph-aware editing primitives: span-preserving patchers (issue #1529),
-transactions (issue #1528) and import rewriting (issue #1530)."""
+transactions (issue #1528), import rewriting (issue #1530) and the graph
+operations built on them: rename (issue #1532) and change_signature (issue
+#1533)."""
 
 from .contract import (
     Expectation,

codebase_rag/editing/contract.py

@@ -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,
     )
 
 
@@ -111,6 +114,15 @@ def _site_key(site: Mapping[str, object]) -> str:
     return f"{site.get(cs.KEY_PATH)}:{site.get(cs.KEY_LINE)}"
 
 
+def _call_key(site: Mapping[str, object]) -> str:
+    """A call's identity for the signature checks.
+
+    Two calls can share a line (`helper(1) + helper(2)`), and rewriting or
+    listing one must not cover the other, so the column is part of the key.
+    """
+    return f"{_site_key(site)}:{site.get(cs.KEY_COL)}"
+
+
 def _carried_by_ancestor(
     pair: tuple[str, str], expected: Iterable[tuple[str, str]]
 ) -> bool:
@@ -200,7 +212,7 @@ def _changed_site_fault(
     loops plus these three verdicts put that function one point over the
     threshold. The decision is per site and reads better named anyway.
     """
-    key = _site_key(site)
+    key = _call_key(site)
     verdict = site["verdict"]
     if verdict == cs.DELTA_ARITY_OK or key in unmapped:
         return None
@@ -232,7 +244,7 @@ def _check_sites_mapped(
             if fault := _changed_site_fault(site, unmapped, rewritten):
                 bad.append(fault)
     for site in delta["arity_findings"]:
-        key = _site_key(site)
+        key = _call_key(site)
         if key not in unmapped:
             bad.append(f"{key} ({site['verdict']})")
     if bad:
@@ -303,7 +315,9 @@ def verify(
     """Pass or fail the delta against the expectation, with reasons.
 
     `rewritten` lists the sites the operation rewrote as `(path:line,
-    resolution)`; `parse_failures` the files the transaction found not to
+    resolution)`, or `(path:line:col, resolution)` for a signature change,
+    whose per-site checks key on the column; `parse_failures` the files the
+    transaction found not to
     parse (the transaction refuses those itself, so a caller normally
     passes none, but the contract states the rule in one place).
     """

codebase_rag/editing/rename.py

@@ -44,6 +44,7 @@
 from .contract import Reingest, Verdict, measure, rename_expectation, verify
 from .imports import ANY_MODULE, ImportRewriter, ImportSite, SymbolMove, _imported
 from .patcher import Patcher, PatcherError, line_col_to_byte
+from .sites import AMBIGUOUS, call_node_at, hierarchy
 from .transaction import (
     EditTransaction,
     StagedTree,
@@ -55,13 +56,6 @@
 
 QueryFn = Callable[[str, PropertyDict | None], list[ResultRow]]
 
-_AMBIGUOUS = frozenset(
-    {
-        cs.EdgeResolution.HEURISTIC.value,
-        cs.EdgeResolution.OVERLOAD.value,
-        cs.EdgeResolution.DYNAMIC.value,
-    }
-)
 _IDENTIFIER_RE = r"(?<![\w])%s(?![\w])"
 
 
@@ -138,20 +132,6 @@ class RenameReport(NamedTuple):
 # --- site collection -----------------------------------------------------------
 
 
-def _hierarchy(fetch_all: QueryFn, project: str, qn: str) -> list[str]:
-    """`qn` plus every method it overrides or is overridden by, transitively."""
-    seen: list[str] = [qn]
-    frontier = [qn]
-    while frontier:
-        current = frontier.pop()
-        for row in graph_query.overrides(fetch_all, project, current):
-            other = row["qualified_name"]
-            if other not in seen:
-                seen.append(other)
-                frontier.append(other)
-    return seen
-
-
 def _name_token(
     source: bytes,
     language: cs.SupportedLanguage | None,
@@ -193,39 +173,6 @@ def _name_token(
     return None
 
 
-def _best_call_at(
-    root: Node, line: int, col: int, recorded_end: tuple[int, int] | None
-) -> Node | None:
-    """The call node at (line, col) that the graph site refers to.
-
-    Several calls can share a start point -- `helper(helper(1))`,
-    `helper(2).upper()`, and both links of `obj.helper(1).helper(2)` -- so the
-    right one is the call ending where the site recorded its end, or the
-    outermost when no end was recorded.
-
-    Extracted from `_callee_span` to keep it under the cognitive complexity
-    limit (S3776). This walk is where that complexity lives: a loop with
-    three levels of nested branching, and nesting multiplies the cost.
-    Extracting the straight-line setup around it would not have helped.
-    """
-    best: Node | None = None
-    stack: list[Node] = [root]
-    while stack:
-        node = stack.pop()
-        if node.start_point == (line - 1, col):
-            func = node.child_by_field_name(cs.FIELD_FUNCTION)
-            if func is not None:
-                if node.end_point == recorded_end:
-                    return node
-                if best is None or (
-                    best.end_point != recorded_end and node.end_byte > best.end_byte
-                ):
-                    best = node
-        if node.start_point[0] <= line - 1 <= node.end_point[0]:
-            stack.extend(node.children)
-    return best
-
-
 def _callee_span(
     source: bytes,
     language: cs.SupportedLanguage | None,
@@ -253,7 +200,7 @@ def _callee_span(
         if end_line is not None and end_col is not None
         else None
     )
-    best = _best_call_at(root, line, col, recorded_end)
+    best = call_node_at(root, line, col, recorded_end)
     if best is None:
         return None
     func = best.child_by_field_name(cs.FIELD_FUNCTION)
@@ -610,11 +557,11 @@ def plan(
         """Collect everything a rename touches; refuse on ambiguity."""
         if not re.fullmatch(r"[A-Za-z_]\w*", new_name):
             raise RenameRefused(cs.RENAME_BAD_NAME.format(name=new_name), [], [])
-        hierarchy = _hierarchy(self.fetch_all, self.project, qn)
+        members = hierarchy(self.fetch_all, self.project, qn)
         sites: list[RenameSite] = []
         unlocatable: list[str] = []
         old_name: str | None = None
-        for member in hierarchy:
+        for member in members:
             member_sites, member_unlocatable, member_name, _label = self._collect(
                 member
             )
@@ -646,15 +593,15 @@ def plan(
                 unlocatable,
             )
         ambiguous = [
-            s for s in sites if s.resolution in _AMBIGUOUS or s.resolution == _CHAIN
+            s for s in sites if s.resolution in AMBIGUOUS or s.resolution == _CHAIN
         ]
         if ambiguous and not allow_heuristic:
             raise RenameRefused(
                 cs.RENAME_AMBIGUOUS.format(qn=qn, count=len(ambiguous)),
                 ambiguous,
                 unlocatable,
             )
-        for member in hierarchy:
+        for member in members:
             for site, module in self._import_sites(member, old_name):
                 sites.append(
                     RenameSite(
@@ -681,7 +628,7 @@ def plan(
             ambiguous=tuple(ambiguous),
             unlocatable=tuple(unlocatable),
             doc_mentions=tuple(self._doc_mentions(old_name)),
-            hierarchy=tuple(hierarchy),
+            hierarchy=tuple(members),
             diff="",
             message=cs.RENAME_PLANNED.format(count=len(sites)),
         )