[nan-yu]

Follow-up to #3276. It fixes the two rollback bugs polina-c found in the RendererState undo log (comment 1, comment 2), and the points from her review of this PR. The Python counterpart is #3268 and the TypeScript one is #3278.

Problem

1. Undoing a deleteSurface replayed the surface through the processor. The data model went in as one updateDataModel at /, which the depth limit of 50 can reject even though each write that built it was accepted. The components went in as one updateComponents, which the processor can reject even though it accepted them one message at a time: a type's reference fields come from the first catalog that declares the type, in component order. When the replay failed, the surface came back incomplete or empty. Recording the surface also deep-copied it, and that copy threw TypeError on a map with non-string keys.
2. Undoing a type or catalog change added the old component back at the end of the surface. The order matters beyond iteration: the core reads reference fields through catalogIds, which follows component order, so a rolled-back state could accept a cycle that an untouched state rejects.
3. Undoing the deletion of a data model key put the key back at the end of its map.

Fix

New a2ui_core APIs. The processor calls none of them, so forward behavior is unchanged:

• SurfaceGroupModel.detachSurface(id) takes a surface out of the group without disposing of it and returns it. To the group's listeners it is the same as deleteSurface: onSurfaceDeleted fires, and the surface's actions no longer reach onAction. addSurface puts the same object back and fires onSurfaceCreated.
• SurfaceComponentsModel.insertComponents(pairs) adds components at given positions, passed as (index, component) pairs in ascending order, in one pass over the model. Like addComponent, it bumps the change counter and emits onCreated, and it resets the cached catalogIds so they follow the new order. insertComponent(index, component) inserts one component, for parity with Python and TypeScript.

a2ui_agent, RendererState:

• Deleting a surface while an apply or transaction is open detaches it instead of disposing of it, then runs the message through the processor, which now finds no surface and only checks it. The undo puts the same SurfaceModel back, with its components in their order, its data model, and everything the processor set on it. Nothing is copied or checked again, and recording a delete is O(1). Surfaces still detached when the outermost frame ends are disposed of then.
• When an entry of an updateComponents may recreate a component (it names another type, or another catalog than the component has), the record stores the component's position. The ids that need a position are collected first, then model.keys.indexed is walked only until all of them are found. Plain updates skip this. On undo, components that kept their type and catalog are restored in place, and recreated components go back to their old positions with one insertComponents call.
• When an updateDataModel deletes a key from a map, the record stores the key's position, and the undo puts the key back there. Other writes are unchanged.
• _revert keeps undoing the rest of the log when one step throws, and the error that caused the rollback propagates.
• _deepCopy, now used only for component properties and metadata, keeps maps with non-string keys instead of throwing.
• Dependency constraint: a2ui_agent uses a2ui_core APIs that 0.3.0 lacks: insertComponent, insertComponents and detachSurface from this PR, and checkPathsAndRecursion and checkDuplicateComponentIds, which Fix performance and create conformance tests. #3314 exported. Its a2ui_core: ^0.3.0 constraint stays as it is here. The repository is a pub workspace, and pub resolves a2ui_core from the workspace at the version in its pubspec.yaml, which only a release changes (see the a2ui-dart-versioning skill). A ^0.3.1 constraint makes flutter pub get fail, and pub rejects overriding a workspace package. The a2ui_agent CHANGELOG notes that it needs the next a2ui_core release, and that the release raises the constraint to ^0.3.1 along with the a2ui_core version.

Measurements

Median of 7 runs on one machine. main is before this PR, and f462c4f is the head polina-c reviewed.

1,000 type-change messages near the end of a 1,000-component surface, in one transaction:

• main: 145 ms
• f462c4f: 250 ms
• this PR: 174 ms

The same at 4,000 components:

• main: 219 ms
• f462c4f: 627 ms
• this PR: 400 ms

The same at 16,000 components:

• main: 507 ms
• f462c4f: 2,767 ms
• this PR: 920 ms

Rolling back one message that changes the type of the first 8,000 components of a 16,000-component surface:

• main: 525 ms
• f462c4f: 15,605 ms
• this PR: 488 ms

Recording 500 type changes on an 8,000-component surface, as a ratio to a bare MessageProcessor applying the same messages (new timing test, fails above 2.5):

• main: 1.1
• f462c4f: 3.4
• this PR: 1.6

Applying and rolling back a type change of 4,000 components of an 8,000-component surface, as a ratio to only applying it (new timing test, fails above 3):

• main: 1.0
• f462c4f: 13.4
• this PR: 1.1

Tests

