Skip to content

fix(plugins): detect JSON import fields from every row and read JSON Lines by byte - #3194

Open
datlechin wants to merge 8 commits into
fix/row-import-sheet-scope-and-new-table-editsfrom
fix/json-import-detects-every-field
Open

datlechin wants to merge 8 commits into
fix/row-import-sheet-scope-and-new-table-editsfrom
fix/json-import-detects-every-field

Conversation

@datlechin

@datlechin datlechin commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

Stacked on #3191, which is stacked on #3183. Review and merge those first; this PR's diff against fix/row-import-sheet-scope-and-new-table-edits is only its own work.

What it fixes

JSON import decided the field list, and the type of every new column, from a sample, while the import itself reads every row.

  • detectSourceFields called sampleRawRows(limit: 200). For .json it parsed the whole file and kept the first 200 rows. For .jsonl and .ndjson it read the first 262,144 bytes. A key first seen after that, which TablePro's own JSON export produces whenever Include NULL values is off, was never offered for mapping and never created in a new table, and the import still reported every row imported. Column types came from the same 200 rows, so an integer column that turns to text on row 5,000 was created as an integer column.
  • The JSON Lines prefix was decoded with String(bytes:encoding: .utf8), which returns nil when byte 262,144 falls inside a multi-byte character. Any NDJSON file of CJK or Vietnamese text larger than 256 KB could open with "No fields found in the file."
  • The prefix was split with text.split(separator: "\n"). In Swift "\r\n" is one Character, so a CRLF file never split at all and also showed no fields.
  • The import read the file with URL.lines, which ends a line at U+2028, U+2029 and U+0085 as well as at a newline. JSON allows all three unescaped inside a string, so such a row was cut in two and both halves failed. URL.lines also replaces bytes that are not UTF-8 with U+FFFD, so a line the .json path would reject was imported with the bad bytes replaced. And it reads through FileHandle.AsyncBytes, whose one process-wide queue stays blocked while another reader waits on a pipe. LSPTransport reads the Copilot language server's stdout that way.

Measured with scratch probes against the system Foundation before changing anything:

Probe Result
NDJSON prefix whose byte 262,144 is inside a character String(bytes:encoding:) returns nil
Old detection on {"a":1}\r\n{"a":2,"b":3}\r\n 0 rows
Old detection, key first seen on row 201 keys ["id"] only
URL.lines on a row holding raw U+2028, U+2029 or U+0085 3 lines where the file has 2
URL.lines on a line with byte 0xFF line read back with U+FFFD
URL.lines on a 2-line file while another task reads a quiet Pipe not finished after 5 s (finishes at once without the parked reader)

Fix

  • JSONLineReader reads a JSON Lines file in 1 MB chunks, ends a line at the 0x0A byte and nowhere else, and hands the line's bytes to JSONSerialization undecoded. No text decoding means no character can be cut. A trailing CR is JSON whitespace, so CRLF parses. Detection and import both use it, so the two agree on what a line is.
  • JSONFieldSurvey folds every row into one small record per field: the first non-null value as the sample, and a type that fits every value. Memory is one record per field, not the values. Detection runs it over every row of both file kinds. Fields stay sorted by name, which is what feat(plugins): remember row import column mappings per table and add Match by Name and Match by Position #3183's Match by Position already assumes when it dims itself for JSON.
  • Values are classified by CFGetTypeID rather than Swift is and as? casts from Any, which cost about ten times as much per value, and rows stay the NSDictionary JSONSerialization returns instead of being bridged to [String: Any] and back. Each line is parsed inside an autorelease pool, as is each chunk read: without them, one synchronous detection over a 200 MB file held 400 to 800 MB of autoreleased objects until it returned.
  • JSONFieldDetectionCache keeps the last JSON Lines file's fields against its path, file number, size and modification date, taken before the read and from the file a symbolic link points to (attributesOfItem describes the link itself, measured). A JSON Lines file's fields do not depend on the table, so picking another table fetches that table's columns and reuses the fields instead of reading the whole file again. A failed or cancelled read is not kept, and a table-keyed .json file is still read for each table.
  • A JSON Lines import batch ends after 500 lines rather than 500 rows (JSONLineBatches). The runner checks for a stop between batches, and a long run of blank or unreadable lines, or one line with no end, used to be read to the end of the file before that check came.
  • ImportFieldDetection replaces the sheet's private detectFields. The stack's .task(id: SourceRead) already cancels the load when the table, destination, options or attempt change, or the sheet closes, but the load awaited a Task.detached that nothing cancelled, so the whole-file read ran on to the end after the sheet had moved on. The helper cancels the detached read with its caller. The scan checks for a stop before every 1 MB chunk of a JSON Lines file, so at most one chunk is parsed after a cancel, and for a JSON array before the parse and on every element after it.
  • While a read for what is on screen has not landed, an empty list reads "Reading the file…" instead of "No fields found in the file." or "No columns found in the file.", which it used to show for the length of the read. It is decided by the stack's own read identity (mapping.loadedRead and lastNewColumnsRead against sourceRead), split out of currentReadIsReady, so there is no frame between a table pick and the read starting where the old text shows. A re-read over rows already on screen keeps them, as fix(plugins): pin the row import sheet to one database and keep new table column edits across a re-read #3191 intends, with the header spinner.

