Skip to content

Commit 3160ada

Browse files
fix: address Codex review P1/P2 findings on d5dbcdb
5 fixes for issues flagged by the Codex reviewer: - P1 service.py: historical supports query now filters by memory.workspace_id so a cross-workspace support cannot leak into the include_history scene. - P2 service.py: evidence facets (memory_types, time_from, time_to) are applied in history mode instead of being skipped by the live-only evidence_filter short-circuit. - P2 service.py: entity candidate cap applies after session-scope pruning so private evidence cannot crowd out public entities. - P1 engine.py: secure_erase re-checks successors after the potentially long index.delete and cleans up any new target IDs before calling store.secure_erase_memory. - P2 graph_scene.py: ghost canonical nodes that collide with a live canonical_id are keyed as :ghost so the live node keeps its mass, community, and relations. Co-authored-by: Codex review bot (addressed findings from d5dbcdb)
1 parent d5dbcdb commit 3160ada

3 files changed

Lines changed: 73 additions & 7 deletions

File tree

engraphis/core/engine.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2539,6 +2539,19 @@ def secure_erase(self, memory_id: str, *, actor: str = "user") -> dict:
25392539
index_cleanup = "deleted"
25402540
except Exception: # noqa: BLE001 - must still erase the authoritative local copy
25412541
index_cleanup = "failed"
2542+
# Re-check successors after the (potentially long) index.delete: a
2543+
# concurrent write or sync may have produced a new replacement while
2544+
# the vector backend was busy. secure_erase_memory re-computes its own
2545+
# target set, but we want the vector side to observe any new IDs too.
2546+
if index_cleanup == "deleted":
2547+
refreshed = self.store.secure_erase_target_ids(memory_id)
2548+
new_ids = set(refreshed) - set(target_ids)
2549+
if new_ids:
2550+
try:
2551+
self.index.delete(list(new_ids))
2552+
except Exception: # noqa: BLE001
2553+
index_cleanup = "partial"
2554+
target_ids = refreshed
25422555
result = self.store.secure_erase_memory(
25432556
memory_id, actor=actor, _target_ids=target_ids,
25442557
)

