From a809812f2bec6595d7f8d85803c278c3cc4da81c Mon Sep 17 00:00:00 2001 From: Danis Ziganshin Date: Sun, 7 Jun 2026 18:09:00 +0300 Subject: [PATCH 1/5] Add discussion doc: concurrency direction (GCD vs async/await) A decision doc (not an implementation ticket) laying out options for the GCD TaskManager: keep & fix, migrate to async/await, or hybrid. Implemen- tation ticket follows once a direction is chosen. Co-Authored-By: Claude Opus 4.8 --- migrations/07-concurrency-direction.md | 79 ++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 migrations/07-concurrency-direction.md diff --git a/migrations/07-concurrency-direction.md b/migrations/07-concurrency-direction.md new file mode 100644 index 0000000..cc40fb4 --- /dev/null +++ b/migrations/07-concurrency-direction.md @@ -0,0 +1,79 @@ +# Discussion: Concurrency direction — keep GCD `TaskManager` or move to async/await? + +> **This is a discussion / decision doc, not an implementation ticket.** +> Nothing is implemented until a direction is chosen below. The PR thread is the +> place to debate it. + +## Current state + +GrowingUp does background work through a small hand-rolled abstraction: + +```swift +public typealias Task = () throws -> Void + +public protocol TaskManager { + func process(tasks: [Task]) +} + +public final class TaskManagerOnGCD: TaskManager { + public func process(tasks: [Task]) { + let queue = DispatchQueue.global(qos: .utility) + let group = DispatchGroup() + for task in tasks { + let workItem = DispatchWorkItem { try? task() } + queue.async(group: group, execute: workItem) + } + } +} +``` + +- Injected into gateways (e.g. `CachePersonsGateway`) via the `Configurator`s. +- Has a test spy (`TaskManagerSpy`). +- Note the current implementation **swallows thrown errors** (`try? task()`), + fires-and-forgets (the `DispatchGroup` is created but never `notify`/`wait`ed), + and the typealias name `Task` now collides with Swift's `_Concurrency.Task`. + +## Options + +### Option A — Keep `TaskManager` (GCD), but fix it +- Smallest change; preserves the protocol seam used by tests/DI. +- Fix the real bugs: propagate/handle errors, actually use `DispatchGroup.notify` + for a completion callback, rename `Task` to avoid the stdlib clash. +- **Cons:** stays on a manual concurrency model; no structured concurrency, + cancellation, or `await` ergonomics. + +### Option B — Migrate to Swift Concurrency (async/await) +- Replace `TaskManager`/`process(tasks:)` with `async` gateway methods and + `async let` / `withThrowingTaskGroup` at call sites. +- Use actors where shared mutable state needs isolation (e.g. caches); make + Core Data access async-friendly. +- **Pros:** structured concurrency, real error propagation, cancellation, + testability without a custom spy, removes the `Task` name clash. +- **Cons:** larger diff; touches gateways, use cases, presenters/view models; + interacts with the SwiftUI and deployment-target tickets. + +### Option C — Hybrid / incremental +- Introduce async/await at new/seam boundaries while keeping `TaskManager` + behind an async adapter, migrating call sites gradually. + +## Recommendation (for debate) + +Lean **Option B (async/await)** *because* the project is already raising its +deployment target (see the iOS 18 ticket) and migrating the UI to SwiftUI — +both of which pair naturally with async/await — and because the current GCD +implementation is quietly buggy (swallowed errors, unused group). The +`TaskManager` protocol seam can be preserved initially (Option C) to keep the +diff reviewable, then retired. + +> The maintainer's stated preference is **not** to keep DispatchGroup in +> preference to async/await. This doc exists to confirm scope/sequencing before +> committing. + +## Decision + +- [ ] **A** — keep & fix GCD `TaskManager` +- [ ] **B** — migrate to async/await +- [ ] **C** — hybrid/incremental, then retire `TaskManager` + +Once a box is checked, a follow-up implementation ticket (same format as the +other `migrations/*.md` prompts) will be authored for the chosen path. From cd3783b9a37183d39d7062d3f2ac13dbf82d6683 Mon Sep 17 00:00:00 2001 From: Danis Ziganshin Date: Sat, 1 Aug 2026 21:32:10 +0300 Subject: [PATCH 2/5] Document async migration stack --- docs/concurrency-migration-plan.html | 408 +++++++++++++++++++++++++ migrations/07-concurrency-direction.md | 5 + 2 files changed, 413 insertions(+) create mode 100644 docs/concurrency-migration-plan.html diff --git a/docs/concurrency-migration-plan.html b/docs/concurrency-migration-plan.html new file mode 100644 index 0000000..eb167f8 --- /dev/null +++ b/docs/concurrency-migration-plan.html @@ -0,0 +1,408 @@ + + + + + + GrowingUp — async/await через stacked PRs + + + +
+
+

