fix: Japanese searches skip the category and summary result tiers - #3363
Conversation
The pre-split in tokenize() treats U+30FC (ー) and U+3005 (々) as separators, so a katakana word is torn into fragments before Intl.Segmenter can segment it: "データベース" tokenizes as ["デ", "タベ", "ス"]. Two consequences follow. Exploratory search requires every query token to be at least three characters (EXPLORATORY_SEARCH_MIN_TOKEN_LENGTH in search.ts, skills.ts and packages.ts), so katakana queries never reach the category, topic and summary tiers. And getFirstSearchToken feeds normalizedDisplayNameFirstToken, an indexed range-scan bound, which collapses to the single character "デ". detectCJKLanguage in the same file already counts ー as katakana when it picks a segmenter; the pre-split now agrees with it.
|
@Yigtwxx is attempting to deploy a commit to the OpenClaw Foundation Team on Vercel. A member of the Team first needs to authorize it. |
|
Codex review: needs maintainer review before merge. Reviewed August 5, 2026, 7:14 PM ET / 23:14 UTC. ClawSweeper reviewWhat this changesThe branch preserves Japanese prolonged and iteration marks during search tokenization and adds confirmed, rate-limited backfills for native and mirrored catalog search-index first tokens. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readinessKeep open for maintainer rollout approval: the tokenizer fix is sound, but existing native and mirrored digest rows require two authenticated production backfills after deployment. Likely related people: Patrick Erichsen (high confidence). Priority: P2 Review scores
Verification
How this fits togetherClawHub catalog search tokenizes user queries and stored skill metadata, then uses those tokens for match tiers and indexed candidate retrieval. The proposed backfills update persisted digest tokens so existing catalog rows remain compatible with the new query tokenization. flowchart LR
A[Japanese query or skill name] --> B[Search tokenizer]
B --> C[Search tokens]
C --> D[Match tiers and index bounds]
E[Stored catalog digests] --> F[Confirmed backfill]
F --> D
D --> G[Catalog search results]
Decision needed
Why: The changed tokenizer alters persisted index-bound values; code review cannot perform the authenticated production migration or decide whether a transient stale-index window is acceptable. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Merge the tokenizer repair only with an explicit production rollout that previews and then runs both confirmed digest backfills, followed by a production dry-run verifying no stale rows remain. Do we have a high-confidence way to reproduce the issue? Yes. Current main's source directly shows the two marks excluded before segmentation, and the supplied focused regression evidence demonstrates the resulting token and match-tier failure. Is this the best way to solve the issue? Yes. Repairing tokenization at its shared source and resynchronizing both native and mirrored persisted first-token fields is narrower and safer than weakening the three-character search threshold or relying on a partial candidate source. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9009eae00355. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (24 earlier review cycles; latest 8 shown)
|
Patrick-Erichsen
left a comment
There was a problem hiding this comment.
This changes Japanese tokenization so category and summary search tiers can use longer tokens. It changes the persisted prefix-index contract without migrating existing rows, and the supported no-Segmenter fallback still fails the stated behavior.
LOC: +29/-4 (2 files)
Findings: add an idempotent rate-controlled skillSearchDigest backfill for existing rows, and make/test the no-Intl.Segmenter path retain usable Japanese word runs rather than standalone marks or sub-three-character tokens.
Best-fix verdict: too narrow. Tokenizer correctness and derived-index migration are one deployment contract; changing only query/write logic leaves existing catalog rows inconsistent.
Alternatives considered: lazy rewrite on later skill mutation leaves search broken indefinitely; lowering the exploratory floor would weaken all languages instead of fixing Japanese token formation.
Code read: tokenizer/digest generation, prefix-index query path, fallback branch, and focused tests.
Remaining uncertainty: no real catalog/Convex search proof or rollout plan is supplied.
Widening the CJK class moves the first token of any name containing a prolonged sound mark or an iteration mark. skillSearchDigest rows recompute that field only when their skill is written, so already-stored rows keep the old one-character token while search uses the new longer token as a range index bound - the row stays on disk and out of recall. Add a cursor-paginated resynchronization next to the existing digest backfills in maintenance.ts, and stop the no-Segmenter fallback from emitting those two marks as standalone tokens that exploratory matching discards.
The catalog search page subscribes to skillSearchDigest, so a backfill that reschedules itself with no delay drives reactive re-reads back to back for the whole run. .agents/skills/clawhub-convex/SKILL.md asks for a delay between backfill batches that write reactively subscribed tables. The delay is an optional argument clamped the same way the batch size is, and it follows repairLegacyPublisherOwnershipForUserHandler, which is the one backfill in this file that already spaces its batches.
|
@Patrick-Erichsen both findings are addressed, the second half of the first one in a follow-up commit. Idempotent, rate-controlled backfill for existing rows - The rate-control half was missing and is now in No- Each mark is attached to the character it extends, which is the rule the widened class states. Removing that branch turns exactly one test red. Verification on One thing worth flagging rather than hiding: the delay now differs from the ten other self-continuing backfills in Also note that this branch now touches |
|
The live-deployment proof is now in the PR body under Evidence -> Live local Convex: the admin backfill path end to end. It is the migration driven through the CLI against a real Convex backend - an anonymous local deployment from Twelve of thirteen rows disagree with the new tokenizer; the Latin control does not. The first page patches ten and returns Two things I would rather state than leave for you to find:
|
The skills.sh mirror persists its own normalizedSlugFirstToken and normalizedDisplayNameFirstToken, derived through the same tokenizer, and external candidate search range-scans both. Widening the katakana class therefore strands mirrored rows exactly the way it stranded native digest rows, and the previous backfill only paged skillSearchDigest. skillsShMirror.ts had its own copy of the first-token rule. Both callers now share getMirrorFirstSearchToken so the two cannot drift apart again.
|
The mirror finding is fixed in It is a real gap and the diagnosis is right:
The duplicated rule is also gone. Both halves of the new mutation are load-bearing: Verification on One limit worth stating plainly: on the live local deployment I could only prove the mutation reaches the real table. Seeding the single mirrored row |
|
On the fallback finding - "Skip leading Japanese extenders in the fallback (P2) ... The Segmenter path drops such non-word segments" - I do not think that premise holds, so I have not made the change. The measurement, on the current head Same output on Node v22.20.0 (V8) and on Bun 1.3.14 (JSC). Two things follow:
Where the two really differ - Happy to be wrong here - if the intent is that the fallback should normalise more aggressively than the Segmenter rather than approximate it, that is a different and larger change than this PR, and I would rather do it separately than bolt it on. On the remaining rollout item: both backfills are admin-gated and must be run once after deploy, native first, then mirror. Both accept |
Both backfills defaulted dryRun to false, and their public admin actions forward omitted arguments straight through. A bare `npx convex run maintenance:backfillSkillSearchDigestFirstTokens` therefore patched skillSearchDigest and scheduled every remaining page, against a table catalog search subscribes to. An operator typo was an immediate production apply rather than a preview. Both now follow the contract the plugin catalog-digest resync already uses: preview unless dryRun is explicitly false, reject an apply whose confirm token does not match, and carry that token into the scheduled continuation so the run does not stall on its own guard after the first page. The native and mirror paths take separate tokens, so neither unlocks the other.
|
The confirmation finding is fixed in That one was mine to fix rather than argue with. Both now use the contract Three things beyond copying that shape:
Coverage: four new tests, two per path - omitted arguments preview without writing or Reverting only One thing I did not do: re-record the live local Convex transcript in the PR body. It was The rollout section in the PR body now carries both forms, preview and apply. |
|
I said above that I had not re-recorded the live transcript. I have now, for the guard The mirror action rejects the same way, including on the native path's token. The part I most wanted off a mock is the scheduler. Re-seeded, one confirmed page of ten, The eleventh row sits stale for the whole delay and then lands. A continuation that No code change since |
|
Thanks for merging this. One thing this PR cannot do for itself, flagged here so it does not get lost: the two Both preview by default, so this form counts the affected rows without writing anything Both accept |
What Problem This Solves
Fixes an issue where searching in Japanese returns fewer skills than it should. A query
whose katakana contains the prolonged sound mark
ー— which covers most of the loanwordvocabulary this registry is full of,
データベース,サーバー,ユーザーインターフェース,コンピューター— never reaches the category, topic and summary match tiers, and matchesonly on name and slug.
The affected surfaces are skill search on
/search, the skills and plugins catalogsearch, and the search digest that backs the catalog index.
Why This Change Was Made
tokenize()delegates word segmentation toIntl.Segmenter, which handles Japanesecorrectly. But before the segmenter runs, the text is split on a hand-written character
class, and that class omits
ー(U+30FC) and々(U+3005). Both are therefore treated asseparators, so the word is already in pieces by the time the segmenter sees it:
tokenize()onmainデータベース["デ", "タベ", "ス"]["データベース"]データベース管理["デ", "タベ", "ス", "管理"]["データベース", "管理"]ユーザーインターフェース["ユ", "ザ", "インタ", "フェ", "ス"]["ユーザー", "インターフェース"]コンピューター["コンピュ", "タ"]["コンピューター"]人々["人"]— the々is dropped["人々"]Two things follow from that.
Exploratory search requires every query token to clear a three-character floor
(
EXPLORATORY_SEARCH_MIN_TOKEN_LENGTH, defined identically inconvex/search.ts,convex/skills.tsandconvex/packages.ts, enforced bymatchesExploratoryTokenPrefixes).["デ", "タベ", "ス"]cannot clear it, so rank tiers 2and 3 — category/topic and summary — are unreachable for these queries.
["データベース"]clears it.And
getFirstSearchTokeninconvex/lib/skillSearchDigest.tsistokenize(value)[0].Its result is stored as
normalizedDisplayNameFirstTokenandnormalizedSlugFirstToken,which are indexed columns used as range-scan bounds in
convex/search.ts. A skill namedデータベース管理indexes under the single characterデinstead ofデータベース, so thebound stops being selective.
The same file already disagrees with itself about this.
detectCJKLanguage, 53 linesbelow, counts katakana with the full Unicode block
/[゠-ヿ]/, which does matchー— that is how the text is correctly routed to the Japanese segmenter in the firstplace. The pre-split then excludes the character the language detector just counted. This
change makes the pre-split agree with it.
I deliberately added only
ーand々rather than widening to the full katakanablock. Widening produces identical
tokenize()output —Intl.Segmenteralready reports・(U+30FB) as not word-like — but it would also pull・,゠andヿinto thesegmentCJKByCharfallback as standalone tokens. The narrow change avoids that.Non-goals:
change to a hot path and
\p{Script=Katakana}does not matchーeither, so it wouldnot fix this on its own.
future writes and queries produce.
User Impact
Japanese-language searches now reach the same result tiers as English and Chinese ones,
so a query like
データベースcan match a skill on its category, topics or summary ratherthan only on its name. Japanese skills also index under their actual first word instead
of a single character.
Evidence
Base commit:
a9d04bb0. The fix is not onmain— both character classes there stillend
ァ-ヺ가-.Focused tests
convex/lib/searchText.test.tsalready existed, but its only Japanese assertion wasexpect(tokens.length).toBeGreaterThan(0), while the Chinese cases directly above itassert real segmentation. This raises the Japanese cases to that standard and adds the
regressions. On the parent commit:
With the change:
The 15 assertions that already passed still pass, including every Chinese, Korean and
ASCII case —
tokenize("中文搜索"),tokenize("안녕하세요"),tokenize("React 组件开发")and
tokenize("Minimax Usage /minimax-usage")all produce identical output before andafter.
The two consequences, measured
The intra-file inconsistency, checked directly:
Deployment consequence: rows written before this tokenizer
skillSearchDigest.normalizedSlugFirstTokenandnormalizedDisplayNameFirstTokenareproduced by this tokenizer (
getFirstSearchTokenistokenize(value)[0]), andconvex/search.ts:1327-1364uses them as range-index bounds. Those rows are recomputedonly when their skill is written, so after deploying this change an untouched row keeps
the old one-character token while the search computes the new longer one. The row stays
on disk and drops out of that first-token range lookup.
Measured on the samples this branch is about - two rows that do not move are kept in as
the control:
maintenance.tsgainsbackfillSkillSearchDigestFirstTokens, a cursor-paginatedinternal mutation that self-schedules until done, plus the admin-gated action that starts
it. It is written against the shape the file already uses for
backfillSkillSearchDigestModerationVerdictsdirectly above it: same args, sameclampInt(batchSize, 10, 200), samedryRun, same return record.Batch spacing between backfill pages
skillSearchDigestis read by live catalog-search subscriptions, and.agents/skills/clawhub-convex/SKILL.md:102asks for a delay between backfill batchesthat write reactively subscribed tables. The mutation takes an optional
delayMs,clamped the way the batch size already is, and hands it to the
runAfterthat schedulesthe next page:
That follows
repairLegacyPublisherOwnershipForUserHandler(
convex/maintenance.ts:3272), which is the one backfill in this file that alreadyspaces its own batches. Two assertions cover it - the 500 ms default, and an explicit
2000 ms plus the clamp that turns 600000 ms into 60000 ms. Restoring
runAfter(0, ...)turns both of them red:
Real Convex runtime: a pre-existing row, before and after
A browser capture cannot show this one. Search runs inside Convex, so a locally changed
tokenizer is not what a deployed frontend talks to. The equivalent real setup is
convex-test, which runs the actual Convex query engine and the real schema against anin-memory database - the same harness
convex/catalogFeed.runtime.test.tsandconvex/publishAttempts.runtime.test.tsalready use.convex/skillSearchDigestFirstTokens.runtime.test.tsinserts a realskillsrow and areal
skillSearchDigestrow holding the pre-change tokenデ, then queries the index thesearch actually queries (
by_active_normalized_display_name_first_token) with the boundsthe search computes:
The second run of the backfill over the same data reports
patched: 0, so it isidempotent and safe to re-run.
Live local Convex: the admin backfill path end to end
The runtime test above proves the index behavior. This is the same migration driven
through the CLI against a real Convex backend - an anonymous local deployment created by
bunx convex dev, cloud port 3210, site port 3211.This transcript was recorded at
8d376a48, before the confirmation guard below existed.The two dry runs at steps 4 and 7 behave identically today. The applying call at step 5
is now a preview as written and needs
{"batchSize":10,"delayMs":15000,"dryRun":false,"confirm":"backfill-skill-search-digest-first-tokens"}to do what its output shows. What a page does once it starts - the paging, the 15 s
spacing, the idempotence - is what that commit leaves untouched.
fixtures/public-corpus/corpus.jsonlholds no record whose slug or display name carriesー(U+30FC) or々(U+3005) - zero of its 1250 rows - so the 13 seeded skills areconstructed: twelve Japanese display names carrying those marks, plus one Latin control.
They are seeded while
a9d04bb0is the deployed code, so their digest rows are written bythe old tokenizer, and this branch is deployed over them afterwards. Cursor strings are
cut from the output below; nothing else is edited.
Twelve of the thirteen stored rows disagree with the new tokenizer and the Latin control
does not. The first page patches ten and returns
isDone: false; the remaining two staystale for the whole 15 s the scheduled continuation waits, then land between
11:03:16and
11:03:18. A further dry run patches nothing, so a re-run is safe.What this run does not show is a user-visible recall change.
convex run search:searchSkillsforデータベース,サーバー,人々andDeploy, before and afterdeploying this branch over the stale rows, returned the same one row each time. The
first-token range lookup is one of several candidate sources inside
nativeSkillSearch,and full-text search on
displayNamestill matched on a catalog this small. The recallthis backfill restores is specific to that range lookup, which is what the runtime test
isolates.
The mirrored skills.sh catalog carries the same keys
skillsShMirrorDigestspersists its ownnormalizedSlugFirstTokenandnormalizedDisplayNameFirstToken, andconvex/search.ts:1073-1099range-scans both whenit collects external candidates. They come out of the same tokenizer this PR widens -
convex/skillsShMirror.tshad its ownfirstSearchToken,tokenize(value)[0] ?? normalizedSearchText(value)- so mirrored rows drift exactly the way native digest rowsdo, and the native backfill above cannot reach them because it pages
skillSearchDigest.backfillSkillsShMirrorDigestFirstTokensis the mirror-side counterpart: same args, sameclampInt(batchSize, 10, 200), samedelayMs, samedryRun, same admin-gated action.The two copies of the first-token rule are now one -
getMirrorFirstSearchTokeninconvex/lib/skillSearchDigest.ts, next to thegetFirstSearchTokenthe native digestalready used - so they cannot drift apart again.
Both halves are covered, and each is load-bearing:
On the live local deployment the mutation reaches the real table - seeding the one
mirrored row
devSeed:seedCanonicalSearchFixturecreates and running the backfill over itreports
{ scanned: 1, patched: 0 }, correct for a Latin display name. The drift caseitself is proved in
convex/maintenance.test.tsrather than live: nothing in the localdev-seed path produces a mirrored row with a Japanese display name, and the importer
protocol that would create one needs a run, a stored source page and a batch lease.
The no-Segmenter fallback
Admitting the two marks into
CJK_REalso reachessegmentCJKByChar, the path taken whenIntl.Segmenteris missing. It emits one token per character, so the marks came outstanding alone - one-character tokens that exploratory matching discards at its
three-character floor. Measured before the fix:
They are now attached to the character they extend (
["デー","タ","ベー","ス"],["人々"]), which is the same rule the widened class states. Removing that branch turnsexactly one test red.
Preview by default, confirmation to apply
Both backfills first landed here with
dryRundefaulting tofalse, and their publicactions forward omitted arguments to the mutation unchanged. A bare
npx convex run maintenance:backfillSkillSearchDigestFirstTokenstherefore patchedskillSearchDigestand scheduled every remaining page: a table-wide write against areactively subscribed table, started by a command that reads like an inspection.
They now follow the contract
resyncPluginCatalogMetadataDigestsBatchInternalalreadyuses in this file:
Three details beyond copying that shape:
confirmwith it. Without that, page two throws onthe guard page one passed, and a confirmed run stalls after its first batch.
(
backfill-skill-search-digest-first-tokensandbackfill-skills-sh-mirror-digest-first-tokens), so neither one starts the othertable's rewrite.
confirmRequired, so the token comes out of the dry run an operatoralready has to do rather than out of the source.
Four new tests cover it, two per path: omitted arguments preview without writing or
scheduling, and an apply with no token or the other path's token throws before the first
paginate. The three existing tests that exercise a real page now pass the token andassert it in the
runAfterpayload. The runtime test drives the guard through a Convexbackend as well - the unconfirmed call reports
patched: 1and leaves the row holdingits stale
デ, and only the confirmed call rewrites it.Live local Convex: the guard itself
Same setup as the transcript above, rebuilt: an anonymous local deployment, thirteen
constructed rows seeded while
a9d04bb0is deployed so their digest rows carry the oldtokenizer's output, then this branch deployed over them. Eleven of the thirteen drift.
Cursor strings are cut; nothing else is edited.
The mirror action rejects the same way, including the native path's token, on a mirror
table this fixture leaves empty (
scanned: 0), so that pair shows the guard rather thana migration:
The token has to survive the scheduler or a confirmed run applies its first page and then
throws on the guard that page just passed. Re-seeded, then one confirmed page of ten with
the continuation 15 s out, polled by dry run:
The eleventh row stays stale for the whole delay and then lands. A continuation that lost
the token would have thrown instead, and that row would still read
"patched":1at poll10.
Rollout
Both backfills are admin-gated and are meant to run once after this deploys, native
first, then mirror. Both preview by default, so this form counts rows without writing:
Applying takes the token that preview returns as
confirmRequired:Both accept
{"delayMs": N}if the default 500 ms between pages is too tight. Both areidempotent, so a re-run after a partial pass is safe.
CI
bun run lint,bun run deadcode:ci,bun run ci:types-build— all pass.bunx tsc -p packages/schema/tsconfig.json --noEmitandbunx tsc -p packages/clawhub/tsconfig.json --noEmit— both clean.bun run format:checkfails onCLAUDE.mdand.agents/skills/autoreview/CLAUDE.md. That is pre-existing: the same two files, andonly those two, fail on
mainata9d04bb0. The two files in this PR are formattedwith
oxfmt.VITE_CONVEX_URL=https://example.invalid bun run ci:uniton this branch:14 failed | 431 passed | 1 skipped (446)files,24 failed | 5790 passed | 9 skippedtests. On
mainata9d04bb0:14 failed | 430 passed | 1 skipped (445)files and24 failed | 5773 passed | 9 skippedtests. The sorted list of failing files isidentical on both sides — they are the suites that shell out to
bun, plusconvex/lib/githubAccount.test.tsandsrc/routes/-management.test.tsx. The failurecount is the same 24 on both sides. This branch adds one file and seventeen passing
tests and introduces no new failure.
The Vercel check on this PR stays pending OpenClaw Foundation authorization, as it does
on every fork PR.
Written with AI assistance. I ran the focused tests, the drift measurement and the
Convex runtime proof above myself,
diffed the full unit suite against
main, and can maintain this code.