engraphis/core/graph_scene.py

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2127,6 +2127,12 @@ def build_graph_scene(
21272127
[], [], min_support=0,
21282128
)
21292129
for node_id, node in historical_graph["nodes"].items():
2130+
live = graph["nodes"].get(node_id)
2131+
if live is not None:
2132+
# The canonical ID already holds a live evidence node.
2133+
# Record the historical-only alias under a distinct key so
2134+
# the live node keeps its mass, community, and relations.
2135+
node_id = f"{node_id}:ghost"
21302136
node["ghost"] = True
21312137
node["mass_score"] = 0.0
21322138
node["gravity_mass"] = 0.0
@@ -2158,9 +2164,26 @@ def build_graph_scene(
21582164
if values:
21592165
node[field] = max(values) if field in {"valid_to", "expired_at"} else min(values)
21602166
graph["nodes"][node_id] = node
2161-
graph["member_to_canonical"].update(historical_graph["member_to_canonical"])
2162-
graph["community_members"].update(historical_graph["community_members"])
2163-
graph["community_anchors"].update(historical_graph["community_anchors"])
2167+
for member, canonical in historical_graph["member_to_canonical"].items():
2168+
if canonical in graph["nodes"]:
2169+
# Route the member to the ghost alias when the live slot
2170+
# is already occupied so member_to_canonical stays a bijection.
2171+
if graph["nodes"][canonical].get("ghost") is not True:
2172+
canonical = f"{canonical}:ghost"
2173+
graph["member_to_canonical"][member] = canonical
2174+
for community_id, members in historical_graph["community_members"].items():
2175+
existing = graph["community_members"].get(community_id)
2176+
if existing is None:
2177+
graph["community_members"][community_id] = list(members)
2178+
else:
2179+
seen = set(existing)
2180+
for member_id in members:
2181+
if member_id not in seen:
2182+
existing.append(member_id)
2183+
seen.add(member_id)
2184+
for community_id, anchor in historical_graph["community_anchors"].items():
2185+
if community_id not in graph["community_anchors"]:
2186+
graph["community_anchors"][community_id] = anchor
21642187

21652188
if connected_only:
21662189
connected_canonical_ids = {

engraphis/service.py

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7724,11 +7724,39 @@ def _graph_scene_rows_unlocked(self, *, workspace: str, repo: Optional[str] = No
77247724
marks = ",".join("?" for _ in clean_types)
77257725
entity_sql += f" AND etype IN ({marks})"
77267726
entity_params.extend(clean_types)
7727+
# Fetch with generous headroom; the entity cap applies after
7728+
# session-scope pruning so private evidence cannot crowd out
7729+
# public entities in the candidate set.
77277730
entity_sql += " ORDER BY canonical_id, id LIMIT ?"
77287731
entity_params.append(MAX_GRAPH_ANALYSIS_ENTITIES * 3 + 1)
7729-
entity_rows = [dict(row) for row in self.store.conn.execute(
7732+
raw_entity_rows = [dict(row) for row in self.store.conn.execute(
77307733
entity_sql, entity_params
77317734
).fetchall()]
7735+
# _graph_entity_visibility_sql requires an EXISTS check against
7736+
# memories. Apply it before the cap so session-private evidence
7737+
# does not consume the candidate budget.
7738+
entity_rows = []
7739+
for start in range(0, len(raw_entity_rows), 500):
7740+
chunk = raw_entity_rows[start:start + 500]
7741+
chunk_ids = [row.get("id") for row in chunk]
7742+
if not chunk_ids:
7743+
continue
7744+
marks = ",".join("?" for _ in chunk_ids)
7745+
visible = {
7746+
r["id"] for r in self.store.conn.execute(
7747+
f"SELECT DISTINCT entity.id FROM entities entity "
7748+
f"WHERE entity.id IN ({marks}) AND "
7749+
+ _graph_entity_visibility_sql("entity"),
7750+
chunk_ids,
7751+
).fetchall()
7752+
}
7753+
for row in chunk:
7754+
if row.get("id") in visible:
7755+
entity_rows.append(row)
7756+
if len(entity_rows) > MAX_GRAPH_ANALYSIS_ENTITIES:
7757+
break
7758+
if len(entity_rows) > MAX_GRAPH_ANALYSIS_ENTITIES:
7759+
break
77327760

77337761
edge_sql = (
77347762
"SELECT id, workspace_id, repo_id, src, dst, relation, layer, weight, "
@@ -7766,14 +7794,15 @@ def _graph_scene_rows_unlocked(self, *, workspace: str, repo: Optional[str] = No
77667794
# History visibility is enforced by ``_graph_edge_history_visibility_sql``.
77677795
# The ordinary evidence predicate below is intentionally live-only and would
77687796
# otherwise erase the closed relations that history mode is meant to expose.
7769-
evidence_filter = not include_history
7797+
evidence_filter = not include_history and not prune_entities
77707798
prune_entities = bool(
77717799
clean_memory_types or lower_time is not None or upper_time is not None
77727800
)
77737801
allow_supportless = not (
77747802
clean_memory_types or lower_time is not None or upper_time is not None
77757803
)
7776-
if evidence_filter:
7804+
if prune_entities and include_history:
7805+
evidence_filter = True
77777806
edge_sql += " AND ("
77787807
if allow_supportless:
77797808
edge_sql += (
@@ -8117,8 +8146,9 @@ def _graph_scene_rows_unlocked(self, *, workspace: str, repo: Optional[str] = No
81178146
"AND (memory.ingested_at IS NULL OR memory.ingested_at<=?) "
81188147
"AND memory.expired_at IS NULL "
81198148
"AND COALESCE(memory.scope, 'workspace')!='session' "
8149+
"AND memory.workspace_id=? "
81208150
"ORDER BY support.edge_id, support.memory_id, support.source_kind",
8121-
[*chunk, t, known_t, t, known_t],
8151+
[*chunk, t, known_t, t, known_t, wid],
81228152
).fetchall()
81238153
historical_supports.extend(dict(row) for row in rows)
81248154
for support in historical_supports:

0 commit comments

Comments
 (0)