Skip to content

fix(storage): quarantine partial HNSW flush without metadata - #1342

Merged
igorls merged 2 commits into
MemPalace:developfrom
fatkobra:fix/1274-missing-hnsw-metadata-gate
May 7, 2026
Merged

igorls merged 2 commits into
MemPalace:developfrom
fatkobra:fix/1274-missing-hnsw-metadata-gate

Conversation

@fatkobra

@fatkobra fatkobra commented May 4, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #1274.

This tightens _segment_appears_healthy() so a missing index_metadata.pickle is treated as healthy only when the HNSW segment still looks fresh or effectively empty.

If data_level0.bin already has non-trivial payload but index_metadata.pickle is missing, the segment is now treated as partially flushed corruption and quarantine_stale_hnsw() renames it before Chroma opens the native HNSW reader.

What changed

Why

#1274 reports a real palace with this segment state:

data_level0.bin       167600 bytes
header.bin            100 bytes
length.bin            400 bytes
link_lists.bin        0 bytes
index_metadata.pickle MISSING

## How to test
ruff format mempalace/backends/chroma.py tests/test_backends.py
ruff check mempalace/backends/chroma.py tests/test_backends.py
python -m pytest tests/test_backends.py -q
python -m pytest tests/test_backends.py tests/test_repair.py -q
python -m pytest tests/ -q

## Checklist
- [x] Tests pass (`python -m pytest tests/ -v`)
- [x] No hardcoded paths
- [x] Linter passes (`ruff check .`)

@igorls igorls added bug Something isn't working storage labels May 6, 2026
Two conflicts, both because MemPalace#1339 (bloated link payloads) merged into
develop after this branch was authored:

- mempalace/backends/chroma.py: _segment_appears_healthy now stacks
  three checks — bloated-link from MemPalace#1339 (top), missing-metadata-with-
  data-floor from this branch (middle), pickle format sniff (bottom).
  All three are complementary; MemPalace#1339 catches structural payload
  corruption, this branch catches pickle truncation, the original
  catches pickle protocol-byte corruption.

- tests/test_backends.py: kept both new imports (_segment_appears_healthy
  from this branch, quarantine_invalid_hnsw_metadata from MemPalace#1285).

Local: 1618 tests pass, ruff lint+format clean against 0.4.x CI pin.
@igorls igorls added this to the v3.3.5 milestone May 7, 2026
@igorls

igorls commented May 7, 2026

Copy link
Copy Markdown
Member

Maintainer-edit: merged develop in to resolve conflicts opened by #1339 (bloated link payloads, merged earlier today).

`_segment_appears_healthy` now stacks three complementary checks:

  1. `_hnsw_payload_appears_sane` (fix(storage): quarantine bloated HNSW link payloads #1339) — link_lists.bin / data_level0.bin ratio guard
  2. missing-metadata + non-empty data floor (this PR) — partial-flush detection
  3. pickle format sniff (original) — protocol-byte corruption detection

Each catches a different shape of broken-on-disk segment. Local: 1618 tests pass on the merge, ruff lint+format clean against the 0.4.x CI pin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SIGSEGV in chromadb bindings.count() on develop tip; _segment_appears_healthy false-negative for partially-flushed segments

2 participants