Skip to content

fix(dedup): correct id-collision warning and prefer defining file#1884

Open
SinghAman21 wants to merge 1 commit into
Graphify-Labs:v8from
SinghAman21:fix/1851-dedup-collision-warning
Open

fix(dedup): correct id-collision warning and prefer defining file#1884
SinghAman21 wants to merge 1 commit into
Graphify-Labs:v8from
SinghAman21:fix/1851-dedup-collision-warning

Conversation

@SinghAman21

Copy link
Copy Markdown

Fix the id-collision warning in dedup (wrong cause, overstated loss, silent drop)

Fixes #1851

I spent a while chasing "data loss" that never happened because of this warning,
so here's a fix. The pre-dedup pass in dedup.py has three problems:

  • The stated cause is usually wrong. It blames "two files with the same name
    in different directories," but node ids are <source-path>_<label>, so any doc
    that references an entity mints the same id as the entity's own node. A
    cross-reference colliding with the definition is dedup working as intended, not
    a filename clash.
  • It overstates the damage. When the colliding nodes are the same entity,
    edges rewire to the survivor and nothing detaches — only the dropped node's
    label/description is lost. The message tells you to re-run extraction per
    subfolder + merge-graphs to avoid a loss that didn't occur.
  • It's silent on the one real drop. The warning was gated on the source files
    differing, so two same-id nodes from the same file (extractor emitting two
    labels for one entity) get dropped with no output at all — which is exactly the
    case where a description disappears unseen.

Also, the survivor was just "first one seen," so which node won depended on chunk
order.

What changed

  • Survivor selection now prefers the node whose own source_file is the one
    encoded in the id (_node_defines_id), falling back to first-seen order. The
    authoritative node wins regardless of merge order.
  • The warning now fires on actual information loss — a materially different
    label/description, or a genuine cross-file collision — and stays quiet on
    redundant same-entity copies. The previously-silent same-file case is now
    surfaced.
  • Reworded the message to name the real id-scheme cause, and it only suggests the
    per-subfolder extract + merge-graphs workaround for genuine cross-file
    collisions.

Testing

  • Updated the two existing collision tests and added two more (defining-file
    tiebreak, and the same-file-different-content warning) — full test_dedup.py
    passes.
  • Ran the issue's exact 3-node scenario end to end: the defining doc survives,
    the same-file duplicate now warns without pushing re-extraction, the
    cross-reference warns with the workaround, and an identical same-file copy
    stays silent.

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.

dedup ID-collision warning misdiagnoses cause, overstates data loss, and misses the silent drop (dedup.py:224-246)

1 participant