Skip to content

feat(search): add optional Chinese FTS tokenizer - #1491

Draft
dqtz5vpvj9-create wants to merge 12 commits into
kenn-io:mainfrom
dqtz5vpvj9-create:feature/chinese-fts
Draft

feat(search): add optional Chinese FTS tokenizer#1491
dqtz5vpvj9-create wants to merge 12 commits into
kenn-io:mainfrom
dqtz5vpvj9-create:feature/chinese-fts

Conversation

@dqtz5vpvj9-create

Copy link
Copy Markdown

Summary

  • add an optional parallel messages_chinese_fts index backed by wangfenjin/simple and cppjieba, routing CJK queries through jieba_query() while preserving the existing Porter index for ASCII searches
  • load the native tokenizer on AgentsView archive connections and use connection-local write triggers so the SQLite archive remains writable by tools and older AgentsView builds that do not load the extension
  • add pinned, reproducible sidecar build and install targets with bundled dictionaries and license files, plus Chinese word, single-character, mixed-language, word-order, and writer-reopen regression coverage

The Chinese index is derived and opt-in. If the sidecar is absent, AgentsView keeps the standard FTS5 path available and safely removes only the rebuildable Chinese index.

@roborev-ci

roborev-ci Bot commented Aug 23, 2026

Copy link
Copy Markdown

roborev: Combined Review (8d61ef9)

Changes need revision: two medium-severity issues can break Chinese indexing or writable startup.

Medium

  • internal/db/db.go:4787reopenLocked opens the writer with the base sqlite3 driver. After a full archive swap, the writer lacks the extension and dictionary configuration, so later message mutations can fail when Chinese FTS triggers access messages_chinese_fts.

    • Fix: Use sqliteArchiveDriverName and add behavioral coverage for Chinese indexing after DB.Reopen().
  • internal/db/db.go:4406 — Removing the tokenizer sidecar can prevent writable startup. Dropping an FTS5 table configured with the unavailable simple tokenizer may fail with no such tokenizer: simple, contradicting the documented fallback behavior.

    • Fix: When the extension is unavailable, leave the index dormant and skip Chinese-index maintenance, then rebuild it when the sidecar returns; alternatively, use cleanup that does not instantiate the unavailable tokenizer.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 6m56s

@dqtz5vpvj9-create

Copy link
Copy Markdown
Author

Addressed the archive-reopen issue in 9085f2ef: reopenLocked now uses sqliteArchiveDriverName, with behavioral coverage that reopens the full DB, writes a new Chinese message, and retrieves it through the Chinese FTS path.

I also added a regression test for the sidecar-removal concern. It creates the simple-tokenized table through the extension-enabled driver, closes those connections, then opens the file through plain sqlite3 and successfully drops messages_chinese_fts without loading the extension. This confirms the documented cleanup path does not instantiate the unavailable tokenizer.

@dqtz5vpvj9-create
dqtz5vpvj9-create marked this pull request as draft August 23, 2026 11:18
@roborev-ci

roborev-ci Bot commented Aug 23, 2026

Copy link
Copy Markdown

roborev: Combined Review (9085f2e)

Medium-severity reliability and search-semantics issues should be addressed before merge.

Medium

  • internal/db/chinese_fts_search.go:20 — Stripping FTS quotes before calling jieba_query makes 中文 搜索 equivalent to the exact phrase "中文 搜索", losing quoted CJK phrase semantics. Tokenize literals while preserving phrase boundaries and operators from the prepared FTS expression, and add coverage distinguishing phrase matches from separated terms.

  • internal/db/db.go:4412 — Table creation and backfill occur as separate autocommit statements, while later opens treat table existence as proof of completion. If the initial backfill fails or is interrupted, an empty or incomplete index can be silently reused. Create and backfill atomically, or persist a completion marker and rebuild when it is absent.

  • internal/db/db.go:4399 — Existing indexes are reused without verifying which library or dictionaries created them. Changes to AGENTSVIEW_SIMPLE_DIR, dictionary contents, or the sidecar version can cause query tokenization to diverge from the stored index and silently miss results. Store a tokenizer/dictionary fingerprint with the archive and rebuild the derived index when it changes.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 8m12s

@roborev-ci

roborev-ci Bot commented Aug 23, 2026

Copy link
Copy Markdown

roborev: Combined Review (484b6b7)

The PR has four medium-severity correctness and reliability issues; no Critical or High findings were reported.

Medium

  • scripts/apply-chinese-fts-review-fixes.py:473 — Generated Go code treats parseTimestamp’s error return as a boolean (if !ok) and removes the definition of sqliteUsageOutputTokens while retaining a reference. The workflow fails during go test and never commits the staged fixes. Check parseTimestamp with err != nil or retain ParseStoredTimestamp, and preserve sqliteUsageOutputTokens.

  • internal/db/chinese_fts_search.go:29 — Chinese queries always pass the de-quoted input through jieba_query, destroying explicit FTS5 syntax. Phrase semantics and operators such as OR are not preserved. For quoted input, select the Chinese table but keep the prepared expression verbatim; use jieba_query only for ordinary text and add phrase/operator tests.

  • internal/db/db.go:4392 — Table existence is treated as proof that the Chinese index is complete and compatible. An interrupted backfill can leave an empty index that is never rebuilt, while extension or dictionary changes can leave incompatibly tokenized rows. Create and backfill transactionally, persist a completion fingerprint covering the schema, extension, and dictionaries, and rebuild when it is absent or mismatched.

  • internal/db/sqlite_driver.go:59 — Every archive connection invokes the extension’s module-global jieba_dict setter without synchronization, allowing concurrent connection creation to race while replacing shared dictionary state. Serialize initialization with a package-level mutex.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 10m22s

@roborev-ci

roborev-ci Bot commented Aug 23, 2026

Copy link
Copy Markdown

roborev: Combined Review (c0df51f)

Code review found one medium-severity concurrency risk in the Chinese FTS tokenizer configuration.

Medium

  • internal/db/sqlite_driver.go:65jieba_dict mutates module-global extension state, but the mutex protects only that assignment and jieba_query. Concurrent Chinese FTS MATCH queries, trigger-driven indexing, or rebuilds may use the tokenizer while a new connection reconfigures the global dictionary, risking native data races or crashes.
    • Fix: Initialize the global dictionary once for the extension’s lifetime, or hold the shared lock across every operation that initializes or uses the Chinese tokenizer.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 9m3s

@roborev-ci

roborev-ci Bot commented Aug 24, 2026

Copy link
Copy Markdown

roborev: Combined Review (55646d6)

The PR has one medium-severity correctness issue; no security vulnerabilities were identified.

Medium

  • internal/db/chinese_fts_runtime.go:36 — The BEFORE INSERT freshness trigger runs during existing-session upserts before conflict handling. When ON CONFLICT DO UPDATE or DO NOTHING applies, neither the AFTER INSERT cleanup nor the transcript-revision-specific AFTER UPDATE cleanup fires. This leaves a pending generation and causes HasChineseFTS to disable Chinese search until the database is reopened.

    Suggested fix: Record insert freshness only when the session ID does not already exist, or add conflict-path cleanup that preserves genuinely foreign pending generations. Add coverage for upserting an existing indexed session.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 8m38s

@mariusvniekerk mariusvniekerk self-assigned this Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants