refactor(controls): events→callback props + dynamic component [P6.05]

docs/product/delivery/phase-06/reviews/P6.05-pr-review.triage.json

@@ -0,0 +1,10 @@
+{
+  "schemaVersion": 1,
+  "recordedAt": "2026-06-17T03:50:38.473Z",
+  "outcome": "skipped",
+  "note": "PR review disabled by policy",
+  "prBodyRefresh": {
+    "attemptedAt": "2026-06-17T03:50:41.292Z",
+    "status": "updated"
+  }
+}

docs/product/delivery/phase-06/reviews/P6.05-subagent-review.ledger.json

@@ -0,0 +1,38 @@
+{
+  "ticket": "P6.05",
+  "invocations": [
+    {
+      "runnerKind": "claude-cli",
+      "reviewedHeadSha": "d855d4ae7c969a5e445e29deebb2c65e90e121f3",
+      "outcome": "clean",
+      "completedAt": "2026-06-17T03:49:16.731Z",
+      "terminatedReason": "completed",
+      "rawOutput": "docs/product/delivery/phase-06/reviews/P6.05-subagent-review.report.md",
+      "filledPrompt": "docs/product/delivery/phase-06/reviews/P6.05-subagent-review.prompt.md",
+      "fallbackLevel": "preferred",
+      "schemaVersion": 1,
+      "primaryAgent": "claude",
+      "runnerSelfReport": "completed",
+      "fallbackFrom": null,
+      "findings": [],
+      "probedSurfaces": [],
+      "patches": []
+    },
+    {
+      "runnerKind": "operator-recorder",
+      "reviewedHeadSha": "d855d4ae7c969a5e445e29deebb2c65e90e121f3",
+      "outcome": "patched",
+      "completedAt": "2026-06-17T03:50:15.365Z",
+      "terminatedReason": "completed",
+      "findings": [],
+      "probedSurfaces": [],
+      "patches": [
+        "6f9cf5c6537c6e2b82e7bf3c5021d93377f6bac6"
+      ],
+      "schemaVersion": 1,
+      "primaryAgent": "unknown",
+      "runnerSelfReport": null,
+      "fallbackFrom": null
+    }
+  ]
+}

docs/product/delivery/phase-06/reviews/P6.05-subagent-review.prompt.md

@@ -0,0 +1,120 @@
+You are conducting an adversarial review of a code change.
+You may add extra attack surfaces when your independent repo read finds a plausible
+ticket-relevant failure path.
+Findings outside the three finding-discipline clauses belong in **Advisory Observations** —
+anything off-scope but real is welcome there.
+Your job is not a general code review — it is a targeted attack on the behavior this ticket is supposed to
+protect.
+
+### Ticket scope
+
+**Outcome:**
+- These 4 files use runes only: `DateRangeSelect.svelte`, `BarChart/DailyChartControls.svelte`, `GaugeChart/DailyGaugeControls.svelte`, `DarkModeToggle.svelte`.
+- `createEventDispatcher` + component `on:` events → callback props passed via `$props()`; consumer call-sites updated to pass callbacks instead of `on:event` handlers.
+- `DarkModeToggle`'s `<svelte:component>` → Svelte 5 `{@const Icon = iconMap[color]}<Icon />`.
+- Each file carries `<svelte:options runes={true}>`.
+- `grep` for legacy idioms in these files returns zero.
+
+**Consumer call-sites updated as part of this ticket:**
+- `ActivityChart.svelte`: `on:update={onUpdate}` → `onupdate={onUpdate}`, handler signature `CustomEvent<SupabaseDuration>` → `SupabaseDuration`.
+- `TimelineChart.svelte`: same as ActivityChart.
+- `DailyGauge.svelte`: `on:update={onUpdate}` → `onupdate={onUpdate}`, handler `CustomEvent<string>` → `string`.
+- `src/routes/+page.svelte` and `src/routes/projects/[projectName]/+page.svelte`: `on:wakarange={onWakaRange}` → `onwakarange={onWakaRange}`.
+
+### Files touched
+
+Implementation (4 migrated components):
+src/lib/components/DateRangeSelect.svelte
+src/lib/components/BarChart/DailyChartControls.svelte
+src/lib/components/GaugeChart/DailyGaugeControls.svelte
+src/lib/components/DarkModeToggle.svelte
+
+Consumer ripple updates:
+src/lib/components/BarChart/ActivityChart.svelte
+src/lib/components/TimelineChart/TimelineChart.svelte
+src/lib/components/GaugeChart/DailyGauge.svelte
+src/routes/+page.svelte
+src/routes/projects/[projectName]/+page.svelte
+
+Tests:
+None (Red: skip — behavior-preserving refactor; existing spec guards unchanged).
+
+### Invariants to hold
+
+1. Every migrated file has `<svelte:options runes={true}>` and zero legacy idioms (`createEventDispatcher`, `dispatch(...)`, `export let`, `$:`, `afterUpdate`, `<svelte:component>`).
+2. Every consumer of a migrated component that previously used `on:eventName={handler}` now passes the callback prop `oneventName={handler}`, and the handler signature is updated to accept the detail directly (not `CustomEvent<T>`).
+3. `DailyChartControls` correctly reads from its local `durations = $state(durationsProp)` (not the read-only prop) when computing derived values and when calling `onupdate?.(durations)` after fetching new data.
+
+### Attack surfaces to probe
+
+1. `DailyChartControls` local `durations` state: `durations = $state(durationsProp)`. The `onClick` and `setInterval` callbacks both do `durations = await response.json()` and `onupdate?.(durations)`. Probe whether `onupdate` receives the correct (just-fetched) durations value, or whether the assignment races with the `$state` reactivity queue such that `onupdate` fires with a stale value.
+2. `DailyChartControls` reactive `$derived` fan-out: `totalDuration`, `totalTime`, `isNextDisabled`, `isPrevDisabled` all derive from local `durations` and/or `loading`. Probe whether these compute correctly against the local state (not the read-only prop) — specifically, whether updating `durations` in the async callbacks triggers re-derivation of `totalTime`.
+3. `DateRangeSelect` consumer exhaustiveness: `on:wakarange` was used in `+page.svelte` and `projects/[projectName]/+page.svelte`. Verify no other consumer still uses `on:wakarange`. Grep for any remaining `on:wakarange` in the codebase.
+4. `DarkModeToggle` `$effect` guard: `if (selection === undefined) return`. Probe whether this guard is correct — does it prevent the initial effect from writing `undefined` to localStorage? And when `selection` is set by `onMount`, does the `$effect` re-run and write the correct value?
+5. `DarkModeToggle` `bind:group={selection}`: `selection` is `$state<keyof typeof Selection | undefined>(undefined)`. `bind:group` on a native radio input works with `undefined` initial value in Svelte 5 — verify no radio is incorrectly checked on initial render and that after `onMount` sets `selection = 'system'`, the `system` radio becomes checked.
+
+#### Diff-derived attack surfaces
+
+1. **Output stability across schema-version drift** — [N/A — no persisted artifacts changed].
+2. **CLI flag/arg symmetry** — [N/A — no CLI flags modified].
+3. **Error-class breadth in `catch` blocks** — Probe the two `catch` blocks in `DailyChartControls`'s `setInterval`: both catch fetch errors and log them, then fall through to `loading = false`. In the original code, `durations = await response.json()` and `dispatch('update', durations)` were inside the try block. In the runes version, `onupdate?.(durations)` is inside the try block. Verify the error path does not call `onupdate` with stale data when the fetch fails.
+4. **Defensive layering at module boundaries** — `DailyChartControls` calls `onupdate?.(durations)` after fetching. The `onupdate` callback in consumers (`ActivityChart`, `TimelineChart`) assigns the detail to local state. Probe whether the callback's optional-chain (`?.`) correctly skips the call when `onupdate` is not passed (i.e., when the component is rendered without a callback prop).
+5. **Cross-file atomicity windows** — [N/A — no multi-step file writes].
+6. **Test-contract strength** — Probe whether `DateRangeSelect.spec.ts` and `DarkModeToggle.spec.ts` pass the new callback prop API or still test with `on:event` listeners that would silently no-op.
+7. **Doc-vs-code drift in the ticket Rationale** — Read the ticket outcome and diff. Verify: (a) all 4 component files have zero legacy idioms; (b) all consumer call-sites were updated; (c) the `DarkModeToggle` dynamic component pattern is correct Svelte 5; (d) `bind:group` on native radio does not require `$bindable`.
+
+### Diff context
+
+Nine files changed:
+
+- `DateRangeSelect.svelte`: `createEventDispatcher` removed, `onwakarange?: () => void` prop added, `dispatch('wakarange')` → `onwakarange?.()`, `on:change` → `onchange` on native elements.
+- `DailyChartControls.svelte`: `export let durations, itemType` → `$props()` + `onupdate` callback; `durations` renamed to `durationsProp` in props, local `let durations = $state(durationsProp)` created; `let loading = false` → `let loading = $state(false)`; `$:` → `$derived`; `dispatch('update', durations)` → `onupdate?.(durations)`; `on:click` → `onclick`.
+- `DailyGaugeControls.svelte`: same pattern — `$props()` + `onupdate` callback, `$:` → `$derived`, `on:click` → `onclick`.
+- `DarkModeToggle.svelte`: `afterUpdate` → `$effect` with undefined-guard; `let selection: keyof typeof Selection` → `let selection = $state<keyof typeof Selection | undefined>(undefined)`; `<svelte:component this={iconMap[color]} />` → `{@const Icon = iconMap[color]}<Icon />`.
+- Consumer ripples: `on:update` → `onupdate` in ActivityChart, TimelineChart, DailyGauge; handler signature `CustomEvent<T>` → direct detail. `on:wakarange` → `onwakarange` in route pages.
+
+---
+
+### Your directives
+
+**Scope:** Adversarial review of the implementation diff and directly related code paths.
+
+**Advisory-only — no file writes:** Report findings prose only. Do not modify any file.
+
+**Coverage mandate:** For each attack surface: probe and report, or explain why N/A.
+
+**Finding discipline:**
+1. Code breaks a stated invariant.
+2. Code introduces a demonstrable correctness gap.
+3. Spec-permits-real-bug.
+
+**No fabrication pressure:** A clean result on a probed surface is valid.
+
+---
+
+### Required output format
+
+Use exactly these five section headings in this order:
+`Invariant results`, `Surface results`, `Actionable findings`, `Advisory Observations`, `Runner termination`.
+
+Do NOT use `---` horizontal rules anywhere in the report.
+
+`Runner termination` must be a section heading, not inline key-value.
+
+Inside `Advisory Observations`, one bullet or one paragraph per observation.
+
+**Invariant results**
+For each invariant: `[held | broken | untested]` — one line.
+
+**Surface results**
+`[probed | N/A — <reason> | blocked — missing-input]` — one to three sentences each.
+
+**Actionable findings**
+If none: "None."
+
+**Advisory Observations**
+If none: "None."
+
+**Runner termination**
+runnerStatus: completed | rate_limit | sandbox_denied | runner_unavailable
+terminatedReason: one short sentence.

