# ChapterService Decomposition Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.

**Goal:** Shrink `app/Services/ChapterService.php` from 1,522 lines / ~28 constructor deps (the largest file in `app/`) into a thin facade (~200 lines, 3 deps) by extracting three cohesive collaborators behind its existing public method surface — improving the AI-Compatibility "Method Complexity" score (currently 46/100) without changing any behavior.

**Architecture:** Facade + delegation — the pattern the AI report credits for `FirebaseBusinessService` (2,619→228) and the just-completed `BaseAuthService` (1,465→234). `ChapterService` is a concrete service with a single caller (`Api\ChapterController`); it keeps its 8 public methods as thin delegators to three new injected, interfaced services, so the controller and all 8 routes are untouched. No HTTP response, DB write, or event-dispatch behavior changes.

**Tech Stack:** PHP 8.3, Laravel 13, Pest 4 (`--parallel`), PHPStan level 5 (larastan) + `phpstan-baseline.neon`, Pint (Laravel preset). CI enforces all three on every PR (`.github/workflows/ci.yml`).

**Spec:** this plan (self-contained); derived from `AI-COMPATIBILITY-REPORT.md` Rec 3.1 ("Decompose the six 1,100+ line files") which names `ChapterService` (1,522) as the largest.

## Global Constraints

- **Behavior-preserving refactor.** The 8 endpoints below must produce byte-identical responses, DB writes, and third-party events (Gympass / Bajaj / Meta). The safety net is the existing suite (2,509 passing tests, incl. the ChapterService feature tests); tests stay green at every step.
- **Preserve the public contract.** These 8 public methods are called by `Api\ChapterController` and MUST keep their exact signatures — after extraction they remain on `ChapterService` as thin delegators:
  - `GetUserProgramProgressByDayID(int $userId, int $programId, int $dayId): array`
  - `GetProgramInstructionsByDayID(int $userId, int $programId, int $dayId): array`
  - `getProgramActivtyDetailsByUserId(int $programId, int $userId, ?User $authenticatedUser = null): array`
  - `getProgramActivtyDetailsById(int $userId, int $programId, int $activityId, ?User $authenticatedUser = null): array`
  - `getProgramActionByUserId(int $programId, int $userId, int $actionId, ?User $authenticatedUser = null): array`
  - `updateChapterStatus(int $userId, ?array $postData, ?User $authenticatedUser = null): array`
  - `updateExercise(int $userId, ?array $postData, ?User $authenticatedUser = null): array`
  - `updateAction(int $userId, ?array $postData, ?User $authenticatedUser = null): array`
  - **Do not rename** these or the routes (the typos `upadateChapterStatus`, `getProgramActivty…` are the external API contract).
- **Every new class:** starts with `declare(strict_types=1);`, has a class-level PHPDoc (required by `CLAUDE.md`), implements an interface (repo tracks **0 interfaces** — a scored Architecture win), uses constructor property promotion, and is Pint-clean + PHPStan-level-5-clean.
- **Per-task quality gate (all must pass before the commit checkpoint):**
  - `vendor/bin/pest --parallel` → 0 failures
  - `vendor/bin/phpstan analyse --memory-limit=2G` → `[OK] No errors`
  - `vendor/bin/pint --test <changed files>` → clean
- **PHPStan baseline (`ChapterService` has 17 entries):** when a moved method carries a baselined error, remove the now-unmatched `ChapterService.php` entry. Prefer **fixing** the error in the new file (the fresh `strict_types` + focused class often makes it trivially fixable); only if it genuinely recurs, re-baseline it at the new path (a **relocation**, not new suppression) and justify it in the report. Never add a baseline entry that masks a new error.
- **One task = one PR.** Each task produces independently shippable software; you may stop after any task. CI must be green before merge.
- **Commits are the user's.** The "Commit checkpoint" marks a review gate; the user stages and commits (or, if they granted per-task-commit consent for this run, the controller commits). Do not run `git commit` unless that consent was given.
- **Placement:** new collaborators live in `app/Services/Chapter/` (a new sub-namespace, mirroring the `Firebase/`, `Marketing/`, `Cron/` precedent so tests are discoverable by path).

