headroom

akadmin/headroom

Fork 0

mirror of https://github.com/headroomlabs-ai/headroom.git synced 2026-08-27 14:17:10 -04:00

Commit graph

Author	SHA1	Message	Date
Abhay Singh	6262c28a48	fix(memory/graph): skip a corrupt row instead of aborting a whole graph scan (#3239 ) ## Description `SQLiteGraphStore._row_to_entity` and `_row_to_relationship` parse stored text back into objects with no error handling: ```python properties=json.loads(row["properties"]), created_at=datetime.fromisoformat(row["created_at"]), metadata=json.loads(row["metadata"]), ``` These run inside row loops in the multi-row scans — `get_relationships` and `query_subgraph` (both the relationship loop and neighbour-entity expansion). A single unparseable row — from a partial write, a manual edit, or a bad migration — raises `ValueError` (`JSONDecodeError`/bad ISO timestamp) inside the loop, aborting the entire query and taking unrelated, perfectly good edges/nodes down with it. Reproduction (A→B and A→C both valid; corrupt only A→B's `properties`): ```python # corrupt one row out-of-band con.execute("UPDATE relationships SET properties='{oops' WHERE target_id=?", (b.id,)) # BEFORE: both of these raise JSONDecodeError, even though A->C is fine: await store.get_relationships(a.id) await store.query_subgraph([a.id], max_hops=1, direction=OUTGOING) ``` This is the same "one bad row breaks the whole scan" robustness gap already fixed for the CCR store (`cache/backends/sqlite.py`) and the vector adapter (`memory/adapters/sqlite_vector.py`); the graph adapter was the remaining store with unguarded row parsing. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/memory/adapters/sqlite_graph.py`: - `_row_to_entity` / `_row_to_relationship` now return `... \| None`, wrapping construction in `except (ValueError, TypeError, KeyError)` and returning `None` (with a `logger.warning`) on a corrupt row. - Multi-row call sites skip `None`: `get_relationships`, `query_subgraph` (initial entities, relationship loop, neighbour expansion), and the per-user entity listing. The single-row `get_entity` / `get_entity_by_name` already return `Entity \| None`, so a corrupt row now reads as "not found" rather than raising. - Added a module `logger`. - `tests/test_sqlite_graph_store.py`: added `test_one_corrupt_row_does_not_abort_a_multi_row_scan` — corrupts one relationship row out-of-band and asserts `get_relationships` returns the one good edge and `query_subgraph` completes with `{A, C}`. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality ### Test Output ```text tests/test_sqlite_graph_store.py::...one_corrupt_row_does_not_abort_a_multi_row_scan -> passes with fix, FAILS without it (verified via git stash) uvx ruff@0.16.2 check headroom/memory/adapters/sqlite_graph.py tests/test_sqlite_graph_store.py -> All checks passed! uvx mypy@1.20.2 headroom/memory/adapters/sqlite_graph.py -> Success: no issues found in 1 source file ``` (Note: this test file has pre-existing, unrelated failures/errors on `main` on Windows — `TestSQLiteGraphStoreMemoryTrackerIntegration` plus temp-file teardown `WinError 32` in the `NamedTemporaryFile`-based fixtures. Verified identical counts before and after this change; my new test uses `tmp_path` and is unaffected.) ## Real Behavior Proof - Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1, ruff 0.16.2 and mypy 1.20.2 via uvx. - Exact command / steps: built A→B and A→C edges, corrupted A→B's `properties` to invalid JSON via a direct sqlite connection, then called `get_relationships(A)` and `query_subgraph([A], OUTGOING)`. Before the fix both raised `JSONDecodeError`; after the fix `get_relationships` returns just the A→C edge and `query_subgraph` returns entities `{A, C}` with one relationship, skipping the corrupt row. - Observed result: corrupt rows are skipped (with a warning log); valid rows in the same scan are returned normally. - Not tested: no corruption occurs in normal operation; the corrupt row is produced out-of-band to exercise the guard (matching the real triggers: partial write, manual edit, migration). ## Runtime Rollout Safety - Rollout-managed feature(s): none. This is a SQLite graph-store read path in the memory subsystem, not a rollout-channel-gated runtime feature. - Minimum rollout channel: N/A. - Stable/default behavior changed: no for well-formed data — every valid row parses and is returned exactly as before. Only the previously-crashing corrupt-row case changes, from an aborted query to a skipped row. - Kill switch / disable path: N/A. - Unsafe override required: no. - Qualification impact: none. - Rollback path: revert this PR. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation (N/A: internal behavior) - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I did not edit `CHANGELOG.md`	2026-08-25 11:40:54 +05:30
chopratejas	f2ecf1d2d0	Add SQLiteGraphStore for bounded, persistent graph storage Replace unbounded InMemoryGraphStore with SQLite-backed implementation: - Persistent storage survives proxy restarts - Memory bounded by configurable SQLite page cache (default 8MB) - Same async interface as InMemoryGraphStore (drop-in replacement) - LocalBackend now uses SQLiteGraphStore by default (graph_persist=True) New files: - headroom/memory/adapters/sqlite_graph.py: SQLite graph store implementation - tests/test_sqlite_graph_store.py: 37 comprehensive tests Key features: - O(log n) lookups via database indexes - Case-insensitive entity name lookup per user - BFS subgraph traversal and shortest path finding - CASCADE delete for entity relationships - MemoryTracker integration via get_memory_stats()	2026-02-01 20:48:57 -08:00

Author

SHA1

Message

Date

Abhay Singh

6262c28a48

fix(memory/graph): skip a corrupt row instead of aborting a whole graph scan (#3239 )

## Description

`SQLiteGraphStore._row_to_entity` and `_row_to_relationship` parse
stored text back into objects with no error handling:

```python
properties=json.loads(row["properties"]),
created_at=datetime.fromisoformat(row["created_at"]),
metadata=json.loads(row["metadata"]),
```

These run inside row loops in the multi-row scans — `get_relationships`
and `query_subgraph` (both the relationship loop and neighbour-entity
expansion). A single unparseable row — from a partial write, a manual
edit, or a bad migration — raises `ValueError` (`JSONDecodeError`/bad
ISO timestamp) *inside the loop*, aborting the **entire** query and
taking unrelated, perfectly good edges/nodes down with it.

Reproduction (A→B and A→C both valid; corrupt only A→B's `properties`):

```python
# corrupt one row out-of-band
con.execute("UPDATE relationships SET properties='{oops' WHERE target_id=?", (b.id,))
# BEFORE: both of these raise JSONDecodeError, even though A->C is fine:
await store.get_relationships(a.id)
await store.query_subgraph([a.id], max_hops=1, direction=OUTGOING)
```

This is the same "one bad row breaks the whole scan" robustness gap
already fixed for the CCR store (`cache/backends/sqlite.py`) and the
vector adapter (`memory/adapters/sqlite_vector.py`); the graph adapter
was the remaining store with unguarded row parsing.

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- `headroom/memory/adapters/sqlite_graph.py`:
- `_row_to_entity` / `_row_to_relationship` now return `... | None`,
wrapping construction in `except (ValueError, TypeError, KeyError)` and
returning `None` (with a `logger.warning`) on a corrupt row.
- Multi-row call sites skip `None`: `get_relationships`,
`query_subgraph` (initial entities, relationship loop, neighbour
expansion), and the per-user entity listing. The single-row `get_entity`
/ `get_entity_by_name` already return `Entity | None`, so a corrupt row
now reads as "not found" rather than raising.
  - Added a module `logger`.
- `tests/test_sqlite_graph_store.py`: added
`test_one_corrupt_row_does_not_abort_a_multi_row_scan` — corrupts one
relationship row out-of-band and asserts `get_relationships` returns the
one good edge and `query_subgraph` completes with `{A, C}`.

## Testing

- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check`)
- [x] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality

### Test Output

```text
tests/test_sqlite_graph_store.py::...one_corrupt_row_does_not_abort_a_multi_row_scan  ->  passes with fix, FAILS without it (verified via git stash)
uvx ruff@0.16.2 check headroom/memory/adapters/sqlite_graph.py tests/test_sqlite_graph_store.py  ->  All checks passed!
uvx mypy@1.20.2 headroom/memory/adapters/sqlite_graph.py  ->  Success: no issues found in 1 source file
```

(Note: this test file has pre-existing, unrelated failures/errors on
`main` on Windows — `TestSQLiteGraphStoreMemoryTrackerIntegration` plus
temp-file teardown `WinError 32` in the `NamedTemporaryFile`-based
fixtures. Verified identical counts before and after this change; my new
test uses `tmp_path` and is unaffected.)

## Real Behavior Proof

- Environment: Windows 11, Python 3.12.11, project venv, pytest 9.1.1,
ruff 0.16.2 and mypy 1.20.2 via uvx.
- Exact command / steps: built A→B and A→C edges, corrupted A→B's
`properties` to invalid JSON via a direct sqlite connection, then called
`get_relationships(A)` and `query_subgraph([A], OUTGOING)`. Before the
fix both raised `JSONDecodeError`; after the fix `get_relationships`
returns just the A→C edge and `query_subgraph` returns entities `{A, C}`
with one relationship, skipping the corrupt row.
- Observed result: corrupt rows are skipped (with a warning log); valid
rows in the same scan are returned normally.
- Not tested: no corruption occurs in normal operation; the corrupt row
is produced out-of-band to exercise the guard (matching the real
triggers: partial write, manual edit, migration).

## Runtime Rollout Safety

- Rollout-managed feature(s): none. This is a SQLite graph-store read
path in the memory subsystem, not a rollout-channel-gated runtime
feature.
- Minimum rollout channel: N/A.
- Stable/default behavior changed: no for well-formed data — every valid
row parses and is returned exactly as before. Only the
previously-crashing corrupt-row case changes, from an aborted query to a
skipped row.
- Kill switch / disable path: N/A.
- Unsafe override required: no.
- Qualification impact: none.
- Rollback path: revert this PR.

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review

## Checklist

- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation (N/A:
internal behavior)
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [x] I did **not** edit `CHANGELOG.md`

2026-08-25 11:40:54 +05:30

chopratejas

f2ecf1d2d0

Add SQLiteGraphStore for bounded, persistent graph storage

Replace unbounded InMemoryGraphStore with SQLite-backed implementation:
- Persistent storage survives proxy restarts
- Memory bounded by configurable SQLite page cache (default 8MB)
- Same async interface as InMemoryGraphStore (drop-in replacement)
- LocalBackend now uses SQLiteGraphStore by default (graph_persist=True)

New files:
- headroom/memory/adapters/sqlite_graph.py: SQLite graph store implementation
- tests/test_sqlite_graph_store.py: 37 comprehensive tests

Key features:
- O(log n) lookups via database indexes
- Case-insensitive entity name lookup per user
- BFS subgraph traversal and shortest path finding
- CASCADE delete for entity relationships
- MemoryTracker integration via get_memory_stats()

2026-02-01 20:48:57 -08:00

2 commits