fix: improve diff conversion for CSV, XML, and file parsing (#77, #78) - #121
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16e56b0f44
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const sheet = wb.Sheets[sheetName]; | ||
| const rows = XLSX.utils.sheet_to_json<Record<string, unknown>>(sheet, { defval: null, raw: false }); | ||
| if (rows.length > MAX_EXCEL_ROWS) { | ||
| const rawRows = XLSX.utils.sheet_to_json<Record<string, unknown>>(sheet, { defval: null, raw: true }); |
There was a problem hiding this comment.
When an XLSX column contains numeric identifiers with formatting such as 000000, raw: true returns the underlying number (for example, 123) instead of the displayed value (000123); it can likewise expose rounded numeric values for long identifiers. The parsed value flows through parseAnyFile into conversion and data-push mappings, so an upsert can use a different external ID and create or update the wrong record. The previous formatted-string parsing should be retained for identifier fidelity rather than relying on raw JavaScript numbers.
Useful? React with 👍 / 👎.
| Array.isArray((parsed as Record<string, unknown>).records) | ||
| ) { | ||
| records = (parsed as Record<string, unknown>).records as unknown[]; |
There was a problem hiding this comment.
When an uploaded JSON document is intended to be one record but has an array-valued field named records, this branch discards the outer record and treats that field's elements as the dataset. For example, {"Id":"1","records":[{"value":"x"}]} loses both Id and the enclosing structure. Since WaveLink's own metadata export includes identifying fields such as exportedAt, recordCount, columns, and records, verify that envelope shape before unwrapping instead of treating every object with a records array as metadata.
Useful? React with 👍 / 👎.
| */ | ||
| function stringifyForCompare(value: unknown): string { | ||
| if (value === null || value === undefined) return ''; | ||
| if (typeof value === 'object') return JSON.stringify(value); |
There was a problem hiding this comment.
When two JSON files contain semantically identical nested objects with different property order, direct JSON.stringify calls produce different strings—for example, {a: 1, b: 2} versus {b: 2, a: 1}. The local Diff flow preserves JSON insertion order and passes these values here, so it incorrectly reports the field and record as changed. Use an order-independent deep comparison or canonical key ordering for objects.
Useful? React with 👍 / 👎.
- Fix data diff comparison logic for edge cases - Improve file parse error handling and encoding detection - Correct CSV delimiter handling in diff view - Harden XML parsing against malformed input Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
16e56b0 to
3ede1ab
Compare
1 participant
Summary
Improves diff conversion logic for CSV, XML, and general file parsing.
Issues Fixed
Changes
🤖 Generated with Claude Code