From 7fe81c7324c16659e843bf285eabb66d8c21219a Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 20:31:40 -0400 Subject: [PATCH 01/12] perf(ios): instant send, prepend anchoring, O(tail) streaming renders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tier 1A of the t3code competitor audit — three client-side changes to how the iOS chat feels. No protocol changes. Instant send. The user's bubble now paints on the tap frame: - Local echoes get a synchronous incremental apply instead of waiting out the 90 ms coalescing debounce that exists for host deltas. A retired generation counter keeps an in-flight coalesced rebuild from overwriting the just-painted bubble. - Image sends echo before the upload, not after. The composer's own downscaled image renders behind an uploading state under a placeholder ref, swapped for the real host path before the send so the echo's dedupe key still matches the transcript row that comes back. - `sending` releases when the host accepts the message. The four-round-trip refresh cascade (transcript, artifacts, summary, session) now runs behind the composer, chained so two quick sends cannot interleave two transcript loads. A second message can be composed immediately. Also fixes the incremental echo path dropping attachments, and gives it the echo-suppression rules the full rebuild applies, so the fast path and the full path agree. Prepend anchoring. Older history inserted above the viewport used to push whatever the reader was looking at down by the height of the inserted page. Scroll geometry is now captured before any presentation change that inserts rows above the current first row — covering both buffered reveal and host history pages — and the offset is restored by the measured height delta in a non-animated transaction. The reveal itself is no longer animated: those rows are off-screen and immediately offset-corrected, so animating them only produced a flash. Bottom-follow, the jump-to-latest pill, and the initial force-pin are untouched. Streaming render caches: - Syntax highlighting was the worst main-thread path in a long reply: the cache keyed on the full code text (new key per token) and applied attributes by walking `index(offsetBy:)` from the start for every token. It now reuses an already-highlighted stable prefix — the same shape `parseMarkdownBlocksForStreaming` uses for prose, split at the last line boundary provably outside a block comment or backtick/triple-quote string — and applies attributes with a single forward cursor. Measured over a 475-tick replay of an 11 KB block: 5.19 ms -> 0.105 ms per tick (49x). - Streaming tail revisions no longer land in the shared 256-entry inline markdown cache. One long turn used to insert hundreds of throwaway entries and evict every completed message, so scrolling back re-parsed the transcript on the main thread. Intermediate revisions render from their own small cache; the final revision is promoted. - Cache keys use the existing `workStableDigest` fingerprint instead of bridging whole strings, and every derived render cache purges on `didReceiveMemoryWarning`. Tests: streaming-equivalence property tests assert the incremental highlight equals a from-scratch highlight at every snapshot (these caught a real bug where the scanner state was stored at end-of-text rather than at the boundary, splitting inside a Python docstring), plus intermediate-exclusion, promote-on-complete, and a 40-message eviction regression. Co-Authored-By: Claude Opus 5 --- apps/ios/ADE/App/ADEAppDelegate.swift | 9 + .../Components/ADECodeRenderingCache.swift | 26 ++ .../Views/Components/FilesCodeSupport.swift | 173 ++++++++++- .../Views/Work/WorkChatAttachmentTray.swift | 75 ++++- .../Work/WorkChatSessionView+Actions.swift | 96 ++++++- .../Work/WorkChatSessionView+Timeline.swift | 2 +- .../ADE/Views/Work/WorkChatSessionView.swift | 105 ++++++- .../ADE/Views/Work/WorkMarkdownParsing.swift | 57 +++- .../ADE/Views/Work/WorkMarkdownViews.swift | 22 +- apps/ios/ADE/Views/Work/WorkModels.swift | 4 + .../WorkSessionDestinationView+Actions.swift | 66 ++++- .../Work/WorkSessionDestinationView.swift | 14 + .../WorkMarkdownStreamingParsingTests.swift | 271 ++++++++++++++++++ 13 files changed, 877 insertions(+), 43 deletions(-) diff --git a/apps/ios/ADE/App/ADEAppDelegate.swift b/apps/ios/ADE/App/ADEAppDelegate.swift index 6356f5246b..8f03e8764a 100644 --- a/apps/ios/ADE/App/ADEAppDelegate.swift +++ b/apps/ios/ADE/App/ADEAppDelegate.swift @@ -25,6 +25,15 @@ final class ADEAppDelegate: NSObject, UIApplicationDelegate { return true } + /// Transcript render caches (parsed Markdown blocks, inline attributed + /// strings, syntax highlighting) are all derived state — a long chat can + /// hold megabytes of it, and every entry can be rebuilt on demand. When the + /// system says it wants memory back, give it these first. + func applicationDidReceiveMemoryWarning(_ application: UIApplication) { + workPurgeMarkdownRenderCaches() + ADECodeRenderingCache.shared.purgeOnMemoryWarning() + } + /// Register the approval-alert category so approval pushes carry inline /// Approve / Deny actions on the lock screen and in Notification Center. The /// brain stamps `aps.category = "ADE_APPROVAL"` on those alerts; the action diff --git a/apps/ios/ADE/Views/Components/ADECodeRenderingCache.swift b/apps/ios/ADE/Views/Components/ADECodeRenderingCache.swift index 3ed0510cfc..0ce2af7454 100644 --- a/apps/ios/ADE/Views/Components/ADECodeRenderingCache.swift +++ b/apps/ios/ADE/Views/Components/ADECodeRenderingCache.swift @@ -8,6 +8,10 @@ final class ADECodeRenderingCache { private let attributedCache = NSCache() private let regexCache = NSCache() private let regexLock = NSLock() + /// Streaming state, not a cache: one in-progress code block per language. + /// Guarded by its own lock because highlighting is reachable from any actor. + private let prefixLock = NSLock() + private var highlightPrefixes: [FilesLanguage: SyntaxHighlightPrefix] = [:] private init() { tokenCache.countLimit = 64 @@ -31,6 +35,28 @@ final class ADECodeRenderingCache { attributedCache.setObject(AttributedStringBox(value: attributed), forKey: key as NSString) } + func highlightPrefix(for language: FilesLanguage) -> SyntaxHighlightPrefix? { + prefixLock.lock() + defer { prefixLock.unlock() } + return highlightPrefixes[language] + } + + func storeHighlightPrefix(_ prefix: SyntaxHighlightPrefix, for language: FilesLanguage) { + prefixLock.lock() + defer { prefixLock.unlock() } + highlightPrefixes[language] = prefix + } + + /// Drops derived renders. Compiled regexes are cheap to hold and hot on every + /// highlight, so they stay. + func purgeOnMemoryWarning() { + tokenCache.removeAllObjects() + attributedCache.removeAllObjects() + prefixLock.lock() + highlightPrefixes.removeAll() + prefixLock.unlock() + } + func regex(for pattern: String) -> NSRegularExpression? { let key = pattern as NSString if let cached = regexCache.object(forKey: key) { diff --git a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift index 0484248e78..ea9fd7a901 100644 --- a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift +++ b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift @@ -126,9 +126,94 @@ struct SyntaxToken: Identifiable, Equatable { let range: NSRange } +/// Whether a scan position sits inside a construct that can span lines. Every +/// token rule is line-anchored except block comments and backtick / triple-quote +/// strings, so a line boundary reached with all of these clear is a point no +/// later text can reinterpret. +/// +/// Ambiguity resolves toward "still inside": a false positive only stops the +/// stable prefix from growing (slower, still correct), while a false negative +/// would let a token straddle the split. +struct SyntaxNestingBalance: Equatable { + var insideBlockComment = false + var insideBacktick = false + var insideTripleQuote = false + + var isClear: Bool { + !insideBlockComment && !insideBacktick && !insideTripleQuote + } +} + +/// The already-highlighted stable prefix of the code block currently streaming +/// in a given language. It always ends just past a newline reached with no +/// multi-line construct open, so a resumed scan can start from a clear state. +struct SyntaxHighlightPrefix { + let text: String + let attributed: AttributedString +} + +/// Returns the position just past the last newline in `text[start...]` reached +/// with no multi-line construct open (never earlier than `start`). `start` is +/// itself such a position, so the scan begins from a clear state. +private func syntaxStableBoundary(in text: String, from start: String.Index) -> String.Index { + var balance = SyntaxNestingBalance() + var boundary = start + var index = start + while index < text.endIndex { + let character = text[index] + let next = text.index(after: index) + + if character == "\n" { + if balance.isClear { boundary = next } + index = next + continue + } + + if balance.insideBlockComment { + if character == "*", next < text.endIndex, text[next] == "/" { + balance.insideBlockComment = false + index = text.index(after: next) + continue + } + } else if balance.insideBacktick { + if character == "`" { balance.insideBacktick = false } + } else if balance.insideTripleQuote { + if syntaxIsTripleQuote(text, at: index) { + balance.insideTripleQuote = false + index = text.index(index, offsetBy: 3) + continue + } + } else if character == "/", next < text.endIndex, text[next] == "*" { + balance.insideBlockComment = true + index = text.index(after: next) + continue + } else if character == "`" { + balance.insideBacktick = true + } else if syntaxIsTripleQuote(text, at: index) { + balance.insideTripleQuote = true + index = text.index(index, offsetBy: 3) + continue + } + + index = next + } + return boundary +} + +private func syntaxIsTripleQuote(_ text: String, at index: String.Index) -> Bool { + let character = text[index] + guard character == "\"" || character == "'" else { return false } + var cursor = index + for _ in 0..<2 { + cursor = text.index(after: cursor) + guard cursor < text.endIndex, text[cursor] == character else { return false } + } + return true +} + struct SyntaxHighlighter { static func tokenize(_ text: String, as language: FilesLanguage) -> [SyntaxToken] { - let cacheKey = "tokens|\(language.rawValue)|\(text)" + let cacheKey = "tokens|\(language.rawValue)|\(workStableDigest(text))" if let cached = ADECodeRenderingCache.shared.tokens(for: cacheKey) { return cached } @@ -156,28 +241,98 @@ struct SyntaxHighlighter { return tokens } + /// Syntax-highlights a code block, incrementally while it is still streaming. + /// + /// A streaming block grows by a few characters per delta. Highlighting the + /// whole text each time is O(n) regex work *plus* O(n) attribute application + /// per token, which made a long block the most expensive main-thread path in + /// an agent reply. This mirrors what `parseMarkdownBlocksForStreaming` does + /// for prose: everything up to the last line boundary that is provably outside + /// a multi-line construct can never be re-interpreted by text arriving later, + /// so it is highlighted once and reused; only the growing tail is re-scanned. static func highlightedAttributedString(_ text: String, as language: FilesLanguage) -> AttributedString { - let cacheKey = "highlighted|\(language.rawValue)|\(text)" + let cacheKey = "highlighted|\(language.rawValue)|\(workStableDigest(text))" if let cached = ADECodeRenderingCache.shared.highlightedString(for: cacheKey) { return cached } + let attributed = incrementallyHighlighted(text, as: language) + ADECodeRenderingCache.shared.storeHighlightedString(attributed, for: cacheKey) + return attributed + } + + private static func incrementallyHighlighted( + _ text: String, + as language: FilesLanguage + ) -> AttributedString { + let reusable = ADECodeRenderingCache.shared.highlightPrefix(for: language) + .flatMap { prefix -> SyntaxHighlightPrefix? in + // Byte-prefix check: only a block that literally grew from this prefix + // may reuse it. A different block of the same language starts over. + guard !prefix.text.isEmpty, text.hasPrefix(prefix.text) else { return nil } + return prefix + } + + let reusedText = reusable?.text ?? "" + let scanStart = text.index(text.startIndex, offsetBy: reusedText.count) + let boundary = syntaxStableBoundary(in: text, from: scanStart) + + var attributed = reusable?.attributed ?? AttributedString() + if boundary > scanStart { + attributed.append(highlightedSegment(text[scanStart.. AttributedString { + let text = String(segment) var attributed = AttributedString(text) attributed.font = .system(.body, design: .monospaced) attributed.foregroundColor = ADEColor.textPrimary + var cursorOffset = 0 + var cursor = attributed.startIndex for token in tokenize(text, as: language) { guard let stringRange = Range(token.range, in: text) else { continue } let startOffset = text.distance(from: text.startIndex, to: stringRange.lowerBound) let endOffset = text.distance(from: text.startIndex, to: stringRange.upperBound) - let lowerBound = attributed.characters.index(attributed.startIndex, offsetBy: startOffset) - let upperBound = attributed.characters.index(attributed.startIndex, offsetBy: endOffset) - let attributeRange = lowerBound.. Bool { + ref.path.hasPrefix(workPendingUploadPathPrefix) +} + +/// Holds the composer's already-downscaled `UIImage` for each in-flight upload +/// so the echo's thumbnail resolves without touching the host. Entries are +/// released as soon as the save returns (or the send fails) — a handoff buffer, +/// not a cache. +@MainActor +final class WorkPendingUploadPreviewStore { + static let shared = WorkPendingUploadPreviewStore() + + private var imagesByPath: [String: UIImage] = [:] + + private init() {} + + func register(_ attachments: [WorkChatInputAttachment]) -> [AgentChatFileRef] { + attachments.map { attachment in + let ref = AgentChatFileRef( + path: "\(workPendingUploadPathPrefix)\(attachment.id.uuidString)", + type: "image" + ) + if let image = attachment.image { + imagesByPath[ref.path] = image + } + return ref + } + } + + func image(forPath path: String) -> UIImage? { + imagesByPath[path] + } + + func release(_ refs: [AgentChatFileRef]) { + for ref in refs where workAttachmentIsPendingUpload(ref) { + imagesByPath.removeValue(forKey: ref.path) + } + } +} + func workChatAttachmentIsImage(_ ref: AgentChatFileRef) -> Bool { let type = ref.type.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() return type == "image" || type == "image-url" @@ -674,6 +722,10 @@ private struct WorkChatAttachmentChip: View { @State private var previewImage: UIImage? @State private var loadFailed = false + private var isUploading: Bool { + workAttachmentIsPendingUpload(attachment) + } + var body: some View { Group { if workChatAttachmentIsImage(attachment) { @@ -703,19 +755,31 @@ private struct WorkChatAttachmentChip: View { .scaledToFill() .frame(width: size, height: size) .clipShape(RoundedRectangle(cornerRadius: 8, style: .continuous)) + .opacity(isUploading ? 0.55 : 1) + .overlay { + if isUploading { + ProgressView() + .controlSize(.small) + .tint(Color.white) + } + } } else { VStack(spacing: 4) { Image(systemName: loadFailed ? "photo.badge.exclamationmark" : "photo") .font(.system(size: 18, weight: .semibold)) .foregroundStyle(Color.white.opacity(0.82)) - Text(loadFailed ? "On desktop" : "Image") + Text(loadFailed ? "On desktop" : (isUploading ? "Sending" : "Image")) .font(.system(size: 9, weight: .semibold)) .foregroundStyle(Color.white.opacity(0.72)) .lineLimit(1) } } } - .accessibilityLabel("Image attachment \(workChatAttachmentDisplayName(attachment))") + .accessibilityLabel( + isUploading + ? "Image attachment, sending" + : "Image attachment \(workChatAttachmentDisplayName(attachment))" + ) } private var fileChip: some View { @@ -742,6 +806,13 @@ private struct WorkChatAttachmentChip: View { @MainActor private func loadPreviewIfNeeded() async { guard workChatAttachmentIsImage(attachment) else { return } + // Still uploading: the composer's downscaled image is already in memory, so + // the echo's thumbnail resolves without a host round-trip. + if workAttachmentIsPendingUpload(attachment) { + previewImage = WorkPendingUploadPreviewStore.shared.image(forPath: attachment.path) + loadFailed = false + return + } let maxPixelSize = max(workChatAttachmentPreviewMinimumPixels, ceil(size * displayScale)) if attachment.type == "image-url", let urlString = attachment.url, let url = URL(string: urlString), let scheme = url.scheme?.lowercased(), diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift index 0b31daab23..3ce96a4dfb 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift @@ -139,6 +139,35 @@ private func workSnapshotByApplyingAssistantTextTail( return nextSnapshot } +/// Mirrors the echo-suppression rules the full rebuild applies +/// (`buildWorkTimeline`) and `reconcileLocalEchoMessages`: an echo whose text + +/// attachments already appear as a delivered user message, or as a pending +/// steer, must not get its own row. +private func workTranscriptAlreadyRepresents( + _ echoes: ArraySlice, + transcript: [WorkChatEnvelope] +) -> Bool { + let echoKeys = Set(echoes.compactMap { workLocalEchoDedupeKey(text: $0.text, attachments: $0.attachments) }) + guard !echoKeys.isEmpty else { return false } + + for steer in derivePendingWorkSteers(from: transcript) { + if let key = workLocalEchoDedupeKey(text: steer.text, attachments: steer.attachments), + echoKeys.contains(key) { + return true + } + } + for envelope in transcript { + guard case .userMessage(let text, let attachments, _, let steerId, let deliveryState, _) = envelope.event else { + continue + } + if deliveryState == "queued", steerId != nil { continue } + if let key = workLocalEchoDedupeKey(text: text, attachments: attachments), echoKeys.contains(key) { + return true + } + } + return false +} + private func workSnapshotByApplyingLocalEchoTail( to snapshot: WorkChatTimelineSnapshot, cache: WorkTimelineIncrementalCache, @@ -163,8 +192,14 @@ private func workSnapshotByApplyingLocalEchoTail( else { return nil } } + let appendedEchoes = localEchoMessages[cache.localEchoCount.. Bool { + guard !localEchoMessages.isEmpty else { return false } + guard timelineSourceKey == (selectedSubagentTaskId ?? "main") else { return false } + + // A brand-new chat has no snapshot to append to; the fold is trivially + // cheap there, so build it inline rather than wait out the debounce. + if timelineSnapshot.timeline.isEmpty { + cancelScheduledTimelineSnapshotRebuild() + rebuildTimelineSnapshot() + return !timelineSnapshot.timeline.isEmpty + } + + guard let nextSnapshot = workSnapshotByApplyingLocalEchoTail( + to: timelineSnapshot, + cache: timelineIncrementalCache, + transcript: transcript, + fallbackEntries: fallbackEntries, + artifacts: artifacts, + localEchoMessages: localEchoMessages + ) else { return false } + + // A coalesced rebuild may already be inside the fold with inputs captured + // before this echo existed. Retire that generation so its result is dropped + // instead of overwriting the bubble we are about to paint. + timelineRebuildGeneration += 1 + + timelineSnapshot = nextSnapshot + timelineIncrementalCache.record( + transcript: transcript, + fallbackEntries: fallbackEntries, + artifacts: artifacts, + localEchoMessages: localEchoMessages + ) + refreshTimelinePresentation(sourceTimeline: nextSnapshot.timeline) + if isNearBottom, !timelineDragActive { + timelineLayoutPinToken &+= 1 + } + return true + } + @MainActor func scheduleTimelineSnapshotRebuild() { resetTimelineSourceIfNeeded() @@ -818,7 +903,12 @@ extension WorkChatSessionView { olderHistoryLoadError = nil let revealedBufferedEntries = hiddenTimelineCount > 0 if hiddenTimelineCount > 0 { - withAnimation(ADEMotion.quick(reduceMotion: reduceMotion)) { + // Deliberately not animated: these rows land *above* the viewport and are + // immediately offset-corrected, so animating them only produces a visible + // flash of the content sliding down and back. + var transaction = Transaction() + transaction.disablesAnimations = true + withTransaction(transaction) { visibleTimelineCount += workTimelinePageSize refreshTimelinePresentation() } diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift index 55c3caf96a..a1ae31ff25 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift @@ -381,7 +381,7 @@ struct WorkAssistantMarkdownBlockRow: View, Equatable { } var body: some View { - WorkMarkdownBlockView(block: model.block) + WorkMarkdownBlockView(block: model.block, isStreamingTail: model.isStreamingTail) .frame(maxWidth: .infinity, alignment: .leading) .contextMenu { Button(action: onCopyMessage) { diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift index 4a1b0505b4..25ae81632a 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift @@ -82,10 +82,43 @@ func workChatShouldContinueAutomaticOlderHistory( && (hasBufferedEntries || hasHostHistory) } +/// Scroll geometry a prepend has to preserve: where the reader was, and how +/// tall the content was, at the instant rows were inserted above them. +struct WorkChatPrependAnchor { + let offsetY: CGFloat + let contentHeight: CGFloat + /// Geometry callbacks to wait for before giving up. Content height is part of + /// the observed value, so a stationary reader burns none of these — the budget + /// only exists so an abandoned prepend can't leave the anchor armed to fire on + /// unrelated growth later. + var remainingAttempts: Int +} + +/// Reference box so scroll geometry can be recorded per frame without +/// invalidating the view. `distanceFromBottom` predates the prepend anchor and +/// keeps its existing meaning. final class WorkChatScrollMetrics { var distanceFromBottom: CGFloat = 0 + var contentHeight: CGFloat = 0 + var offsetY: CGFloat = 0 + var prependAnchor: WorkChatPrependAnchor? +} + +/// The slice of `ScrollGeometry` the transcript reacts to. Rounded so sub-pixel +/// jitter doesn't wake the observer. +struct WorkChatScrollGeometrySample: Equatable { + let offsetY: CGFloat + let contentHeight: CGFloat + + init(_ geometry: ScrollGeometry) { + self.offsetY = (geometry.contentOffset.y * 2).rounded() / 2 + self.contentHeight = (geometry.contentSize.height * 2).rounded() / 2 + } } +/// Number of geometry callbacks a prepend anchor stays armed for. +let workChatPrependAnchorAttempts = 12 + struct WorkChatSummaryRenderContext: Equatable { let isAvailable: Bool let provider: String @@ -213,6 +246,9 @@ struct WorkChatSessionView: View { @State var scrollViewportWidth: CGFloat = 0 @State var composerLayoutHeight: CGFloat = 150 @State var scrollMetrics = WorkChatScrollMetrics() + /// Only ever written to restore the reader's position after a prepend. Bottom + /// follow and the jump-to-latest pill keep using `ScrollViewProxy.scrollTo`. + @State var scrollPosition = ScrollPosition() @State var timelineDragActive = false @State var bottomStickinessReleasedByUser = false @State var timelineSnapshot = WorkChatTimelineSnapshot.empty @@ -635,9 +671,60 @@ struct WorkChatSessionView: View { ) } guard nextPresentation != timelinePresentation else { return } + armPrependAnchorIfRowsInsertedAbove(nextPresentation) timelinePresentation = nextPresentation } + /// Records where the reader is whenever the next presentation inserts rows + /// above the ones already on screen — whether that came from revealing locally + /// buffered entries or from an older page landing from the host. Without this + /// the LazyVStack grows upward, `contentOffset` stays put, and whatever the + /// user was reading slides down by the height of the inserted page. + @MainActor + private func armPrependAnchorIfRowsInsertedAbove(_ nextPresentation: WorkTimelinePresentation) { + guard nextPresentation.visibleEntries.count > timelinePresentation.visibleEntries.count, + let previousFirstId = timelinePresentation.visibleEntries.first?.id, + nextPresentation.visibleEntries.first?.id != previousFirstId, + scrollMetrics.contentHeight > 0 + else { return } + // Only a genuine prepend: the row that used to lead the list has to still be + // in the list, just further down. + guard nextPresentation.visibleEntries.contains(where: { $0.id == previousFirstId }) else { return } + + scrollMetrics.prependAnchor = WorkChatPrependAnchor( + offsetY: scrollMetrics.offsetY, + contentHeight: scrollMetrics.contentHeight, + remainingAttempts: workChatPrependAnchorAttempts + ) + } + + /// Re-applies the reader's position after a prepend lands. Content grew only + /// above the viewport, so the height delta *is* the distance the reader was + /// pushed down; adding it back to the captured offset puts the previously + /// first-visible row at exactly the same screen y. + @MainActor + private func restorePrependAnchorIfNeeded(_ sample: WorkChatScrollGeometrySample) { + guard var anchor = scrollMetrics.prependAnchor else { return } + + let insertedHeight = sample.contentHeight - anchor.contentHeight + guard insertedHeight > 0.5 else { + anchor.remainingAttempts -= 1 + scrollMetrics.prependAnchor = anchor.remainingAttempts > 0 ? anchor : nil + return + } + + scrollMetrics.prependAnchor = nil + // Bottom-follow owns the scroll position when the reader is parked at the + // tail; a prepend there is invisible anyway. + guard !isNearBottom else { return } + + var transaction = Transaction() + transaction.disablesAnimations = true + withTransaction(transaction) { + scrollPosition.scrollTo(y: anchor.offsetY + insertedHeight) + } + } + var canCompose: Bool { // Typing stays available so users can draft while disconnected or while // a turn is running, except when a blocking pending-input card is open — @@ -1066,6 +1153,16 @@ struct WorkChatSessionView: View { .clipped() .scrollIndicators(.hidden) .scrollDismissesKeyboard(.interactively) + .scrollPosition($scrollPosition) + .onScrollGeometryChange(for: WorkChatScrollGeometrySample.self) { geometry in + WorkChatScrollGeometrySample(geometry) + } action: { _, sample in + // Recorded into a reference box, not @State: this fires per scroll + // frame and must not invalidate the transcript. + scrollMetrics.offsetY = sample.offsetY + scrollMetrics.contentHeight = sample.contentHeight + restorePrependAnchorIfNeeded(sample) + } .coordinateSpace(name: workChatScrollCoordinateSpace) .background( GeometryReader { geometry in @@ -1294,6 +1391,10 @@ struct WorkChatSessionView: View { scheduleTimelineSnapshotRebuild() } .onChange(of: localEchoMessages) { _, _ in + // The user's own message is the one timeline change that must not wait + // out the coalescing debounce — it has to be on screen by the frame + // after the tap. + guard !applyLocalEchoTailImmediatelyIfPossible() else { return } scheduleTimelineSnapshotRebuild() } .onChange(of: blockingPendingInputId) { _, newId in @@ -1931,13 +2032,15 @@ func workTimelineRenderEntries( ? parseMarkdownBlocksForStreaming(preview.text, cacheKey: "\(message.id):preview") : parseMarkdownBlocks(preview.text) rendered.reserveCapacity(rendered.count + blocks.count + (preview.isTruncated ? 1 : 0)) + let streamingTailBlockId = message.id == streamingAssistantMessageId ? blocks.last?.id : nil for block in blocks { let model = WorkAssistantMarkdownBlockRenderModel( id: "\(entry.id)-\(block.id)", messageId: message.id, turnId: message.turnId, itemId: message.itemId, - block: block + block: block, + isStreamingTail: block.id == streamingTailBlockId ) rendered.append(WorkTimelineRenderEntry( id: model.id, diff --git a/apps/ios/ADE/Views/Work/WorkMarkdownParsing.swift b/apps/ios/ADE/Views/Work/WorkMarkdownParsing.swift index 141d616cab..d099e2c520 100644 --- a/apps/ios/ADE/Views/Work/WorkMarkdownParsing.swift +++ b/apps/ios/ADE/Views/Work/WorkMarkdownParsing.swift @@ -92,7 +92,7 @@ struct WorkMarkdownBlock: Identifiable, Equatable { } func parseMarkdownBlocks(_ markdown: String) -> [WorkMarkdownBlock] { - let key = markdown as NSString + let key = workStableDigest(markdown) as NSString if let cached = workMarkdownBlocksCache.object(forKey: key) { return cached.value } @@ -400,6 +400,15 @@ private let workMarkdownBlocksCache: NSCache = { + let cache = NSCache() + cache.countLimit = 8 + return cache +}() + /// Per-message state for `parseMarkdownBlocksForStreaming`, keyed by message /// id. Immutable snapshot box (replaced wholesale on each delta) so concurrent /// readers never observe a half-updated entry. Only one message streams at a @@ -433,11 +442,33 @@ func workStableDigest(_ string: String) -> String { return String(hash, radix: 16, uppercase: false) } -func markdownAttributedString(_ text: String) -> AttributedString { - let key = text as NSString +/// Renders inline Markdown, with a separate lane for the revision that is still +/// growing. +/// +/// `intermediate` marks the tail block of a streaming message: it is re-rendered +/// several times a second with text that will never be looked up again. Those +/// revisions used to land in the shared 256-entry cache, so one long turn could +/// insert hundreds of throwaway entries and evict every completed message — +/// scrolling back after a turn then re-parsed the whole transcript on the main +/// thread. Intermediate revisions now live in their own tiny cache and never +/// displace finished work; when the turn ends the same text comes back through +/// the normal path and is promoted into the shared cache. +func markdownAttributedString(_ text: String, intermediate: Bool = false) -> AttributedString { + let key = workStableDigest(text) as NSString if let cached = workMarkdownCache.object(forKey: key) { return cached.value } + if intermediate, let cached = workStreamingInlineMarkdownCache.object(forKey: key) { + return cached.value + } + + func store(_ value: AttributedString) { + if intermediate { + workStreamingInlineMarkdownCache.setObject(WorkMarkdownCacheBox(value), forKey: key) + } else { + workMarkdownCache.setObject(WorkMarkdownCacheBox(value), forKey: key) + } + } // Preserve line breaks so multi-line paragraphs render correctly — the // default `AttributedString(markdown:)` initializer collapses them. @@ -446,7 +477,7 @@ func markdownAttributedString(_ text: String) -> AttributedString { ) guard var attributed = try? AttributedString(markdown: text, options: options) else { let fallback = AttributedString(text) - workMarkdownCache.setObject(WorkMarkdownCacheBox(fallback), forKey: key) + store(fallback) return fallback } @@ -489,6 +520,22 @@ func markdownAttributedString(_ text: String) -> AttributedString { } } - workMarkdownCache.setObject(WorkMarkdownCacheBox(attributed), forKey: key) + store(attributed) return attributed } + +/// Whether the shared inline-markdown cache is currently holding a render for +/// `text`. Exists so the "streaming tail must not evict finished messages" +/// behaviour is directly assertable. +func workMarkdownSharedCacheHolds(_ text: String) -> Bool { + workMarkdownCache.object(forKey: workStableDigest(text) as NSString) != nil +} + +/// Drops every derived-render cache. Called on `didReceiveMemoryWarning`: these +/// hold parsed copies of the transcript, all of which can be rebuilt on demand. +func workPurgeMarkdownRenderCaches() { + workMarkdownCache.removeAllObjects() + workMarkdownBlocksCache.removeAllObjects() + workStreamingMarkdownCache.removeAllObjects() + workStreamingInlineMarkdownCache.removeAllObjects() +} diff --git a/apps/ios/ADE/Views/Work/WorkMarkdownViews.swift b/apps/ios/ADE/Views/Work/WorkMarkdownViews.swift index 7ded11de61..4fe37b006b 100644 --- a/apps/ios/ADE/Views/Work/WorkMarkdownViews.swift +++ b/apps/ios/ADE/Views/Work/WorkMarkdownViews.swift @@ -4,9 +4,12 @@ import AVKit struct WorkInlineMarkdownText: View { let text: String + /// Set on the one block that is still growing, so its throwaway revisions + /// stay out of the shared inline-markdown cache. + var isStreamingTail = false var body: some View { - Text(markdownAttributedString(text)) + Text(markdownAttributedString(text, intermediate: isStreamingTail)) .foregroundStyle(ADEColor.textPrimary) .tint(ADEColor.accent) .frame(maxWidth: .infinity, alignment: .leading) @@ -31,9 +34,13 @@ struct WorkMarkdownRenderer: View { } var body: some View { + let blocks = self.blocks + // Only the last block of a streaming message is still growing; everything + // above it is final and belongs in the shared caches. + let streamingTailId = streamingCacheKey == nil ? nil : blocks.last?.id VStack(alignment: .leading, spacing: 10) { ForEach(blocks) { block in - WorkMarkdownBlockView(block: block) + WorkMarkdownBlockView(block: block, isStreamingTail: block.id == streamingTailId) } } } @@ -41,13 +48,14 @@ struct WorkMarkdownRenderer: View { struct WorkMarkdownBlockView: View { let block: WorkMarkdownBlock + var isStreamingTail = false var body: some View { switch block.kind { case .paragraph(let text): - WorkInlineMarkdownText(text: text) + WorkInlineMarkdownText(text: text, isStreamingTail: isStreamingTail) case .heading(let level, let text): - WorkInlineMarkdownText(text: text) + WorkInlineMarkdownText(text: text, isStreamingTail: isStreamingTail) .font(headingFont(level: level)) case .unorderedList(let items): VStack(alignment: .leading, spacing: 6) { @@ -55,7 +63,7 @@ struct WorkMarkdownBlockView: View { HStack(alignment: .top, spacing: 8) { Text("•") .foregroundStyle(ADEColor.accent) - WorkInlineMarkdownText(text: item) + WorkInlineMarkdownText(text: item, isStreamingTail: isStreamingTail) } } } @@ -65,7 +73,7 @@ struct WorkMarkdownBlockView: View { HStack(alignment: .top, spacing: 8) { Text("\(start + index).") .foregroundStyle(ADEColor.accent) - WorkInlineMarkdownText(text: item) + WorkInlineMarkdownText(text: item, isStreamingTail: isStreamingTail) } } } @@ -76,7 +84,7 @@ struct WorkMarkdownBlockView: View { .frame(width: 3) VStack(alignment: .leading, spacing: 4) { ForEach(Array(lines.enumerated()), id: \.offset) { _, line in - WorkInlineMarkdownText(text: line) + WorkInlineMarkdownText(text: line, isStreamingTail: isStreamingTail) } } } diff --git a/apps/ios/ADE/Views/Work/WorkModels.swift b/apps/ios/ADE/Views/Work/WorkModels.swift index ad3711ef39..716ac71dd9 100644 --- a/apps/ios/ADE/Views/Work/WorkModels.swift +++ b/apps/ios/ADE/Views/Work/WorkModels.swift @@ -655,12 +655,16 @@ struct WorkAssistantMarkdownBlockRenderModel: Identifiable, Equatable { let turnId: String? let itemId: String? let block: WorkMarkdownBlock + /// The one block still receiving deltas. Its renders are throwaway, so they + /// are kept out of the shared inline-markdown cache. + var isStreamingTail = false static func == (lhs: WorkAssistantMarkdownBlockRenderModel, rhs: WorkAssistantMarkdownBlockRenderModel) -> Bool { lhs.id == rhs.id && lhs.messageId == rhs.messageId && lhs.turnId == rhs.turnId && lhs.itemId == rhs.itemId + && lhs.isStreamingTail == rhs.isStreamingTail && lhs.block == rhs.block } } diff --git a/apps/ios/ADE/Views/Work/WorkSessionDestinationView+Actions.swift b/apps/ios/ADE/Views/Work/WorkSessionDestinationView+Actions.swift index 9f04a8115a..338359d9ec 100644 --- a/apps/ios/ADE/Views/Work/WorkSessionDestinationView+Actions.swift +++ b/apps/ios/ADE/Views/Work/WorkSessionDestinationView+Actions.swift @@ -31,6 +31,25 @@ extension WorkSessionDestinationView { guard !text.isEmpty else { return false } guard canSendChatMessages else { return false } + // The echo goes up before the upload, not after it. Saving attachments is a + // per-image round-trip to the host; waiting for it left the tap with no + // visible result for seconds. Placeholder refs render the composer's own + // downscaled image with an uploading state, then get swapped for the real + // host paths before anything is sent. + let pendingUploadRefs = WorkPendingUploadPreviewStore.shared.register( + workChatInputReadyAttachments(inputAttachments) + ) + let initialDeliveryState = (sendWillQueueChatMessage || useSteer) ? "queued" : "sending" + let echo = WorkLocalEchoMessage( + text: text, + timestamp: workDateFormatter.string(from: Date()), + deliveryState: initialDeliveryState, + attachments: pendingUploadRefs.isEmpty ? nil : pendingUploadRefs + ) + let echoId = echo.id + localEchoMessages.append(echo) + sending = true + let attachmentRefs: [AgentChatFileRef] do { attachmentRefs = try await workChatSaveInputAttachments( @@ -39,21 +58,19 @@ extension WorkSessionDestinationView { chatSessionId: sessionId ) } catch { + sending = false + WorkPendingUploadPreviewStore.shared.release(pendingUploadRefs) + localEchoMessages.removeAll { $0.id == echoId } ADEHaptics.error() errorMessage = error.localizedDescription return false } + // Swap placeholders for host paths before the send: the echo's dedupe key + // (text + attachment refs) has to match the transcript row that comes back, + // or reconciliation would leave a duplicate bubble behind. + updateLocalEchoAttachments(echoId: echoId, attachments: attachmentRefs.isEmpty ? nil : attachmentRefs) + WorkPendingUploadPreviewStore.shared.release(pendingUploadRefs) - let initialDeliveryState = (sendWillQueueChatMessage || useSteer) ? "queued" : "sending" - let echo = WorkLocalEchoMessage( - text: text, - timestamp: workDateFormatter.string(from: Date()), - deliveryState: initialDeliveryState, - attachments: attachmentRefs.isEmpty ? nil : attachmentRefs - ) - let echoId = echo.id - localEchoMessages.append(echo) - sending = true defer { sending = false } do { let delivery: SyncChatMessageDelivery @@ -100,13 +117,12 @@ extension WorkSessionDestinationView { timestamp: echo.timestamp, attachments: attachmentRefs.isEmpty ? nil : attachmentRefs ) - await refreshChatStateAfterAction(forceRemote: true) + scheduledPostSendReconciliation(reconcileLocalEchoes: false) errorMessage = "Couldn’t send immediately. The message is still queued." return true } updateLocalEchoDeliveryState(echoId: echoId, deliveryState: nil) - await refreshChatStateAfterAction(forceRemote: true) - reconcileLocalEchoMessages() + scheduledPostSendReconciliation() break } updateLocalEchoDeliveryState(echoId: echoId, deliveryState: "queued") @@ -120,8 +136,7 @@ extension WorkSessionDestinationView { } case .sent: updateLocalEchoDeliveryState(echoId: echoId, deliveryState: nil) - await refreshChatStateAfterAction(forceRemote: true) - reconcileLocalEchoMessages() + scheduledPostSendReconciliation() case .dropped: // The steer queue is full; the host dropped the message (and emitted its // own transcript notice). Pull the optimistic echo so it doesn't linger @@ -145,6 +160,27 @@ extension WorkSessionDestinationView { } } + /// Runs the post-send refresh cascade (transcript → artifacts → summary → + /// session) behind the composer instead of in front of it. + /// + /// The host has already accepted the message at this point; holding `sending` + /// through four serial round-trips kept the spinner up and the composer gated + /// for the whole cascade. Chained onto the previous post-send refresh so two + /// quick sends can't interleave two transcript loads. + @MainActor + func scheduledPostSendReconciliation(reconcileLocalEchoes: Bool = true) { + let previous = postSendRefreshTask + postSendRefreshTask = Task { @MainActor in + await previous?.value + guard !Task.isCancelled else { return } + await refreshChatStateAfterAction(forceRemote: true) + guard !Task.isCancelled else { return } + if reconcileLocalEchoes { + reconcileLocalEchoMessages() + } + } + } + @MainActor func interruptSession(mode: AgentChatStopMode = .stopAndClear) async { do { diff --git a/apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift b/apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift index 1003c573bd..0199de0337 100644 --- a/apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift +++ b/apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift @@ -595,6 +595,11 @@ struct WorkSessionDestinationView: View { @State var artifacts: [ComputerUseArtifactSummary] = [] @State var artifactsRenderSignature = 0 @State var localEchoMessages: [WorkLocalEchoMessage] = [] + /// Post-send reconciliation runs behind the composer rather than in front of + /// it, so `sending` can drop the moment the host accepts the message. Chained + /// rather than fire-and-forget: two quick sends must not interleave two + /// transcript loads. + @State var postSendRefreshTask: Task? @State var optimisticPendingSteers: [WorkPendingSteerModel] = [] @State var subagentSnapshots: [WorkSubagentSnapshot] = [] @State var remoteSubagentSnapshots: [WorkSubagentSnapshot] = [] @@ -1312,6 +1317,8 @@ struct WorkSessionDestinationView: View { self.announcedLaneId = nil } cleanupLoadedArtifactContent() + postSendRefreshTask?.cancel() + postSendRefreshTask = nil let wasCrossProject = isCrossProject let wasPersonalChat = personalChat if wasCrossProject || wasPersonalChat { @@ -2953,6 +2960,13 @@ struct WorkSessionDestinationView: View { localEchoMessages[index].deliveryState = deliveryState } + @MainActor + func updateLocalEchoAttachments(echoId: String, attachments: [AgentChatFileRef]?) { + guard let index = localEchoMessages.firstIndex(where: { $0.id == echoId }) else { return } + guard localEchoMessages[index].attachments != attachments else { return } + localEchoMessages[index].attachments = attachments + } + @MainActor func pollIfNeeded() async { guard isLiveAndReachable, diff --git a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift index 77d4c6ff37..5ce8006c35 100644 --- a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift +++ b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift @@ -1,3 +1,4 @@ +import SwiftUI import XCTest @testable import ADE @@ -166,3 +167,273 @@ final class WorkMarkdownStreamingParsingTests: XCTestCase { ) } } + +/// The syntax highlighter reuses an already-highlighted stable prefix while a +/// code block streams. The property that has to hold is the same one the +/// markdown parser is held to: at every snapshot, the incremental render must +/// equal a from-scratch render of the same text. +final class SyntaxHighlighterStreamingTests: XCTestCase { + override func setUp() { + super.setUp() + // Streaming prefix state is process-wide; start each case cold. + ADECodeRenderingCache.shared.purgeOnMemoryWarning() + } + + private func assertIncrementalMatchesFullHighlight( + _ fullText: String, + as language: FilesLanguage, + deltaSizes: [Int] = [1, 4, 11], + file: StaticString = #filePath, + line: UInt = #line + ) { + var snapshot = "" + var remainder = Substring(fullText) + var sizeIndex = 0 + while !remainder.isEmpty { + snapshot += remainder.prefix(deltaSizes[sizeIndex % deltaSizes.count]) + remainder = remainder.dropFirst(deltaSizes[sizeIndex % deltaSizes.count]) + sizeIndex += 1 + + let incremental = SyntaxHighlighter.highlightedAttributedString(snapshot, as: language) + let reference = SyntaxHighlighter.highlightedSegment(Substring(snapshot), as: language) + if incremental != reference { + XCTFail( + """ + Highlight mismatch at snapshot length \(snapshot.count). + Snapshot: \(snapshot.debugDescription) + First differing run: \(Self.firstRunDifference(incremental, reference) ?? "") + """, + file: file, line: line + ) + return + } + } + } + + /// Reports the first run whose text or attributes diverge, so a failure names + /// the construct that broke rather than dumping two whole documents. + private static func firstRunDifference( + _ lhs: AttributedString, + _ rhs: AttributedString + ) -> String? { + let lhsRuns = Array(lhs.runs) + let rhsRuns = Array(rhs.runs) + for index in 0.. Int { + value += amount + return value + } + } + """, + as: .swift + ) + } + + func testStreamingBlockCommentSpanningLinesMatchesFullHighlight() { + // A stable boundary must never land inside the comment: the prefix would + // then be highlighted as code and never corrected. + assertIncrementalMatchesFullHighlight( + """ + let a = 1 + /* opening + still inside + and here */ + let b = 2 + """, + as: .swift + ) + } + + func testStreamingTemplateLiteralSpanningLinesMatchesFullHighlight() { + assertIncrementalMatchesFullHighlight( + """ + const q = `select * + from t + where id = 1` + const n = 42 + """, + as: .typescript + ) + } + + func testStreamingPythonTripleQuotedStringMatchesFullHighlight() { + assertIncrementalMatchesFullHighlight( + """ + def f(): + \"\"\"Doc line one. + + Doc line two. + \"\"\" + return 1 + """, + as: .python + ) + } + + /// Replays a long code block as a token stream and reports the cost of the + /// incremental path against the previous whole-text algorithm, which is + /// reproduced here (full tokenize + `index(offsetBy:)` walked from the start + /// for every token). Not a pass/fail threshold — it prints the numbers the + /// change is justified by, and fails only if the incremental path is slower. + func testStreamingHighlightIsCheaperThanWholeTextHighlight() { + let line = " let value\(Int.random(in: 0...9)) = compute(from: \"input\", count: 12) // step\n" + let fullText = String(repeating: line, count: 200) + + var snapshots: [String] = [] + var snapshot = "" + var remainder = Substring(fullText) + while !remainder.isEmpty { + snapshot += remainder.prefix(24) + remainder = remainder.dropFirst(24) + snapshots.append(snapshot) + } + + ADECodeRenderingCache.shared.purgeOnMemoryWarning() + let incrementalStart = Date() + for snapshot in snapshots { + _ = SyntaxHighlighter.highlightedAttributedString(snapshot, as: .swift) + } + let incrementalSeconds = Date().timeIntervalSince(incrementalStart) + + ADECodeRenderingCache.shared.purgeOnMemoryWarning() + let legacyStart = Date() + for snapshot in snapshots { + _ = Self.legacyHighlight(snapshot, as: .swift) + } + let legacySeconds = Date().timeIntervalSince(legacyStart) + + let ticks = Double(snapshots.count) + print(String( + format: "streaming highlight over %d ticks (%d chars): incremental %.1f ms total / %.3f ms per tick, previous %.1f ms total / %.3f ms per tick (%.1fx)", + snapshots.count, + fullText.count, + incrementalSeconds * 1000, + incrementalSeconds * 1000 / ticks, + legacySeconds * 1000, + legacySeconds * 1000 / ticks, + legacySeconds / max(incrementalSeconds, .leastNonzeroMagnitude) + )) + XCTAssertLessThan(incrementalSeconds, legacySeconds) + } + + /// The pre-change algorithm, kept only as the benchmark's baseline: tokenize + /// the whole text, then walk from `startIndex` for every token. The per-token + /// tints are `fileprivate` to the highlighter, so this applies stand-ins — the + /// cost being measured is the index walk and the run splitting, which are + /// identical either way. + private static func legacyHighlight(_ text: String, as language: FilesLanguage) -> AttributedString { + var attributed = AttributedString(text) + attributed.font = .system(.body, design: .monospaced) + for token in SyntaxHighlighter.tokenize(text, as: language) { + guard let stringRange = Range(token.range, in: text) else { continue } + let startOffset = text.distance(from: text.startIndex, to: stringRange.lowerBound) + let endOffset = text.distance(from: text.startIndex, to: stringRange.upperBound) + let lowerBound = attributed.characters.index(attributed.startIndex, offsetBy: startOffset) + let upperBound = attributed.characters.index(attributed.startIndex, offsetBy: endOffset) + attributed[lowerBound.. Date: Sun, 9 Aug 2026 21:13:50 -0400 Subject: [PATCH 02/12] fix(ios): quality-pass corrections to the mobile-feel trio MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six findings from the /quality dual review (four raised independently by CodeRabbit), all verified against the code before applying. Prepend anchoring restored by total content height, which double-counts a streaming tail. A reader scrolled back through history is exactly when an assistant reply is still growing at the bottom, and that growth landed in the same `contentSize.height` delta the correction was derived from, so the restore overshot by the tail's growth. The displacement is now measured on the anchored row itself, via a single geometry probe that rides whichever row leads the list (and stays pinned to the anchored row while a prepend is in flight). Tail growth contributes nothing to it. The probe publishes its own row id alongside the measurement — which row carries it is decided during body evaluation, and an observer that recomputed the id later could pair it with a different row's y. `highlightedSegment` retained an `AttributedString.Index` across the attribute assignment that followed it. Attribute mutation invalidates indices, so that was undefined even though it happened to work. It now paints a role per UTF-16 position and appends runs in one forward walk over immutable text, needing no `AttributedString` index at all. That rewrite was also untested: the streaming-equivalence tests compared `highlightedAttributedString` against `highlightedSegment`, which share their span logic, so they could not catch a change in it. Added an oracle test against the pre-change whole-text algorithm — it immediately caught a real defect (a token fully containing a later one lost its trailing portion), and `SyntaxTokenRole.tint`/`.font` are no longer private so the oracle can apply the real attributes instead of stand-ins. HTML comments span lines exactly like `/* */`, and the boundary scanner did not model them, so a stable prefix could freeze mis-highlighted markup mid-comment. Added the state plus a `syntaxMatches` marker helper. Echo suppression tested set membership, so one transcript row retired every echo sharing its dedupe key. Sending the same text twice ("ok", "continue") made both bubbles vanish on the first matching row — reachable now that `sending` releases at host acceptance rather than after the refresh cascade. Suppression counts matches and consumes one slot per echo, applied to all three sites that had the pattern (timeline build, reconciliation, and the incremental fast path's agreement guard). Dropped the wall-clock assertion from the highlighter benchmark; it stays diagnostic, with correctness pinned by the oracle test instead. Renamed `scheduledPostSendReconciliation` to the verb form. 1311 tests, 17 failures — the same pre-existing set as clean main. Co-Authored-By: Claude Opus 5 --- .../Views/Components/FilesCodeSupport.swift | 117 ++++++++++++----- .../Work/WorkChatSessionView+Actions.swift | 44 +++---- .../ADE/Views/Work/WorkChatSessionView.swift | 124 +++++++++++++----- .../WorkSessionDestinationView+Actions.swift | 8 +- .../Work/WorkSessionDestinationView.swift | 29 ++-- .../ADE/Views/Work/WorkTimelineHelpers.swift | 71 ++++++++-- .../WorkMarkdownStreamingParsingTests.swift | 121 +++++++++++++++-- 7 files changed, 372 insertions(+), 142 deletions(-) diff --git a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift index ea9fd7a901..efd0296991 100644 --- a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift +++ b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift @@ -138,9 +138,12 @@ struct SyntaxNestingBalance: Equatable { var insideBlockComment = false var insideBacktick = false var insideTripleQuote = false + /// HTML's comment rule is `(?s)`, so it spans lines exactly the way + /// a `/* */` block does. + var insideHtmlComment = false var isClear: Bool { - !insideBlockComment && !insideBacktick && !insideTripleQuote + !insideBlockComment && !insideBacktick && !insideTripleQuote && !insideHtmlComment } } @@ -183,6 +186,16 @@ private func syntaxStableBoundary(in text: String, from start: String.Index) -> index = text.index(index, offsetBy: 3) continue } + } else if balance.insideHtmlComment { + if syntaxMatches("-->", in: text, at: index) { + balance.insideHtmlComment = false + index = text.index(index, offsetBy: 3) + continue + } + } else if syntaxMatches(" + Text + + """), + ] + for (language, source) in sources { + ADECodeRenderingCache.shared.purgeOnMemoryWarning() + let rewritten = SyntaxHighlighter.highlightedSegment(Substring(source), as: language) + let legacy = Self.legacyHighlight(source, as: language) + if rewritten != legacy { + XCTFail( + """ + \(language.rawValue) highlight diverged from the previous algorithm. + First differing run: \(Self.firstRunDifference(rewritten, legacy) ?? "") + """ + ) + } + } + } + /// Replays a long code block as a token stream and reports the cost of the /// incremental path against the previous whole-text algorithm, which is /// reproduced here (full tokenize + `index(offsetBy:)` walked from the start - /// for every token). Not a pass/fail threshold — it prints the numbers the - /// change is justified by, and fails only if the incremental path is slower. - func testStreamingHighlightIsCheaperThanWholeTextHighlight() { + /// for every token). + /// + /// Diagnostic only. The correctness gate is + /// `testHighlightMatchesPreviousWholeTextAlgorithm`; asserting on wall-clock + /// here would just add a flake under CI load. + func testStreamingHighlightCostIsReported() { let line = " let value\(Int.random(in: 0...9)) = compute(from: \"input\", count: 12) // step\n" let fullText = String(repeating: line, count: 200) @@ -333,29 +382,45 @@ final class SyntaxHighlighterStreamingTests: XCTestCase { legacySeconds * 1000 / ticks, legacySeconds / max(incrementalSeconds, .leastNonzeroMagnitude) )) - XCTAssertLessThan(incrementalSeconds, legacySeconds) } - /// The pre-change algorithm, kept only as the benchmark's baseline: tokenize - /// the whole text, then walk from `startIndex` for every token. The per-token - /// tints are `fileprivate` to the highlighter, so this applies stand-ins — the - /// cost being measured is the index walk and the run splitting, which are - /// identical either way. + /// The pre-change algorithm, verbatim: tokenize the whole text, then walk from + /// `startIndex` for every token, letting later tokens overwrite the ranges + /// they overlap. Serves as both the benchmark baseline and the correctness + /// oracle, so it applies the real per-role attributes. private static func legacyHighlight(_ text: String, as language: FilesLanguage) -> AttributedString { var attributed = AttributedString(text) attributed.font = .system(.body, design: .monospaced) + attributed.foregroundColor = ADEColor.textPrimary for token in SyntaxHighlighter.tokenize(text, as: language) { guard let stringRange = Range(token.range, in: text) else { continue } let startOffset = text.distance(from: text.startIndex, to: stringRange.lowerBound) let endOffset = text.distance(from: text.startIndex, to: stringRange.upperBound) let lowerBound = attributed.characters.index(attributed.startIndex, offsetBy: startOffset) let upperBound = attributed.characters.index(attributed.startIndex, offsetBy: endOffset) - attributed[lowerBound.. + + after + + """, + as: .html + ) + } + func testDifferentBlockOfSameLanguageDoesNotReuseForeignPrefix() { let first = "let alpha = 1\nlet beta = 2\n" _ = SyntaxHighlighter.highlightedAttributedString(first, as: .swift) @@ -367,6 +432,40 @@ final class SyntaxHighlighterStreamingTests: XCTestCase { } } +/// Echo suppression counts matches instead of testing set membership. Releasing +/// `sending` as soon as the host accepts a message makes back-to-back identical +/// sends easy, and a set test retired both of them on the first matching row. +final class WorkLocalEchoSuppressionTests: XCTestCase { + func testOneRepresentedRowRetiresOnlyOneOfTwoIdenticalEchoes() { + let echoes = [ + WorkLocalEchoMessage(text: "continue", timestamp: "2026-08-09T00:00:01Z"), + WorkLocalEchoMessage(text: "continue", timestamp: "2026-08-09T00:00:02Z"), + ] + guard let key = workLocalEchoDedupeKey(text: "continue", attachments: nil) else { + return XCTFail("expected a dedupe key for non-empty text") + } + + let afterFirstRow = workUnrepresentedLocalEchoMessages(echoes, representedKeyCounts: [key: 1]) + XCTAssertEqual(afterFirstRow.count, 1) + XCTAssertEqual(afterFirstRow.first?.id, echoes[1].id, "the newer echo must survive") + + let afterBothRows = workUnrepresentedLocalEchoMessages(echoes, representedKeyCounts: [key: 2]) + XCTAssertTrue(afterBothRows.isEmpty) + } + + func testUnrelatedEchoesAreUntouched() { + let echoes = [ + WorkLocalEchoMessage(text: "first", timestamp: "2026-08-09T00:00:01Z"), + WorkLocalEchoMessage(text: "second", timestamp: "2026-08-09T00:00:02Z"), + ] + guard let key = workLocalEchoDedupeKey(text: "first", attachments: nil) else { + return XCTFail("expected a dedupe key for non-empty text") + } + let remaining = workUnrepresentedLocalEchoMessages(echoes, representedKeyCounts: [key: 1]) + XCTAssertEqual(remaining.map(\.text), ["second"]) + } +} + /// A streaming turn used to insert one throwaway render per delta into the /// shared inline-markdown cache, evicting every finished message in a long /// chat. Intermediate revisions now render without displacing finished work, From c025f67c65f908ff1841ccd8e4eaf2d882a82c51 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:23:31 -0400 Subject: [PATCH 03/12] test(ios): consolidate echo coverage, document the perceived-latency work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the local-echo suppression tests out of the markdown streaming file and into `WorkSessionCanonicalStateTests`, which already owns local-echo dedupe coverage — the two attachment-identity cases there cover echoes with *different* keys, and the counted-suppression cases sit directly beside them. No new test file, so no pbxproj registration and no new sibling in an already-large folder. Documents the three perceived-latency mechanisms in the iOS companion doc. One of them was already asserted there ("preserving the visible scroll anchor as pages prepend") without an implementation behind it; that claim is now true, and the note records why the correction is measured on the anchored row rather than on total content height. Co-Authored-By: Claude Opus 5 --- .../WorkMarkdownStreamingParsingTests.swift | 34 --------- .../WorkSessionCanonicalStateTests.swift | 74 +++++++++++++++++++ .../sync-and-multi-device/ios-companion.md | 47 ++++++++++++ 3 files changed, 121 insertions(+), 34 deletions(-) diff --git a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift index 5ab1410f80..59a882ab6b 100644 --- a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift +++ b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift @@ -432,40 +432,6 @@ final class SyntaxHighlighterStreamingTests: XCTestCase { } } -/// Echo suppression counts matches instead of testing set membership. Releasing -/// `sending` as soon as the host accepts a message makes back-to-back identical -/// sends easy, and a set test retired both of them on the first matching row. -final class WorkLocalEchoSuppressionTests: XCTestCase { - func testOneRepresentedRowRetiresOnlyOneOfTwoIdenticalEchoes() { - let echoes = [ - WorkLocalEchoMessage(text: "continue", timestamp: "2026-08-09T00:00:01Z"), - WorkLocalEchoMessage(text: "continue", timestamp: "2026-08-09T00:00:02Z"), - ] - guard let key = workLocalEchoDedupeKey(text: "continue", attachments: nil) else { - return XCTFail("expected a dedupe key for non-empty text") - } - - let afterFirstRow = workUnrepresentedLocalEchoMessages(echoes, representedKeyCounts: [key: 1]) - XCTAssertEqual(afterFirstRow.count, 1) - XCTAssertEqual(afterFirstRow.first?.id, echoes[1].id, "the newer echo must survive") - - let afterBothRows = workUnrepresentedLocalEchoMessages(echoes, representedKeyCounts: [key: 2]) - XCTAssertTrue(afterBothRows.isEmpty) - } - - func testUnrelatedEchoesAreUntouched() { - let echoes = [ - WorkLocalEchoMessage(text: "first", timestamp: "2026-08-09T00:00:01Z"), - WorkLocalEchoMessage(text: "second", timestamp: "2026-08-09T00:00:02Z"), - ] - guard let key = workLocalEchoDedupeKey(text: "first", attachments: nil) else { - return XCTFail("expected a dedupe key for non-empty text") - } - let remaining = workUnrepresentedLocalEchoMessages(echoes, representedKeyCounts: [key: 1]) - XCTAssertEqual(remaining.map(\.text), ["second"]) - } -} - /// A streaming turn used to insert one throwaway render per delta into the /// shared inline-markdown cache, evicting every finished message in a long /// chat. Intermediate revisions now render without displacing finished work, diff --git a/apps/ios/ADETests/WorkSessionCanonicalStateTests.swift b/apps/ios/ADETests/WorkSessionCanonicalStateTests.swift index fa586e245e..411125ba4d 100644 --- a/apps/ios/ADETests/WorkSessionCanonicalStateTests.swift +++ b/apps/ios/ADETests/WorkSessionCanonicalStateTests.swift @@ -778,6 +778,80 @@ final class WorkSessionCanonicalStateTests: XCTestCase { XCTAssertEqual(visibleUserMessages.filter { $0.attachments == [second] }.count, 1) } + /// Two sends of the *same* text share one dedupe key, unlike the attachment + /// cases above. Suppression counts represented rows instead of testing set + /// membership, so the first matching transcript row retires exactly one echo — + /// `sending` now releases as soon as the host accepts a message, which makes + /// back-to-back identical sends easy to produce. + func testIdenticalEchoesAreRetiredOneRowAtATime() { + let echoes = [ + WorkLocalEchoMessage(text: "continue", timestamp: iso(now)), + WorkLocalEchoMessage(text: "continue", timestamp: iso(now.addingTimeInterval(1))), + ] + let transcript = [ + WorkChatEnvelope( + sessionId: "chat-1", + timestamp: iso(now), + sequence: 1, + event: .userMessage( + text: "continue", + attachments: nil, + turnId: nil, + steerId: nil, + deliveryState: nil, + processed: nil + ) + ) + ] + + let snapshot = buildWorkChatTimelineSnapshot( + transcript: transcript, + fallbackEntries: [], + artifacts: [], + localEchoMessages: echoes + ) + let visibleUserMessages = snapshot.timeline.compactMap { entry -> WorkChatMessage? in + if case .message(let message) = entry.payload, message.role == "user" { return message } + return nil + } + // One transcript row plus the still-unrepresented second echo. + XCTAssertEqual(visibleUserMessages.count, 2) + XCTAssertEqual(visibleUserMessages.filter { $0.markdown == "continue" }.count, 2) + + let remaining = workUnrepresentedLocalEchoMessages( + echoes, + representedKeyCounts: workRepresentedEchoKeyCounts(from: transcript) + ) + XCTAssertEqual(remaining.count, 1) + XCTAssertEqual(remaining.first?.id, echoes[1].id, "the newer echo must survive") + } + + func testBothIdenticalEchoesRetireOnceBothRowsArrive() { + let echoes = [ + WorkLocalEchoMessage(text: "continue", timestamp: iso(now)), + WorkLocalEchoMessage(text: "continue", timestamp: iso(now.addingTimeInterval(1))), + ] + let transcript = (1...2).map { sequence in + WorkChatEnvelope( + sessionId: "chat-1", + timestamp: iso(now.addingTimeInterval(TimeInterval(sequence))), + sequence: sequence, + event: .userMessage( + text: "continue", + attachments: nil, + turnId: nil, + steerId: nil, + deliveryState: nil, + processed: nil + ) + ) + } + + let counts = workRepresentedEchoKeyCounts(from: transcript) + XCTAssertEqual(counts.values.reduce(0, +), 2) + XCTAssertTrue(workUnrepresentedLocalEchoMessages(echoes, representedKeyCounts: counts).isEmpty) + } + // MARK: - Fixtures private func makeSession( diff --git a/docs/features/sync-and-multi-device/ios-companion.md b/docs/features/sync-and-multi-device/ios-companion.md index 6a4acf914e..090ec12d86 100644 --- a/docs/features/sync-and-multi-device/ios-companion.md +++ b/docs/features/sync-and-multi-device/ios-companion.md @@ -561,6 +561,53 @@ The Work model/activity parity path is concentrated in these files: fallback, and the non-queueable `chat.cancelScheduledWork` wrapper used by Chat Info. +#### Perceived latency in the chat surface + +Three mechanisms keep the transcript feeling native. They are easy to regress, +because each one trades a simpler implementation for a property the eye notices. + +**The user's own bubble paints on the tap frame.** Every other timeline change +goes through the 90 ms coalescing rebuild in `scheduleTimelineSnapshotRebuild`, +which is right for host deltas arriving 6-7×/s and wrong for the one change the +user just caused. `applyLocalEchoTailImmediatelyIfPossible` appends the echo to +the existing snapshot synchronously and retires any in-flight rebuild +generation, so a coalesced fold cannot overwrite the bubble it was built +without. Image sends echo *before* the upload: the composer's downscaled +`UIImage` renders behind an uploading state under an `ade-pending-upload://` +placeholder ref (`WorkPendingUploadPreviewStore`), swapped for the real host +path before the message is sent so the echo's dedupe key still matches the +transcript row that returns. `sending` releases when the host accepts the +message; the transcript/artifact/summary/session refresh runs behind the +composer, chained so two quick sends cannot interleave two transcript loads. + +Because that makes back-to-back identical sends easy, echo suppression counts +represented rows rather than testing set membership — two sends of "continue" +share one dedupe key, and one matching transcript row must retire exactly one of +them (`workUnrepresentedLocalEchoMessages`). + +**Prepended history does not move the reader.** Older pages insert above the +viewport, so the `LazyVStack` grows upward while `contentOffset` stays put. The +correction is measured on the row that led the list before the insert, via a +single geometry probe that rides that row (`WorkChatPrependProbePreferenceKey`), +and is applied through `ScrollPosition` in a non-animated transaction. +Deliberately not total content height: a reply streaming into the tail grows the +content at the same time, and a reader scrolled back through history is exactly +when that happens, so a total-height correction would add the tail's growth and +overshoot. Bottom-follow, the jump-to-latest pill, and the initial force-pin are +untouched. + +**Long replies cost O(tail), not O(message).** `parseMarkdownBlocksForStreaming` +already split prose at a stable boundary; syntax highlighting now does the same, +reusing an already-highlighted stable prefix split at the last line boundary +provably outside a block comment, backtick/triple-quote string, or HTML comment. +Attributes are applied by painting a role per UTF-16 position and appending runs +over immutable text — never by retaining an `AttributedString` index across an +attribute assignment, which is undefined. Streaming tail revisions render from +their own small cache instead of the shared 256-entry inline-markdown cache, so +one long turn cannot evict every completed message and force a main-thread +re-parse on scrollback; the final revision is promoted. All derived render +caches drop on `applicationDidReceiveMemoryWarning`. + Deployment target: iOS 26+. iPhone and iPad (adaptive layouts planned for Phase 7). From b1348d9af132e11bbfaa5e0f2b6cf65250fc8382 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 21:58:02 -0400 Subject: [PATCH 04/12] fix(ios): escaped delimiters and the upload preview handoff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from Greptile on #1058, both verified against the code. The boundary scanner closed a template literal on an escaped backtick. The TypeScript rule consumes `\\.`, so `\`` stays inside the string for the tokenizer; treating it as the closer let a following newline advance the stable boundary into the middle of the literal and freeze mis-highlighted text into the immutable prefix. Backslash now skips the next character inside backtick and triple-quote states — the same class, and Python's triple-quoted rule consumes escapes too. Applied unconditionally rather than per language: Go's raw strings have no escapes, so treating a backslash as one there can only miss a closing backtick, which stalls the boundary (slower, still correct), whereas the opposite mistake in TypeScript renders wrongly. Swapping the echo's placeholder refs for host paths replaces the chip, and releasing the in-memory image at that moment left the fresh chip fetching the copy we had just uploaded — a visible flash of the generic placeholder. The image is now promoted onto the host path instead of dropped, so the handoff is seamless and the phone never re-downloads its own upload. The store is bounded to roughly one message's worth of attachments in insertion order, and a save that did not return a ref per placeholder releases rather than pairing positionally, which would attach one image's bytes to another's path. Also: CI's earlier `test-desktop (7)` failure was a flake in `FilesWorkbench.test.tsx` ("expected '-1' to be '2000'"), unrelated to this iOS-only diff — green locally, green on main at the same base, and green on re-run of the same commit. Co-Authored-By: Claude Opus 5 --- .../Views/Components/FilesCodeSupport.swift | 17 ++++ .../Views/Work/WorkChatAttachmentTray.swift | 72 ++++++++++++++--- .../WorkSessionDestinationView+Actions.swift | 5 +- .../WorkMarkdownStreamingParsingTests.swift | 81 +++++++++++++++++++ 4 files changed, 164 insertions(+), 11 deletions(-) diff --git a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift index efd0296991..34852f4e0e 100644 --- a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift +++ b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift @@ -179,8 +179,25 @@ private func syntaxStableBoundary(in text: String, from start: String.Index) -> continue } } else if balance.insideBacktick { + // The template-literal and triple-quote rules both consume `\\.`, so an + // escaped delimiter does not close the string for the tokenizer and must + // not close it here either — a boundary landing inside the literal would + // freeze mis-highlighted text into the immutable prefix. + // + // Applied unconditionally rather than per language: Go's raw strings have + // no escapes, so treating a backslash as one there can only *miss* a + // closing backtick, which stalls the boundary (slower, still correct). + // The opposite mistake in TypeScript is a wrong render. + if character == "\\", next < text.endIndex { + index = text.index(after: next) + continue + } if character == "`" { balance.insideBacktick = false } } else if balance.insideTripleQuote { + if character == "\\", next < text.endIndex { + index = text.index(after: next) + continue + } if syntaxIsTripleQuote(text, at: index) { balance.insideTripleQuote = false index = text.index(index, offsetBy: 3) diff --git a/apps/ios/ADE/Views/Work/WorkChatAttachmentTray.swift b/apps/ios/ADE/Views/Work/WorkChatAttachmentTray.swift index 97900adfc1..1f02198c56 100644 --- a/apps/ios/ADE/Views/Work/WorkChatAttachmentTray.swift +++ b/apps/ios/ADE/Views/Work/WorkChatAttachmentTray.swift @@ -32,19 +32,30 @@ private enum WorkChatRemoteImageError: Error { /// A ref with this prefix never reaches the wire. let workPendingUploadPathPrefix = "ade-pending-upload://" +/// Roughly one message's worth of attachments (`workChatInputAttachmentLimit`). +let workPendingUploadPreviewLimit = 10 + func workAttachmentIsPendingUpload(_ ref: AgentChatFileRef) -> Bool { ref.path.hasPrefix(workPendingUploadPathPrefix) } -/// Holds the composer's already-downscaled `UIImage` for each in-flight upload -/// so the echo's thumbnail resolves without touching the host. Entries are -/// released as soon as the save returns (or the send fails) — a handoff buffer, -/// not a cache. +/// Holds the composer's already-downscaled `UIImage` for the images a send is +/// carrying, first under a placeholder path and then under the real host path. +/// +/// Keeping it past the upload is deliberate. Swapping the echo's refs replaces +/// the chip, and a fresh chip loading the host copy asynchronously would show +/// the generic placeholder in the gap — a visible flash of the image the phone +/// already has in memory. Promoting the entry to the host path also means the +/// phone never re-downloads its own upload. +/// +/// Bounded by `workPendingUploadPreviewLimit` in insertion order, so it holds +/// about one message's worth of attachments rather than growing with the chat. @MainActor final class WorkPendingUploadPreviewStore { static let shared = WorkPendingUploadPreviewStore() private var imagesByPath: [String: UIImage] = [:] + private var insertionOrder: [String] = [] private init() {} @@ -55,21 +66,55 @@ final class WorkPendingUploadPreviewStore { type: "image" ) if let image = attachment.image { - imagesByPath[ref.path] = image + store(image, forPath: ref.path) } return ref } } + /// Re-keys each placeholder's image onto the host path the save returned. + /// Positional, so it only applies when the save produced a ref for every + /// placeholder; otherwise the placeholders are simply released, because a + /// mismatched pairing would attach one image's bytes to another's path. + func promote(_ placeholders: [AgentChatFileRef], to saved: [AgentChatFileRef]) { + guard placeholders.count == saved.count else { + release(placeholders) + return + } + for (placeholder, savedRef) in zip(placeholders, saved) { + guard workAttachmentIsPendingUpload(placeholder) else { continue } + let image = imagesByPath[placeholder.path] + removeEntry(forPath: placeholder.path) + guard let image, !workAttachmentIsPendingUpload(savedRef) else { continue } + store(image, forPath: savedRef.path) + } + } + func image(forPath path: String) -> UIImage? { imagesByPath[path] } func release(_ refs: [AgentChatFileRef]) { - for ref in refs where workAttachmentIsPendingUpload(ref) { - imagesByPath.removeValue(forKey: ref.path) + for ref in refs { + removeEntry(forPath: ref.path) } } + + private func store(_ image: UIImage, forPath path: String) { + if imagesByPath[path] == nil { + insertionOrder.append(path) + } + imagesByPath[path] = image + while insertionOrder.count > workPendingUploadPreviewLimit { + let oldest = insertionOrder.removeFirst() + imagesByPath.removeValue(forKey: oldest) + } + } + + private func removeEntry(forPath path: String) { + guard imagesByPath.removeValue(forKey: path) != nil else { return } + insertionOrder.removeAll { $0 == path } + } } func workChatAttachmentIsImage(_ ref: AgentChatFileRef) -> Bool { @@ -806,10 +851,17 @@ private struct WorkChatAttachmentChip: View { @MainActor private func loadPreviewIfNeeded() async { guard workChatAttachmentIsImage(attachment) else { return } - // Still uploading: the composer's downscaled image is already in memory, so - // the echo's thumbnail resolves without a host round-trip. + // The phone already holds this image if it is the one being sent — while it + // uploads under a placeholder path, and afterwards under the host path it + // was promoted to. Resolving locally avoids both a placeholder flash across + // the swap and a re-download of our own upload. + if let local = WorkPendingUploadPreviewStore.shared.image(forPath: attachment.path) { + previewImage = local + loadFailed = false + return + } if workAttachmentIsPendingUpload(attachment) { - previewImage = WorkPendingUploadPreviewStore.shared.image(forPath: attachment.path) + previewImage = nil loadFailed = false return } diff --git a/apps/ios/ADE/Views/Work/WorkSessionDestinationView+Actions.swift b/apps/ios/ADE/Views/Work/WorkSessionDestinationView+Actions.swift index 2e29af5fde..30dfdd974f 100644 --- a/apps/ios/ADE/Views/Work/WorkSessionDestinationView+Actions.swift +++ b/apps/ios/ADE/Views/Work/WorkSessionDestinationView+Actions.swift @@ -69,7 +69,10 @@ extension WorkSessionDestinationView { // (text + attachment refs) has to match the transcript row that comes back, // or reconciliation would leave a duplicate bubble behind. updateLocalEchoAttachments(echoId: echoId, attachments: attachmentRefs.isEmpty ? nil : attachmentRefs) - WorkPendingUploadPreviewStore.shared.release(pendingUploadRefs) + // Promote rather than release: the swap replaces the chip, and dropping the + // in-memory image here would flash the generic placeholder while the fresh + // chip fetched the copy we just uploaded. + WorkPendingUploadPreviewStore.shared.promote(pendingUploadRefs, to: attachmentRefs) defer { sending = false } do { diff --git a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift index 59a882ab6b..beea557a70 100644 --- a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift +++ b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift @@ -404,6 +404,35 @@ final class SyntaxHighlighterStreamingTests: XCTestCase { return attributed } + func testStreamingEscapedBacktickKeepsTemplateLiteralOpen() { + // The template-literal rule consumes `\\.`, so an escaped backtick does not + // end the string. A boundary landing on the newline after it would freeze + // the rest of the literal into the prefix as mis-highlighted code. + assertIncrementalMatchesFullHighlight( + #""" + const q = `a \` b + still inside + ` + `second` + const n = 42 + """#, + as: .typescript + ) + } + + func testStreamingEscapedQuoteInsideTripleQuotedStringStaysOpen() { + assertIncrementalMatchesFullHighlight( + #""" + def f(): + s = """doc \" line + + more + """ + return s + """#, + as: .python + ) + } + func testStreamingHtmlCommentSpanningLinesMatchesFullHighlight() { // HTML's comment rule spans lines like a `/* */` block; a stable boundary // landing inside one would freeze mis-highlighted markup into the prefix. @@ -432,6 +461,58 @@ final class SyntaxHighlighterStreamingTests: XCTestCase { } } +/// The composer's downscaled image has to survive the placeholder → host-path +/// swap, or the fresh chip flashes the generic placeholder while it re-fetches +/// the image the phone just uploaded. +@MainActor +final class WorkPendingUploadPreviewStoreTests: XCTestCase { + private func makeAttachment() -> WorkChatInputAttachment { + WorkChatInputAttachment( + image: UIImage(systemName: "photo") ?? UIImage(), + uploadData: Data([0x01]), + filename: "shot.jpg", + state: .ready + ) + } + + func testPromotedImageResolvesUnderTheHostPath() { + let store = WorkPendingUploadPreviewStore.shared + let placeholders = store.register([makeAttachment()]) + XCTAssertEqual(placeholders.count, 1) + XCTAssertNotNil(store.image(forPath: placeholders[0].path)) + + let saved = [AgentChatFileRef(path: "/project/.ade/attachments/shot.jpg", type: "image")] + store.promote(placeholders, to: saved) + + XCTAssertNotNil(store.image(forPath: saved[0].path), "no image means the chip flashes a placeholder") + XCTAssertNil(store.image(forPath: placeholders[0].path), "the placeholder key must not linger") + store.release(saved) + } + + func testMismatchedSaveCountReleasesRatherThanMispairing() { + let store = WorkPendingUploadPreviewStore.shared + let placeholders = store.register([makeAttachment(), makeAttachment()]) + // One attachment failed to produce a ref: pairing positionally would attach + // the first image's bytes to a path it does not belong to. + store.promote(placeholders, to: [AgentChatFileRef(path: "/project/.ade/attachments/only.jpg", type: "image")]) + + XCTAssertNil(store.image(forPath: "/project/.ade/attachments/only.jpg")) + XCTAssertTrue(placeholders.allSatisfy { store.image(forPath: $0.path) == nil }) + } + + func testStoreIsBoundedToRoughlyOneMessageOfAttachments() { + let store = WorkPendingUploadPreviewStore.shared + let refs = store.register((0..<(workPendingUploadPreviewLimit + 4)).map { _ in makeAttachment() }) + let retained = refs.filter { store.image(forPath: $0.path) != nil } + XCTAssertEqual(retained.count, workPendingUploadPreviewLimit) + XCTAssertTrue( + retained.allSatisfy { refs.suffix(workPendingUploadPreviewLimit).contains($0) }, + "the newest entries are the ones worth keeping" + ) + store.release(refs) + } +} + /// A streaming turn used to insert one throwaway render per delta into the /// shared inline-markdown cache, evicting every finished message in a long /// chat. Intermediate revisions now render without displacing finished work, From 796345571ba4b9e962ab0134102e51c47917b390 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:23:20 -0400 Subject: [PATCH 05/12] fix(ios): drive the highlight boundary from per-language delimiters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex was right that the stable-prefix scan cannot be language-agnostic, and chasing it turned up that my previous two attempts were both unsound. Escapes are per *delimiter*, not per language. The last commit skipped the character after any backslash, which is correct for a TypeScript template literal and wrong for a Go raw string, where `\` before the closing backtick does not escape it. Because parity is cumulative, one uncounted delimiter flips the reading of every later line — the added Go test fails on that commit: a following multi-line raw string looks closed and the boundary lands inside it. Each delimiter now carries its own `escapes` flag from a table that sits next to the token rules it mirrors. Deriving the boundary from the tokens instead was also tried and is wrong for streaming: while a block comment is still unterminated no token covers it, so the boundary advances into text that becomes a comment once the closer arrives. That is recorded in the doc comment so it is not attempted a third time. What the scan asks is deliberately weaker than "what is open here?". The tokenizer runs each rule independently over the whole text, so a `'` inside a `//` comment really does open a string match and no state machine can mirror that. Balance cannot be fooled the same way: anything unbalanced since the last boundary simply refuses the split, so a wrong guess costs a shorter prefix, never a wrong render. That also covers the multi-line quoted strings Codex flagged in HTML and YAML — every rule using `"(?:[^"\\]|\\.)*"` can cross a newline, since `[^"\\]` matches one. The scan resumes at the reused prefix rather than rescanning the block, and each segment tokenizes only itself, so the per-tick cost stays proportional to the new tail: 5.86 ms -> 0.147 ms (39.7x) on the same 475-tick replay. New coverage: Go raw string with a trailing backslash, HTML attribute and YAML value spanning lines, an apostrophe in a JS comment, and escaped delimiters in TypeScript and Python. 1311 tests, same 13 pre-existing failing cases as clean main. Co-Authored-By: Claude Opus 5 --- .../Views/Components/FilesCodeSupport.swift | 250 +++++++++++------- .../WorkMarkdownStreamingParsingTests.swift | 54 ++++ 2 files changed, 213 insertions(+), 91 deletions(-) diff --git a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift index 34852f4e0e..fd9ca8184c 100644 --- a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift +++ b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift @@ -126,106 +126,128 @@ struct SyntaxToken: Identifiable, Equatable { let range: NSRange } -/// Whether a scan position sits inside a construct that can span lines. Every -/// token rule is line-anchored except block comments and backtick / triple-quote -/// strings, so a line boundary reached with all of these clear is a point no -/// later text can reinterpret. -/// -/// Ambiguity resolves toward "still inside": a false positive only stops the -/// stable prefix from growing (slower, still correct), while a false negative -/// would let a token straddle the split. -struct SyntaxNestingBalance: Equatable { - var insideBlockComment = false - var insideBacktick = false - var insideTripleQuote = false - /// HTML's comment rule is `(?s)`, so it spans lines exactly the way - /// a `/* */` block does. - var insideHtmlComment = false - - var isClear: Bool { - !insideBlockComment && !insideBacktick && !insideTripleQuote && !insideHtmlComment - } -} - /// The already-highlighted stable prefix of the code block currently streaming -/// in a given language. It always ends just past a newline reached with no -/// multi-line construct open, so a resumed scan can start from a clear state. +/// in a given language. It always ends just past a newline that no token spans, +/// so re-highlighting from there cannot disagree with a whole-text render. struct SyntaxHighlightPrefix { let text: String let attributed: AttributedString } -/// Returns the position just past the last newline in `text[start...]` reached -/// with no multi-line construct open (never earlier than `start`). `start` is -/// itself such a position, so the scan begins from a clear state. -private func syntaxStableBoundary(in text: String, from start: String.Index) -> String.Index { - var balance = SyntaxNestingBalance() - var boundary = start +/// The delimiters whose matches can run across a newline, per language. +/// +/// Nearly every rule here can: not only block comments and backticks, but any +/// `"(?:[^"\\]|\\.)*"` string, because `[^"\\]` matches `\n`. Which characters +/// those are is language-specific — `'` opens a string in Python but not in +/// JSON, and Go's raw backticks process no escapes — so the boundary scan reads +/// this table instead of assuming one grammar for every language. +struct SyntaxMultilineDelimiters { + /// A delimiter that is its own closer (`"`, `'`, `` ` ``). + /// + /// `escapes` mirrors whether that rule's pattern consumes `\\.`. It is per + /// delimiter, not per language: Go's `"` strings take escapes while its raw + /// backtick strings do not, and getting it wrong in either direction + /// mis-counts the delimiter and flips parity for every following line. + struct Symmetric { + let character: Character + var escapes: Bool = true + } + + var symmetric: [Symmetric] = [] + /// Open/close pairs (`/* */`, ``). Neither processes escapes. + var pairs: [(open: String, close: String)] = [] + + static let none = SyntaxMultilineDelimiters() +} + +/// UTF-16 offset just past the last newline at which every multi-line delimiter +/// is balanced, or 0 when there is no such newline. +/// +/// This asks a deliberately weaker question than "what is open here?". A state +/// machine would have to model how the rules interact, but the tokenizer runs +/// each rule independently over the whole text, so a `'` inside a `//` comment +/// really does start a string match. Balance can't be fooled that way: an +/// unbalanced delimiter anywhere since the last boundary simply refuses the +/// split. Wrong guesses only ever cost a shorter prefix — never a wrong render. +/// +/// The tokens themselves can't answer this either: while a block comment is +/// still unterminated mid-stream, no token covers it yet, and a boundary placed +/// inside it would be frozen in before the closer arrives. +/// Scanning resumes at `startOffset`, which must itself be a confirmed boundary: +/// everything before it is balanced by definition, so the counts start clean and +/// the per-tick cost is the length of the new tail rather than the whole block. +private func syntaxStableBoundaryOffset( + in text: String, + from startOffset: Int, + delimiters: SyntaxMultilineDelimiters +) -> Int { + guard !text.isEmpty, startOffset <= text.utf16.count else { return startOffset } + let start = String.Index(utf16Offset: startOffset, in: text) + + // Each symmetric delimiter is counted with its own escape rule, so one + // delimiter's escapes cannot mis-count another's. + var counts = [Int](repeating: 0, count: delimiters.symmetric.count) + var pendingEscape = [Bool](repeating: false, count: delimiters.symmetric.count) + var pairDepths = [Int](repeating: 0, count: delimiters.pairs.count) + var boundary = startOffset + var offset = startOffset var index = start + + func isBalanced() -> Bool { + counts.allSatisfy { $0 % 2 == 0 } && pairDepths.allSatisfy { $0 == 0 } + } + while index < text.endIndex { let character = text[index] - let next = text.index(after: index) + let width = character.utf16.count if character == "\n" { - if balance.isClear { boundary = next } - index = next + if isBalanced() { + boundary = offset + width + // Everything before here is confirmed closed, so later lines start clean. + for position in counts.indices { counts[position] = 0 } + } + for position in pendingEscape.indices { pendingEscape[position] = false } + offset += width + index = text.index(after: index) continue } - if balance.insideBlockComment { - if character == "*", next < text.endIndex, text[next] == "/" { - balance.insideBlockComment = false - index = text.index(after: next) - continue + var matchedPair = false + for (position, pair) in delimiters.pairs.enumerated() { + if syntaxMatches(pair.open, in: text, at: index) { + pairDepths[position] += 1 + offset += pair.open.utf16.count + index = text.index(index, offsetBy: pair.open.count) + matchedPair = true + break } - } else if balance.insideBacktick { - // The template-literal and triple-quote rules both consume `\\.`, so an - // escaped delimiter does not close the string for the tokenizer and must - // not close it here either — a boundary landing inside the literal would - // freeze mis-highlighted text into the immutable prefix. - // - // Applied unconditionally rather than per language: Go's raw strings have - // no escapes, so treating a backslash as one there can only *miss* a - // closing backtick, which stalls the boundary (slower, still correct). - // The opposite mistake in TypeScript is a wrong render. - if character == "\\", next < text.endIndex { - index = text.index(after: next) - continue + if syntaxMatches(pair.close, in: text, at: index) { + pairDepths[position] = max(0, pairDepths[position] - 1) + offset += pair.close.utf16.count + index = text.index(index, offsetBy: pair.close.count) + matchedPair = true + break } - if character == "`" { balance.insideBacktick = false } - } else if balance.insideTripleQuote { - if character == "\\", next < text.endIndex { - index = text.index(after: next) + } + if matchedPair { continue } + + for (position, delimiter) in delimiters.symmetric.enumerated() { + if pendingEscape[position] { + pendingEscape[position] = false continue } - if syntaxIsTripleQuote(text, at: index) { - balance.insideTripleQuote = false - index = text.index(index, offsetBy: 3) + if character == "\\", delimiter.escapes { + pendingEscape[position] = true continue } - } else if balance.insideHtmlComment { - if syntaxMatches("-->", in: text, at: index) { - balance.insideHtmlComment = false - index = text.index(index, offsetBy: 3) - continue + if character == delimiter.character { + counts[position] += 1 } - } else if syntaxMatches("")] + ) + case .css: + return SyntaxMultilineDelimiters(symmetric: [quote, apostrophe], pairs: blockComment) + case .json: + return SyntaxMultilineDelimiters(symmetric: [quote]) + case .yaml: + return SyntaxMultilineDelimiters(symmetric: [quote, apostrophe]) + case .markdown: + // Covers both `inline` and ``` fences: a fence is three backticks, so an + // open fence reads as unbalanced until its closer arrives. + return SyntaxMultilineDelimiters(symmetric: [Symmetric(character: "`", escapes: false)]) + case .plaintext: + return .none + } + } + private static func tokenRules(for language: FilesLanguage) -> [TokenRule] { let numberRule = TokenRule(role: .number, pattern: #"\b\d+(?:\.\d+)?\b"#) switch language { diff --git a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift index beea557a70..efd93144d2 100644 --- a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift +++ b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift @@ -433,6 +433,60 @@ final class SyntaxHighlighterStreamingTests: XCTestCase { ) } + func testStreamingGoRawStringWithTrailingBackslashMatchesFullHighlight() { + // Go raw strings process no escapes, so the backslash before the closing + // backtick does not escape it. Treating it as an escape desynchronizes the + // delimiter parity and can leave a later multi-line raw string looking + // closed — the boundary would then split inside it. + assertIncrementalMatchesFullHighlight( + #""" + a := `C:\path\` + b := `multi + line` + c := 1 + """#, + as: .go + ) + } + + func testStreamingHtmlAttributeStringSpanningLinesMatchesFullHighlight() { + // The quote rules use `[^"\\]`, which matches newlines, so an attribute + // value left open runs across lines for the tokenizer. + assertIncrementalMatchesFullHighlight( + """ +
+ text +
+ """, + as: .html + ) + } + + func testStreamingYamlQuotedValueSpanningLinesMatchesFullHighlight() { + assertIncrementalMatchesFullHighlight( + """ + key: "first + continued" + other: 2 + """, + as: .yaml + ) + } + + func testStreamingApostropheInCommentDoesNotSplitInsideAStringMatch() { + // A lone apostrophe in a comment still opens a string match for the rule + // that scans independently of the comment rule. + assertIncrementalMatchesFullHighlight( + """ + // don't do this + const x = 'ok' + const y = 2 + """, + as: .javascript + ) + } + func testStreamingHtmlCommentSpanningLinesMatchesFullHighlight() { // HTML's comment rule spans lines like a `/* */` block; a stable boundary // landing inside one would freeze mis-highlighted markup into the prefix. From aa66f5368639677a85f0da2009f80d2fab488bd1 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:40:54 -0400 Subject: [PATCH 06/12] fix(ios): stop guessing at grammar; probe by source entry; hash echo refs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from Codex, all verified. Some rules cross a newline with no delimiter to count at all: CSS matches a selector list through `[...\s,>+~]*\s*\{`, YAML's key rule opens with `^\s*`, and a Markdown link's `[^\]]+` spans lines. This is the third distinct way the boundary model has been incomplete, so rather than add a fourth construct, `multilineDelimiters(for:)` is now optional and those three languages return nil: no prefix reuse, whole-text highlight per tick, exactly what they did before incremental highlighting existed. Modeling those rules would mean re-implementing each regex, and a model that is *nearly* right is what produced the bugs. Declaring the gap costs three languages the speedup and costs correctness nothing. Everything else — Swift, TypeScript, JavaScript, Python, Rust, Go, Java, HTML, JSON — keeps it, and a test pins which side each language is on. The prepend probe compared a render-row id against a timeline-entry id. A streaming or expanded assistant message renders as several suffixed block rows, so when such a message led the visible list the probe never installed, the anchor never armed, and paging silently fell back to jumping. Resolved through `sourceEntryId`, picking that entry's first block, and published in timeline-entry id space so the anchor comparison still matches. `workIncrementalLocalEchoSignature` omitted attachment refs. Swapping a pending upload for its host path changes nothing else about the echo, so the assistant-tail fast path could treat it as unchanged and keep rendering the uploading chip until a canonical refresh. The render and presentation signatures already hashed refs; this one now does too. 1323 tests, same 13 pre-existing failing cases as clean main. 38.6x held. Co-Authored-By: Claude Opus 5 --- .../Views/Components/FilesCodeSupport.swift | 32 +++++++++++++------ .../Work/WorkChatSessionView+Actions.swift | 10 ++++++ .../ADE/Views/Work/WorkChatSessionView.swift | 12 +++++-- .../WorkMarkdownStreamingParsingTests.swift | 31 ++++++++++++++++++ 4 files changed, 73 insertions(+), 12 deletions(-) diff --git a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift index fd9ca8184c..be04b18c46 100644 --- a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift +++ b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift @@ -317,6 +317,11 @@ struct SyntaxHighlighter { _ text: String, as language: FilesLanguage ) -> AttributedString { + guard let delimiters = multilineDelimiters(for: language) else { + // This language has newline-crossing rules the balance scan cannot model. + return highlightedSegment(text[...], as: language) + } + let reusable = ADECodeRenderingCache.shared.highlightPrefix(for: language) .flatMap { prefix -> SyntaxHighlightPrefix? in // Byte-prefix check: only a block that literally grew from this prefix @@ -331,7 +336,7 @@ struct SyntaxHighlighter { let boundaryOffset = syntaxStableBoundaryOffset( in: text, from: scanOffset, - delimiters: multilineDelimiters(for: language) + delimiters: delimiters ) let boundary = String.Index(utf16Offset: boundaryOffset, in: text) @@ -432,7 +437,18 @@ struct SyntaxHighlighter { /// Mirrors the newline-crossing constructs in `tokenRules(for:)`. Keep the two /// in step: a delimiter missing here can let the stable prefix split inside a /// construct, and an extra one only shortens the prefix. - static func multilineDelimiters(for language: FilesLanguage) -> SyntaxMultilineDelimiters { + /// The delimiters for a language whose newline-crossing constructs the balance + /// scan can model completely, or `nil` when it cannot. + /// + /// `nil` means "do not reuse a prefix for this language" — it highlights whole + /// text per tick, exactly as it did before incremental highlighting existed. + /// Some rules cross a newline without any delimiter at all: CSS matches a + /// selector list through `[...\s,>+~]*\s*\{`, YAML's key rule opens with + /// `^\s*`, and a Markdown link's `[^\]]+` spans lines. Modeling those would + /// mean re-implementing each regex, and a model that is *nearly* right is what + /// produced three separate boundary bugs here. Declaring the gap costs those + /// three languages the speedup and costs correctness nothing. + static func multilineDelimiters(for language: FilesLanguage) -> SyntaxMultilineDelimiters? { typealias Symmetric = SyntaxMultilineDelimiters.Symmetric let blockComment = [(open: "/*", close: "*/")] let quote = Symmetric(character: "\"") @@ -461,18 +477,14 @@ struct SyntaxHighlighter { symmetric: [quote, apostrophe], pairs: [(open: "")] ) - case .css: - return SyntaxMultilineDelimiters(symmetric: [quote, apostrophe], pairs: blockComment) case .json: return SyntaxMultilineDelimiters(symmetric: [quote]) - case .yaml: - return SyntaxMultilineDelimiters(symmetric: [quote, apostrophe]) - case .markdown: - // Covers both `inline` and ``` fences: a fence is three backticks, so an - // open fence reads as unbalanced until its closer arrives. - return SyntaxMultilineDelimiters(symmetric: [Symmetric(character: "`", escapes: false)]) case .plaintext: return .none + case .css, .yaml, .markdown: + // CSS selector lists, YAML's `^\s*` key rule, and Markdown links all cross + // newlines without a delimiter to count. No prefix reuse for these. + return nil } } diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift index 2cfaf2e5e4..4b1911f3ac 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift @@ -544,6 +544,16 @@ private func workIncrementalLocalEchoSignature(_ localEchoMessages: [WorkLocalEc hasher.combine(echo.text.hashValue) hasher.combine(echo.timestamp) hasher.combine(echo.deliveryState) + // Attachment refs change without touching count, text, timestamp, or + // delivery state when a pending upload is swapped for its host path. Leaving + // them out let the assistant-tail fast path treat the echo as unchanged and + // keep rendering the uploading chip. + hasher.combine(echo.attachments?.count ?? 0) + for attachment in echo.attachments ?? [] { + hasher.combine(attachment.path) + hasher.combine(attachment.type) + hasher.combine(attachment.url) + } } return hasher.finalize() } diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift index 629f68efff..2a9a3261c0 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift @@ -883,6 +883,12 @@ struct WorkChatSessionView: View { let streamingMessageId = streamingAssistantMessageId let userBubbleWidth = maxUserBubbleWidth let probeRowId = prependProbeRowId + // A streaming or expanded assistant message renders as several suffixed + // block rows, so the probed *timeline* entry has no render row with a + // matching id. Resolve through `sourceEntryId` and pick its first block, + // or the probe silently never installs and the anchor never arms. + let probeRenderRowId = visibleTimelineRenderEntries + .first { $0.sourceEntryId == probeRowId }?.id ForEach(visibleTimelineRenderEntries) { entry in timelineRenderEntryView( for: entry, @@ -894,12 +900,14 @@ struct WorkChatSessionView: View { // Exactly one row carries this probe. It measures how far a prepend // pushed the reader's content down, which total content height cannot // do while the tail is also streaming. - if entry.id == probeRowId { + if let probeRowId, entry.id == probeRenderRowId { GeometryReader { geometry in Color.clear.preference( key: WorkChatPrependProbePreferenceKey.self, + // Published in timeline-entry id space, which is what the anchor + // compares against. value: WorkChatPrependProbeSample( - rowId: entry.id, + rowId: probeRowId, y: geometry.frame(in: .named(workChatScrollCoordinateSpace)).minY ) ) diff --git a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift index efd93144d2..9933ecf7ab 100644 --- a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift +++ b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift @@ -504,6 +504,37 @@ final class SyntaxHighlighterStreamingTests: XCTestCase { ) } + func testLanguagesWithUnmodelledMultilineRulesOptOutOfPrefixReuse() { + // CSS selector lists, YAML's `^\s*` key rule, and Markdown links all cross a + // newline with no delimiter to count, so these must not reuse a prefix. + for language in [FilesLanguage.css, .yaml, .markdown] { + XCTAssertNil( + SyntaxHighlighter.multilineDelimiters(for: language), + "\(language.rawValue) has newline-crossing rules the balance scan cannot model" + ) + } + for language in [FilesLanguage.swift, .typescript, .javascript, .python, .rust, .go, .java, .html, .json] { + XCTAssertNotNil(SyntaxHighlighter.multilineDelimiters(for: language)) + } + } + + func testMultilineCssSelectorStillMatchesFullHighlight() { + assertIncrementalMatchesFullHighlight( + """ + .foo, + .bar { + color: red; + } + """, + as: .css + ) + } + + func testMultilineYamlAndMarkdownStillMatchFullHighlight() { + assertIncrementalMatchesFullHighlight("a:\n\n b: 1\nc: 2", as: .yaml) + assertIncrementalMatchesFullHighlight("see [long\nlink](https://x.test)\n\ntext", as: .markdown) + } + func testDifferentBlockOfSameLanguageDoesNotReuseForeignPrefix() { let first = "let alpha = 1\nlet beta = 2\n" _ = SyntaxHighlighter.highlightedAttributedString(first, as: .swift) From 192f53323de420a7c8fac48b124bf4b748b81439 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 23:02:03 -0400 Subject: [PATCH 07/12] fix(ios): pin the prefix-reuse claim; thumbnail the upload previews MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from Codex and CodeRabbit, all verified. JSON opts out of prefix reuse. Its key rule only matches once `(?=\s*:)` finds the colon, which can arrive after the newline, so a key streamed as `"key"\n: 1` would freeze unhighlighted into the prefix. That is the fourth distinct rule shape this boundary has been wrong about, so the claim is now pinned rather than re-argued: a test fingerprints the rule patterns of every language still allowed to reuse a prefix, and editing one of those rules fails with instructions to re-check it against the delimiter model. The property is about the patterns and cannot be re-derived at runtime; the only honest thing to do is make drift impossible to land silently. While measuring whether to drop prefix reuse entirely, the earlier claim that the quadratic index walk was the dominant cost turned out to be wrong. Whole-text highlighting with the role fill runs at 5.7 ms/tick against the previous 5.4 ms — no better. The whole win is prefix reuse (0.139 ms/tick, 38.7x), which is why the answer here is to bound the claim rather than abandon it. The benchmark now prints all three numbers so this is not mis-stated again. The pending-upload store held the composer's *upload* render — up to 2400px, roughly 23 MB decoded — so ten attachments could pin a quarter-gigabyte after the composer cleared. It now stores a 256px chip-sized thumbnail (about 260 KB), which is also what the host path would have produced for these 56-72pt chips, and purges on `didReceiveMemoryWarning` along with the other render caches. The prepend restore corrected from the offset captured at arm time, so a reader who kept scrolling while the page loaded was snapped back to where they started. It corrects from the live offset now; only the inserted height needs undoing. That leaves the anchor's captured `offsetY` unread, so it is gone rather than left as a field nothing consults. 1325 tests, same 13 pre-existing failing cases as clean main. Co-Authored-By: Claude Opus 5 --- apps/ios/ADE/App/ADEAppDelegate.swift | 3 + .../Views/Components/FilesCodeSupport.swift | 24 ++++++-- .../Views/Work/WorkChatAttachmentTray.swift | 32 +++++++++- .../ADE/Views/Work/WorkChatSessionView.swift | 9 ++- .../WorkMarkdownStreamingParsingTests.swift | 61 +++++++++++++++++-- 5 files changed, 115 insertions(+), 14 deletions(-) diff --git a/apps/ios/ADE/App/ADEAppDelegate.swift b/apps/ios/ADE/App/ADEAppDelegate.swift index 8f03e8764a..690afc72f9 100644 --- a/apps/ios/ADE/App/ADEAppDelegate.swift +++ b/apps/ios/ADE/App/ADEAppDelegate.swift @@ -32,6 +32,9 @@ final class ADEAppDelegate: NSObject, UIApplicationDelegate { func applicationDidReceiveMemoryWarning(_ application: UIApplication) { workPurgeMarkdownRenderCaches() ADECodeRenderingCache.shared.purgeOnMemoryWarning() + MainActor.assumeIsolated { + WorkPendingUploadPreviewStore.shared.purge() + } } /// Register the approval-alert category so approval pushes carry inline diff --git a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift index be04b18c46..d5d5675826 100644 --- a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift +++ b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift @@ -477,17 +477,31 @@ struct SyntaxHighlighter { symmetric: [quote, apostrophe], pairs: [(open: "")] ) - case .json: - return SyntaxMultilineDelimiters(symmetric: [quote]) case .plaintext: return .none - case .css, .yaml, .markdown: - // CSS selector lists, YAML's `^\s*` key rule, and Markdown links all cross - // newlines without a delimiter to count. No prefix reuse for these. + case .css, .yaml, .markdown, .json: + // Each has a rule whose match depends on text the boundary cannot see: + // CSS's selector list runs through `[...\s,>+~]*\s*\{`, YAML's key rule + // opens with `^\s*`, a Markdown link's `[^\]]+` spans lines, and JSON's + // key rule only matches once its `(?=\s*:)` lookahead finds the colon — + // which may arrive after the newline. No prefix reuse for these. return nil } } + /// Fingerprint of a language's rule patterns. + /// + /// Whether a language may reuse a stable prefix is a claim about *these + /// patterns*: that nothing in them can match across a newline except the + /// delimiters `multilineDelimiters(for:)` counts. That claim cannot be + /// re-derived at runtime, and every time it has been wrong the symptom was a + /// completed code block frozen mis-highlighted in cache. A pinned test hashes + /// this, so editing a rule for an opted-in language fails loudly instead of + /// silently invalidating the boundary. + static func tokenRuleFingerprint(for language: FilesLanguage) -> String { + workStableDigest(tokenRules(for: language).map(\.pattern).joined(separator: "\u{1F}")) + } + private static func tokenRules(for language: FilesLanguage) -> [TokenRule] { let numberRule = TokenRule(role: .number, pattern: #"\b\d+(?:\.\d+)?\b"#) switch language { diff --git a/apps/ios/ADE/Views/Work/WorkChatAttachmentTray.swift b/apps/ios/ADE/Views/Work/WorkChatAttachmentTray.swift index 1f02198c56..bd333d701a 100644 --- a/apps/ios/ADE/Views/Work/WorkChatAttachmentTray.swift +++ b/apps/ios/ADE/Views/Work/WorkChatAttachmentTray.swift @@ -35,6 +35,12 @@ let workPendingUploadPathPrefix = "ade-pending-upload://" /// Roughly one message's worth of attachments (`workChatInputAttachmentLimit`). let workPendingUploadPreviewLimit = 10 +/// Chips render at 56-72pt, so ~256px covers 3x displays. The composer's own +/// image is the *upload* render at up to 2400px — around 23 MB decoded, which +/// ten of would be a quarter-gigabyte resident for thumbnails nobody sees at +/// that size. +private let workPendingUploadPreviewMaxPixels: CGFloat = 256 + func workAttachmentIsPendingUpload(_ ref: AgentChatFileRef) -> Bool { ref.path.hasPrefix(workPendingUploadPathPrefix) } @@ -65,13 +71,20 @@ final class WorkPendingUploadPreviewStore { path: "\(workPendingUploadPathPrefix)\(attachment.id.uuidString)", type: "image" ) - if let image = attachment.image { - store(image, forPath: ref.path) + if let thumbnail = attachment.image.map(workPendingUploadThumbnail) { + store(thumbnail, forPath: ref.path) } return ref } } + /// Drops every held thumbnail. Called on `didReceiveMemoryWarning` — these + /// exist only to smooth a handoff, and the host copy can always be refetched. + func purge() { + imagesByPath.removeAll() + insertionOrder.removeAll() + } + /// Re-keys each placeholder's image onto the host path the save returned. /// Positional, so it only applies when the save produced a ref for every /// placeholder; otherwise the placeholders are simply released, because a @@ -117,6 +130,21 @@ final class WorkPendingUploadPreviewStore { } } +/// Downscales the composer's upload-sized render to chip size. Returns the +/// original when it is already small enough. +@MainActor +private func workPendingUploadThumbnail(_ image: UIImage) -> UIImage { + let longestSide = max(image.size.width, image.size.height) + guard longestSide > workPendingUploadPreviewMaxPixels, longestSide > 0 else { return image } + let scale = workPendingUploadPreviewMaxPixels / longestSide + let target = CGSize(width: image.size.width * scale, height: image.size.height * scale) + let format = UIGraphicsImageRendererFormat.default() + format.scale = 1 + return UIGraphicsImageRenderer(size: target, format: format).image { _ in + image.draw(in: CGRect(origin: .zero, size: target)) + } +} + func workChatAttachmentIsImage(_ ref: AgentChatFileRef) -> Bool { let type = ref.type.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() return type == "image" || type == "image-url" diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift index 2a9a3261c0..bdd35dc629 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift @@ -92,7 +92,6 @@ func workChatShouldContinueAutomaticOlderHistory( struct WorkChatPrependAnchor { let rowId: String let rowY: CGFloat - let offsetY: CGFloat /// Layout passes to wait for before giving up, so an abandoned prepend cannot /// leave the anchor armed to fire on an unrelated later change. var remainingAttempts: Int @@ -720,7 +719,6 @@ struct WorkChatSessionView: View { scrollMetrics.prependAnchor = WorkChatPrependAnchor( rowId: previousFirstId, rowY: previousFirstRowY, - offsetY: scrollMetrics.offsetY, remainingAttempts: workChatPrependAnchorAttempts ) } @@ -750,7 +748,12 @@ struct WorkChatSessionView: View { var transaction = Transaction() transaction.disablesAnimations = true withTransaction(transaction) { - scrollPosition.scrollTo(y: anchor.offsetY + displacement) + // Corrected from the *live* offset, not the captured one. The reader can + // keep scrolling between arming and the layout pass that measures the + // displacement; correcting from an offset captured at arm time would snap + // them back to where they were when the page started loading. Only the + // inserted height needs undoing. + scrollPosition.scrollTo(y: scrollMetrics.offsetY + displacement) } } diff --git a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift index 9933ecf7ab..f2a38a5b32 100644 --- a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift +++ b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift @@ -364,12 +364,24 @@ final class SyntaxHighlighterStreamingTests: XCTestCase { } let incrementalSeconds = Date().timeIntervalSince(incrementalStart) + ADECodeRenderingCache.shared.purgeOnMemoryWarning() + let wholeTextStart = Date() + for snapshot in snapshots { + _ = SyntaxHighlighter.highlightedSegment(Substring(snapshot), as: .swift) + } + let wholeTextSeconds = Date().timeIntervalSince(wholeTextStart) + ADECodeRenderingCache.shared.purgeOnMemoryWarning() let legacyStart = Date() for snapshot in snapshots { _ = Self.legacyHighlight(snapshot, as: .swift) } let legacySeconds = Date().timeIntervalSince(legacyStart) + print(String( + format: "whole-text with the role fill (no prefix reuse): %.3f ms per tick (%.1fx vs previous)", + wholeTextSeconds * 1000 / Double(snapshots.count), + legacySeconds / max(wholeTextSeconds, .leastNonzeroMagnitude) + )) let ticks = Double(snapshots.count) print(String( @@ -504,20 +516,60 @@ final class SyntaxHighlighterStreamingTests: XCTestCase { ) } + /// Languages allowed to reuse a stable prefix, each pinned to the rule + /// patterns that claim was made about. + /// + /// Reuse is only sound while nothing in a language's rules can match across a + /// newline except the delimiters the boundary counts. That is a property of + /// the patterns, not something the code can re-derive, and every time it has + /// been wrong the symptom was a completed block frozen mis-highlighted in + /// cache. If one of these fingerprints changes, re-check the new pattern + /// against `multilineDelimiters(for:)` before updating the constant. + private static let prefixReuseFingerprints: [FilesLanguage: String] = [ + .swift: "2c7b721d13b3bcc9", + .typescript: "fa894f542c775be5", + .javascript: "2f738c3408aa7929", + .python: "6de225fbadd012d8", + .rust: "563f92af2415db71", + .go: "3575d64645ceff4c", + .java: "aa8e57e8d480c530", + .html: "561765deebafcca7", + ] + func testLanguagesWithUnmodelledMultilineRulesOptOutOfPrefixReuse() { - // CSS selector lists, YAML's `^\s*` key rule, and Markdown links all cross a - // newline with no delimiter to count, so these must not reuse a prefix. - for language in [FilesLanguage.css, .yaml, .markdown] { + // Each of these has a rule whose match crosses, or depends on text past, a + // newline with no delimiter to count: CSS selector lists, YAML's `^\s*` key + // rule, Markdown links, and JSON's `(?=\s*:)` key lookahead. + for language in [FilesLanguage.css, .yaml, .markdown, .json] { XCTAssertNil( SyntaxHighlighter.multilineDelimiters(for: language), "\(language.rawValue) has newline-crossing rules the balance scan cannot model" ) } - for language in [FilesLanguage.swift, .typescript, .javascript, .python, .rust, .go, .java, .html, .json] { + for language in Self.prefixReuseFingerprints.keys { XCTAssertNotNil(SyntaxHighlighter.multilineDelimiters(for: language)) } } + func testPrefixReuseLanguagesStillHaveTheRulesThatClaimWasMadeAbout() { + for (language, pinned) in Self.prefixReuseFingerprints where !pinned.isEmpty { + XCTAssertEqual( + SyntaxHighlighter.tokenRuleFingerprint(for: language), pinned, + """ + \(language.rawValue)'s token rules changed. Prefix reuse assumes no rule \ + matches across a newline except the counted delimiters — re-check the new \ + pattern against multilineDelimiters(for:), then update this fingerprint. + """ + ) + } + } + + func testStreamingJsonKeyLookaheadMatchesFullHighlight() { + // The key rule only matches once `(?=\s*:)` finds the colon, which can + // arrive after the newline — the key would otherwise freeze unhighlighted. + assertIncrementalMatchesFullHighlight("{\n \"key\"\n: 1,\n \"b\": 2\n}", as: .json) + } + func testMultilineCssSelectorStillMatchesFullHighlight() { assertIncrementalMatchesFullHighlight( """ @@ -668,3 +720,4 @@ final class WorkInlineMarkdownCacheTests: XCTestCase { XCTAssertFalse(workMarkdownSharedCacheHolds(text)) } } + From 7c39d31ea91166257e55e52993d054708e3be52c Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 23:36:41 -0400 Subject: [PATCH 08/12] fix(ios): separate the reader's scroll from the inserted height MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the prepend correction with the other half of the same formula. CodeRabbit's finding was that correcting from the offset captured at arm time snaps a still-scrolling reader back; Codex's is that the captured offset is still needed, because the probed row moves by the inserted height *minus* whatever the reader scrolled — scrolling moves the row up the screen too. Correcting by that raw displacement cancels their movement. Adding the offset change back isolates the insertion: with an inserted height H and a user scroll D, the row moves H - D while the offset moves D, so the sum is H either way, and a pure scroll with no prepend sums to zero and correctly restores nothing. The anchor's `offsetY` returns for that purpose rather than as the thing corrections are applied to. 1325 tests, same 13 pre-existing failing cases as clean main. Co-Authored-By: Claude Opus 5 --- .../ADE/Views/Work/WorkChatSessionView.swift | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift index bdd35dc629..79a21a861b 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift @@ -92,6 +92,10 @@ func workChatShouldContinueAutomaticOlderHistory( struct WorkChatPrependAnchor { let rowId: String let rowY: CGFloat + /// The reader's offset when the prepend was armed. Not what the correction is + /// applied to — it is how the reader's own scrolling is separated from the + /// insertion, since the probed row moves by both. + let offsetY: CGFloat /// Layout passes to wait for before giving up, so an abandoned prepend cannot /// leave the anchor armed to fire on an unrelated later change. var remainingAttempts: Int @@ -719,6 +723,7 @@ struct WorkChatSessionView: View { scrollMetrics.prependAnchor = WorkChatPrependAnchor( rowId: previousFirstId, rowY: previousFirstRowY, + offsetY: scrollMetrics.offsetY, remainingAttempts: workChatPrependAnchorAttempts ) } @@ -732,9 +737,16 @@ struct WorkChatSessionView: View { func restorePrependAnchorIfNeeded(probed: WorkChatPrependProbeSample?) { guard var anchor = scrollMetrics.prependAnchor else { return } - // Only a measurement of the anchored row itself can say how far it moved. - let displacement = probed?.rowId == anchor.rowId ? (probed?.y ?? anchor.rowY) - anchor.rowY : 0 - guard displacement > 0.5 else { + // The anchored row moves by the height inserted above it *minus* whatever + // the reader scrolled in the meantime, because scrolling moves the row up + // the screen too. Adding the offset change back isolates the insertion: + // with an inserted height H and a user scroll D, the row moves H - D and the + // offset moves D, so the sum is H either way — and a pure scroll with no + // prepend sums to zero and correctly restores nothing. + let rowShift = probed?.rowId == anchor.rowId ? (probed?.y ?? anchor.rowY) - anchor.rowY : 0 + let scrolled = scrollMetrics.offsetY - anchor.offsetY + let insertedHeight = rowShift + scrolled + guard insertedHeight > 0.5 else { anchor.remainingAttempts -= 1 scrollMetrics.prependAnchor = anchor.remainingAttempts > 0 ? anchor : nil return @@ -748,12 +760,9 @@ struct WorkChatSessionView: View { var transaction = Transaction() transaction.disablesAnimations = true withTransaction(transaction) { - // Corrected from the *live* offset, not the captured one. The reader can - // keep scrolling between arming and the layout pass that measures the - // displacement; correcting from an offset captured at arm time would snap - // them back to where they were when the page started loading. Only the - // inserted height needs undoing. - scrollPosition.scrollTo(y: scrollMetrics.offsetY + displacement) + // Applied to the live offset so a scroll during the prepend is kept; + // only the inserted height is undone. + scrollPosition.scrollTo(y: scrollMetrics.offsetY + insertedHeight) } } From a2fea3997abd14b9febdc1352992b3486d10ca72 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 9 Aug 2026 23:55:08 -0400 Subject: [PATCH 09/12] fix(ios): make echo reconciliation idempotent; keep the prepend probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more findings from Codex, both verified. Echo reconciliation ran against the same transcript more than once — `loadTranscript` reconciles, then the post-send pass reconciles again — and consuming a represented count per call is not idempotent. With two identical sends and one canonical row, the first call correctly retired one echo and the second applied that same count to the already-pruned array and retired the survivor. That is the same bubble-disappears symptom the counted suppression was introduced to fix, one layer down. A key is now retired only once the transcript holds at least as many rows as there are echoes for it, which is stable under repetition and costs nothing in between: `buildWorkTimeline` filters the surplus out of the rendered timeline, and that filter is a pure function of the full echo list. The logic moved to `workLocalEchoesRetiredByTranscript` so the property can be tested directly — the new test reconciles three times against one row and asserts the survivor lives. The prepend probe cleared its recorded sample whenever no row published one. A LazyVStack recycles the probed row if the reader scrolls away while an older-page request is in flight, so the page could land with nothing to arm against and push whatever they had moved on to. The last real measurement is retained instead. 1326 tests, same 13 pre-existing failing cases as clean main. Co-Authored-By: Claude Opus 5 --- .../ADE/Views/Work/WorkChatSessionView.swift | 10 ++++- .../Work/WorkSessionDestinationView.swift | 10 +---- .../ADE/Views/Work/WorkTimelineHelpers.swift | 36 +++++++++++++++ .../WorkSessionCanonicalStateTests.swift | 45 +++++++++++++++++++ 4 files changed, 90 insertions(+), 11 deletions(-) diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift index 79a21a861b..e7ccb8c9af 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift @@ -1311,8 +1311,14 @@ struct WorkChatSessionView: View { .onPreferenceChange(WorkChatPrependProbePreferenceKey.self) { sample in // Recorded into a reference box, not @State: this fires on every // layout pass and must not invalidate the transcript. - scrollMetrics.probeRowId = sample?.rowId - scrollMetrics.probeRowY = sample?.y + // Keep the last real measurement rather than clearing on nil. The + // probed row can be recycled out of the LazyVStack while an older-page + // request is in flight, and forgetting it there means the page lands + // with no anchor to arm and pushes whatever the reader moved on to. + if let sample { + scrollMetrics.probeRowId = sample.rowId + scrollMetrics.probeRowY = sample.y + } restorePrependAnchorIfNeeded(probed: sample) } .onPreferenceChange(WorkChatComposerLayoutHeightPreferenceKey.self) { height in diff --git a/apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift b/apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift index 0eec9c39f7..c2ecba0d5d 100644 --- a/apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift +++ b/apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift @@ -2686,15 +2686,7 @@ struct WorkSessionDestinationView: View { @MainActor func reconcileLocalEchoMessages() { - guard !localEchoMessages.isEmpty else { return } - // Counted rather than set-membership: two sends of the same text share one - // dedupe key, and a single matching transcript row must retire exactly one - // of them. `sending` now releases as soon as the host accepts a message, so - // back-to-back identical sends are easy to produce. - let next = workUnrepresentedLocalEchoMessages( - localEchoMessages, - representedKeyCounts: workRepresentedEchoKeyCounts(from: transcript) - ) + let next = workLocalEchoesRetiredByTranscript(localEchoMessages, transcript: transcript) guard next.count != localEchoMessages.count else { return } localEchoMessages = next } diff --git a/apps/ios/ADE/Views/Work/WorkTimelineHelpers.swift b/apps/ios/ADE/Views/Work/WorkTimelineHelpers.swift index f298f2c718..c206a8c3ea 100644 --- a/apps/ios/ADE/Views/Work/WorkTimelineHelpers.swift +++ b/apps/ios/ADE/Views/Work/WorkTimelineHelpers.swift @@ -2218,6 +2218,42 @@ func workUnrepresentedLocalEchoMessages( } } +/// The echoes still worth keeping after the transcript has caught up. +/// +/// Idempotent by construction, which is the requirement: reconciliation runs +/// more than once against the same transcript (`loadTranscript` reconciles, then +/// the post-send pass reconciles again). Consuming a represented count per call +/// is not idempotent — with two identical sends and one canonical row, the first +/// call correctly retires one echo and the second applies the same count to the +/// already-pruned array and retires the survivor. +/// +/// Retiring a key only once the transcript holds at least as many rows as there +/// are echoes for it is stable under repetition, and costs nothing in between: +/// `buildWorkTimeline` filters the surplus out of the rendered timeline, and +/// that filter is a pure function of the full echo list. +func workLocalEchoesRetiredByTranscript( + _ echoes: [WorkLocalEchoMessage], + transcript: [WorkChatEnvelope] +) -> [WorkLocalEchoMessage] { + guard !echoes.isEmpty else { return echoes } + let representedCounts = workRepresentedEchoKeyCounts(from: transcript) + guard !representedCounts.isEmpty else { return echoes } + + var echoCounts: [String: Int] = [:] + for echo in echoes { + guard let key = workLocalEchoDedupeKey(text: echo.text, attachments: echo.attachments) else { continue } + echoCounts[key, default: 0] += 1 + } + + return echoes.filter { echo in + guard let key = workLocalEchoDedupeKey(text: echo.text, attachments: echo.attachments), + let represented = representedCounts[key], + let outstanding = echoCounts[key] + else { return true } + return represented < outstanding + } +} + /// How many times each echo dedupe key is already represented in the transcript, /// counting delivered user messages and pending steers. func workRepresentedEchoKeyCounts(from transcript: [WorkChatEnvelope]) -> [String: Int] { diff --git a/apps/ios/ADETests/WorkSessionCanonicalStateTests.swift b/apps/ios/ADETests/WorkSessionCanonicalStateTests.swift index 411125ba4d..3e0c1707fd 100644 --- a/apps/ios/ADETests/WorkSessionCanonicalStateTests.swift +++ b/apps/ios/ADETests/WorkSessionCanonicalStateTests.swift @@ -826,6 +826,51 @@ final class WorkSessionCanonicalStateTests: XCTestCase { XCTAssertEqual(remaining.first?.id, echoes[1].id, "the newer echo must survive") } + /// Reconciliation runs repeatedly against the same transcript — `loadTranscript` + /// reconciles, then the post-send pass reconciles again — so retiring by a + /// consumed count would retire the survivor on the second call. + func testRepeatedReconciliationAgainstOneRowKeepsTheUnrepresentedEcho() { + var echoes = [ + WorkLocalEchoMessage(text: "continue", timestamp: iso(now)), + WorkLocalEchoMessage(text: "continue", timestamp: iso(now.addingTimeInterval(1))), + ] + let oneRow = [ + WorkChatEnvelope( + sessionId: "chat-1", + timestamp: iso(now), + sequence: 1, + event: .userMessage( + text: "continue", + attachments: nil, + turnId: nil, + steerId: nil, + deliveryState: nil, + processed: nil + ) + ) + ] + + for pass in 1...3 { + echoes = workLocalEchoesRetiredByTranscript(echoes, transcript: oneRow) + XCTAssertEqual( + echoes.count, 2, + "pass \(pass): one canonical row must not retire both identical echoes" + ) + } + // The rendered timeline still shows one row and one echo, not two of each. + let snapshot = buildWorkChatTimelineSnapshot( + transcript: oneRow, + fallbackEntries: [], + artifacts: [], + localEchoMessages: echoes + ) + let userMessages = snapshot.timeline.filter { entry in + if case .message(let message) = entry.payload { return message.role == "user" } + return false + } + XCTAssertEqual(userMessages.count, 2) + } + func testBothIdenticalEchoesRetireOnceBothRowsArrive() { let echoes = [ WorkLocalEchoMessage(text: "continue", timestamp: iso(now)), From beb3195e256deaf1060e762783c8ec75ffdcc7b4 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 02:39:36 -0400 Subject: [PATCH 10/12] fix(ios): clamp the prepend restore to the scrollable range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-referencing the anchor against t3code's shipped implementation of the same pattern (`ThreadDetailView.restore(_:in:dataSource:)`) surfaced one thing theirs does that this did not: they bound the computed offset to `[-adjustedContentInset.top, contentSize.height - bounds.height + inset]` before `setContentOffset`. A measured inserted height should never produce an out-of-range target, since the content grew by at least that much. But the retained last-probe path — added so a recycled row does not lose the anchor — can carry a measurement from before the recycle, and an unbounded restore turns a stale number into an overscroll past the end of the transcript. Bounded, the same staleness lands a little off instead. The geometry observer tracks the scrollable height again for this, and only this; the correction itself is still derived from the anchored row's own displacement, never from total content growth. 1331 tests, same 13 pre-existing failing cases as clean main. Co-Authored-By: Claude Opus 5 --- .../ADE/Views/Work/WorkChatSessionView.swift | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift index e7ccb8c9af..bda8551e79 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift @@ -107,6 +107,7 @@ struct WorkChatPrependAnchor { final class WorkChatScrollMetrics { var distanceFromBottom: CGFloat = 0 var offsetY: CGFloat = 0 + var scrollableHeight: CGFloat = 0 /// Position of the row currently being probed (the list's first row, or the /// armed row while a prepend is in flight), in the scroll coordinate space. var probeRowId: String? @@ -114,6 +115,21 @@ final class WorkChatScrollMetrics { var prependAnchor: WorkChatPrependAnchor? } +/// The scroll geometry the transcript reacts to, rounded so sub-pixel jitter +/// doesn't wake the observer. +struct WorkChatScrollGeometrySample: Equatable { + let offsetY: CGFloat + /// Largest in-range content offset, used only to clamp a restore. + let scrollableHeight: CGFloat + + init(_ geometry: ScrollGeometry) { + self.offsetY = (geometry.contentOffset.y * 2).rounded() / 2 + let scrollable = geometry.contentSize.height - geometry.containerSize.height + + geometry.contentInsets.top + geometry.contentInsets.bottom + self.scrollableHeight = max(0, (scrollable * 2).rounded() / 2) + } +} + /// Number of layout passes a prepend anchor stays armed for. let workChatPrependAnchorAttempts = 12 @@ -761,8 +777,13 @@ struct WorkChatSessionView: View { transaction.disablesAnimations = true withTransaction(transaction) { // Applied to the live offset so a scroll during the prepend is kept; - // only the inserted height is undone. - scrollPosition.scrollTo(y: scrollMetrics.offsetY + insertedHeight) + // only the inserted height is undone. Clamped to the scrollable range the + // way t3code's `restore(_:in:dataSource:)` bounds its `setContentOffset`: + // a measured height should never land out of range, but the retained + // last-probe path can carry a stale measurement, and a bounded restore + // fails as a slightly-wrong position instead of a blank overscroll. + let target = min(max(0, scrollMetrics.offsetY + insertedHeight), scrollMetrics.scrollableHeight) + scrollPosition.scrollTo(y: target) } } @@ -1220,13 +1241,13 @@ struct WorkChatSessionView: View { .scrollIndicators(.hidden) .scrollDismissesKeyboard(.interactively) .scrollPosition($scrollPosition) - .onScrollGeometryChange(for: CGFloat.self) { geometry in - // Rounded so sub-pixel jitter doesn't wake the observer. - (geometry.contentOffset.y * 2).rounded() / 2 - } action: { _, offsetY in + .onScrollGeometryChange(for: WorkChatScrollGeometrySample.self) { geometry in + WorkChatScrollGeometrySample(geometry) + } action: { _, sample in // Recorded into a reference box, not @State: this fires per scroll // frame and must not invalidate the transcript. - scrollMetrics.offsetY = offsetY + scrollMetrics.offsetY = sample.offsetY + scrollMetrics.scrollableHeight = sample.scrollableHeight } .coordinateSpace(name: workChatScrollCoordinateSpace) .background( From d75211f3af3a87b34c4ff18a7d940fdb6d6862c6 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 02:56:54 -0400 Subject: [PATCH 11/12] fix(ios): only treat a backslash as an escape inside an open string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scan honored a delimiter's escape rule wherever a backslash appeared, including where that delimiter had nothing open. In JavaScript a `\'` sitting in a comment therefore swallowed the apostrophe — but the string rule scans independently of the comment rule and has no preceding-backslash check, so it opens a match right there and runs to the next apostrophe, lines later. The newline in between was marked stable and the span froze into the prefix before its closer arrived. Escapes now apply only while that delimiter's count is odd, which is the scan's own notion of "inside one of these". The template-literal and triple-quote cases still hold — those backslashes occur inside an open string, which is exactly when the rule now fires — and Go's raw strings are unaffected either way. 1332 tests. The failing set matches the baseline apart from `SyncEnvelopeChunkAssemblerTests.testOutboundFramesStayInsideBudgetAndReassemble`, which passes in isolation (18/18) and cannot be reached from a syntax highlighter; recorded as a flake, not adopted. Co-Authored-By: Claude Opus 5 --- .../ADE/Views/Components/FilesCodeSupport.swift | 8 +++++++- .../WorkMarkdownStreamingParsingTests.swift | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift index d5d5675826..fa43a784e4 100644 --- a/apps/ios/ADE/Views/Components/FilesCodeSupport.swift +++ b/apps/ios/ADE/Views/Components/FilesCodeSupport.swift @@ -237,7 +237,13 @@ private func syntaxStableBoundaryOffset( pendingEscape[position] = false continue } - if character == "\\", delimiter.escapes { + // Only inside an open string of *this* delimiter (odd count). Outside + // one a backslash escapes nothing here, and the rules agree: the string + // patterns have no preceding-backslash check, so a `\\'` sitting in a + // comment really can open a match that runs to the next apostrophe lines + // later. Swallowing it would mark that newline stable and freeze the + // span before the closer arrives. + if character == "\\", delimiter.escapes, counts[position] % 2 == 1 { pendingEscape[position] = true continue } diff --git a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift index f2a38a5b32..a0ca853e59 100644 --- a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift +++ b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift @@ -499,6 +499,21 @@ final class SyntaxHighlighterStreamingTests: XCTestCase { ) } + func testEscapedQuoteOutsideAStringStillOpensOne() { + // `\'` in a comment is not an escape — nothing is open for it to escape. + // The string rule has no preceding-backslash check either, so it opens a + // match there that runs to the apostrophe two lines later; treating the + // backslash as an escape would swallow it and mark the newline stable. + assertIncrementalMatchesFullHighlight( + #""" + // path\' here + // it's fine + const x = 1 + """#, + as: .javascript + ) + } + func testStreamingHtmlCommentSpanningLinesMatchesFullHighlight() { // HTML's comment rule spans lines like a `/* */` block; a stable boundary // landing inside one would freeze mis-highlighted markup into the prefix. From 219a49f721cabd3199705d40de18f638ecdcf62d Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 03:14:48 -0400 Subject: [PATCH 12/12] fix(ios): route streaming table cells through the intermediate cache The tail block of a streaming message can parse as a table, and that branch built `WorkMarkdownTable` without forwarding `isStreamingTail`, so its cells fell back to the shared completed-message cache. A table growing cell by cell therefore did exactly what the intermediate exclusion exists to prevent: filled the 256-entry cache with throwaway revisions and evicted the finished messages above it, putting scroll-back re-parsing back on the main thread. The state is threaded through the table into its header and body cells. The regression test streams a cell token by token and asserts a previously cached completed message survives, then that the settled cell is promoted like any other block. 1333 tests, same 13 pre-existing failing cases as clean main. Co-Authored-By: Claude Opus 5 --- .../ADE/Views/Work/WorkMarkdownViews.swift | 10 +++++--- .../WorkMarkdownStreamingParsingTests.swift | 25 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/apps/ios/ADE/Views/Work/WorkMarkdownViews.swift b/apps/ios/ADE/Views/Work/WorkMarkdownViews.swift index 4fe37b006b..21875aa01d 100644 --- a/apps/ios/ADE/Views/Work/WorkMarkdownViews.swift +++ b/apps/ios/ADE/Views/Work/WorkMarkdownViews.swift @@ -91,7 +91,7 @@ struct WorkMarkdownBlockView: View { .padding(10) .background(ADEColor.surfaceBackground.opacity(0.45), in: RoundedRectangle(cornerRadius: 12, style: .continuous)) case .table(let headers, let rows): - WorkMarkdownTable(headers: headers, rows: rows) + WorkMarkdownTable(headers: headers, rows: rows, isStreamingTail: isStreamingTail) case .code(let language, let code): WorkCodeBlockView(language: language, code: code) case .rule: @@ -111,13 +111,17 @@ struct WorkMarkdownBlockView: View { struct WorkMarkdownTable: View { let headers: [String] let rows: [[String]] + /// Cells of a still-growing table are throwaway revisions like any other + /// streaming tail; without this they land in the shared completed-message + /// cache and evict it, which is the eviction bug this branch fixes for prose. + var isStreamingTail = false var body: some View { ScrollView(.horizontal, showsIndicators: false) { VStack(spacing: 0) { HStack(spacing: 0) { ForEach(headers.indices, id: \.self) { index in - WorkInlineMarkdownText(text: headers[index]) + WorkInlineMarkdownText(text: headers[index], isStreamingTail: isStreamingTail) .font(.caption.weight(.semibold)) .padding(10) .frame(minWidth: 120, alignment: .leading) @@ -128,7 +132,7 @@ struct WorkMarkdownTable: View { Divider() HStack(spacing: 0) { ForEach(headers.indices, id: \.self) { index in - WorkInlineMarkdownText(text: index < row.count ? row[index] : "") + WorkInlineMarkdownText(text: index < row.count ? row[index] : "", isStreamingTail: isStreamingTail) .font(.caption) .padding(10) .frame(minWidth: 120, alignment: .leading) diff --git a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift index a0ca853e59..ba9946cd87 100644 --- a/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift +++ b/apps/ios/ADETests/WorkMarkdownStreamingParsingTests.swift @@ -727,6 +727,31 @@ final class WorkInlineMarkdownCacheTests: XCTestCase { } } + /// A streaming tail that parses as a table renders through `WorkMarkdownTable` + /// rather than the paragraph path, so its cells need the same intermediate + /// routing — a long table would otherwise evict the completed messages the + /// exclusion exists to protect. + func testStreamingTableCellRevisionsStayOutOfTheSharedCache() { + let completed = "A finished message worth keeping cached." + _ = markdownAttributedString(completed) + XCTAssertTrue(workMarkdownSharedCacheHolds(completed)) + + // Cells arriving token by token, the way a table streams. + var cell = "" + for token in ["Build", " status", " green", " for", " every", " shard"] { + cell += token + _ = markdownAttributedString(cell, intermediate: true) + XCTAssertFalse( + workMarkdownSharedCacheHolds(cell), + "streaming cell \(cell.debugDescription) must not occupy the shared cache" + ) + } + + XCTAssertTrue(workMarkdownSharedCacheHolds(completed), "the finished message must survive") + _ = markdownAttributedString(cell, intermediate: false) + XCTAssertTrue(workMarkdownSharedCacheHolds(cell), "the settled cell is promoted like any other block") + } + func testMemoryWarningPurgeDropsRenders() { let text = "Something worth caching." _ = markdownAttributedString(text)