Skip to content

[ISSUE-854] feat(ai): add NearestNeighborIndex contract with linear default adapter - #878

Merged
kitalkuyo-gita merged 2 commits into
apache:masterfrom
E2ern1ty:issue-854-ann-adapter
Oct 12, 2026
Merged

kitalkuyo-gita merged 2 commits into
apache:masterfrom
E2ern1ty:issue-854-ann-adapter

Conversation

@E2ern1ty

@E2ern1ty E2ern1ty commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Implements #854: a minimal nearest-neighbor adapter interface under geaflow-ai's index/ann package, with the exact linear scan as the default implementation — no new dependencies, no change to any existing code path, no performance claim.

Added:

  1. NearestNeighborIndex<T> — the contract. add(id, vector) / search(query, topK) / size(), scored through the existing IVector.match. The javadoc pins the ordering rules the issue asks to be documented:
    • results ordered by descending score, ties broken by insertion order (added-first ranks first), so identical data + query always produce the identical ranking;
    • score direction fixed as entryVector.match(query), because some IVector implementations are not symmetric;
    • append-only: an id added twice is two entries;
    • topK <= 0 throws IllegalArgumentException;
    • an approximate adapter is explicitly allowed to miss neighbors, but must keep the result shape (≤ topK, non-increasing scores, no phantom duplicates).
  2. Neighbor<T> — id + score result record.
  3. LinearNearestNeighborIndex<T> — the default adapter: scores every entry, sorts score-descending with the insertion ordinal as tie-break key, returns a fresh list. This is the correctness baseline an ANN implementation will be measured against, not an optimization.
  4. NearestNeighborIndexContractTest — an abstract suite any adapter runs against (newIndex() factory), split deliberately:
    • shape tests (topK bound, page monotonicity, empty index, argument validation, append-only accounting, larger-topK-extends-without-reordering, no page duplicates) — required of every adapter including future approximate ones;
    • exactness tests (scores equal IVector.match verbatim against hand-checked cosine values, full deterministic ordering, bit-for-bit repeat determinism, collinear/zero-vector tie groups in insertion order) — required of adapters claiming exactness. A future ANN adapter extends the suite and excludes the exactness tests while its recall is approximate; that is the concrete form of the acceptance criterion "the linear adapter passes the same contract that an ANN adapter will use".
  5. LinearNearestNeighborIndexTest — runs the full contract plus linear-specific checks (null rejection, returned page independent of the index).

How was this PR tested?

Production verification

Started GeaFlowMemoryServer from this branch (classpath launch) and drove the HTTP path end to end: /graph/create → /graph/addEntitySchema → /graph/insertEntity × 3 (English + CJK documents) → /query/context → /query/exec → /query/result. Behavior is identical to the master baseline recorded during PR #877's verification: a matching query (alpaca) ranks its document first, a non-matching query (quetzal) still recalls everything through the KeywordVector.toString() wrapper tokens. The new index/ann package compiles into the same artifact and leaves the running service's behavior untouched — as intended for a pure contract addition.

Notes for reviewers

@E2ern1ty

Copy link
Copy Markdown
Contributor Author

Same note as on #877: the red build checks are the pre-existing geaflow-mcp pom issue from the runner's Maven update, not this change (it fails before any compilation). Fix proposed in #879; will rebase once it lands.

@E2ern1ty

Copy link
Copy Markdown
Contributor Author

Update: same as #877 — the pom fix from #879 is cherry-picked into this branch, and the new workflow runs need approval to start.

@E2ern1ty
E2ern1ty force-pushed the issue-854-ann-adapter branch from 50b75ac to b45ec55 Compare October 10, 2026 06:24
@E2ern1ty

Copy link
Copy Markdown
Contributor Author

Branch rebased onto master (includes merged #879) — diff is back to the five ann-package files only. Workflow runs pending approval.

@E2ern1ty

Copy link
Copy Markdown
Contributor Author

The JDK8 build failure was mine: one List.of(...) call in the contract test is a Java 9+ API and the CI's -Pjdk8 profile compiles at source 1.8, so geaflow-ai test-compile failed. Replaced with Arrays.asList and pushed — no other 9+ APIs remain in the PR (scanned). The newly pushed workflow run needs approval again; sorry for the extra round.

@kitalkuyo-gita kitalkuyo-gita left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kitalkuyo-gita
kitalkuyo-gita merged commit 3f11902 into apache:master Oct 12, 2026
2 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.

2 participants