Skip to content

fix: return nearest element from binarySearchNearest instead of window edge - #843

Open
NemeZZiZZ wants to merge 1 commit into
klinecharts:mainfrom
NemeZZiZZ:fix/binary-search-nearest
Open

fix: return nearest element from binarySearchNearest instead of window edge#843
NemeZZiZZ wants to merge 1 commit into
klinecharts:mainfrom
NemeZZiZZ:fix/binary-search-nearest

Conversation

@NemeZZiZZ

Copy link
Copy Markdown
Contributor

Problem

binarySearchNearest narrows the window to ≤ 2 (up to three candidates: left, left+1, right) and then returns left unconditionally — the window's left edge, not the nearest element:

if (mid <= 2) { break }
}
return left

Verified by execution: [10, 20, 30, 40, 50], target 39 → returns index 2 (value 30) instead of 3 (value 40); target 100 → index 3 instead of 4.

Consumers: Chart.scrollToTimestamp (Chart.ts:1172), Chart.zoomAtTimestamp (Chart.ts:1202), Store.timestampToDataIndex (Store.ts:1061) — all land 1–2 bars off the requested timestamp.

Fix

After the loop, compare the remaining candidates in [left, right] (at most three) and return the one with the smallest distance to the target. Ties resolve to the lower index.

Verification

Fuzz against brute-force nearest, 5000 random strictly-increasing arrays (lengths 1–300, targets in/around/beyond range): 5000/5000 match, ties to the lower index. The audit examples: 39 → 3, 100 → 4, 21 → 1, exact hit 30 → 2.

Note: on arrays with duplicate keys the result is still an index at the minimal distance, but among ties spanning more than the final window any tied index may be returned (pre-existing behavior; irrelevant for bar timestamps, which are unique).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant