Comfy-Org / Comfy-Org/ComfyUI_frontend

First-run tour: give the run its own transition table (RunEvent + reduceRun)

Open
#14,621 3 comments 1 reaction 1 assignee Claimed by @MaanilVerma View on GitHub
area:new-user-onboarding developer experience enhancement priority:recommended
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

From the architecture review of the first-run tour stack. Recorded as DEC-9 during design; not built.

## The asymmetry

`TourState` got the full treatment — one discriminated union, one writer, one exhaustive pure reducer (`reduceTour`), and every derived value a `computed`. That rewrite is done and it is good.

`RunState` did not. It is a plain 4-string `ref` in `useFirstRunTourController.ts` with **nine write sites across three unrelated mechanisms**: a status watcher, a socket-grace timer, a capture-phase DOM click that writes it optimistically, and two resets.

`command grep -rn 'reduceRun\|RunEvent' src/` returns zero hits.

## Why it is worth doing

This is not a style preference — it is the mechanism behind a real user-visible defect (linked below): a paid user out of credits lands on a card that never reaches a terminal state, because the outcome is *inferred* from stores that a deliberate decision elsewhere keeps empty.

With a transition table, "the card always reaches a terminal state" becomes a property you can prove by inspection instead of one you establish by tracing nine writes across three mechanisms.

```ts
export type RunEvent =
| { type: 'submitted' }
| { type: 'refused'; reason: 'no_credits' | 'rejected' }
| { type: 'accepted' }
| { type: 'settled'; outcome: 'succeeded' | 'failed' }
| { type: 'offlineExpired' }
| { type: 'reset' }

export function reduceRun(state: RunState, event: RunEvent): RunState
```

## Honest cost

~60–80 lines relocated into a pure module, plus an exhaustive transition-table test. **It removes no behaviour and fixes no bug on its own** — the refused case still needs a `refused` dispatch from wherever the queue call fails, which the controller does not currently observe.

It is worth doing because the state is entirely local to one file — nothing like the blast radius the `TourState` rewrite carried — and it will not be this cheap again.

---

**Related:** #14619 is the user-visible defect this would have prevented — a paid subscriber out of credits left on a card that never resolves, because the outcome is inferred from error stores that `app.ts` deliberately keeps empty for account preconditions.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.