Skip to content

Commit e4318d6

Browse files
authored
Merge pull request #2288 from MichaelWave369/fix/graph-stats-completeness
fix(graph): report complete palace graph stats
2 parents ff7abc2 + c14d876 commit e4318d6

4 files changed

Lines changed: 142 additions & 29 deletions

File tree

mempalace/mcp_server.py

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
list_tunnels,
9595
delete_tunnel,
9696
follow_tunnels,
97+
_load_tunnels as _load_graph_tunnels,
9798
)
9899
from .hallways import ( # noqa: E402
99100
list_hallways,
@@ -2048,7 +2049,7 @@ def _sqlite_graph_stats():
20482049
(non-chroma backend, missing/unbootstrapped palace, sqlite error). The
20492050
reconstruction mirrors ``palace_graph.build_graph`` /
20502051
``palace_graph.graph_stats`` exactly: a node is a room with a non-empty
2051-
wing and a usable room name (the catch-all ``"general"`` is excluded), and
2052+
wing and a usable room name (including the catch-all ``"general"``), and
20522053
edges are the per-hall cross-wing crossings of multi-wing rooms.
20532054
"""
20542055
rows = None
@@ -2074,46 +2075,56 @@ def _sqlite_graph_stats():
20742075

20752076

20762077
def _graph_stats_from_grouped_rows(rows):
2077-
"""Rebuild ``graph_stats`` from ``(room, wing, hall, n)`` grouped rows.
2078+
"""Rebuild ``graph_stats`` from grouped sqlite metadata rows.
20782079
2079-
Backends may append a fifth ``last_date`` column for ``find_tunnels``;
2080-
stats do not use it, so extra columns are ignored rather than unpacked.
2080+
Rows are ``(room, wing, hall, n)`` with an optional fifth ``last_date``
2081+
column. Because grouping includes ``hall``, one room placement can occupy
2082+
multiple SQL rows; room instances therefore use a distinct ``(wing, room)`` set.
20812083
"""
20822084
from collections import Counter, defaultdict
20832085

20842086
room_data = defaultdict(lambda: {"wings": set(), "halls": set(), "count": 0})
2087+
room_instances = set()
20852088
for row in rows:
20862089
room, wing, hall, n = row[0], row[1], row[2], row[3]
2087-
if not room or room == "general" or not wing:
2090+
if not room or not wing:
20882091
continue
2089-
node = room_data[room]
2090-
node["wings"].add(str(wing))
2092+
room_key = str(room)
2093+
wing_key = str(wing)
2094+
room_instances.add((wing_key, room_key))
2095+
node = room_data[room_key]
2096+
node["wings"].add(wing_key)
20912097
if hall:
20922098
node["halls"].add(str(hall))
20932099
node["count"] += int(n)
20942100

2095-
tunnel_rooms = 0
2101+
passive_tunnel_rooms = 0
20962102
total_edges = 0
20972103
wing_counts = Counter()
20982104
for data in room_data.values():
20992105
n_wings = len(data["wings"])
21002106
for wing in data["wings"]:
21012107
wing_counts[wing] += 1
21022108
if n_wings >= 2:
2103-
tunnel_rooms += 1
2109+
passive_tunnel_rooms += 1
21042110
total_edges += (n_wings * (n_wings - 1) // 2) * len(data["halls"])
21052111

21062112
top_tunnels = [
21072113
{"room": room, "wings": sorted(data["wings"]), "count": data["count"]}
2108-
for room, data in sorted(room_data.items(), key=lambda kv: (-len(kv[1]["wings"]), kv[0]))[
2109-
:10
2110-
]
2114+
for room, data in sorted(
2115+
room_data.items(), key=lambda item: (-len(item[1]["wings"]), item[0])
2116+
)[:10]
21112117
if len(data["wings"]) >= 2
21122118
]
2119+
explicit_tunnel_count = len(_load_graph_tunnels(_config))
21132120
return {
21142121
"total_rooms": len(room_data),
2115-
"tunnel_rooms": tunnel_rooms,
2122+
"total_room_instances": len(room_instances),
2123+
"tunnel_rooms": passive_tunnel_rooms,
2124+
"passive_tunnel_rooms": passive_tunnel_rooms,
2125+
"explicit_tunnels": explicit_tunnel_count,
21162126
"total_edges": total_edges,
2127+
"total_connections": total_edges + explicit_tunnel_count,
21172128
"rooms_per_wing": dict(wing_counts.most_common()),
21182129
"top_tunnels": top_tunnels,
21192130
}

mempalace/palace_graph.py

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ def _nodes_edges_from_grouped_rows(rows):
135135
for row in rows:
136136
room, wing, hall, n = row[0], row[1], row[2], row[3]
137137
last_date = row[4] if len(row) > 4 else ""
138-
if not room or room == "general" or not wing:
138+
if not room or not wing:
139139
continue
140140
node = room_data[str(room)]
141141
node["wings"].add(str(wing))
@@ -252,7 +252,7 @@ def build_graph(col=None, config=None):
252252
wing = meta.get("wing", "")
253253
hall = meta.get("hall", "")
254254
date = meta.get("date", "")
255-
if room and room != "general" and wing:
255+
if room and wing:
256256
room_data[room]["wings"].add(wing)
257257
if hall:
258258
room_data[room]["halls"].add(hall)
@@ -409,24 +409,37 @@ def find_tunnels(wing_a: str = None, wing_b: str = None, col=None, config=None):
409409

410410

411411
def graph_stats(col=None, config=None):
412-
"""Summary statistics about the palace graph."""
412+
"""Summary statistics about the palace graph.
413+
414+
``total_rooms`` keeps its historical meaning: unique room-name nodes in
415+
the passive graph. ``total_room_instances`` counts distinct (wing, room)
416+
placements, which is the number users naturally compare with ``status``.
417+
Explicit tunnel records are reported separately so the overview does not
418+
silently omit agent-created graph connections.
419+
"""
413420
nodes, edges = build_graph(col, config)
414421

415-
tunnel_rooms = sum(1 for n in nodes.values() if len(n["wings"]) >= 2)
422+
passive_tunnel_rooms = sum(1 for n in nodes.values() if len(n["wings"]) >= 2)
423+
total_room_instances = sum(len(n["wings"]) for n in nodes.values())
424+
explicit_tunnel_count = len(_load_tunnels(config))
416425
wing_counts = Counter()
417426
for data in nodes.values():
418-
for w in data["wings"]:
419-
wing_counts[w] += 1
427+
for wing in data["wings"]:
428+
wing_counts[wing] += 1
420429

421430
return {
422431
"total_rooms": len(nodes),
423-
"tunnel_rooms": tunnel_rooms,
432+
"total_room_instances": total_room_instances,
433+
"tunnel_rooms": passive_tunnel_rooms,
434+
"passive_tunnel_rooms": passive_tunnel_rooms,
435+
"explicit_tunnels": explicit_tunnel_count,
424436
"total_edges": len(edges),
437+
"total_connections": len(edges) + explicit_tunnel_count,
425438
"rooms_per_wing": dict(wing_counts.most_common()),
426439
"top_tunnels": [
427-
{"room": r, "wings": d["wings"], "count": d["count"]}
428-
for r, d in sorted(nodes.items(), key=lambda x: -len(x[1]["wings"]))[:10]
429-
if len(d["wings"]) >= 2
440+
{"room": room, "wings": data["wings"], "count": data["count"]}
441+
for room, data in sorted(nodes.items(), key=lambda item: -len(item[1]["wings"]))[:10]
442+
if len(data["wings"]) >= 2
430443
],
431444
}
432445

tests/test_mcp_server.py

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,12 +1362,12 @@ def _no_client_open(*_a, **_k):
13621362
monkeypatch.setattr(mcp_server, "_get_collection", _no_client_open)
13631363

13641364
stats = mcp_server.tool_graph_stats()
1365-
# "general" room and the wing-less drawer are excluded, matching
1366-
# build_graph's per-drawer filter.
1367-
assert stats["total_rooms"] == 2
1365+
# "general" is a real room; the wing-less drawer is still excluded.
1366+
# Existing tripwires above guarantee the collection/HNSW path stays unopened.
1367+
assert stats["total_rooms"] == 3
13681368
assert stats["tunnel_rooms"] == 1
13691369
assert stats["total_edges"] == 1
1370-
assert stats["rooms_per_wing"] == {"wing_code": 2, "wing_project": 1}
1370+
assert stats["rooms_per_wing"] == {"wing_code": 3, "wing_project": 1}
13711371
assert stats["top_tunnels"] == [
13721372
{"room": "chromadb", "wings": ["wing_code", "wing_project"], "count": 2}
13731373
]
@@ -8138,3 +8138,40 @@ def test_search_schema_declares_window_properties(self):
81388138
assert "before" in schema["properties"]
81398139
assert schema["properties"]["since"]["type"] == "string"
81408140
assert schema["properties"]["before"]["type"] == "string"
8141+
8142+
8143+
def test_2288_grouped_graph_stats_count_distinct_room_instances(monkeypatch):
8144+
from mempalace import mcp_server
8145+
8146+
monkeypatch.setattr(
8147+
mcp_server,
8148+
"_load_graph_tunnels",
8149+
lambda config=None: [{"id": "t1"}, {"id": "t2"}],
8150+
)
8151+
rows = [
8152+
("fact", "desercion", "facts", 1, "2026-01-01"),
8153+
("general", "desercion-pascual", "misc", 1, "2026-01-01"),
8154+
("general", "desertion", "misc", 2, "2026-01-02"),
8155+
# Same placement, different hall: this must not add a room instance.
8156+
("general", "desertion", "other", 3, "2026-01-03"),
8157+
("heatstgnn-model-selection", "desertion", "models", 1, "2026-01-01"),
8158+
("diary", "desertion", "journal", 1, "2026-01-01"),
8159+
("general", "matlab-drive", "misc", 1, "2026-01-01"),
8160+
("documentation", "octopus", "docs", 1, "2026-01-01"),
8161+
("plans", "octopus", "plans", 1, "2026-01-01"),
8162+
("controller", "octopus", "control", 1, "2026-01-01"),
8163+
]
8164+
stats = mcp_server._graph_stats_from_grouped_rows(rows)
8165+
8166+
assert stats["total_rooms"] == 7
8167+
assert stats["total_room_instances"] == 9
8168+
assert stats["tunnel_rooms"] == stats["passive_tunnel_rooms"] == 1
8169+
assert stats["explicit_tunnels"] == 2
8170+
assert stats["total_connections"] == stats["total_edges"] + 2
8171+
assert set(stats["rooms_per_wing"]) == {
8172+
"desercion",
8173+
"desercion-pascual",
8174+
"desertion",
8175+
"matlab-drive",
8176+
"octopus",
8177+
}

tests/test_palace_graph.py

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,14 +113,15 @@ def test_multi_wing_creates_edges(self):
113113
assert edges[0]["wing_b"] == "wing_project"
114114
assert edges[0]["hall"] == "databases"
115115

116-
def test_general_room_excluded(self):
116+
def test_general_room_included(self):
117117
col = _make_fake_collection(
118118
[
119119
{"room": "general", "wing": "wing_code", "hall": "misc", "date": ""},
120120
]
121121
)
122122
nodes, edges = build_graph(col=col)
123-
assert "general" not in nodes
123+
assert "general" in nodes
124+
assert nodes["general"]["wings"] == ["wing_code"]
124125

125126
def test_missing_wing_excluded(self):
126127
col = _make_fake_collection(
@@ -356,3 +357,54 @@ def test_mixed_backend_artifacts_do_not_get_sniffed(self, tmp_path):
356357
(palace / "chroma.sqlite3").write_bytes(self.MAGIC)
357358
(palace / "sqlite_exact.sqlite3").write_bytes(self.MAGIC)
358359
assert sqlite_grouped_counts_reader(self._config(str(palace))) is None
360+
361+
362+
def test_2288_grouped_general_room_is_not_filtered():
363+
from mempalace.palace_graph import _nodes_edges_from_grouped_rows
364+
365+
nodes, edges = _nodes_edges_from_grouped_rows(
366+
[
367+
("general", "wing_a", "hall_one", 2, "2026-01-01"),
368+
("general", "wing_a", "hall_two", 3, "2026-01-02"),
369+
("general", "wing_b", "hall_one", 1, "2026-01-03"),
370+
]
371+
)
372+
assert nodes["general"]["wings"] == ["wing_a", "wing_b"]
373+
assert nodes["general"]["halls"] == ["hall_one", "hall_two"]
374+
assert nodes["general"]["count"] == 6
375+
assert len(edges) == 2
376+
377+
378+
def test_2288_graph_stats_preserve_room_names_and_count_room_instances():
379+
invalidate_graph_cache()
380+
col = _make_fake_collection(
381+
[
382+
{"room": "fact", "wing": "desercion"},
383+
{"room": "general", "wing": "desercion-pascual"},
384+
{"room": "general", "wing": "desertion"},
385+
{"room": "heatstgnn-model-selection", "wing": "desertion"},
386+
{"room": "diary", "wing": "desertion"},
387+
{"room": "general", "wing": "matlab-drive"},
388+
{"room": "documentation", "wing": "octopus"},
389+
{"room": "plans", "wing": "octopus"},
390+
{"room": "controller", "wing": "octopus"},
391+
]
392+
)
393+
with patch.dict(
394+
graph_stats.__globals__,
395+
{"_load_tunnels": lambda config=None: [{"id": "t1"}, {"id": "t2"}]},
396+
):
397+
stats = graph_stats(col=col)
398+
399+
assert stats["total_rooms"] == 7
400+
assert stats["total_room_instances"] == 9
401+
assert stats["tunnel_rooms"] == stats["passive_tunnel_rooms"] == 1
402+
assert stats["explicit_tunnels"] == 2
403+
assert stats["total_connections"] == stats["total_edges"] + 2
404+
assert set(stats["rooms_per_wing"]) == {
405+
"desercion",
406+
"desercion-pascual",
407+
"desertion",
408+
"matlab-drive",
409+
"octopus",
410+
}

0 commit comments

Comments
 (0)