feat(resolution): IncrementalResolution — graph-aware entity linking

[galshubeli]

New IncrementalResolution strategy + exports, and it is wired as the default resolver in the GraphRAG facade.

Behavior change: when an LLM and embedder are configured, ingestion now defaults to IncrementalResolution (was ExactMatchResolution), which adds LLM + embedding calls per ingest. It falls back to ExactMatchResolution automatically when no LLM/embedder is present, so LLM-free setups are unchanged; opt out explicitly with resolver=ExactMatchResolution(). Documented in docs/ingestion.md, docs/strategies.md, and CHANGELOG.

Closes #277

---

Closes #277

What

Adds IncrementalResolution, a resolution strategy that resolves a document's
entities against the knowledge graph built so far — so a mention in today's
document can be linked to an entity extracted from a document ingested earlier.
Ordinary batch resolvers only deduplicate a document's entities among
themselves and structurally cannot see cross-document duplicates.

Algorithm

A funnel — cheap, certain work first; the LLM only where cheap signals can't decide:

1. collapse — merge a batch's own same-name duplicates (description-gated, no LLM)
2. retrieve — for each survivor, fetch look-alike existing graph entities
3. pile — group survivors that share a candidate (union-find)
4. link — one LLM call per pile: which items are the same entity, and which
   existing node (if any) is the merge target

Two duplication axes, each handled where it's cheapest and most reliable:

• same name, different type (GraphRAG Concept vs Technology) → resolved for
  free by description similarity. An LLM told to be careful tends to over-split
  these; the embedding is cheaper and more accurate.
• different name, same entity (llama_index ≈ LlamaIndex, or a new mention
  of an existing node) → the LLM partitions a small pile, the framing that keeps
  genuine look-alikes apart (FALKORDB_USERNAME ≠ FALKORDB_PASSWORD).

Design choices

• Fail toward splitting — thin evidence or a failed call leaves entities separate.
• Existing nodes win the id — the store's MERGE (n {id}) attaches the new
  mention with no special-casing; existing edges untouched.
• Facts are never invented — the LLM owns free text (canonical name, type,
  merged description); provenance is unioned and immutable-property conflicts are
  flagged (_needs_review), never guessed.
• Testable without a database — the graph lookup is an injected
  candidate_retriever; in production it wraps the graph store's name/vector search.

Scope

Adds the new strategy and makes it the default resolver (see the Behavior change note at the top). Closes the same-name/different-type homograph gap in #277.

Tests

tests/test_incremental_resolution.py — 7 tests: cross-document linking into an
existing node while rejecting a look-alike, new-entity when no candidates (no LLM
call), free same-name collapse, genuine-homograph preservation, immutable-conflict
flagging, malformed-LLM fail-safe, and name normalization. Ruff clean; existing
resolution suite unaffected.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added an incremental, graph-aware entity resolution strategy that links new mentions to existing knowledge-graph entities and merges duplicates when appropriate.
  • Updated default ingestion/update duplicate resolution to use this strategy when LLM + embeddings are configured; otherwise it falls back to deterministic exact-match.
  • Exposed IncrementalResolution for direct importing.
• Bug Fixes
  • Fail-safe behavior: malformed or failed LLM partitioning won’t apply merges.
• Documentation
  • Updated ingestion/strategy docs and changelog to reflect the new defaults.
• Tests
  • Added comprehensive async and unit tests covering linking, homographs, conflicts, and robustness.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds IncrementalResolution, a graph-aware entity resolution strategy with staged merging, graph candidate retrieval, LLM linking, conflict tracking, conditional ingestion wiring, public exports, documentation, and behavioral tests.

Changes

Incremental entity resolution

