Skip to content

test(signature): mapping tests for change_signature (#1533, 7/9) - #2168

Open
vitali87 wants to merge 1 commit into
pr-split/split-1912-change-signature/pr-6from
pr-split/split-1912-change-signature/pr-7
Open

vitali87 wants to merge 1 commit into
pr-split/split-1912-change-signature/pr-6from
pr-split/split-1912-change-signature/pr-7

Conversation

@vitali87

@vitali87 vitali87 commented Sep 22, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Adds codebase_rag/tests/test_change_signature_mapping.py: how the mapping sources each new parameter at a call site, and the sites it cannot complete, which must be listed rather than rewritten.
  • Tests only: no production code changes.

Demo

No user-visible change: adds the mapping tests for change_signature.

Type of Change

  • Bug fix
  • New feature
  • Performance improvement
  • Refactoring (no functional changes)
  • Documentation
  • CI/CD or tooling
  • Dependencies

None of these applies: the change adds tests only.

Related Issues

Part of #1533 (split from #1912; stack: #2163 → #2161 → #2162 → #2166 → #2164 → #2169 → #2168 → #2165 → #2167).

Test Plan

  • Unit tests pass (make test-parallel or uv run pytest -n auto -m "not integration")

  • New tests added

  • Integration tests pass (make test-integration, requires Docker)

  • Manual testing (describe below)

  • uv run python -m pytest codebase_rag/tests/test_change_signature_mapping.py -p no:randomly -q at 7adfaa747: 15 passed.

  • The full suite was not run locally. The CI Unit Tests run at 7adfaa747 was cancelled before it finished.

Checklist

  • PR title follows Conventional Commits format
  • All pre-commit checks pass (make pre-commit)
  • No hardcoded strings in non-config/non-constants files
  • No # type: ignore, cast(), Any, or object type hints
  • New comments and docstrings explain why, not what (see Comment Policy)

SKIP=no-commit-to-branch pre-commit run --from-ref fa9969c18 --to-ref 7adfaa747 (ruff, ruff-format, ty, bandit, generate-readme and the file hygiene hooks): passed. The unit-suite step of make pre-commit was not run locally.

Unchecked items:

  • The tests write their fixture source and expected text inline rather than through constants.
  • test_change_signature_mapping.py has one # type: ignore[no-untyped-def] and a dict[str, object] hint, both on the both_links test stub.
  • Comments were not re-audited against the policy in this pass.

Files changed

  • codebase_rag/tests/test_change_signature_mapping.py

Diff stats

+358 additions, -0 deletions (358 LOC)

Dependencies

This PR depends on: pr-6

Dependency graph

Merge in this order:

pr-1: feat(signature): contract expectation, shared call-site helpers and constants for change_signature (#1533, 1/9)
└── pr-2: feat(signature): parameter specs, mapping sources and the change_signature contract check (#1533, 2/9)
    └── pr-3: feat(signature): read the header, bind call arguments, render the rewrite (#1533, 3/9)
        └── pr-4: feat(signature): the change_signature operation (#1533, 4/9)
            └── pr-5: feat(signature): expose change_signature through the CLI and MCP, with docs (#1533, 5/9)
                └── pr-6: test(signature): acceptance, transaction and MCP tests for change_signature (#1533, 6/9)
                    └── pr-7: test(signature): mapping tests for change_signature (#1533, 7/9)  <-- this PR
                        └── pr-8: test(signature): definition and renamed-parameter tests for change_signature (#1533, 8/9)
                            └── pr-9: test(signature): hierarchy, receiver and default guards for change_signature (#1533, 9/9)

Summary by CodeRabbit

  • Tests
    • Expanded coverage for updating function-call arguments when signatures change, including positional and keyword arguments, inserted defaults, and calls that cannot be safely updated.
    • Added checks for missing or outdated source locations, complex call patterns, unsupported arguments, invalid mappings, and alternate mapping formats.
    • Verified that successful updates produce the expected results and rewritten code.

@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 63682d5 and 4d5f2a9.

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

No actionable comments were generated in the recent review.

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: QUIET

Plan: Advanced

Run ID: 7881ccec-20be-4082-bd2f-6705a351034a

📥 Commits

Reviewing files that changed from the base of the PR and between 712c700 and 2d92155.

📒 Files selected for processing (1)
  • codebase_rag/tests/test_change_signature_mapping.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 2 remain after this review.

Walkthrough

This change adds tests for change_signature parameter mappings. The tests cover rewritten definitions and call sites, unmapped or stale call locations, unsupported call arguments, mapping validation, and mapping-text parsing.

Changes: Signature mapping test coverage

Layer / File(s) Summary
Parameter mapping and rewritten calls
codebase_rag/tests/test_change_signature_mapping.py
Tests check parameter renaming, default insertion, and unmapped required parameters. They also check rewritten code and runtime values.
Call-site matching and unmapped reporting
codebase_rag/tests/test_change_signature_mapping.py
Tests cover stale or missing locations, chained calls, surplus arguments, and arguments that the mapping cannot read. They check which calls are rewritten or reported as unmapped.
Mapping validation and text parsing
codebase_rag/tests/test_change_signature_mapping.py
Tests cover invalid or incomplete mappings and parsing mapping text forms.

Priority: Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Other

Merge Risk: Minimal · previous review 2d921

The added tests cover mapping behavior without changing production code. No issue identified here needs to be resolved before merging.

Pre-merge checks: Passed 4
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly identifies a test-only change for change_signature mapping and follows Conventional Commits format.
Description check Passed The description covers the summary, demo status, change type, related work, test commands and results, checklist status, changed files, dependencies, and merge order. It also records skipped checks an…
Full details: Description check

Explanation

The description covers the summary, demo status, change type, related work, test commands and results, checklist status, changed files, dependencies, and merge order. It also records skipped checks and known deviations.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@vitali87
vitali87 added this pull request to stack #2170 September 22, 2026 23:23
@vitali87
vitali87 force-pushed the pr-split/split-1912-change-signature/pr-7 branch from 8e224f5 to bbe95e6 Compare October 4, 2026 15:32
@vitali87
vitali87 force-pushed the pr-split/split-1912-change-signature/pr-7 branch 2 times, most recently from 5d9fa52 to bfe4f9e Compare October 18, 2026 20:35
@vitali87 vitali87 removed the merge_conflict label Oct 18, 2026 — with Claude
@vitali87
vitali87 force-pushed the pr-split/split-1912-change-signature/pr-7 branch from bfe4f9e to 9b61f21 Compare October 19, 2026 11:00

Copy link
Copy Markdown
Owner Author

Unit Tests (macos-latest, py3.12, shard 1/2) failed on 9b61f21, but not because of this PR's code. One test out of 11,619 failed: test_indexing_bench.py::test_ensure_corpus_leaves_clean_checkout_untouched. In it, the git binary was killed by SIGTRAP during a plain git add a.py in a pytest temp dir (CalledProcessError ... died with <Signals.SIGTRAP: 5>). That is the runner's git crashing, not an assertion. This PR only adds test_change_signature_mapping.py, and nothing in the stack touches the bench test or the code it runs. There's no fix to port. I'll re-run the failed job once when this run finishes.


Generated by Claude Code

@vitali87
vitali87 force-pushed the pr-split/split-1912-change-signature/pr-7 branch from 9b61f21 to 360e58d Compare October 20, 2026 19:25
@vitali87
vitali87 force-pushed the pr-split/split-1912-change-signature/pr-7 branch from 360e58d to 8b22de0 Compare October 21, 2026 04:02
@vitali87
vitali87 force-pushed the pr-split/split-1912-change-signature/pr-7 branch from 63682d5 to 4d5f2a9 Compare October 28, 2026 05:31
@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant