Repository navigation
Conversation
Aggregate IVF batch query counters, account for bucket rerank evaluations, and publish parallel search legacy counters. Add minimal regression coverage. Signed-off-by: LHT129 <tianlan.lht@antgroup.com> Assisted-by: DeepSeekHarness:gpt-6.1-sol
LHT129
added
kind/bug
Merger | Merge ReadinessHead: Updated: 2026-11-04 03:05:40 UTC Status: BLOCKED Merger decision: HOLD (shadow assessment; no merge executed).
Next owner: Author. Required next step: Resolve the GitHub merge conditions, then reassess. Merge reliability: 0.00 / 1.00 (WITHHELD)
Evidence subtotal: 0.80; applicable ceiling: 0.00. Final score = min(subtotal, ceiling). Policy:
AI review: COMPLETE. Static review: patch is correct; CI is reported separately. Review findings: 0; P0/P1 findings in that review: 0; coverage: 7/7 changed files. Findings are not a count of currently unresolved threads. CI summary: 1 neutral, 15 skipped, 21 success. Check source: Next action: Merge commit freshness: Waiting for GitHub to refresh the merge commit for the current head and base. Only current head checks are shown.; Human review: GitHub: REVIEW_REQUIRED; approvals >= 1; Code Owner approval required; stale approvals dismissed; Conversations: 2 unresolved; resolution required; GitHub merge state: BLOCKED CI check details
This is an uncalibrated evidence score, not a probability, correctness guarantee or approval. Zero means the assessment is withheld or lacks positive evidence, not a proven-bad patch. Automatic merging is disabled. New commits or base changes invalidate earlier evidence; never authorize a merge from this score alone. |
Reviewer | Review completeAutomated pull request review completed. Model: No actionable inline findings were found. |
vsag-bot
added
area/testing
|
Jev-driven classification Applied: Option probabilities
Model: |
Merge Protections🟢 All 3 merge protections satisfied — ready to merge. Show 3 satisfied protections🟢 Require kind label
🟢 Require version label
🟢 Require linked issue for feature/bug PRs
|
mergify
Bot
added
the
module/datacell
vsag-bot
left a comment
vsag-bot
left a comment
There was a problem hiding this comment.
Reviewer | Code review
Automated pull request review completed.
Model: gpt-6.1-sol; reasoning effort: medium.
Review scope: medium (258 changed lines across 7 files).
No actionable inline findings were found.
Review coverage checks completed; this does not prove absence of defects.
Static review only; compilation and runtime tests are outside this review's scope.
File coverage: 7/7; observed passing test runs: 0.
Reviewed commit d01ee87.
|
Validation progress for d01ee87: the fresh Circle-equivalent build completed all 744 build steps. The full non-HGraph functional group (filter |
vsag-lgtm
left a comment
vsag-lgtm
left a comment
There was a problem hiding this comment.
Automated review @ d01ee873 — 2 inline suggestion(s).
|
|
||
| computer->ScanBatchDists(unique_offsets.size(), codes.data, unique_dists.data()); | ||
| if (ctx != nullptr and ctx->stats != nullptr and ctx->track_distance_evaluations) { | ||
| ctx->stats->AddDistance(ctx->distance_phase, backend_, unique_offsets.size()); |
There was a problem hiding this comment.
[warning] Needs confirmation: the new distance tracking is skipped for the PQFS early-return path above.
BucketDataCell::Query returns early for QUANTIZATION_TYPE_VALUE_PQFS and falls back to a loop of QueryOneById, which does not record distance evaluations. As a result, buckets backed by PQFastScan will not populate distance_evaluations or the RERANK phase/backend counters under BucketReorder, leaving the statistics incomplete for that backend. If PQFS reranking is a supported configuration, consider tracking those evaluations in the PQFS branch as well.
| } | ||
| one_request.params_str_ = json.Dump(); | ||
| auto one_result = this->SearchWithRequest(one_request); | ||
| const auto one_stats = JsonType::Parse(one_result->GetStatistics()); |
There was a problem hiding this comment.
[suggestion] Aggregating batch statistics by serializing and reparsing JSON for each sub-query adds measurable overhead.
Each sub-query already builds a SearchStatistics object and serializes it via stats.Dump(); parsing it back with JsonType::Parse introduces per-query allocation and parsing costs that grow with batch size. Consider carrying the SearchStatistics (or a lightweight snapshot) through the result object so the batch path can merge counters directly without round-tripping through JSON.
|
Circle-equivalent make test_parallel has now finished: the original script reports success for all three complete groups (unit, [hgraph], and other), without dropping tests beyond its maintained ~[daily] filter. Both functional groups passed: 191 cases / 1,246,671 assertions and 290 cases / 8,767,632 assertions. Full logs are retained; this run uses the previously disclosed GCC15 warning compatibility flags and system OpenBLAS configuration. This successful configuration does not erase the separately disclosed Release baseline numeric failures. Python wheel/clean-environment pytest is still pending. |
Change Type
Linked Issue
Fixes: #3249
What Changed
dist_cmp/hopsafter worker completion without duplicating newer distance accounting.Test Evidence
make fmtwith clang-format 20.1.8, committed-tree format check,git diff --checkmake lintmake testmake cov, run tests, and collect coverageEnvironment: Ubuntu 26.04, GCC15.2, Release. Unrelated baseline GCC15 diagnostics required
-Wno-error=template-id-cdtor -Wno-error=stringop-overflow; this is not a claim of supported-toolchain CI validation.TSan: independent build of the same commit via
make tsan COMPILE_JOBS=8passed. With the maintained four suppressions andOPENBLAS_NUM_THREADS=1,TSAN_OPTIONS=suppressions=omp.supp ./build/tests/functests "[concurrent]~[daily]"passed: 2,251,758 assertions in 28 cases, no ThreadSanitizer report.Circle-equivalent
make test_parallelcompleted with exit 0 on the same commit: fresh build 744/744; full unit group 1,403 passed / 1 native CUDA-unavailable skip (1,404 cases), all 86,768,517 assertions passed;[hgraph]~[daily]191 cases / 1,246,671 assertions passed;~[hgraph]~[daily]290 cases / 8,767,632 assertions passed. Original filters were unchanged. Configuration: Ninja, MKL OFF, system OpenBLAS 0.3.32 ON, COMPILE_JOBS=4, ASAN/Sanitize OFF, CCACHE OFF, and the disclosed GCC15 warning flags. Normalize and SINDI FP16 tests passed in this configuration; the separately recorded Release baseline failures remain disclosed.Python 3.10.22: a fresh wheel built successfully with the disclosed GCC15 warning compatibility flags and installed into a separate clean environment. However, the required clean-environment pytest did not pass: import fails with
ImportError: libvsag.so.0.11: cannot open shared object file: No such file or directory(pytest exit 4). ELF inspection finds the extension RUNPATH points to a cleaned build directory rather than the installed bundled library. With an explicit task-onlyLD_LIBRARY_PATHpointing to the wheel's installedpyvsagdirectory, import alone succeeds, but the standard pytest command crashes during import (exit 139); importing NumPy before pyvsag reproduces this crash. Additional diagnostic run with the explicit library path andimport pyvsagbefore invokingpytest.mainpassed all 137 tests (3 warnings, 18.68s, exit 0). This import-order workaround is not a substitute for the required standard command. The original strict GCC15 build failure and all subsequent failure logs are retained.make pyvsagreturning 0 is not treated as pytest success. These packaging/runtime failures have not been established as regressions from this statistics-only change and are not silently fixed in this PR.Submission exceptions explicitly approved by the requester: open the PR with the known baseline failures, GCC15 toolchain deviation and then-pending Python/Circle checks disclosed, and report their actual outcomes afterward. Circle has now passed as described above; Python remains unsuccessful as described above. macOS arm64 ASan was separately explicitly deferred. No unsuccessful or deferred check is claimed as passed.
Compatibility Impact
Performance and Concurrency Impact
Documentation Impact
Risk and Rollback
Checklist