feat: map change-signature call arguments by vitali87 · Pull Request #2041 · vitali87/code-graph-rag

vitali87 · 2026-09-20T21:57:01Z

Call-site lookup and argument mapping helpers for positional, keyword, splat, and surplus cases.

Files changed

codebase_rag/editing/signature_calls.py

Diff stats

+136 additions, -0 deletions (136 LOC)

Dependencies

This PR depends on: pr-1

Dependency graph

Merge in this order:

pr-1: feat: add change-signature types and constants
├── pr-2: feat: parse and render signature parameters
│   ├── pr-4: feat: rewrite signature definitions and call sites
│   │   └── pr-5: feat: plan change-signature graph edits
│   │       └── pr-6: feat: expose change-signature via API, CLI, and MCP
│   │           ├── pr-7: test: cover change-signature units
│   │           │   ├── pr-8: test: cover change-signature acceptance flows
│   │           │   └── pr-9: test: cover change-signature regressions
│   │           └── pr-10: docs: document change-signature architecture
│   └── pr-5: feat: plan change-signature graph edits
│       └── pr-6: feat: expose change-signature via API, CLI, and MCP
│           ├── pr-7: test: cover change-signature units
│           │   ├── pr-8: test: cover change-signature acceptance flows
│           │   └── pr-9: test: cover change-signature regressions
│           └── pr-10: docs: document change-signature architecture
├── pr-3: feat: map change-signature call arguments  <-- this PR
│   ├── pr-4: feat: rewrite signature definitions and call sites
│   │   └── pr-5: feat: plan change-signature graph edits
│   │       └── pr-6: feat: expose change-signature via API, CLI, and MCP
│   │           ├── pr-7: test: cover change-signature units
│   │           │   ├── pr-8: test: cover change-signature acceptance flows
│   │           │   └── pr-9: test: cover change-signature regressions
│   │           └── pr-10: docs: document change-signature architecture
│   └── pr-5: feat: plan change-signature graph edits
│       └── pr-6: feat: expose change-signature via API, CLI, and MCP
│           ├── pr-7: test: cover change-signature units
│           │   ├── pr-8: test: cover change-signature acceptance flows
│           │   └── pr-9: test: cover change-signature regressions
│           └── pr-10: docs: document change-signature architecture
├── pr-4: feat: rewrite signature definitions and call sites
│   └── pr-5: feat: plan change-signature graph edits
│       └── pr-6: feat: expose change-signature via API, CLI, and MCP
│           ├── pr-7: test: cover change-signature units
│           │   ├── pr-8: test: cover change-signature acceptance flows
│           │   └── pr-9: test: cover change-signature regressions
│           └── pr-10: docs: document change-signature architecture
└── pr-5: feat: plan change-signature graph edits
    └── pr-6: feat: expose change-signature via API, CLI, and MCP
        ├── pr-7: test: cover change-signature units
        │   ├── pr-8: test: cover change-signature acceptance flows
        │   └── pr-9: test: cover change-signature regressions
        └── pr-10: docs: document change-signature architecture

coderabbitai · 2026-09-20T21:57:08Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)

^feat/.*
^fix/.*

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: vitali87/code-graph-rag/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 6e0f9909-6523-4fc3-9318-aff8bf0a7987

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

greptile-apps · 2026-09-20T22:29:23Z

Confidence Score: 4/5

Not safe to merge until C# named arguments are preserved or rewrites that could change their binding are refused.

Findings

Preserve C# argument names ▶

Summary

This change adds call-site argument mapping for signature edits. C# named argument labels are discarded before the new mapper assigns argument positions, so a reordered named call can have its parameter bindings changed during rewriting.

T-Rex validation blocked

The C# parser and argument splitter were exercised successfully, but the final mapper rerun could not complete after correcting an isolated harness dependency because the validation session reached its execution limit.

_{Reviews (1) · Last reviewed commit: "feat: map change-signature call argument..."}

greptile-apps · 2026-09-20T22:29:26Z

+    values: dict[int, tuple[str, bool]] = {}
+    for index, node in enumerate(positional):


Preserve C# argument names

For a C# call such as F(b: 2, a: 1), the argument splitter discards b and a and this code enumerates the remaining values as positional arguments. A rewrite can therefore emit F(2, 1), changing the original bindings from a=1, b=2 to a=2, b=1. Preserve C# argument labels through mapping, or refuse rewrites that cannot retain their bindings.

_{Ran code and verified through T-Rex}

greptile-apps · 2026-09-20T22:29:31Z

Comments Outside Diff

