spec: First-deck-success activation wave by aalemayhu · Pull Request #2446 · 2anki/server

aalemayhu · 2026-05-19T09:56:03Z

What this spec covers

One activation-focused wave bundling four filed, reproducible dead-ends in the first 30 seconds of a new visitor's journey. Each is independent; the wave framing exists so the four merges can be measured as one activation lift, not four scattered fixes.

Anonymous trial-on-registration: unblock the upload-limit dead-end #2362 — Anonymous trial-on-registration: replace the "Create free account" / hidden "Start 1-hour trial" dual CTA with a single "Create account and start trial" primary action; trial starts atomically with signup.
Improve the default empty flashcard error #721 + feat: create a better default error message #731 — Replace the hostile empty-deck string ("Could not create any cards. Did you write any togglelists?") with copy that names what happened, points at common-problems docs, and distinguishes the empty-result case from the parser-error case.
Login page should terminate redirect loops on invalid session cookie #2352 — Login page distinguishes "no cookie" from "invalid cookie" and clears the bad cookie on a single render, breaking the redirect loop returning users hit when SECRET rotates or OAuth client changes.
Provide new users with onboarding #252 — Four-step popover tour on first authenticated visit to /upload, persistent Skip on every step, onboarded-flag stored on the user row (not localStorage) so it doesn't reappear on a new device.

Spec: Documentation/specs/first-deck-success.md

Trio synthesis

PM: Activation is the funnel stage with the worst leakage today; ship the four together so the wave is measurable as one jump.
Designer: The user moment is "I uploaded my notes and I don't have my deck yet." Every dead-end here is a copy or flow problem; the trial-on-registration single primary action and the empty-deck error rewrite carry most of the lift.
Engineer: Four loosely-coupled changes, mostly in web/. One server change (atomic signup-with-trial). One small migration (users.onboarded_at). Effort S+S+M+M = M+ overall.
Agreement: Ship as one wave with one combined changelog entry once all four PRs land in main.
Conflict: PM wanted server-side draft storage of the uploaded file across registration; engineer pushed back on the storage primitive. Resolved by using sessionStorage of the filename reference + re-prompt to re-attach, not binary persistence.
Resulting plan: Four PRs, one per issue, merged in this order: empty-deck copy -> login-loop fix -> trial-on-registration -> onboarding.

Status

Draft. The same PR /implement will graduate to ready-for-review later. Do not start a second branch when implementation begins.

^{Need help on this PR? Tag @codesmith with what you need.}

Let Codesmith autofix CI failures and bot reviews

Aggregate spec covering #2362 (anonymous trial-on-registration), #721+#731 (empty-deck error copy), #2352 (login redirect loop), and #252 (first-upload onboarding tour) as one activation wave. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

netlify · 2026-05-19T09:56:09Z

✅ Deploy Preview for notion2anki ready!

Name	Link
🔨 Latest commit	`5cde959`
🔍 Latest deploy log	https://app.netlify.com/projects/notion2anki/deploys/6a0c33b678ef250008121abb
😎 Deploy Preview	https://deploy-preview-2446--notion2anki.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

sonarqubecloud · 2026-05-19T09:56:31Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@codesmith

## What When an upload produces no cards, the upload page now shows the same info card whether the server caught it (`EmptyDeckError` → 400) or the parser produced an empty 200 response. Body copy is format-agnostic — works for Notion, markdown, csv, PDF — and links to `/documentation/help/common-problems`. Generic parser failures get a sharper fallback that names the file context and routes the user to support. Wave PR 1 of 4 from the [first-deck-success spec](#2446). Issues #721 and #731. ## Why The previous server message ("No toggles found in [filename]…") read wrong for any non-Notion upload — markdown and PDF files don't use Notion toggles, so the suggestion to "wrap your content in toggles" was incorrect for those formats. The default empty-deck body on the 200+0 path had the same Notion-centric bias. The conversion-error fallback ("Something went wrong. Try again. If the problem keeps happening, email support@2anki.net.") didn't name the file context, so users couldn't tell if it was a network issue or a parser failure. ## How - `EmptyDeckResponse` server-side now carries `code: 'empty_export'` and `docsLink: '/documentation/help/common-problems'` alongside the message. Existing `empty_export` code in `UploadErrorBody` is now actually used. - `UploadForm` routes any 400 with `code: 'empty_export'` to the existing `'emptyDeck'` zone-state (info card) instead of the generic error state. Helper `zoneStateForUploadError` keeps the three upload-handler call sites consistent. - Default `renderEmptyDeckBody` branch (non-Google-Drive) renders the spec text with an inline link to common-problems. - "Download empty deck" button is hidden when no blob is available (the 400 path has no downloadable empty deck). - `classifyUploadError` falls back to upload-context copy ("Something broke while reading this file. Try again, or send the file to support@2anki.net so we can fix the parser.") rather than the generic "Something went wrong." that's correct for thrown errors but wrong for upload responses. ## Measuring success Empty-deck-to-docs link clicks become trackable from `/documentation/help/common-problems` referrer. The parser-error fallback rate stays low (it's a true-fallback path); if it spikes, that's a regression elsewhere. ## Testing - Server: `pnpm test src/services/UploadService.test.ts` — new assertions for `code`, message, and `docsLink`. - Web: `pnpm --filter 2anki-web test:run` — three new vitest cases: - 200 + 0-cards renders the new body copy with the common-problems link - 400 + `code: 'empty_export'` lands in the emptyDeck info card (not the error state, no download button) - `classifyUploadError` unit test for the parser-error fallback - All 742 web tests + 7 UploadService server tests pass. - Web typecheck + biome lint clean. ## Risks - Behavior change: a thrown `EmptyDeckError` server-side used to render under "Something went wrong" with the WarningIcon; it now renders as the info-card empty-deck state. Visually different. - The "Download empty deck" button is now hidden when there's no blob — previously it rendered even with no link (dead click). No user impact. ## Goal alignment Activation. The first-deck-success spec frames the four upload-screen dead-ends as the cheapest activation win. Empty-deck copy that names the next step (instead of confusing non-Notion uploaders with toggle vocabulary) keeps users moving instead of bouncing. ## Drive-by fix The pre-push tsc hook caught two pre-existing errors in `src/services/NotionService/blocks/lists/BlockTable.tsx` (from PR #2445, merged earlier today) where `r.type === 'table_row'` doesn't narrow against `PartialBlockObjectResponse | BlockObjectResponse`. One commit on this branch adds the `'type' in r` guard. Existing 9 BlockTable tests still pass. ## Notes - **Changelog**: deferred per the [first-deck-success spec](#2446) — one combined changelog entry lands with the final PR of the wave (onboarding tour). The draft wording is in the spec. - **Sonar local**: skipped — anonymous run unauthorized for this project, no token configured locally. Change is copy-only plus a one-line helper; cognitive complexity unlikely.  --- <a href="https://app.blacksmith.sh/2anki/codesmith/server/pr/2453"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> Need help on this PR? Tag <code>@codesmith</code> with what you need. - [ ] Let Codesmith autofix CI failures and bot reviews  --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@codesmith

