Skip to content

feat: analyze extract/inline scopes and transactions - #2057

Closed
vitali87 wants to merge 1 commit into
pr-split/split-pr1550-source/pr-1from
pr-split/split-pr1550-source/pr-2
Closed

vitali87 wants to merge 1 commit into
pr-split/split-pr1550-source/pr-1from
pr-split/split-pr1550-source/pr-2

Conversation

@vitali87

Copy link
Copy Markdown
Owner

Add shared span/scope analysis plus transaction and postcondition helpers used by extract and inline.

Files changed

  • codebase_rag/editing/extract_scope.py
  • codebase_rag/editing/extract_transaction.py

Diff stats

+307 additions, -0 deletions (307 LOC)

Dependencies

This PR depends on: pr-1

Dependency graph

Merge in this order:

pr-1: feat: add extract/inline public types and constants
└── pr-2: feat: analyze extract/inline scopes and transactions  <-- this PR
    └── pr-3: feat: implement extract refactoring operation
        ├── pr-4: feat: implement inline refactoring operation
        │   ├── pr-5: feat: expose extract/inline through CLI
        │   ├── pr-6: feat: expose extract/inline through MCP
        │   │   ├── pr-8: test: cover inline, MCP, and CRLF regression
        │   │   └── pr-10: docs: list extract/inline MCP tools
        │   └── pr-9: docs: document extract/inline architecture
        └── pr-7: test: cover extract refactoring
            └── pr-8: test: cover inline, MCP, and CRLF regression

@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 45 seconds.

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: 6cf927ce-82e9-457e-a6c0-ba3e142a8bba

📥 Commits

Reviewing files that changed from the base of the PR and between 613d406 and c6c2213.

📒 Files selected for processing (2)
  • codebase_rag/editing/extract_scope.py
  • codebase_rag/editing/extract_transaction.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: 2/5

Not safe to merge: postcondition rollback can remove an unrelated edit, and scope analysis can change extracted code behavior. The repository naming requirement must also be satisfied before merging.

Findings

  1. P1 Track escaping bindings ▶
  2. P1 Exclude assignment targets ▶
  3. P1 Rollback the committed edit ▶
  4. P2 Allow switch-local breaks ▶
  5. P2 Name scope analysis clearly ▶

Summary

This change adds shared scope analysis and edit postcondition helpers for extraction and inline operations.

The scope analyzer can pass assignment targets as helper inputs and omit bindings created by imports or destructuring. It also rejects otherwise valid JavaScript switch extraction when a case contains a local break. Failed postcondition handling can roll back a newer unrelated edit while leaving the failed extract applied.

T-Rex validation blocked

The parser-driven binding and assignment-target checks could not complete because the full project environment requires cmake or cmake3 to build pymgclient, and neither is available.

Reviews (1) · Last reviewed commit: "feat: analyze extract/inline scopes and ..."

Comment thread codebase_rag/editing/extract_scope.py Resolved
Comment thread codebase_rag/editing/extract_scope.py Resolved
Comment thread codebase_rag/editing/extract_scope.py Resolved
Comment thread codebase_rag/editing/extract_scope.py Resolved
@greptile-apps

greptile-apps Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

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.

  • P1 Failed extract postcondition can undo a newer unrelated edit ▶

    • Bug
      • _enforce invokes undo_last(repo_root) after measuring and verifying the postcondition. Because _commit has already released its transaction lock, another transaction can commit in that interval. undo_last reverses the newest history entry, which is the unrelated edit, leaving the failed extract in place while _enforce returns applied=False.
    • Cause
      • The rollback is selected by history position (undo_last) rather than by the transaction ID of the extract that just committed, and the postcondition path is outside the original commit lock.
    • Fix
      • Preserve the committed transaction ID and use undo_transaction(repo_root, transaction_id) (handling TransactionConflict explicitly), or retain an operation-wide lock spanning commit, reingestion, postcondition verification, and targeted rollback.

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

🟡 Changes recommended

Scope analysis misses supported syntax forms, and rollback can leave edits applied after an undo failure.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

Adds shared scope-analysis and transaction/postcondition helpers for upcoming extract/inline refactoring operations.

Changes:

  • Adds Tree-sitter span, binding, read, early-exit, and indentation helpers.
  • Adds patch commit and postcondition enforcement with rollback handling.
File Description
codebase_rag/​editing/​extract_scope.py Implements extract/inline scope analysis.
codebase_rag/​editing/​extract_transaction.py Provides shared transaction and contract helpers.

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

Comment thread codebase_rag/editing/extract_scope.py Resolved

vitali87 commented Oct 9, 2026

Copy link
Copy Markdown
Owner Author

Closing as superseded: this is part of the split of #1550, which was closed without merging, and it targets the stale feat/move-op branch rather than main. The branch is kept, so the extract/inline work can be rebased onto main if it's picked up again.


Generated by Claude Code

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