Layer / File(s)	Summary
Resolution contracts and exports graphrag_sdk/src/graphrag_sdk/ingestion/resolution_strategies/incremental_resolution.py, graphrag_sdk/src/graphrag_sdk/ingestion/resolution_strategies/__init__.py, graphrag_sdk/src/graphrag_sdk/__init__.py	Defines IncrementalResolution, its configuration and supporting types, and exposes it through both package facades.
Resolution pipeline and merge semantics graphrag_sdk/src/graphrag_sdk/ingestion/resolution_strategies/incremental_resolution.py	Collapses same-name duplicates, retrieves graph candidates, forms connected piles, applies LLM decisions, remaps entities and relationships, unions provenance, and records immutable-property conflicts.
Conditional ingestion integration graphrag_sdk/src/graphrag_sdk/api/main.py	Selects IncrementalResolution when LLM and embedder instances are available, wires graph candidate retrieval, and retains ExactMatchResolution as the fallback.
Behavioral validation and published behavior graphrag_sdk/tests/test_incremental_resolution.py, docs/ingestion.md, docs/strategies.md, CHANGELOG.md	Tests resolution and failure-handling behavior while documenting the new defaults and strategy.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ExtractedEntities
  participant IncrementalResolution
  participant CandidateRetriever
  participant LLMInterface
  participant GraphData
  ExtractedEntities->>IncrementalResolution: resolve graph data
  IncrementalResolution->>CandidateRetriever: retrieve graph candidates
  CandidateRetriever-->>IncrementalResolution: return candidate nodes
  IncrementalResolution->>LLMInterface: classify candidate piles
  LLMInterface-->>IncrementalResolution: return linking decisions
  IncrementalResolution->>GraphData: apply merges and relationship remaps

Loading

Possibly related issues

• Issue 277: Adds description-similarity-based merging for same-name, different-type entities within IncrementalResolution.

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 61.90% 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding IncrementalResolution for graph-aware entity linking.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/incremental-resolution

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Copilot]

Pull request overview

Adds a new IncrementalResolution strategy that performs graph-aware, incremental entity linking: it collapses within-batch same-name duplicates, retrieves similar existing graph entities, forms “piles” of connected survivors, and uses an LLM once per pile to decide merges/links—enabling cross-document deduplication.

Changes:

• Introduces IncrementalResolution resolver with injected candidate_retriever + embedding-based within-batch collapse + LLM pile partitioning/linking.
• Exposes IncrementalResolution via graphrag_sdk.ingestion.resolution_strategies and top-level package exports.
• Adds a dedicated test suite covering core behaviors (linking, no-candidate fast path, free merges, homographs, conflicts, malformed LLM response, name normalization).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
graphrag_sdk/tests/test_incremental_resolution.py	Adds tests for cross-document linking and the incremental funnel stages.
graphrag_sdk/src/graphrag_sdk/ingestion/resolution_strategies/incremental_resolution.py	Implements the new incremental, graph-aware resolution strategy.
graphrag_sdk/src/graphrag_sdk/ingestion/resolution_strategies/init.py	Re-exports IncrementalResolution from the resolution strategies package.
graphrag_sdk/src/graphrag_sdk/init.py	Re-exports IncrementalResolution from the SDK’s main package interface.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 3

🤖 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
`@graphrag_sdk/src/graphrag_sdk/ingestion/resolution_strategies/incremental_resolution.py`:
- Around line 384-399: Widen the exception guard in _embed to include the
embedding result conversion and normalization setup, including np.array
construction, so malformed or ragged vectors are caught and return None through
the existing warning path. Preserve the current successful embedding behavior
and fail-toward-splitting semantics.
- Around line 321-345: Update _build_pile so survivor items are limited to
self.pile_cap before candidates are added, preserving pile order and survivor
indices. Ensure the returned items never exceed pile_cap, and leave truncated
survivors unlinked for this round; retain the existing candidate deduplication
and candidate-order behavior for remaining capacity.
- Around line 362-382: Update _absorb to preserve the loser’s existing
_CONFLICTS entries and propagate its _NEEDS_REVIEW flag before skipping those
metadata keys in the property loop. Merge conflict records with the survivor’s
conflicts without discarding prior entries, and set _NEEDS_REVIEW whenever
either node was previously flagged or a new immutable-property conflict is
detected.

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: e0150ac9-2783-43d0-88e9-b5ad1d8e159a

📥 Commits

Reviewing files that changed from the base of the PR and between 0ab92ba and 2b505f7.

