Skip to content

Fix embedded-FalkorDB claim read invisibility and default to sentence-transformers#1002

Open
nndn wants to merge 7 commits into
mainfrom
nandan/pot-1918-p01-write-read-flow
Open

Fix embedded-FalkorDB claim read invisibility and default to sentence-transformers#1002
nndn wants to merge 7 commits into
mainfrom
nandan/pot-1918-p01-write-read-flow

Conversation

@nndn

@nndn nndn commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

On the default falkordb_lite backend, claims anchored on a pot's first-created entity (internal node id 0 — always the Repository in the standard flow) were invisible to every read surface while commit reported success. The embedded engine returns zero rows for a source-anchored traversal when the bound source node is id 0; both claim read paths and the embedding-attach query used exactly that shape, and every miss was silent. This PR fixes the query shapes, makes the remaining failure modes loud, adds a real re-embed repair, and switches the default embedder to sentence-transformers.

1. Edge-first claim reads

  • FIND_CLAIMS_CYPHER and both vector queries drop endpoint-node bindings — unbound endpoints keep labels() filters working and compile to a pure Edge By Index Scan (pinned with an EXPLAIN-based regression test).
  • fact_query is now lexical membership + vector score overlay: a claim whose embedding is missing, stale, or unreachable degrades to a lexical score instead of disappearing from reads (verify readback, views, search, UI). Vector-query failures log a warning naming the repair command instead of silently returning [].

2. Loud, verifiable embedding attach

  • The attach MATCH is edge-first and count-verified; zero matches raise and are reported per edge, with a batch summary warning.
  • commit --verify readback reports unembedded_claim_keys when the backend runs in vector mode, escalating verification status to watch with a repair recommendation.

3. Real semantic_index repair + index migration

  • graph repair --semantic-index (previously a no-op) re-embeds claims whose embeddings are missing, written by a different model, or written at different dimensions; on a dimension change it drops and recreates the vector index.
  • potpie setup gains a soft embeddings.reindex step, so upgraded installs are migrated by the standard onboarding command.

4. sentence-transformers by default

  • build_embedder() unset default: localauto. The shipped potpie wheel already carries sentence-transformers, and setup already selects and persists it — this closes the gap for processes that never ran setup (fresh daemon, MCP, CI), which silently used the 256-dim hashing embedder.
  • graph status now surfaces the active embedder {name, dimensions} beside match_mode — hashing-vector and semantic-vector are different quality tiers and no longer look identical.
  • The test suite pins CONTEXT_ENGINE_EMBEDDER=local in conftest for offline determinism; default-path tests monkeypatch installation state.
  • Safe for existing pots: the repair pass in (3) migrates hashing-256 stores in place.

5. Lazy vector-index creation

Only setup's provision step created the vector index, so a home where setup never ran wrote embeddings no vector query could reach. The writer now ensures indexes once before its first vector write.

Test plan

  • Unit + conformance: 2,102 passed (2 pre-existing environmental failures in daemon subprocess tests — bare python not on PATH — fail identically on main)
  • Integration on real falkordblite: EXPLAIN plan-shape regression pin; full recover-and-migrate cycle (strip an embedding + switch embedder model/dims → repair re-embeds all, rebuilds the index, search works at the new dimensions)
  • Live scratch-home end-to-end: fresh install → vector mode with sentence-transformers/all-MiniLM-L6-v2 (384 dims) and zero config; commit --verify reads back with no missing/unembedded claims; semantic search returns scored results; repair is a no-op on healthy pots; a claim committed under the hashing embedder is migrated in place by repair

🤖 Generated with Claude Code

nndn and others added 5 commits July 9, 2026 17:29
Embedded FalkorDB returns zero rows for a source-anchored traversal when
the bound source is internal node id 0 — always the Repository, the first
entity a baseline flow creates. Both claim read paths used that shape.

- FIND_CLAIMS_CYPHER now scans edge-first with unbound endpoints;
  labels(a)/labels(b) filters still evaluate (POT-1918 / P0-1).
- The vector fact_query drops its post-procedure MATCH entirely and
  filters on edge properties; entity-label filters re-apply in Python.
