Skip to content

enhance: add boost score support#50372

Open
junjiejiangjjj wants to merge 1 commit into
milvus-io:masterfrom
junjiejiangjjj:go-boost
Open

enhance: add boost score support#50372
junjiejiangjjj wants to merge 1 commit into
milvus-io:masterfrom
junjiejiangjjj:go-boost

Conversation

@junjiejiangjjj

Copy link
Copy Markdown
Contributor

Implement boost score evaluation for the Go search reduce pipeline, including
QueryNode task integration, segcore C API bindings, and score expression
combination support.

Add boost score runner logic in core, expose segment-level boost scoring to Go,
and cover the new behavior with C++, Go, and Python client tests.

@sre-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: junjiejiangjjj
To complete the pull request process, please assign wxyucs after the PR has been reviewed.
You can assign the PR to them by writing /assign @wxyucs in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot requested review from aoiasd and congqixia June 8, 2026 08:05
@sre-ci-robot sre-ci-robot added size/XXL Denotes a PR that changes 1000+ lines. area/compilation area/test sig/testing labels Jun 8, 2026
@mergify mergify Bot added dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement labels Jun 8, 2026
@mergify

mergify Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@junjiejiangjjj Please associate the related issue to the body of your Pull Request. (eg. "issue: #")

@sre-ci-robot

Copy link
Copy Markdown
Contributor

[ci-v2-notice]
Notice: New ci-v2 system is enabled for this PR.

To rerun ci-v2 checks, comment with:

  • /ci-rerun-code-check // for ci-v2/code-check
  • /ci-rerun-code-check-macos // for Code Checker MacOS (GitHub Actions)
  • /ci-rerun-build // for ci-v2/build
  • /ci-rerun-build-all // for ci-v2/build-all (multi-arch builds)
  • /ci-rerun-buildenv // for ci-v2/build-env (build milvus-env builder images)
  • /ci-rerun-ut-integration // for ci-v2/ut-integration, will rerun ci-v2/build
  • /ci-rerun-ut-go // for ci-v2/ut-go, will rerun ci-v2/build
  • /ci-rerun-ut-cpp // for ci-v2/ut-cpp
  • /ci-rerun-ut // for all ci-v2/ut-integration, ci-v2/ut-go, ci-v2/ut-cpp, will rerun ci-v2/build
  • /ci-rerun-e2e-default // for ci-v2/e2e-default
  • /ci-rerun-e2e-amd // for ci-v2/e2e-amd (e2e pool dispatcher)
  • /ci-rerun-build-ut-cov // for ci-v2/build-ut-cov (build + unit tests in one pipeline)
  • /ci-rerun-gosdk // for ci-v2/go-sdk (Go SDK E2E tests, ARM)

If you have any questions or requests, please contact @zhikunyao.

@junjiejiangjjj junjiejiangjjj marked this pull request as draft June 8, 2026 08:14
@sre-ci-robot sre-ci-robot added the do-not-merge/work-in-progress Don't merge even CI passed. label Jun 8, 2026
@sre-ci-robot

Copy link
Copy Markdown
Contributor

❌ CI Loop Results 8a5beda

Stage Result Duration Tests

Total: 9min | Pipeline | Artifacts

}
}

if len(values) == 0 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR changes score_combine's null handling in processChunk: previously any null input nulled the whole row; now null inputs are skipped and the row is only nulled when every input is null. ScoreCombineExpr is shared with the pre-existing decay reranker, which combines $score with the decay factor via NewScoreCombineExpr(ModeMultiply) (rerank_builder.go:512) and where DecayExpr emits a null factor for null field values (decay_expr.go:259). As a result, decay over a nullable numeric field now returns the raw $score (decay effectively ignored) where it previously produced a null score, silently changing decay ranking. Scope the null-skip to the boost path, or add decay-on-nullable coverage to confirm the change is intended.