---

## File Structure

New files (each created with its interface + test):

| File | Responsibility |
|---|---|
| `app/Services/Chapter/ActivityEventDispatcherInterface.php` | Contract for Gympass/Bajaj/Meta activity-event dispatch |
| `app/Services/Chapter/ActivityEventDispatcher.php` | Fire FB/Meta, Gympass, and Bajaj events for chapter/exercise/action progress |
| `app/Services/Chapter/ChapterProgressReaderInterface.php` | Contract for read/progress payload assembly |
| `app/Services/Chapter/ChapterProgressReader.php` | Build the day-progress, instructions, and activity/action read payloads |
| `app/Services/Chapter/ActivityProgressUpdaterInterface.php` | Contract for chapter/exercise/action write flows |
| `app/Services/Chapter/ActivityProgressUpdater.php` | Persist chapter/exercise/action progress + orchestrate side effects |

Modified files:

| File | Change |
|---|---|
| `app/Services/ChapterService.php` | Inject 3 new services; 8 public methods become delegators; move private helpers out; constructor shrinks ~28 → 3 |
| `app/Providers/AppServiceProvider.php` | Bind each new interface → implementation |
| `phpstan-baseline.neon` | Reduce/relocate the `ChapterService` entries as code moves (never new suppression) |

Method → destination map (verified line numbers in current file):

| Method | Line | Destination | Kept as facade delegator? |
|---|---|---|---|
| `GetUserProgramProgressByDayID` | 178 | Reader | yes (contract) |
| `buildChaptersPayload` | 263 | Reader (private) | — |
| `buildFormsPayload` | 312 | Reader (private) | — |
| `buildActionPayload` | 369 | Reader (private*) | — (public-but-internal; see Task 2) |
| `buildExercisesPayload` | 419 | Reader (private) | — |
| `getDayTimeDetail` | 470 | Reader (private) | — |
| `GetProgramInstructionsByDayID` | 533 | Reader | yes (contract) |
| `resolveTodayId` | 598 | Reader (private) | — |
| `getInstructionsPayload` | 620 | Reader (private) | — |
| `updateChapterStatus` | 663 | Updater (split into phases) | yes (contract) |
| `sendFBEvent` | 812 | EventDispatcher | — (public-but-internal) |
| `sendEventWithLogging` | 851 | EventDispatcher (private) | — |
| `updateExercise` | 872 | Updater (split) | yes (contract) |
| `updateOrCreateUserExercise` | 998 | Updater (private) | — |
| `updateOrCreateUserChapter` | 1036 | Updater (private) | — |
| `callGympassEvent` | 1072 | EventDispatcher | — |
| `updateUserQuitStatus` | 1100 | Updater (private) | — |
| `callBajajHealthEvent` | 1121 | EventDispatcher | — |
| `checkAndCreateNextUserDay` | 1154 | Updater (private) | — |
| `getProgramActivtyDetailsByUserId` | 1178 | Reader | yes (contract) |
| `getProgramActivtyDetailsById` | 1229 | Reader | yes (contract) |
| `getProgramActionByUserId` | 1312 | Reader | yes (contract) |
| `updateAction` | 1381 | Updater (split) | yes (contract) |
| `updateOrCreateUserAction` | 1493 | Updater (private) | — |

---

## Task 0: Establish the characterization baseline

**Files:** none (verification only).

- [ ] **Step 1: Confirm the full suite is green** — Run: `vendor/bin/pest --parallel` → expect `0 failed`.
- [ ] **Step 2: Confirm PHPStan is green** — Run: `vendor/bin/phpstan analyse --memory-limit=2G` → expect `[OK] No errors`.
- [ ] **Step 3: Record the covering tests** — Run: `ls tests/Feature/Api/Chapter tests/Feature/Api/UserActivity tests/Unit/Services/ChapterServiceTest.php`. These are the behavior contract that must stay green through Tasks 1–4. Note any endpoint WITHOUT a feature test (e.g. `updateAction`, `getUserAction`) in the PR description — if the write path you're about to split has no feature test, add a minimal one before Task 3.

