diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 0000000..045d3bc --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,8 @@ +# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json + +reviews: + auto_review: + enabled: true + drafts: true + base_branches: + - ".*" diff --git a/docs/concurrency-migration-plan.html b/docs/concurrency-migration-plan.html new file mode 100644 index 0000000..9313e31 --- /dev/null +++ b/docs/concurrency-migration-plan.html @@ -0,0 +1,419 @@ + + + + + + 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 и после rebase синхронизирован с актуальным master.

+ + + + + + +
Локальная готовностьgh 2.97.0, github/gh-stack 0.1.0, GitHub authentication активна.
Linux toolingSwift установлен через swiftly; repository scripts используют доступный swift-format напрямую или через xcrun.
GitHub featureStacked PRs находятся в public preview; доступность функции нужно проверить для репозитория.
ОграничениеВсе ветки стека должны находиться в одном репозитории; GitHub Desktop пока не поддерживается.
+
+ +
+

Цель

+

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

+ +
+ +
+

Что даёт 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: codex/linux-swift-format

+
+
+

Prerequisite — Linux Swift checks

+

codex/linux-swift-format → 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.

+ +

Prerequisite — Linux Swift checks

+

Научить formatting scripts выбирать xcrun swift-format в Xcode toolchain или standalone swift-format на Linux. Не пытаться собирать Apple-platform targets без Xcode SDK.

+

Проверка: formatter запускается с одной и той же .swift-format policy на macOS и Linux.

+ +

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/linux-swift-format
+# implement, test, commit
+
+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 --json
+gh stack submit --auto
+

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, затем Linux tooling, 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. +
+
+ +
+

Проверка

+ + +
+

Verification Needed:

+

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

+
+
+ +
+

Риски

+ +
+ +
+

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

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

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

+ +
+
+ + diff --git a/migrations/07-concurrency-direction.md b/migrations/07-concurrency-direction.md new file mode 100644 index 0000000..cebe6fd --- /dev/null +++ b/migrations/07-concurrency-direction.md @@ -0,0 +1,84 @@ +# 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. + +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: + +```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.