diff --git a/.cursor/commands/preflight-learning-path.md b/.cursor/commands/preflight-learning-path.md new file mode 100644 index 000000000..ad34a3aa7 --- /dev/null +++ b/.cursor/commands/preflight-learning-path.md @@ -0,0 +1,36 @@ +## Command: /preflight-learning-path + +Self-review a learning path package in `interactive-tutorials` **before** opening a PR. Mirrors [/review-learning-path-pr](review-learning-path-pr.md) (shared static checklists, shared claim-check, Playwright DOM) from the author side, with fewer stop-and-type moments: the agent runs what it can, then pauses only when it needs you (login, smoke choice, results / fixes). + +When invoked, read and follow **[preflight-learning-path/SKILL.md](../skills/preflight-learning-path/SKILL.md)** in order. + +LP packages are **single-repo** (`interactive-tutorials` only). Metadata lives in package `website.yaml`; the website repo is read-only for conversion ([PR #416](https://github.com/grafana/interactive-tutorials/pull/416)). Learning Hub criteria: [learning-hub-standards.md](../skills/review-learning-path/learning-hub-standards.md). + +### Setup you need + +- This repo checked out with your `{slug}-lj/` package +- **Playwright MCP** enabled in Cursor (for DOM checks) +- Access to `https://learn.grafana.net/` (Okta login in the Playwright browser when asked) +- Block Editor only if you choose a guided smoke walk later + +--- + +## First message to the author + +> I'll preflight your learning path with as few interruptions as possible. I'll run the static checks on my own, then pause when I need you logged into Playwright, again for an optional Block Editor smoke choice, then once for results and fix options. +> +> | When I pause | Why | +> |---|---| +> | Path unclear | Confirm which `{slug}-lj/` package | +> | Playwright not ready | Fix or add MCP | +> | Before live DOM | Okta login + which stack | +> | After DOM | Smoke: already-tested / walk-me / skip | +> | Results | What to fix (if anything) | +> +> **Setup:** Playwright MCP should be on. If it is missing or broken, I'll help fix it. You'll Okta-login in the Playwright browser for DOM checks. +> +> Preflight reports stay under `.cursor/lp-preflight-state/` (gitignored). Don't force-add them to your PR. +> +> **To start:** Share the path package (`{slug}-lj/` directory), or tell me to infer it from your current branch. If you already named the path, I'll begin the static checks now. + +If the path is already clear from the user message, skip asking and start identify + static immediately. diff --git a/.cursor/learning-path-workflows/workflows.md b/.cursor/learning-path-workflows/workflows.md index 4b542aa25..24be14b1a 100644 --- a/.cursor/learning-path-workflows/workflows.md +++ b/.cursor/learning-path-workflows/workflows.md @@ -103,3 +103,4 @@ The learning path slug is mysql-data-source. | `/build-interactive-lj` | Existing milestone markdown in website repo (read-only source) | `content.json` + `manifest.json` + `website.yaml` files in interactive-tutorials | | `/create-learning-path` | Feature description (no existing markdown) | `content.json` + `manifest.json` + `website.yaml` files | | `/review-learning-path-pr` | Existing LP PR in `interactive-tutorials` (share PR URL/number) | Human-guided review: static workbook, Block Editor smoke test, chat-approved inline comments, submit | +| `/preflight-learning-path` | Local `{slug}-lj/` package before opening a PR | Author self-review with few pauses (login, smoke choice, results): mirrors review static + Playwright checks and shared claim-check, readiness report, optional fixes / frontend testid PR | diff --git a/.cursor/skills/preflight-learning-path/SKILL.md b/.cursor/skills/preflight-learning-path/SKILL.md new file mode 100644 index 000000000..f3c3fa539 --- /dev/null +++ b/.cursor/skills/preflight-learning-path/SKILL.md @@ -0,0 +1,457 @@ +--- +name: preflight-learning-path +description: >- + Guide a learning path author through pre-PR self-review in interactive-tutorials. + Mirrors /review-learning-path-pr (shared static checklists, shared claim-check, + Playwright DOM, readiness) from the author side, then optional package fixes and + optional frontend data-testid PR. Use when the user runs /preflight-learning-path. +--- + +# Preflight learning path (author self-review) + +Walk through a `{slug}-lj/` package **before you open a PR**. Mirrors [review-learning-path](../review-learning-path/SKILL.md): shared static checklists, shared [claim-check](../review-learning-path/claim-check.md), Playwright DOM, then readiness and optional fixes. Author flow batches agent work and pauses only at real human gates (not a five-stop quiz). + +**Terminology:** Say **learning path** or **path** in author messages; use `{path_dir}` in agent notes. + +**Entry command:** [/preflight-learning-path](../../commands/preflight-learning-path.md) + +**Mirror skill:** [review-learning-path](../review-learning-path/SKILL.md). Shared claim policy lives under the review skill so both callers cannot drift. + +**Do NOT read external reference files upfront.** Each step loads its own references on demand. + +**Skill memory:** State lives in `.cursor/lp-preflight-state/` (gitignored; never commit). Static pass dispatches [audit-guide](../audit-guide/SKILL.md), which writes `{milestone}/assets/`. See [Commit safety](#commit-safety). + +**Routing:** [reference-checks.md](reference-checks.md) · [claim-check.md](claim-check.md) (pointer) · [author-testing.md](author-testing.md) · [frontend-selector-pr.md](frontend-selector-pr.md) · shared [claim-check](../review-learning-path/claim-check.md) · shared [learning-hub-standards.md](../review-learning-path/learning-hub-standards.md) · shared [review reference-checks](../review-learning-path/reference-checks.md) + +**Related:** [audit-guide](../audit-guide/SKILL.md) · [update-guide](../update-guide/SKILL.md) · [review-learning-path](../review-learning-path/SKILL.md) + +--- + +## Voice (author-facing) + +Write like a calm teammate helping someone ship, not like a linter report. + +| Do | Don't | +|---|---| +| Short sentences; plain words | Jargon in chat (`post-inline`, `MUST FIX`, "refresh readiness", rule numbers) | +| Say what is wrong, why it matters, which file | Dump severity labels or audit counts | +| Offer only actions that can resolve the item | Offer **fix N** when the real fix is upstream | +| Celebrate clean passes briefly | Hedge with "seems fine" / "maybe" | +| Pause only when you truly need the author | Quiz them with "reply yes" between agent-only steps | + +No em dashes in author chat, readiness text, or drafted PR bodies. Use periods, commas, or parentheses. + +Address the author as **you**. Prefer "copy fixes," "product claims vs docs," "UI selectors," "smoke test," "ready / not-ready summary." + +--- + +## Author experience (hard flow) + +Do as much as you can without stopping. Authors should not confirm every internal phase. + +**Pause only for:** + +1. Path unclear (ambiguous / missing `{path_dir}`) +2. Playwright MCP blocked or broken (setup) +3. Okta / browser ready before DOM checks (and stack choice, unless already known) +4. Block Editor smoke choice (and walk-me replies if they choose guided smoke) +5. Results + fix menu (and later fix / frontend / done) + +**Do not** pause for "Phase 0 complete, reply yes" or "Phase 1 complete, reply yes" when the path is known and MCP is healthy. Run identify → static → then ask for login in one beat. + +Internal **checkpoint** names (see [state schema](reference-checks.md#state-file-schema)) stay in state and agent notes. Author chat can say "checking your path," "live checks," "results" instead of a quiz board. + +--- + +## Human-in-the-loop contract + +| You (agent) | Author (human) | +|---|---| +| Infer path when clear; confirm only if ambiguous | Share `{path_dir}` when asked | +| Run static + claim-check without a mid-stop | Wait or do other work | +| Open Playwright; ask when logged in | Okta in Playwright browser; reply when ready | +| Run DOM checks; then ask about smoke | `already-tested` / `walk-me` / `skip-smoke` | +| One results + fix menu | `fix all` / `fix N` / `frontend` / `done` / `show report` | +| Apply package or frontend fixes only when asked | Approve edits / push | + +--- + +## Workflow overview + +``` +Input (path_dir or current branch) + │ + ├─ Identify + MCP check ─────── quiet unless path/MCP needs the author + │ + ├─ Static pass ─────────────── audit + path/LH + claim-check (no author yes) + │ + ├─ PAUSE: login + stack ────── Playwright Okta; stack (or static-only if allowed) + │ + ├─ Playwright DOM ──────────── required (unless static-only) + │ + ├─ PAUSE: smoke choice ─────── already-tested / walk-me / skip-smoke + │ + ├─ Results + fix menu ──────── one readiness summary (former Phase 3) + │ + ├─ Optional package fixes ──── fix all / fix N + │ + └─ Optional frontend testid ── when needs-frontend +``` + +--- + +## Inputs + +- **Required**: `{path_dir}` ending in `-lj/` (for example `monitor-azure-resources-lj`). Infer from current branch if omitted; confirm if ambiguous. +- **Optional**: `website_slug` (`{path_dir}` minus `-lj`). Website repo is read-only for conversion ([PR #416](https://github.com/grafana/interactive-tutorials/pull/416)). +- **Optional**: `learn_host` (default `learn.grafana.net`). +- **Optional**: `waive_live_testing` via `static-only: ` at the login/stack pause. See [static-only preflight](reference-checks.md#static-only-preflight). + +--- + +## Prerequisites + +See [author-testing.md § Prerequisites](author-testing.md#prerequisites). Identify verifies Playwright MCP before live checks. + +--- + +## Safety invariants + +1. **Do not modify** `content.json`, `manifest.json`, or `website.yaml` until the author requests package-fixable fixes. +2. **Never commit** preflight artifacts (`.cursor/lp-preflight-state/` or audit-guide files under `{milestone}/assets/`). Remind the author once that reports are local and gitignored. +3. **Surface only review-level** findings in chat and readiness (same bar as review [comment-style](../review-learning-path/comment-style.md)). Never dump Internal/Discard nits. +4. **No em dashes** in author-facing chat, readiness text, or drafted PR bodies. +5. **One preflight run per path slug** (resume from state; do not fork duplicate state files). +6. **Never** plan companion website / `pathfinder_data` / shortcode work as package blockers. + +--- + +## Commit safety + +Same snapshot/cleanup rules as [review-learning-path § Commit safety](../review-learning-path/SKILL.md#commit-safety). + +| Artifact | Where | Mitigation | +|---|---|---| +| Findings, claim-check, readiness, state JSON | `.cursor/lp-preflight-state/` | `.gitignore` (must ship with this skill) | +| audit-guide reports | `{milestone}/assets/` | Snapshot before static; delete new files before login pause | +| Pre-existing package `assets/` | Author/migrate notes | Never delete paths listed in `pre_review_assets` | + +**Static snapshot:** `find {milestone}/assets -type f 2>/dev/null | sort` → store in state `pre_review_assets`. + +**Static cleanup (mandatory before login pause):** remove audit files not in the snapshot; verify `git status --porcelain -- {path_dir}` shows no untracked audit paths. + +--- + +## How this skill runs + +1. Tell the author briefly what you are starting (path name when known). Do not lecture. +2. Run every agent-only step you can before the next real pause. +3. At a pause: one message, one clear ask. Plain language. **Do not advance** until they reply. +4. Do **not** invent extra "reply yes if this looks right" stops between agent-only work. +5. If they say "keep going" during a real pause (login / smoke / results), still honor that pause's need (e.g. you cannot DOM-check without login). + +--- + +## Results menu format (after live) + +| Section | Content | +|---|---| +| **Header** | Friendly outcome (e.g. almost ready, with N copy fixes first) | +| **What we checked** | Plain bullets: claims vs docs, UI selectors, smoke choice | +| **Findings** | Numbered: what is wrong, why it matters, which file | +| **Your turn** | Only actions that can resolve open items | +| **Heads-up** | Optional stack note; gitignore reminder if useful | + +Do **not** open with a "What this check is" primer. + +**Useful replies:** `ready` · `add playwright mcp` · `fix playwright mcp` · `static-only: ` · `already-tested: ` · `walk-me` · `skip-smoke` · `pass` / `fail step N - …` / `N/A - …` · `show report` · `fix all` · `fix 1` / `fix 2` / `fix 3` · `fix 1,3` · `frontend` · `done` · `resume` / `start fresh` + +After package fixes, ask: **Want an updated ready / not-ready summary?** (not "refresh readiness"). + +--- + +## Resume + +If `.cursor/lp-preflight-state/{slug}.json` exists: + +> **Resume?** I have an in-progress preflight for `{path_dir}` (stopped at **`{checkpoint}`**). +> +> Reply **resume** to pick up, or **start fresh** to begin again. + +On resume, read `checkpoint` + `status` from state and continue from that gate. Do not invent a numeric phase. + +--- + +## Identify path + MCP (quiet unless blocked) + +**Goal:** Confirm `{path_dir}`, `path_type`, milestones; verify Playwright MCP; init state. + +**Persist:** After init (or when blocked on MCP), set `checkpoint` to `identify` (still blocked) or `login` (ready for login pause). Set `status: in_progress`. + +### Author chat + +If `{path_dir}` is missing or ambiguous: + +> Which learning path package should I preflight? Share the `{slug}-lj/` directory, or tell me to infer it from your current branch. + +If path is clear, say one short line (e.g. "Preflighting `{path_dir}`. Running the static checks now.") and continue. Do **not** ask them to confirm the path/type/MCP table when everything looks fine. + +Mention once (early, not every pause): reports land under `.cursor/lp-preflight-state/` and are gitignored; do not force-add them to a PR. + +### Agent steps + +1. Infer or confirm `{path_dir}` (directory ending in `-lj`). +2. Record branch and HEAD sha. +3. Infer `website_slug` = `{path_dir}` minus `-lj` when website repo is in workspace (read-only). +4. Infer `path_type`: `new`, `conversion`, or `update` per [path type](reference-checks.md#path-type). +5. List milestones from path `manifest.json` `milestones` and dirs under `{path_dir}/`. +6. **Verify Playwright MCP** (`user-playwright`). Cover missing, needs-auth, toggled-off, **and configured-but-broken** per [If Playwright MCP is missing or broken](author-testing.md#if-playwright-mcp-is-missing-or-broken). Do not silently skip. Do not edit `mcp.json` until they agree. +7. Write `.cursor/lp-preflight-state/{slug}.json` ([state schema](reference-checks.md#state-file-schema)). + +If MCP is blocked, stop with the blocked shape in author-testing.md (`checkpoint: identify`). Do not run live later until tools work. + +--- + +## Static pass (no author yes) + +**Goal:** Audit every milestone + path consistency + Learning Hub + claim-check. Keep findings for the final results menu. + +**Persist:** Keep `checkpoint: login` while static runs (next human gate). Do not invent a separate author-facing "static done" pause. + +### Agent steps + +1. Snapshot `pre_review_assets`; dispatch [audit-guide](../audit-guide/SKILL.md) per milestone (parallel OK). +2. Walk shared [review reference-checks](../review-learning-path/reference-checks.md) + [learning-hub-standards.md](../review-learning-path/learning-hub-standards.md). +3. **Always scan** for framing-in-milestones, [section intro markdown that may number as a step](../review-learning-path/reference-checks.md#section-intro-markdown-numbered-as-a-step), and [false noops](../review-learning-path/reference-checks.md#noop-and-non-interactive-steps). +4. Run Pathfinder CLI `validate --packages {path_dir}` if available. +5. Run the shared [claim-check](../review-learning-path/claim-check.md) pass (preflight pointer: [claim-check.md](claim-check.md)). Write `{slug}-claim-check.md`. Route Contradicted / Unsupported / Overstated as Fix before PR. Do not edit package JSON here. +6. Tag findings with review [finding routing](../review-learning-path/reference-checks.md#finding-routing). Keep only review-level items for later author chat. +7. Write `{slug}-findings.md` (findings + verify-live notes). +8. Mandatory audit cleanup; verify `git status`. +9. Do not cite rule numbers or audit severity labels in chat. + +Then go straight to the login pause (unless `static-only` was already set). + +--- + +## PAUSE: Login + stack + +**Goal:** Author is logged into Playwright on `{learn_host}`; record stack (or allow static-only when rules permit). + +**Persist:** Before asking, `checkpoint: login`. After `ready`, set `checkpoint: dom`. After allowed `static-only`, set `checkpoint: results` and `waive_live_testing`. + +> **Live checks next** +> +> I need you logged into `{learn_host}` in the **Playwright** browser (Okta there). I can open it for you if it is not open yet. +> +> Also tell me which stack you are using (for example `learn.grafana.net shared`, `fresh Cloud stack`). +> +> Reply **ready** when the Playwright browser is logged in (include the stack in the same reply if you have not already). +> +> Or **`static-only: `** to skip live testing (not for **new** / **conversion** interactive paths). + +**Wait for:** `ready` (+ stack), or allowed `static-only: `. + +Record `stack_state`, or `waive_live_testing` + `static_only_reason`. + +**Reject** bare `static-only` and static-only on **new** / **conversion** with interactive milestones. Precedence: [static-only preflight](reference-checks.md#static-only-preflight). + +If static-only: skip Playwright + smoke; jump to results menu with **Not live-tested** notes. + +--- + +## Playwright DOM (no author yes between milestones) + +Details: [author-testing.md](author-testing.md). + +**Persist:** `checkpoint: dom` while sweeping; set `checkpoint: smoke` when DOM is done (before the smoke ask). + +| `path_type` | Scope | +|---|---| +| **new** / **conversion** | Every interactive milestone in path `milestones` | +| **update** | Touched interactive milestones first; full path on request | + +For each milestone in scope (path order; skip prose-only / terminal): + +1. Derive start URL ([milestone start URL](../review-learning-path/reference-checks.md#milestone-start-url)). +2. Navigate; check each `reftarget`: exists / missing / below-fold / state-dependent. +3. Record in `playwright.{milestone-slug}`. + +Keep chat quiet during the sweep (or a short progress line). Prefer documenting stack gaps over false "missing" on the wrong stack. + +Then go to the smoke pause. + +--- + +## PAUSE: Block Editor smoke + +**Persist:** Before asking, `checkpoint: smoke`. After the author replies, set `checkpoint: results` (or stay on `smoke` during walk-me until scoped milestones finish). + +> Have you already smoke-tested this path in Block Editor (Show me / Do it)? +> +> - **`already-tested: `** if yes (stack + anything flaky) +> - **`walk-me`** for a guided per-milestone check now (local JSON import) +> - **`skip-smoke`** to continue without recording Block Editor evidence + +| Reply | Behavior | Readiness | +|---|---|---| +| `already-tested: …` | Store dogfood evidence | Can still be **Ready for PR** if Playwright clean and no open blockers | +| `walk-me` | Guided loop per [author-testing.md](author-testing.md) | Same when scoped milestones pass or documented N/A | +| `skip-smoke` | Continue | Cap at **Open PR with notes** | + +After smoke (or walk-me finishes), build readiness and show the **results menu** once. + +--- + +## Results + fix menu + +**Goal:** One readiness outcome; right next actions. Author chat shape: [Author-facing findings](reference-checks.md#author-facing-findings). + +**Persist:** `checkpoint: results` when showing the menu. On `fix all` / `fix N` → `checkpoint: package_fixes`. On `frontend` → `checkpoint: frontend`. On `done` → `status: complete`, keep `checkpoint: results` (or `frontend` if that was last). + +### Agent steps + +1. Apply [selector decision tree](../review-learning-path/reference-checks.md#selector-decision-tree); promote only review-level items. +2. Apply [readiness gate](reference-checks.md#readiness-gate). +3. Write `{slug}-readiness.md` with outcome + [PR opener checklist](reference-checks.md#pr-opener-checklist). +4. Show the results menu below. Mark each finding package-fixable or needs-frontend. +5. Map replies: `fix all` / `fix N` → package fixes; `frontend` → frontend walkthrough; `show report` → readiness path; `done` → wrap with PR-opener notes. + +### When there are findings + +> **{Friendly outcome}** +> +> **What we checked** +> - {Product claims vs live docs} +> - {UI selectors on {stack}} +> - {Block Editor smoke: skipped / already-tested / walked} +> +> **Please fix these {N}** ({short kind}) +> +> 1. {Plain problem}. {Why / better wording}. +> (`{file or dirs}`) +> 2. … +> +> **Your turn** *(include only lines that apply)* +> - **fix all** / **fix N** / **fix 1,3** — package edits we can make here +> - **frontend** — upstream `data-testid` for item(s) {N} +> - **done** — open a PR and leave these for review +> - **show report** — longer write-up +> +> **Heads-up** *(optional)* +> {Stack note. Remind once if needed: preflight reports are gitignored under `.cursor/lp-preflight-state/`.} + +When every open item needs-frontend, omit **fix all** / **fix N**. Lead with **frontend**. + +### When clean + +> **{Ready for PR | Open PR with notes}** +> +> **What we checked** +> - … +> +> Nothing here that would draw a review comment on copy or selectors. Nice work. +> +> **Your turn:** Reply **done** if you are opening the PR, or **show report** for the write-up. +> +> *(If Open PR with notes: say why in one line, e.g. Block Editor smoke was skipped.)* + +--- + +## Package fixes (optional) + +**Goal:** Surgical edits for **package-fixable** findings only (`fix all` / `fix N`). + +**Persist:** `checkpoint: package_fixes` while editing; return to `checkpoint: results` after offering an updated summary. + +> Working on {fix all | item N | items …}. I will edit `content.json` / `manifest.json` / `website.yaml` only, same discipline as [update-guide](../update-guide/SKILL.md). + +1. Apply only requested package-fixable findings. +2. Re-run Pathfinder CLI validate if content/manifests changed. +3. Suggest re-running Playwright for touched interactive milestones when relevant. +4. Do not commit unless the author explicitly asks. + +> Applied {N} fix(es). +> +> **Your turn:** Want an **updated ready / not-ready summary**? Or reply **frontend** / **done**. + +--- + +## Frontend selector PR (optional) + +When a stable selector is missing upstream, follow [frontend-selector-pr.md](frontend-selector-pr.md). Canonical example: [grafana/grafana-cmab-app#1795](https://github.com/grafana/grafana-cmab-app/pull/1795). + +**Persist:** `checkpoint: frontend` for the whole walkthrough. + +> Frontend PR {url} *(or deferred)*. +> +> **Your turn:** Reply **done** when you are ready to open the learning path PR (or wait on the testid merge). + +--- + +## Anti-patterns + +**Do not** + +- Quiz the author between agent-only steps ("Phase N complete, reply yes") +- Edit package JSON without an explicit ask +- Leave audit artifacts in milestone `assets/` +- Commit `.cursor/lp-preflight-state/` +- Surface Internal/Discard nits in chat or readiness +- Use em dashes, rule numbers, Blocker labels, or "refresh readiness" +- Force a full Block Editor loop when they already dogfooded +- Offer **fix N** as the main path for a needs-frontend finding +- Recommend **Ready for PR** with open review-level items +- Recommend **Ready for PR** on **new** / **conversion** interactive paths when Playwright was skipped +- Request website-repo changes as package blockers +- Use the PR review tool instead of local import (no PR yet) +- Leave `checkpoint` stale across gates (resume depends on it) + +**Do** + +- Batch agent work; pause only for real human gates +- Persist named `checkpoint` after each gate +- Keep chat short, clear, and friendly +- Dedupe findings by root cause +- Run CLI validate when available +- Offer package vs frontend actions correctly after the results menu +- Run the shared claim-check (same policy as review) + +--- + +## Generated files + +Write under `.cursor/lp-preflight-state/` (never commit; gitignored): + +| File | Purpose | +|---|---| +| `{slug}.json` | Machine state ([schema](reference-checks.md#state-file-schema)) | +| `{slug}-findings.md` | Findings + verify-live notes | +| `{slug}-claim-check.md` | Claim-check report | +| `{slug}-readiness.md` | Ready / not-ready summary + PR opener checklist | +| `{slug}/audits/{milestone}/` | Optional copied audit reports before cleanup | + +```markdown +--- +disclaimer: Auto-generated by preflight-learning-path skill. Do not edit manually. +notice: To regenerate, re-run the skill from the relevant pause. +path_dir: {path_dir} +--- +``` + +--- + +## Deep references + +| Topic | Doc | +|---|---| +| Author routing + readiness | [reference-checks.md](reference-checks.md) | +| Claim-check (product facts) | [../review-learning-path/claim-check.md](../review-learning-path/claim-check.md) (pointer: [claim-check.md](claim-check.md)) | +| Prereqs + Playwright + optional smoke | [author-testing.md](author-testing.md) | +| Frontend testid PR | [frontend-selector-pr.md](frontend-selector-pr.md) | +| Shared static checklists + finding routing | [../review-learning-path/reference-checks.md](../review-learning-path/reference-checks.md) | +| Learning Hub structure | [../review-learning-path/learning-hub-standards.md](../review-learning-path/learning-hub-standards.md) | +| Comment voice (review-level bar) | [../review-learning-path/comment-style.md](../review-learning-path/comment-style.md) | +| Reviewer workflow (mirror) | [../review-learning-path/SKILL.md](../review-learning-path/SKILL.md) | +| `website.yaml` | [docs/website-yaml-reference.md](../../../docs/website-yaml-reference.md) | +| Single-repo LP workflows | [learning-path-workflows/workflows.md](../../learning-path-workflows/workflows.md) | diff --git a/.cursor/skills/preflight-learning-path/author-testing.md b/.cursor/skills/preflight-learning-path/author-testing.md new file mode 100644 index 000000000..29306658f --- /dev/null +++ b/.cursor/skills/preflight-learning-path/author-testing.md @@ -0,0 +1,144 @@ +# Author testing (preflight) + +Live-test helpers for [preflight-learning-path/SKILL.md](SKILL.md). + +Aligns with the five-phase [review-learning-path](../review-learning-path/SKILL.md) live path, lighter for authors: Playwright DOM is required; Block Editor is opt-in. Fits the hard author flow: pause for login, then smoke choice; do not quiz between DOM milestones. + +--- + +## Prerequisites + +Confirm during identify (quietly unless blocked). Remind in the slash-command first message. + +| Need | Required when | Why | How to verify / fix | +|---|---|---|---| +| `interactive-tutorials` checkout with `{slug}-lj/` | Always | Package under test | Path on disk / current branch | +| **Playwright MCP** enabled (`user-playwright`) | Live path (default for new/conversion interactive) | Agent DOM checks | Identify: list or call Playwright MCP tools. If missing or broken: follow [If Playwright MCP is missing or broken](#if-playwright-mcp-is-missing-or-broken) | +| Okta login in the **Playwright** browser to `{learn_host}` (default `learn.grafana.net`) | Before Playwright DOM loop | MCP cannot complete SAML alone | Login pause: author logs in, replies `ready` | +| Pathfinder Block Editor on learn | Only if author chooses `walk-me` | Optional smoke coaching | `{learn_host}/plugins/grafana-pathfinder-app?dev=true` → **?** → Debug → Block Editor → import local JSON | +| Pathfinder CLI (`grafana-pathfinder-app` built locally) | Nice-to-have | `validate --packages` | Note if missing; do not abort preflight | +| `gh` auth + write access to the UI repo | Frontend walkthrough only | Frontend testid PR | Check when entering frontend steps | +| `website` repo in workspace | Optional (conversion) | Read-only legacy compare | Never write | + +Same Playwright + Okta role as [learning-path-workflows/workflows.md](../../learning-path-workflows/workflows.md) and `/create-learning-path`. + +### If Playwright MCP is missing or broken + +Stop before live DOM checks. Do **not** silently skip DOM checks on new/conversion interactive paths. `static-only: ` only when [static-only rules](reference-checks.md#static-only-preflight) allow it. + +Diagnose which case applies, then offer help (manual steps always; agent action when possible): + +| Case | How you can tell | Offer | +|---|---|---| +| **Config missing** from `~/.cursor/mcp.json` (no `playwright` / `@playwright/mcp` entry) | Read `mcp.json`; server absent | Ask: **Want me to add the Playwright MCP config for you?** If yes, merge the standard block below into `mcp.json`. Then ask them to reload MCP and reply when ready so you recheck. | +| **Needs auth** (`needsAuth` / only `mcp_auth` tool) | `GetMcpTools` / server status | Ask: **Want me to start the Playwright MCP connect flow?** If yes, call `mcp_auth` for `user-playwright` (empty args), wait, recheck. | +| **Configured but toggled off** in Cursor Settings | Entry exists in `mcp.json`; tools still unavailable | Manual only: Settings → MCP → enable **Playwright**. Agent cannot flip that toggle. | +| **Configured but broken** | Entry exists; server shows error / tools fail / navigate fails. Common: custom CDP like `http://localhost:9222` with nothing listening | Explain the failure plainly. Offer to replace with the standard `npx @playwright/mcp@latest` block (reply **add playwright mcp** or **fix playwright mcp**). Warn that this overwrites a custom playwright entry; keep other servers. Then reload and recheck. | +| **Errored / won't start** | Tools missing; logs or STATUS show install/runtime errors | Diagnose (`npx`, Node, network). Offer the standard config; author confirms reload. | + +Standard config to add or restore (do not overwrite unrelated servers; for **configured but broken**, replace only the `playwright` server entry after the author agrees): + +```json +{ + "mcpServers": { + "playwright": { + "command": "npx", + "args": ["@playwright/mcp@latest"] + } + } +} +``` + +**Blocked checkpoint shape (author-facing):** + +> Playwright MCP isn't ready, so I can't run the live DOM checks on this path yet. +> +> **Manual setup:** Cursor Settings → MCP → enable **Playwright** (or fix / re-add it), then reload. +> +> If Playwright is listed but broken (for example a CDP URL like `localhost:9222` with no browser listening), reply **fix playwright mcp** and I'll switch it to the standard `npx @playwright/mcp@latest` config. You'll still need to reload MCP afterward. +> +> **Or:** reply **add playwright mcp** if it is missing entirely. +> +> Skipping live checks with `static-only` isn't a fit for new or conversion interactive paths. + +Never edit `mcp.json` until the author agrees. Never claim MCP is ready until `GetMcpTools` for `user-playwright` shows usable browser tools (and a quick navigate smoke succeeds when diagnosing "broken"). + +--- + +## Two browsers (live path) + +| Browser | Who | Purpose | +|---|---|---| +| **Playwright** (MCP) | Agent (+ author Okta login) | Required DOM existence checks | +| **Normal browser** | Author | Optional Block Editor (`walk-me` only) | + +DOM exists ≠ Show me / Do it works. Reviewers still run full Block Editor smoke on the PR. + +--- + +## Playwright DOM (required) + +After author replies `ready` at the login pause: + +For each interactive milestone in scope (path order): + +1. Start URL: first `on-page:/path` in milestone blocks, else path manifest `startingLocation` (same as [review milestone start URL](../review-learning-path/reference-checks.md#milestone-start-url)). +2. Navigate on `{learn_host}`. +3. For each `reftarget`, record: `exists` / `missing` / `below-fold` / `state-dependent`. +4. Do **not** stop for author confirmation between milestones. One short progress line is enough; save the summary for the results menu. + +Respect [live testing prerequisites](../review-learning-path/reference-checks.md#live-testing-prerequisites-phase-2) (stack state, credential-gated UI). Prefer documenting stack gaps over false "missing selector" on the wrong stack. + +Apply [selector decision tree](../review-learning-path/reference-checks.md#selector-decision-tree) when promoting findings after live results. + +--- + +## Block Editor (opt-in) + +Authors are expected to dogfood while writing. Do **not** force a per-milestone interrogation by default. + +### Prompt (once, after Playwright) + +> Have you already smoke-tested this path in Block Editor (Show me / Do it)? +> +> Reply **`already-tested: `** if yes (stack + anything flaky). +> Reply **`walk-me`** if you want a guided per-milestone check now. +> Reply **`skip-smoke`** to continue without recording Block Editor evidence. + +### Local import (`walk-me` only) + +No PR yet, so **do not** use the PR review tool. + +1. Open `{learn_host}/plugins/grafana-pathfinder-app?dev=true` in the normal browser. +2. **?** → Debug → **Block Editor**. +3. Import `{path_dir}/{milestone}/content.json` from the local checkout. +4. Open the derived start URL; run every **Show me** and **Do it**. + +### Guided loop prompts + +For each scoped milestone: + +> **Milestone {i} of {M}: `{milestone-id}`** +> +> 1. Import the local `content.json` +> 2. Open `{derived_start_url}` +> 3. Run every **Show me** and **Do it** +> +> **While you run it, watch for:** wrong highlights, Do it that no-ops or errors, steps that never unlock, and fake step numbers (intro prose or "You'll…" lines counted as steps). +> +> **Your turn:** Reply **pass**, **fail step N -** *what happened*, or **N/A -** *reason*. + +Do **not** give the fake-step-number glance its own numbered setup step. Keep it inside the short **While you run it, watch for** line. + +Record in state `pathfinder.{milestone-slug}`. + +--- + +## Milestone scope + +| `path_type` | Scope | +|---|---| +| **new** / **conversion** | Every interactive milestone in path `milestones` | +| **update** | Touched interactive milestones first; full path on request | + +Skip terminal-only / external-CLI milestones for DOM and walk-me unless the author asks. diff --git a/.cursor/skills/preflight-learning-path/claim-check.md b/.cursor/skills/preflight-learning-path/claim-check.md new file mode 100644 index 000000000..175874c72 --- /dev/null +++ b/.cursor/skills/preflight-learning-path/claim-check.md @@ -0,0 +1,7 @@ +# Claim check (preflight pointer) + +**Canonical policy:** [../review-learning-path/claim-check.md](../review-learning-path/claim-check.md) + +Shared by `/preflight-learning-path` and `/review-learning-path-pr`. Do not maintain a second verdict table here. + +When preflight runs the static pass, follow that file. Write the report to `.cursor/lp-preflight-state/{slug}-claim-check.md` and use the **Preflight (results menu)** chat section. diff --git a/.cursor/skills/preflight-learning-path/frontend-selector-pr.md b/.cursor/skills/preflight-learning-path/frontend-selector-pr.md new file mode 100644 index 000000000..ee3480d25 --- /dev/null +++ b/.cursor/skills/preflight-learning-path/frontend-selector-pr.md @@ -0,0 +1,137 @@ +# Frontend selector PR (preflight) + +Walk an author through adding a stable `data-testid` upstream when Pathfinder needs one and the DOM has no good selector. + +**Canonical example:** [grafana/grafana-cmab-app#1795](https://github.com/grafana/grafana-cmab-app/pull/1795) (Add `data-testid` selectors to Usage page components). + +Use that PR as the model for: + +- Small, uniform component changes (testids only) +- Plain kebab-case names matching the product area +- PR title/body that explains Pathfinder authoring needs stable selectors +- Friendly, low-ceremony tone + +Do not paste the whole #1795 diff. Link it and mirror the pattern. No em dashes in drafted PR bodies. + +--- + +## When to enter + +Trigger when live checks show a step needs a durable selector and: + +- No `data-testid` / strong semantic attribute exists in the DOM, and +- Live Playwright or walk-me failed on that step (or the author wants a proactive testid anyway), and +- They reply **`frontend`** at the results menu or ask after package fixes + +At the results menu, treat this as a **needs-frontend** finding: lead with **frontend**, not **fix N**. A package-only `reftarget` tweak is not the real fix. Only offer a temporary weak selector in the guide if the author explicitly asks, and label it as brittle. + +If live passed with a justified `:contains()` and no stable selector exists, that stays Internal in review terms. Offer this walkthrough as an optional improvement, not a Fix-before-PR demand, unless Playwright/walk-me failed on that step. + +--- + +## Walkthrough (checkpointed) + +These pauses are intentional (author must approve name, repo, draft, push). Announce **Frontend selector (step X of 5)** at each stop. One ask per message. Do not call this "Phase 5" in author chat. + +### 1. Confirm the element + +From Playwright: URL, visible label, current weak selector, screenshot or DOM snippet if helpful. + +Propose a `data-testid` (plain kebab-case, product-area prefix, matching existing testids in that app when present). + +> **Frontend selector (step 1 of 5)** +> +> **Element that failed live** +> - Page: {url or page name} +> - Control: {visible label} +> - Guide selector today: `{reftarget}` +> +> **Proposed `data-testid`:** `{proposed-name}` +> +> **Your turn:** Reply **yes** if that name is fine, or suggest a different kebab-case name. + +**Wait for:** author OK on the name (or edits). + +### 2. Locate owning source + +- Default core UI: `grafana/grafana` +- Plugin app pages (`/a/grafana-*-app/...`): that plugin repo (as #1795 did for CMAB) + +Use workspace clones or GitHub search. Do **not** invent file paths. + +> **Frontend selector (step 2 of 5)** +> +> Best guess for owning source: `{repo}` ({why}). +> +> Next I will confirm the exact file(s). I will not invent paths. +> +> **Your turn:** Reply **yes** if `{repo}` looks right, or name a different repo. + +**Wait for:** confirm repo + file(s). + +### 3. Draft the change + +- Minimal diff: add `data-testid` (and wire through component props if needed) +- Match local naming conventions +- Draft PR title + body in the style of #1795 (Pathfinder needs stable selectors; keep scope tight) + +Example body shape (adapt; do not copy blindly): + +```markdown +Hey folks, I was building a Pathfinder guide and noticed some UI elements lacked stable selector IDs. + +Adds `data-testid` attributes to {components} following the existing kebab-case naming on this page. These selectors are needed for interactive Pathfinder guides, which prefer `data-testid` over text or class-based targeting. + +Example pattern: https://github.com/grafana/grafana-cmab-app/pull/1795 +``` + +> **Frontend selector (step 3 of 5)** +> +> Draft change: add `{testid}` on {component summary}. Testids only; no behavior change. +> +> **Title:** {title} +> +> **Body:** (show draft) +> +> **Your turn:** Reply **yes** to approve, or say what to change. + +**Wait for:** author approval of the draft. + +### 4. Open the PR + +1. Confirm `gh` auth and write access +2. Create branch, commit, push (**ask before push**) +3. `gh pr create` with the approved title/body +4. Follow existing grafana org repo norms; do not change repo visibility + +> **Frontend selector (step 4 of 5)** +> +> Ready to open the PR in `{repo}` with the approved title/body. +> +> **Your turn:** Reply **yes** to push and create the PR, or **hold** to stop here. + +Record `frontend_pr_url` in preflight state after create. + +### 5. Update the guide selector + +Update the milestone `reftarget` to the new testid **only after** the author confirms the frontend change is available on their test stack. + +Until then: keep a temporary justified fallback if needed, and note the upstream PR URL in readiness / Open PR with notes. + +> **Frontend selector (step 5 of 5)** +> +> Frontend PR: {url} +> +> When `{testid}` is on your test stack, we can update `{milestone}` to `[data-testid="{testid}"]`. We should not change the guide before that lands. +> +> **Your turn:** Reply **update guide** when the testid is available, or **done** to leave the guide as-is for now. + +--- + +## Anti-patterns + +- Giant refactors or drive-by UI changes in the testid PR +- Inventing component paths +- Updating guide JSON to a testid that is not on the test stack yet without saying so +- Treating a missing upstream testid as something to "fix" only inside `interactive-tutorials` when the DOM has no stable hook +- Offering bare **fix N** at the results menu as if a guide edit adds the testid diff --git a/.cursor/skills/preflight-learning-path/reference-checks.md b/.cursor/skills/preflight-learning-path/reference-checks.md new file mode 100644 index 000000000..bb1a651af --- /dev/null +++ b/.cursor/skills/preflight-learning-path/reference-checks.md @@ -0,0 +1,215 @@ +# Reference checks (preflight-learning-path) + +Author-side routing and readiness for [preflight-learning-path/SKILL.md](SKILL.md). + +**Shared checklists (source of truth):** [../review-learning-path/reference-checks.md](../review-learning-path/reference-checks.md) and [../review-learning-path/learning-hub-standards.md](../review-learning-path/learning-hub-standards.md). Run every checklist the five-phase review coach runs in Phase 1. + +**Publishing model (PR [#416](https://github.com/grafana/interactive-tutorials/pull/416)):** Single-repo packages in `interactive-tutorials`. Metadata in package `website.yaml`; prose in `content.json`. Website repo is read-only for conversion. Never plan companion website / `pathfinder_data` / shortcode work. + +**Voice bar:** Same severity bar as review [comment-style.md](../review-learning-path/comment-style.md) (review-level only in chat). No em dashes. Prefer everyday words in chat (see [Author-facing findings](#author-facing-findings) and [Voice](SKILL.md#voice-author-facing)). + +--- + +## Author-facing findings + +Use this shape for the **one results menu** after live (or after allowed static-only). Do **not** open with a "What this check is" primer. Match [Voice](SKILL.md#voice-author-facing). + +1. **Friendly outcome line** (e.g. "almost ready, with 3 copy fixes first"). Keep gate labels in the readiness file; chat can stay plainer. +2. **What we checked** — short bullets in plain language (product claims vs docs, UI selectors on `{stack}`, Block Editor smoke choice). +3. **Numbered findings** — each item: plain problem, why it matters or better wording, then the file/dir in parentheses. Enough context that the author can decide without opening the skill. Treat each item as **package-fixable** or **needs-frontend** (see below). +4. **Your turn** — offer only actions that can resolve the open items: + - **fix all** / **fix N** / combos — only for **package-fixable** items (prose, manifest, `website.yaml`, guide `reftarget` when a better selector already exists in the DOM) + - **frontend** — when any open item **needs-frontend** (live failed; no durable `data-testid` / strong semantic selector in the DOM). Do **not** offer **fix N** for that item as if a guide edit alone is the real fix + - **done** — open PR / leave for review + - **show report** +5. **Heads-up** — optional stack/testing notes (already-installed plugins, read-only provisioned sources, etc.). + +### Package-fixable vs needs-frontend + +| Kind | Examples | Results menu offer | +|---|---|---| +| **Package-fixable** | Claim-check copy, framing/`depends`, fake in-section steps, false noops, wrong `reftarget` when a stable testid already exists in the DOM | **fix N** / **fix all** | +| **Needs-frontend** | Playwright or walk-me failed; element has no durable selector in the DOM | **frontend** (primary). Optional: clearly labeled **try temporary selector** only if the author asks for a brittle package workaround. Never present bare **fix N** as the main path for this kind | + +When the list mixes both kinds, number everything, but in **Your turn** say which numbers **fix** covers and that **frontend** covers the selector gap (e.g. "fix 2,3 for copy; **frontend** for item 1"). + +Keep the identify + static work quiet when the path is known. Put the full numbered + fix-choice treatment in the **one results menu** after live (or after allowed static-only). + +--- + +## Finding severity (author) + +Map review [finding routing](../review-learning-path/reference-checks.md#finding-routing) 1:1: + +| Review tier | Author action | +|---|---| +| **Post inline** | **Fix before PR** (surface in chat + readiness) | +| **Internal** | Hide (do not mention in chat or readiness) | +| **Discard** | Hide | + +Do not invent an author-only softer bar. Do not surface "Polish / follow-up" lists. + +### Must surface (post-inline examples) + +Apply the same five-phase coach rules (cite shared reference-checks; do not soften): + +- Playwright / Block Editor runtime fail (when live failed or `walk-me` failed) +- **Framing milestones** in path `milestones`, or first hands-on `depends` on a framing ID (must be `depends: []`) +- **Fake steps in sections:** missing bookends outside the section; in-section intro markdown that numbers as a step (e.g. first child "You'll …") +- **False noops:** learner-action copy with `noop` and no `reftarget` +- Missing / broken required `website.yaml` identity; Learning Hub structure the author must change +- Path root / manifest `id` mismatch; Pathfinder CLI validate failure +- Secrets `doIt: true`; confirmed 404s; conversion prose only in legacy markdown +- Fragile / wrong selectors when live fails, or stable `data-testid` exists in DOM and the guide uses a weak selector +- **Claim-check MUST FIX:** Contradicted, Unsupported, or Overstated product facts per shared [claim-check.md](../review-learning-path/claim-check.md) (made-up counts, invented names, docs contradictions) + +### Never surface + +Wording polish, justified `:contains()` when live passed, CODEOWNERS reminders, audit noise, selector polish when live passed, landing screenshot notes, milestone-count guidelines. + +--- + +## Checklists to run (Phase 1) + +Apply every section from [../review-learning-path/reference-checks.md](../review-learning-path/reference-checks.md): + +- Milestone `content.json` checks +- Section intro markdown numbered as a step +- Framing milestones / framing vs not framing +- Path root `content.json` +- Learning Hub structure + [learning-hub-standards.md](../review-learning-path/learning-hub-standards.md) +- `website.yaml` +- Valid manifests (CLI) +- Targeting / recommender +- Supplementary content +- Legacy website source (conversion, read-only) +- noop and non-interactive steps +- CODEOWNERS (discard for author chat) + +Then run shared [claim-check.md](../review-learning-path/claim-check.md) across path root + milestone prose. Route Contradicted / Unsupported / Overstated as Fix before PR. Hide Supported from chat. Author-decides items may appear in readiness as open questions. + +Tag each finding; keep only post-inline for author-facing output. + +--- + +## Path type + +Infer from branch diff, directory age, and legacy website source. Record in `{slug}.json` → `path_type`. + +| Type | Signals | Live emphasis | +|---|---|---| +| **new** | New `{slug}-lj/`; no legacy website folder | Full Playwright; `website.yaml` + path root completeness | +| **conversion** | Built via `/build-interactive-lj`; prose-heavy | Legacy prose captured in package; Playwright full path | +| **update** | Changes existing package only | Touched interactive milestones first | + +--- + +## Static-only preflight + +`static-only: ` at the **login / stack pause** skips Playwright and smoke (jump to the results menu). + +**Precedence (evaluate in order; first match wins):** + +1. **Bare `static-only` (no reason)** → always **reject**. +2. **`path_type` is `new` or `conversion` and the path has interactive milestones** → always **reject**, even if the author cites "no stack access" or "practice run." Playwright is mandatory here. +3. **`path_type` is `update` and the change touches only markdown / `website.yaml` (no interactive selector work)** → **allow** with a non-empty reason. Cap readiness (never **Ready for PR** if interactive milestones exist and were not live-tested). +4. **Practice run / no stack access on `update` only** → **allow** with a non-empty reason. Cap readiness the same way. +5. Anything else → **reject** and ask for a clearer reason or proceed with live checks. + +| Situation | Allowed? | +|---|---| +| Bare `static-only` | **No** | +| **new** / **conversion** with interactive milestones (any reason, including no stack) | **No** | +| **update** touching only markdown / `website.yaml` | Yes, with reason | +| **update** practice / no stack access | Yes, with reason (caps readiness) | + +Record `waive_live_testing` + `static_only_reason` only when allowed. + +When live was skipped or incomplete, readiness must include **Not live-tested** (interactive path `milestones` minus recorded Playwright results). Never recommend **Ready for PR** for new/conversion interactive when Playwright was waived. + +--- + +## Readiness gate + +Recommend **Ready for PR** only when all are true: + +1. Zero open **Fix before PR** (post-inline) items +2. Pathfinder CLI validate passed, or CLI unavailable was noted and no other blockers +3. Playwright: no unexplained **missing** selectors on claimed-tested milestones (or documented stack prerequisite) +4. Block Editor: `already-tested` or successful `walk-me` (not `skip-smoke`) for new/conversion interactive, **or** author accepts **Open PR with notes** +5. `git status` clean of audit-guide artifacts under `{path_dir}` +6. Not `waive_live_testing` on new/conversion interactive (or outcome is **Open PR with notes**) + +| Outcome | When | +|---|---| +| **Ready for PR** | Gate conditions met | +| **Fix then re-preflight** | Open post-inline items or Playwright/walk-me failures | +| **Open PR with notes** | Package mergeable but smoke skipped, or fresh-stack retest notes for the reviewer | + +--- + +## PR opener checklist + +Include in `{slug}-readiness.md`: + +- [ ] Path `{path_dir}` validates with Pathfinder CLI (or CLI unavailable noted) +- [ ] First hands-on milestone `depends: []` +- [ ] No framing IDs in path `manifest.json` `milestones` +- [ ] `schemaVersion: "1.1.0"` or omitted on milestone `content.json` +- [ ] Playwright DOM checked for scoped interactive milestones +- [ ] Block Editor: already-tested notes, walk-me results, or skip noted for reviewer +- [ ] Path and milestone `website.yaml` complete +- [ ] **Not live-tested** section in PR description if any interactive milestone skipped Playwright +- [ ] Single PR in `interactive-tutorials` only (no companion website PR) + +--- + +## State file schema + +Path: `.cursor/lp-preflight-state/{slug}.json` + +```json +{ + "path_dir": "monitor-azure-resources-lj", + "slug": "monitor-azure-resources-lj", + "website_slug": "monitor-azure-resources", + "path_type": "conversion", + "branch": "docs/my-path", + "head_commit": "abc123", + "learn_host": "learn.grafana.net", + "stack_state": "learn.grafana.net shared", + "waive_live_testing": false, + "static_only_reason": null, + "smoke_mode": null, + "smoke_notes": null, + "pre_review_assets": {}, + "checkpoint": "login", + "status": "in_progress", + "readiness": null, + "playwright": {}, + "pathfinder": {}, + "frontend_pr_url": null +} +``` + +| Field | Notes | +|---|---| +| `smoke_mode` | `already-tested` \| `walk-me` \| `skip-smoke` \| null | +| `smoke_notes` | Author notes for already-tested / skip | +| `pathfinder` | Only when `walk-me`; keys = milestone slug | +| `playwright` | DOM results per milestone | +| `checkpoint` | Named gate (required). One of: `identify` \| `login` \| `dom` \| `smoke` \| `results` \| `package_fixes` \| `frontend`. Persist at every gate per [SKILL.md](SKILL.md). Do **not** use integer phase 0–5. | +| `status` | `in_progress` \| `blocked` \| `complete` | +| `readiness` | `Ready for PR` \| `Fix then re-preflight` \| `Open PR with notes` | + +Legacy state files with numeric `phase` are invalid for resume: ask **start fresh**, or map once (`0`→`identify`, `1`→`login`, `2`→`dom`, `3`→`results`, `4`→`package_fixes`, `5`→`frontend`) then rewrite `checkpoint` and drop `phase`. + +--- + +## Pathfinder CLI validate + +```bash +node {pathfinder-app}/dist/cli/cli/index.js validate --packages {path_dir} +``` + +CLI failure → Fix before PR. CLI missing → note in readiness; do not abort the whole preflight. diff --git a/.cursor/skills/review-learning-path/SKILL.md b/.cursor/skills/review-learning-path/SKILL.md index 459d29114..0154c9571 100644 --- a/.cursor/skills/review-learning-path/SKILL.md +++ b/.cursor/skills/review-learning-path/SKILL.md @@ -21,9 +21,9 @@ Help a **human reviewer** work through a `{slug}-lj/` pull request in **five pha **Skill memory:** State lives in `.cursor/pr-review-state/` (gitignored). Phase 1 dispatches [audit-guide](../audit-guide/SKILL.md), which writes `{milestone}/assets/` on the PR branch. See [Commit safety](#commit-safety). -**Routing:** [reference-checks.md](reference-checks.md) · [comment-style.md](comment-style.md) · [learning-hub-standards.md](learning-hub-standards.md) · [github-review.md](github-review.md) +**Routing:** [reference-checks.md](reference-checks.md) · [claim-check.md](claim-check.md) · [comment-style.md](comment-style.md) · [learning-hub-standards.md](learning-hub-standards.md) · [github-review.md](github-review.md) -**Related:** [audit-guide](../audit-guide/SKILL.md) · [review-guide-pr.mdc](../../review-guide-pr.mdc) +**Related:** [audit-guide](../audit-guide/SKILL.md) · [review-guide-pr.mdc](../../review-guide-pr.mdc) · [preflight-learning-path](../preflight-learning-path/SKILL.md) (author pre-PR mirror) --- @@ -148,7 +148,7 @@ If `.cursor/pr-review-state/pr-{n}.json` exists: ## Phase 1: Static pass -**Goal:** Audit every milestone + path consistency + Learning Hub checks. Output: **internal workbook only**. +**Goal:** Audit every milestone + path consistency + Learning Hub checks + shared [claim-check](claim-check.md). Output: **internal workbook only**. Combines the former Phases 1–2 and workbook write. @@ -162,15 +162,16 @@ Combines the former Phases 1–2 and workbook write. 1. Snapshot `pre_review_assets`; dispatch [audit-guide](../audit-guide/SKILL.md) per milestone (parallel OK). 2. Walk all [reference-checks.md](reference-checks.md) checklists + [learning-hub-standards.md](learning-hub-standards.md). -3. **Always scan** for [section intro markdown that may number as a step](reference-checks.md#section-intro-markdown-numbered-as-step) and [false noops](reference-checks.md#noop-and-non-interactive-steps). Put matches under **Verify in Block Editor**. +3. **Always scan** for [section intro markdown that may number as a step](reference-checks.md#section-intro-markdown-numbered-as-a-step) and [false noops](reference-checks.md#noop-and-non-interactive-steps). Put matches under **Verify in Block Editor**. 4. Run Pathfinder CLI validate if available. -5. Tag every finding with [finding routing](reference-checks.md#finding-routing): **post inline**, **internal**, or **discard**. -6. Write `pr-{n}-findings.md` — header: *"Reviewer workbook — internal only. Do not paste to PR."* +5. Run the shared [claim-check](claim-check.md) pass. Write `pr-{n}-claim-check.md`. Route Contradicted / Unsupported / Overstated as **post inline** (same bar as preflight). Do not edit package JSON here. +6. Tag every finding with [finding routing](reference-checks.md#finding-routing): **post inline**, **internal**, or **discard**. +7. Write `pr-{n}-findings.md` — header: *"Reviewer workbook — internal only. Do not paste to PR."* - **Verify in Block Editor** — items that need live test to confirm (include section-intro markdown and false-noop candidates) - - **Post inline if confirmed** — static compliance issues (broken depends, id mismatch, CLI fail) + - **Post inline if confirmed** — static compliance issues (broken depends, id mismatch, CLI fail) and claim-check MUST FIX items - **Internal** — nits, LH editorial, selector polish, `website.yaml` gaps -7. Mandatory audit cleanup; verify `git status`. -8. **Do not** cite rule numbers or blocking counts in chat. +8. Mandatory audit cleanup; verify `git status`. +9. **Do not** cite rule numbers or blocking counts in chat. ### Checkpoint @@ -399,6 +400,7 @@ Not a numbered phase. New review cycle for major author pushes. Use REST inline |---|---|---| | `pr-{n}.json` | 0+ | State ([schema](github-review.md#state-file-schema)) | | `pr-{n}-findings.md` | 1 | **Internal workbook** — never paste to author | +| `pr-{n}-claim-check.md` | 1 | Shared claim-check report ([claim-check.md](claim-check.md)) | | `pr-{n}-review-body.md` | 3 → 4 | Final summary for GitHub submit | Workbook frontmatter: @@ -418,6 +420,7 @@ pr_number: {n} | Topic | Doc | |---|---| | Finding routing + checklists | [reference-checks.md](reference-checks.md) | +| Claim-check (shared with preflight) | [claim-check.md](claim-check.md) | | Comment voice + summary | [comment-style.md](comment-style.md) | | Learning Hub checks (workbook) | [learning-hub-standards.md](learning-hub-standards.md) | | GitHub GraphQL | [github-review.md](github-review.md) | diff --git a/.cursor/skills/review-learning-path/claim-check.md b/.cursor/skills/review-learning-path/claim-check.md new file mode 100644 index 000000000..9b56af5b7 --- /dev/null +++ b/.cursor/skills/review-learning-path/claim-check.md @@ -0,0 +1,133 @@ +# Claim check (shared learning-path policy) + +Adversarial factual check for Pathfinder learning path packages. Adapted from the Learning Hub claim-check skill and the MUST FIX bar in journey audit reports. Goal: catch invented product facts, not Hugo slide structure. + +**Canonical policy for both** [review-learning-path](SKILL.md) and [preflight-learning-path](../preflight-learning-path/SKILL.md). Do not fork a second verdict table. Preflight keeps a thin pointer at `../preflight-learning-path/claim-check.md` that redirects here. + +**When to run:** During the static pass, after audit-guide and path / Learning Hub checks, before the live gate. + +**Independence:** Prefer a fresh agent pass that did not author the package prose. Default stance: unproven. A claim is flagged unless a source plainly supports it. + +--- + +## Scope (path package) + +Read all prose that learners see: + +- Path root `content.json` markdown blocks +- Every milestone `content.json` markdown (including framing dirs not in path `milestones`, if present) +- Interactive step `content` / tooltips that state product facts (skip pure click instructions) +- Path and milestone `website.yaml` `description` / CTA copy when it asserts product behavior + +Skip: pure navigation instructions ("Click **Save**"), structural Pathfinder rules, and restatements of UI chrome with no product claim. + +Website repo (when in workspace) is **read-only** for sources (`content/docs/...`). Do not require a companion website PR. + +--- + +## What counts as a claim + +Falsifiable statements about the product, including: + +- What a feature does or does not do +- Feature / product / alert / dashboard names +- Versions, defaults, limits, prerequisites, platform availability +- Absolutes and quantifiers (only, never, all, every, must, N dashboards, N alerts) +- Comparisons that rank or prescribe one Grafana capability over another + +### Not claims + +- "In this milestone you will…" framing +- UI click paths without a product assertion +- Learning Hub boilerplate that matches standards verbatim + +--- + +## Procedure + +1. Extract claims (batch by milestone; include path root). +2. For each claim, find a source: local `website` docs first, then WebFetch to live grafana.com docs. No source → **Unsupported**. +3. Open the source; quote the supporting or contradicting line. +4. Verdict (see below). Sweep phrase + category across the whole package so the same error is one finding with multiple locations. +5. Write the report (path depends on caller; see below). + +Do not edit package JSON during the claim check. Fixes wait for author request (preflight) or reviewer post approval (review). + +--- + +## Verdicts + +| Verdict | Meaning | Shared routing | +|---|---|---| +| **Supported** | Source plainly states the claim | Hide (optional Supported list in the report only) | +| **Contradicted** | Source says something different | Review-level: **post inline** / Fix before PR | +| **Unsupported** | No source, or source does not address it | Review-level: **post inline** / Fix before PR | +| **Overstated** | Source supports a weaker version; claim adds absolute/superlative/framing | Review-level: **post inline** / Fix before PR | +| **Author decides** | Product-truth call a human must make | Workbook / readiness open question; do not auto-edit | + +Only Contradicted / Unsupported / Overstated count toward review-level findings (dedupe by root cause). Soft style nits stay Internal / out of author chat. + +--- + +## Catches that matter (linter cannot) + +1. Made-up facts +2. Invented terms / alert names / dashboard names +3. Overstatements (counts, "zero impact," "most/all/every") +4. Competitive or prescriptive framing without a source +5. Pre-GA / preview features taught as GA + +--- + +## Report paths + +| Caller | Report file | +|---|---| +| **preflight** | `.cursor/lp-preflight-state/{slug}-claim-check.md` | +| **review** | `.cursor/pr-review-state/pr-{n}-claim-check.md` | + +```markdown +--- +disclaimer: Auto-generated by learning-path claim-check. Do not edit manually. +path_dir: {path_dir} +--- + +# Claim check: {path_dir} + +**Claims checked: N | Supported: N | Contradicted: N | Unsupported: N | Overstated: N** + +## MUST FIX (review-level) + +### Contradicted +1. `{file}` - {claim}. Source: {quote or URL}. Fix: {one line}. + +### Unsupported +… + +### Overstated +… + +## Author decides +… + +## Sources used +… +``` + +MUST FIX first, then author-decides. Do not dump Supported claims into chat. No em dashes in the report body. + +--- + +## Caller-specific chat + +### Preflight (results menu) + +Merge MUST FIX items into the findings that feed the **one results menu** (dedupe). Use plain language (problem + why + file). Do **not** put verdict labels like "Contradicted" in author bullets. Do not open with a "What this check is" primer. + +Offer **fix all** / **fix N** / combos for package-fixable claim items per [preflight Author-facing findings](../preflight-learning-path/reference-checks.md#author-facing-findings). + +If none, say the claim check found no contradicted or unsupported product facts (as part of What we checked / clean results). + +### Review (Phase 1 workbook) + +Merge MUST FIX items into `pr-{n}-findings.md` under **Post inline if confirmed** (or **Verify in Block Editor** when live confirmation is needed). Tag **post inline** per [finding routing](reference-checks.md#finding-routing). Do not dump Supported into chat. Author-decides may stay Internal or become a short open question in the draft summary. diff --git a/.cursor/skills/review-learning-path/reference-checks.md b/.cursor/skills/review-learning-path/reference-checks.md index d76397d0b..9ea314375 100644 --- a/.cursor/skills/review-learning-path/reference-checks.md +++ b/.cursor/skills/review-learning-path/reference-checks.md @@ -69,7 +69,7 @@ Run via [audit-guide](../audit-guide/SKILL.md) plus confirm every row: | `schemaVersion` not `"1.1.0"` when present | post inline | | Markdown `##` / `###` for grouping | internal | | Section bookends missing (rule 14) — no intro/summary **around** the section | **post inline** | -| In-section intro markdown that may number as a step | **post inline** (see [section intro markdown](#section-intro-markdown-numbered-as-step)). Prefer bookends **outside** the section per rule 14. | +| In-section intro markdown that may number as a step | **post inline** (see [section intro markdown](#section-intro-markdown-numbered-as-a-step)). Prefer bookends **outside** the section per rule 14. | | Missing `exists-reftarget`, `navmenu-open` | internal until live fails | | Missing `on-page` | internal until live fails | | `lazyRender` missing on virtualized targets | internal until live fails | @@ -209,13 +209,22 @@ Prose not captured in package on conversion → **post inline**. Front matter ma `static-only: ` at end of Phase 1 skips Phase 2. +**Precedence (evaluate in order; first match wins):** + +1. **Bare `static-only` (no reason)** → always **reject**. +2. **`pr_type` is `new` or `conversion` and the path has interactive milestones** → always **reject**, even if the reviewer cites "practice" or "no stack." Playwright is mandatory here. +3. **`pr_type` is `update` with only markdown / `website.yaml` changes** → **allow** with a non-empty reason. +4. **Practice / archaeology on a merged PR** → **allow** with a non-empty reason (never suggest APPROVE when waived). +5. Anything else → **reject**. + | Situation | Allowed? | |---|---| -| **new** / **conversion** with interactive milestones | **No** | +| Bare `static-only` | **No** | +| **new** / **conversion** with interactive milestones (any reason) | **No** | | **update** with only markdown / `website.yaml` | Yes, with reason | | Practice / archaeology on merged PR | Yes, with reason | -Record `waive_live_testing: true` and `static_only_reason`. Never suggest APPROVE when waived. +Record `waive_live_testing: true` and `static_only_reason` only when allowed. Never suggest APPROVE when waived. ### Not live-tested disclosure (required) diff --git a/AGENTS.md b/AGENTS.md index 7eb80c276..0ce891391 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -42,6 +42,7 @@ Full reference documentation lives in `docs/`. AI-oriented references live in `. | Update/modify an existing guide | [update-guide](.cursor/skills/update-guide/SKILL.md) skill | [authoring-guide.mdc](.cursor/authoring-guide.mdc), [best-practices.mdc](.cursor/best-practices.mdc), [review-guide-pr.mdc](.cursor/review-guide-pr.mdc) | | Review a guide PR | [review-guide-pr.mdc](.cursor/review-guide-pr.mdc) | [authoring-guide.mdc](.cursor/authoring-guide.mdc), [best-practices.mdc](.cursor/best-practices.mdc), [edge-cases-and-troubleshooting.mdc](.cursor/edge-cases-and-troubleshooting.mdc), `docs/` | | Review a learning path PR | [/review-learning-path-pr](.cursor/commands/review-learning-path-pr.md) | [review-learning-path](.cursor/skills/review-learning-path/SKILL.md), [audit-guide](.cursor/skills/audit-guide/SKILL.md), [review-guide-pr.mdc](.cursor/review-guide-pr.mdc) | +| Preflight a learning path (before PR) | [/preflight-learning-path](.cursor/commands/preflight-learning-path.md) | [preflight-learning-path](.cursor/skills/preflight-learning-path/SKILL.md), [audit-guide](.cursor/skills/audit-guide/SKILL.md), [review-learning-path](.cursor/skills/review-learning-path/SKILL.md) | | Decision trees & code smells | [best-practices.mdc](.cursor/best-practices.mdc) | [authoring-guide.mdc](.cursor/authoring-guide.mdc), `docs/` | | Create new guide | `/new` command | [authoring-guide.mdc](.cursor/authoring-guide.mdc), [complete-example-tutorial.mdc](.cursor/complete-example-tutorial.mdc) | | Validate guide | `/lint`, `/check`, `/attack` commands | [authoring-guide.mdc](.cursor/authoring-guide.mdc), [best-practices.mdc](.cursor/best-practices.mdc) | @@ -82,4 +83,5 @@ Full reference documentation lives in `docs/`. AI-oriented references live in `. | [/check](.cursor/commands/check.md) | Check guide quality against best practices | | [/attack](.cursor/commands/attack.md) | Find issues by simulating confused users | | [/review-learning-path-pr](.cursor/commands/review-learning-path-pr.md) | Full learning path PR review (audit, consistency, Playwright, Pathfinder, GitHub submit) | +| [/preflight-learning-path](.cursor/commands/preflight-learning-path.md) | Author pre-PR self-review for a learning path (mirrors review checks + shared claim-check, then optional fixes) | | [/build-interactive-lj](.cursor/commands/build-interactive-lj/README.md) | Multi-phase learning journey builder | diff --git a/CLAUDE.md b/CLAUDE.md index d37dbb087..7971ce87c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,6 +17,7 @@ When a doc here disagrees with the upstream schema, the schema wins. Authoritati | Update / modify an existing guide | [update-guide](.cursor/skills/update-guide/SKILL.md) skill | [authoring-guide.mdc](.cursor/authoring-guide.mdc), [best-practices.mdc](.cursor/best-practices.mdc), [review-guide-pr.mdc](.cursor/review-guide-pr.mdc) | | Review a guide PR | [.cursor/review-guide-pr.mdc](.cursor/review-guide-pr.mdc) | [authoring-guide.mdc](.cursor/authoring-guide.mdc), [best-practices.mdc](.cursor/best-practices.mdc), [edge-cases-and-troubleshooting.mdc](.cursor/edge-cases-and-troubleshooting.mdc) | | Review a learning path PR | [/review-learning-path-pr](.cursor/commands/review-learning-path-pr.md) | [review-learning-path](.cursor/skills/review-learning-path/SKILL.md), [audit-guide](.cursor/skills/audit-guide/SKILL.md) | +| Preflight a learning path (before PR) | [/preflight-learning-path](.cursor/commands/preflight-learning-path.md) | [preflight-learning-path](.cursor/skills/preflight-learning-path/SKILL.md), [audit-guide](.cursor/skills/audit-guide/SKILL.md), [review-learning-path](.cursor/skills/review-learning-path/SKILL.md) | | Decision trees & code smells | [.cursor/best-practices.mdc](.cursor/best-practices.mdc) | [authoring-guide.mdc](.cursor/authoring-guide.mdc), [docs/](docs/) | | Create a new guide | [/new](.cursor/commands/new.md) command | [authoring-guide.mdc](.cursor/authoring-guide.mdc), [complete-example-tutorial.mdc](.cursor/complete-example-tutorial.mdc) | | Validate a guide | [/lint](.cursor/commands/lint.md), [/check](.cursor/commands/check.md), [/attack](.cursor/commands/attack.md) | [authoring-guide.mdc](.cursor/authoring-guide.mdc), [best-practices.mdc](.cursor/best-practices.mdc) | @@ -45,6 +46,8 @@ Multi-phase workflow skills with their own `SKILL.md` files. Skills write interm - [.cursor/skills/audit-guide/](.cursor/skills/audit-guide/SKILL.md) — comprehensive read-only audit (structural + semantic + adversarial) producing one prioritised report - [.cursor/skills/update-guide/](.cursor/skills/update-guide/SKILL.md) — modify an existing guide with a planning checkpoint, review pass, and CLI-validated rollback - [.cursor/skills/migrate-guide/](.cursor/skills/migrate-guide/SKILL.md) — migrate a guide or learning path to the Pathfinder package format +- [.cursor/skills/review-learning-path/](.cursor/skills/review-learning-path/SKILL.md) — human-guided learning path PR review (five-phase coach) +- [.cursor/skills/preflight-learning-path/](.cursor/skills/preflight-learning-path/SKILL.md) — author pre-PR self-review for a learning path ## Shared Content