📒 Files selected for processing (4)

• graphrag_sdk/src/graphrag_sdk/__init__.py
• graphrag_sdk/src/graphrag_sdk/ingestion/resolution_strategies/__init__.py
• graphrag_sdk/src/graphrag_sdk/ingestion/resolution_strategies/incremental_resolution.py
• graphrag_sdk/tests/test_incremental_resolution.py

[Naseem77]

Findings:

1.  [medium] incremental_resolution.py:_build_pile  — pile_cap only caps candidates. 15 survivors with cap=12 → 15-item prompt and every graph candidate evicted, so the LLM call can never link. (Proven by failing gap test.)
2.  [medium] _absorb + graph_store._clean_properties  —  _merge_conflicts  is a list of dicts; the store filters it to empty and drops it.  _needs_review=True  persists but the conflict details never reach the graph. (Proven by failing gap test.)
3.  [medium] api/main.py:_graph_candidate_retriever  — exact  toLower(name)  equality only; the PR's headline  llama_index  ≈  LlamaIndex  case is unreachable with defaults. Verified live on FalkorDB: both variant lookups return 0 rows. No vector search despite the docstring.
4.  [low]  — same-name nodes with no descriptions auto cross-type merge (description falls back to name → cosine 1.0), contradicting fail-toward-splitting.
5.  [low]  — PR body claims "no shared code modified" but the default resolver changed → new LLM/embedding cost per ingest for existing users. Docs/CHANGELOG do disclose it; PR description should too.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

[coderabbitai]

Actionable comments posted: 1

🤖 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
`@graphrag_sdk/src/graphrag_sdk/ingestion/resolution_strategies/incremental_resolution.py`:
- Around line 197-201: The chunked linking flow around _link_pile can produce
multiple surviving carriers with the same final_id, causing duplicate upserts
and overwritten metadata. Deduplicate surviving_nodes by resolved final_id
before resolve() returns, retaining one carrier per target and preserving the
intended provenance; update test_large_pile_still_links_candidates with an
assertion that res.nodes contains unique final ids.

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: cacf7c60-02d3-4f09-b014-24a6d8b0efc6

📥 Commits

Reviewing files that changed from the base of the PR and between 0b6a8bc and 37cb92d.

📒 Files selected for processing (5)

• docs/ingestion.md
• docs/strategies.md
• graphrag_sdk/src/graphrag_sdk/api/main.py
• graphrag_sdk/src/graphrag_sdk/ingestion/resolution_strategies/incremental_resolution.py
• graphrag_sdk/tests/test_incremental_resolution.py

🚧 Files skipped from review as they are similar to previous changes (3)

• docs/strategies.md
• docs/ingestion.md
• graphrag_sdk/src/graphrag_sdk/api/main.py

[galshubeli]

Thanks @Naseem77 — every finding was correct, and testing against a real FalkorDB (as you did) was exactly what my unit tests missed. All five addressed in 37cb92d, a12ff0b (+ the earlier bbb5667):

1. pile_cap only caps candidates — resolve() now splits an oversized pile into chunks (survivor_budget = pile_cap - top_k) so survivors can't evict every candidate. test_large_pile_still_links_candidates.

2. _merge_conflicts dropped by the store — this was the key one. Changed it to a list of strings ("field: a vs b") since _clean_properties drops lists of dicts, and _absorb now propagates a loser's prior conflicts + _needs_review. Live round-trip verified: the string form reads back from FalkorDB as ['founded: 2001 vs 1998']; the old dict form reads back null.

