fix(api): preserve a zero annotation-reply score threshold - #42640
Open
JessYanCoding wants to merge 1 commit into
Open
JessYanCoding wants to merge 1 commit into
JessYanCoding wants to merge 1 commit into
Conversation
`AnnotationReplyFeature.query` read the app's configured similarity threshold as
`enabled_config["score_threshold"] or 1`. That is not a null guard: the key is
declared `score_threshold: float` on the `AnnotationReplyEnabledConfig` TypedDict,
`AppAnnotationSetting.score_threshold` is `Float, nullable=False`, and
`enable_annotation_reply_task` always writes the value the user submitted. The only
input the `or` can ever intercept is the legitimate value `0.0`.
`0.0` is the loosest setting the feature offers — the config slider runs 0 to 100
and sends `val / 100`, labelling its left stop "0.0 / Easy Match". Coercing it to
`1` applies a threshold stricter than the slider's own right-hand stop, so an app
configured for the loosest matching stops replying with annotations altogether,
which is the opposite of what was asked for.
Every sibling threshold path already treats `None` as the disabled sentinel and
keeps `0.0` as an active value — `dataset_retrieval` uses
`retrieval_model.get("score_threshold", 0.0)` and guards with
`score_threshold is None or ... >= score_threshold`, and the easy-UI dataset config
manager checks `score_threshold_val is not None`. The annotation path was the only
one coercing.
The existing test encoded the bug: it persisted `score_threshold=0` and asserted
the vector search received `score_threshold=1`. That assertion is corrected, and a
parametrized regression test now pins both `0.0` and a non-zero threshold to the
value that reaches `search_by_vector`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JessYanCoding
requested review from
QuantumGhost and
laipz8200
as code owners
September 21, 2026 06:47
Contributor
Pyrefly Type Coverage
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #42640 +/- ##
========================================
Coverage 87.57% 87.57%
========================================
Files 5467 5467
Lines 317685 317820 +135
Branches 63688 63718 +30
========================================
+ Hits 278208 278329 +121
- Misses 34112 34118 +6
- Partials 5365 5373 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Fixes #42639.
AnnotationReplyFeature.queryread the app's configured similarity threshold as:That is not a null guard. The key is declared
score_threshold: floatonAnnotationReplyEnabledConfig(api/models/model.py:127),AppAnnotationSetting.score_thresholdisFloat, nullable=False(:1997),load_annotation_reply_configpopulates it unconditionally (:2036), andenable_annotation_reply_taskwrites the submitted value verbatim (api/tasks/annotation/enable_annotation_reply_task.py:85,93). The only input theorcan ever intercept is the legitimate value0.0.0.0is the loosest setting the feature offers — the config modal sendsval / 100from a 0-100 slider whose left stop is labelled0.0 / Easy Match. Coercing it to1applies a threshold stricter than the slider's own right-hand Accurate Match stop, so an app configured for the loosest matching stops replying with annotations entirely.Why this is the odd one out
Every sibling threshold path treats
Noneas the disabled sentinel and keeps0.0as an active value:core/rag/retrieval/dataset_retrieval.py:751,:1208retrieval_model.get("score_threshold", 0.0)core/rag/retrieval/dataset_retrieval.py:1490if score_threshold is None or ... >= score_thresholdcore/app/app_config/easy_ui_based_app/dataset/manager.py:108-121if score_threshold_val is not Nonecore/tools/utils/dataset_retriever/dataset_retriever_tool.py:151.get("score_threshold", 0.0)The annotation path was the only one coercing.
The existing test encoded the bug
tests/unit_tests/core/app/features/test_annotation_reply.pypersistedscore_threshold=0at line 102 and then asserted the vector search receivedscore_threshold=1at line 123. That assertion is corrected here, which is also why reverting only the test file leaves the suite red:A parametrized regression test now pins both
0.0and a non-zero threshold to the value that reachessearch_by_vector.Verification
On
22aff66, from the repo root:pytest api/tests/unit_tests/core/app/features/test_annotation_reply.py— 7 passed (5 onmain, so +2)pytest api/tests/unit_tests/core/app/features/ api/tests/unit_tests/controllers/service_api/app/test_annotation.py api/tests/unit_tests/tasks— 392 passedpytest api/tests/unit_tests— 19502 passed, 6 skipped. The single failure,libs/test_zip_filename_recovery.py::test_zip64_extensible_data_is_not_part_of_central_directory, reproduces on a clean checkout of the same commit and is unrelated.or 1with the new tests in place — 2 failed (the[0.0]case and the corrected assertion); the[0.5]case still passes, so the test is specific to the zero pathruff format --check/ruff checkon./api— cleanpyrefly checkandmypy --check-untyped-defson the changed source — cleanpyrefly --config=tests/unit_tests/pyrefly.tomlon the changed test file — 5 diagnostics before and after, so no new strict-mode debtapi/uv.lockunchangedScreenshots
N/A (backend-only fix).
Checklist
make lint && make type-check(backend) andvp staged(frontend) to appease the lint godsFrom Claude Code