ADORSYS-GIS / ADORSYS-GIS/lightbridge-code-intelligence

[Ticket]: Add a uniqueness constraint on :Symbol(repo_id, commit, node_id)

Open
#631 0 comments 0 reactions 1 assignee Claimed by @leghadjeu-christian View on GitHub
ticket
Dominant language
Rust
Stars
0
Forks
0
Avg merge
14h 13m
Merged PRs (30d)
16

Description

### Type
Technical debt

### Summary
We need a Neo4j uniqueness constraint on `:Symbol(repo_id, commit, node_id)` because
`upsert_graph`'s `MERGE` currently has nothing backing that key — only a vector index and a fulltext
index exist today.

Expected result: `(repo_id, commit, node_id)` is enforced unique at the database level, and
`MERGE`'s existence check becomes an index lookup instead of a full label scan.

### Intent
`ensure_indexes` (`services/control-plane/src/integrations/neo4j.rs`) creates a vector index and a
fulltext index at startup, but nothing enforces uniqueness on the exact key `upsert_graph` `MERGE`s
against. Verified this does **not** cause duplicate nodes from a duplicate `node_id` *within one*
`UNWIND` batch (tested directly against a live Neo4j — Cypher processes `UNWIND` rows sequentially
through `MERGE` by default, so within-batch duplicates already collapse correctly). The real,
still-open gap is two independent things a constraint would close: concurrent `upsert_graph` calls
for the same `(repo_id, commit, node_id)` racing without a constraint, and `MERGE`'s existence check
falling back to a full label scan instead of an index lookup without one — a real cost at the row
counts `upsert_graph` now writes in one statement since #626.

### Source of truth (links)
Follow-up item from #626's review — see that PR's discussion and the implementation plan shared
alongside it (item 4, "Uniqueness constraint on :Symbol(repo_id, commit, node_id)").

### Current Behavior
```rust
pub async fn ensure_indexes(graph: &Graph, dimension: i64) -> anyhow::Result<()> {
create_index_idempotent(graph, "CREATE VECTOR INDEX symbol_embedding_idx IF NOT EXISTS ...", "symbol_embedding_idx").await?;
create_index_idempotent(graph, "CREATE FULLTEXT INDEX symbol_label_fulltext IF NOT EXISTS ...", "symbol_label_fulltext").await?;
Ok(())
}
```
No constraint on the `MERGE` key.

### Expected Behavior
A third call through the same idempotent helper:
```rust
create_index_idempotent(
graph,
"CREATE CONSTRAINT symbol_unique IF NOT EXISTS \
FOR (s:Symbol) REQUIRE (s.repo_id, s.commit, s.node_id) IS UNIQUE",
"symbol_unique",
).await?;
```

### Acceptance Criteria
- [ ] Given `ensure_indexes` runs at startup, when it completes, then the composite uniqueness
constraint exists (idempotently, like its two siblings).
- [ ] Given the existing live-Neo4j tests (`upsert_graph_round_trips_against_live_neo4j`,
`prune_graph_keeps_only_the_keep_set`), when run against the constrained schema, then they still
pass unchanged — proves the constraint doesn't reject the legitimate re-index/idempotent-`MERGE`
path.

### Out of Scope
- Running the live-Neo4j tests in CI — not part of this ticket (they still run manually/ignored for
now).
- Any change to `upsert_graph`/`upsert_code_chunks` themselves.

### Technical Context
`services/control-plane/src/integrations/neo4j.rs::ensure_indexes`, `create_index_idempotent`.

### Risks
Low — purely additive schema, idempotent by construction like the two existing indexes it sits
alongside.

### Test Plan
Existing live-Neo4j integration tests (`cargo test -p control-plane -- --ignored neo4j` against a
local `docker compose up -d neo4j`), run manually.

### Verification evidence
Test output: see the eventual PR.

### Human accountable owner
@leghadjeu-christian

### AI Usage Declaration
Drafting the ticket, Understanding code, Proposing implementation

### Human verification completed
- [x] I am the accountable owner and accept responsibility for this ticket.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.