Merge pull request #236 from amal66/olp-pr/e2e-llm-unskip

[Docs 13] docs(e2e): exact ANTHROPIC_API_KEY setup to enable the 4 LLM-gated specs

.github/workflows/e2e.yml

@@ -9,9 +9,13 @@
 #
 # The suite is green with NO secret: the 4 LLM-dependent specs (chat + critical
 # path) skip themselves when ANTHROPIC_API_KEY is absent (see e2e/llm.ts), so a
-# keyless run passes the other ~23 specs. Set the ANTHROPIC_API_KEY secret under
-# Settings > Secrets and variables > Actions to also run and enforce those 4.
-# See docs/e2e-ci.md for the full picture.
+# keyless run passes the other 23 specs. The gate exists because the app has no
+# keyless model — with no provider key configured, ChatInput blocks every send —
+# not because of the (best-effort) title-generation call. To run and enforce
+# those 4, set the ANTHROPIC_API_KEY repository secret; exact steps, the
+# fork-PR caveat (fork `pull_request` runs never receive secrets, so they
+# always take the skip path), cost, and how to verify the specs ran are in
+# docs/e2e-ci.md ("Enable the LLM specs").
 #
 # The e2e user (e2e@mike.local) is bootstrapped in-job by e2e/auth.setup.ts against
 # the local Supabase admin API, so no E2E_PASSWORD secret is needed — the defaults
@@ -40,8 +44,9 @@ jobs:
       PLAYWRIGHT_BASE_URL: http://localhost:3000
       # Exposed to the Playwright process (not just backend/.env) so the LLM-gated
       # specs can skip themselves when no key is present — see e2e/llm.ts. When the
-      # secret is set they run and must pass; when absent they skip, so a keyless
-      # run (e.g. a fork PR) is still green on the other ~20 specs.
+      # secret is set they run and are enforced; when absent they skip, so a keyless
+      # run (e.g. a fork PR, which never receives this secret) is still green on
+      # the other 23 specs. Setup: docs/e2e-ci.md, "Enable the LLM specs".
       ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
     steps:
       - uses: actions/checkout@v4

docs/e2e-ci.md

@@ -35,20 +35,94 @@ and on manual `workflow_dispatch`, the `e2e / playwright` job:
 the local Supabase admin API, so no login secret is needed — the credentials
 baked into that file are the single source of truth.
 
-Typical run: **~7 minutes**, **23 passed / 4 skipped / 0 failed** with no secret.
+A keyless run is expected to end **23 passed / 4 skipped / 0 failed** — the
+suite has 27 specs, 4 of them LLM-gated (see "Confirm the specs ran" below).
 
 ## Optional secret (fuller coverage)
 
 | Secret | What it unlocks | Without it |
 |---|---|---|
-| `ANTHROPIC_API_KEY` | The 4 LLM-dependent specs (chat rename/delete/submit, critical-path "ask a question") send a message and assert a **streamed** answer. With the key set they run and must pass. | Those 4 specs **skip** (see `e2e/llm.ts`) instead of hanging, so the run is still green on the other ~23 specs. |
+| `ANTHROPIC_API_KEY` | The 4 LLM-dependent specs (chat rename/delete/submit, critical-path "ask a question") send a message and assert a **streamed** answer. With the key set they run and are enforced. | Those 4 specs **skip** (see `e2e/llm.ts`) instead of hanging, so the run is still green on the other 23 specs. |
 
 The suite is green **without** any secret — the LLM specs skip themselves via
 `test.skip(!process.env.ANTHROPIC_API_KEY, …)`, which keeps keyless runs (local,
-and fork PRs with no secret access) green and fast. Add the key under
-**Settings → Secrets and variables → Actions → New repository secret** to also
-run and enforce the LLM specs. For fork PRs, GitHub withholds secrets until a
-maintainer approves the run.
+and fork PRs with no secret access) green and fast. The gate is a blanket one on
+purpose: the app ships **no keyless model** — every model in the picker
+(`frontend/src/app/components/assistant/ModelToggle.tsx`) needs an
+Anthropic/Google/OpenAI key before `ChatInput` will send at all — so without a
+key the 4 specs cannot even submit a message. (The auto title-generation call is
+*not* the reason for the gate; keyless it returns a 500 the specs already
+tolerate.) Setup steps below.
+
+## Enable the LLM specs
+
+### 1. Add the repository secret
+
+UI path:
+
+1. Open the repository on GitHub → **Settings**.
+2. In the left sidebar: **Secrets and variables → Actions**.
+3. On the **Secrets** tab, click **New repository secret**.
+4. **Name:** `ANTHROPIC_API_KEY` — exactly this name; both the workflow env and
+   `e2e/llm.ts` read it. **Secret:** an Anthropic API key (`sk-ant-…`) from
+   <https://console.anthropic.com/settings/keys>.
+5. Click **Add secret**.
+
+CLI equivalent (repo admin):
+
+```bash
+gh secret set ANTHROPIC_API_KEY --repo Open-Legal-Products/mike
+# paste the key at the prompt (or pipe it: --body "$ANTHROPIC_API_KEY")
+```
+
+### 2. The fork-PR caveat
+
+On `pull_request` events from **forks**, GitHub withholds repository secrets, so
+fork PRs — most external contributions — still run keyless and skip the 4 specs.
+That is by design and keeps those runs green. Runs that actually receive the
+secret and exercise the specs are:
+
+- PRs from branches pushed to this repository (maintainer branches), and
+- manual runs: **Actions → e2e → Run workflow** (`workflow_dispatch`) on any
+  branch.
+
+So after adding the secret, the quickest way to see the specs run is a
+`workflow_dispatch` run from the Actions tab.
+
+### 3. Expected cost per run
+
+A handful of short completions: one streamed chat answer per LLM spec plus a few
+small title generations (`claude-haiku-4-5`, 64-token cap). On the order of a
+few cents per run — negligible next to the CI minutes.
+
+### 4. Confirm the specs ran (not skipped)
+
+Open the **Run Playwright** step in the Actions log:
+
+- **Keyless run:** the summary ends with `4 skipped` / `23 passed`, and each
+  skipped spec carries the reason
+  `requires a model key — set the ANTHROPIC_API_KEY secret to run LLM-dependent specs`.
+- **With the secret:** the summary shows `27 passed` and **no `skipped` line**;
+  searching the log for `requires a model key` finds nothing.
+
+The uploaded `playwright-report` artifact shows the same per-spec statuses.
+
+### Model selection (gap fixed in this branch)
+
+Earlier revisions of the four specs drove the model picker to a keyless
+**"Demo (no key needed)"** entry that exists in the amal66 fork but **not in
+this repository** (no `mike-demo` model id, no demo provider), so setting the
+secret unskipped them and they then failed at model selection. This branch
+carries the fix (commit `test(e2e): select a real Claude model in LLM-gated
+specs`): the specs' `selectClaudeModel` helper picks **Claude Sonnet 4.6** in
+the ModelToggle whenever the key is set, and the critical-path response
+assertion checks for a nonempty streamed assistant answer instead of the
+fork's canned demo reply. The **23 passed / 4 skipped** keyless and
+**27 passed / 0 skipped** with-key figures come from that commit's own
+verification against a full local stack (backend, prod-build frontend, local
+Supabase + MinIO — see its commit message); they have not been re-measured
+since this branch was rebased onto current `main`. The secret setup above is
+all that is needed.
 
 ## Make it merge-blocking
 

