Fix HNSW allow-replace-deleted mode before shared memory

[BCathcart]

Finishes solving #1282 and solves #1288.

There are three things addressed here:

1. Fully recovering to a healthy state when loading an RDB that contains an HNSW index with duplicate labels.

#1283 prevents new instances of duplicate labels being created. It also prevents the validation logic from rejecting existing RDBs with duplicate labels and restores the HNSW index to the state it was in at the time of save, but that isn't a fully healthy state. There will be dangling vector pointers in the index (see #1282 (comment)). This PR actually repairs the HNSW state so that there are no dangling pointers by enforcing label uniqueness. This gives a clear one-to-one mapping of labels in tracked_vectors_ and labels in data_level0_memory_.

2. Cleaning up an overwritten slot's vector from tracked_vectors_.

The whole point of enabling allow-replace-deleted is to reduce memory usage with the potential risk of reducing the quality of the graph structure (incoming edges lose effectiveness when the vector is arbitrarily replaced). The problem was, when a slot was overwritten, we weren't actually freeing the most significant part of the memory associated with it: the old vector (#1288). Now the HNSW library layer returns the label of an overwritten slot so that tracked_vectors_ can be cleaned up.

3. Removes tombstoned labels from label_lookup_.

Labels are used by the search module to operate on live vectors. Once a vector is deleted from the keyspace, and the module has told the HNSW library to tombstone the slot with that label, there is no more use for mapping from label -> slot. label_lookup_ now only contains live labels as a simplification and space optimization.

[greptile-apps]

Greptile Summary

This change repairs HNSW handling for legacy RDBs that contain duplicate labels, preserves live-vector ownership when deleted slots are reused, and adds focused recovery coverage.

One issue was verified: the updated integration test ends with a trailing blank line, causing whitespace checks to fail.

T-Rex validation blocked

The focused HNSW recovery test could not run because the indexes_test binary is absent and the required cmake and ninja tools are unavailable. This prevented runtime coverage of duplicate-label RDB recovery, future-label allocation after restoring tombstones, and deleted-slot vector cleanup.

Confidence Score: 4/5

The HNSW recovery implementation was not runtime-exercised in this environment, but the only verified defect is a non-security whitespace failure.

There is one independent P2 finding and no P0 or P1 findings, so the score is 4 under the required scoring table.

Files Needing Attention: integration/test_hnsw_allow_replace_deleted.py needs its trailing blank line removed. The HNSW implementation and recovery tests need execution in an environment with CMake, Ninja, and the built test binary.

T-Rex Logs

What T-Rex did

• T-Rex produced a proof for a posted P2 finding and linked the related review comment.
• Whitespace validation reported a trailing blank line in the submission, as shown in the log artifact.
• Attempted to run the focused HNSW legacy-RDB recovery validation using the dedicated harness, but execution could not proceed because required binaries and tools were unavailable.
• A validation log records that the focused recovery test was blocked before execution due to missing binary and build tools.

_{Ran code and verified through T-Rex}

_{Reviews (1): Last reviewed commit: "Fix HNSW allow-replace-deleted mode" | Re-trigger Greptile}

[Aksha1812]

/assign-reviewers

[github-actions]

Reviewers for this PR

• First Pass Reviewer: @neerajr0 — Please do your best to do a detailed review on the PR and get a response on your feedback. Once the first pass is done, notify the maintainer assigned to this PR to follow up on the final review and getting the PR merged. You can reach out to the people owning the relevant code paths for more help on the review.
• Maintainer Reviewer: @yairgott — Once the first review is done, please follow up with a final review and help to merge the change in.

_{Assigned automatically to the least-assigned members of the reviewer pools in .github/reviewer-pools.json. Use /reviewer or /remove-reviewer to adjust.}

[yairgott]

This should be hardcoded to false instead of passing algo_->allow_replace_deleted_:

    auto evicted = algo_->addPoint((T *)record.data(), internal_id, false);

Passing allow_replace_deleted_ here is actually a bug that could lead to a server crash. If a document is modified but its label is inexplicably missing from label_lookup_ (e.g. due to an RDB anomaly or transient state), passing true gives HNSW permission to steal a vacant tombstone slot. It will then return the tombstone's old label in evicted, which triggers the CHECK(!evicted.has_value()) and crashes the entire server process.

By passing false, you explicitly deny HNSW permission to steal a tombstone. Instead, if the label is missing, HNSW will gracefully allocate a brand new slot for it. evicted will safely evaluate to std::nullopt (preventing the crash), and the module will seamlessly resurrect the missing node back into the graph.

This is also aligned with my fix to main branch.

[Frank-Gu-81]

Hi @BCathcart 👋 — flagging this as a P1 launch blocker for valkey-search 1.3 RC1. We're cutting the release branch the morning of Sept 14 (RC1 lands Sept 15), so all P1s need to be merged before then.

First-pass reviewer: @neerajr0 — if your first-pass review is already done, please ignore this message; otherwise, please prioritize getting this PR reviewed.

Second-pass reviewer: @yairgott — please take a look/followup with the final review and merge once everything looks good.

If anything is blocking merge (open changes, CI, design questions), drop a note here so we can unblock quickly. Board: #1346. Thanks so much! 🙏