chadtmiller.com/knotless

Switch repository

Loading…

What changes

Scrolling past a file no longer loses its diff. In the iOS app's Files changed, a file scrolled off screen before its diff arrived stayed on "This file's diff didn't load." until you left the screen. It now shows its spinner again and loads when you come back to it. Collapsing a file while it loads had the same result, and the same fix.

A read that really fails can be tried again. A network error or a server error now shows Try again under the message, where before it stuck in the same way.

Why

Each file reads its diff from a .task on its spinner, in a lazy stack, and SwiftUI cancels that task when the row leaves the screen. load() used try?, so a cancelled read looked the same as a failed one, and it then set lines = [], which removed the spinner and the task that would have read it again. Loading in Shared.swift already passes over a cancelled read in this way.

The server was not at fault. On the pull that showed this, 31 of its 32 files were asked for at once, and each answered 200 in about 1 to 2 seconds. Each of the four files shown failing answered with rows the app's model reads.

How it was checked

testFlickingPastAPullsFilesLeavesThemToLoad is new in FlowTests. It opens a merged pull's files on live knotless.dev, flicks down and back, then scrolls on, and fails if a file shows the message or a spinner never ends.

Before the fix it passes at this Mac's real speed, as the reads finish before a row leaves the screen. With a temporary 3 second delay added to getPullDiff, not part of this pull, it fails on the message. After the fix it passes both with that delay and without it.

just ios-look files with an empty getPullDiff answer draws the failed state, attached in light and dark.

Shortcomings

The bug was only reproduced with the added delay, so that delay stands in for a slow phone connection. I have not tried this on a phone. Prod's logs were not read, so what the phone's own requests got is not known.

Try again was tapped only against a recorded answer that fails again, so the picture looks the same before and after. Nothing has yet shown it loading a diff after a failure. It clears failed, which brings back the spinner and its task.

A diff that is read again costs another request, as each row's read starts over when it returns.

🤖 Generated with Claude Code

This fixes the iOS Files changed view. A cancelled getPullDiff read, from a row scrolling off screen or a file collapsing, no longer counts as a failure. load() now returns on a CancellationError or a URLError.cancelled, so the spinner and its .task stay and run again when the row returns. Real failures set failed, which shows the message with a Try again button. The button clears failed, which brings the spinner and its task back. I read the whole FileSection and found nothing wrong. I didn't build it or run the UI test. The new UI test depends on live knotless.dev data and on timing, and the body says it passes without the bug at normal speed. I'm not asking for changes over that.