• Regression tests in renderer_state_test.dart for every repro in the reviews. They fail at the code they report on and pass here: the deep data model, the catalog change that must keep rejecting the cycle, the map with non-string keys, the deleted surface that one updateComponents could not rebuild, the component moved to the surface's catalog, and a component property holding a map with non-string keys. Other tests cover: several recreated components in one message keep their order; the order is kept after an undone deleteSurface; a rollback puts back the same surface object, and a committed delete disposes of it; a deleted data model key goes back to its place.
• a2ui_core: tests for insertComponent, insertComponents (positions, catalogIds, events, and rejected batches leaving the model unchanged), and detachSurface (events, action forwarding, the same object coming back).
• renderer_state_timing_test.dart: two performance-tagged tests, the two ratios above.
• dart/a2ui_core, dart/a2ui_agent and dart/a2ui_flutter: dart format --set-exit-if-changed and flutter analyze --fatal-infos are clean, and flutter test passes in all three (core 1831, agent 1123, flutter 309). dart test -P performance passes for core (5) and agent (14).

[gemini-code-assist]

Code Review

This pull request enhances the rollback mechanism in RendererState to preserve the original order of components when they are recreated or when a deleted surface is restored. It introduces SurfaceComponentsModel.insertComponent to support inserting components at specific positions and ensures that errors thrown during an undo operation do not halt the remaining rollback process. The feedback suggests retaining explicit type annotations on loop variables when iterating over non-literal collections in both renderer_state.dart and component_model.dart to comply with Dart lints.

[polina-c]

I reviewed f462c4f. The fixes for both comments from #3276 work. The three repro tests fail on main and pass here, and so do the two new order tests. dart analyze, dart format and dart test (including -P performance) pass in a2ui_agent and a2ui_core.

I also fuzzed rollback, comparing the state before and after each rollback, including component order and catalogIds:

• v0.9, 3,000 cases: no differences other than data model key order (below). At main the same fuzz finds 980.
• v1.0 with two catalogs that both declare Box, 20,000 cases: 126 differences. Every one comes from the replay in the first inline comment.

Main points, explained inline:

1. Undoing a deleteSurface still replays its components as one updateComponents. The processor can reject that message even though it accepted the same components one at a time. The undo error is now dropped, so the surface comes back empty.
2. Two performance regressions: recording a message that may change a component's type, and rolling back many type changes.
3. Two new branches have no tests.

Smaller things:

• A rolled-back delete of a data model key puts the key back at the end of its map. With {a: 1, b: 2}, a rolled-back updateDataModel that sets /a to null leaves {b: 2, a: 1}. Main already did this, but #3278 now puts the key back where it was, so Dart and TypeScript differ here.
• The description says reading the positions "costs O(surface), the same as the processor's own handling of a recreated component". The measurements in the inline comment show about 7 times the processor's cost at 16,000 components. Could the description show the measurement as a line naming it and the size, then a before/after list?

_{🤖 Agent-generated}

[polina-c]

A clarification on my review (#3347 (review)). "Two new branches have no tests" means two code paths in renderer_state.dart, not git branches. The PR's new tests cover the main fixes. These two cases are the gap:

1. The (type == null ? existing.catalog : null) fallback in _mayRecreate (line 484). It handles an update that names the type and leaves out catalogId, which moves the component to the surface's catalog.
2. The non-string-key case of _deepCopy (line 904). The non-string-key test goes through the data model, which no longer uses _deepCopy. What still reaches it is a v1.0 surface whose metadata holds such a map.

I checked each one at f462c4f. With the line simplified to component['catalogId'] ?? existing.catalog, all 1115 tests in dart/a2ui_agent pass. With main's _deepCopy put back, they pass too. The inline comments on those lines have a test for each, which fails with that change and passes at the head.

_{🤖 Agent-generated}

[nan-yu]

Thanks for clarifying. Both code paths have tests at cc879f7:

1. _mayRecreate's (type == null ? existing.catalog : null) fallback: your test, 'a rollback puts back a component that an update moved to the surface's catalog', is added. It fails with the line simplified to component['catalogId'] ?? existing.catalog and passes as written.
2. _deepCopy's non-string-key case: your surface metadata test is added. Since this head restores a deleted surface by putting the same SurfaceModel back (SurfaceGroupModel.detachSurface) instead of copying it, surface metadata no longer goes through _deepCopy, so that test passes with or without the branch. What still reaches _deepCopy is the copy of a component's properties when an update replaces or changes the component. A second test covers that: a property with schema {'type': 'object'} holds {1: 'one'}, and the rollback restores it. It fails with main's _deepCopy and passes at the head.