You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
Changes need revision: two medium-severity issues can break Chinese indexing or writable startup.
Medium
internal/db/db.go:4787 — reopenLocked 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.
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.
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.
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.
Code review found one medium-severity concurrency risk in the Chinese FTS tokenizer configuration.
Medium
internal/db/sqlite_driver.go:65 — jieba_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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
messages_chinese_ftsindex backed by wangfenjin/simple and cppjieba, routing CJK queries throughjieba_query()while preserving the existing Porter index for ASCII searchesThe 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.