Skip to content

Fix invalid JSON numbers from CSVRow::to_json - #334

Merged
vincentlaucsb merged 2 commits into
vincentlaucsb:masterfrom
youdie006:json-number-output
Oct 10, 2026
Merged

vincentlaucsb merged 2 commits into
vincentlaucsb:masterfrom
youdie006:json-number-output

Conversation

@youdie006

Copy link
Copy Markdown
Contributor

Follow-up to #323/#325. data_type() classifies some text as numeric that JSON doesn't accept as a number: .5, 5., 1.e5, a space before a sign or zero ( +2, 08), and 0x hex. to_json() / to_json_array() wrote those as-is, so a row like .5,5., 08 ,0x1F produced [.5,5., 08 ,0x1F], which JSON parsers reject (the README says the output is properly escaped).

Numeric fields are now trimmed and given the missing digit around ., and hex integers are written in decimal (0x1F -> 31), the same value CSVField::get() returns. Hex beyond int64 is still classified as a string and stays quoted.

Added cases to tests/test_csv_row_json.cpp; they fail before the change, and the JSON tests pass after it with the repo's -Werror flags. I also fed random numeric strings through to_json_array and checked the output with Python's json.loads: no invalid output remains, and every value equals float(s) (or int(s, 16)).

Written with AI assistance (Claude); I have reviewed the change.

data_type() accepts numeric text that JSON does not: .5, 5., 1.e5, a
space before a sign or zero, and 0x hex. Trim numeric fields, add the
missing digit around the decimal point, and write hex integers in
decimal.
@codecov

codecov Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.07692% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.57%. Comparing base (9dbbd2e) to head (7eb474e).

Files with missing lines Patch % Lines
include/internal/json_converter.hpp 73.07% 1 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #334      +/-   ##
==========================================
- Coverage   91.60%   91.57%   -0.04%     
==========================================
  Files          55       55              
  Lines        3574     3595      +21     
  Branches     1058     1068      +10     
==========================================
+ Hits         3274     3292      +18     
  Misses        115      115              
- Partials      185      188       +3     
Flag Coverage Δ
cpp 91.42% <73.07%> (-0.04%) ⬇️
python 93.64% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vincentlaucsb

Copy link
Copy Markdown
Owner

I added a padded hex edge case to the test suite.

Thanks for your PR and for the detailed test suite.

@vincentlaucsb
vincentlaucsb merged commit 4f5b383 into vincentlaucsb:master Oct 10, 2026
35 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