Skip to content

fix(csv-parse): honor normalized objname in result collection - #523

Merged
wdavidw merged 2 commits into
adaltas:masterfrom
lux-liang:fix/parse-objname-normalization
Oct 9, 2026
Merged

wdavidw merged 2 commits into
adaltas:masterfrom
lux-liang:fix/parse-objname-normalization

Conversation

@lux-liang

@lux-liang lux-liang commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

The sync collector treats objname: 0 as disabled when choosing its result container, returning an array whose string-keyed records serialize to []. The callback collector treats objname: null or false as enabled even though the parser normalizes them to disabled, losing record fields.

Use the parser's normalized objname in both collectors. Keyed results retain their null prototype; disabled mode returns complete record arrays. Three added regression cases cover the sync zero index and the two disabled callback values. Existing tests cover the other keyed-record behavior.

Validation on native Windows Node.js 24.19.0:

  • All 15 focused JS/TS tests pass. The three retained regressions fail against the exact original source.
  • Original build, lint and csv-parse TypeScript checks pass.
  • The original package test command encounters the unchanged Windows sample-loader path error, also verified on the original base. Supplemental coverage of all 89 non-sample test files passes: 549 tests, 3 existing pending.
  • The original hosted Test workflow passes on the latest commit, dad8ff7, with Node.js 20, 22 and 24.

AI assistance was used for implementation, testing and review.

@lux-liang
lux-liang marked this pull request as ready for review October 9, 2026 03:38
@wdavidw

wdavidw commented Oct 9, 2026

Copy link
Copy Markdown
Member

Please limit the number of tests to the minimal. IA generates way too many tests and this become a technical debt.

@lux-liang

Copy link
Copy Markdown
Contributor Author

Reduced the added coverage from 16 cases to 3: sync objname: 0 and callback objname: null / false. Removed the extra matrices covered by existing tests. All three retained cases fail on the original source and pass with the fix; the parser changes are unchanged.

@wdavidw
wdavidw merged commit 78b1e18 into adaltas:master Oct 9, 2026
5 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