@junjiejiangjjj junjiejiangjjj marked this pull request as ready for review June 9, 2026 12:17
@sre-ci-robot sre-ci-robot removed the do-not-merge/work-in-progress Don't merge even CI passed. label Jun 9, 2026
// Both null — use tie-break if available
less, useTypedLess := makeArrayRowLess(sortChunk, tbChunk, o.desc)
if useTypedLess {
sort.Slice(indices, func(i, j int) bool {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new typed fast path sorts with unstable sort.Slice, whereas the pre-PR code used a single sort.SliceStable, and its comparator only tie-breaks on $id, not $element_indices/$offset. In element-level search one entity can produce multiple rows that share $id and an equal boosted $score; the unstable sort no longer preserves their input order, so which element/offset survives the downstream merge can change relative to the prior stable behavior. (Go's sort.Slice is deterministic for identical input, so this is a loss of the stable-ordering guarantee rather than true run-to-run randomness.) Use sort.SliceStable, or extend the comparator with a full tie-break on $element_indices/$offset.

type boostScoreFunc func(context.Context, segments.Segment, *segcore.SearchRequest, *planpb.ScoreFunction, *arrow.Chunked) (*arrow.Chunked, error)

func newSegmentBoostScoreRunner(scoreFunc boostScoreFunc, segment segments.Segment, searchReq *segcore.SearchRequest, scorer *planpb.ScoreFunction) expr.BoostScoreRunner {
return func(ctx context.Context, offsets *arrow.Chunked) (*arrow.Chunked, error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The single-segment boost path scores synchronously through a C ABI that has no cancellation token, so threading a context here can't actually interrupt the in-flight work — the dropped context is a symptom, not the root cause. A caller that cancels or hits its deadline (e.g. client disconnect) still blocks until the single-segment boost completes; the durable fix is to route single-segment boost through the async API. Low severity since the topK-bounded workload keeps the uninterruptible window short.

@sre-ci-robot

Copy link
Copy Markdown
Contributor

✅ CI Loop Results ffd789b

Stage Result Duration Tests
✅ Build SUCCESS 10.3min -
✅ Code-Check SUCCESS 8.1min -
✅ UT-GO SUCCESS 22.8min 1020 passed
✅ UT-Integration SUCCESS 24.3min 46 passed
✅ UT-CPP-Cov SUCCESS 36.7min 7841 passed

Total: 80min | Pipeline | Artifacts

Overall Coverage: 71.2%
Diff Coverage: CPP 68.3% (125 hit, 58 miss, 183 measurable lines, 779 unmeasured) | Go 53.5% (285 hit, 248 miss, 533 measurable lines, 401 unmeasured)
Diff Coverage HTML: view changed lines
Total Patch Coverage: 57.3% (410/716 measurable lines, 1180 unmeasured)

@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.42984% with 313 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.90%. Comparing base (d10a9f5) to head (ffd789b).
⚠️ Report is 6 commits behind head on master.

⚠️ Current head ffd789b differs from pull request most recent head 8a7386d

Please upload reports for the commit 8a7386d to get more accurate results.

Files with missing lines Patch % Lines
internal/util/segcore/boost_score.go 0.00% 166 Missing ⚠️
internal/querynodev2/tasks/boost_score.go 67.94% 36 Missing and 14 partials ⚠️
internal/core/src/rescores/BoostScoreRunner.cpp 25.00% 39 Missing ⚠️
...nal/util/function/chain/expr/score_combine_expr.go 76.25% 17 Missing and 2 partials ⚠️
...ternal/querynodev2/segments/segment_boost_score.go 25.00% 18 Missing ⚠️
...ernal/util/function/chain/expr/boost_score_expr.go 87.75% 4 Missing and 2 partials ⚠️
internal/util/function/chain/operator_sort.go 86.36% 4 Missing and 2 partials ⚠️
internal/core/src/segcore/boost_score.cpp 95.74% 4 Missing ⚠️
internal/querynodev2/tasks/search_task.go 0.00% 2 Missing and 1 partial ⚠️
internal/core/src/exec/operator/RescoresNode.cpp 0.00% 2 Missing ⚠️

❌ Your patch check has failed because the patch coverage (60.42%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #50372      +/-   ##
==========================================
- Coverage   78.95%   78.90%   -0.06%     
==========================================
  Files        2239     2246       +7     
  Lines      396935   397635     +700     
==========================================
+ Hits       313401   313740     +339     
- Misses      73947    74293     +346     
- Partials     9587     9602      +15     
Components Coverage Δ
Client 80.36% <ø> (ø)
Core 85.84% <83.08%> (-0.10%) ⬇️
Go 76.79% <48.95%> (-0.04%) ⬇️
Files with missing lines Coverage Δ
internal/core/src/query/PlanProto.cpp 61.21% <ø> (-0.92%) ⬇️
internal/core/src/rescores/Utils.h 0.00% <ø> (-38.47%) ⬇️
internal/core/unittest/test_boost_score_c.cpp 100.00% <100.00%> (ø)
internal/core/unittest/test_exec.cpp 100.00% <100.00%> (ø)
internal/util/function/chain/operator_map.go 91.86% <100.00%> (+0.29%) ⬆️
internal/util/function/chain/rerank_builder.go 91.13% <100.00%> (ø)
internal/core/src/exec/operator/RescoresNode.cpp 0.00% <0.00%> (-66.27%) ⬇️
internal/querynodev2/tasks/search_task.go 88.57% <0.00%> (-0.86%) ⬇️
internal/core/src/segcore/boost_score.cpp 95.74% <95.74%> (ø)
...ernal/util/function/chain/expr/boost_score_expr.go 87.75% <87.75%> (ø)
... and 6 more

... and 47 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sre-ci-robot sre-ci-robot added the low-code-coverage add test-label from zhikun, diff coverage > 80% label Jun 9, 2026
  Implement boost score evaluation for the Go search reduce pipeline, including
  QueryNode task integration, segcore C API bindings, and score expression
  combination support.

  Add boost score runner logic in core, expose segment-level boost scoring to Go,
  and cover the new behavior with C++, Go, and Python client tests.

Signed-off-by: junjie.jiang <junjie.jiang@zilliz.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/compilation area/test dco-passed DCO check passed. do-not-merge/missing-related-issue kind/enhancement Issues or changes related to enhancement low-code-coverage add test-label from zhikun, diff coverage > 80% sig/testing size/XXL Denotes a PR that changes 1000+ lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants