Group Replay step rollback state

CHANGELOG.md

@@ -4,6 +4,8 @@ Dated delivery history and release records. Each release is a `## vX.Y.Z — YYY
 
 ## Unreleased
 
+- Restore Replay's pending step inputs together when a step fails before commit.
+
 - Reuse the validated replacement tree when settling a Replay host update, keeping the ordinary reload and rollback path.
 
 - Resolve `SplitPane` and `Scrollbar` geometry once per dispatched event and reuse it to reconcile active drags.

src/replay/replay.cpp

@@ -51,8 +51,9 @@ namespace detail {
 class ReplayPlayer {
 public:
     ReplayPlayer(ReplayRecording recording, TextShaper& text)
-        : recording_(std::move(recording)), text_(text), viewport_(recording_.viewport),
-          scale_(recording_.environment.scale), host_(recording_.environment.host),
+        : recording_(std::move(recording)), text_(text), state_{.viewport = recording_.viewport,
+                                                                .scale = recording_.environment.scale,
+                                                                .host = recording_.environment.host},
           input_(recording_.policy.compatible_pointer ? PointerSnapshotPolicy::compatible_author_ids
                                                       : PointerSnapshotPolicy::tree_identity) {}
 
@@ -63,8 +64,8 @@ class ReplayPlayer {
         auto tree = create_tree(recording_.document, "");
         if (!tree) return false;
         install_tree(std::move(tree), recording_.styles);
-        for (std::size_t i = 0; i < host_.size(); ++i) {
-            if (!current_.tree->find(host_[i].owner)) {
+        for (std::size_t i = 0; i < state_.host.size(); ++i) {
+            if (!current_.tree->find(state_.host[i].owner)) {
                 errors_.push_back({"target_not_found",
                                    Severity::error,
                                    "/environment/host/" + std::to_string(i) + "/owner",
@@ -96,12 +97,7 @@ class ReplayPlayer {
         }
         recording_.steps.push_back(std::move(next));
         const auto& step = recording_.steps.back();
-        const auto clock = clock_;
-        const auto viewport = viewport_;
-        const auto offsets = offsets_;
-        const auto scale = scale_;
-        const auto animation_time = animation_time_;
-        const auto host = host_;
+        auto previous = state_;
         committed_ = false;
         bool ok = true;
         if (const auto* event = std::get_if<InputEvent>(&step)) {
@@ -126,19 +122,19 @@ class ReplayPlayer {
             check.positive(scale_step->scale, at + "/scale");
             ok = errors_.empty();
             if (ok) {
-                scale_ = scale_step->scale;
+                state_.scale = scale_step->scale;
                 ok = settle(i, at);
             }
         } else if (const auto* tick = std::get_if<ReplayTick>(&step)) {
-            if (tick->time < animation_time_) {
+            if (tick->time < state_.animation_time) {
                 errors_.push_back({"event_order",
                                    Severity::error,
                                    at + "/time",
                                    "Use nonnegative, non-decreasing animation-clock microseconds.",
                                    {}});
                 ok = false;
             } else {
-                animation_time_ = tick->time;
+                state_.animation_time = tick->time;
                 ok = settle(i, at);
             }
         } else {
@@ -149,12 +145,7 @@ class ReplayPlayer {
         if (committed_) {
             closed_ = true;
         } else {
-            clock_ = clock;
-            viewport_ = viewport;
-            offsets_ = offsets;
-            scale_ = scale;
-            animation_time_ = animation_time;
-            host_ = host;
+            state_ = std::move(previous);
         }
         return false;
     }
@@ -194,7 +185,7 @@ class ReplayPlayer {
 
 private:
     void cancel_pending() {
-        for (auto& slot : host_)
+        for (auto& slot : state_.host)
             if (slot.readiness == ReplayReadiness::loading) slot.readiness = ReplayReadiness::cancelled;
     }
 
@@ -209,13 +200,14 @@ class ReplayPlayer {
         }
         relocate(errors_, check_host_slot(update.slot, "/slot"), at);
         if (!errors_.empty()) return false;
+        auto& slots = state_.host;
         auto found =
-            std::find_if(host_.begin(), host_.end(), [&](const auto& slot) { return slot.id == update.slot.id; });
+            std::find_if(slots.begin(), slots.end(), [&](const auto& slot) { return slot.id == update.slot.id; });
         const auto reject = [&](const char* code, const char* field, const char* message) {
             errors_.push_back({code, Severity::error, at + "/slot" + field, message, {}});
             return false;
         };
-        if (found == host_.end()) return reject("undeclared_state", "/id", "Use a declared host slot.");
+        if (found == slots.end()) return reject("undeclared_state", "/id", "Use a declared host slot.");
         if (found->owner != update.slot.owner)
             return reject("stale_owner", "/owner", "A slot cannot change its declared owner.");
         if (update.slot.readiness == ReplayReadiness::loading) {
@@ -238,9 +230,9 @@ class ReplayPlayer {
         if (!tree) return false;
         if (update.slot.readiness == ReplayReadiness::loading && !tree->find(found->owner))
             return reject("stale_owner", "/owner", "A new request requires its owner in the replacement view.");
-        const auto index = static_cast<std::size_t>(found - host_.begin());
+        const auto index = static_cast<std::size_t>(found - slots.begin());
         cancel_pending();
-        host_[index] = update.slot;
+        slots[index] = update.slot;
         return replace(std::move(tree), update.view.styles, step, view_at);
     }
 
@@ -274,7 +266,7 @@ class ReplayPlayer {
     void install_tree(std::unique_ptr<UiTree> tree, const std::vector<ResolvedStyle>& styles) {
         current_.tree = std::move(tree);
         current_.styles = styles;
-        offsets_.assign(current_.tree->size(), Point{});
+        state_.offsets.assign(current_.tree->size(), Point{});
     }
 
     // Settles the validated replacement tree, revealing recovered focus under the host policy since
@@ -301,15 +293,15 @@ class ReplayPlayer {
             }
             keep = revealed.value->empty();
         }
-        viewport_ = viewport;
+        state_.viewport = viewport;
         return settle(step, at, keep);
     }
 
     // Full-tree layout and paint at an update point, then re-target stationary pointers and recover focus.
     // The new layout is the commit point. With `reveal`, recovered focus is scrolled into view and laid
     // out again before the generation is recorded, so no generation shows it hidden.
     bool settle(std::optional<std::size_t> step, const std::string& at, bool reveal = false) {
-        auto layout = compute_layout({current_.tree.get(), current_.styles, viewport_, &text_, offsets_});
+        auto layout = compute_layout({current_.tree.get(), current_.styles, state_.viewport, &text_, state_.offsets});
         if (!layout) {
             relocate(errors_, std::move(layout.diagnostics), at);
             return false;
@@ -334,7 +326,7 @@ class ReplayPlayer {
                 return false;
             }
             if (!revealed.value->empty()) {
-                for (const auto& update : *revealed.value) offsets_[update.container.index] = update.offset;
+                for (const auto& update : *revealed.value) state_.offsets[update.container.index] = update.offset;
                 return settle(step, at);
             }
         }
@@ -344,10 +336,10 @@ class ReplayPlayer {
             return false;
         }
         ReplayGeneration generation{step, {}, std::move(*paint.value)};
-        generation.viewport = viewport_;
-        generation.device_scale = scale_;
-        generation.animation_time = animation_time_;
-        generation.host = host_;
+        generation.viewport = state_.viewport;
+        generation.device_scale = state_.scale;
+        generation.animation_time = state_.animation_time;
+        generation.host = state_.host;
         generation.boxes.reserve(current_.layout.boxes.size());
         for (const auto& box : current_.layout.boxes)
             generation.boxes.push_back({box.node.index, box.parent, box.border_box, box.border, box.padding,
@@ -373,7 +365,7 @@ class ReplayPlayer {
 
     // Rejects time running backwards across pointer, scroll, and command steps, which share one stream.
     bool in_order(const InputEvent& event, const std::string& at) {
-        if (!clock_ || event.timestamp >= *clock_) return true;
+        if (!state_.input_time || event.timestamp >= *state_.input_time) return true;
         errors_.push_back({"event_order",
                            Severity::error,
                            at + "/timestamp",
@@ -395,8 +387,8 @@ class ReplayPlayer {
             relocate(errors_, std::move(routed.diagnostics), at);
             return false;
         }
-        clock_ = event.timestamp;
-        for (const auto& update : routed.value->updates) offsets_[update.container.index] = update.offset;
+        state_.input_time = event.timestamp;
+        for (const auto& update : routed.value->updates) state_.offsets[update.container.index] = update.offset;
         return settle(step, at);
     }
 
@@ -408,7 +400,7 @@ class ReplayPlayer {
             return false;
         }
         committed_ = true;
-        clock_ = event.timestamp;
+        state_.input_time = event.timestamp;
         record(step, result.value->actions);
         // Host press policy: a primary press also focuses its press owner when that owner is focusable.
         // Pointer dispatch itself never moves focus, and a press never reveals.
@@ -437,7 +429,7 @@ class ReplayPlayer {
             return false;
         }
         committed_ = true;
-        clock_ = event.timestamp;
+        state_.input_time = event.timestamp;
         const auto previous = focused_;
         focused_ = result.value->focused;
         record(step, result.value->actions);
@@ -500,7 +492,7 @@ class ReplayPlayer {
             return false;
         }
         if (revealed.value->empty()) return observe(step, at);
-        for (const auto& update : *revealed.value) offsets_[update.container.index] = update.offset;
+        for (const auto& update : *revealed.value) state_.offsets[update.container.index] = update.offset;
         return settle(step, at);
     }
 
@@ -514,13 +506,16 @@ class ReplayPlayer {
 
     ReplayRecording recording_; // Accepted steps only.
     TextShaper& text_;
-    Size viewport_;
-    float scale_ = 1;
-    std::chrono::microseconds animation_time_{};
-    std::vector<ReplayHostSlot> host_;
+    // Requested inputs that can change before a step commits. The installed snapshot is restored by replace().
+    struct StepState {
+        Size viewport;
+        float scale;
+        std::chrono::microseconds animation_time{};
+        std::vector<ReplayHostSlot> host;
+        std::vector<Point> offsets; // By node index; reset by reload.
+        std::optional<std::chrono::microseconds> input_time;
+    } state_;
     Snapshot current_;
-    std::vector<Point> offsets_; // Requested scroll offsets by node index; reset by reload.
-    std::optional<std::chrono::microseconds> clock_;
     PointerDispatcher input_;
     FocusDispatcher focus_;
     std::optional<NodeHandle> focused_; // Current focus in the current tree.

tests/replay/replay_tests.cpp

@@ -630,6 +630,21 @@ void session_steps_match_playback() {
     check(!rejected && has(rejected.diagnostics, "unsupported_version", "/version"), "Session version must be checked");
 }
 
+void rejected_reload_preserves_requested_scroll() {
+    const auto source = scrolled_menu();
+    auto session = open(source);
+    check(bool(session.apply(tessera::InputEvent{1us, tessera::Scroll{{100, 30}, {0, 1000}}})), "Scroll rejected");
+    const auto scrolled = offset(session.output().generations.back());
+    check(scrolled > 0, "The fixture must scroll before the rejected reload");
+
+    const auto rejected = session.apply(tessera::ReplayReload{source.document, {}});
+    check(!rejected && has(rejected.diagnostics, "style_count", "/steps/1/styles"), "Bad reload accepted");
+    check(bool(session.apply(tessera::ReplayTick{1us})), "The session must settle another step after rejection");
+    check(offset(session.output().generations.back()) == scrolled,
+          "The next layout must use the scroll offset from before the rejected reload");
+    check(play(session.recording()) == session.output(), "The accepted steps must reproduce the scrolled session");
+}
+
 // An in-process consumer captures the current generation, acts through resolved handles that are recorded as
 // fixture identities, and has stale and invalid steps rejected without changing the session.
 void session_consumer_rejects_stale_targets() {
@@ -871,6 +886,7 @@ int main() {
         semantic_action_steps_match_clicks();
         press_policy_focuses_without_reveal();
         session_steps_match_playback();
+        rejected_reload_preserves_requested_scroll();
         session_consumer_rejects_stale_targets();
         comparison_locates_differences();
         invalid_recordings_are_located_without_output();