Dropped as duplicates of the stack

This branch was built off main alongside the stack and grew its own copy of the sheet machinery. Merging fix/row-import-sheet-scope-and-new-table-edits in, the stack's version wins and these are gone:

What remains in RowImportSheet is the call to ImportFieldDetection and the reading placeholder.

Behaviour changes worth knowing

  • A JSON Lines line that is not valid UTF-8 is now reported as unreadable, the same as the .json path, instead of imported with U+FFFD.
  • A line of Unicode whitespace other than space, tab and CR (for example only U+00A0) is now an unreadable line rather than a blank one. JSON whitespace is those three plus LF.
  • A file with classic Mac CR-only line endings reads as one line. It already showed no fields, since detection never split on CR either.

Detection time

Measured with a swiftc -O probe that compiles the real plugin sources against the build's TableProPluginKit, on a generated 200 MB NDJSON file: 724,537 rows of 12 keys each, one key on every 97th row and one only on the last line. The machine was shared with other builds, at a load average of 60 to 95 on 12 cores.

Pass Wall Peak RSS
New detection, 200 MB NDJSON 3.4 s 13 MB
of which reading and JSONSerialization 2.0 s 12 MB
of which line splitting 0.03 s 11 MB
Old import read path on the same file (URL.lines, trim, parse) 8.7 s
URL.lines splitting alone 9.8 s
Same rows as a 200 MB JSON array, parse only (what the old detection already paid) 1.3 s 695 MB
New detection, same JSON array 2.5 s 1.2 GB

Every key was found, including the one on the last line, and every type matched the data. A full scan of a 200 MB JSON Lines file costs about 3.5 seconds, once per file rather than once per table picked, and it stops when the sheet closes or the read changes, so it is not capped.

Tests

verify.sh test on the JSON suites and the stack's: JSONImportFieldDetectionTests JSONImportPluginTests JSONImportSkipTests JSONLineBatchesTests JSONLineReaderTests JSONFieldDetectionCacheTests ImportFieldDetectionTests RowImportMappingTests ImportColumnMatcherTests NewTableDraftTests ImportDataSinkAdapterMappingTests CSVImportPluginTests XLSXSheetParserTests TransferAlertWindowOwnershipTests: 170 executed, 170 passed. Two earlier runs never started: the test runner hung before connecting, and then lost its channel when testmanagerd was restarted, on every worktree on the machine at the same minute.

  • JSONLineReaderTests (8): LF splitting with and without a final newline across chunk sizes 1, 3, 64 and 1 MB; blank lines and their line numbers; CR kept in the line; U+2028, U+2029 and U+0085 not ending a line; a multi-byte character straddling every chunk boundary from 1 to 12 bytes; a line many chunks long; a stop checked before every chunk, so a file with no newline is abandoned.
  • JSONImportFieldDetectionTests: a key first seen after row 200 in JSON Lines and after element 200 in a JSON array; a key past the first megabyte; a file built so bytes 262,144 and 1,048,576 each fall inside a character; CRLF; U+2028 in a string; type fitting every value; table-keyed files; unreadable lines passed over; null-only fields; cancellation for both file kinds. Through the plugin: a second table pick answers without opening the JSON Lines file again (the file is made unreadable in between), and a table-keyed .json file still gives each table its own fields.
  • JSONFieldDetectionCacheTests (5): an unchanged file is read once, a changed file is read again, a file behind a symbolic link is read again once the file changes, another file is read itself, and a failed read is not kept.
  • JSONLineBatchesTests (3): the first batch over 5,000 unreadable lines ends after exactly 500 lines, batches cover every line once with each row under its own line number, and a stop mode throws at the first unreadable line.
  • JSONImportSkipTests: U+2028, U+2029 and U+0085 import as one row; CRLF imports; a non-UTF-8 line is reported and skipped; an import finishes while another task reads a quiet pipe.
  • JSONImportPluginTests: a whitespace-only line is blank, a line ending in CR parses, a non-UTF-8 line throws, and a type settled on text stays text.
  • ImportFieldDetectionTests: cancelling the caller cancels a detection already running.