Async/await через stacked PRs

+

План превращает существующий PR #8 в нижний слой нативного GitHub Stack и раскладывает миграцию concurrency на три небольших, последовательно зависимых изменения.

+
+ + + + + +
+

Контекст

+

Текущий TaskManagerOnGCD запускает операции с изображениями fire-and-forget, проглатывает ошибки и создаёт неиспользуемый DispatchGroup. Core Data может вернуть успех раньше, чем фотография действительно записана.

+

PR #8 пока содержит только discussion document. Его ветка discuss/concurrency-direction отстаёт от актуального master на 128 коммитов, поэтому реализацию нельзя начинать до синхронизации.

+ + + + + + +
Локальная готовностьgh 2.96.0, git 2.55.0, GitHub authentication активна.
Не хватаетРасширение github/gh-stack ещё не установлено.
GitHub featureStacked PRs находятся в public preview и могут измениться.
ОграничениеВсе ветки стека должны находиться в одном репозитории; GitHub Desktop пока не поддерживается.
+
+ +
+

Цель

+

Перейти на structured concurrency без одного большого PR и без временного сохранения GCD-архитектуры как конечного решения.

+
    +
  • Ошибки обязательных операций доходят до UI.
    • +
  • Completion или async result выдаётся только после согласованного persistence state.
    • +
  • Каждый PR имеет небольшой, понятный diff и собственную проверку.
    • +
  • SwiftData, UI-редизайн и общая SwiftUI-миграция остаются вне этой работы.
    • +
+
+ +
+

Что даёт GitHub Stack

+

В нативном Stack нижний PR направлен в master, а каждый следующий — в ветку слоя под ним. GitHub показывает карту стека, применяет правила защиты и default-branch CI к каждому слою, а после merge нижнего PR автоматически rebase/retarget оставшиеся.

+

Review и merge идут снизу вверх. При необходимости GitHub позволяет слить часть стека или весь стек одной операцией, но для этой миграции безопаснее принимать слои последовательно после отдельной проверки.

+
+ +
+

Предлагаемый стек

+ +
+
+

Верх · PR 8C — call sites и cleanup

+

codex/concurrency-call-sites-cleanup → base: codex/concurrency-async-api

+
+
+

PR 8B — async API Core

+

codex/concurrency-async-api → base: codex/concurrency-image-store

+
+
+

PR 8A — image persistence correctness

+

codex/concurrency-image-store → base: discuss/concurrency-direction

+
+
+

Низ · существующий PR #8 — решение и план

+

discuss/concurrency-direction → base: master

+
+
+ +

PR #8 — решение и план

+

Обновить ветку от origin/master, сохранить discussion document и эту HTML-страницу. Зафиксировать решение: async/await — конечное состояние; incremental path — только способ доставки.

+

Проверка: документация соответствует актуальным call sites; git diff --check.

+ +

PR 8A — image persistence correctness

+

Добавить async ImageStore, заменить TaskManager и spy, но временно сохранить callback API PersonsGateway. CachePersonsGateway должен дожидаться обязательных файловых операций и выдавать completion ровно один раз.

+

Проверка: focused tests на порядок save/edit/delete, rollback частично записанных файлов и отсутствие двойного completion.

+ +

PR 8B — async API Core

+

Добавить async throws API в PersonsGateway и use cases рядом с существующим callback API. Реализовать мосты и Core Data mapping так, чтобы проект продолжал собираться до миграции UI.

+

Проверка: async gateway/use-case tests, error propagation и cancellation boundary; callback call sites пока не меняются.

+ +

PR 8C — call sites и cleanup

+

Перевести presenters и widget на Task с явной main-actor изоляцией. После миграции удалить callback typealiases, переходные overloads, TaskManager.swift и TaskManagerSpy.swift.

+

Проверка: полный build/test/lint, ручные add/replace/clear/remove для обеих фотографий и widget reload.

+
+ +
+

Рабочий процесс со стеком

+ +

1. Подготовить инструмент

+ 
gh extension install github/gh-stack
+gh stack --help
+ +

2. Обновить и принять существующую ветку PR #8

+ 
git fetch origin
+git rebase origin/master
+gh stack init --base master discuss/concurrency-direction
+

Перед rebase рабочее дерево должно быть чистым. Текущий plan commit относится к PR #8; сохранённый отдельно ImageCaptureFlowView.swift не должен попасть в стек.

+ +

3. Добавлять слои только сверху

+ 
gh stack add codex/concurrency-image-store
+# implement, test, commit
+
+gh stack add codex/concurrency-async-api
+# implement, test, commit
+
+gh stack add codex/concurrency-call-sites-cleanup
+# implement, test, commit
+ +

4. Опубликовать единый Stack

+ 
gh stack view
+gh stack submit
+

