NewtonsoftJson package update. Refactoring to improve scaling with large CSVs. - #52
Open
KRuusuv wants to merge 5 commits into
Open
NewtonsoftJson package update. Refactoring to improve scaling with large CSVs.#52KRuusuv wants to merge 5 commits into
KRuusuv wants to merge 5 commits into
Conversation
…lower memory consumption and similar processing times when converting a 58 MB CSV and comparing to the most recent version of the ConvertToJson task. Tests with very small datasets showed a slight increase in processing time. Overall, the changes appear to come with a minor increase to constant overhead, but should provide slightly better scaling. Note on dependencies: Newtonsoft.Json version was updated from 12.0.1 to 13.0.4
Memory usage optimizations and minor tweaks.
WalkthroughConvertToJSON now streams JSON output with JTokenWriter (per-row objects, cancellation-aware) instead of accumulating rows; projects upgraded to Newtonsoft.Json 13.0.4, package version bumped to 1.3.0, and CHANGELOG updated (2026-05-22). ChangesCSV to JSON Streaming Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Frends.CSV.ConvertToJSON/CHANGELOG.md (1)
6-6: 💤 Low valueConsider more specific changelog description.
While the current description is acceptable, it could be more specific about what changed and the benefit to users. According to Keep a Changelog best practices, entries should clearly communicate what changed from a user perspective.
📝 Suggested improvement
-- Refactoring to improve scaling with large CSVs. +- Improved memory usage and performance when converting large CSV files by implementing streaming JSON output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Frends.CSV.ConvertToJSON/CHANGELOG.md` at line 6, Update the CHANGELOG entry that currently reads "Refactoring to improve scaling with large CSVs." to a more specific user-facing description: state which components or functions were refactored (e.g., streaming parser, memory buffering, or ReadAll-to-buffer removal), describe the observable benefit (reduced memory usage, faster processing, ability to handle N-GB files), and note any user-facing behavior changes or configuration flags (if any), so the entry clearly explains what changed and how it helps users.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Frends.CSV.ConvertToJSON/CHANGELOG.md`:
- Line 5: The changelog entry uses the incorrect package identifier
"NewtonsoftJson"; update the text to the correct NuGet package name
"Newtonsoft.Json" (include the period) so the line reads "Update package
Newtonsoft.Json to version 13.0.4."; ensure any other occurrences of
"NewtonsoftJson" in the change notes are similarly corrected.
- Line 3: The changelog dates for release headings are incorrect: update the
date string in the header "## [1.3.0] - 2025-05-12" to "## [1.3.0] - 2026-05-22"
and also correct the duplicated year for the "1.2.0" entry that currently shows
"2025-05-12" to the appropriate "2026-05-22" (or the actual release date if
different), ensuring both version headings reflect the correct year.
In
`@Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON.cs`:
- Around line 121-137: The branch that handles a CSV with no headers writes the
first JSON object without checking for cancellation; before calling
writer.WriteStartObject() (and similarly before each
writer.WriteStartObject()/writer.WriteValue(...) sequence around the single-row
handling where headers, headerCount and index are used) add a cancellation check
such as cancellationToken.ThrowIfCancellationRequested() (or the existing
CancellationToken variable used in this class) so the method aborts and does not
return success when cancellation was requested.
---
Nitpick comments:
In `@Frends.CSV.ConvertToJSON/CHANGELOG.md`:
- Line 6: Update the CHANGELOG entry that currently reads "Refactoring to
improve scaling with large CSVs." to a more specific user-facing description:
state which components or functions were refactored (e.g., streaming parser,
memory buffering, or ReadAll-to-buffer removal), describe the observable benefit
(reduced memory usage, faster processing, ability to handle N-GB files), and
note any user-facing behavior changes or configuration flags (if any), so the
entry clearly explains what changed and how it helps users.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: afe5536f-7565-45fe-98e6-747aefde25a0
📒 Files selected for processing (4)
Frends.CSV.ConvertToJSON/CHANGELOG.mdFrends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON.Tests/Frends.CSV.ConvertToJSON.Tests.csprojFrends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON.csFrends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON.csproj
…d writing the first line separately and without checking for cancellation.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON.cs (1)
132-145:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCheck cancellation during final-row field serialization.
This loop only observes cancellation before
WriteStartObject(). If the token is cancelled while writing fields for the last row, the method can still finish the object and returnSuccess = truebecause there is no later check in this branch.Suggested fix
do { cancellationToken.ThrowIfCancellationRequested(); writer.WriteStartObject(); for (var index = 0; index < headerCount; index++) { + cancellationToken.ThrowIfCancellationRequested(); writer.WritePropertyName(headers[index]); writer.WriteValue(csvReader.GetField(index)); } + cancellationToken.ThrowIfCancellationRequested(); writer.WriteEndObject(); } while (csvReader.Read());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON.cs` around lines 132 - 145, The loop that serializes the final CSV row in the method (the do/while block using csvReader, writer, headerCount and headers) only calls cancellationToken.ThrowIfCancellationRequested() once at the top, so cancellation during per-field writes can still complete and return success; fix it by invoking cancellationToken.ThrowIfCancellationRequested() inside the for loop before writing each field (and optionally again just before writer.WriteEndObject()) so cancellation aborts mid-row, ensuring the method honors the token during field serialization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON.cs`:
- Around line 132-145: The loop that serializes the final CSV row in the method
(the do/while block using csvReader, writer, headerCount and headers) only calls
cancellationToken.ThrowIfCancellationRequested() once at the top, so
cancellation during per-field writes can still complete and return success; fix
it by invoking cancellationToken.ThrowIfCancellationRequested() inside the for
loop before writing each field (and optionally again just before
writer.WriteEndObject()) so cancellation aborts mid-row, ensuring the method
honors the token during field serialization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08939893-e020-4ea9-8572-bd963637cc28
📒 Files selected for processing (3)
Frends.CSV.ConvertToJSON/CHANGELOG.mdFrends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON.csFrends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON.csproj
💤 Files with no reviewable changes (1)
- Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON/Frends.CSV.ConvertToJSON.csproj
✅ Files skipped from review due to trivial changes (1)
- Frends.CSV.ConvertToJSON/CHANGELOG.md
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
1 participant
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.
Please review my changes :)
Review Checklist
Summary by CodeRabbit
Chores
Refactor