- fact_query results are now lexical-membership + vector-score overlay:
  a claim whose embedding is missing, stale, or unreachable degrades to
  a lexical score instead of disappearing from every read surface.
- Vector query failures log a warning naming the repair command instead
  of silently returning [].

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The FalkorDB writer attached fact embeddings through a source-anchored
MATCH — the id-0 planner-defect shape — and a zero-row match set nothing,
raised nothing, and logged nothing. All repo-subject claims on a fresh
pot lost their embeddings this way (POT-1918 / P0-1).

- Attach MATCH is now edge-first (unbound endpoints) and returns
  count(r); zero matches raise and are reported per edge.
- Batch summary warning names the repair command.
- commit --verify readback now flags committed claims that read back
  without a fact embedding (vector-mode backends only) as
  verification.unembedded_claim_keys, escalating status to "watch" with
  a repair recommendation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
'graph repair --semantic-index' previously returned "nothing to rebuild".
It now re-embeds every claim whose embedding is missing (a failed attach),
written by a different model, or written at different dimensions — making
it both the recovery pass for POT-1918/P0-1 damage and the migration path
for embedder changes (hashing-256 → sentence-transformers-384).

- FalkorDB backend: edge-first claim scan, retrieval-card rebuild from
  stored props, count-verified re-embed writes, and vector-index
  drop/recreate when stored dimensions disagree with the active embedder.
- In-memory/embedded backend: re-embeds missing or mis-sized rows.
- ClaimQueryAnalytics runs both semantic_index and entity_summaries
  targets, reporting failures as semantic_index_failed.
- potpie setup gains a soft embeddings.reindex step so upgraded installs
  are migrated by the standard onboarding command.
- Integration: EXPLAIN-based plan-shape pin (no bound node scan under the
  claim edge scan) + full recover-and-migrate cycle on real falkordblite.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ntity

The shipped potpie wheel already carries sentence-transformers via the
[all] extra, and `potpie setup` already selects and downloads it — but the
library-level default stayed `local` (hashing), so any process that never
ran setup (fresh daemon, MCP, CI) silently used fuzzy-lexical vectors.

- build_embedder() unset default: local → auto (sentence-transformers when
  importable, hashing otherwise — with an INFO line, since the downgrade
  changes match quality).
- graph status now reports the active embedder {name, dimensions} beside
  match_mode: "vector" from the hashing fallback and from a semantic model
  are different quality tiers and must not look the same.
- Test suite pins CONTEXT_ENGINE_EMBEDDER=local in conftest for
  determinism/offline; default-path tests monkeypatch installation state.

Safe for existing pots because `graph repair semantic_index` and the setup
reindex step migrate hashing-256 stores to the new model/dimensions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The vector index was only created by setup's backend.provision step, so a
pot written on a home where setup never ran (fresh installs driven purely
through the CLI graph surface, library embedding, tests) attached
embeddings that no vector query could reach — surfacing as 'Invalid
arguments for procedure db.idx.vector.queryRelationships' on every search.
Found by the live end-to-end check; the write path now ensures indexes
once per writer before its first vector attach.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jul 9, 2026

Copy link
Copy Markdown

POT-1918

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Added semantic index repair to re-embed stale/missing claim embeddings and refresh vector indexes, plus a setup step to run it.
    • Search ranking now overlays vector scores without hiding lexical results when embeddings are present.
    • Status/verification payloads now expose active embedder identity and surface unembedded claim keys with recommended next actions.
  • Bug Fixes
    • Graph reads and writes now avoid bound-node scanning, preserving visibility and preventing silent embedding attachment no-ops.
    • Vector query failures degrade gracefully, falling back to lexical scoring.
  • Tests
    • Expanded FalkorDB and repair regression coverage, including index migration and failure handling.

Walkthrough

Adds semantic-index repair and lexical-first vector overlays, exposes embedder and verification diagnostics, adds setup reindexing, changes local embedder defaults, and expands graph backend tests.

Changes

Semantic index repair and vector overlay reads

