Fix chunk-tracking reseed: distinguish absent row from present-but-empty row (#3609) by jaideeppyne · Pull Request #3660 · HKUDS/LightRAG

jaideeppyne · 2026-08-17T14:20:32Z

What

aedit_entity and aedit_relation decided whether a chunk-tracking row existed using the truthiness of its chunk_ids list:

has_stored_data = stored_data and isinstance(stored_data, dict) and stored_data.get("chunk_ids")

That collapses two states that mean different things — row absent (never migrated / unknown) and row present but empty ({"chunk_ids": [], "count": 0} — authoritative: this object tracks no chunks) — into the same branch, which then reseeds existing_full_chunk_ids from the graph node's source_id. Since source_id may still name chunks a previous purge pruned, an ordinary edit can write stale attribution back into a curated empty row.

This is the call-site version of the same class of bug that #3591 hardened inside compute_incremental_chunk_ids, and it mirrors the "test row presence, not list truthiness" rule AGENTS.md already states for recovery anchors (the defect behind #3400).

Change

Test row presence — a dict carrying a chunk_ids list — instead of list truthiness, and reseed from source_id only when no usable row exists at all. Legacy/partial shapes ({}, {"count": 0}) and malformed ones ({"chunk_ids": null}) count as absent/unknown. The predicate lives in one shared helper, utils.has_chunk_tracking_row() (with the authority-model docstring), adopted at every call site: the entity and relation edit blocks, both rename migrations, the entity-merge block, and the ingestion merges.
Entity-rename relation-key migration (utils_graph.py, the rename branch): a present-but-empty relation row is now migrated to the new key instead of being dropped. Previously the if relation_chunk_ids: guard deleted the row under the old key and skipped the upsert for an empty row, making it absent — which re-armed the exact reseed this PR fixes, on the next relation edit. The upsert is now skipped only when the row was genuinely absent and source_id yielded nothing.
Ingestion merge paths (operate.py: _merge_nodes_then_upsert, _merge_edges_then_upsert, and the edge-endpoint node update inside it): fall back to the graph source_id only when no tracking row exists, never when the row is present but empty. Before this, an ordinary document insert could resurrect purged chunk ids into the authoritative store — the most reachable variant of the defect.
_merge_entities_impl step 9 (originally listed as out of scope below; folded in after review): row presence is decided per input entity — a usable row is authoritative as-is, including an empty list; an input with an absent/malformed row falls back to its own pre-merge graph source_id (captured before step 5 overwrites the target node with the merged payload, whose source_id unions every input); the ordered union is written only when at least one usable row exists; with none present the target row stays absent. One entity's curated-empty row proves only that that entity owns nothing, so "partially known" is never promoted to "all empty" — and merging all-curated-empty rows now leaves the target a present empty row instead of an absent one.
Crash-safe key migrations: every tracking-row migration now upserts the new key before deleting the old one — the entity and relation rows on rename, the target row before the source-row deletes on merge, and the merge's relation-row migration (which deletes only keys outside the new key set, since a relation already attached to an existing target keeps its key). On RPC-backed KV storages each call commits independently, so the old delete-first order could permanently lose a curated row on a crash between the two calls — turning it absent and re-arming the reseed. An orphaned old-key row from the reverse failure is only dead bookkeeping.

Reachability

To be precise about scope (per the issue, filed "for verification rather than as a confirmed bug"):

The edit-only paths (aedit_entity / aedit_relation with source_id in updated_data) are Python-API / partial-write reachable — the HTTP layer does not expose source_id. The regression test for them reproduces the reseed from directly seeded state; it protects an invariant, it is not a confirmed user-visible incident.
The entity-rename → relation-edit chain is reachable through the HTTP API: rename is exposed, and (before this PR) it silently dropped a curated empty relation row, so a subsequent relation edit reseeded stale attribution into the authoritative store. The rename test drives exactly that sequence and fails without the migration fix above.
The entity-merge chain (amerge_entities) and the document-insert ingestion merges are likewise reachable (HTTP merge endpoint / ordinary inserts); both are covered by the review follow-up commits (ecffba1, f86ef93).

Deliberate behavior note

When a present-but-empty row coexists with a source_id change, genuine additions (new - old) are now appended to the empty baseline rather than the row being reseeded from source_id first. This is intentional and matches the authority model (a curated empty row is the truth; only real new chunks are added).

Out of scope (noted, not changed here)

_purge_kg_contributions (lightrag.py, entity + relation classification) does the same absent-vs-empty conflation, but applying the presence check there is a deletion-behavior change (a present-empty row would stop falling back to the graph source_id and instead route the object into entities_to_delete). That needs its own issue + tests, so both sites are annotated with a NOTE(#3609) comment rather than changed.

(An earlier revision also listed the _merge_entities_impl entity block here; that item has since been implemented — see the Change section above.)

Testing

Initial submission: ./scripts/test.sh tests/pipeline tests/utils → 827 passed, plus 2 failures that are pre-existing and unrelated to this change (test_chunk_options_regex_scrub::test_audit_line_bounds_the_untrusted_pattern and test_strict_capability_gate::test_gaps_are_warned_... — both a ValueError: too many values to unpack from a version skew in my local env; they reproduce identically with this PR's changes reverted).

Regression tests in tests/pipeline/test_doc_status_chunk_preservation.py:

test_edit_does_not_reseed_present_but_empty_chunk_tracking — present-but-empty entity row is not reseeded from a stale source_id (fails on main).
test_rename_migrates_present_but_empty_relation_tracking_row — a curated empty relation row survives the key migration on rename (fails without the migration fix; verified by reverting only utils_graph.py).
test_edit_seeds_chunk_tracking_from_source_id_when_row_absent — absent-row pin: an absent row is still seeded from source_id on edit (pins the direction the guard was narrowed).
test_edit_treats_row_without_chunk_ids_key_as_absent — a {} row (present, no chunk_ids key) is treated as absent/unknown.

Later review rounds added further cases to that file (legacy {"count": 0} shapes, malformed {"chunk_ids": null} rows), plus four new test files (commits ecffba1, f86ef93):

tests/utils/test_has_chunk_tracking_row.py — predicate truth table.
tests/extraction/test_chunk_tracking_presence_ingestion.py — the three operate.py ingestion sites, with absent-row direction pins.
tests/extraction/test_merge_entities_chunk_tracking.py — per-input-entity presence (mixed empty/absent/malformed states, existing-target merge) and upsert-before-delete ordering for entity and relation rows.
tests/extraction/test_rename_tracking_migration_ordering.py — rename migration ordering for the entity row and migrated relation rows.

Each behavioral test fails on the code as it stood before its commit; the absent-row pins pass on both sides, confirming the guards were narrowed, not widened. Follow-up verification: tests/extraction + tests/utils + tests/api/routes + tests/pipeline → 1584 passed (remaining failures are pre-existing sandbox-environment issues, identical on the unmodified head). ruff check / ruff format clean.

/cc @Solaris-star — #3609 was assigned to you with @danielaskdd's go-ahead; happy to coordinate on the issue thread so we don't duplicate the remaining follow-up (the purge item above).

Description updated by Claude Code at the maintainer's request, to fold in the review follow-up commits (ecffba1, f86ef93).

…pty row (HKUDS#3609) aedit_entity/aedit_relation decided whether a chunk-tracking row existed using the truthiness of its chunk_ids list, collapsing 'row absent' (never migrated) and 'row present but empty' (authoritative: tracks no chunks) into one branch that then reseeds existing_full_chunk_ids from the graph node's source_id. Since source_id may still name chunks a previous purge pruned, an edit could write stale attribution back into a curated empty row. Test row presence (isinstance(dict) and 'chunk_ids' in row) instead of list truthiness, and reseed from source_id only when no row exists. Applied to both the entity and relation blocks. Adds a regression test that persists {'chunk_ids': [], 'count': 0} next to a stale graph source_id and asserts a description-only edit does not repopulate it.

danielaskdd · 2026-08-18T04:12:08Z

Thanks for picking this up. I reviewed the patch and verified it empirically against main (base d69210584). The change is correct as far as it goes, but it is incomplete in a way that blocks merge: the relation-key migration during an entity rename can still turn an authoritative empty tracking row into an absent row, allowing a later edit to reseed stale attribution.

What I verified as correct

The fix-proof is behavioral, not symbolic. Reverting only lightrag/utils_graph.py to main on your branch makes the new test fail with the real symptom — Entity Edit: find 1 chunks related to 'StaleEntity', row persisted as {"chunk_ids": ["chunk-already-purged"], "count": 1} — and it passes with the patch. That is the right kind of regression test.
No regressions: ./scripts/test.sh tests/pipeline tests/utils → 826 passed on your branch.
Absent-row semantics are unchanged (None → isinstance False → still seeds from source_id), so the migration fallback is preserved.
Treating {} (row present, no chunk_ids key) as absent/unknown is the semantics I asked for.
The entity block's rename branch is genuinely fixed by this patch: with a present-but-empty row and old == new, compute_incremental_chunk_ids([], old, new) returns [], so the curated empty row survives the rename.
The behavior change when a present-but-empty row coexists with a source_id change — genuine additions (new - old) appended to the empty baseline instead of reseeding first — is deliberate and matches the authority model. Worth a sentence in the PR description so the reader knows it was considered.

Blocking: the third call site is untouched, and it re-arms the reseed you just fixed

This was point 2 of my comment on #3609 and it needs to be folded into this PR. lightrag/utils_graph.py:683-716 — the relation-key migration inside the entity-rename branch:

await relation_chunks_storage.delete([old_storage_key])

if relation_chunk_ids:          # <-- line 710: a curated empty row is silently not migrated
    await relation_chunks_storage.upsert({new_storage_key: {...}})

The presence test above it (if old_stored_data and isinstance(old_stored_data, dict)) is fine. The if relation_chunk_ids: guard on the upsert is not: a row of {"chunk_ids": [], "count": 0} is deleted under the old key and never written under the new one, so it becomes absent — which is exactly the state your patch now (correctly) treats as "seed from source_id".

I probed this on your branch, not on main, and the full chain still reproduces:

Seed EntA/EntB with a curated empty relation tracking row and an edge whose source_id still names a purged chunk (chunk-stale).
await rag.aedit_entity("EntA", {"entity_name": "EntZ"}, allow_rename=True) → the empty row is gone under both keys: OLD ROW: None, NEW ROW: None.
await rag.aedit_relation("EntZ", "EntB", {"description": "updated"}) → not has_stored_row → reseed → {'chunk_ids': ['chunk-stale'], 'count': 1}.

Stale attribution resurrected into the authoritative store — the same outcome the PR is titled after.

This migration path is independently reachable through the HTTP API's entity-rename endpoint. The two sites you fixed can also receive source_id through HTTP: the graph edit endpoints accept it in updated_data for both entities and relations. The direct regression test proves the affected behavior for a seeded persisted state; it does not, by itself, establish that every such precondition is produced by a normal end-to-end API sequence. As it stands, this PR fixes two manifestations but leaves this third re-arming path live.

Also needs addressing

Tests — one of the three requested cases is present. Missing:
- the absent-row pin (get_by_id must return None, not {}), asserting the migration fallback still seeds from source_id. I checked: no existing test covers that branch for the edit paths, so your narrowed guard is currently unpinned in the direction it was narrowed.
- the rename case (step 2 above — the curated empty relation row must survive the key migration).
- the {} case, pinning "row exists, no chunk_ids key" as absent/unknown.
PR description — reachability wording. The heading "Reachability — confirmed" overstates what the current test shows. It reproduces the reseed from directly seeded state; it does not establish that the precondition arises from a normal API sequence. Please state the boundary explicitly, as agreed on the issue: this protects an invariant, it is not a confirmed user-visible incident. After the rename gap is fixed, the rename → relation-edit sequence is API-executable once that state exists; an end-to-end test is still needed before claiming ordinary API flows establish the precondition.
Point 3 of the issue comment — a one-line note, no code change. _merge_entities_impl's entity block (utils_graph.py:1974-1978) still uses truthiness plus if chunk_ids: and has no source_id fallback at all, unlike its relation counterpart at :1723. Please don't change its fallback semantics here; just note the inconsistency in the PR description.
Point 4 — leave _purge_kg_contributions alone. lightrag/lightrag.py:5330-5334 and :5395-5400 do the same conflation. Applying the presence check there is a deletion-behavior change (an empty row would stop falling back and route the object into entities_to_delete), so it needs its own issue and tests. State in the PR description that this behavior is deliberately out of scope.
Please report the suite I asked for. ./scripts/test.sh tests/pipeline tests/utils with pass counts, not just the single file.

Finally, per the second acceptance item on the issue: if you conclude any remaining truthiness check is safe as-is, annotate it with why, so the next reader does not have to redo this analysis.

One process note

#3609 was claimed by @Solaris-star and I gave an explicit go-ahead there before this PR was opened. That is not a reason to close this one — but please coordinate on the issue thread so the two of you are not duplicating the remaining work.

Generated by Claude Code

…HKUDS#3609) Addresses the blocking review on HKUDS#3660: the entity-rename relation-key migration in utils_graph.py still re-armed the reseed the edit paths now guard against. - utils_graph.py: when migrating a relation chunk-tracking row to its new key during rename, migrate a present-but-empty row ({"chunk_ids": [], "count": 0}) instead of dropping it. The old `if relation_chunk_ids:` guard deleted the row under the old key and skipped the upsert for an empty row, making it absent — so the next relation edit reseeded it from a possibly-stale source_id. Now the write is skipped only when the row was genuinely absent AND source_id yields nothing. This is the one path reachable via the HTTP API (entity rename). - Tests (tests/pipeline/test_doc_status_chunk_preservation.py): * rename migrates a curated empty relation row (fails without the fix above) * absent-row pin: an absent tracking row is still seeded from source_id on edit * a row of {} (present, no chunk_ids key) is treated as absent/unknown - lightrag.py: annotate the two _purge_kg_contributions sites (entity + relation) that intentionally keep truthiness semantics, pointing at HKUDS#3609 — applying the presence check there is a deletion-behavior change that needs its own issue.

jaideeppyne · 2026-08-18T04:57:18Z

Thanks for the thorough review and for verifying it empirically — the behavioral fix-proof is exactly the bar I wanted to hit. Pushed f8dd3b0 addressing everything.

Blocking: third call site (relation-key migration on rename). Folded in. In utils_graph.py, the rename branch now migrates a present-but-empty relation row to the new key instead of dropping it. The old if relation_chunk_ids: guard is replaced by if old_row_present or relation_chunk_ids:, so the upsert is skipped only when the row was genuinely absent and source_id yielded nothing. I kept your pre-approved presence test (bool(old_stored_data) and isinstance(...)) as the branch selector and derive old_row_present from it. Verified with your reproduction: rename → aedit_relation no longer resurrects chunk-stale.

Tests. Added the three you asked for, all in test_doc_status_chunk_preservation.py:

test_rename_migrates_present_but_empty_relation_tracking_row — the rename case. I confirmed it fails with only utils_graph.py reverted (curated empty row → NEW ROW: None) and passes with the fix.
test_edit_seeds_chunk_tracking_from_source_id_when_row_absent — the absent-row pin (get_by_id → None, edit must seed from source_id).
test_edit_treats_row_without_chunk_ids_key_as_absent — the {} case (present, no chunk_ids key → absent/unknown).

PR description.

Reworded the reachability section per your note: the edit-only paths are partial-write/Python-API reachable and the test protects an invariant (not a confirmed incident); the rename → edit chain is HTTP-reachable and is now described as such.
Added the deliberate-behavior sentence (present-empty + source_id change → additions appended to the empty baseline, not reseeded first).
Point 3: noted the _merge_entities_impl entity-block inconsistency (truthiness + if chunk_ids:, no source_id fallback, unlike the relation counterpart) in an "out of scope" section — not changed.

Point 4 (_purge_kg_contributions). Left the behavior alone; annotated both sites (entity NOTE(#3609) and relation NOTE(#3609)) explaining that applying the presence check there is a deletion-behavior change that needs its own issue + tests.

Remaining truthiness checks. The one truthiness check I kept in the migration is annotated with why it's safe (a curated empty row is a non-empty dict → truthy → treated as present; {}/None → absent → reseed, which is the intended semantics).

Suite. ./scripts/test.sh tests/pipeline tests/utils → 827 passed. Two failures show in my local env (test_audit_line_bounds_the_untrusted_pattern, test_gaps_are_warned_...) but they're pre-existing and unrelated — a ValueError: too many values to unpack from a version skew locally; they reproduce identically with this PR's changes reverted.

Coordination. /cc @Solaris-star in the PR body and I'll drop a note on #3609 so we don't duplicate the remaining merge/purge follow-ups — didn't mean to step on the assignment, happy to hand off or split.

danielaskdd · 2026-08-18T06:20:38Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f8dd3b07d4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@codex

…iness Per @codex on HKUDS#3660: the rename relation-key migration classified the old row with `bool(old_stored_data) and isinstance(old_stored_data, dict)`, so a legacy/partial shape like `{"count": 0}` (no `chunk_ids` key) was treated as *present* and persisted as an authoritative empty row at the new key — the next edit would then preserve that empty list and the relation would lose its live chunk attribution. Use the same presence check as the entity/relation edit paths (`isinstance(row, dict) and "chunk_ids" in row`), so only a curated `{"chunk_ids": [], "count": 0}` counts as present (and is migrated as-is), while `{}` / `{"count": 0}` / absent rows are reseeded from the edge's `source_id`. Adds a regression test with a `{"count": 0}` relation row and an edge whose `source_id` names a live chunk: after rename the new-key row must be reseeded from `source_id` (not persisted empty). It fails on the previous truthiness check and passes now.

jaideeppyne · 2026-08-18T06:35:52Z

@danielaskdd @codex good catch — fixed in df4833b.

The rename migration now classifies the old relation row by the same presence check the entity/relation edit paths use — isinstance(row, dict) and "chunk_ids" in row — instead of truthiness. So:

{"chunk_ids": [], "count": 0} (curated empty) → present → migrated as-is;
{} / {"count": 0} / absent → unknown → reseeded from the edge's source_id.

Added a regression test (test_rename_reseeds_relation_row_without_chunk_ids_key) that seeds a {"count": 0} relation row with an edge whose source_id names a live chunk and asserts the new-key row is reseeded (not persisted empty). It fails on the old truthiness check and passes now. All four existing chunk-tracking tests still pass.

chatgpt-codex-connector · 2026-08-18T06:36:09Z

To use Codex here, create an environment for this repo.

danielaskdd · 2026-08-18T06:39:32Z

@codex review

chatgpt-codex-connector · 2026-08-18T06:41:39Z

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: df4833b3bf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

danielaskdd · 2026-08-18T07:17:50Z

Thanks for the update. I re-reviewed the current head df4833b3bfe00d5b72129d78d88ca0785d532f8a; two items remain:

Blocking — validate the chunk_ids value type, not only key presence.

The new predicate at utils_graph.py:626 (and the equivalent predicates at :704 and :1181) treats {"chunk_ids": null} as a present/authoritative row. The subsequent list comprehensions at :642, :712, and :1204 then iterate None and raise TypeError.

This is a regression from the old truthiness behavior, which classified this malformed/legacy shape as unknown and reseeded from source_id. It is reachable whenever the relevant update enters the tracking branch: entity/relation source_id edits, and relation-key migration during an entity rename. At that point graph and VDB mutations have already occurred, so the route returns a 500 after partial mutation.

Please use the same schema-level presence notion at all three sites, e.g. isinstance(row, dict) and isinstance(row.get("chunk_ids"), list), so null (and other malformed non-list values) follows the unknown/reseed path. Please add regression coverage for {"chunk_ids": null} as well.
Non-blocking, but please consider folding this in — relation tracking in _merge_entities_impl has the same partial-row classification bug.

At utils_graph.py:1741-1746, {"count": 0} is treated as an existing row, so chunk_ids becomes [] rather than falling back to the edge's live source_id. The merge then deletes the old key and persists {"chunk_ids": [], "count": 0} at the new key (:1795-1808). Subsequent ordinary relation edits treat that row as authoritative and will not recover the existing attribution from source_id; repair then requires the integrity-audit path.

This is pre-existing/outside this PR's changed lines, so it should not block the focused fix above. However, it is the same contract that commit 3 corrected for rename migration, and is reachable through POST /graph/entities/merge (also through the edit endpoint's auto-merge path). A one-line alignment with the new predicate plus a focused merge regression test would make this PR materially more complete. If you prefer not to expand scope, please add this relation merge branch to the out-of-scope note and open a follow-up.

@danielaskdd

…n branch Per @danielaskdd's re-review of HKUDS#3660: 1. (blocking) The presence predicates classified `{"chunk_ids": null}` as a present/authoritative row, then iterated `None` -> `TypeError` (a 500 after partial mutation) whenever the tracking branch was entered (source_id edits, rename migration). Test the *schema* at all three sites — `isinstance(row, dict) and isinstance(row.get("chunk_ids"), list)` — so null and other malformed non-list shapes follow the unknown/reseed path. 2. Fold in the same fix for the relation branch of `_merge_entities_impl`: a partial row like `{"count": 0}` is now reseeded from the edge's live `source_id` on merge instead of being persisted as an authoritative empty row at the merged key. Adds regression tests: renaming an entity with a `{"chunk_ids": null}` row (crashed before, now reseeds) and merging a relation whose row is `{"count": 0}` (now reseeds from source_id). Full chunk-preservation suite: 34 passed.

jaideeppyne · 2026-08-18T10:45:00Z

Thanks for the careful re-review — both addressed in d11ba1b.

1 (blocking) — validate the value, not just the key. All three predicates (entity edit :626, relation migration :704, relation edit :1181) now use isinstance(row, dict) and isinstance(row.get("chunk_ids"), list), so {"chunk_ids": null} (and any malformed non-list shape) follows the unknown/reseed path instead of iterating None. Added test_edit_treats_null_chunk_ids_as_absent: renaming an entity with a {"chunk_ids": null} row raised on the old key-presence check and now reseeds from source_id.

2 — folded in. Aligned the _merge_entities_impl relation branch (:1741) to the same schema-level predicate, so a partial row like {"count": 0} is reseeded from the edge's live source_id on merge rather than persisted as an authoritative empty row at the merged key. Added test_merge_reseeds_relation_row_without_chunk_ids (fails without the change; reachable via POST /graph/entities/merge).

Full test_doc_status_chunk_preservation.py suite: 34 passed.

danielaskdd · 2026-08-18T13:18:45Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d11ba1b900

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Treat malformed relation tracking rows with non-list chunk_ids as absent so source_id edits reseed them instead of failing after graph and vector mutations. Add a regression test for null chunk_ids on relation edits. Refs HKUDS#3609

danielaskdd · 2026-08-20T09:39:39Z

@codex review

chatgpt-codex-connector · 2026-08-20T09:41:46Z

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 1e92abba72

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…n/merge, crash-safe key migrations Follow-up to the HKUDS#3609 fix (PR HKUDS#3660), closing the remaining paths where a present-but-empty chunk-tracking row could be destroyed or reseeded from a possibly-stale graph source_id: - Extract the row-presence predicate into utils.has_chunk_tracking_row() and adopt it at every call site (entity/relation edit, rename migration, relation-key migration on rename, entity merge, ingestion merges), so the absent-vs-empty distinction is defined once. - operate.py ingestion merges (_merge_nodes_then_upsert, _merge_edges_then_upsert, and the edge-endpoint node update): fall back to the graph source_id only when NO tracking row exists, never when the row is present but empty. Previously an ordinary document insert could resurrect purged chunk ids into the authoritative store — the most reachable variant of the original defect. - _merge_entities_impl step 9: decide row presence by schema instead of list truthiness, and upsert the target row whenever any source had one. Merging entities whose rows were all curated-empty used to leave the target with an ABSENT row, re-arming the reseed on the next edit via the HTTP-reachable merge endpoint. Also upsert the target row before deleting the source rows. - Rename key migrations (_edit_entity_impl, entity row and relation rows): upsert the new key BEFORE deleting the old one. On RPC-backed KV storages each call commits independently, so the old order opened a crash window in which the row existed under neither key — turning a curated row absent. An orphaned old-key row is dead bookkeeping; a lost row re-arms the stale reseed. New regression tests (all fail without the fixes): - tests/utils/test_has_chunk_tracking_row.py — predicate truth table - tests/extraction/test_chunk_tracking_presence_ingestion.py — the three operate.py sites, with absent-row direction pins - tests/extraction/test_merge_entities_chunk_tracking.py — curated-empty preservation through merge + upsert-before-delete ordering - tests/extraction/test_rename_tracking_migration_ordering.py — rename migration ordering for entity and relation rows Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z1LA6qjSSkjXmyzxdSnTx

… crash-safe relation-row migration Review follow-up on the previous commit's _merge_entities_impl step 9 fix, which had a mixed-state defect: it collected chunk ids only from inputs that HAD a tracking row, then wrote the target row whenever ANY row was present. With input A holding an authoritative empty row and input B holding no row at all, the merge wrote an authoritative empty target row — promoting "partially known" to "all empty" and discarding B's attribution with no recovery path (the empty row is authoritative, so later edits never reseed it from the merged node's source_id). Step 9 now decides presence PER INPUT ENTITY: - a usable row (has_chunk_tracking_row) is used as-is, including an authoritative empty list; - an absent/malformed row falls back to THAT entity's own pre-merge graph source_id (captured in steps 1-2, before step 5 overwrites the target node with the merged payload — the merged source_id unions every input's chunks and must not be attributed to a single input); - the ordered union is written only when at least one usable row exists (target upsert still precedes source-row deletes); - with no row present at all the target row stays absent (unknown stays unknown — a fabricated row would promote UNKNOWN to authoritative). Also closes the adjacent crash window in the same function's relation-row migration: it deleted every old relation key BEFORE upserting the migrated rows, so a crash between the two calls lost them all on RPC-backed KV storages. The upsert now commits first, and only keys outside the new key set are deleted — a key can be both old and new (relations already attached to an existing target keep their key), and deleting it after the upsert would drop the just-written union. New regression tests (all fail on the previous implementation; the three prior guarantees — curated-empty preservation, unknown-stays-unknown, target-before-source ordering — still pass on both): - mixed authoritative-empty + absent row: target gets only the absent input's graph chunks; - mixed valid + malformed row: target gets tracking union graph fallback; - existing target without a row: its pre-merge source_id contributes; - relation rows: upsert precedes delete, and a shared old/new key is never deleted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z1LA6qjSSkjXmyzxdSnTx

danielaskdd · 2026-08-20T13:17:57Z

Summary of the two review follow-up commits @danielaskdd merged into this branch (ecffba1, f86ef93), which extend the fix beyond the edit/rename paths this PR originally covered.

What changed

Shared predicate — utils.has_chunk_tracking_row() (ecffba1)
The row-presence check (isinstance(row, dict) and isinstance(row.get("chunk_ids"), list)) was hand-copied at four sites and about to spread further. It is now a single named helper carrying the authority-model docstring, adopted at every call site: entity/relation edit, both rename migrations, entity merge, and the ingestion merges below.

Ingestion merge paths in operate.py (ecffba1)
_merge_nodes_then_upsert, _merge_edges_then_upsert, and the edge-endpoint node update inside it still used list truthiness, so an ordinary document insert could reseed a present-but-empty tracking row from the graph's possibly-stale source_id — the most reachable variant of the defect this PR fixes. They now fall back to source_id only when no tracking row exists at all.

_merge_entities_impl step 9 — the item the PR body listed as out of scope (ecffba1, corrected in f86ef93)
Presence is now decided per input entity:

a usable row is authoritative as-is, including an empty list;
an input with an absent/malformed row falls back to its own pre-merge graph source_id (captured before step 5 overwrites the target node with the merged payload, whose source_id unions every input);
the ordered union is written only when at least one usable row exists — one entity's curated-empty row proves only that that entity owns nothing, so "partially known" is never promoted to "all empty";
with no row present at all the target row stays absent (unknown stays unknown).

Previously, merging entities whose rows were all curated-empty left the target with an absent row, re-arming the reseed on the next edit through the HTTP-reachable merge endpoint.

Crash-safe key migrations (ecffba1, f86ef93)
All tracking-row migrations now upsert the new key before deleting the old one: the entity row and relation rows on rename, the target row before source-row deletes on merge, and the merge's relation-row migration (which previously deleted every old key before upserting — a crash between the two calls lost all migrated rows on RPC-backed KV storages). Since a relation key can be both old and new (relations already attached to an existing target keep their key), only keys outside the new key set are deleted.

Verification

New regression tests: tests/utils/test_has_chunk_tracking_row.py, tests/extraction/test_chunk_tracking_presence_ingestion.py, tests/extraction/test_merge_entities_chunk_tracking.py, tests/extraction/test_rename_tracking_migration_ordering.py. Each behavioral test fails on the code as it stood before its commit; the absent-row direction pins (fall back to source_id when no row exists) pass on both, confirming the guards were narrowed, not widened. tests/extraction + tests/utils + tests/api/routes + tests/pipeline: 1584 passed. ruff check / ruff format clean.

PR description is now partially stale

"Out of scope" section: the _merge_entities_impl entity-block item is now implemented (per-entity presence + own-source_id fallback + write ordering). Only the _purge_kg_contributions item remains deferred (still NOTE(#3609)-annotated, unchanged).
"Change" section: predates the shared has_chunk_tracking_row() helper (the quoted predicate has since evolved to require a chunk_ids list), the operate.py ingestion fixes, and the upsert-before-delete migration ordering.
"Testing" section: lists only the original four tests/pipeline regression tests, not the four new test files above.

Generated by Claude Code

danielaskdd · 2026-08-20T13:27:35Z

@codex review

chatgpt-codex-connector · 2026-08-20T13:31:20Z

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 07c4ef7586

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

jaideeppyne mentioned this pull request Aug 18, 2026

Chunk-tracking reseed: has_stored_data truthiness check conflates an absent row with an empty one #3609

Closed

chatgpt-codex-connector Bot reviewed Aug 18, 2026

View reviewed changes

Comment thread lightrag/utils_graph.py Resolved

chatgpt-codex-connector Bot reviewed Aug 18, 2026

View reviewed changes

Comment thread lightrag/utils_graph.py Resolved

fix(graph): validate relation chunk tracking rows

1e92abb

Treat malformed relation tracking rows with non-list chunk_ids as absent so source_id edits reseed them instead of failing after graph and vector mutations. Add a regression test for null chunk_ids on relation edits. Refs HKUDS#3609

danielaskdd and others added 4 commits August 20, 2026 18:56

Merge branch 'main' into fix/chunk-tracking-empty-row-reseed

ac0121b

Merge branch 'main' into fix/chunk-tracking-empty-row-reseed

07c4ef7

danielaskdd merged commit 5992004 into HKUDS:main Aug 20, 2026
4 checks passed

Conversation

jaideeppyne commented Aug 17, 2026 • edited by danielaskdd Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Change

Reachability

Deliberate behavior note

Out of scope (noted, not changed here)

Testing

Uh oh!

danielaskdd commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I verified as correct

Blocking: the third call site is untouched, and it re-arms the reseed you just fixed

Also needs addressing

One process note

Uh oh!

jaideeppyne commented Aug 18, 2026

Uh oh!

danielaskdd commented Aug 18, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

jaideeppyne commented Aug 18, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 18, 2026

Uh oh!

danielaskdd commented Aug 18, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 18, 2026

Uh oh!

danielaskdd commented Aug 18, 2026

Uh oh!

jaideeppyne commented Aug 18, 2026

Uh oh!

danielaskdd commented Aug 18, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

danielaskdd commented Aug 20, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 20, 2026

Uh oh!

danielaskdd commented Aug 20, 2026

What changed

Verification

PR description is now partially stale

Uh oh!

danielaskdd commented Aug 20, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 20, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jaideeppyne commented Aug 17, 2026 •

edited by danielaskdd

Loading

danielaskdd commented Aug 18, 2026 •

edited

Loading