## What When a user visits `/login` with a stale or invalid session cookie, `LoginPage` now clears that cookie on render. Previously the stale cookie persisted in the browser, causing the server-side `checkUser` to see a cookie and redirect the user back to `/upload` on the next hard navigation — creating a loop. ## Why Fixes #2352. Part of wave 2 of 4 in the first-deck-success activation wave (umbrella PR #2446). The root cause: `checkUser` in `UsersController.ts` calls `getUserFrom(req.cookies.token)`. With a valid token it redirects to `/upload` (or wherever). With no cookie it serves `index.html` (the login form). With a **stale cookie** (expired JWT, rotated SECRET), `getUserFrom` throws and Express 5 returns a 500. The stale cookie persists across browser sessions until it expires or the user manually clears it — but during that window the user can land in a redirect loop. The fix is symptom-level and intentional per the spec: it prevents the loop regardless of why the cookie is invalid. Root causes (SECRET rotation, OAuth client rotation) remain separate issues. ## How `LoginPage.tsx` adds a `useEffect` that watches the `token` cookie and the `isError` flag from `useUserLocals`. When both are true (cookie present, fetch errored), it removes the cookie via `useCookies`. The fix does not affect the login flow, the `useHandleLoginSubmit` hook, or server-side code. The cookie was set without `HttpOnly` (plain `res.cookie('token', token)` in `UsersControllers.ts`), so it is readable and deletable from JavaScript. ## Measuring success After this ships: a user with a stale cookie who navigates to `/login` will see the login form on first render without a redirect loop. Measurable via Sentry (no more 500 cascade on `/login` from stale cookies) and by direct testing with an expired JWT. ## Testing - Unit tests added: `web/src/pages/LoginPage/LoginPage.test.tsx` — 4 tests covering: - No-cookie path renders form normally - Valid cookie + user data renders form normally - Stale cookie + `isError` from `useUserLocals` → cookie removed - No cookie + `isError` → no change (nothing to remove) - All 19 LoginPage tests pass (4 new + 15 existing) - Full web vitest suite: 746 tests pass across 96 files - TypeScript: clean - Biome lint: clean ## Changelog Deferred to the wave's final PR per the first-deck-success spec (umbrella PR #2446). A single combined entry will land there. ## Risks - The `useEffect` fires on every render where cookie+isError are true, but since `removeCookie` is stable and React deduplicates effects, this is safe. - `useUserLocals` uses `retry: 3` — the effect fires after the third failure settles (`isError` becomes true), not on each retry attempt. - Does not change the server-side `checkUser` or `getUserFrom` behavior. The root loop causes (SECRET rotation, etc.) are tracked separately. ## Goal alignment Removes a friction point that makes the site look broken to returning users with stale cookies — directly addresses onboarding/re-activation conversion. Moves toward the 300K-user goal by not losing users at the login door. ## Sonar `sonar-scanner` not run locally — `SONAR_TOKEN` not configured in this environment. Changes are small (1 `useEffect`, 1 test file). No new HTTP calls, no user-input reflection, no zip extraction, no taint paths.  --- <a href="https://app.blacksmith.sh/2anki/codesmith/server/pr/2454"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> Need help on this PR? Tag <code>@codesmith</code> with what you need. - [ ] Let Codesmith autofix CI failures and bot reviews  --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

@codesmith

