Reviewing AI code: I check the intent, not the diff
Generated code is rarely sloppy, it is over-engineered. That barely shows up in a diff, which is why my checkpoint sits before the first line of code.
· 8 min
Andrey Gershengoren · · 9 min
The question comes up in every long-lived project eventually: do we migrate to async/await? And it is almost always asked wrongly — as a yes-or-no question about the whole codebase. My answer these days is a counter-question: where exactly, and does that place deliver a value or a stream?
Because async/await does not replace Combine or RxSwift. It replaces callbacks. Confuse the two and you rewrite reactive chains as AsyncSequences, gain nothing, and lose operators you already had.
Legacy code at this scale is never uniform. In the projects where I worked on this, three generations sat side by side: callback-based networking from the Objective-C era, a reactive middle layer in RxSwift or Combine, and newer features already written with async/await. Plus PromiseKit in corners nobody had touched in years.
That is not negligence, it is the ordinary result of years of product development. And it means there is no state in which "the migration is done". There are only boundaries you draw deliberately, and boundaries that happen by accident.
What makes this more than a matter of taste: once a module compiles under Swift 6 checking, the compiler demands isolation and Sendable information from everything crossing that boundary. That pulls in work nobody wrote on the original ticket.
The case where migration always pays off is the classic cache or service that several places read and write at once. What stood there before was a queue and a comment explaining which method may be called on which thread.
final class TokenStore {
private let queue = DispatchQueue(label: "token")
private var token: Token?
// Only call on `queue`! (comment from 2019)
func current() -> Token? { queue.sync { token } }
func update(_ new: Token) { queue.async { self.token = new } }
}
The comment is the real diagnosis: the rule exists, but nothing enforces it. That is exactly what an actor takes over.
actor TokenStore {
private var token: Token?
func current() -> Token? { token }
func update(_ new: Token) { token = new }
}
The gain is not the shorter spelling. It is that the compiler now enforces the rule instead of a comment. Every access from outside is await and therefore visible in the code — wrong calls become compile errors instead of a crash every few weeks.
One thing that stands out in practice: most of these classes need no isolation of their own, they belong on the main actor, because they are only ever used close to the UI. @MainActor on the class is then the more honest and cheaper answer than a dedicated actor.
This is where the line runs. An actor delivers a value on request. A reactive stream keeps delivering whenever something changes — search input, connection state, location updates, a model three screens depend on.
searchField.textPublisher
.debounce(for: .milliseconds(300), scheduler: RunLoop.main)
.removeDuplicates()
.flatMapLatest { try await api.search($0) } // no such thing
Rebuilding this chain in AsyncSequences means writing debounce, removeDuplicates and above all the cancellation of the previous request yourself. You trade proven operators for your own code, which gets to make the same mistakes again. I started this once and reverted it.
My rule: reactive chains stay where they are. What gets migrated is what hangs off their ends — the individual network call, the individual database query.
For both sides to coexist you need exactly two crossings, and they belong in one place rather than scattered around.
From reactive to async, to pick up a single value at the end of a chain:
extension Publisher where Failure: Error {
func firstValue() async throws -> Output {
for try await value in values { return value }
throw CancellationError()
}
}
And the other direction, to use an old callback API from async code:
func loadProfile() async throws -> Profile {
try await withCheckedThrowingContinuation { continuation in
legacyClient.fetchProfile { result in
continuation.resume(with: result)
}
}
}
The second bridge holds the trap that actually cost us time: a continuation must be resumed exactly once. Old callback APIs do not always honour that — some fire twice on a retry, some never fire at all. The first case crashes at runtime, the second leaves the task suspended forever. So before every withCheckedContinuation comes a look at the callback's implementation, not just its signature.
I migrate from the leaves inward: first the places nothing depends on, then upward. The opposite route — starting at the top with a @MainActor on the view model — looks faster and produces a cascade of isolation errors through every layer below.
In practice: one module per step, turn on strict concurrency inside that module, resolve the errors, ship. Not a project-wide switch followed by months of fighting warnings.
@preconcurrency on an import is a legitimate tool for third-party dependencies that carry no annotations yet. For your own code it is a note, not a solution — it suppresses the question instead of answering it.
The honest price: a migrating project carries two concurrency models at once for a long time, and that is harder to read than either model alone. New team members have to understand both, and every feature raises the question of which world it gets written in. This transitional period lasts longer than anyone plans for.
The second cost is more concrete: the conversion produces changes in files that were previously stable, with nothing in it for the user. That is regression risk without visible return — defensible only where bugs were actually occurring.
For an app that runs stably, changes rarely and shows no concurrency bugs, migration is an end in itself. Nothing forces the issue as long as you stay on the existing language mode.
Likewise for a team with no experience of the model and no time to build it: half-understood actor isolation produces workarounds — state gets pushed into nonisolated corners, Task { } becomes the escape hatch — and you end up with the same disorder as before, in newer syntax.
And for an app that will be rebuilt or moved to shared logic within twelve months, the investment belongs there, not in renovating what is being replaced.
The most useful question before any of these migrations is not "is async/await better", but: is there shared, mutable state here that today is protected only by a comment? Where the answer is yes, the work pays for itself, because a convention becomes a compiler check.
Where only a stream of events flows, you already have the right abstraction. Replacing it costs weeks and delivers the same behaviour.
Generated code is rarely sloppy, it is over-engineered. That barely shows up in a diff, which is why my checkpoint sits before the first line of code.
· 8 min
Anyone getting into an unfamiliar codebase should not read it but interrogate it. The order of the questions decides whether you know something after a day or have seen everything after a week.
· 9 min
You describe the situation, I tell you whether and how I can help. No slides, no sales pitch.