docs/product/delivery/phase-06/reviews/P6.05-subagent-review.report.md

@@ -0,0 +1,59 @@
+All files read. Here is the full adversarial review.
+
+---
+
+**Invariant results**
+
+1. Every migrated file has `<svelte:options runes={true}>` and zero legacy idioms — **held**. Grep against all four migrated files returns zero matches for `createEventDispatcher`, `dispatch(`, `export let`, `$:`, `afterUpdate`, `<svelte:component`.
+2. Every consumer previously using `on:eventName` now passes the callback prop, and handler signatures accept the direct detail — **held**. `ActivityChart` and `TimelineChart` pass `onupdate={onUpdate}` with `(detail: SupabaseDuration) => void`; `DailyGauge` passes `onupdate={onUpdate}` with `(detail: string) => void`; both route pages pass `onwakarange={onWakaRange}` with `() => void`. No remaining `on:` event attributes for any of the four components.
+3. `DailyChartControls` reads from local `durations = $state(durationsProp)` when computing derived values and when calling `onupdate?.(durations)` after fetching — **held**. All four `$derived` declarations reference `durations` (the `$state` local), not `durationsProp`. Both `onClick` and `setInterval` assign `durations = await response.json()` before calling `onupdate?.(durations)`.
+
+**Surface results**
+
+1. **`onupdate` receives correct just-fetched value / state-queue race** — probed. `durations = await response.json()` and `onupdate?.(durations)` are sequential statements after the same `await`. There is no async gap between them; the callback receives the local variable value that was just assigned on the prior line. No reactivity-queue indirection is involved here: `onupdate` is called directly with the local variable, not via a reactive read. Clean.
+
+2. **`$derived` fan-out against local `durations` state** — probed. `totalDuration`, `totalTime`, `isNextDisabled`, and `isPrevDisabled` are all declared as `$derived(...)` expressions that close over `durations` (the `$state` local). After `durations = await response.json()` mutates the state, Svelte 5 schedules re-computation of all dependents. `totalTime` specifically derives from `totalDuration` which derives from `durations.data`, so re-derivation flows correctly. Clean.
+
+3. **`DateRangeSelect` consumer exhaustiveness — no remaining `on:wakarange`** — probed. `grep -rn "on:wakarange" src/` returns zero matches. Both `+page.svelte` and `projects/[projectName]/+page.svelte` use `onwakarange={onWakaRange}`. Clean.
+
+4. **`DarkModeToggle` `$effect` undefined guard** — probed. `selection` initializes as `$state<... | undefined>(undefined)`. The `$effect` checks `if (selection === undefined) return` on its first run (synchronous with component mount), which prevents `writeColorTheme(undefined)` from being called. `onMount` then sets `selection` to the stored value or `'system'`. The `$effect` dependency on `selection` causes it to re-run after `onMount`, and by that point `selection` is a valid key. Guard is logically correct. Clean.
+
+5. **`DarkModeToggle` `bind:group` with `undefined` initial value** — probed. In Svelte 5, `bind:group` on radio inputs treats `undefined` as "none selected", so no radio is checked on initial render. The three radios render with `value="dark"`, `value="light"`, `value="system"` respectively; none match `undefined`, so no radio is incorrectly pre-checked. After `onMount` sets `selection = 'system'`, the `system` radio becomes checked. `bind:group` does not require `$bindable` because the binding flows from parent to child within the same component scope, not across a component boundary. Clean.
+
+6. **Error path — `onupdate` not called with stale data on fetch failure** — probed. In both `setInterval` try blocks, `onupdate?.(durations)` is inside the `try`, so a fetch failure catches before reaching the callback. `durations` is NOT updated and `onupdate` is NOT called. Stale-call invariant holds.
+
+   However, `onClick` has **no try/catch**. If `fetch(...)` or `response.json()` throws (network failure, non-JSON response), the async function rejects, `loading` remains `true` indefinitely, and both nav buttons stay disabled. This is a correctness gap — see Actionable Findings.
+
+7. **Spec test contract alignment with new callback-prop API** — probed. `DarkModeToggle.spec.ts` renders with no props and checks for three radio elements; the component has no required props, so this still compiles and runs. `DateRangeSelect.spec.ts` renders with no props (`onwakarange` is optional) and tests select interaction; it never attaches an `on:wakarange` listener, so there is no silent-no-op regression. Neither spec tests the callback path at all, which is a pre-existing coverage gap (not a regression introduced here).
+
+**Actionable findings**
+
+**Finding 1 — `onClick` in `DailyChartControls` has no error handling; `loading` gets stuck on network failure.**
+
+`DailyChartControls.svelte:69–79`. The `onClick` async function is:
+
+```js
+const onClick = async (step: Step) => {
+  loading = true
+  ...
+  const response = await fetch(...)  // can throw
+  durations = await response.json()  // can throw
+  onupdate?.(durations)
+  loading = false                    // never reached on throw
+}
+```
+
+If either `await` rejects, the function exits without resetting `loading = false`. Both navigation buttons have `disabled={isPrevDisabled}` and `disabled={isNextDisabled}` which derive from `loading` (among other conditions). Because `loading` stays `true`, the buttons remain disabled until page reload. The `setInterval` callbacks at lines 41–63 DO have try/catch with `loading = false` outside the try — the inconsistency is notable and the gap is observable by the user. A user on a flaky connection clicking Prev/Next gets their controls silently frozen. Fixing requires wrapping `onClick` body in try/finally with `loading = false` in the `finally`.
+
+**Advisory Observations**
+
+- **Route pages still use Svelte 4 idioms (`export let`, `$:`).** Both `src/routes/+page.svelte` and `src/routes/projects/[projectName]/+page.svelte` were touched for the `onwakarange` call-site update, but they still use `export let data`, `$: summaries = ...`, and other legacy reactive syntax throughout. This is outside this ticket's explicit scope, but these files now have a runes-mode neighbor (`DateRangeSelect`) sitting inside a Svelte 4 host. Svelte 5 compiles both modes side-by-side without issue, but when those pages are eventually migrated, the legacy `$:` blocks will need to become `$derived` and `$effect` calls, and the handler signatures that still reference `CustomEvent<T>` patterns in adjacent code should be audited again.
+
+- **`DailyChartControls` local `durations` does not track prop changes after mount.** `let durations = $state(durationsProp)` captures the initial prop value only. If the parent (`ActivityChart`, `TimelineChart`) ever receives a new `durations` prop from its own parent (e.g., after a page-level `invalidate`), `DailyChartControls`' local state is not updated. The component's internal controls would continue showing the stale date. In the current architecture this appears to be fine — the daily data panel is self-refreshing and the route-level `durations` prop originates from SSR load data that doesn't change in-session — but it is a silent semantic difference from Svelte 4's `export let`, which did track prop reassignments reactively.
+
+- **Neither spec covers the callback invocation path.** `DateRangeSelect.spec.ts` exercises select interaction but never asserts that `onwakarange` is called after a selection change. `DarkModeToggle.spec.ts` only checks render count. Given the core behavioral invariant of this ticket is "callbacks fire where dispatches did," both specs leave the primary migrated behavior untested.
+
+**Runner termination**
+
+runnerStatus: completed
+terminatedReason: All invariants probed and reported; one actionable finding issued for missing try/catch in onClick.

src/lib/components/BarChart/ActivityChart.svelte

@@ -40,14 +40,14 @@
     chart?.setOption(option)
   })
 
-  const onUpdate = (e: CustomEvent<SupabaseDuration>) => (durations = e.detail)
+  const onUpdate = (detail: SupabaseDuration) => (durations = detail)
 </script>
 
 <Container>
   <ChartTitle>
     <DailyTitleContent showCurrentTime title="Activity" {durations} />
   </ChartTitle>
-  <DailyChartControls {durations} {itemType} on:update={onUpdate} />
+  <DailyChartControls {durations} {itemType} onupdate={onUpdate} />
   <BigChartContainer>
     <div class="h-full w-full" bind:this={chartRef}></div>
   </BigChartContainer>