Layer / File(s) Summary
Repair predicates and result contracts
potpie/context-engine/adapters/outbound/graph/semantic_index_repair.py, potpie/context-engine/adapters/outbound/graph/backends/claim_query_analytics.py, potpie/context-engine/domain/graph_plans.py, potpie/context-engine/domain/ports/services/graph_service.py
Defines repair targets and predicates, aggregates repair reports, and adds embedder and unembedded-claim fields to status and verification DTOs.
Backend semantic index repair
potpie/context-engine/adapters/outbound/graph/backends/falkordb_backend.py, potpie/context-engine/adapters/outbound/graph/backends/in_memory_backend.py
Re-embeds missing or stale claims, reports repaired and failed entries, and rebuilds the FalkorDB vector index when dimensions change.
Canonical claim queries and vector overlays
potpie/context-engine/adapters/outbound/graph/canonical_claim_query.py, potpie/context-engine/adapters/outbound/graph/falkordb_reader.py, potpie/context-engine/adapters/outbound/graph/neo4j_reader.py, potpie/context-engine/adapters/outbound/graph/in_memory_reader.py, potpie/context-engine/adapters/outbound/graph/falkordb_inspection.py
Uses edge-first queries and shared helpers to preserve lexical claims while overlaying vector ranking and applying label filters in Python.
FalkorDB edge embedding attachment
potpie/context-engine/adapters/outbound/graph/falkordb_writer.py
Ensures vector indexes, validates relationship updates, and returns structured attachment failures.
Embedder status and reindex lifecycle
potpie/context-engine/adapters/inbound/cli/commands/graph.py, potpie/context-engine/adapters/outbound/intelligence/local_embedder.py, potpie/context-engine/application/services/graph_service.py, potpie/context-engine/application/services/setup_orchestrator.py, potpie/context-engine/application/services/graph_workbench.py
Reports embedder identity, defaults to automatic local model selection with hashing fallback, runs a soft embeddings reindex step, and flags unembedded claims during verification.
Validation coverage
potpie/context-engine/tests/*
Adds unit and integration coverage for repair behavior, edge-first queries, vector overlays, writer attachment validation, verification diagnostics, and embedder selection.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ClaimQueryStore
  participant GraphBackend
  participant Embedder

  Client->>ClaimQueryStore: find_claims(filter)
  ClaimQueryStore->>GraphBackend: run lexical claim query
  ClaimQueryStore->>GraphBackend: run vector overlay query when enabled
  GraphBackend-->>ClaimQueryStore: lexical rows and vector scores
  ClaimQueryStore->>ClaimQueryStore: merge vector scores with lexical rows
  ClaimQueryStore-->>Client: ranked claim results
Loading

Possibly related PRs

  • potpie-ai/potpie#819: Introduces the FalkorDB graph and claim-query backend used by these changes.
  • potpie-ai/potpie#905: Modifies the shared DataPlaneStatus contract also extended here with embedder identity.
  • potpie-ai/potpie#976: Modifies embedding lifecycle wiring and local embedder setup related to this reindex flow.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the PR’s main changes: FalkorDB claim read fixes and the new default embedder behavior.
Description check ✅ Passed The description accurately covers the claim-read fix, semantic repair, embedder default change, status reporting, and tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nndn nndn changed the title Fix falkordb_lite write→read invisibility and default to sentence-transformers (POT-1918 / P0-1) Fix embedded-FalkorDB claim read invisibility and default to sentence-transformers Jul 9, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
potpie/context-engine/adapters/outbound/graph/backends/in_memory_backend.py (1)

499-527: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Silent no-op when semantic repair is requested but no embedder is configured, unlike the FalkorDB backend.

When wants_semantic_index_repair(targets) is true but self.store.embedder is None, this branch is skipped entirely — no semantic_index key, no detail. If entity_summary repair also isn't requested, the caller falls through to the generic "in_memory projections are computed on read; nothing to rebuild" message, which is misleading (the real reason nothing happened is the missing embedder, not that there's nothing to rebuild).

Compare with FalkorDBGraphBackend._repair_semantic_index, which always wires the callback and returns an explicit "no embedder configured; nothing to re-embed" detail. This inconsistency degrades operator visibility for anything that calls analytics.repair directly (e.g., a future graph repair --semantic-index CLI path) against an in-memory/test backend.

♻️ Suggested fix
-        if wants_semantic_index_repair(targets) and self.store.embedder is not None:
-            reembedded = self._repair_semantic_index(pot_id)
-            repaired[SEMANTIC_INDEX_TARGET] = reembedded
-            details.append(f"re-embedded {reembedded} stale claim(s)")
-            changed = changed or bool(reembedded)
+        if wants_semantic_index_repair(targets):
+            if self.store.embedder is not None:
+                reembedded = self._repair_semantic_index(pot_id)
+                repaired[SEMANTIC_INDEX_TARGET] = reembedded
+                details.append(f"re-embedded {reembedded} stale claim(s)")
+                changed = changed or bool(reembedded)
+            else:
+                details.append("no embedder configured; nothing to re-embed")

Separately, _repair_semantic_index (Line 529) swallows per-row embedding exceptions with a bare continue and never counts/reports failures, whereas FalkorDBGraphBackend._repair_semantic_index tracks and surfaces a semantic_index_failed count. setup_orchestrator._semantic_reindex relies on that key to decide FAILED vs DONE; against this backend it will always report DONE even if some rows silently failed to re-embed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@potpie/context-engine/adapters/outbound/graph/backends/in_memory_backend.py`
around lines 499 - 527, Update in_memory_backend.py so repair() does not
silently fall through when wants_semantic_index_repair(targets) is requested but
self.store.embedder is None; instead record the semantic_index target in
repaired and return an explicit detail like “no embedder configured; nothing to
re-embed,” matching FalkorDBGraphBackend behavior. Also update
_repair_semantic_index() to stop swallowing per-row embedding failures: count
failed rows, surface that in the returned repair summary (for example via a
semantic_index_failed-style count or equivalent detail), and ensure repair()
includes that information so callers like setup_orchestrator._semantic_reindex
can detect partial failure correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@potpie/context-engine/adapters/outbound/graph/backends/falkordb_backend.py`:
- Around line 76-83: The tuple re-embed Cypher in _REEMBED_BY_TUPLE_CYPHER
should not use a null source_ref as a wildcard match, since that can update
every RELATES_TO edge sharing the same tuple. Update the fallback logic in the
FalkorDB backend to either require an explicit source_ref match or skip
tuple-based repair when source_ref is missing, and make the matching behavior in
the re-embed path precise enough to preserve edge identity.
- Around line 85-88: The vector index repair in FalkorDB is graph-wide, but the
current repair flow only rewrites claims for a single group_id and does not
surface index failures. Update the repair path in FalkorDBBackend so DROP/CREATE
VECTOR INDEX errors are captured and returned in the repair result instead of
being logged only, and make sure the index maintenance logic clearly reflects
that it affects the whole RELATES_TO graph rather than just one pot. Keep the
fix localized around the vector-index helper constants and the repair method
that applies them.

In `@potpie/context-engine/adapters/outbound/graph/backends/in_memory_backend.py`:
- Around line 529-549: The semantic repair logic in _repair_semantic_index only
validates vector length, so same-dimension embedding model drift is never
detected. Update the in_memory_backend repair path to consider embedding model
identity too, if that metadata is available on ClaimRow or the store state, and
make the decision consistent with FalkorDBGraphBackend.claim_needs_reembed. If
the in-memory backend intentionally cannot persist model metadata, keep the
current behavior but add an explicit note or guard in _repair_semantic_index
clarifying that parity with the persistent backend is not expected.

In `@potpie/context-engine/adapters/outbound/graph/canonical_claim_query.py`:
- Around line 243-250: Materialize vector_scored before iterating twice in the
claim ranking logic, since it is typed as an Iterable and may be a one-shot
generator. In the function that builds out_scored and then appends vector-only
rows, convert vector_scored to a reusable collection first, then use that cached
collection for both the scores map and the final loop so no entries are lost.

In `@potpie/context-engine/adapters/outbound/graph/falkordb_reader.py`:
- Around line 134-153: The FalkorDB reader currently performs lexical and
optional vector work even when an explicit zero-row limit is requested, which
can trigger unnecessary queries and warnings. Update the query flow in the
reader method that builds rows from _find_claims_lexical and
merge_vector_scored_rows to mirror the Neo4j reader’s early return behavior: if
filter_.limit is 0, return an empty result before any FalkorDB or vector
processing happens. Keep the existing slicing logic for other non-negative
limits unchanged.

In `@potpie/context-engine/adapters/outbound/graph/falkordb_writer.py`:
- Around line 362-365: The index-creation flow in `FalkorDBWriter` is marking
`_indexes_ensured` true even when `_ensure_indexes_sync` has silently swallowed
a `graph.query(stmt)` failure. Update `_ensure_indexes_sync` so it returns
whether indexes were already present or were created successfully, and have the
caller only set `_indexes_ensured` after a successful result; if creation fails,
leave it false so later writes can retry. Use the `FalkorDBWriter` methods
`_ensure_indexes_sync` and the `_indexes_ensured` guard as the main touchpoints.

---

Outside diff comments:
In `@potpie/context-engine/adapters/outbound/graph/backends/in_memory_backend.py`:
- Around line 499-527: Update in_memory_backend.py so repair() does not silently
fall through when wants_semantic_index_repair(targets) is requested but
self.store.embedder is None; instead record the semantic_index target in
repaired and return an explicit detail like “no embedder configured; nothing to
re-embed,” matching FalkorDBGraphBackend behavior. Also update
_repair_semantic_index() to stop swallowing per-row embedding failures: count
failed rows, surface that in the returned repair summary (for example via a
semantic_index_failed-style count or equivalent detail), and ensure repair()
includes that information so callers like setup_orchestrator._semantic_reindex
can detect partial failure correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2531d7ae-7a5e-4996-860c-dfa4580e8ea5

📥 Commits

Reviewing files that changed from the base of the PR and between a6ae3da and f0d5328.

📒 Files selected for processing (24)
  • potpie/context-engine/adapters/inbound/cli/commands/graph.py
  • potpie/context-engine/adapters/outbound/graph/backends/claim_query_analytics.py
  • potpie/context-engine/adapters/outbound/graph/backends/falkordb_backend.py
  • potpie/context-engine/adapters/outbound/graph/backends/in_memory_backend.py
  • potpie/context-engine/adapters/outbound/graph/canonical_claim_query.py
  • potpie/context-engine/adapters/outbound/graph/falkordb_reader.py
  • potpie/context-engine/adapters/outbound/graph/falkordb_writer.py
  • potpie/context-engine/adapters/outbound/graph/in_memory_reader.py
  • potpie/context-engine/adapters/outbound/graph/neo4j_reader.py
  • potpie/context-engine/adapters/outbound/graph/semantic_index_repair.py
  • potpie/context-engine/adapters/outbound/intelligence/local_embedder.py
  • potpie/context-engine/application/services/graph_service.py
  • potpie/context-engine/application/services/graph_workbench.py
  • potpie/context-engine/application/services/setup_orchestrator.py
  • potpie/context-engine/domain/graph_plans.py
  • potpie/context-engine/domain/ports/services/graph_service.py
  • potpie/context-engine/tests/conftest.py
  • potpie/context-engine/tests/integration/test_falkordb_roundtrip.py
  • potpie/context-engine/tests/unit/test_falkordb_reader.py
  • potpie/context-engine/tests/unit/test_falkordb_writer.py
  • potpie/context-engine/tests/unit/test_graph_workbench_plans.py
  • potpie/context-engine/tests/unit/test_local_embedder_setup.py
  • potpie/context-engine/tests/unit/test_neo4j_claim_query.py
  • potpie/context-engine/tests/unit/test_semantic_index_repair.py

Comment on lines +529 to +549
def _repair_semantic_index(self, pot_id: str) -> int:
"""Re-embed rows whose stored vector is missing or mis-sized."""
embedder = self.store.embedder
assert embedder is not None
dims = int(getattr(embedder, "dimensions", 0))
repaired = 0
for idx, row in enumerate(self.store.rows):
if row.pot_id != pot_id:
continue
if row.fact_embedding is not None and len(row.fact_embedding) == dims:
continue
try:
embedding = tuple(
float(x) for x in embedder.embed(card_for_row(row))
)
except Exception: # noqa: BLE001 - repair must not die on one row.
continue
self.store.rows[idx] = _with_embedding(row, embedding)
repaired += 1
return repaired

@coderabbitai coderabbitai Bot Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Only checks embedding length, not model identity — no detection of same-dimension model drift.

Unlike FalkorDBGraphBackend's claim_needs_reembed (which also compares embedding_model), this skips re-embedding whenever len(row.fact_embedding) == dims, even if the vector was produced by a different (but same-dimension) model. This is likely acceptable for an in-memory/test backend that doesn't persist embedding_model/embedding_dim metadata on ClaimRow, but it means the two backends' repair semantics aren't at parity.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@potpie/context-engine/adapters/outbound/graph/backends/in_memory_backend.py`
around lines 529 - 549, The semantic repair logic in _repair_semantic_index only
validates vector length, so same-dimension embedding model drift is never
detected. Update the in_memory_backend repair path to consider embedding model
identity too, if that metadata is available on ClaimRow or the store state, and
make the decision consistent with FalkorDBGraphBackend.claim_needs_reembed. If
the in-memory backend intentionally cannot persist model metadata, keep the
current behavior but add an explicit note or guard in _repair_semantic_index
clarifying that parity with the persistent backend is not expected.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipping this one deliberately: ClaimRow doesn't persist embedding_model/embedding_dim metadata, so the in-memory/embedded repair keys on dimensions only. Same-dimension model drift can't occur within a single in-memory process (one embedder instance), and the embedded store re-derives embeddings through the active embedder on repair anyway.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nndn Thanks for the clarification — that makes sense. Since ClaimRow has no embedding_model/embedding_dim fields and the in-memory backend only ever runs with a single embedder instance per process, same-dimension model drift isn't a real scenario there, and repair already re-derives via the active embedder. Withdrawing the nitpick.


✏️ Learnings added
Learnt from: nndn
Repo: potpie-ai/potpie PR: 1002
File: potpie/context-engine/adapters/outbound/graph/backends/in_memory_backend.py:529-547
Timestamp: 2026-07-10T06:12:43.561Z
Learning: In `potpie/context-engine/adapters/outbound/graph/backends/in_memory_backend.py`, `ClaimRow` does not persist `embedding_model`/`embedding_dim` metadata, so `_Analytics._repair_semantic_index` intentionally keys re-embedding decisions on embedding dimension length only, unlike `FalkorDBGraphBackend`'s `claim_needs_reembed` which also compares `embedding_model`. This is by design: the in-memory/embedded backend runs with a single embedder instance per process, so same-dimension model drift cannot occur, and repair re-derives embeddings via the active embedder regardless. Parity with the persistent FalkorDB backend's model-identity check is not expected or required here.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment thread potpie/context-engine/adapters/outbound/graph/falkordb_reader.py
Comment thread potpie/context-engine/adapters/outbound/graph/falkordb_writer.py Outdated
The premerge CLI journey exposed a sharper version of the read-invisibility
defect: after the embedded store is persisted and reloaded (every CLI call
is a new process), any query plan that resolves the edge ENDPOINTS through
node scans silently returns zero rows when internal node id 0 — the first
entity ever created — is an endpoint. Even an unbound `labels(a)` reference
in WHERE (with a null parameter) plans an `All Node Scan` under the edge
index scan and hits it. In-process reads never reproduce it, which is why
the existing roundtrip tests stayed green.

- FIND_CLAIMS_CYPHER no longer references the endpoints at all; the
  subject_label/object_label filters are applied in Python via the
  node-only entity_labels lookup (shared filter_rows_by_labels helper,
  FalkorDB + Neo4j readers).
- The explorer edge queries (slice/neighborhood) read source/target off
  the edge properties instead of the endpoint nodes.
- The writer's lazy index ensure creates only the vector index — creating
  the range indexes mid-flight flipped planner behaviour for every other
  query on stores that were never provisioned (exactly the journey route),
  and a failed creation is retried on the next batch instead of cached.
- A live falkordblite regression test pins the full journey shape:
  seed + indexes, close, reopen, then read/count/inspect.

Also addresses review feedback: a null source_ref no longer acts as a
wildcard in the tuple re-embed, index drop/recreate failures surface in
the repair report, merge_vector_scored_rows accepts one-shot iterators,
and the FalkorDB reader honours limit=0 before querying.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
potpie/context-engine/adapters/outbound/graph/falkordb_writer.py (1)

236-252: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Provisioning path still swallows real index-creation failures.

_ensure_vector_index_sync (below) was added to fix exactly this pattern for the lazy write path, but ensure_indexes() / _ensure_indexes_sync() — the method FalkorDBGraphBackend.provision() calls for potpie graph provision/setup — still swallows every graph.query(stmt) exception at debug level and unconditionally returns success. A real failure here makes provision() report state=DONE while indexes (including the vector index) were never created.

🛡️ Suggested direction (mirrors the sibling fix)
-    async def ensure_indexes(self) -> bool:
+    async def ensure_indexes(self) -> bool:
         if not self.enabled:
             return False
         graph = self._get_graph()
         embedding_dim = int(getattr(self._embedder, "dimensions", 1536))
-        await asyncio.to_thread(self._ensure_indexes_sync, graph, embedding_dim)
-        return True
+        return await asyncio.to_thread(self._ensure_indexes_sync, graph, embedding_dim)

     `@staticmethod`
-    def _ensure_indexes_sync(graph: Any, embedding_dim: int = 1536) -> None:
-        for stmt in _index_statements(embedding_dim):
-            try:
-                graph.query(stmt)
-            except Exception as exc:  # noqa: BLE001
-                # Re-running creates an "already indexed" error; best-effort.
-                logger.debug("falkordb index skipped (%s): %s", stmt, exc)
+    def _ensure_indexes_sync(graph: Any, embedding_dim: int = 1536) -> bool:
+        ok = True
+        for stmt in _index_statements(embedding_dim):
+            try:
+                graph.query(stmt)
+            except Exception as exc:  # noqa: BLE001
+                if "already indexed" in str(exc).lower():
+                    continue
+                logger.warning("falkordb index creation failed (%s): %s", stmt, exc)
+                ok = False
+        return ok
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@potpie/context-engine/adapters/outbound/graph/falkordb_writer.py` around
lines 236 - 252, Update FalkorDBGraphBackend.ensure_indexes and
_ensure_indexes_sync so index-creation failures are not silently swallowed or
reported as successful provisioning. Preserve the special handling for
already-existing indexes, but re-raise or otherwise propagate genuine
graph.query(stmt) errors so provision() can fail instead of returning
state=DONE; mirror the behavior of _ensure_vector_index_sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@potpie/context-engine/adapters/outbound/graph/falkordb_writer.py`:
- Around line 236-252: Update FalkorDBGraphBackend.ensure_indexes and
_ensure_indexes_sync so index-creation failures are not silently swallowed or
reported as successful provisioning. Preserve the special handling for
already-existing indexes, but re-raise or otherwise propagate genuine
graph.query(stmt) errors so provision() can fail instead of returning
state=DONE; mirror the behavior of _ensure_vector_index_sync.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 478dee68-3b15-487a-87a8-4a347c2b2031

📥 Commits

Reviewing files that changed from the base of the PR and between f0d5328 and f3ef1c8.

📒 Files selected for processing (16)
  • potpie/context-engine/adapters/outbound/graph/backends/falkordb_backend.py
  • potpie/context-engine/adapters/outbound/graph/backends/in_memory_backend.py
  • potpie/context-engine/adapters/outbound/graph/canonical_claim_query.py
  • potpie/context-engine/adapters/outbound/graph/falkordb_inspection.py
  • potpie/context-engine/adapters/outbound/graph/falkordb_reader.py
  • potpie/context-engine/adapters/outbound/graph/falkordb_writer.py
  • potpie/context-engine/adapters/outbound/graph/neo4j_reader.py
  • potpie/context-engine/adapters/outbound/graph/semantic_index_repair.py
  • potpie/context-engine/application/services/graph_workbench.py
  • potpie/context-engine/tests/integration/test_falkordb_roundtrip.py
  • potpie/context-engine/tests/unit/test_falkordb_inspection.py
  • potpie/context-engine/tests/unit/test_falkordb_reader.py
  • potpie/context-engine/tests/unit/test_falkordb_writer.py
  • potpie/context-engine/tests/unit/test_graph_workbench_plans.py
  • potpie/context-engine/tests/unit/test_neo4j_claim_query.py
  • potpie/context-engine/tests/unit/test_semantic_index_repair.py

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant