Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
285 changes: 285 additions & 0 deletions app/modules/parsing/graph_construction/incremental_update_service.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,285 @@
"""Incremental knowledge-graph updates.

This module provides building blocks for updating a project's knowledge
graph in response to a partial change set (a subset of files added,
modified or deleted), rather than wiping the entire graph and
reconstructing it from scratch.

Components:

* :class:`FileSnapshot` — a content-hashed view of the files currently
represented in the graph. Used as the "before" side of differential
analysis.
* :func:`detect_changes` — computes the set of added / modified /
deleted files between two snapshots.
* :class:`IncrementalGraphUpdater` — applies a :class:`ChangeSet` to
the live neo4j-backed graph: removes nodes (and their relationships)
belonging to deleted or modified files, then writes new / modified
file subgraphs back in. Untouched files, their nodes, and their
relationships are preserved verbatim, as are their already-generated
inferences (which the global content-hash cache returns as cache
hits on the next inference run).
"""

from __future__ import annotations

import hashlib
from dataclasses import dataclass, field
from typing import Dict, Iterable, List, Optional, Set

from app.modules.parsing.graph_construction.code_graph_service import (
CodeGraphService,
)
from app.modules.utils.logger import setup_logger

logger = setup_logger(__name__)


@dataclass(frozen=True)
class FileSnapshot:
"""Content-hashed view of a set of files at a point in time.

``files`` maps repo-relative file path → SHA-256 hex digest of the
file contents. Two snapshots are comparable via :func:`detect_changes`.
"""

files: Dict[str, str] = field(default_factory=dict)

@classmethod
def from_files(cls, files: Dict[str, str]) -> "FileSnapshot":
return cls(files=dict(files))
Comment on lines +38 to +50

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

FileSnapshot is only shallowly immutable.

frozen=True prevents rebinding files, but callers can still mutate snapshot.files[...] in place. That can silently skew later detect_changes() results if a snapshot is reused. Wrap the mapping in a read-only container during construction.

Proposed fix
+from types import MappingProxyType
-from typing import Dict, Iterable, List, Optional, Set
+from typing import Dict, Iterable, List, Mapping, Optional, Set
@@
 `@dataclass`(frozen=True)
 class FileSnapshot:
@@
-    files: Dict[str, str] = field(default_factory=dict)
+    files: Mapping[str, str] = field(default_factory=dict)
+
+    def __post_init__(self) -> None:
+        object.__setattr__(self, "files", MappingProxyType(dict(self.files)))
@@
     `@classmethod`
     def from_files(cls, files: Dict[str, str]) -> "FileSnapshot":
