fix(dedup): guard dedup_palace's own count() print, not just get_source_groups
Independent review of the preceding commit found that dedup_palace's own 'Drawers: N' count() print runs a few lines before it calls get_source_groups -- #92's palace_path guard on get_source_groups only covered that function's internal count(), leaving this earlier, separate call site in the same function fully exposed to the same #1222 SIGSEGV/panic class. Same fix pattern: preflight hnsw_capacity_status before this print and abort the whole dedup_palace run on divergence. show_stats has no equivalent exposure -- it goes straight to get_source_groups with no separate count() print.
This commit is contained in:
parent
8b5e372951
commit
13196cd056
|
|
@ -183,6 +183,19 @@ def dedup_palace(
|
|||
|
||||
col = get_collection(palace_path, COLLECTION_NAME)
|
||||
|
||||
# Preflight HNSW divergence before this function's own count() print --
|
||||
# get_source_groups's palace_path guard (added alongside this one) only
|
||||
# covers its own internal count(), not this earlier one. count() on a
|
||||
# diverged segment can hard-crash the process (#1222); a try/except
|
||||
# cannot catch that, so it must never be reached at all when diverged.
|
||||
from .backends.chroma import hnsw_capacity_status
|
||||
|
||||
capacity_info = hnsw_capacity_status(palace_path, COLLECTION_NAME)
|
||||
if capacity_info.get("diverged"):
|
||||
print(f"\n HNSW index is diverged: {capacity_info.get('message', '')}")
|
||||
print(" Run `mempalace repair --mode from-sqlite --archive-existing` first.")
|
||||
return
|
||||
|
||||
print(f" Palace: {palace_path}")
|
||||
print(f" Drawers: {col.count():,}")
|
||||
print(f" Threshold: {threshold}")
|
||||
|
|
|
|||
|
|
@ -259,6 +259,26 @@ def test_dedup_palace_dry_run(mock_get_collection, mock_groups, mock_dedup_group
|
|||
mock_dedup_group.assert_called_once()
|
||||
|
||||
|
||||
@patch("mempalace.dedup.get_source_groups")
|
||||
@patch("mempalace.dedup.get_collection")
|
||||
def test_dedup_palace_aborts_on_hnsw_divergence(mock_get_collection, mock_groups, tmp_path):
|
||||
"""dedup_palace's OWN count() print (a few lines before it calls
|
||||
get_source_groups) is a separate call site from #92 -- count() on a
|
||||
diverged segment can hard-crash the process, so this print must never
|
||||
be reached either when hnsw_capacity_status reports divergence."""
|
||||
mock_col = MagicMock()
|
||||
mock_col.count.side_effect = AssertionError("count() must not be called when diverged")
|
||||
_install_mock_collection(mock_get_collection, mock_col)
|
||||
|
||||
with patch(
|
||||
"mempalace.backends.chroma.hnsw_capacity_status",
|
||||
return_value={"diverged": True, "message": "test divergence"},
|
||||
):
|
||||
dedup.dedup_palace(palace_path=str(tmp_path), dry_run=True)
|
||||
|
||||
mock_groups.assert_not_called()
|
||||
|
||||
|
||||
@patch("mempalace.dedup.dedup_source_group")
|
||||
@patch("mempalace.dedup.get_source_groups")
|
||||
@patch("mempalace.dedup.get_collection")
|
||||
|
|
|
|||
Loading…
Reference in New Issue