feat: expose import graph sites for move by vitali87 · Pull Request #2075 · vitali87/code-graph-rag

vitali87 · 2026-09-20T22:56:16Z

Query imports by module and make the stateful test ingestor preserve per-site edge properties.

Files changed

codebase_rag/cypher_queries.py
evals/cgr_graph.py
codebase_rag/tests/test_change_signature.py

Diff stats

+65 additions, -33 deletions (98 LOC)

Dependencies

This PR depends on: pr-1

Dependency graph

Merge in this order:

pr-1: feat: add move public API and constants
├── pr-2: feat: parse move spans and imports
│   └── pr-5: feat: retarget move imports safely
│       └── pr-6: feat: implement move refactoring operation
│           ├── pr-7: feat: expose move through CLI
│           │   └── pr-11: docs: document move operation
│           ├── pr-8: feat: expose move through MCP
│           │   ├── pr-10: test: cover move MCP wiring
│           │   └── pr-11: docs: document move operation
│           └── pr-9: test: cover move operation behavior
│               └── pr-10: test: cover move MCP wiring
├── pr-3: feat: expose import graph sites for move  <-- this PR
│   └── pr-5: feat: retarget move imports safely
│       └── pr-6: feat: implement move refactoring operation
│           ├── pr-7: feat: expose move through CLI
│           │   └── pr-11: docs: document move operation
│           ├── pr-8: feat: expose move through MCP
│           │   ├── pr-10: test: cover move MCP wiring
│           │   └── pr-11: docs: document move operation
│           └── pr-9: test: cover move operation behavior
│               └── pr-10: test: cover move MCP wiring
└── pr-4: feat: detect move contract deltas
    └── pr-5: feat: retarget move imports safely
        └── pr-6: feat: implement move refactoring operation
            ├── pr-7: feat: expose move through CLI
            │   └── pr-11: docs: document move operation
            ├── pr-8: feat: expose move through MCP
            │   ├── pr-10: test: cover move MCP wiring
            │   └── pr-11: docs: document move operation
            └── pr-9: test: cover move operation behavior
                └── pr-10: test: cover move MCP wiring

coderabbitai · 2026-09-20T22:56:21Z

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: 465731a0-d5f4-4e6f-92e0-8965a191717a

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-20T23:02:45Z

Confidence Score: 4/5

Safe to merge from a production-behavior perspective: the reproduced issue is limited to the in-memory test fixture, but it should be corrected to keep test results faithful to graph behavior.

Findings

Preserve site-less relationships ▶

Summary

This PR adds outgoing-import query support and expands the stateful graph fixture to retain relationship-site metadata. One non-blocking fixture-fidelity issue remains: a propertyless relationship is lost when the same edge already has a property-bearing site.

_{Reviews (1) · Last reviewed commit: "feat: expose import graph sites for move"}

greptile-apps · 2026-09-20T23:02:48Z

        if properties:
            self.edge_props[edge] = dict(properties)
+            sites = self.edge_sites.setdefault(edge, [])
+            if dict(properties) not in sites:
+                sites.append(dict(properties))
+
+    def sites_of(self, edge: _RelTuple) -> list[PropertyDict]:
+        """Every site of an edge (one empty record for a site-less edge)."""
+        return self.edge_sites.get(edge) or [self.edge_props.get(edge, {})]


Preserve site-less relationships

When the same source, type, and target are recorded first with site properties and then without them, this code retains only the property-bearing site. sites_of() consequently returns fewer relationship rows than the graph would expose, so tests using this fixture can miss behavior involving imports that have both site-bearing and site-less relationships. This is non-blocking for merge, but store an empty site for propertyless insertions to keep the fixture faithful.

Knowledge Base Used: Models and Schemas

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Artifacts

Evidence from the check

A narrow Python harness loads the exact checked-out stateful ingestor class and inserts matching property-bearing and propertyless CALLS relationships, ending with the reproducible assertion.