… trial atomically (#2455) ## What Anonymous users who hit the upload limit (card count or file size) now see a single primary CTA — "Create account and start trial" — instead of the previous two-button layout ("See upgrade options" + hidden "Start 1-hour trial"). The CTA links to \`/register?redirect=/upload&start_trial=1\`. The registration endpoint atomically creates the account and starts the 1-hour trial in a single request. The originally-uploaded filename is preserved across the redirect via \`sessionStorage\`, and a one-line banner on the post-register upload page reads "Re-attach <filename> to convert." ## Why Fixes #2362. Part of umbrella PR #2446 (first-deck-success activation wave). Wave 3 of 4. Merge order: after PR #2352, before PR #252 (onboarding tour). Today the trial CTA for anonymous users was the "Sign in to start trial" link on the limit screen. The user had to: (1) sign in or register, (2) come back to upload, (3) separately start the trial. This PR collapses those steps into one: click the CTA → register → trial is already started. ## How **Server** (`src/controllers/UsersControllers.ts`): The existing `/api/users/register` endpoint now reads `start_trial` from the request body. If set to `'1'`, it calls `StartTrialUseCase` after the user row is created and the JWT cookie is set. If the trial start fails (already used, already paid, or any error), the failure is logged but the signup succeeds — account creation is the more valuable side effect. **Client** (`Backend.ts` / `RegisterForm` / `RegisterPage`): `RegisterPage` reads `start_trial=1` from the query string and passes it as a prop to `RegisterForm`. `RegisterForm` forwards it to the backend `register` call. The submit button label changes to "Create account and start trial" when the flag is present. **Upload limit screen** (`UploadForm.tsx`): For anonymous users, replaced the two-button layout with a single `<Link>` to `/register?redirect=/upload&start_trial=1`. The filename is stored in `sessionStorage` (`upload_pending_filename`) when clicking the CTA. Added `saveFilenameForReattach` helper. **Post-register banner** (`UploadPage.tsx` + CSS): Reads `upload_pending_filename` from `sessionStorage` on mount. If present, shows a one-line status banner: "Re-attach <filename> to convert." The sessionStorage entry is cleared on the next successful upload. The `getLimitDescription` function was refactored to take a `'anonymous' | 'trial_available' | 'trial_used'` context instead of a boolean, giving each case its own copy. ## Measuring success - Log line `Trial start failed after registration` appearing in prod logs at <1% rate confirms the happy path works and the fallback is catching edge cases - A new `trial_started_at` row set on the same DB write as `created_at` (query: `SELECT count(*) FROM users WHERE trial_started_at::date = created_at::date AND created_at > now() - interval '24 hours'`) confirms atomic registration + trial is happening in production ## Testing - Unit tests added: 3 new server-side tests in `UsersControllers.test.ts` covering the trial-on-register happy path, fallback when trial already used, and absence of trial when flag is not set - 2 new UploadPage tests for the reattach banner (shown / not shown based on sessionStorage) - Existing UploadForm limit-screen test updated to assert the new single-CTA design for anonymous users - All 742 web tests pass; all 34 server controller tests pass; tsc clean on both server and web; Biome lint clean ## Changelog Deferred to wave's final PR per spec (#2446 umbrella). No entry in this PR. ## Risks - `start_trial` can only be triggered from within the same atomic `/users/register` request — there is no path for an attacker to trigger a trial on a third-party user's account. The flag is body-only (not a query param on the endpoint itself) and gated behind a new account creation. An existing user attempting to re-register gets a 400 before the trial code is reached. - If `StartTrialUseCase` throws unexpectedly, the error is caught and logged; the signup response is still 200 with the JWT cookie set. The user can start the trial manually via the existing "Start 1-hour trial" button if needed. - Filename stored in `sessionStorage` is rendered via React's normal text path (no `dangerouslySetInnerHTML`). CWE-79 safe. - JWT secret is unchanged — no hardcoded fallback introduced. CWE-321 safe. - `/security-review` is recommended before merge: this PR touches the signup flow (CWE-862 check above) and the trial-start path. ## Goal alignment Directly addresses the first-deck-success activation gap: anonymous users who hit the limit can now convert in a single click-through instead of a multi-step detour. Reduces drop-off at the most critical conversion funnel step. ## Sonar `sonar-scanner` was not run locally (no `SONAR_TOKEN` configured in this environment). New code paths are covered by unit tests. The only new security-adjacent path is the `start_trial` body flag handling, which does not touch user input beyond a string equality check against a fixed value.  --- <a href="https://app.blacksmith.sh/2anki/codesmith/server/pr/2455"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> Need help on this PR? Tag <code>@codesmith</code> with what you need. - [ ] Let Codesmith autofix CI failures and bot reviews  --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

@codesmith

## What Adds a four-step popover tour that appears once on a new user's first authenticated visit to `/upload`. Each step has Back / Next / Skip controls. Pressing Skip marks the user as onboarded (stored in `users.onboarded_at TIMESTAMPTZ NULL`) so the tour never appears again — including on new devices. Closes #252. Wave 4 of 4 — closes the first-deck-success spec (#2446). ## Why A new account-holder reaching `/upload` with no orientation gives up because they don't know where to start. The four-step tour surfaces the four key actions at exactly the right moment without blocking the workflow. ## How - **Migration**: `20260608000000_add_onboarded_at_to_users.js` — `users.onboarded_at TIMESTAMPTZ NULL DEFAULT NULL`. `pnpm kanel` types updated. - **Server**: `markOnboarded(userId)` in `UsersRepository` (idempotent — only flips NULL → now()), `markOnboarded` controller method reading `userId` from `res.locals` (CWE-639), new route `PATCH /api/users/me/onboarded` behind `RequireAuthentication`. - **`getLocals` response**: adds `created_at` and `onboarded_at` to the user object so the frontend can gate the tour without a second fetch. - **Client**: `OnboardingTour` component (~150 LOC, zero new deps), `markOnboarded.ts` API call. Tour gates on `created_at >= 2026-06-08` (migration timestamp) AND `onboarded_at === null`. - **Cutoff**: users created before the migration don't see the tour — no re-onboarding existing users. ## Measuring success Log line: `PATCH /api/users/me/onboarded` 204 responses at > 0 per day within the first week post-deploy. Metric to track: ratio of new users (created_at >= 2026-06-08) who complete their first conversion within 24 hours of signup. ## Testing - Vitest: 10 new unit tests in `OnboardingTour.test.tsx` covering first-time render, already-onboarded suppression, pre-migration-cutoff suppression, step navigation, Back/Next/Skip, and final-step Skip. - Playwright: 4 e2e tests in `onboarding-tour.spec.ts` covering the Skip end-to-end path and the three suppression conditions. - `StripeController.test.ts`: updated `buildUser` fixture to include `onboarded_at: null` after the kanel regen. - All 752 Vitest tests pass. Server tsc clean. ## Changelog ``` { type: 'feature', title: 'Upload page — a short guided tour walks you through your first conversion on a new account, and a clearer message shows when a file produces no cards', date: '2026-06-08' } ``` This entry covers the first-deck-success wave (#2446): this PR (tour), #2453 (empty-deck copy), trial-on-reg, and login-loop. Per the spec, the combined wave entry lands in this final PR. ## Risks - The tour only appears for new users (created_at ≥ migration timestamp). Existing users are unaffected. - `markOnboarded` is idempotent — calling it twice is safe. - Rollback: drop the `onboarded_at` column via the `down` migration; remove the route; revert the client component. Tour won't appear. - The kanel types were updated manually (kanel binary not installed in worktree). Running `pnpm kanel` post-migration will produce identical output. ## Security - Endpoint reads `userId` from `res.locals.user` (set by `RequireAuthentication` middleware) — never from the request body. CWE-639 mitigated. - No sensitive data logged or returned. - User-row mutation: `/security-review` recommended before merge. ## Goal alignment Reduces first-visit abandonment for new users, directly contributing to the 300K-user retention goal. A user who completes one successful conversion is significantly more likely to return. ## Note on sonar-scanner `sonar-scanner` was not run locally (`SONAR_TOKEN` not configured in this worktree environment). No new SQL, no HTML injection surface, no zip extraction, no user-controlled URLs — the new code path is outside Sonar's high-risk triggers. Reviewers should watch for cognitive complexity on the component.  --- <a href="https://app.blacksmith.sh/2anki/codesmith/server/pr/2456"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> Need help on this PR? Tag <code>@codesmith</code> with what you need. - [ ] Let Codesmith autofix CI failures and bot reviews  --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

aalemayhu · 2026-05-19T14:41:35Z

Wave shipped. Five sub-PRs all merged to main:

fix: empty-deck and parser-error copy on the upload page #2453 fix: empty-deck and parser-error copy on the upload page
fix: clear invalid session cookie on login render #2454 fix: clear invalid session cookie on login render
feat: single CTA on anonymous limit screen — create account and start trial atomically #2455 feat: single CTA on anonymous limit screen — create account and start trial atomically
feat: four-step onboarding tour on first upload visit #2456 feat: four-step onboarding tour on first upload visit (carries the wave changelog entry)
fix: lower onboarding-tour cutoff to today's ship date #2457 fix: lower onboarding-tour cutoff to today's ship date (follow-up gate fix)

Closing the umbrella; spec text remains in this branch's history (git log -p -- Documentation/specs/first-deck-success.md).

aalemayhu mentioned this pull request May 19, 2026

fix: empty-deck and parser-error copy on the upload page #2453

Merged

1 task

This was referenced May 19, 2026

fix: clear invalid session cookie on login render #2454

Merged

feat: single CTA on anonymous limit screen — create account and start trial atomically #2455

Merged

feat: four-step onboarding tour on first upload visit #2456

Merged

aalemayhu closed this May 19, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

spec: First-deck-success activation wave#2446

spec: First-deck-success activation wave#2446
aalemayhu wants to merge 1 commit into
mainfrom
feat/spec-first-deck-success

aalemayhu commented May 19, 2026 •

edited by blacksmith-sh Bot

Loading

Uh oh!

netlify Bot commented May 19, 2026 •

edited

Loading

Uh oh!

sonarqubecloud Bot commented May 19, 2026

Uh oh!

aalemayhu commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

aalemayhu commented May 19, 2026 • edited by blacksmith-sh Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this spec covers

Trio synthesis

Status

Uh oh!

netlify Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for notion2anki ready!

Uh oh!

sonarqubecloud Bot commented May 19, 2026

Quality Gate passed

Uh oh!

aalemayhu commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

aalemayhu commented May 19, 2026 •

edited by blacksmith-sh Bot

Loading

netlify Bot commented May 19, 2026 •

edited

Loading