3. Default retriever exact-match only — the retriever now folds separators and tries the concatenated form. Live-verified on FalkorDB: graphrag_sdk→GraphRAG-SDK ✓, Cypher→both type variants ✓, and llama_index→LlamaIndex does match (via separator-removal). It's still name-based/best-effort (not semantic, and asymmetric), so I've corrected the docstring + docs to describe it honestly rather than claim vector search. Full semantic/variant linking needs a custom candidate_retriever or a post-finalize() pass (entity embeddings don't exist mid-ingest).

4. No-description cross-type auto-merge — _same_name_merges now requires real descriptions on all members; without them it won't cross types on name-identity alone. test_no_description_homograph_not_merged.

5. Stale PR body — updated: removed the "no shared code modified" claim and added a Behavior-change note (default resolver change + per-ingest cost + opt-out).

A CodeRabbit re-review then caught a bug my pile-chunking fix introduced (two carriers linked to the same graph id → duplicate upsert); fixed in a12ff0b by deduping same-id survivors. Full unit suite green (1008 passed) + the live validation script. Ready for another look when you have a moment.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

[Naseem77]

Summary: Adds a well-designed, well-tested graph-aware resolution strategy (596 lines) and flips it to the default resolver for LLM+embedder setups. Core algorithm is solid with genuine fail-safe behavior; the main risks are the retriever's per-entity graph scan, an LLM output shape that isn't validated (overlapping groups), and description overwrite on link.

Findings:

1.  [medium] graphrag_sdk/src/graphrag_sdk/api/main.py:1766  Candidate retriever is a full  Entity  scan per resolved entity
   • Problem: The Cypher wraps  n.name  in  toLower()  /  replace() , so no index can serve it. The docstring's claim that "a range index on  Entity(name)  keeps it cheap" is incorrect: functions applied to the property defeat range indexes.
   • Impact: O(graph entities × survivors) per document. On large graphs (the stored perf memory shows FalkorDB writes at 13K+ nodes/s, so graphs get big fast) ingest latency will grow linearly with graph size, per entity, per document.
   • Fix: Store a precomputed  name_normalized  property at write time and query it with an indexed equality, or use a CALL db.idx fulltext lookup. At minimum correct the docstring.
2.  [medium] incremental_resolution.py:_link_pile  LLM groups are not validated as disjoint
   • Problem:  _parse_decisions  accepts groups where the same ref appears in multiple groups. A survivor absorbed by group A can be the carrier of group B:  _absorb  runs twice (duplicated provenance/conflicts), and  remap[carrier.id]  is silently overwritten last-wins, so group A's members transitively follow group B's target after  _flatten .
   • Impact: A sloppy but parseable LLM response can merge unrelated entities across groups, which contradicts the fail-toward-splitting stance. The malformed-response test covers unparseable JSON, not this overlapping-refs case.
   • Fix: Track consumed refs across a pile's decisions and drop (or skip the whole verdict for) any group reusing a ref. Add a test.
3.  [medium] incremental_resolution.py:_ask / _apply_canonical  Linking replaces the existing node's description with a summary of 180-char truncated inputs
   • Problem: The prompt shows each description truncated to 180 chars, the LLM writes a merged description from those snippets, and  _apply_canonical  + the store's  SET n += props  overwrite the existing graph node's full  description  (and  name ) with it.
   • Impact: Repeated ingests progressively erode rich descriptions accumulated on hub entities; information loss is silent and compounding.
   • Fix: Send fuller descriptions (budgeted by  max_summary_tokens ), or merge the LLM summary with the stored description instead of replacing, or leave the existing node's description unless the LLM saw it untruncated.
4.  [medium] api/main.py:_default_resolver  Default behavior change adds LLM + embedding + graph-query cost to every ingest
   • Problem: Any user with LLM+embedder configured (the common setup) silently gets extra per-pile LLM calls, per-name-group embedding calls, and per-entity graph queries on upgrade.
   • Impact: Ingest latency and cost regression; extraction is already the bottleneck (~15s/chunk per hsbc_perf). Nondeterministic merges may also surprise pipelines that relied on ExactMatch determinism.
   • Fix: It is documented (CHANGELOG, docs, PR body), so this is a deliberate product call, not a blocker. Consider a minor-version bump signal and a one-line log at ingest start naming the active resolver (the  ctx.log  in resolve() covers this partially).
5.  [low] incremental_resolution.py:_same_name_merges  Free cross-type merge trusts a fixed cosine bar
   • Problem:  same_name_threshold=0.80  on min pairwise description cosine auto-merges without LLM review. With some embedders, two same-name but distinct entities with short, domain-adjacent descriptions can clear 0.80.
   • Impact: Wrong merge (the unrecoverable direction) via the one path that bypasses the LLM. Risk is acknowledged in the docstring and gated on all members having real descriptions, so severity is low.
   • Fix: None required; consider making the threshold embedder-aware or routing borderline (0.75-0.85) groups to the LLM pile instead.
6.  [low] incremental_resolution.py:_fetch_candidates  Sequential awaits per survivor
   • Problem: Candidates are fetched one survivor at a time.
   • Impact: Adds N round-trips of latency per document on top of finding 1.
   • Fix:  asyncio.gather  with per-task exception handling (the existing per-survivor degradation maps cleanly onto  return_exceptions=True ).
7.  [low] api/main.py:_graph_candidate_retriever  Separator folding misses 3+ consecutive separators and the reverse direction
   • Problem:  replace(' ', ' ')  applied once doesn't fully collapse runs of separators, and a  LlamaIndex  mention won't find a stored  llama_index .
   • Impact: Missed links; best-effort behavior is explicitly documented in the docstring, so informational only.
   • Fix: Optional; a stored normalized-name property (finding 1) fixes both at once.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

graphrag_sdk/src/graphrag_sdk/api/main.py:1782

• The default graph candidate query tries to normalize stored names by collapsing double spaces (replace(..., ' ', ' ')). This does not fully normalize runs of 3+ separators (e.g. 'a___b' → 'a b' → 'a b'), so lookups can miss valid matches even though the Python-side normalization collapses arbitrary runs. Comparing on a fully separator-stripped form avoids this edge case and keeps the matcher consistent for arbitrary separator runs.

            folded = re.sub(r"[\s\-_]+", " ", base).strip()
            variants = list({base, folded, folded.replace(" ", "")})
            result = await store.query_raw(
                "MATCH (n:__Entity__) "
                "WHERE toLower(n.name) IN $variants "
                "OR replace(replace(replace(toLower(n.name), '-', ' '), '_', ' '), '  ', ' ') "
                "= $folded "
                "RETURN n.id, labels(n), n.name, n.description LIMIT $k",
                {"variants": variants, "folded": folded, "k": k},
            )

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

[galshubeli]

@Naseem77 all findings addressed. #3 landed earlier (4cce21f); the rest in 0d2b7f1:

#1 / #7 — retriever scan → indexed name_key. The store now writes a separator-free name_key (graphrag_sdk/GraphRAG-SDK/GraphRAG SDK → graphragsdk) on every entity and ensures a range index; the default retriever does an indexed equality lookup instead of a toLower(name) scan, so it no longer grows with graph size. It's also symmetric now, so llama_index ↔ LlamaIndex both match (that's #7). Corrected the false "range index on __Entity__(name) keeps it cheap" docstring. Verified live on FalkorDB: name_key stored, range index present, symmetric matches.

#2 — disjoint LLM groups. _link_pile tracks placed refs and drops any group that reuses one, so a sloppy-but-parseable response can't double-absorb a node or last-wins-merge unrelated entities. Test added.

#5 — cross-type free merge. Now requires the higher cross_type_threshold (0.90); same-type still merges at same_name_threshold (0.80). The only unrecoverable free path now demands stronger evidence. Test added.

#6 — sequential fetch. Candidate retrieval runs concurrently via asyncio.gather (order preserved, per-item failure → no candidates).

#4 — cost signal. The facade now logs the active resolver and its per-ingest cost on each ingest; the default change is in the CHANGELOG/docs/PR body.

Full unit suite green (1011), integration green. Live validation script covers #1/#7 end-to-end. Ready for another look.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

graphrag_sdk/src/graphrag_sdk/storage/graph_store.py:355

• ensure_name_key_index() treats any exception containing the substring "already" as success. This is overly broad and can incorrectly report success on unrelated failures (and it diverges from the existing index-idempotency markers used in VectorStore, which check specifically for "already indexed" / "already exists").

        except Exception as exc:
            if "already" in str(exc).lower():
                return True  # already indexed — the desired end state
            logger.warning("Could not create name_key index: %s", exc)
            return False