You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@geaflow.apache.org by "E2ern1ty (via GitHub)" <gi...@apache.org> on 2026/10/09 03:42:30 UTC

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

E2ern1ty opened a new pull request, #877:
URL: https://github.com/apache/geaflow/pull/877

   ### 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 #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?
   
   - [x] `mvn -pl geaflow-ai test` — 73 tests, 0 failures (the 14 new ones included).
   - [x] Negative check: corrupting one golden row makes the corresponding test fail with a readable diff; restored afterwards.
   - [ ] Production environment verified
   
   ### 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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org
For additional commands, e-mail: issues-help@geaflow.apache.org


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

Posted by "E2ern1ty (via GitHub)" <gi...@apache.org>.
E2ern1ty commented on PR #877:
URL: https://github.com/apache/geaflow/pull/877#issuecomment-6095225229

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org
For additional commands, e-mail: issues-help@geaflow.apache.org


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

Posted by "kitalkuyo-gita (via GitHub)" <gi...@apache.org>.
kitalkuyo-gita merged PR #877:
URL: https://github.com/apache/geaflow/pull/877


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org
For additional commands, e-mail: issues-help@geaflow.apache.org


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

Posted by "E2ern1ty (via GitHub)" <gi...@apache.org>.
E2ern1ty commented on PR #877:
URL: https://github.com/apache/geaflow/pull/877#issuecomment-6092829836

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org
For additional commands, e-mail: issues-help@geaflow.apache.org


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

Posted by "E2ern1ty (via GitHub)" <gi...@apache.org>.
E2ern1ty commented on PR #877:
URL: https://github.com/apache/geaflow/pull/877#issuecomment-6094638453

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org
For additional commands, e-mail: issues-help@geaflow.apache.org


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

Posted by "E2ern1ty (via GitHub)" <gi...@apache.org>.
E2ern1ty commented on PR #877:
URL: https://github.com/apache/geaflow/pull/877#issuecomment-6093396273

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@geaflow.apache.org
For additional commands, e-mail: issues-help@geaflow.apache.org