Skip to content

[ISSUE-856] test(ai): golden tests for keyword index tokenization, serialization and lifecycle - #877

Merged
kitalkuyo-gita merged 2 commits into
apache:masterfrom
E2ern1ty:issue-856-keyword-golden-tests
Oct 12, 2026
Merged

kitalkuyo-gita merged 2 commits into
apache:masterfrom
E2ern1ty:issue-856-keyword-golden-tests

Conversation

@E2ern1ty

@E2ern1ty E2ern1ty commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Implements #856: golden tests that pin the keyword index's tokenization, serialization format, and lifecycle/retrieval semantics, so that changes to them (a Lucene bump, an edit to query formatting, or the resident keyword index of #825) become visible diffs instead of silent recall shifts.

Added tests (all in geaflow-ai, following the acceptance criteria of the issue):

  1. KeywordTokenizationGoldenTest + tokenization_golden.txt / query_format_golden.txt
    • Analyzer behavior (from SearchStore, the instance production shares): lower-casing, hyphen/slash splitting, intra-word dots kept, CJK emitted as single characters.
    • English stop words are NOT removed — StandardAnalyzer() in the pinned lucene-core 8.11.2 is built with an empty stop-word set (verified via the constructor bytecode), so the/and/of are real searchable terms.
    • SearchUtils.formatQuery: excluded characters become spaces and the substring http is erased — so https://x degrades to s x (https contains http).
  2. KeywordIndexSerializationGoldenTest + serialized_docs_golden.txt
    • Pins the wire format of the Lucene documents: every KeywordVector lands in content as the wrapper KeywordVector{vec=[a, b]}, multiple vectors joined with the two-space delimiter; vertices carry id/label/content, edges src/dst/label/content.
  3. KeywordIndexLifecycleGoldenTest
    • Lifecycle contract: index → close (commit) → search → discard. Searching before close silently returns empty (uncommitted IndexNotFoundException swallowed); indexing after close fails (Lucene's IndexWriterConfig sharing guard) — the store is single-shot, which is exactly why SessionOperator rebuilds per query, and what feat(ai): make the keyword search index resident and incremental #825's resident index proposes to change.
    • Common-token pollution: on a corpus larger than TopN, an exact term still returns exactly its document; querying the wrapper token keywordvector matches every document and is capped at TopN — the noise a dedicated query representation would remove.
    • Incremental add/delete documented: adds accumulate until close; deletion is a query-time filter through the GraphAccessor (a hit whose entity is gone, or whose label is unregistered, never surfaces — the Lucene doc survives).
    • Unicode: CJK matching is single-character OR — querying 学习 also recalls a document indexed under 习题 through the shared character 习.
    • Stop words searchable: a document containing the is found by querying the.

Why golden files (and not plain assertions)?

The recorded expectations were generated against the real analyzer/store on master — including several behaviors nobody had written down (empty stop-word set, https→s, single-shot lifecycle, wrapper-token pollution). The goldens turn each of these into a decision a future change has to acknowledge.

How was this PR tested?

  • mvn -pl geaflow-ai test — 73 tests, 0 failures (the 14 new ones included).
  • Negative check: corrupting one golden row makes the corresponding test fail with a readable diff; restored afterwards.
  • Production environment verified

Production verification

Started GeaFlowMemoryServer from this branch (classpath launch) and drove the full HTTP path: /graph/create → /graph/addEntitySchema → /graph/insertEntity × 5 (English, CJK and shared-filler documents) → /query/context → /query/exec → /query/result, across several sessions. Findings:

  • The pinned behaviors hold on the live path: case-insensitive matching (ALPACA matches like alpaca), CJK queries flow through single-character matching, and the build/close/search rebuild happens per /query/exec exactly as the lifecycle test records.
  • The common-token pollution the issue suspected is fully visible end-to-end: querying a word that exists in no document (quetzal), and even an empty query, each return every entity of the graph; the query text only affects scoring. The root cause is precisely what the goldens pin: KeywordVector.toString() wraps both the indexed content and the query string (SessionOperator.apply → contents.add(v.toString())), so every query carries the keywordvector/vec tokens that every document contains. commonWrapperTokensDoNotCrowdOutExactAnchoring is the executable statement of this fact.
  • One layered-semantics note recorded for completeness: an empty query string throws at the GraphSearchStore layer (locked by emptyQueryFailsTheSearchStore), but the HTTP layer never surfaces it — the wrapper guarantees the parsed query is never empty, so an empty request body degrades to the same full-graph recall rather than an error.

Coordination with the resident keyword index PR

Per the issue's constraint, these tests target master's behavior (the PR branch is cut from master, no engine changes). PR #825 will need to update the lifecycle goldens where it changes the single-shot rebuild semantics — the failing golden names the exact behavior that moved. The production verification above should also serve that PR as the before-state of retrieval quality.

@E2ern1ty

Copy link
Copy Markdown
Contributor Author

Note for reviewers: the red build checks here are not caused by this PR — every open PR fails in ~25s with the same reactor model error (duplicate geaflow-cluster dependency in geaflow-mcp/pom.xml, exposed by the newer Maven on the CI runner image). Fix proposed in #879; once that lands I'll rebase.

@E2ern1ty

Copy link
Copy Markdown
Contributor Author

Update: I cherry-picked the one-line pom fix from #879 into this branch so CI can run independently of that PR's merge. The newly pushed workflow runs need approval to start (same first-run gate as before) — once approved they should go green, as #879's identical change just passed both JDK8 and JDK11 builds.

@E2ern1ty
E2ern1ty force-pushed the issue-856-keyword-golden-tests branch from 457deb6 to db59c97 Compare October 10, 2026 06:24
@E2ern1ty

Copy link
Copy Markdown
Contributor Author

Branch rebased onto master (now includes the merged #879) — the temporary pom fix is gone from the diff, this PR is back to tests-only. The pushed workflow runs just need approval to start.

@E2ern1ty

Copy link
Copy Markdown
Contributor Author

Heads-up: this branch hit the same JDK8-only compile issue as #878 (a List.of in the serialization golden test — the CI compiles at source 1.8, my local JDK 21 hid it). Fixed and pushed; the JDK8 run currently in flight was started before the fix, so expect it red — the next run after approval carries the fix.

@kitalkuyo-gita
kitalkuyo-gita merged commit f08c7ff 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