> **No commit** — Task 0 is a gate.

---

## Task 1: Extract `ActivityEventDispatcher` (lowest risk — do first)

The event helpers are cross-cutting and cohesive, shared by all three write methods. Extracting them first proves the pattern and pulls the Gympass/Bajaj/Meta deps out of the god class.

**Files:**
- Create: `app/Services/Chapter/ActivityEventDispatcherInterface.php`
- Create: `app/Services/Chapter/ActivityEventDispatcher.php`
- Create: `tests/Unit/Services/Chapter/ActivityEventDispatcherTest.php`
- Modify: `app/Services/ChapterService.php` (remove `sendFBEvent`, `sendEventWithLogging`, `callGympassEvent`, `callBajajHealthEvent`; rewire their call sites)
- Modify: `app/Providers/AppServiceProvider.php`

**Interfaces:**
- Consumes (constructor DI — verified from the 4 method bodies): `App\Services\MetaCapiService`, `App\Services\GympassEventService`, `App\Services\BajajHealthEventService`, `App\Http\Queries\PartnerEventQuery`, `App\Http\Queries\UserConfigQuery`.
- Produces:
```php
interface ActivityEventDispatcherInterface
{
    /** @param array<string,mixed> $postData */
    public function sendFBEvent(array $postData, mixed $userData): void;
    public function callGympassEvent(string $eventName, string $eventDetail, mixed $userData): void;
    public function callBajajHealthEvent(int $userId, string $vEmail, int $partnerId, string $vUserPartnerId, string $srcId, string $progressDetails): void;
}
```
(`sendEventWithLogging` moves too but stays `private` — it's the internal try/catch wrapper `sendFBEvent` uses.)

- [ ] **Step 1: Confirm baseline green** — `vendor/bin/pest --parallel && vendor/bin/phpstan analyse --memory-limit=2G`.
- [ ] **Step 2: Create the interface** — `app/Services/Chapter/ActivityEventDispatcherInterface.php` with `declare(strict_types=1);`, namespace `App\Services\Chapter`, class-level PHPDoc, the 3 signatures above.
- [ ] **Step 3: Create the implementation** — `ActivityEventDispatcher.php` (`declare(strict_types=1);`, implements the interface, class PHPDoc, promoted constructor deps). Move the bodies of `sendFBEvent`, `sendEventWithLogging` (private), `callGympassEvent`, `callBajajHealthEvent` verbatim; rewrite `$this->metaCapiService` / `$this->gympassEventService` / `$this->bajajHealthEventService` / `$this->partnerEventQuery` / `$this->userConfigQuery` to the promoted properties and internal `$this->sendEventWithLogging(...)` unchanged.
- [ ] **Step 4: Write a focused unit test** — `tests/Unit/Services/Chapter/ActivityEventDispatcherTest.php`. Mock the 5 deps; assert `callGympassEvent` forwards the right args to `GympassEventService`, and that `sendFBEvent` swallows a thrown exception (via `sendEventWithLogging`) rather than propagating. Use `Log::spy()` + `Log::shouldHaveReceived('error')` per the repo's convention if asserting the logging path.
- [ ] **Step 5: Run the new test** — `vendor/bin/pest tests/Unit/Services/Chapter/ActivityEventDispatcherTest.php` → PASS.
- [ ] **Step 6: Bind the interface** — in `AppServiceProvider::register()`, add `$this->app->bind(ActivityEventDispatcherInterface::class, ActivityEventDispatcher::class);`.
- [ ] **Step 7: Rewire `ChapterService`** — inject `ActivityEventDispatcherInterface $activityEventDispatcher`. In `updateChapterStatus`/`updateExercise`/`updateAction` (still in this file), change every `$this->sendFBEvent(...)` → `$this->activityEventDispatcher->sendFBEvent(...)`, `$this->callGympassEvent(...)` → `$this->activityEventDispatcher->callGympassEvent(...)`, `$this->callBajajHealthEvent(...)` → `$this->activityEventDispatcher->callBajajHealthEvent(...)` (verified call sites: FB at L714/746/752/789, Gympass at L705/916/1428, Bajaj at L726/775/962). Delete the 4 moved methods from `ChapterService`. Remove now-unused deps from `ChapterService`'s constructor ONLY if grep proves no remaining method uses them (defer to Task 4 if unsure).
- [ ] **Step 8: Reduce the baseline** — Run `vendor/bin/phpstan analyse --memory-limit=2G`. If it reports unmatched `ChapterService.php` baseline entries for the moved event code, remove exactly those entries. If a moved error now fires in `ActivityEventDispatcher.php`, fix it in-code if trivial (with strict types it often is), else relocate the entry to the new path and note it.
- [ ] **Step 9: Full gate** — `vendor/bin/pest --parallel && vendor/bin/phpstan analyse --memory-limit=2G && vendor/bin/pint --test app/Services/Chapter app/Services/ChapterService.php app/Providers/AppServiceProvider.php` → all green.
- [ ] **Step 10: Commit checkpoint** — `refactor(chapter): extract ActivityEventDispatcher from ChapterService`.

---

## Task 2: Extract `ChapterProgressReader`

The read side (5 GET endpoints + payload builders). The 5 public read methods are contract methods → they stay as delegators on `ChapterService`.

**Files:**
- Create: `app/Services/Chapter/ChapterProgressReaderInterface.php`, `app/Services/Chapter/ChapterProgressReader.php`, `tests/Unit/Services/Chapter/ChapterProgressReaderTest.php`
- Modify: `app/Services/ChapterService.php`, `app/Providers/AppServiceProvider.php`

**Interfaces:**
- Consumes: determine the read deps by reading the moved method bodies — Run `sed -n '178,663p;1178,1381p' app/Services/ChapterService.php | grep -oE '\$this->[a-zA-Z]+' | sort -u` and inject exactly the query/service members it lists (expected: `chapterQuery`, `dayQuery`, `exerciseQuery`, `formQuery`, `instructionQuery`, `actionQuery`, `userChapterQuery`, `userExerciseQuery`, `userActionQuery`, `userDayQuery`, `userProgramQuery`, `programQuery`, plus any others the bodies reference). Do NOT copy all ~28.
- Produces:
```php
interface ChapterProgressReaderInterface
{
    /** @return array<string,mixed> */
    public function GetUserProgramProgressByDayID(int $userId, int $programId, int $dayId): array;
    /** @return array<string,mixed> */
    public function GetProgramInstructionsByDayID(int $userId, int $programId, int $dayId): array;
    /** @return array<string,mixed> */
    public function getProgramActivtyDetailsByUserId(int $programId, int $userId, ?User $authenticatedUser = null): array;
    /** @return array<string,mixed> */
    public function getProgramActivtyDetailsById(int $userId, int $programId, int $activityId, ?User $authenticatedUser = null): array;
    /** @return array<string,mixed> */
    public function getProgramActionByUserId(int $programId, int $userId, int $actionId, ?User $authenticatedUser = null): array;
}
```

- [ ] **Step 1: Confirm baseline green** — `vendor/bin/pest --parallel && vendor/bin/phpstan analyse --memory-limit=2G`.
- [ ] **Step 2: List the read deps** — run the `sed | grep` above; that set is the constructor DI.
- [ ] **Step 3: Create the interface** (strict types, PHPDoc, the 5 signatures above).
- [ ] **Step 4: Create the implementation** — move the 5 public read methods + the helpers `buildChaptersPayload`, `buildFormsPayload`, `buildActionPayload`, `buildExercisesPayload`, `getDayTimeDetail`, `resolveTodayId`, `getInstructionsPayload` verbatim; rewrite `$this->*` to promoted properties. Make `buildActionPayload` **private** in the reader UNLESS `tests/Unit/Services/ChapterServiceTest.php` calls `ChapterService->buildActionPayload(...)` directly (grep it first: `grep -n buildActionPayload tests/Unit/Services/ChapterServiceTest.php`); if a test calls it, keep it public and add a facade delegator in Step 7.
- [ ] **Step 5: Add a focused test** — `ChapterProgressReaderTest.php` covering one read path (e.g. `GetProgramInstructionsByDayID` with mocked queries) asserting the assembled payload shape matches current output. Prefer building the real reader from the same mocks the existing `ChapterServiceTest` uses.
- [ ] **Step 6: Run the new test** → PASS.
- [ ] **Step 7: Bind + inject + delegate** — bind the interface; inject `ChapterProgressReaderInterface` into `ChapterService`; replace the 5 read methods' bodies with `return $this->chapterProgressReader->METHOD(...);`. Delete the moved private helpers from `ChapterService`.
- [ ] **Step 8: Reduce/relocate baseline** as in Task 1 Step 8.
- [ ] **Step 9: Full gate** — `pest --parallel && phpstan && pint --test app/Services/Chapter app/Services/ChapterService.php app/Providers/AppServiceProvider.php`.
- [ ] **Step 10: Commit checkpoint** — `refactor(chapter): extract ChapterProgressReader`.

---

## Task 3: Extract `ActivityProgressUpdater` (+ split the 3 god methods)

The write side. `updateChapterStatus` (149 L), `updateExercise` (126 L), `updateAction` (112 L) are contract methods → delegators. Their persistence helpers move too. This task also splits each god method into phase methods.

**Files:**
- Create: `app/Services/Chapter/ActivityProgressUpdaterInterface.php`, `app/Services/Chapter/ActivityProgressUpdater.php`, `tests/Unit/Services/Chapter/ActivityProgressUpdaterTest.php`
- Modify: `app/Services/ChapterService.php`, `app/Providers/AppServiceProvider.php`

**Interfaces:**
- Consumes: `ActivityEventDispatcherInterface` (Task 1 — the writers fire events through it), plus the write-side queries — determine via `sed -n '663,812p;872,1178p;1381,1522p' app/Services/ChapterService.php | grep -oE '\$this->[a-zA-Z]+' | sort -u` (expected: `userChapterQuery`, `userExerciseQuery`, `userActionQuery`, `userDayQuery`, `userProgramQuery`, `userQuitStatusQuery`, `dayQuery`, `programQuery`, `userInfoQuery`, `userResponseFileService`, `userUtilityService`, `activityEventDispatcher`, and others the bodies reference).
- Produces:
```php
interface ActivityProgressUpdaterInterface
{
    /** @param array<string,mixed>|null $postData @return array<string,mixed> */
    public function updateChapterStatus(int $userId, ?array $postData, ?User $authenticatedUser = null): array;
    /** @param array<string,mixed>|null $postData @return array<string,mixed> */
    public function updateExercise(int $userId, ?array $postData, ?User $authenticatedUser = null): array;
    /** @param array<string,mixed>|null $postData @return array<string,mixed> */
    public function updateAction(int $userId, ?array $postData, ?User $authenticatedUser = null): array;
}
```

- [ ] **Step 1: Confirm baseline green.**
- [ ] **Step 2: List the write deps** via the `sed | grep` above → constructor DI (include `ActivityEventDispatcherInterface`).
- [ ] **Step 3: Create the interface** (strict types, PHPDoc, the 3 signatures above).
- [ ] **Step 4: Create the implementation** — move `updateChapterStatus`, `updateExercise`, `updateAction` and the private helpers `updateOrCreateUserExercise`, `updateOrCreateUserChapter`, `updateOrCreateUserAction`, `updateUserQuitStatus`, `checkAndCreateNextUserDay` verbatim; rewrite `$this->*` to promoted properties and `$this->activityEventDispatcher->...` for the event calls (already rewired in Task 1).
- [ ] **Step 5: Split each god method into phase methods** *inside the updater* — e.g. `updateChapterStatus` → `guardRequest()` → `persistChapter()` (`updateOrCreateUserChapter`) → `syncQuitStatus()` → `dispatchChapterEvents()` → `advanceDay()`. Each `private function` ≤ ~40 lines, called in the SAME order with the SAME data flow. Do the same for `updateExercise` and `updateAction`. Preserve exact ordering and side effects (this is what takes them off the "god methods >100 lines" list).
- [ ] **Step 6: Add characterization coverage** — the existing `UpadateChapterStatusApiTest` / `UpdateExerciseApiTest` feature tests exercise these; confirm they now hit `ActivityProgressUpdater`. Add a focused test asserting `updateAction`'s persistence + Gympass event fire once each (if `updateAction` has no feature test, this is required, not optional).
- [ ] **Step 7: Bind + inject + delegate** — bind the interface; inject `ActivityProgressUpdaterInterface` into `ChapterService`; replace the 3 write methods' bodies with `return $this->activityProgressUpdater->METHOD(...);`. Delete the moved helpers from `ChapterService`.
- [ ] **Step 8: Reduce/relocate baseline** as before.
- [ ] **Step 9: Full gate** — `pest --parallel` (watch the UserActivity feature tests) `&& phpstan && pint --test`.
- [ ] **Step 10: Commit checkpoint** — `refactor(chapter): extract ActivityProgressUpdater; split write god-methods into phases`.

---

## Task 4: Slim `ChapterService` to a facade + prune the constructor

**Files:** `app/Services/ChapterService.php` (cleanup only).

- [ ] **Step 1: Measure** — Run: `wc -l app/Services/ChapterService.php` — expect ~150–220 lines (from 1,522).
- [ ] **Step 2: Prune the constructor** — `ChapterService`'s methods are now all delegators. Run `grep -oE '\$this->[a-zA-Z]+' app/Services/ChapterService.php | sort -u` — the only members used should be `$this->chapterProgressReader`, `$this->activityProgressUpdater`, `$this->activityEventDispatcher` (and `buildActionPayload`'s delegate target if kept). Remove every other constructor param + promoted property + `use` import. (Unlike BaseAuthService, `ChapterService` has NO subclasses, so nothing else reads its properties — the prune is unconstrained.) Confirm the expected keep-set is exactly those 3 services; if grep shows another member still used, keep it and note why.
- [ ] **Step 3: Confirm the contract is intact** — the 8 public methods still present as delegators with unchanged signatures; `Api\ChapterController` is not in the diff (unchanged).
- [ ] **Step 4: Add/verify class-level PHPDoc** on the slimmed `ChapterService` describing it as a facade over the three collaborators.
- [ ] **Step 5: Full gate** — `pest --parallel && phpstan && pint --test app/Services/ChapterService.php app/Services/Chapter`.
- [ ] **Step 6: Commit checkpoint** — `refactor(chapter): slim ChapterService to a facade over 3 services`.

---

## Self-Review

- **Spec coverage:** Rec 3.1's target (decompose `ChapterService` 1,522) is covered by Tasks 1–4; the "split god methods" sub-goal is Task 3 Step 5; `strict_types`/interfaces/class-PHPDoc are in Global Constraints and every task's Step 3. ✅
- **Contract safety:** all 8 controller-called public methods survive as delegators (Global Constraints lists them; Tasks 2 & 3 keep them). `Api\ChapterController` and the 8 routes are never modified. ✅
- **Type consistency:** the interface signatures in each task's "Produces" block are copied verbatim from the current `ChapterService` method signatures (lines 178–1493). ✅
- **Dependency ordering:** Task 1 (EventDispatcher) precedes Task 3 (Updater) because the writers depend on the dispatcher — flagged in Task 3 Consumes. Reader (Task 2) is independent. ✅
- **Baseline hazard:** flagged in Global Constraints + each task's Step 8 — moved errors are removed at the old path and fixed-or-relocated (never new suppression) at the new path. ✅
- **Coverage hazard:** flagged in Task 0 Step 3 and Task 3 Step 6 — endpoints lacking a feature test (`updateAction`/`getUserAction`) get one before the write split, so the highest-risk change keeps a safety net.