Skip to content

test: cover change-signature acceptance flows - #2046

Closed
vitali87 wants to merge 1 commit into
pr-split/split-pr1548-rebased/pr-7from
pr-split/split-pr1548-rebased/pr-8
Closed

vitali87 wants to merge 1 commit into
pr-split/split-pr1548-rebased/pr-7from
pr-split/split-pr1548-rebased/pr-8

Conversation

@vitali87

Copy link
Copy Markdown
Owner

End-to-end acceptance coverage for mapped, unmapped, hierarchy, TypeScript, and rollback flows.

Files changed

  • codebase_rag/tests/test_change_signature_acceptance.py

Diff stats

+316 additions, -0 deletions (316 LOC)

Dependencies

This PR depends on: pr-7

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  <-- this PR
│   │           │   └── 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  <-- this PR
│           │   └── pr-9: test: cover change-signature regressions
│           └── pr-10: docs: document change-signature architecture
├── pr-3: feat: map change-signature call arguments
│   ├── 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  <-- this PR
│   │           │   └── 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  <-- this PR
│           │   └── 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  <-- this PR
│           │   └── 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  <-- this PR
        │   └── pr-9: test: cover change-signature regressions
        └── pr-10: docs: document change-signature architecture

@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 8 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 8 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

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

Review profile: CHILL

Plan: Advanced

Run ID: 89cc4af5-8b52-401b-a59e-4fdb3510e84d

📥 Commits

Reviewing files that changed from the base of the PR and between 8a3e861 and 9766c86.

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

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

greptile-apps Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

Safe to merge; no actionable issues were found.

What we checked:

  • Exercised the contract-triggered rollback flow with an unindexed caller and observed the signature contract fail. T-Rex
  • Compared the helper signature and existing call-site graph edges before the change and after rollback to verify the graph's consistency. T-Rex
  • Confirmed the original graph state was restored, the change was not applied, and the edit history remained empty. T-Rex
  • Authored and ran the rollback-graph-state probe and produced the after-state log; decisive output shows preexisting_graph_restored: true and identical baseline/post-rollback state. T-Rex
  • Referenced the implementation of undo_last and re-ingest and noted the related acceptance-test references to corroborate rollback validation. T-Rex

Summary

  • Adds end-to-end acceptance coverage for Python and TypeScript signature changes, positional reordering, unmapped and heuristic callers, method hierarchies, invalid mappings, and contract-triggered rollback.
  • The rollback scenario preserves source files and transaction history after a rejected edit.
  • The in-memory graph returns to its pre-edit signature and existing call-site state after the rollback.

Reviews (1) · Last reviewed commit: "test: cover change-signature acceptance ..."

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The change is test-only and the added cases are coherent and consistent with existing unit-test patterns, with only a minor test naming mismatch noted.

Review effort: Lite
Findings: 1 Low severity

Open (1)
What changed in this PR

Adds end-to-end acceptance tests for the change_signature editing operation, exercising parameter insertion/reordering, unmapped/heuristic-site handling, hierarchy rewrites, TypeScript rewrites, and transactional rollback on contract failure.

Changes:

  • Introduces a new acceptance test suite covering successful rewrites across Python and TypeScript.
  • Adds negative/guard-rail coverage for refused mappings (type mismatch, invalid ordering, surplus-arg callers, heuristic edges).
  • Verifies contract enforcement behavior, including rollback and history cleanup on post-reingest failures.
File Description
codebase_rag/​tests/​test_change_signature_acceptance.py New acceptance coverage for mapped/unmapped/heuristic/hierarchy/TypeScript/rollback change-signature flows

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

Comment on lines +31 to +33
def test_add_required_parameter_with_a_default_mapping(
repo: tuple[Path, _StatefulIngestor, GraphUpdater],
) -> None:
@vitali87
vitali87 force-pushed the pr-split/split-pr1548-rebased/pr-8 branch from 5a9aaa2 to 9766c86 Compare September 22, 2026 22:00
@vitali87
vitali87 removed this pull request from stack #2055 September 22, 2026 22:08
@vitali87
vitali87 added this pull request to stack #2133 September 22, 2026 22:09
@vitali87

Copy link
Copy Markdown
Owner Author

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.

@sonarqubecloud

Copy link
Copy Markdown

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.

2 participants