Skip to content

Commit 7200fdf

Browse files
committed
fix: serialize chat animations behind a fence to stop the missing-final-attributes crash
ChatLayout answers attribute queries with nil during restoreContentOffset; an inset write or push overlapping a settling batch spring collided with UIKit's animated bounds-change cross-fade and threw. All animated transactions now bracket a fence, mutations defer and replay, insets apply via upstream's keyboard recipe, and entrance motion moves off layout attributes onto cells. Includes the concurrent quality pass (Reduce Motion tokens, spring parameterization, receipt-reveal fencing).
1 parent 5b224a0 commit 7200fdf

17 files changed

Lines changed: 731 additions & 328 deletions

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,7 @@ Types: `feat`, `fix`, `refactor`, `test`, `docs`, `chore`
554554
| Calling `router.present(.x)` next to a viewmodel mutator that may *block* the flow | A viewmodel that surfaces a blocking error via `session.dialogItem` (e.g. `GiveViewModel.showNoBalanceError`) does not stop the router — `DialogWindow` renders the dialog above the sheet, but the sheet is still presented underneath and reappears once the dialog is dismissed. Gate the router on the precondition: expose `attemptPresent() -> Bool` on the viewmodel and write `if vm.attemptPresent() { router.present(.x) }`. Putting the check inside an `isPresented` `didSet` is not enough — the router call still runs unconditionally on the next line. |
555555
| Parsing keypad-emitted amounts with `Decimal(string:)` or `NumberFormatter.decimal(from:)` | `KeyPadView`'s decimal key inserts `AmountValidator.localizedDecimalSeparator`, so on comma-decimal locales the bound string contains ",". `Decimal(string:)` stops at the comma and silently drops the fraction; `NumberFormatter.decimal(from:)` only parses the device locale's format. **Parse keypad strings with `AmountValidator`** (in FlipcashCore's Validation family) — it normalizes the locale separator before parsing. `NumberFormatter.decimal(from:)` remains appropriate for currency-formatted strings (already through a formatter, locale-correct). |
556556
| Injecting shared DI via a custom keyPath `@Environment(\.key)` with a trapping default | SwiftUI resolves keyPath env **eagerly** during dynamic-list/transition `DynamicProperty` updates (against a placeholder environment), so a `fatalError`/`preconditionFailure` default fires and crashes at launch (`<dep> was not injected`). Inject shared DI as **type-based `@Environment(Type.self)` on an `@Observable`** — the trap is deferred to body access, so it survives the eager pass. That's why `Container`/`SessionContainer` are `@Observable`. Safe-value `@Entry` keyPath defaults (e.g. `nestedSheetDepth = 0`) are unaffected — the hazard is specifically a *crashing* default. |
557+
| Touching the chat transcript's diff/batch pipeline without the fence | ChatLayout's `restoreContentOffset` answers attribute queries with nil while it re-anchors; if that forced layout pass overlaps animated layout work (a settling batch spring, an inset write inside an animation context), UIKit throws `NSInternalInconsistencyException: missing final attributes for cell`. Every animated transaction in `ChatViewController` brackets itself on `ChatAnimationFence`, and every mutation that must not overlap one (transcript push, `setBottomInset`, offset restores, `freezeInset`/`restoreInset`) defers through `fence.whenIdle` and applies insets via the upstream recipe (`performWithoutAnimation` + empty-batch interrupt + write inside `performBatchUpdates` + restore in the same transaction). Also: **never set transforms on `ChatLayoutAttributes`** — the layout round-trips attribute frames through its keep-at-bottom compensation, so a scale gets baked into stored frames; entrance motion goes on the *cell* in `willDisplay` (`playEntranceIfNeeded`). Contract pinned by `ChatAnimationFenceTests`, the fence section of `ChatViewControllerTests`, and the baseline-attributes tests in `ChatMotionTests`. |
557558

558559
---
559560

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
//
2+
// ChatAnimationFenceTests.swift
3+
// FlipcashTests
4+
//
5+
// Copyright © 2026 Code Inc. All rights reserved.
6+
//
7+
8+
import Testing
9+
@testable import FlipcashUI
10+
11+
/// The fence is the transcript's crash guard: ChatLayout's `restoreContentOffset` answers
12+
/// attribute queries with nil while it re-anchors, so inset writes, offset restores, and
13+
/// transcript pushes must never overlap an animated transaction — UIKit throws
14+
/// `NSInternalInconsistencyException: missing final attributes for cell` when they collide.
15+
/// These tests pin the serialization contract every chat-transcript mutation relies on.
16+
@MainActor
17+
@Suite("Chat animation fence")
18+
struct ChatAnimationFenceTests {
19+
20+
@Test("Work runs immediately while idle")
21+
func idle_runsImmediately() {
22+
let fence = ChatAnimationFence()
23+
var ran = false
24+
fence.whenIdle { ran = true }
25+
#expect(ran)
26+
}
27+
28+
@Test("Work defers while any transaction is active and replays when the last settles")
29+
func active_defersUntilLastEnd() {
30+
let fence = ChatAnimationFence()
31+
fence.begin()
32+
fence.begin()
33+
var ran = false
34+
fence.whenIdle { ran = true }
35+
fence.end()
36+
#expect(!ran, "one transaction is still in flight")
37+
fence.end()
38+
#expect(ran)
39+
}
40+
41+
@Test("Deferred work replays in registration order")
42+
func drain_replaysInOrder() {
43+
let fence = ChatAnimationFence()
44+
fence.begin()
45+
var order: [Int] = []
46+
fence.whenIdle { order.append(1) }
47+
fence.whenIdle { order.append(2) }
48+
fence.end()
49+
#expect(order == [1, 2])
50+
}
51+
52+
@Test("A replay that begins a new transaction re-defers the work queued behind it")
53+
func drain_replayBeginningTransaction_reDefersRemainder() {
54+
// The crash shape: a held transcript push replays and starts a new batch; a held inset
55+
// write queued behind it must wait for that batch too — never run inside it.
56+
let fence = ChatAnimationFence()
57+
fence.begin()
58+
var insetApplied = false
59+
fence.whenIdle { fence.begin() } // the replayed push opens its own transaction
60+
fence.whenIdle { insetApplied = true } // the inset write queued behind it
61+
fence.end()
62+
#expect(!insetApplied, "the remainder must wait behind the replay's transaction")
63+
fence.end()
64+
#expect(insetApplied)
65+
}
66+
67+
@Test("Work registered during a drain joins the queue instead of jumping it")
68+
func drain_nestedWhenIdle_runsAfterCurrentQueue() {
69+
let fence = ChatAnimationFence()
70+
fence.begin()
71+
var order: [Int] = []
72+
fence.whenIdle {
73+
order.append(1)
74+
fence.whenIdle { order.append(3) } // registered mid-drain
75+
}
76+
fence.whenIdle { order.append(2) }
77+
fence.end()
78+
#expect(order == [1, 2, 3])
79+
}
80+
}

FlipcashTests/Chat/ChatChangesetFlatteningTests.swift

Lines changed: 18 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -19,39 +19,22 @@ import FlipcashCore
1919
@Suite("Chat changeset flattening")
2020
struct ChatChangesetFlatteningTests {
2121

22-
private func message(
23-
_ id: String,
24-
sender: ChatMessage.Sender = .me,
25-
continuedByNext: Bool = false,
26-
continuationFromPrevious: Bool = false,
27-
receipt: String? = nil
28-
) -> ChatItem {
29-
.message(ChatMessage(
30-
id: id,
31-
text: "text-\(id)",
32-
sender: sender,
33-
isContinuationFromPrevious: continuationFromPrevious,
34-
isContinuedByNext: continuedByNext,
35-
receipt: receipt
36-
))
37-
}
38-
3922
@Test("A send (previous-row update + insert) flattens to a single changeset")
40-
func updateAndInsert_flattensToOne() {
23+
func updateAndInsert_flattensToOne() throws {
4124
// A new own message migrates the receipt off the previous row and flips its grouping —
4225
// an update — while the new row is an insert. DifferenceKit stages these separately.
43-
let before = [message("a", receipt: "Delivered")]
44-
let after = [
45-
message("a", continuedByNext: true),
46-
message("b", continuationFromPrevious: true, receipt: "Delivered"),
26+
let before: [ChatItem] = [.text("a", receipt: "Delivered")]
27+
let after: [ChatItem] = [
28+
.text("a", continuedByNext: true),
29+
.text("b", continuationFromPrevious: true, receipt: "Delivered"),
4730
]
4831

4932
let staged = StagedChangeset(source: before, target: after)
5033
#expect(staged.count == 2, "premise: DifferenceKit stages [updates]+[inserts] separately")
5134

5235
let flattened = staged.flattenIfPossible()
5336
#expect(flattened.count == 1)
54-
guard let merged = flattened.first else { return }
37+
let merged = try #require(flattened.first)
5538
#expect(merged.elementUpdated == [ElementPath(element: 0, section: 0)])
5639
#expect(merged.elementInserted == [ElementPath(element: 1, section: 0)])
5740
#expect(merged.elementDeleted.isEmpty)
@@ -60,21 +43,21 @@ struct ChatChangesetFlatteningTests {
6043
}
6144

6245
@Test("An arrival while typing (update + delete + insert) flattens to a single changeset")
63-
func updateDeleteAndInsert_flattensToOne() {
46+
func updateDeleteAndInsert_flattensToOne() throws {
6447
// The typing indicator clears (delete), the reply lands (insert), and the previous row's
6548
// grouping flips (update) — three DifferenceKit stages in one push.
66-
let before = [message("a", sender: .other), .typingIndicator]
67-
let after = [
68-
message("a", sender: .other, continuedByNext: true),
69-
message("b", sender: .other, continuationFromPrevious: true),
49+
let before: [ChatItem] = [.text("a", sender: .other), .typingIndicator]
50+
let after: [ChatItem] = [
51+
.text("a", sender: .other, continuedByNext: true),
52+
.text("b", sender: .other, continuationFromPrevious: true),
7053
]
7154

7255
let staged = StagedChangeset(source: before, target: after)
7356
#expect(staged.count == 3, "premise: DifferenceKit stages [updates]+[deletes]+[inserts] separately")
7457

7558
let flattened = staged.flattenIfPossible()
7659
#expect(flattened.count == 1)
77-
guard let merged = flattened.first else { return }
60+
let merged = try #require(flattened.first)
7861
#expect(merged.elementUpdated == [ElementPath(element: 0, section: 0)])
7962
#expect(merged.elementDeleted == [ElementPath(element: 1, section: 0)])
8063
#expect(merged.elementInserted == [ElementPath(element: 1, section: 0)])
@@ -83,14 +66,14 @@ struct ChatChangesetFlatteningTests {
8366
}
8467

8568
@Test("A pure delete + insert pair still flattens to a single changeset")
86-
func deleteAndInsert_stillFlattensToOne() {
69+
func deleteAndInsert_stillFlattensToOne() throws {
8770
// The pre-existing behavior (typing indicator swaps for a message with no other change).
88-
let before = [message("a", sender: .other), .typingIndicator]
89-
let after = [message("a", sender: .other), message("b", sender: .other)]
71+
let before: [ChatItem] = [.text("a", sender: .other), .typingIndicator]
72+
let after: [ChatItem] = [.text("a", sender: .other), .text("b", sender: .other)]
9073

9174
let flattened = StagedChangeset(source: before, target: after).flattenIfPossible()
9275
#expect(flattened.count == 1)
93-
guard let merged = flattened.first else { return }
76+
let merged = try #require(flattened.first)
9477
#expect(merged.elementDeleted == [ElementPath(element: 1, section: 0)])
9578
#expect(merged.elementInserted == [ElementPath(element: 1, section: 0)])
9679
#expect(merged.data == after)
@@ -100,8 +83,8 @@ struct ChatChangesetFlatteningTests {
10083
func moves_areNotFlattened() {
10184
// A move's source index is relative to the post-delete stage, not the original source, so
10285
// merging would corrupt indices. Reorders never happen in a transcript; keep them staged.
103-
let before = [message("a"), message("b")]
104-
let after = [message("b"), .message(ChatMessage(id: "a", text: "edited", sender: .me))]
86+
let before: [ChatItem] = [.text("a"), .text("b")]
87+
let after: [ChatItem] = [.text("b"), .message(ChatMessage(id: "a", text: "edited", sender: .me))]
10588

10689
let staged = StagedChangeset(source: before, target: after)
10790
#expect(staged.contains { !$0.elementMoved.isEmpty }, "premise: a reorder diffs to a move")

FlipcashTests/Chat/ChatMessageCopyTests.swift

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,7 @@ import FlipcashCore
1515
struct ChatMessageCopyTests {
1616

1717
private func loadedController(_ items: [ChatItem]) -> ChatViewController {
18-
let controller = ChatViewController()
19-
controller.loadViewIfNeeded()
20-
controller.update(items: items)
21-
return controller
18+
.loaded(items: items)
2219
}
2320

2421
private func configuration(_ controller: ChatViewController, at index: Int) -> UIContextMenuConfiguration? {

0 commit comments

Comments
 (0)