-        return cls(files=dict(files))
+        return cls(files=files)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@dataclass(frozen=True)
class FileSnapshot:
"""Content-hashed view of a set of files at a point in time.
``files`` maps repo-relative file pathSHA-256 hex digest of the
file contents. Two snapshots are comparable via :func:`detect_changes`.
"""
files: Dict[str, str] = field(default_factory=dict)
@classmethod
def from_files(cls, files: Dict[str, str]) -> "FileSnapshot":
return cls(files=dict(files))
`@dataclass`(frozen=True)
class FileSnapshot:
"""Content-hashed view of a set of files at a point in time.
``files`` maps repo-relative file pathSHA-256 hex digest of the
file contents. Two snapshots are comparable via :func:`detect_changes`.
"""
files: Mapping[str, str] = field(default_factory=dict)
def __post_init__(self) -> None:
object.__setattr__(self, "files", MappingProxyType(dict(self.files)))
`@classmethod`
def from_files(cls, files: Dict[str, str]) -> "FileSnapshot":
return cls(files=files)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/modules/parsing/graph_construction/incremental_update_service.py` around
lines 38 - 50, FileSnapshot is only shallowly immutable because frozen=True
prevents rebinding but allows in-place mutation of the files dict; modify
FileSnapshot.from_files to wrap the copied dict in an immutable read-only
mapping (e.g., use types.MappingProxyType on dict(files)) and update the files
type annotation to a read-only mapping (e.g., Mapping[str, str]) so callers
cannot mutate snapshot.files and subsequent detect_changes() sees stable data.


@staticmethod
def hash_content(content: str | bytes) -> str:
if isinstance(content, str):
content = content.encode("utf-8")
return hashlib.sha256(content).hexdigest()


@dataclass(frozen=True)
class ChangeSet:
"""Differential between two :class:`FileSnapshot` instances.

* ``added`` — files present in the new snapshot but not the old one.
* ``modified`` — files present in both but with a different content hash.
* ``deleted`` — files present in the old snapshot but not the new one.

``affected_files`` returns the union of files whose graph
representation needs to be removed before the new subgraph is
inserted (i.e. modified + deleted). ``upserted_files`` returns the
files whose new subgraph should be inserted (added + modified).
"""

added: List[str] = field(default_factory=list)
modified: List[str] = field(default_factory=list)
deleted: List[str] = field(default_factory=list)

@property
def affected_files(self) -> List[str]:
return list(self.modified) + list(self.deleted)

@property
def upserted_files(self) -> List[str]:
return list(self.added) + list(self.modified)

def is_empty(self) -> bool:
return not (self.added or self.modified or self.deleted)


def detect_changes(old: FileSnapshot, new: FileSnapshot) -> ChangeSet:
"""Compute the :class:`ChangeSet` between two snapshots."""
old_files = old.files
new_files = new.files

old_keys: Set[str] = set(old_files)
new_keys: Set[str] = set(new_files)

added = sorted(new_keys - old_keys)
deleted = sorted(old_keys - new_keys)
modified = sorted(
path for path in (old_keys & new_keys) if old_files[path] != new_files[path]
)

return ChangeSet(added=added, modified=modified, deleted=deleted)


class IncrementalGraphUpdater:
"""Apply a :class:`ChangeSet` to an existing knowledge graph.

Wraps a :class:`CodeGraphService` so the same neo4j driver is
reused for the partial-cleanup and incremental-store operations.
"""

def __init__(self, code_graph_service: CodeGraphService):
self.code_graph_service = code_graph_service

# ------------------------------------------------------------------
# Snapshot capture
# ------------------------------------------------------------------
def get_existing_file_snapshot(self, project_id: str) -> FileSnapshot:
"""Reconstruct a :class:`FileSnapshot` from the live graph.

Each ``FILE`` node carries a ``file_path`` and ``text`` property
(the file's source). We hash the text to produce the snapshot
entry. Files without a ``text`` property fall back to an empty
hash sentinel so they still participate in deletion detection.
"""
snapshot: Dict[str, str] = {}
with self.code_graph_service.driver.session() as session:
result = session.run(
"""
MATCH (n:FILE {repoId: $project_id})
RETURN n.file_path AS file_path,
COALESCE(n.text, '') AS text
""",
project_id=project_id,
)
for record in result:
path = record["file_path"]
if not path:
continue
snapshot[path] = FileSnapshot.hash_content(record["text"] or "")
return FileSnapshot(files=snapshot)
Comment on lines +119 to +142

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Snapshot/delete label mismatch can keep deleted files in future diffs.

get_existing_file_snapshot() rebuilds state from :FILE nodes, but cleanup_files() only deletes :NODE. If FILE is not also labeled NODE, modified/deleted files survive in the next snapshot and future diffs will treat them as still present. Cleanup should target the same file-backed nodes that snapshot reconstruction reads from.

One possible fix
-                MATCH (n:NODE {repoId: $project_id})
+                MATCH (n {repoId: $project_id})
                 WHERE n.file_path IN $paths

Also applies to: 147-175

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/modules/parsing/graph_construction/incremental_update_service.py` around
lines 119 - 142, get_existing_file_snapshot builds the snapshot from nodes
labeled FILE, but cleanup_files only removes nodes labeled NODE, so FILE-labeled
nodes can survive and reappear in future diffs; update the cleanup logic
(cleanup_files and any related deletion queries) to target the same file-backed
nodes used by get_existing_file_snapshot—i.e., delete nodes with the FILE label
(or both FILE and NODE) instead of only :NODE—so that removed files are actually
removed from the live graph and won't be included in subsequent snapshots.


# ------------------------------------------------------------------
# Scoped cleanup
# ------------------------------------------------------------------
def cleanup_files(self, project_id: str, file_paths: Iterable[str]) -> int:
"""Remove nodes and relationships for the given file paths.

Matches every ``NODE`` belonging to ``project_id`` whose
``file_path`` is in ``file_paths`` and ``DETACH DELETE``s it.
``DETACH`` drops the node's incident relationships, so
relationship integrity for the remaining (untouched) part of
the graph is preserved automatically: only relationships with
an endpoint inside the affected file set are removed.

Returns the number of nodes deleted.
"""
paths = [p for p in file_paths if p]
if not paths:
return 0
with self.code_graph_service.driver.session() as session:
result = session.run(
"""
MATCH (n:NODE {repoId: $project_id})
WHERE n.file_path IN $paths
WITH n, count(*) AS _
DETACH DELETE n
RETURN count(n) AS deleted
""",
project_id=project_id,
paths=paths,
)
record = result.single()
deleted = int(record["deleted"]) if record else 0
Comment on lines +162 to +175

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Delete and reinsert are not atomic.