submit отправляет ветки, создаёт недостающие PR и связывает их в GitHub Stack. Существующий PR #8 должен стать нижним слоем; если GitHub не подхватит его автоматически, использовать gh stack link для связывания существующих PR.

+ +

5. Вносить review fixes в правильный слой

+ 
gh stack checkout BRANCH
+# fix, test, commit
+gh stack rebase --upstack
+gh stack push
+ +

6. Merge снизу вверх

+

Сначала #8, затем 8A, 8B и 8C. После каждого merge убедиться, что GitHub автоматически retargeted следующий PR и его diff не изменился по смыслу.

+
+ +
+

Семантика операций с данными

+ +

Add

+
    +
  1. Записать новые изображения.
    2. +
  2. После успешной записи создать Core Data record.
    3. +
  3. При Core Data failure удалить созданные orphan-файлы.
    4. +
+ +

Edit / replace

+
    +
  1. Записать новые файлы.
    2. +
  2. Переключить image IDs в Core Data.
    3. +
  3. Только после commit удалить старые файлы.
    4. +
  4. При Core Data failure сохранить старые файлы и очистить новые.
    5. +
+ +

Remove

+
    +
  1. Удалить Core Data record.
    2. +
  2. После успеха удалить файлы и memory-cache entries.
    3. +
  3. Cleanup failure логировать как orphan cleanup; не возвращать ложный failure после уже совершённого удаления Person.
    4. +
+
+ +
+

Проверка

+
    +
  • gh stack view --json показывает четыре слоя в правильном порядке.
    • +
  • В GitHub UI видна stack map; base каждого PR совпадает с веткой под ним.
    • +
  • Default-branch Actions запускаются и проходят для каждого слоя.
    • +
  • Diff каждого PR содержит только свою concern и остаётся быстрым для review.
    • +
  • После review fix команда gh stack rebase --upstack переносит изменение наверх без посторонних diff.
    • +
  • После merge нижнего слоя следующий PR автоматически становится направленным в master.
    • +
+ +
+

Verification Needed:

+

Перед созданием стека установить extension и убедиться, что public preview доступен аккаунту zigdanis. После первого gh stack submit отдельно проверить, что уже существующий PR #8 был включён в stack, а не остался самостоятельным PR.

+
+
+ +
+

Риски

+
    +
  • Public preview: команды или UI могут измениться; перед исполнением сверить актуальную документацию.
    • +
  • Старый base PR #8: rebase перепишет историю и потребует безопасного force-with-lease push.
    • +
  • Неверный слой: fix, внесённый слишком высоко, загрязнит последующие diff; изменения нужно коммитить в слой, которому они принадлежат.
    • +
  • Промежуточная компиляция: каждый слой обязан быть зелёным сам по себе; нельзя удалять callback API раньше миграции всех call sites.
    • +
  • UIKit и Sendable: UIImage не должен бесконтрольно пересекать isolation boundaries; формат async ImageStore нужно подтвердить небольшим spike.
    • +
+
+ +
+

Открытые вопросы

+
    +
  1. Считать ли cleanup старого orphan-файла best-effort после успешного Core Data commit? Рекомендация: да, с обязательным logging.
    2. +
  2. Оставить ли PR #8 отдельным documentation layer? Рекомендация: да — он уже существует и естественно становится корнем стека.
    3. +
  3. Открывать ли 8A–8C сразу? Рекомендация: как drafts через gh stack submit, переводить в ready снизу вверх.
    4. +
  4. Merge всего стека или по одному? Рекомендация: по одному снизу вверх, потому что миграция касается пользовательских фотографий.
    5. +
+
+ +
+

Официальные источники

+ +
+
+ + diff --git a/migrations/07-concurrency-direction.md b/migrations/07-concurrency-direction.md index cc40fb4..cebe6fd 100644 --- a/migrations/07-concurrency-direction.md +++ b/migrations/07-concurrency-direction.md @@ -4,6 +4,11 @@ > Nothing is implemented until a direction is chosen below. The PR thread is the > place to debate it. +The current, readable implementation proposal is available as +[`docs/concurrency-migration-plan.html`](../docs/concurrency-migration-plan.html). +It recommends async/await as the destination, delivered in three reviewable +steps so the image-persistence correctness issue is fixed first. + ## Current state GrowingUp does background work through a small hand-rolled abstraction: From a1374ece329b63aedc649953056c2c347e87941b Mon Sep 17 00:00:00 2001 From: Danis Ziganshin Date: Sat, 1 Aug 2026 21:47:47 +0300 Subject: [PATCH 3/5] Refresh concurrency stack plan --- docs/concurrency-migration-plan.html | 39 ++++++++++++++++++---------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/docs/concurrency-migration-plan.html b/docs/concurrency-migration-plan.html index eb167f8..acaaeff 100644 --- a/docs/concurrency-migration-plan.html +++ b/docs/concurrency-migration-plan.html @@ -206,7 +206,7 @@

Async/await через stacked PRs