Skip to content

Conversation

@xav-db
Copy link
Member

@xav-db xav-db commented Feb 5, 2026

UpsertE was incorrectly using the first edge from the source iterator instead of looking up edges by the specified from_node/to_node parameters.

Fixes #850

Greptile Overview

Greptile Summary

Fixed a critical bug in UpsertE where it incorrectly used the first edge from the source iterator instead of looking up edges by the specified from_node/to_node/label parameters.

Key Changes:

  • Refactored upsert_e() in upsert.rs:353-520 to query the database directly using edge endpoints rather than relying on iterator content
  • Added database lookup logic that searches out_edges_db by from_node + label_hash, then filters by matching to_node
  • Removed the mut modifier from self parameter since the iterator is no longer consumed
  • Updated test_upsert_e_ignores_iterator_content() to reflect the new behavior where iterator content is completely ignored
  • Added two comprehensive regression tests (test_upsert_e_creates_edge_between_correct_nodes_issue_850 and test_upsert_e_updates_correct_edge_when_multiple_edges_exist_issue_850) that verify the fix

Impact:
This fix ensures that UpsertE operations target the correct edges based on their endpoints, preventing accidental updates to unrelated edges. The change makes the behavior more predictable and aligns with the semantic meaning of the from_node and to_node parameters.

Important Files Changed

Filename Overview
helix-db/src/helix_engine/traversal_core/ops/util/upsert.rs Fixed upsert_e to lookup edges by from_node/to_node/label instead of using source iterator, resolving Issue #850
helix-db/src/helix_engine/tests/traversal_tests/upsert_tests.rs Added regression tests for Issue #850 and updated existing test to reflect new behavior of ignoring iterator content

Sequence Diagram

sequenceDiagram
    participant Client
    participant UpsertE as upsert_e()
    participant Storage as HelixGraphStorage
    participant OutEdgesDB as out_edges_db
    participant EdgesDB as edges_db
    participant InEdgesDB as in_edges_db

    Note over Client,InEdgesDB: Before Fix (Issue #850)
    Client->>UpsertE: upsert_e(label, from_node, to_node, props)
    UpsertE->>UpsertE: Get first edge from iterator
    Note over UpsertE: ❌ Ignores from_node/to_node parameters!
    UpsertE->>EdgesDB: Update wrong edge
    UpsertE-->>Client: Return incorrect edge

    Note over Client,InEdgesDB: After Fix (Current)
    Client->>UpsertE: upsert_e(label, from_node, to_node, props)
    Note over UpsertE: ✓ Ignores iterator content
    UpsertE->>UpsertE: hash_label(label)
    UpsertE->>OutEdgesDB: get_duplicates(from_node + label_hash)
    OutEdgesDB-->>UpsertE: Iterator of edges
    UpsertE->>UpsertE: Filter by to_node match
    alt Edge exists
        UpsertE->>Storage: get_edge(edge_id)
        Storage-->>UpsertE: existing_edge
        UpsertE->>UpsertE: Merge properties
        UpsertE->>EdgesDB: put(updated_edge)
        UpsertE-->>Client: Return updated edge
    else Edge does not exist
        UpsertE->>UpsertE: Create new edge
        UpsertE->>EdgesDB: put(new_edge)
        UpsertE->>OutEdgesDB: put(from_node mapping)
        UpsertE->>InEdgesDB: put(to_node mapping)
        UpsertE-->>Client: Return new edge
    end
Loading

UpsertE was incorrectly using the first edge from the source iterator
instead of looking up edges by the specified from_node/to_node parameters.

Changes:
- Modified upsert_e to query out_edges_db directly using from_node +
  to_node + label to find existing edges
- Source iterator content is now ignored for edge lookup
- Added regression tests for issue #850
@xav-db xav-db merged commit 842f357 into dev Feb 6, 2026
26 checks passed
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