Repository navigation
fix(search): keep Chinese function words out of lexical query admission - #1969
jasondeng1997 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Mixed boundary bigrams and analyzer-shaped Latin identifiers still cause incorrect lexical admission.
2 open findings
What changed in this PR
Adds Chinese function-character filtering to stabilize lexical query admission.
Changes:
- Filters Chinese function unigrams and function-only bigrams.
- Adds regression and projection-preservation tests.
| File | Description |
|---|---|
src/powercontext/builtin/artifacts/search.py |
Adds Chinese query-term classification and filtering. |
tests/builtin/runtime/test_recall_sufficiency.py |
Covers Chinese admission, quoting, and content preservation. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
a8c72c5 to
59530d1
Compare
|
Rebased onto master ( Why the rebase: the previous head carried a red
The new head re-ran the full suite and reported 33 checks, none failing. Nothing about the fix itself changed: |
59530d1 to
2a0d178
Compare
|
Re-triggered: the head moved CI had finished on That is a subprocess-reaping timeout, and it is not from this branch:
I could not re-run the failed job: I am not a collaborator on this repository, and both |
2a0d178 to
c93c678
Compare
|
CI finished on
|
c93c678 to
902f5bf
Compare
|
Rolled again ( The same tree has now been through CI three times
Every job that has ever failed on this branch has also passed on this exact tree. No job fails deterministically for this content, so there is no commit-level fix that could make them pass — re-running is the only thing that changes the result. What each red actually is
ScopeNone of these paths is reachable from Since I have no write access here and both |
902f5bf to
d913ab5
Compare
|
Rebased onto current Rebased Verified locally on the new base, running the same commands CI runs:
|
| job | run 1 59530d16 |
run 2 2a0d178d |
run 3 c93c678f |
run 4 902f5bf8 |
|---|---|---|---|---|
controlled-cli |
pass | fail | fail | fail |
tests (3.12, unit) |
fail | pass | pass | fail |
tests (3.14, e2e) |
pass | pass | fail | — |
Acceptance (oceanbase) |
pass | fail | pass | — |
| remaining 5 test legs | pass | pass | pass | pass |
Net
The change itself is complete and verified. What is left is CI infra flakiness that also fires on master and on other contributors' PRs — controlled-cli needs a re-run here, not another commit. If it would help, I am happy to open a separate issue tracking the 0xC0000409 host crash and the artifact-ETag race.
|
@jasondeng1997 I have approved your PR. please fix the ci failed. and reply the copilot's review comment. |
d913ab5 to
207a63d
Compare
|
@AlexStocks thanks for the approval. Both requests are done. Review comments — all four answered in place, one per thread. They were substantive and two were real bugs in this PR, not style:
What Filtering is now per occurrence and decided by provenance rather than by the shape of the token. Measured (same probes the reviewers used):
Local verification before pushing: Red-tested: with On CI — the previous head |
207a63d to
2271a86
Compare
analyze_fts_query documents that unquoted function words cannot supply the sole overlap during a relaxed recall-gate round, but _QUERY_FUNCTION_WORDS is Latin-only, so that filter never applied to a Chinese query. Analyzer v1 does not segment Chinese: a run becomes one unigram per character plus one bigram per adjacent pair. Every function character a Chinese query adds joins the query own denominator, required_matches = ceil(len(query_terms) * 0.25), and can also join the numerator when the stored entry happens to contain that character. Against the entry text reported in the issue, on ad75ccf, "PowerContext Server 地址是什么？" has 11 terms, required 3 and matched 2, so it is rejected, while "的 PowerContext Server 地址是什么？" has 12 terms, required 3 and matched 3, so it is admitted. The leading 的 matched the 的 in the entry and crossed a threshold the query itself had raised, which is why one extra character flipped the result. _QUERY_FUNCTION_CHARACTERS mirrors the categories of _QUERY_FUNCTION_WORDS and also covers the traditional writing of the same characters, because Analyzer v1 treats every Han ideograph alike and codepoints are what reach the filter: 麼 and 么 are different characters, so a simplified-only set would leave a traditional query unguarded. Negations are absent for the same reason they are absent in the English list. Characters that also carry content stay out, because a lone character cannot distinguish 地址, 地方, 得到 and 取得 from the particle 地, and 有, 会, 能, 要 and 帮 appear in 有效, 会议, 功能, 要求 and 帮助; 著 is left out on the same principle, being also the verb in 著名 and 著作. Filtering is per occurrence and decided by provenance rather than by the shape of the token. A u_/b_ term counts as Chinese only when its source span opens on a CJK character, because flush_cjk emits those terms over CJK runs while flush_word emits verbatim terms over alphanumeric runs. A literal identifier such as u_7684 therefore keeps its documented whole-term behavior, while the same string generated from 的 is filtered. A bigram counts as a function word only when both of its characters are one. Several listed characters also carry content (为 in 行为, 向 in 方向, 并 in 并发), and a bigram is the only evidence a two-character compound has, so retiring every pair that merely touches a listed character removes the evidence rather than the function word. Measured with real SQLite Memory search, 行为, 方向 and 并发 each lost their pair and stopped being returned for the exact phrase; under this rule all three are returned again. The change is query-side only. analyze_text is untouched, so persisted Analyzer v1 projections keep every term, and no threshold, constant or fusion input changes. The short question from the report is admitted, the long form this pull request documents as rejected stays rejected, and appending a pure function word to a query leaves the threshold unchanged in both writings. The boundary the pair rule leaves is deliberate. An inserted function character beside content can still contribute one mixed bigram, so 的 raises required_matches where the characters around it carry content: "alpha 项目地址" keeps 8 terms and needs 2, while "alpha 的项目地址" keeps 9 and needs 3. Closing that would need the segmentation Analyzer v1 does not have, and the attempt is what cost the three compounds above. Tested: pytest tests/builtin/runtime/test_recall_sufficiency.py -q (109 passed) and together with tests/e2e/test_recall_sufficiency_gate.py (146 passed); pytest tests/builtin/artifacts/experience/test_search.py tests/builtin/artifacts/topic_memory/test_search.py tests/builtin/persistence/test_topic_memory.py tests/builtin/persistence/test_topic_memory_integrity.py -q (126 passed, 26 skipped); ruff format --check, ruff check and ty check on both changed files. With search.py restored to ad75ccf, 27 of these tests fail; with it restored to the previous commit of this branch, 6 fail, including every assertion added for the compounds above. Refs oceanbase#1912
2271a86 to
317d072
Compare
|
@AlexStocks Both of your requests are addressed, and this revision also fixes a regression my own previous commit introduced. CI — the previous head went fully green (36 success / 1 skipped / 0 failed) once rebased onto Copilot's comments — the original four threads were answered on the previous revision; the two still unresolved are answered now:
One boundary remains, and it is now documented rather than claimed away: a function character inserted next to content still contributes one mixed bigram, so Verification on The two review threads are answered but still show as unresolved — that toggle needs a maintainer. |
| if term.startswith("u_"): | ||
| return _analyzed_character(term[2:]) in _QUERY_FUNCTION_CHARACTERS |
|
Let's proceed with #1979 as the main implementation for word analysis and query filtering, and close #1969 for now to avoid maintaining overlapping approaches. Please carry the regression cases from this PR into #1979's validation, including the reported Chinese query variants, literal analyzer-shaped identifiers, and content compounds such as 行为, 方向 and 并发. Closing this PR does not mean those cases are already verified in #1979. |
Closes #1912
Problem
analyze_fts_queryis documented to stop unquoted function words from supplying the sole overlapduring a relaxed recall-gate round, but
_QUERY_FUNCTION_WORDSis Latin-only, so that filter hasnever applied to a Chinese query. Analyzer v1 does not segment Chinese: a run becomes one unigram
per character plus one bigram per adjacent pair. Every function character a Chinese query adds
therefore
required_matches = ceil(len(query_terms) * 0.25), andBoth halves are reproducible for the pair in the report. Against the reported entry text
(
；and？fullwidth here), onad75ccf2:PowerContext Server 地址PowerContext Server 地址是什么？的 PowerContext Server 地址是什么？The third query was admitted because the leading
的matched the的in the entry, crossing athreshold the query itself had raised. The reported "one extra character makes it match" is
deterministic, not random: the query moves its own admission bar.
Fix
Extend the mechanism that already exists rather than adding a second one.
_QUERY_FUNCTION_CHARACTERSmirrors the categories of_QUERY_FUNCTION_WORDS: structural andaspectual particles, the copula, coverbs, conjunctions and adverbs, demonstrative and personal
pronouns, interrogatives and sentence particles, and politeness markers. Negations are absent,
for the same reason they are absent from the English list. The set also covers the traditional
writing of the same characters, because Analyzer v1 treats every Han ideograph alike and the
codepoint is what reaches the filter:
麼and么are different characters, so asimplified-only set would leave a traditional query unguarded.
_is_query_function_termapplies it to Analyzer v1 terms. A unigram is a function word exactlywhen its character is. A bigram is one only when both of its characters are, because a pair
that carries one content character still carries evidence.
u_/b_term counts as Chinese only when its source span opens on a CJK character, becauseflush_cjkemits those terms over CJK runs whileflush_wordemits verbatim terms overalphanumeric runs. A literal identifier such as
u_7684therefore keeps its documentedwhole-term behavior, while the same string generated from
的is filtered.地and得(地址,地方, 得到, 取得) and
有,会,能,要,帮(有效, 会议, 功能, 要求, 帮助) — because a lonecharacter cannot tell the two uses apart.
著is left out on the same principle, being also theverb in 著名 and 著作.
The same queries after the change:
PowerContext Server 地址PowerContext Server 地址是什么？的 PowerContext Server 地址是什么？All six pure-function suffixes covered by the tests (
的,了,吗,我的,是的,它们的)leave the term count and the threshold identical to the base query, in both writings.
No threshold, constant or persisted projection changes.
analyze_textis untouched, so storedAnalyzer v1 projections keep every term; the filter stays query-side, as the module already
documents.
Scope
still raises
required_matchesexactly as before:请告诉我 PowerContext Server 地址是什么？(11 terms, required 3, matched 2) and
我之前让你记住的 PowerContext Server 地址是什么？(14 terms, required 4, matched 2) both remain rejected, as they are on
ad75ccf2. Those casesare about the 0.25 coverage rule, not about function words.
The pair rule leaves one boundary, and it is deliberate rather than overlooked. A function
character inserted next to content still contributes one mixed bigram to the denominator, so
的raisesrequired_matcheswhere the characters around it carry content:alpha 项目地址keeps 8 terms and needs 2, while
alpha 的项目地址keeps 9 and needs 3. Retiring mixed pairs aswell is exactly what removes the only evidence a two-character compound has — with that rule
行为,方向and并发each fall to a single term (u_884c,u_65b9,u_5e76) and an exactphrase query against an entry containing the phrase is refused. Closing the boundary needs the
segmentation Analyzer v1 does not have, so it is documented and pinned by
test_mixed_boundary_bigram_keeps_one_pair_and_pins_the_shiftinstead of traded for a recallfailure on real compounds.
Tests
tests/builtin/runtime/test_recall_sufficiency.py:test_reported_chinese_function_word_prefix_admits_consistently— the reported pair, on thereported entry text.
test_chinese_function_words_do_not_raise_the_admission_bar— six pure-function suffixes mustleave
fts_query_requirementsunchanged.test_chinese_content_characters_stay_query_evidence— seven content compounds keep theirunigrams and bigrams.
test_chinese_bigram_drops_only_when_both_characters_are_function_words—址是is evidence,是什/什么are not.test_content_compound_that_contains_a_listed_character_stays_recallable—行为,方向and并发stay recallable for the exact phrase.test_mixed_boundary_bigram_keeps_one_pair_and_pins_the_shift— the boundary documented above.test_polite_long_query_keeps_the_documented_coverage_rejection— the Scope case stays rejectedwhile the shorter question is admitted.
test_traditional_chinese_function_characters_share_the_simplified_threshold— sixteen pairs.test_literal_analyzer_shaped_identifier_stays_query_evidenceandtest_literal_identifier_survives_a_query_full_of_chinese_function_words— a literalu_/b_identifier keeps whole-term behavior even beside generated function tokens.
test_quoted_chinese_function_text_stays_query_evidence— quoting still opts a function word in.test_query_normalization_leaves_the_chinese_projection_unchanged— the persisted projectionkeeps every term.
Validation
pytest tests/builtin/runtime/test_recall_sufficiency.py -q— 109 passedpytest tests/builtin/runtime/test_recall_sufficiency.py tests/e2e/test_recall_sufficiency_gate.py -q— 146 passed
pytest tests/builtin/artifacts/experience/test_search.py tests/builtin/artifacts/topic_memory/test_search.py tests/builtin/persistence/test_topic_memory.py tests/builtin/persistence/test_topic_memory_integrity.py -q— 126 passed, 26 skipped
ruff format --check,ruff check,ty checkon both changed files — cleanRed baseline: with
src/powercontext/builtin/artifacts/search.pyrestored toad75ccf2, 27tests fail. With it restored to the previous commit of this branch (which classified a bigram as
functional when either character was), 6 fail — every assertion added for the compounds and
for the mixed-pair boundary above, plus the Scope rejection.
Refs #1912