e2e/llm.ts

@@ -1,16 +1,34 @@
 /**
  * Helpers for specs that require a live LLM turn.
  *
- * A few specs create/populate a chat by sending a message and awaiting a
- * streamed answer. That only works when a model key is configured — in CI the
- * `ANTHROPIC_API_KEY` secret, which `.github/workflows/e2e.yml` exposes to the
- * Playwright process. When it is absent (a plain local run, or a fork PR with no
- * secret access) the app blocks message send behind the ApiKeyMissingModal, so
- * these specs would hang to their timeout.
+ * Four specs (chat rename, chat delete, project-assistant create+submit, and
+ * the critical-path "ask a question" flow) create/populate a chat by sending a
+ * message. This codebase ships no keyless model: every entry in the model
+ * picker (frontend/src/app/components/assistant/ModelToggle.tsx MODELS) is an
+ * Anthropic/Google/OpenAI model whose availability requires a configured
+ * provider key — backend env var or user-stored key
+ * (backend/src/lib/userApiKeys.ts) — and ChatInput.handleSubmit refuses to
+ * send when the selected model's key is missing (ApiKeyMissingPopup). The
+ * backend enforces the same model set server-side
+ * (backend/src/lib/llm/models.ts ALL_MODELS/providerForModel). So with no key
+ * there is no way for these specs to send a message at all; unguarded they
+ * would hang to their timeout.
  *
- * Guarding them with `test.skip(!hasLlmKey, LLM_SKIP_REASON)` keeps a keyless
- * run green and fast on the ~20 specs that don't need a model, while still
- * running — and enforcing — the LLM specs whenever the key is present.
+ * The auto title-generation call (POST /chat/:id/generate-title) is NOT why
+ * the gate exists: keyless it just returns 500, and the specs already treat it
+ * as best-effort (`.catch(() => null)`).
+ *
+ * In CI the key is the `ANTHROPIC_API_KEY` repository secret, which
+ * `.github/workflows/e2e.yml` exposes both to the backend (backend/.env) and
+ * to the Playwright process. Guarding with
+ * `test.skip(!hasLlmKey, LLM_SKIP_REASON)` keeps a keyless run (a plain local
+ * run, or a fork PR with no secret access) green and fast on the other 23
+ * specs, while still running — and enforcing — the LLM specs whenever the key
+ * is present. Setup steps: docs/e2e-ci.md, "Enable the LLM specs".
+ *
+ * When the key IS set, the specs' selectClaudeModel helper picks "Claude
+ * Sonnet 4.6" in the ModelToggle (see docs/e2e-ci.md, "Model selection"), so
+ * the unskipped specs submit against a model this repository actually ships.
  */
 export const hasLlmKey = Boolean(process.env.ANTHROPIC_API_KEY);