diff --git a/deploy/README.md b/deploy/README.md index 270d582..209d706 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -179,10 +179,29 @@ Nothing applies the DDL on boot any more, so it is applied from a terminal: pnpm --filter @workspace/scripts run seed:academy --schema-only ``` +That one command applies **two** schemas into the one database, because two +different things own tables in it: + +| File | Owns | +|---|---| +| `server/app/schema.sql` | What the API *writes*: `academy_users`, `learner_progress`, `course_purchases`, `login_events`, `ai_usage_events`, `api_rate_limits` | +| `scripts/src/academy-schema.sql` | What the academy *reads*: the question bank, coding challenges and lecture series | + +The first used to be applied by `initialize_database()` on every boot, which is +why nothing ever had to run it deliberately. Serverless ended that, and for a +while nothing ran it at all: the seed script knew only about the content schema, +so on Vercel `learner_progress` was a table the code wrote to and the database +had never heard of. Both are idempotent — re-running costs nothing. + Without it the tables do not exist, the database-backed routes answer `503`, and the academy falls back to its bundled content — which means a missing schema is invisible to a visitor and equally invisible to whoever deployed it. +`--check` counts what landed, and now includes `academy_users` and +`learner_progress`. A count of zero there is the useful signal: it proves the +table exists, which a 503 from `/api/progress` cannot distinguish from a table +that was never created. + ### Seeding the academy content The three collections — question bank, coding challenges, lecture series — live in the client's diff --git a/docs/superpowers/specs/2026-09-27-sheets-progress-store-design.md b/docs/superpowers/specs/2026-09-27-sheets-progress-store-design.md new file mode 100644 index 0000000..41f55b4 --- /dev/null +++ b/docs/superpowers/specs/2026-09-27-sheets-progress-store-design.md @@ -0,0 +1,183 @@ +# Learner progress in a Google Sheet, AI quota in Redis + +**Date:** 2026-09-27 +**Status:** approved design, not yet implemented + +## Why + +Each signed-in learner's progress — lectures viewed, coding challenges completed, +mock interview, resume review — is meant to follow them between devices. The code +for that has existed since `routes/progress.py` was written and has never once +run in production: `learner_progress` was only ever created by +`initialize_database()` at boot, boot-time DDL was removed when the API became a +Vercel function, and the Supabase project the table would have lived in no longer +answers. + +Rather than revive a database for one small table, the store becomes the +`LearnPracticeWorkData` spreadsheet. That removes the last reason to own a +Supabase project. + +Removing the database has a second consequence that this spec also fixes. In +production the AI quotas count in Postgres and are declared +`when_unavailable="refuse"`, because they guard a key that bills per call. With +`DATABASE_URL` set and the database unreachable, **every AI request is refused +with a 429 that reads to a visitor exactly like an exhausted quota** — verified +against production on 2026-09-27, first request of the day. Unsetting +`DATABASE_URL` does not help: `shared_quota_problem()` then returns a reason and +the same buckets still refuse. The quota needs a shared store that is not +Postgres. + +## Decisions + +| Decision | Chosen | Rejected, and why | +|---|---|---| +| Progress store | The spreadsheet, as system of record | Postgres mirror — keeps the Supabase dependency, which is the thing being removed | +| Sheet access | Apps Script web app called over HTTPS | Service account + Sheets REST: no locking, so two devices racing lose a write; also a private key in an env var and a JWT signer in a bundle already tuned against a 225 MB ceiling. Append-only event rows: unbounded growth and compaction, to avoid a mutex option A gives for free | +| Identity stored | Google `sub` only | Email — anyone the spreadsheet is shared with could read every learner's address, where Postgres had RLS and the anon key could not reach the table | +| AI quota store | Redis (Vercel Marketplace, slug `redis`) | Per-instance in-memory: turns 10/day into 10/day/instance against keys that bill per call | + +## Architecture + +`ProgressService` already takes its two operations as injected callables, so the +seam exists and nothing above it changes — not the client, not `routes/progress.py`, +not `ProgressBody`. + +``` +routes/progress.py + │ + ▼ +ProgressService(load, merge) unchanged + │ + ▼ +sheets_store.py new + │ httpx.post(SHEETS_WEBAPP_URL, token, {op, sub, progress}) + ▼ +Apps Script web app new, bound to LearnPracticeWorkData + │ LockService.getScriptLock() + ▼ +learner_progress tab +``` + +One interface narrows: `merge_progress(subject, email, incoming)` becomes +`merge_progress(subject, incoming)`, because email is no longer stored. That is +the `MergeProgress` type alias in `progress.py`, the call in +`ProgressService.merge_for_user`, and the injection in `dependencies.py`. + +### The sheet + +A new tab, `learner_progress`, one row per learner: + +| Column | Type | Notes | +|---|---|---| +| `google_sub` | text | Primary key. The only identifier stored | +| `resume_started` | boolean | | +| `resume_completed` | boolean | | +| `interview_started` | boolean | | +| `interview_answers` | number | | +| `interview_completed` | boolean | | +| `practice_completed` | JSON array as text | Capped at 500 ids | +| `lectures_viewed` | JSON array as text | Capped at 500 ids | +| `last_tool` | text | `resume` \| `interview` \| `practice` \| empty | +| `updated_at` | ISO 8601 text | | + +500 ids serialised as JSON is a few kilobytes, far under the 50,000-character +cell limit. + +### The web app + +`server/sheets/Progress.gs`, committed to the repository even though it is +deployed through the browser — otherwise the most important part of this feature +is reviewable by nobody. Deployed "execute as me / anyone with the link", so the +shared token is the only guard in front of the sheet: at least 32 random +characters, compared with a length-independent equality check rather than `===`, +and never logged. + +``` +POST { op: "load", sub } → { progress } +POST { op: "merge", sub, progress } → { progress } // the union +header: X-Academy-Token: +``` + +Both operations run inside `LockService.getScriptLock()` with a bounded wait, so +read-modify-write is serialised. The response body is the same camelCase shape +`validateProgress()` on the client already accepts: + +```json +{ "progress": { "resumeStarted": false, "resumeCompleted": false, + "interviewStarted": false, "interviewAnswers": 0, + "interviewCompleted": false, "practiceCompleted": [], + "lecturesViewed": [], "lastTool": null } } +``` + +A `sub` with no row loads as that object rather than an error, matching +`EMPTY_PROGRESS` today. + +### Merge semantics + +Identical to the SQL being replaced, because the client adopts whatever comes +back and a weaker rule silently loses work: + +| Field | Rule | +|---|---| +| the five booleans | `stored OR incoming` | +| `interviewAnswers` | `max(stored, incoming)` | +| `practiceCompleted`, `lecturesViewed` | union, then capped at 500 | +| `lastTool` | incoming when set, else stored | +| `updated_at` | now | + +Last-write-wins is the bug this prevents: finish three challenges on a laptop, +open a phone holding the older copy, and a plain write erases them. + +### AI quota on Redis + +`api_rate_limits` becomes `INCR` plus `EXPIRE` on the same HMAC digest key, so no +raw identity reaches Redis. `SharedRateLimiter` keeps its buckets and its +`when_unavailable` policies exactly as they are; only the backing store and the +one question `shared_quota_problem()` asks change — from "is `DATABASE_URL` set" +to "is `REDIS_URL` set". + +`hit_rate_limit(bucket, key_hash, limit, window_seconds)` and +`release_rate_limit(bucket, key_hash)` keep their signatures, so the +`SharedRateLimiter` class itself needs no change — only the two module functions +it calls, and the one question `shared_quota_problem()` asks in the same file. + +## Error handling + +`ProgressService._call` already distinguishes the two cases and the client +already treats both as "keep using localStorage". `sheets_store` preserves that +contract: + +| Condition | Result | +|---|---| +| `SHEETS_WEBAPP_URL` unset | `None` → 503, *"Progress is not available on this server."* | +| HTTP error, timeout, malformed body | raises → 500 | +| Unknown `sub` | the empty progress object, 200 | + +Apps Script cold starts are seconds, not milliseconds, so the client timeout +must tolerate that; a slow sync is invisible because the UI renders from +`localStorage` regardless. + +## Testing + +| Layer | Covers | +|---|---| +| `tests/unit/progressMerge.spec.ts` | The merge function, read out of `Progress.gs` and exercised in Node — the trick `hreflang.spec.ts` and `contentSchema.spec.ts` already use to hold a non-TypeScript artifact to its contract. Table-driven over the same cases the SQL merge had | +| `server/tests/test_sheets_store.py` | `load`/`merge` against a fake transport: the empty-row case, the 503 when unconfigured, the 500 on a bad response, and that email never appears in a request body | +| `server/tests/test_rate_limit_config.py` | Redis-backed counting, window expiry, and that each bucket's `when_unavailable` policy is unchanged | +| `server/tests/test_progress.py` | Already covers `ProgressService` against injected fakes; it should keep passing untouched, which is the check that the seam held | + +## Rollout + +No data to migrate: `learner_progress` never held a row. + +1. Create the `learner_progress` tab and deploy `Progress.gs` as a web app. +2. Set `SHEETS_WEBAPP_URL` and `SHEETS_WEBAPP_TOKEN` on the Vercel project. +3. Provision Redis through `vercel integration add redis`, which sets its own variables. +4. Swap the injection in `dependencies.py`; delete the Postgres progress functions and `learner_progress` from `server/app/schema.sql`. +5. Deploy, then verify a real round trip by signing in on two browsers and confirming the union. + +## Out of scope + +- **Content.** `/api/content/*` reads Supabase REST and will answer 503 without it, so the academy renders its bundled content modules — which is what it does today and what a visitor already sees. No regression, but the content API becomes decorative until separately decided. +- **Purchases, entitlements, login events.** Still Postgres-shaped. `SALES_ENABLED` is `false`, so nothing breaks; leaving that code alone keeps this change's blast radius to progress and quota. +- **Reporting on the sheet.** A learner-facing view, charts, or aggregation across rows. diff --git a/replit.md b/replit.md index f05f429..29ce995 100644 --- a/replit.md +++ b/replit.md @@ -122,9 +122,10 @@ value; there is nothing else to update. ### Purchases `checkout.session.completed` webhooks write a row into `course_purchases`, which is what ties a -Stripe payment to a person. FastAPI creates the table and indexes idempotently at startup when a -database is configured; `lib/db/src/schema/coursePurchases.ts` remains the TypeScript schema used -by repository tooling. +Stripe payment to a person. The table lives in `server/app/schema.sql` and is created by +`seed:academy --schema-only`, not at startup — a Vercel function boots per invocation, so +boot-time DDL was a schema round-trip in front of a visitor's request. `lib/db/src/schema/ +coursePurchases.ts` remains the TypeScript schema used by repository tooling. `GET /api/entitlements/course` reads it back for the caller's verified Google identity. Without a database the server degrades rather than guessing: webhooks cannot persist purchases diff --git a/scripts/src/seed-academy-content.ts b/scripts/src/seed-academy-content.ts index ce85201..bb76757 100644 --- a/scripts/src/seed-academy-content.ts +++ b/scripts/src/seed-academy-content.ts @@ -1,5 +1,10 @@ /** - * Applies the academy content schema and seed to Supabase. + * Applies both schemas and the content seed to Supabase. + * + * Two schemas, because two different things own tables in one database: + * `server/app/schema.sql` is everything the API writes (sign-ins, learner + * progress, purchases, quota rows) and `scripts/src/academy-schema.sql` is the + * content the academy reads. * * pnpm --filter @workspace/scripts run seed:academy * @@ -180,13 +185,20 @@ union all select 'coding challenges', l.lang, count(*) from coding_challenges c join coding_challenge_levels l on l.id = c.level_id group by l.lang union all select 'lecture series', t.lang, count(*) from lecture_items i join lecture_tracks t on t.id = i.track_id group by t.lang +-- Not content: these are the API's own tables. They are listed here because a +-- count of 0 proves the table exists, which is the one thing a --check run +-- could not tell you before. A missing table and an empty one look identical +-- from outside, and only one of them loses a signed-in reader's progress. +union all select 'signed-in readers', '-', count(*) from academy_users +union all select 'saved progress', '-', count(*) from learner_progress order by 1, 2;`; /** Ready lectures with no href render as live cards that open nothing. */ const DEAD_CARDS = `select count(*) from lecture_items where ready and coalesce(url, '') = '';`; +/** `file` is relative to the repository root, because the two schemas do not live together. */ function apply(where: Target, file: string): void { - const sql = readFileSync(path.join(root, 'scripts', 'src', file), 'utf8'); + const sql = readFileSync(path.join(root, file), 'utf8'); psql(where, ['-v', 'ON_ERROR_STOP=1', '-q'], sql); console.log(` applied ${file}`); } @@ -197,8 +209,17 @@ function main(): void { console.log(`Seeding academy content into ${where.describe}`); if (!flags.has('--check')) { - if (!flags.has('--seed-only')) apply(where, 'academy-schema.sql'); - if (!flags.has('--schema-only')) apply(where, 'academy-seed.sql'); + if (!flags.has('--seed-only')) { + // Two schemas, one database. `server/app/schema.sql` owns what the API + // writes — sign-ins, learner progress, purchases, quota rows — and used + // to be applied by `initialize_database()` on every boot. Serverless + // ended that: a Vercel function boots per invocation, so the DDL moved + // here and nothing else runs it. Without this line `learner_progress` + // never exists in production and every signed-in save fails. + apply(where, path.join('server', 'app', 'schema.sql')); + apply(where, path.join('scripts', 'src', 'academy-schema.sql')); + } + if (!flags.has('--schema-only')) apply(where, path.join('scripts', 'src', 'academy-seed.sql')); } console.log(psql(where, ['-A', '-F', ' | ', '-c', COUNTS]).trimEnd()); diff --git a/server/app/schema.sql b/server/app/schema.sql index 5d2a485..e7183d8 100644 --- a/server/app/schema.sql +++ b/server/app/schema.sql @@ -1,9 +1,15 @@ -- Everything this API owns in Postgres. -- --- `initialize_database()` runs this file at boot, every boot, so every --- statement is idempotent and the file is the whole story: there is no +-- Every statement is idempotent and the file is the whole story: there is no -- migration history to replay and no ordering to remember. -- +-- `initialize_database()` used to run it at boot, every boot. On Vercel a boot +-- is a single invocation, so that became a schema round-trip in front of a +-- visitor's request and a race between instances. It is applied deliberately +-- instead, by `pnpm --filter @workspace/scripts run seed:academy --schema-only` +-- — which is the only thing that runs it now. Nothing creates these tables on +-- their own any more. +-- -- It is applied to the same Supabase project that holds the content tables, -- and that is the reason for the security block at the bottom. `public` is the -- schema PostgREST exposes, so a table created here without row level security