Build passes. verify.sh lint on the 15 Swift files this PR changes against the base reports 0 violations (the step's FAIL is the stale /Applications/Xcode-beta.app path in .claude/, not touched here). python3 scripts/localization.py verify passes with the one new key, "Reading the file…". The docs checks pass for the one paragraph added to docs/features/import-export.mdx.

Not verified

  • No UI test, and the sheet was not driven by hand. The stack's RowImportMappingMemoryUITests and RowImportNewTableEditsUITests drive the sheet with CSV; this PR's sheet change is the placeholder text and the detection call.
  • The Copilot stall was reproduced with a task parked on a Pipe, the same read LSPTransport does, not by running the app with Copilot signed in.
  • Timings come from a shared machine under load.

Review

Codex reviewed the restacked branch against fix/row-import-sheet-scope-and-new-table-edits and found the survey, the line reader, cancellation, the cache, batching and the sheet integration correct. Its one finding, P3, asks to remove the /// doc comments this branch adds under the no-comments rule. Not taken: they record measured reasons (why a line ends only at 0x0A, why attributesOfItem needs the link resolved, why a batch is bounded by lines), the same kind the stack's own RowImportSheet, RowImportMapping and NewTableDraft carry, and removing them would lose what the measurements found.

The earlier reviews of this branch on main (three Codex passes and one independent review) found the redundant and uncancelled whole-file reads, the row-bounded batch, the long-line stop and the option-change bug. The redundant reads are now covered by the stack reading only the destination on screen and by this branch's JSON Lines cache; the uncancelled reads, the batch and the long-line stop by the JSON changes above. The option-change bug is the one #3183 fixes, which is why this branch's own fix for it is dropped.

Branch note

The PR branch also carries a merge of main (#3179 and #3180) made on GitHub before the restack. fix/row-import-sheet-scope-and-new-table-edits does not have those two commits yet, so they show in this diff until #3191 picks up #3183's own merge of main. They are not part of this change.

Found while investigating #3172 (#3183)

CI fixes

The failed Package Tests job was SyncRecordMapperTests.unknownWireValueFailsClosed: the sync mapper's duplicate decoder turned an unknown Safe Mode value into Off. It now uses SafeModeLevel(wireValue:isReadOnly:), requiring confirmation while retaining legacy read-only restrictions. Added regressions for unknown read-only values and renames that preserve an unknown wire value.

Kept the existing stacked base on fix/row-import-sheet-scope-and-new-table-edits.

Validation for these fixes:

  • Full swift test --package-path Packages/TableProCore: passed (1,197 Swift Testing tests plus XCTest).
  • SwiftLint on the changed Swift files: zero violations. The helper's documentation check still flags four pre-existing stale paths: TableProApp/rust-dameng, Contents/MacOS, Contents/Helpers, and /Applications/Xcode-beta.app.

GitHub Actions rerun after pushing these fixes.

@mintlify

mintlify Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 29, 2026, 5:09 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

@datlechin
datlechin changed the base branch from main to fix/row-import-sheet-scope-and-new-table-edits September 29, 2026 17:09
…ew-table-edits' into fix/json-import-detects-every-field

# Conflicts:
#	CHANGELOG.md
…ew-table-edits' into fix/json-import-detects-every-field

# Conflicts:
#	CHANGELOG.md
#	Plugins/JSONImportPlugin/JSONImportParsing.swift

This branch was successfully deployed

1 active (outdated) deployment
staging - docs — 8edc5cd1 Deployed Sep 30, 2026 by mintlify[bot]
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.

1 participant