From ff48a25186d062d9338fc4db31af9b757b8b5ee1 Mon Sep 17 00:00:00 2001 From: James Magahern Date: Fri, 16 Oct 2026 13:58:04 -0700 Subject: [PATCH] ios: simplify Agent transcript positioning and entrances --- .../Sybil/Sources/Sybil/SybilAgentView.swift | 289 +++++++----------- .../SybilUITests/TranscriptFadeUITests.swift | 73 +++++ 2 files changed, 181 insertions(+), 181 deletions(-) diff --git a/ios/Packages/Sybil/Sources/Sybil/SybilAgentView.swift b/ios/Packages/Sybil/Sources/Sybil/SybilAgentView.swift index 8a10090..e0b81f3 100644 --- a/ios/Packages/Sybil/Sources/Sybil/SybilAgentView.swift +++ b/ios/Packages/Sybil/Sources/Sybil/SybilAgentView.swift @@ -28,17 +28,11 @@ private struct AgentTranscriptView: View { @State private var scrollPhase: ScrollPhase = .idle @State private var historyLimit = 100 @State private var historyStartID: String? - @State private var scrollPosition = ScrollPosition() + private static let bottomID = "agent-bottom" @State private var scrollTracking = AgentScrollTracking() @State private var appeared = false private var isScrolling: Bool { scrollPhase.isScrolling } - private var visibleReadingAnchor: AgentReadingAnchor? { - scrollTracking.rowFrames.filter { $0.value.maxY > topInset && $0.value.minY < scrollTracking.metrics.height } - .min { $0.value.minY < $1.value.minY } - .map { AgentReadingAnchor(id: $0.key, offset: $0.value.minY) } - } - var body: some View { #if DEBUG let _ = AgentRenderDiagnostics.record("agentBody") @@ -48,132 +42,115 @@ private struct AgentTranscriptView: View { let allMessages = transcript.messages let startIndex = transcript.startIndex let displayed = transcript.displayed - ScrollView { - LazyVStack(alignment: .leading, spacing: 15) { - if allMessages.count > displayed.count { - Button("Load earlier messages") { - // Loading older history may move the viewport as lazy - // row estimates settle; do not chase the old position. - scrollTracking.rowFrames = [:] - atLatest = false - historyLimit += 100 - historyStartID = allMessages[max(0, startIndex - 100)].id + ScrollViewReader { proxy in + ScrollView { + LazyVStack(alignment: .leading, spacing: 15) { + if allMessages.count > displayed.count { + Button("Load earlier messages") { + // Loading older history may move the viewport as lazy + // row estimates settle; do not chase the old position. + atLatest = false + historyLimit += 100 + historyStartID = allMessages[max(0, startIndex - 100)].id + } + .frame(maxWidth: .infinity) + .padding(.vertical, 10) } - .frame(maxWidth: .infinity) - .padding(.vertical, 10) - } - if room.isLoading { ProgressView("Loading messages…").frame(maxWidth: .infinity) } - ForEach(Array(displayed.enumerated()), id: \.element.id) { index, message in - AgentTranscriptRow(message: message, room: room, tracksReadingAnchor: !atLatest, - startsDay: index == 0 || !Calendar.current.isDate(message.createdAt, inSameDayAs: displayed[index - 1].createdAt)) - .equatable() - .modifier(AgentEntrance(animate: appeared && rendered.entranceIDs.contains(message.id), reduceMotion: reduceMotion)) - .id(message.id) - } - if rendered.isTyping { - HStack(spacing: 9) { - AgentAvatar() - ProgressView().controlSize(.small) - Text("Sybil is thinking…").font(.sybil(.caption)) + if room.isLoading { ProgressView("Loading messages…").frame(maxWidth: .infinity) } + ForEach(Array(displayed.enumerated()), id: \.element.id) { index, message in + AgentTranscriptRow(message: message, room: room, + startsDay: index == 0 || !Calendar.current.isDate(message.createdAt, inSameDayAs: displayed[index - 1].createdAt)) + .equatable() + .modifier(AgentEntrance(animate: appeared && rendered.entranceIDs.contains(message.id), reduceMotion: reduceMotion)) + .id(message.id) } - .foregroundStyle(SybilTheme.textMuted) - .accessibilityIdentifier("agent-typing") + if rendered.isTyping { + HStack(spacing: 9) { + AgentAvatar() + ProgressView().controlSize(.small) + Text("Sybil is thinking…").font(.sybil(.caption)) + } + .foregroundStyle(SybilTheme.textMuted) + .accessibilityIdentifier("agent-typing") + } + // Include bottom padding in the target so Latest reaches + // the actual content end before native keyboard avoidance. + Color.clear.frame(height: 17).id(Self.bottomID) } - Color.clear.frame(height: 1) + // Keep native lazy-row targets without binding scroll position. + .scrollTargetLayout() + .padding(.horizontal, 16) + // Match normal chat: content clears the header, while the + // viewport extends beneath the shared navigation fade. + .padding(.top, 16 + topInset) + .frame(maxWidth: SybilLayout.webContentMaxWidth) + .frame(maxWidth: .infinity) } - .scrollTargetLayout() - .padding(.horizontal, 16) - .padding(.bottom, 16) - // Match normal chat: content clears the header, while the - // viewport extends beneath the shared navigation fade. - .padding(.top, 16 + topInset) - .frame(maxWidth: SybilLayout.webContentMaxWidth) - .frame(maxWidth: .infinity) - } - .accessibilityIdentifier("agent-transcript") - .coordinateSpace(name: "agent-scroll") - .scrollPosition($scrollPosition) - .defaultScrollAnchor(.bottom, for: .initialOffset) - // Let SwiftUI follow size changes, including keyboard avoidance, - // without competing scrollTo calls from geometry updates. - .defaultScrollAnchor(atLatest ? .bottom : nil, for: .sizeChanges) - .defaultScrollAnchor(.bottom, for: .alignment) - .scrollDismissesKeyboard(.interactively) - .onScrollPhaseChange { _, phase in - scrollPhase = phase - } - .onScrollGeometryChange(for: AgentScrollMetrics.self) { geometry in - AgentScrollMetrics(offset: geometry.contentOffset.y, - height: geometry.containerSize.height, - contentHeight: geometry.contentSize.height, - bottomInset: geometry.contentInsets.bottom, topInset: geometry.contentInsets.top, - visibleBottom: geometry.visibleRect.maxY) - } action: { _, next in - #if DEBUG - AgentRenderDiagnostics.record("scrollGeometry") - #endif - scrollTracking.metrics = next - if scrollTracking.atBottom != next.atBottom { scrollTracking.atBottom = next.atBottom } - // Only user scrolling changes follow intent. Animated Latest - // navigation and native keyboard resizing must not cancel it. - if (isScrolling && scrollPhase != .animating) || next.contentHeight <= next.height { - atLatest = next.atBottom + .accessibilityIdentifier("agent-transcript") + .defaultScrollAnchor(.bottom, for: .initialOffset) + // Let SwiftUI follow size changes, including keyboard avoidance, + // without competing scrollTo calls from geometry updates. + .defaultScrollAnchor(atLatest ? .bottom : nil, for: .sizeChanges) + .defaultScrollAnchor(.bottom, for: .alignment) + .scrollDismissesKeyboard(.interactively) + .onScrollPhaseChange { _, phase in + scrollPhase = phase } - } - .onPreferenceChange(AgentRowFrames.self) { frames in - #if DEBUG - AgentRenderDiagnostics.record("rowPreferences") - #endif - let anchor = !atLatest && !isScrolling ? visibleReadingAnchor : nil - if let anchor, let frame = frames[anchor.id], abs(frame.minY - anchor.offset) > 1 { - // Preserve ordinary reading position after new messages or - // text reflow. Normalize the saved frames to - // the requested offset so the next layout cannot undo it. - let requested = scrollTracking.metrics.offset + frame.minY - anchor.offset - let target = min(max(requested, -scrollTracking.metrics.topInset), scrollTracking.metrics.maximumOffset) - let appliedShift = target - scrollTracking.metrics.offset - scrollTracking.rowFrames = frames.mapValues { $0.offsetBy(dx: 0, dy: -appliedShift) } - if abs(appliedShift) > 1 { - // Geometry reports the raw offset; ScrollPosition uses - // zero at the top inset, including the safe area. - scrollPosition.scrollTo(y: target + scrollTracking.metrics.topInset) + .onScrollGeometryChange(for: Bool.self) { geometry in + geometry.containerSize.height > 0 && + geometry.contentSize.height + geometry.contentInsets.bottom - geometry.visibleRect.maxY <= 24 + } action: { _, atBottom in + #if DEBUG + AgentRenderDiagnostics.record("scrollGeometry") + #endif + scrollTracking.atBottom = atBottom + // Only user scrolling changes follow intent. Animated Latest + // navigation and native keyboard resizing must not cancel it. + if isScrolling && scrollPhase != .animating { + atLatest = atBottom } - } else { - scrollTracking.rowFrames = frames } - } - .overlay(alignment: .bottomTrailing) { - if !atLatest { - Button { + .overlay(alignment: .bottomTrailing) { + if !atLatest { + Button { + atLatest = true + withAnimation(reduceMotion ? nil : .easeOut(duration: 0.24)) { proxy.scrollTo(Self.bottomID, anchor: .bottom) } + } label: { + Label(room.unreadCount > 0 ? "\(room.unreadCount) unread" : "Latest", systemImage: "arrow.down") + .font(.sybil(.caption, weight: .semibold)) + .padding(12) + .background(SybilTheme.surfaceStrong, in: Capsule()) + .overlay(Capsule().stroke(SybilTheme.border)) + } + .padding(14) + .accessibilityIdentifier("agent-jump-latest") + } + } + .onAppear { + appeared = true + if !room.isLoading { + historyStartID = displayed.first?.id + proxy.scrollTo(Self.bottomID, anchor: .bottom) + } + } + .onChange(of: room.isLoading) { _, loading in + if !loading && historyStartID == nil { + historyStartID = displayed.first?.id + proxy.scrollTo(Self.bottomID, anchor: .bottom) + } + } + .onChange(of: allMessages.count) { _, _ in + if atLatest { proxy.scrollTo(Self.bottomID, anchor: .bottom) } + } + .onChange(of: latestRequest) { _, _ in + if !atLatest { atLatest = true - withAnimation(reduceMotion ? nil : .easeOut(duration: 0.24)) { scrollPosition.scrollTo(edge: .bottom) } - } label: { - Label(room.unreadCount > 0 ? "\(room.unreadCount) unread" : "Latest", systemImage: "arrow.down") - .font(.sybil(.caption, weight: .semibold)) - .padding(12) - .background(SybilTheme.surfaceStrong, in: Capsule()) - .overlay(Capsule().stroke(SybilTheme.border)) + proxy.scrollTo(Self.bottomID, anchor: .bottom) } - .padding(14) - .accessibilityIdentifier("agent-jump-latest") } + .modifier(AgentReadAcknowledgement(room: room, tracking: scrollTracking, + atLatest: atLatest, isScrolling: isScrolling)) } - .onAppear { - appeared = true - if !room.isLoading { historyStartID = displayed.first?.id } - } - .onChange(of: room.isLoading) { _, loading in - if !loading && historyStartID == nil { historyStartID = displayed.first?.id } - } - .onChange(of: allMessages.count) { _, _ in - if atLatest { scrollPosition.scrollTo(edge: .bottom) } - } - .onChange(of: latestRequest) { _, _ in - atLatest = true - scrollPosition.scrollTo(edge: .bottom) - } - .modifier(AgentReadAcknowledgement(room: room, tracking: scrollTracking, - atLatest: atLatest, isScrolling: isScrolling)) } } @@ -198,9 +175,9 @@ private struct AgentReadAcknowledgement: ViewModifier { .task(id: readKey) { // Capture the rendered cursor before scheduling. New network input // cancels this task and must settle in layout before acknowledgement. - guard atLatest, tracking.metrics.atBottom, !isScrolling, !room.isObscured, !room.isModalPresented, !room.isAttachmentModalPresented, scenePhase == .active, !room.isLoading else { return } + guard atLatest, tracking.atBottom, !isScrolling, !room.isObscured, !room.isModalPresented, !room.isAttachmentModalPresented, scenePhase == .active, !room.isLoading else { return } try? await Task.sleep(for: .milliseconds(reduceMotion ? 80 : 360)) - guard !Task.isCancelled, atLatest, tracking.metrics.atBottom, !isScrolling, !room.isObscured, !room.isModalPresented, !room.isAttachmentModalPresented, scenePhase == .active else { return } + guard !Task.isCancelled, atLatest, tracking.atBottom, !isScrolling, !room.isObscured, !room.isModalPresented, !room.isAttachmentModalPresented, scenePhase == .active else { return } room.markRead(renderedCursor: cursor) } } @@ -275,16 +252,14 @@ struct AgentAvatar: View { } // Activity/read publications still update the transcript. Completed rows depend -// only on message values, heading and whether a reading anchor is needed. +// only on message values and heading; scrolling needs no per-row geometry. private struct AgentTranscriptRow: View, Equatable { var message: Message var room: SybilAgentRoom - var tracksReadingAnchor: Bool var startsDay: Bool nonisolated static func == (lhs: Self, rhs: Self) -> Bool { lhs.message == rhs.message && lhs.room === rhs.room && lhs.startsDay == rhs.startsDay - && lhs.tracksReadingAnchor == rhs.tracksReadingAnchor } var body: some View { @@ -300,15 +275,6 @@ private struct AgentTranscriptRow: View, Equatable { .padding(.vertical, 6) } AgentMessageRow(message: message, room: room) - .background { - // Pinned-bottom layout needs only the scroll view's metrics. - // Rows report frames only while preserving a reading anchor. - if tracksReadingAnchor { - GeometryReader { geometry in - Color.clear.preference(key: AgentRowFrames.self, value: [message.id: geometry.frame(in: .named("agent-scroll"))]) - } - } - } } } } @@ -345,63 +311,24 @@ private struct AgentMessageRow: View { } } -// Animate both occupied height and content, so incoming rows push older history up. -private struct AgentGrowingLayout: Layout { - var progress: CGFloat - var animatableData: CGFloat { get { progress } set { progress = newValue } } - func sizeThatFits(proposal: ProposedViewSize, subviews: Subviews, cache: inout ()) -> CGSize { - let size = subviews.first?.sizeThatFits(ProposedViewSize(width: proposal.width, height: nil)) ?? .zero - return CGSize(width: proposal.width ?? size.width, height: size.height * progress) - } - func placeSubviews(in bounds: CGRect, proposal: ProposedViewSize, subviews: Subviews, cache: inout ()) { - subviews.first?.place(at: bounds.origin, proposal: ProposedViewSize(width: bounds.width, height: nil)) - } -} +// Keep the row's measured height stable for lazy layout. Animate its appearance +// without collapsing the scroll target while the composer or keyboard resizes. private struct AgentEntrance: ViewModifier { var animate: Bool var reduceMotion: Bool @State private var entered = false func body(content: Content) -> some View { let preparing = animate && !entered && !reduceMotion - AgentGrowingLayout(progress: preparing ? 0 : 1) { - content.opacity(preparing ? 0 : 1).offset(y: preparing ? 16 : 0) - } - .clipped() + content + .opacity(preparing ? 0 : 1) + .offset(y: preparing ? 16 : 0) .onAppear { withAnimation(animate && !reduceMotion ? .easeOut(duration: 0.24) : nil) { entered = true } } } } -// Pixel geometry is imperative anchor bookkeeping, not rendered content. Keeping -// it outside observation avoids rebuilding the entire transcript on every scroll -// frame. Only the at-bottom boundary invalidates the read acknowledgement. +// Only the bottom boundary invalidates read acknowledgement, not the transcript. @Observable private final class AgentScrollTracking { var atBottom = false - @ObservationIgnored var metrics = AgentScrollMetrics() - @ObservationIgnored var rowFrames: [String: CGRect] = [:] -} - -private struct AgentScrollMetrics: Equatable { - var offset: CGFloat = 0 - var height: CGFloat = 0 - var contentHeight: CGFloat = 0 - var bottomInset: CGFloat = 0 - var topInset: CGFloat = 0 - var visibleBottom: CGFloat = 0 - // visibleRect includes the viewport extended into the safe area, as well as - // the alignment inset used when a short transcript is pinned to the bottom. - var distanceFromBottom: CGFloat { contentHeight + bottomInset - visibleBottom } - var atBottom: Bool { height > 0 && distanceFromBottom <= 24 } - var maximumOffset: CGFloat { max(-topInset, offset + distanceFromBottom) } -} -private struct AgentReadingAnchor { - var id: String - var offset: CGFloat -} -private struct AgentRowFrames: PreferenceKey { - static let defaultValue: [String: CGRect] = [:] - static func reduce(value: inout [String: CGRect], nextValue: () -> [String: CGRect]) { - value.merge(nextValue(), uniquingKeysWith: { _, next in next }) - } } diff --git a/ios/Tests/SybilUITests/TranscriptFadeUITests.swift b/ios/Tests/SybilUITests/TranscriptFadeUITests.swift index c603ff0..68ddb77 100644 --- a/ios/Tests/SybilUITests/TranscriptFadeUITests.swift +++ b/ios/Tests/SybilUITests/TranscriptFadeUITests.swift @@ -55,6 +55,79 @@ final class TranscriptFadeUITests: XCTestCase { return try XCTUnwrap(JSONSerialization.jsonObject(with: data) as? [String: Any]) } + private func reloadMixedHistory() async throws { + app.terminate() + let rows = (1...160).map { index in + let short = "History message \(index): local simulator fixture." + return ["role": index % 3 == 0 ? "user" : "assistant", + "content": index == 160 || index % 3 == 0 ? short : + "### Investigation \(index)\n\n" + String(repeating: "A longer transcript paragraph with **emphasis** and `inline code`.\n\n", count: 4) + + "- Preserve visible content.\n- Keep the composer available.\n\n```swift\nrender(messages[\(index)])\n```"] + } + try await control(["action": "presentation-samples", "messages": rows]) + app.launch() + } + + func testInitialOpenAndTabReturnShowLatestWithoutScrolling() async throws { + try await reloadMixedHistory() + openAgent() + let latest = app.staticTexts["History message 160: local simulator fixture."] + XCTAssertTrue(latest.isHittable) + XCTAssertLessThan(latest.frame.maxY, composer.frame.minY) + capture("agent-initial-open") + + showSidebar() + app.buttons["Settings"].tap() + XCTAssertFalse(composer.exists) + openAgent() + XCTAssertTrue(latest.isHittable, "Returning to Agent must not need a scroll to materialize rows") + XCTAssertLessThan(latest.frame.maxY, composer.frame.minY) + capture("agent-tab-return") + } + + func testSendClearsMultilineComposerAndKeepsLatestAboveKeyboard() async throws { + try await reloadMixedHistory() + openAgent() + composer.tap() + if let existing = composer.value as? String, !existing.isEmpty, existing != "Enter Prompt" { + composer.press(forDuration: 1) + let selectAll = app.descendants(matching: .any).matching(NSPredicate(format: "label == %@", "Select All")).firstMatch + if selectAll.waitForExistence(timeout: 2) { selectAll.tap() } + else { composer.typeText(String(repeating: XCUIKeyboardKey.rightArrow.rawValue, count: existing.count)) } + composer.typeText(String(repeating: XCUIKeyboardKey.delete.rawValue, count: existing.count)) + } + let draft = "Keyboard send\nsecond line\nthird line" + composer.typeText(draft) + XCTAssertTrue(app.keyboards.firstMatch.exists) + let multilineHeight = composer.frame.height + app.buttons["agent-send"].tap() + let cleared = XCTNSPredicateExpectation(predicate: NSPredicate(format: "value == %@ OR value == %@", "", "Enter Prompt"), object: composer) + await fulfillment(of: [cleared], timeout: 5) + XCTAssertLessThan(composer.frame.height, multilineHeight) + XCTAssertTrue(app.keyboards.firstMatch.exists) + let sent = app.staticTexts[draft] + XCTAssertTrue(sent.waitForExistence(timeout: 5)) + XCTAssertTrue(sent.isHittable) + let reply = app.staticTexts.containing(NSPredicate(format: "label CONTAINS %@", "Finished: Keyboard send")).firstMatch + let followed = XCTNSPredicateExpectation(predicate: NSPredicate(format: "hittable == true"), object: reply) + await fulfillment(of: [followed], timeout: 10) + capture("agent-send-cleared-composer-with-keyboard") + XCTAssertGreaterThan(reply.frame.minY, transcript.frame.minY + 64) + XCTAssertLessThan(reply.frame.maxY, composer.frame.minY) + // Finished: is only the first Markdown paragraph. Measure the final + // reply content, leaving the same allowance for metadata and spacing. + let replyEnd = app.staticTexts["The local fixture returned three matches."] + XCTAssertTrue(replyEnd.isHittable) + XCTAssertLessThan(replyEnd.frame.maxY, composer.frame.minY) + let bottomGap = composer.frame.minY - replyEnd.frame.maxY + print("Agent send settled final-content gap: \(bottomGap)pt") + XCTAssertLessThan(bottomGap, 100, "Composer clearing must not leave the transcript shifted above the bottom") + XCTAssertTrue(app.buttons["agent-attach"].isHittable) + try await Task.sleep(for: .milliseconds(500)) + let read = try await control(["action": "state"]) + XCTAssertEqual(read["readCursor"] as? Int, read["cursor"] as? Int) + } + func testSharedHeaderFadeGeometry() async throws { showSidebar() let chat = app.buttons.containing(NSPredicate(format: "label CONTAINS %@", "Transcript fade comparison")).firstMatch