These findings sit on lines the diff does not cover, so they could not be posted inline. Each one leaves this list once its file changes.

C named call argument labels are discarded before signature-call mapping ▶
- Bug
  - Confirmed at the executed parser/split boundary. For F(b: 2, a: 1), tree-sitter exposes two C# argument nodes with identifier labels b and a, but _split_call_arguments unwraps each to the last named child-the integer expression-and returns positional ['2', '1'] with no keyword entries. _map_arguments then enumerates those returned positional values at signature_calls.py:73-75; therefore a signature mapping that preserves old parameter slots can emit F(2, 1), changing bindings from a=1,b=2 to a=2,b=1.
- Cause
  - codebase_rag/parsers/call_processor.py:632-636 intentionally unwraps C# argument nodes to child.named_children[-1], discarding the preceding label; :637-642 only extracts named arguments for Dart and Python. codebase_rag/editing/signature_calls.py:70-75 treats all returned C# values as positional.
- Fix
  - Add C# named-argument extraction in _split_call_arguments (or preserve argument metadata) by reading the C# argument label and mapping it into keyword, analogous to Python/Dart. Ensure C# mappings either preserve name: value syntax where valid or refuse mappings requiring a rename/reorder that cannot be represented safely. Add an integration regression for reordered C# names F(b: 2, a: 1).

Copilot

Copilot review overview

🟡 Changes recommended

A critical missing-module import and multiple argument-mapping defects must be addressed.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 3

Open (4)

Import references missing signature_params module · New
C# named arguments lose labels during rewriting · New
Dart named arguments are incorrectly rejected · New
Name-based parameter sources are not resolved · New

What changed in this PR

Adds call-site lookup and argument-remapping helpers for change_signature, covering positional, keyword, splat, and surplus arguments.

Changes:

Locates calls using recorded source spans.
Maps arguments and reports unmappable cases.

File	Summary
`codebase_rag/editing/signature_calls.py`	Adds call lookup and argument-mapping logic; unresolved import and several mapping issues remain.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+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


+    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)


+        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 spec.from_index is not None:
+            found = values.pop(spec.from_index, None)


vitali87 · 2026-09-22T23:35:51Z

Superseded: change_signature lands through the split of #1912, which replaces #1548, as the stack #2163 → #2161 → #2162 → #2166 → #2164 → #2169 → #2168 → #2165 → #2167 on main. Closing this split-pr1548 stack at the owner's direction.

feat: map change-signature call arguments

fa426c1

github-project-automation Bot added this to @vitali87's graph code Sep 20, 2026

vitali87 mentioned this pull request Sep 20, 2026

feat: change_signature edit operation with per-site argument mapping (#1533) #1548

Closed

vitali87 marked this pull request as ready for review September 20, 2026 22:22

greptile-apps Bot reviewed Sep 20, 2026

View reviewed changes

vitali87 requested a lite review from Copilot September 21, 2026 13:52

Copilot started reviewing on behalf of vitali87 September 21, 2026 14:04

View session

Copilot AI reviewed Sep 21, 2026

View reviewed changes

vitali87 mentioned this pull request Sep 22, 2026

feat: rebuild the change_signature edit operation on main (#1533) #1912

Closed

vitali87 closed this Sep 22, 2026

github-project-automation Bot moved this to Done in @vitali87's graph code Sep 22, 2026

Navigation Menu

Repository navigation

feat: map change-signature call arguments - #2041

feat: map change-signature call arguments#2041
vitali87 wants to merge 1 commit into
pr-split/split-pr1548-rebased/pr-1from
pr-split/split-pr1548-rebased/pr-3

vitali87 commented Sep 20, 2026

Uh oh!

coderabbitai Bot commented Sep 20, 2026 •

edited

Loading

Review skipped

Uh oh!

greptile-apps Bot commented Sep 20, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Sep 20, 2026

Uh oh!

greptile-apps Bot commented Sep 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

vitali87 commented Sep 22, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		values: dict[int, tuple[str, bool]] = {}
		for index, node in enumerate(positional):

		if spec.from_index is not None:
		found = values.pop(spec.from_index, None)

Conversation

vitali87 commented Sep 20, 2026

Files changed

Diff stats

Dependencies

Dependency graph

Uh oh!

coderabbitai Bot commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

greptile-apps Bot commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 4/5

Findings

Summary

T-Rex validation blocked

Uh oh!

greptile-apps Bot Sep 20, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Sep 20, 2026

Comments Outside Diff

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

vitali87 commented Sep 22, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Sep 20, 2026 •

edited

Loading

greptile-apps Bot commented Sep 20, 2026 •

edited

Loading