The cleanup commits before _store_subgraph() runs. If _store_graph() fails, the affected files stay deleted and the project graph is left partially updated. That breaks the rollback/partial-failure requirement called out in the PR objective. Run both steps in a single write transaction or add a compensating rollback path.

Also applies to: 213-223, 284-285

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/modules/parsing/graph_construction/incremental_update_service.py` around
lines 162 - 175, The delete-before-insert is not atomic: the session.run
deletion using self.code_graph_service.driver.session() must be combined with
the subsequent _store_subgraph()/_store_graph() call inside a single write
transaction (e.g., use a write_transaction or transaction context) so that both
deletion and reinsertion commit together, or implement a compensating rollback
path if the store fails; update the code paths around the MATCH...DETACH DELETE
query and the calls to _store_subgraph()/_store_graph() to run within the same
transactional unit provided by code_graph_service (or explicitly roll back and
restore deleted nodes on failure).

logger.info(
"[INCREMENTAL] Removed %d nodes across %d files for project %s",
deleted,
len(paths),
project_id,
)
return deleted

# ------------------------------------------------------------------
# Apply a change set
# ------------------------------------------------------------------
def apply_changes(
self,
project_id: str,
user_id: str,
changeset: ChangeSet,
new_artifacts: Optional[object] = None,
) -> Dict[str, int]:
"""Apply ``changeset`` to the live graph.

* Deletes nodes whose ``file_path`` is in
``changeset.affected_files`` (modified + deleted).
* If ``new_artifacts`` is supplied, reconstructs a subgraph
from it (using the same reconstruction path as the full
pipeline) and writes only the nodes/relationships belonging
to ``changeset.upserted_files`` into the graph, leaving every
unaffected node and relationship in place.

Returns a small stats dict for observability.
"""
if changeset.is_empty():
logger.info(
"[INCREMENTAL] No changes detected for project %s — skipping update",
project_id,
)
return {"deleted_nodes": 0, "inserted_nodes": 0, "inserted_edges": 0}

deleted_nodes = self.cleanup_files(project_id, changeset.affected_files)

inserted_nodes = 0
inserted_edges = 0
if new_artifacts is not None and changeset.upserted_files:
inserted_nodes, inserted_edges = self._store_subgraph(
project_id=project_id,
user_id=user_id,
new_artifacts=new_artifacts,
upserted_files=set(changeset.upserted_files),
)

logger.info(
"[INCREMENTAL] Applied changeset for project %s: "
"+%d nodes, +%d edges, -%d nodes (added=%d modified=%d deleted=%d)",
project_id,
inserted_nodes,
inserted_edges,
deleted_nodes,
len(changeset.added),
len(changeset.modified),
len(changeset.deleted),
)
return {
"deleted_nodes": deleted_nodes,
"inserted_nodes": inserted_nodes,
"inserted_edges": inserted_edges,
}

# ------------------------------------------------------------------
# Internal: scoped subgraph insertion
# ------------------------------------------------------------------
def _store_subgraph(
self,
*,
project_id: str,
user_id: str,
new_artifacts: object,
upserted_files: Set[str],
) -> tuple[int, int]:
"""Insert only the portion of ``new_artifacts`` whose nodes
live in ``upserted_files``.

Reconstructs an ``nx.MultiDiGraph`` from the parser artifacts
(same helper used by the full-graph path), filters it down to
the upserted file set, and feeds the filtered graph through
:meth:`CodeGraphService._store_graph`.
"""
# Lazy import — keeps RepoMap / tree_sitter off the import path
# for callers that only need change detection.
from app.modules.parsing.graph_construction.parsing_repomap import (
_reconstruct_graph_from_payload,
)

nx_graph = _reconstruct_graph_from_payload(new_artifacts)

# Drop nodes whose file_path is not in the upserted set. Edges
# touching dropped nodes are removed by networkx automatically
# when their endpoints disappear.
to_remove = [
node_id
for node_id, data in nx_graph.nodes(data=True)
if data.get("file") not in upserted_files
]
nx_graph.remove_nodes_from(to_remove)

node_count = nx_graph.number_of_nodes()
edge_count = nx_graph.number_of_edges()
if node_count == 0:
return 0, 0

self.code_graph_service._store_graph(nx_graph, project_id, user_id)
return node_count, edge_count
Loading