Skip to content

fix: take merge_offset into account when needle fits in a single chunk - #32

Merged
TomAFrench merged 1 commit into
mainfrom
ab/bug-merge-inital-and-final-chunks
Jul 24, 2026
Merged

fix: take merge_offset into account when needle fits in a single chunk#32
TomAFrench merged 1 commit into
mainfrom
ab/bug-merge-inital-and-final-chunks

Conversation

@asterite

Copy link
Copy Markdown
Contributor

Problem Resolved

Resolves https://github.com/noir-lang/noir-library-claude/issues/17

Summary of Changes

When a needle's length is less than the chunk's size (31) there's a final part of the code that copies from needle and haystck according to where inside the chunk we are. The logic worked when the needle matched exactly the beginning of the chunk but it didn't take into account that the needle be shifted with respect to the chunk start.

Son when comparing "ABCDEFGHIJKLMNOPQRSTUVWXYZ" vs. "BCE" it copied A, then BC but then incorrectly copied D instead of E (because of this offset mismatch) and then continued copying from haystack. Then the strings matches, incorrectly. When taking the merge offset into account we get "ABCEEFG..." so they don't match.

Note in the new code that when merge_offset is 0, we get let predicate = (i >= 0) & (lhs_index < substring_length), which is what the old code was doing (i >= 0 is always true) but when taking the offset into account we get a shift both in the current chunk's offset (i) as well as in the needle offset (lhs_index, the bytes we already copied from the needle).

PR Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@asterite
asterite requested a review from TomAFrench July 23, 2026 15:17
@TomAFrench
TomAFrench merged commit deef741 into main Jul 24, 2026
21 of 23 checks passed
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.

2 participants