Command output from the check

The executed baseline adds only the property-bearing CALLS relationship and shows one stored site, establishing the expected starting state.

Command output from the check

The executed paired run adds a propertyless relationship after the property-bearing one and reports confirmed because sites_of omits the empty site.

_{Ran code and verified through T-Rex}

greptile-apps · 2026-09-20T23:02:52Z

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.

Stateful ingestor drops a propertyless relationship after a property-bearing duplicate ▶
- Bug
  - In evals/cgr_graph.py:237-245, ensure_relationship_batch() adds to edge_sites only for truthy properties. When an identical source/type/target relationship is first inserted with {"line": 10, "col": 2} and then inserted without properties, sites_of() returns only the first site instead of both relationship records.
- Cause
  - The edge_sites.setdefault(...).append(...) path is inside if properties:. sites_of() then prefers the nonempty stored list and has no representation from which to synthesize the later propertyless site.
- Fix
  - Record a site for every relationship insertion, including an empty property dictionary for None/empty properties; retain duplicate-site deduplication only if it matches the production graph's exact relationship semantics.

Copilot

Copilot review overview

🟡 Changes recommended

Five unresolved moderate findings affect module-root queries and per-site edge fidelity.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

Capture every inbound edge site during restoration · New
Deduplicate edge sites by relationship merge keys · New

What changed in this PR

Adds module-level import-site querying and per-site relationship handling for move refactoring.

Changes:

Adds CYPHER_GRAPH_IMPORTS_OF.
Preserves per-site edge properties in the stateful evaluator.
Updates signature-change tests for stored call sites.

File	Review summary
`evals/cgr_graph.py`	Adds per-site tracking and query support. Four moderate findings: missing `Module`/project filtering (1 vote), incorrect merge-key deduplication (3 votes), incomplete inbound-site capture (2 votes), and omitted site-less relationships (1 vote).
`codebase_rag/tests/test_change_signature.py`	Updates heuristic-site handling.
`codebase_rag/cypher_queries.py`	Adds the import-site query. One moderate finding (1 vote): root-module matching must include the exact project name.

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

+        # Every distinct site of an endpoint pair (issue #1522): the real
+        # store keeps one edge per site; `edge_props` keeps the last one
+        # for the callers that only need the pair.
+        self.edge_sites: dict[_RelTuple, list[PropertyDict]] = {}


+            sites = self.edge_sites.setdefault(edge, [])
+            if dict(properties) not in sites:
+                sites.append(dict(properties))


vitali87 · 2026-10-09T11:13:13Z

Closing as superseded: this is part of the split of #1549, which was closed without merging, and it targets the stale feat/change-signature branch rather than main. The change_signature base was rebuilt on main as #1912 (now stacked as #2161–#2169). The branch is kept, so the move-op work can be rebased onto main if it's picked up again.

Generated by Claude Code

feat: expose import graph sites for move

7a9740a

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

greptile-apps Bot reviewed Sep 20, 2026

View reviewed changes

vitali87 mentioned this pull request Sep 20, 2026

feat: move edit operation with importer rewriting and cycle refusal (#1534) #1549

Closed

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

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

View session

Copilot AI reviewed Sep 21, 2026

View reviewed changes

vitali87 closed this Oct 9, 2026

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

Navigation Menu

Repository navigation

feat: expose import graph sites for move - #2075

feat: expose import graph sites for move#2075
vitali87 wants to merge 1 commit into
pr-split/split-pr1549-source/pr-1from
pr-split/split-pr1549-source/pr-3

vitali87 commented Sep 20, 2026

Uh oh!

coderabbitai Bot commented Sep 20, 2026

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 Oct 9, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

vitali87 commented Sep 20, 2026

Files changed

Diff stats

Dependencies

Dependency graph

Uh oh!

coderabbitai Bot commented Sep 20, 2026

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

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 Oct 9, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented Sep 20, 2026 •

edited

Loading