Repository navigation
fix(csv-core): NFA read_record mishandles records split across calls - #438
Open
prostomarkeloff wants to merge 1 commit into
Open
prostomarkeloff wants to merge 1 commit into
prostomarkeloff wants to merge 1 commit into
Conversation
read_record_nfa seeded nout with self.output_pos and returned the accumulated total, writing into an already-offset output slice. When a record spans multiple read_record calls the fields come out shifted (e.g. ["ab","c\0","\0\0"] instead of ["ab","cd","ef"]). Follow the DFA convention: nout counts bytes written this call, ends are self.output_pos + nout, and output_pos accumulates.
prostomarkeloff
force-pushed
the
fix-nfa-read-record-output-pos
branch
from
October 23, 2026 16:19
ac7b6d6 to
d5fd5cd
Compare
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.
Found with find-dup-defs using its
convergepass, which reports behavioural divergences between near-duplicate functions — hereread_record_dfaandread_record_nfa, which seednoutdifferently (0vsself.output_pos).read_record_nfastartsnoutatself.output_posand returns the running total, whileread_record_dfastartsnoutat0(bytes written this call) and recordsends[i] = self.output_pos + nout. When a record is delivered across severalread_recordcalls — the normal streaming case where the caller passesout[outpos..]/ends[endpos..]— the NFA writes at the wrong offset and reports wrong end positions.Single-call inputs (the existing tests) hide this because
output_posstays0throughout.ReaderBuilder::nfais a#[doc(hidden)]debug option, but it's the reference the DFA is diffed against, so it should agree with the DFA.Example, feeding
ab,cthend,ef\n:Fix
Bring the NFA to the DFA's convention:
noutcounts bytes written this call,ends[nend] = self.output_pos + nout, andoutput_posaccumulates (+= nout) rather than being used as the write offset.Tests
Added a test that feeds one record in two chunks and asserts NFA output equals DFA output.
cargo test -p csv-coreis green.Disclosure: found and prepared with the help of an AI-assisted tool; I have reviewed the change and the test.