Skip to content

test(signature): acceptance, transaction and MCP tests for change_signature (#1533, 6/9) - #2169

Merged
vitali87 merged 1 commit into
mainfrom
pr-split/split-1912-change-signature/pr-6
Oct 30, 2026
Merged

vitali87 merged 1 commit into
mainfrom
pr-split/split-1912-change-signature/pr-6

Conversation

@vitali87

@vitali87 vitali87 commented Sep 22, 2026 •

Copy link
Copy Markdown
Owner

Summary

Demo

No user-visible change: adds the acceptance, transaction and MCP tests for change_signature, and their shared fixture.

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.py -p no:randomly -q at fa9969c18: 10 passed.

  • The full suite was not run locally. The CI Unit Tests run at fa9969c18 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 09c5ae4f4 --to-ref fa9969c18 (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.
  • Comments were not re-audited against the policy in this pass.

Files changed

  • codebase_rag/tests/change_signature_support.py
  • codebase_rag/tests/test_change_signature.py

Diff stats

+425 additions, -0 deletions (425 LOC)

Dependencies

This PR depends on: pr-5

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)  <-- this PR
                    └── pr-7: test(signature): mapping tests for change_signature (#1533, 7/9)
                        └── 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 changing function signatures, including adding and reordering parameters while accounting for positional and keyword callers.
    • Added checks for dry-run behavior, transaction history, rollback when a caller is invalid, and handling of heuristic-resolved call sites.
    • Added validation of the MCP tool’s schema, responses, and refusal behavior.

@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 fddc17c and eac8482.

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: b01adb2b-f95a-4d6c-a6fe-5070545ec37c

📥 Commits

Reviewing files that changed from the base of the PR and between 4ba9c3f and 712c700.

📒 Files selected for processing (2)
  • codebase_rag/tests/change_signature_support.py
  • codebase_rag/tests/test_change_signature.py

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

Walkthrough

This change adds shared indexed-project test fixtures and integration tests for signature changes. The tests cover parameter mapping and reordering, heuristic call-site handling, dry-run and transaction behavior, rollback after contract failure, and MCP tool responses.

Changes: Signature Change Test Coverage

Layer / File(s) Summary
Indexed project fixture
codebase_rag/tests/change_signature_support.py
Adds a Python fixture project, helpers to write and index it, a subprocess smoke test, and a helper to set a call-edge resolution.
Signature mapping and call-site tests
codebase_rag/tests/test_change_signature.py
Tests mapped parameter insertion, parameter reordering, and how heuristic-resolved call sites are handled by default and when allow_heuristic=True.
Transaction and MCP tests
codebase_rag/tests/test_change_signature.py
Tests dry-run and transaction history behavior, rollback after re-ingestion finds an invalid caller, and MCP schema, response, and refusal behavior.

Priority: Low

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

Change: Other

Merge Risk: Minimal · previous review 712c7

This change adds test coverage for signature changes, dry runs, rollback, and MCP responses. It does not alter production behavior, and no actionable merge risk was found.

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 the acceptance, transaction, and MCP tests for change_signature and follows the repository's Conventional Commits format.
Description check Passed The description follows the required template and documents the scope, lack of user-visible changes, related issue, test coverage, executed commands, and known test limitations.
✨ 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.

@github-actions

Copy link
Copy Markdown
Contributor

pr-split analysis

Metric Value
Total LOC 425
Files changed 2
Suggested groups 2
Largest group 331 LOC
LOC overflow 0
File scatter 0

This PR has 425 LOC and could be split into 2 smaller PRs:

Group Title Diff Depends On Files
pr-1 chore(split): review test-change-signature-1 +331/-0 — codebase_rag/tests/test_change_signature.py
pr-2 chore(split): review change-signature-support-2 +94/-0 — codebase_rag/tests/change_signature_support.py

Run pr-split split locally to create these sub-PRs, or adjust --max-loc to change the target size.

Copy link
Copy Markdown
Owner Author

Unit Tests (macos-latest, py3.13, shard 1/2) failed on 7b24eb6 with one setup error, in test_language_source_checkout.py::TestInstalledCgrRefuses::test_refusal_is_printed_once. The fixture's git commit -q -m init died with SIGTRAP; the other 11,569 tests in the shard passed.

This isn't caused by this PR. Its diff only adds change_signature_support.py and test_change_signature.py. The failing test and conftest.py are identical to main's, and the error is the runner's own git process crashing, not an assertion. No fix exists, since nothing in the code fails. I'll re-run the failed job once when the run finishes.


Generated by Claude Code

@vitali87
vitali87 force-pushed the pr-split/split-1912-change-signature/pr-6 branch from 7b24eb6 to f82b930 Compare October 18, 2026 18:36
@vitali87
vitali87 force-pushed the pr-split/split-1912-change-signature/pr-6 branch from f82b930 to 15ddaee 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-6 branch from 15ddaee to 24f59aa Compare October 19, 2026 11:00
@vitali87
vitali87 force-pushed the pr-split/split-1912-change-signature/pr-6 branch from 24f59aa to da7bf2b Compare October 20, 2026 19:25
@vitali87
vitali87 force-pushed the pr-split/split-1912-change-signature/pr-6 branch from da7bf2b to 2b28262 Compare October 21, 2026 04:02
@vitali87
vitali87 force-pushed the pr-split/split-1912-change-signature/pr-6 branch from d504534 to fddc17c Compare October 28, 2026 05:31
Base automatically changed from pr-split/split-1912-change-signature/pr-5 to main October 28, 2026 21:16
@vitali87
vitali87 force-pushed the pr-split/split-1912-change-signature/pr-6 branch from fddc17c to eac8482 Compare October 28, 2026 21:16
@sonarqubecloud

Copy link
Copy Markdown

@vitali87

Copy link
Copy Markdown
Owner Author

@greptile review

@greptile-apps

This comment has been minimized.

@greptile-apps

greptile-apps Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] This tests-only PR appears safe to merge.

Summary

Adds shared fixtures and 10 tests for change_signature.

  • Signature edit tests check added and reordered parameters against callers.
  • Signature edit tests make guessed call sites visible unless callers opt in.
  • Signature edit tests check preview, history, and rollback outcomes.
  • MCP signature tests check previews and refusal payloads.

Reviews (1) · Last reviewed commit: "test(signature): acceptance, transaction..." · Reviewed by Greptile

@vitali87
vitali87 merged commit 30f7d16 into main Oct 30, 2026
42 checks passed
@vitali87
vitali87 deleted the pr-split/split-1912-change-signature/pr-6 branch October 30, 2026 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant