Skip to content

fix(coordinator): stop the row inspector's JSON field undoing every keystroke - #3056

Merged
datlechin merged 2 commits into
mainfrom
fix/3051-json-field-editor-loop
Sep 22, 2026
Merged

datlechin merged 2 commits into
mainfrom
fix/3051-json-field-editor-loop

Conversation

@datlechin

Copy link
Copy Markdown
Member

Fixes #3051.

What was happening

Typing one character in the row inspector's JSON field sent the app to 100% CPU for one to two minutes, and threw the character away.

Measured in an instrumented Debug build against a SQLite table holding a JSON document in a TEXT column. One keystroke, then 30 seconds of idling:

probe count
JsonEditorView.body evaluations 6,944 and climbing
SourceEditor.updateNSViewController passes 432 in the first 2s
propagateEdit() 431
syncFromBinding() 430
MultiRowEditState.updateField 431
the scroll write in updateControllerWithState 0

CPU sat at 100.8%. The editor ended up showing the pre-edit text, with no modified marker.

Root cause

Two things compose. Neither hangs on its own.

The field's value Binding getter read a per-render snapshot, not the store. InspectorFieldListView built it as Binding(get: { state.editableText }, ...) over a FieldValueState captured by value when that render ran.

An onChange action closure belongs to the render that registered it, so it sees a context one render older still. JsonEditorView wrote .onChange(of: context.value.wrappedValue) { _ in syncFromBinding() }, discarding the newValue SwiftUI hands it and re-reading from the captured self. In the log the same binding answers 1171 at 24.584 and 952 at 24.587 with no write in between.

The field stages a compacted form of what the editor displays, so syncFromBinding compared the new text against the pre-edit value, decided the editor was stale, and overwrote it, dropping the keystroke. propagateEdit then pushed that back, resolvePendingValue mapped it to nil, the next snapshot read the original again, and round it went at about 75 rounds a second.

The scrollPoint: and setFrame: frames in the crash reports are real but are passengers: the scroll arm fired zero times and RepresentableSyncPhase was idle on every pass. They are the editor's normal per-update work, re-entered 75 times a second because the view tree above it was looping.

The issue's first suspicion, that writing state from onChange is illegal, is not right. A probe confirms a @State write inside updateNSViewController logs Modifying state during view update and is dropped, while the same write from an onChange action logs nothing and is handled normally. Apple's own samples write state from onChange. The declaration is explicit that the parameter is the new value:

@available(macOS, deprecated: 14.0, ...)
public func onChange<V>(of value: V, perform action: @escaping (_ newValue: V) -> Void) -> some View

The fix

  • JsonEditorView takes the newValue the change delivered and never re-reads the context.
  • JsonFieldEditingModel holds the reconciliation rule outside the view, so the fixpoint can be driven without a window. It compares an arriving value against lastSynced, the editor's own last published text, rather than against what the editor currently displays, which is what tells an echo from a real external change.
  • The field's value binding getter reads the store through MultiRowEditState.currentText(at:). That closes the stale-snapshot hazard for every field editor, not just JSON.

Storage behaviour is unchanged: a JSON field still opens laid out and still stages the compacted form.

Verification

  • verify.sh build, test, docs and lint all PASS (logs under .analysis/fix-3051-json-field-editor-loop/logs/).
  • Before and after on the live repro, same sandbox, same keystroke, same OSLog probes. Before: 6,944 body evaluations and climbing, 100.8% CPU, character discarded. After: 0.0% CPU, the typed text survives, and the field marks itself modified.
  • JsonFieldEditingModelTests drives the editor and the store against each other until they stop moving and asserts that takes one round, including a keystroke sequence through a transiently invalid document, the revert-to-original echo, and the Set NULL echo.
  • InspectorJsonFieldEditUITests types into the field and asserts the character is still there. Chinook has no JSON column and no JSON-looking text value, so the app seeds a json_fixture table into the sample when TABLEPRO_UI_TEST_SEED_JSON_TABLE is set, gated on the storage sandbox.

Deliberately not fixed here

The investigation found a second, independent defect and a first attempt at it is not in this PR.

MultiRowEditState.resolvePendingValue compacts every JSON value before it reaches the database, and isJson comes from columnTypeEnum.isJsonType || (originalValue ?? "").looksLikeJson. That rewrites whitespace the server would have preserved on PostgreSQL json ("the json type stores an exact copy of the input text"), on MariaDB (whose JSON is a LONGTEXT alias), on SQLite, DuckDB, libSQL, Turso and Cloudflare D1, and on any text column whose value happens to parse.

I built a per-engine policy for it and reverted it. Three review rounds each found new real defects in that code, the last of which is structural: the policy answered "does this server preserve JSON source text?" from the saved connection label, and MariaDB reports its JSON columns as "JSON", byte-identical to MySQL, so only the live server banner can tell them apart. A connection saved as MySQL can reach a MariaDB server.

It needs its own design, around live driver capability rather than a static table, plus the json[] array element path and the field-identity reuse case. Filed as a follow-up with the measurements, the competitor survey and all three rounds' findings.

@datlechin

Copy link
Copy Markdown
Member Author

The whitespace defect described under Deliberately not fixed here is now tracked as #3057, with the measurements, the competitor survey and all three review rounds' findings.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
@datlechin
datlechin merged commit e6cac66 into main Sep 22, 2026
3 checks passed
@datlechin
datlechin deleted the fix/3051-json-field-editor-loop branch September 22, 2026 18:06
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.

Main thread spins in a SwiftUI layout/update loop while typing in the row inspector's JSON field editor

1 participant