From 705d4ef93e0263818427a8543eb0ff13f69e0f42 Mon Sep 17 00:00:00 2001 From: Konstantin Tursunov Date: Thu, 30 Jul 2026 15:57:34 +0800 Subject: [PATCH 1/5] =?UTF-8?q?feat(skills):=20add=20QA=20skills=20?= =?UTF-8?q?=E2=80=94=20file-bug-insight,=20drive-ui,=20playwright-cli,=20q?= =?UTF-8?q?uality-vector-tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four skills for the QA workflow, all verified against the live repo, the GitHub project board and the installed tooling rather than written from memory. file-bug-insight — file an Insight defect as a GitHub issue. Named to avoid colliding with the user-level `file-bug` skill, which silently shadowed it (user-level wins a name collision and the project copy never loads). Carries the medallion evidence walk, layer localization, the four-heading body template with a worked example, the public-repo scrub rules and the live board/priority field IDs. Issues are created with the native Type=Bug and no labels; component, team and planning labels are a grooming call. drive-ui — get an authenticated browser on any stand and capture evidence. Merges local-stand coverage (fakeidp default, the DEV_USER_EMAIL seed behind 403/caller_unresolved, ports and port-forward) with remote-stand browser acquisition (attach over CDP, never launch, detach never close). Includes the console/requests check that stops a gold-view defect being filed against the SPA. playwright-cli — the browser command surface, vendored so drive-ui has its commands in-repo. quality-vector-tests — author the Testing section of a feature issue, grouped by the five quality vectors with every check as a Metric/How-measured/Target triple. Provenance and specific feature numbers removed so it stays usable as the issues it was derived from move on. Reviewed for correctness and efficiency; fixes in this commit include the worked example's code links (handlers.rs L1070, entities.rs L39), the ClickHouse commands (silver. prefix and the required password), the duplicate search limit (closed issues rank after open, so a short window never reaches a regression), the CI-scanner premise (semgrep/trivy/codeql do run here), the metric-spec coverage denominator, and connector counts. Signed-off-by: Konstantin Tursunov --- .claude/skills/drive-ui/SKILL.md | 117 +++++ .claude/skills/file-bug-insight/SKILL.md | 232 ++++++++++ .claude/skills/playwright-cli/SKILL.md | 404 ++++++++++++++++++ .../references/element-attributes.md | 23 + .../references/playwright-tests.md | 39 ++ .../references/request-mocking.md | 87 ++++ .../playwright-cli/references/running-code.md | 241 +++++++++++ .../references/session-management.md | 225 ++++++++++ .../references/spec-driven-testing.md | 305 +++++++++++++ .../references/storage-state.md | 275 ++++++++++++ .../references/test-generation.md | 134 ++++++ .../playwright-cli/references/tracing.md | 139 ++++++ .../references/video-recording.md | 143 +++++++ .claude/skills/quality-vector-tests/SKILL.md | 262 ++++++++++++ .../references/example-lean.md | 115 +++++ .../references/example-migration.md | 87 ++++ .../references/example-port.md | 79 ++++ .../references/vector-mapping.md | 31 ++ 18 files changed, 2938 insertions(+) create mode 100644 .claude/skills/drive-ui/SKILL.md create mode 100644 .claude/skills/file-bug-insight/SKILL.md create mode 100644 .claude/skills/playwright-cli/SKILL.md create mode 100644 .claude/skills/playwright-cli/references/element-attributes.md create mode 100644 .claude/skills/playwright-cli/references/playwright-tests.md create mode 100644 .claude/skills/playwright-cli/references/request-mocking.md create mode 100644 .claude/skills/playwright-cli/references/running-code.md create mode 100644 .claude/skills/playwright-cli/references/session-management.md create mode 100644 .claude/skills/playwright-cli/references/spec-driven-testing.md create mode 100644 .claude/skills/playwright-cli/references/storage-state.md create mode 100644 .claude/skills/playwright-cli/references/test-generation.md create mode 100644 .claude/skills/playwright-cli/references/tracing.md create mode 100644 .claude/skills/playwright-cli/references/video-recording.md create mode 100644 .claude/skills/quality-vector-tests/SKILL.md create mode 100644 .claude/skills/quality-vector-tests/references/example-lean.md create mode 100644 .claude/skills/quality-vector-tests/references/example-migration.md create mode 100644 .claude/skills/quality-vector-tests/references/example-port.md create mode 100644 .claude/skills/quality-vector-tests/references/vector-mapping.md diff --git a/.claude/skills/drive-ui/SKILL.md b/.claude/skills/drive-ui/SKILL.md new file mode 100644 index 000000000..f6b376341 --- /dev/null +++ b/.claude/skills/drive-ui/SKILL.md @@ -0,0 +1,117 @@ +--- +name: drive-ui +description: "Drive the Insight web UI in a real browser to see, verify, or capture evidence from any stand — a local compose or kind install, or a shared remote one. Use this whenever the task means LOOKING at the dashboard rather than reading its code: 'check the IC page', 'is that chart still broken', 'screenshot the metrics drilldown', 'reproduce it in the UI', 'grab evidence for a bug', 'open the stand and look at X', or any UI defect you are about to file. Read it BEFORE launching a browser at any remote stand, because the Entra-plus-passkey ones cannot be logged into from a browser you launched, and the wrong acquisition move costs the user a login they cannot complete. Also read it before blaming the UI for a wrong number — the check that settles it is here. The `playwright-cli` skill owns the commands; this skill owns getting an authenticated browser and capturing evidence someone can act on, and hands the issue itself to `file-bug-insight`." +disable-model-invocation: false +user-invocable: true +allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Skill +--- + +# Driving the Insight UI + +Two things about the Insight UI go wrong expensively, and neither is about clicking: **getting a browser that is already authenticated**, and **capturing evidence a reader can act on**. This skill covers both, for local and remote stands alike. The `playwright-cli` skill owns the command surface — snapshots, refs, clicks, screenshots, tracing — so read it for *how* to drive. + +The SPA itself is not in this repo (`src/frontend/` holds only the Helm chart); it lives in `insight-front`. So when you need to read the UI's code rather than run it, look there. + +## Pick the stand, then get a browser + +The right acquisition move depends entirely on the identity provider, so start there — not with `open`. + +| Stand | Auth | Move | +|---|---|---| +| local compose (`http://localhost:3000`) | FakeIdP bypass, or Keycloak | `playwright-cli open` — no login step in fakeidp mode | +| local k8s / kind (`http://localhost:8080` after a port-forward) | Keycloak, username + password | `playwright-cli open --persistent` | +| a shared remote stand behind Keycloak | username + password | `playwright-cli open --persistent` | +| a remote stand behind Microsoft Entra | **passkey** | Attach to the user's own Chrome — never launch | + +### Local: the login is free, the person is not + +`./dev-compose.sh up` defaults to `AUTH_MODE=fakeidp`, which is a bypass — no credentials to type. The UI is at `http://localhost:3000` and the gateway at `http://localhost:8080`. For a kind install there is no Vite origin; port-forward instead (`kubectl port-forward svc/insight-gateway 8080:80`) and use `http://localhost:8080`. + +What actually blocks you is identity, not auth. FakeIdP logs in as `DEV_USER_EMAIL` (`dev@company.nonpresent` by default), and that email has to resolve to a seeded person row or the app rejects the session — a `403` / "person not found" at login, or a `401 urn:insight:error:caller_unresolved` from identity on `/api` calls. The fix is a seed, not a browser move: + +```sh +./dev-compose.sh seed identity # makes DEV_USER_EMAIL resolve to a person +``` + +Read that symptom as "the stand isn't ready", not "the product is broken" — filing it would be a `layer: stand` finding, which `file-bug-insight` will bounce. + +With `AUTH_MODE=keycloak` you get a real username-and-password form, and `--persistent` is worth it so the session survives between runs. + +### Behind a passkey: attach, never launch + +A passkey is bound to the user's real Chrome profile and its platform authenticator. A browser you launch — Playwright's bundled Chromium *or* system Chrome with a fresh profile — cannot complete that login. Asking the user to "just sign in" in your window sends them to a prompt that cannot succeed, and you find out only when the wait times out. So borrow the browser they are already signed into. + +```sh +playwright-cli attach --cdp=chrome # preferred: one checkbox, no restart +playwright-cli attach --extension=chrome # when CDP is blocked or the user prefers it +``` + +If CDP fails with `Could not connect to chrome: DevToolsActivePort file not found`, the user has to open `chrome://inspect/#remote-debugging` and tick **"Allow remote debugging for this browser instance"**. Open the page for them if you like — `open -a "Google Chrome" "chrome://inspect/#remote-debugging"` — but the click is theirs. Their windows, tabs and session all survive, which is why this beats every alternative. + +**Never suggest restarting Chrome with `--remote-debugging-port`.** Chrome 136+ refuses remote debugging on the *default* profile, so the restart either fails or hands you an empty logged-out profile — which is how you end up asking for a passkey you cannot get. + +**Detach, don't close.** While attached you are holding the user's own browser, and `playwright-cli close` closes *their* windows. Use `playwright-cli -s= detach`. `playwright-cli list` tells you which sessions you launched yourself; those are the only ones safe to kill. + +Bank the session once you have it, so later runs skip auth entirely: + +```sh +playwright-cli state-save "$EVIDENCE/stand-state.json" # then state-load next time +``` + +Re-save when it goes stale — the symptom is landing back on a login page. + +## Know when the page is actually ready + +Gate on **the app having rendered**, not on a token appearing in storage. The session lives behind the NGINX BFF, so an `oidc.user` entry in `sessionStorage` may never exist even on a perfectly good session. Polling for one is an invisible hang: the browser is fine, the user is logged in, and you wait for something that is not coming. + +Wait for a real element instead — snapshot and look for a control the page cannot render without: + +```sh +playwright-cli goto "http://localhost:3000/ic//personal" +playwright-cli snapshot --depth=6 +``` + +When a wait does time out, **screenshot before giving up**. A picture of the actual page — "Sign-in failed", an empty state, a 502 — turns a silent hang into a diagnosis in one look. The same reflex applies to any recipe from memory or an old note: routes and the auth stack have changed inside a month before. Check it against the running build, and say so when it turns out stale. + +## Routes worth knowing + +- `/ic//personal` — IC dashboard, personal view (`@` encodes as `%40`) +- `/ic//team` — team view for the same person +- `/metrics` — metrics catalog + +Group cards expose `aria-label="Open details"`, which makes them stable targets: + +```sh +playwright-cli click "getByRole('button', { name: 'Open Git output details' })" +``` + +## Before you blame the UI + +A wrong number on screen is usually not a frontend bug. Two commands settle it, and skipping them is how a gold-view defect gets filed against the SPA: + +```sh +playwright-cli console # client-side errors behind a broken or empty widget +playwright-cli requests # then `request ` for the failing call's status and body +``` + +If the API already returned the wrong value, the layer is `analytics` or below and the UI is just the messenger. `metric-parity` walks it the rest of the way down the medallion. + +## Capture evidence someone can act on + +Write artifacts **outside this repo** — nothing here is gitignored for scratch output, so leftovers surface in someone's `git status` and ride along on the next `git add -A`: + +```sh +EVIDENCE="$(mktemp -d)" # or a path under the session scratchpad dir +``` + +For a UI defect, three artifacts answer the three questions a reader has: + +1. **The offending element, tight** — `playwright-cli screenshot ` on the widget, not the whole page. Answers "where do I look?" +2. **A contrast shot** — the same widget in a state that is correct, or a sibling that behaves. Answers "how do I know it's wrong?" +3. **The page snapshot** — `playwright-cli snapshot --filename="$EVIDENCE/.yml"`, plus `--boxes` when the complaint is about position or overlap. Answers "what was actually on screen?" + +Annotate before capturing rather than describing the element in prose afterwards. Then hand the issue to `file-bug-insight` — and be straight about the constraint it will repeat: GitHub has no API for uploading images to an issue, so the user drags the PNGs in themselves. + +## When you can't get a browser + +The user may not have time to flip a toggle or sign in, and that is a normal outcome rather than a blocker. Say plainly what you could not verify, then get what you can from the code: dashboard composition in `insight-front` is data-driven, which often settles "are these two charts really the same" with no browser at all. A finding grounded in code plus an explicit "not visually confirmed" is honest and useful. A finding that implies visual confirmation it never got is neither. diff --git a/.claude/skills/file-bug-insight/SKILL.md b/.claude/skills/file-bug-insight/SKILL.md new file mode 100644 index 000000000..45f0066ff --- /dev/null +++ b/.claude/skills/file-bug-insight/SKILL.md @@ -0,0 +1,232 @@ +--- +name: file-bug-insight +description: "File an Insight defect as a GitHub issue in constructorfabric/insight — triage against existing issues, gather evidence, localize the fix to the layer that owns it, draft a report that reads in under a minute, confirm, create, and verify the metadata landed. Use whenever the user asks to file/report/raise/log a bug, ticket, defect or issue, and trigger PROACTIVELY once an investigation has converged on 'this is broken and should be recorded' — don't wait for the words 'file a bug'. Also fires on 'log this', 'report it', 'this is broken, make a ticket', 'turn this into an issue', 'we should file two bugs for X and Y'. The repo is PUBLIC, so the default flow is draft → confirm → create and the body must be scrubbed of internal detail. Prefer this over the general `file-bug` skill for anything in the Insight product — dashboards, metrics, connectors, dbt, ClickHouse, identity, the Helm install — since it carries the medallion evidence walk, the layer localization and the live board IDs; the general skill is for a Constructor *platform* defect that belongs in YouTrack." +disable-model-invocation: false +user-invocable: true +allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Skill, Agent, AskUserQuestion +--- + +# File an Insight bug + +Turn an observed defect into an issue someone else can act on in under a minute, carrying the evidence that proves it and a root cause traced from the real code. + +**The tracker is always `constructorfabric/insight` on GitHub** — inside this repo there is no routing decision to make. (A Constructor *platform* bug — APS, Learn, Proctor, a platform stand's auth or navigation — goes to YouTrack instead; that routing lives in the general `file-bug` skill, not here.) + +**The repo is public.** PRs close issues with `Closes #N`, so an issue here is outward-facing. Two consequences: draft → confirm → create is the default flow, and the body gets scrubbed before it goes anywhere. + +## Coming from the QA fleet + +A finding may arrive already carrying a `verdict`, an `existing_issue` and a `layer`. That shape is defined in `.claude/skills/explore-ui/references/finding-contract.md` — read it when the file is there, and don't stall when it isn't: the fields are self-describing, and a defect you found yourself has to clear the same four gates anyway. + +- **`verdict` must be `CONFIRMED`.** An `UNVERIFIED` finding is a hypothesis, and filing one spends a reader's attention on a maybe. Reproduce it yourself first, or hand it to the `qa-finding-refuter` agent where the fleet is installed. +- **`existing_issue` must have been searched.** If it names an issue, comment on that issue instead of filing. If the match is *closed*, say so in the comment — a regression is more urgent than a new bug. +- **`layer: stand` is not a product bug.** A `join_use_nulls` view mismatch, a stale `schema_status` cache, an unseeded connector, a tenant mismatch — these are environment faults. File one only when the deploy path itself is the defect, and then it's a `deploy` bug about the chart or migration, not a metric bug. +- **`layer: unknown` is not filable.** Localize it first — walk the medallion by hand as below, or hand it to `qa-warehouse-analyst` where the fleet is installed — because the assignee and the grooming call both follow the layer. + +## Companion skills + +Each of these owns a slice of the work. Some are still being built out here, so check that one exists before relying on it, and fall back to the hand-run commands in this skill rather than stalling. + +| Skill | Owns | Reach for it when | +|---|---|---| +| `playwright-cli` | the browser command surface — snapshots, refs, clicks, screenshots, console, network | exploring a stand or reproducing any UI defect | +| `drive-ui` | getting an *authenticated* browser on any stand — fakeidp and the `DEV_USER_EMAIL` seed locally, a passkey attach on a remote one — plus the routes and the evidence set | any UI defect, local or remote | +| `metric-parity` | the full bronze → silver → gold walk | localizing a wrong number to a layer | +| `release-verify` | install and seed health | settling "product bug, or empty instance?" | + +One check belongs here rather than in `drive-ui`, because getting it wrong misroutes the bug: before calling a wrong on-screen value a frontend defect, look at the browser console and the API response behind it (`playwright-cli console`, then `requests` and `request `). If the API already returned the wrong number, the layer is `analytics` or below and the UI is only the messenger. + +## Triage — before you gather + +Three checks that routinely change the plan. + +**Search first.** Never file a blind duplicate: + +```sh +gh issue list --repo constructorfabric/insight --state all --search "" --limit 100 +``` + +**Keep the limit high.** Closed issues rank after every open one, so a short window returns open matches only — a search for "threshold" gives 30 open and 0 closed at `--limit 30`, and 57 open plus 43 closed at 100. A closed match is the *more* urgent finding, since it means a regression. + +Search more than once with different vocabulary — the metric key, the field name, the group title, the error code, the user-visible label. Same defect → add your evidence to the existing issue. Genuinely different root cause or fix site → file new and cross-link with a one-line `related to #N` (a bare link, not a "how this differs" writeup — that reads as noise). + +**Product bug, or environment artifact?** A metric that is empty because nothing was seeded or synced is not a product defect. File only what would still be wrong on a correctly populated instance. The cheapest check is the bottom of the medallion: no bronze rows for that connector and window means a seed or sync gap, so stop. (`release-verify` sweeps this for the whole install where it exists.) + +**One bug or several?** One issue per distinct root cause and fix site. Split a shared symptom with different causes; use a fix checklist for several touch-points of the *same* fix. + +## Gather evidence — never write from memory + +Collect first, write second. The evidence must let someone else reproduce this. + +**Artifacts do not go in this repo.** Nothing in this tree is gitignored for scratch output — `scratch/`, `tmp/`, `artifacts/` are merely untracked, so a screenshot or a body file left behind surfaces in someone's `git status` and rides along on the next `git add -A`. Write evidence and the issue body to the session scratchpad directory your environment names, or to a fresh `mktemp -d`; that is what the `--body-file` path below assumes. (`../insight-workspace/scratch/` also works when that checkout sits alongside this one.) + +- **Data / metric bugs** — trace the medallion to where the value *first* goes wrong. Empty **bronze** means a sync or seed artifact, not a bug. Rows in bronze dropped at **silver** is a staging bug. Rows in silver but wrong in **gold** is a model or view bug. Don't file "gold is broken" when the story is "nothing upstream". + ```sh + CH=(docker exec insight-clickhouse clickhouse-client -u insight --password "${CLICKHOUSE_PASSWORD:-insight-local}") + "${CH[@]}" -q "SELECT … FROM insight. WHERE …" # gold — served + "${CH[@]}" -q "SELECT … FROM silver.class__ WHERE …" # silver — dedup / identity + "${CH[@]}" -q "SELECT … FROM bronze_. WHERE …" # bronze — raw ingest + ``` + Every layer needs its database prefix: the client connects to `insight`, so an unqualified `class_*` resolves to the wrong database. The password is required — compose sets `CLICKHOUSE_PASSWORD` with `insight-local` as the default. + For a remote stand: `../insight-workspace/scripts/ch.sh query --target ""` (`ch.sh` lists its targets). Those three queries *are* the three-layer walk; `metric-parity` automates it where it exists. +- **UI bugs** — reproduce it in a browser first (`drive-ui` owns the stand and the browser; `playwright-cli` owns the commands), then lead with a tight annotated shot of the broken widget plus a contrast shot of something that renders correctly. The stand URL belongs in your commands, never in the issue. +- **Pipeline / config bugs with no UI** — the failure signal itself: the exact error and stack, or a row-count contrast that runs the code's own filter (returns 0) against the unfiltered count (>0). **If the failure is silent** — completes "successfully" with zero effect — say so explicitly. That is the key symptom. +- **What the metric is *supposed* to do** lives in `docs/domain/metrics/specs/DESIGN.md` and the model under `src/ingestion/`. Read the intent before calling behaviour wrong. + +## Localize the fix from the actual code + +Naming *where the fix lands* is what makes an issue actionable and routes it to an owner. Read and quote the real code; never infer a formula from a metric's name. Use "X-side, not Y-side" when it disambiguates a layer — *"client-side, not API-side: the API correctly returns 403; the SPA renders the menu entry unconditionally."* + +Gold is defined in two places and they are not interchangeable: the dbt models in `src/ingestion/gold/` materialize the measure observation tables, while the `insight.*` views and marts are created by the migrations in `src/ingestion/scripts/migrations/`. Views get redefined across several migrations — grep them all and read the **latest-timestamped** one before quoting a formula. + +Verify every reference against `main` before linking it; your worktree may differ: + +```sh +gh api "repos/constructorfabric/insight/contents/?ref=main" \ + -H "Accept: application/vnd.github.raw" | grep -nE '' +``` + +## Type and priority + +- **Issue Type = `Bug`** — the native type (`--type "Bug"`), never a `bug` label. +- **Priority is the Insight #40 project *field*, not a label.** Never add `priority:*`. Options: `Blocker` (blocks the next installable release), `High` (meaningful demo features), `Medium` (default). Suggest a level and confirm it. +- **Don't label.** Component, team, release and planning labels are applied during grooming by the people who own that call, and a wrong one routes the bug to the wrong team. Name the owning layer in Root Cause instead, in words. + +Naming the layer is still your job — it just belongs in Root Cause, in words, not in a label: + +| Fleet `layer` | Symptom shape | +|---|---| +| `frontend` | Correct in ClickHouse, renders wrong — axis, colours, series, formatting, null-vs-zero | +| `analytics` | 500s, wrong filter or bucketing in the serving path, wrong measure binding | +| `ingestion` | Wrong at or before gold: bad dedup, dropped rows, wrong view expression, schema drift | +| `ingestion` (identity) | People or org empty / mis-resolved, producer↔consumer mismatch | +| `stand` (deploy path only) | Broken on a fresh install: missing config or wiring, chart or secret gap | + +The layer follows the *fix*, not the symptom — a wrong number from a gold view is an ingestion bug even though it surfaces in the UI. + +## Body template — four headings + +```markdown +## Summary + + +## Steps to Reproduce +1. +2. +3. + +**Expected:** +**Actual:** + + + +## Root Cause +<2–4 sentences. Name the file, view or expression and quote it, each reference linked to `main`.> + +## Notes ← optional, one line (e.g. `related to #N`) +``` + +**No `## Impact` heading.** It restates the Summary in longer words. Affected instances or states go next to the evidence in Steps; a knock-on effect is one line in Notes. Wanting the heading back means the Summary sentence is not carrying its weight. + +Additive when they sharpen the report: an **Examples** table (current-wrong → correct) for a rule, threshold, sign or mapping bug; a **Fix checklist** with each site linked when the fix spans several places. Link code, don't paste strings: `[file.ext#L79](https://github.com/constructorfabric/insight/blob/main/#L79)`. + +## Write plainly + +One idea per sentence. Short declarative lines a tired on-call reader parses on the first pass. If a sentence has more than one comma-joined clause plus a dash-aside, split it. State what happens, then why. + +- ✗ *"Deploy-side, not migration-side: the hook is skipped/lost on a successful fresh install while Helm reports success, leaving the gold layer unbuilt (install-time logs were unavailable — the Job leaves no trace because it never ran)."* +- ✓ *"The post-install hook never runs on a fresh install. Helm still reports success. No hook Job or Pod is ever created, so the gold layer stays unbuilt. The fix is deploy-side — the migration script works when run by hand."* + +**Say each fact once.** Every fact lives in exactly one section. Repetition teaches the reader to skim, and skimming is how the one load-bearing line gets missed. + +**Title = the plain, user-visible symptom.** No metric IDs, table or column symbols, or migration names — those live in the body. Don't append the diagnosis as a trailing clause, don't reach for filler adverbs, and don't use a qualifier the reader can't resolve from the title alone ("after a database migration" — which one?). + +- ✗ *"YouTrack sync is reported failed and its transforms are skipped even though the data synced successfully"* → ✓ *"YouTrack sync is reported failed and its transforms never run"* +- ✗ *"A connector sync fails **outright** when the previous sync is still running"* → ✓ drop `outright`; "fails" already says it. + +**No prescribed fix and no acceptance criteria** — that is the assignee's call. Describe an expected result in plain language and point at the prototype as the source of truth; don't specify exact colours or pixel values. + +## Worked example + +A real filed bug, condensed. Read it for calibration on how little text a complete report needs. + +> **Adding a threshold to a metric makes its threshold list fail permanently** +> +> ## Summary +> Once a metric has its first threshold, every read of that metric's thresholds fails, so thresholds can no longer be viewed, edited or removed. +> +> ## Steps to Reproduce +> 1. Create a metric, then `POST /v1/metrics/{id}/thresholds` with any valid body. +> 2. Read them back: `GET /v1/metrics/{id}/thresholds`. +> 3. Both calls return 500 `application/problem+json`, and the log names the decode: +> ``` +> failed to list thresholds error=Query Error: error occurred while decoding column +> "value": mismatched types; Rust type `core::option::Option` (as SQL type +> `DOUBLE`) is not compatible with SQL type `DECIMAL` +> ``` +> +> **Expected:** 201 with the created threshold, then 200 with the list. +> **Actual:** 500 on the create and on every later read of that metric's thresholds. +> +> The row is inserted despite the 500 — `SELECT field_name, operator, value FROM thresholds` returns it. The write path works and the read path does not, which is why one successful-looking create disables the endpoint for good. Reproduces on a freshly migrated database with no other data. +> +> ## Root Cause +> The `value` column is `DECIMAL(20,6)` ([`m20260414_000001_init.rs#L86`](https://github.com/constructorfabric/insight/blob/main/src/backend/services/analytics/src/migration/m20260414_000001_init.rs#L86)) but the entity maps it to `f64` ([`entities.rs#L39`](https://github.com/constructorfabric/insight/blob/main/src/backend/services/analytics/src/infra/db/entities.rs#L39)), and sqlx-mysql cannot decode `NEWDECIMAL` into `f64` — the `column_type = "Decimal(…)"` annotation on the field does not change how the value is read back. Writes coerce server-side; reads fail during decode, and the handler wraps that as internal ([`handlers.rs#L1070`](https://github.com/constructorfabric/insight/blob/main/src/backend/services/analytics/src/api/handlers.rs#L1070)). The newer admin threshold table reads the same shape safely by casting in raw SQL. +> +> ## Notes +> `metric_threshold` (admin) is unaffected — different read path. + +Three things that example gets right, and they are the ones reports usually miss. The title is a symptom a user could have reported, with the diagnosis left for Root Cause. The "row is inserted despite the 500" line is load-bearing — without it a triager reads a 500 as a flaky write and moves on. And Root Cause stops at *where the defect is*, naming the safe read path as a hint without prescribing the fix. + +## Scrub the body + +Keep **out**: internal hostnames of any kind, the phrase "dev stand", cluster and kube context names, workspace paths (`wiki/…`, `scratch/…`), JWTs, tokens, credentials, and exact data values tied to a real person (genericize `14,753` → "~14.7k"; use `jane.doe@corp.com`). + +Keep **in**: the repo's own code references — file paths, view, table and column names, API routes. Those *are* the product and are what make the bug actionable. + +## Confirm before creating + +Show the title, the type, the priority you propose and the rendered body, then wait — unless the user said "just create it". Creation is not a draft: the repo is public and watchers are notified the moment the issue exists, so a wrong title or an unscrubbed line is already out. This is also where the priority gets settled, since it is your suggestion until the user picks one. + +## Create, board, priority, images + +Write the scrubbed body to a file **outside this repo** — never inline a multi-line body. + +```sh +BODY="$(mktemp -d)/bug-body.md" # or a path under the session scratchpad dir + +# 1. Create — native Type=Bug, NO labels (grooming applies those), NO bug label +gh issue create --repo constructorfabric/insight \ + --type "Bug" --title "" \ + --body-file "$BODY" + +# 2. Add to the Insight board — idempotent; auto-add is unreliable +gh project item-add 40 --owner constructorfabric --url <issue-url> + +# 3. Set the Priority FIELD. Parse with jq, NOT python — issue bodies carry control chars +ITEM=$(gh project item-list 40 --owner constructorfabric --limit 800 --format json \ + | jq -r --argjson n <ISSUE_NUMBER> '.items[] | select((.content.number // -1)==$n) | .id') +gh project item-edit --project-id PVT_kwDOERGOus4Ba9e9 --id "$ITEM" \ + --field-id PVTSSF_lADOERGOus4Ba9e9zhVxXAs \ + --single-select-option-id <Blocker=79628723 | High=0a877460 | Medium=da944a9c> +``` + +Verify those IDs with `gh project field-list 40 --owner constructorfabric` if an edit fails. + +**Images — the honest constraint.** GitHub has **no API to upload an image to an issue**, and `gh` cannot do it either; the web UI uploads via drag-drop. Create the issue with the body, then tell the user to drag the PNGs into the description box. Don't imply they will be attached automatically. For a data or pipeline bug the inline query proof is usually the evidence and no screenshot is needed. + +## Verify what landed + +```sh +gh issue view <n> --repo constructorfabric/insight --json title,labels,body,url +gh api repos/constructorfabric/insight/issues/<n> --jq '.type.name' # → "Bug" +``` + +Confirm: Type is `Bug`; no `bug` or `priority:` label; the body renders and is grep-clean of internal detail. The `item-add` and `item-edit` calls above already report whether the board and Priority field took, so don't re-read them. Report the URL with a one-line summary. + +Don't self-assign, and don't post a status comment unless asked. On this board, moving an issue is a separate decision — *To Verify* means development is done and awaiting validation, *Done* means QA verified it, and publishing that claim is the user's call. diff --git a/.claude/skills/playwright-cli/SKILL.md b/.claude/skills/playwright-cli/SKILL.md new file mode 100644 index 000000000..252e62ce4 --- /dev/null +++ b/.claude/skills/playwright-cli/SKILL.md @@ -0,0 +1,404 @@ +--- +name: playwright-cli +description: Automate browser interactions, test web pages and work with Playwright tests. +allowed-tools: Bash(playwright-cli:*) Bash(npx:*) Bash(npm:*) +--- + +# Browser Automation with playwright-cli + +## Quick start + +```bash +# open new browser +playwright-cli open +# navigate to a page +playwright-cli goto https://playwright.dev +# interact with the page using refs from the snapshot +playwright-cli click e15 +playwright-cli type "page.click" +playwright-cli press Enter +# take a screenshot (rarely used, as snapshot is more common) +playwright-cli screenshot +# close the browser +playwright-cli close +``` + +## Commands + +### Core + +```bash +playwright-cli open +# open and navigate right away +playwright-cli open https://example.com/ +playwright-cli goto https://playwright.dev +playwright-cli type "search query" +playwright-cli click e3 +playwright-cli dblclick e7 +# --submit presses Enter after filling the element +playwright-cli fill e5 "user@example.com" --submit +playwright-cli drag e2 e8 +# drop files or data onto an element (from outside the page) +playwright-cli drop e4 --path=./image.png +playwright-cli drop e4 --data="text/plain=hello world" +playwright-cli hover e4 +playwright-cli select e9 "option-value" +playwright-cli upload ./document.pdf +playwright-cli check e12 +playwright-cli uncheck e12 +playwright-cli snapshot +playwright-cli eval "document.title" +playwright-cli eval "el => el.textContent" e5 +# get element id, class, or any attribute not visible in the snapshot +playwright-cli eval "el => el.id" e5 +playwright-cli eval "el => el.getAttribute('data-testid')" e5 +playwright-cli dialog-accept +playwright-cli dialog-accept "confirmation text" +playwright-cli dialog-dismiss +playwright-cli resize 1920 1080 +playwright-cli close +``` + +### Navigation + +```bash +playwright-cli go-back +playwright-cli go-forward +playwright-cli reload +``` + +### Keyboard + +```bash +playwright-cli press Enter +playwright-cli press ArrowDown +playwright-cli keydown Shift +playwright-cli keyup Shift +``` + +### Mouse + +```bash +playwright-cli mousemove 150 300 +playwright-cli mousedown +playwright-cli mousedown right +playwright-cli mouseup +playwright-cli mouseup right +playwright-cli mousewheel 0 100 +``` + +### Save as + +```bash +playwright-cli screenshot +playwright-cli screenshot e5 +playwright-cli screenshot --filename=page.png +playwright-cli pdf --filename=page.pdf +``` + +### Tabs + +```bash +playwright-cli tab-list +playwright-cli tab-new +playwright-cli tab-new https://example.com/page +playwright-cli tab-close +playwright-cli tab-close 2 +playwright-cli tab-select 0 +``` + +### Storage + +```bash +playwright-cli state-save +playwright-cli state-save auth.json +playwright-cli state-load auth.json + +# Cookies +playwright-cli cookie-list +playwright-cli cookie-list --domain=example.com +playwright-cli cookie-get session_id +playwright-cli cookie-set session_id abc123 +playwright-cli cookie-set session_id abc123 --domain=example.com --httpOnly --secure +playwright-cli cookie-delete session_id +playwright-cli cookie-clear + +# LocalStorage +playwright-cli localstorage-list +playwright-cli localstorage-get theme +playwright-cli localstorage-set theme dark +playwright-cli localstorage-delete theme +playwright-cli localstorage-clear + +# SessionStorage +playwright-cli sessionstorage-list +playwright-cli sessionstorage-get step +playwright-cli sessionstorage-set step 3 +playwright-cli sessionstorage-delete step +playwright-cli sessionstorage-clear +``` + +### Network + +```bash +playwright-cli route "**/*.jpg" --status=404 +playwright-cli route "https://api.example.com/**" --body='{"mock": true}' +playwright-cli route-list +playwright-cli unroute "**/*.jpg" +playwright-cli unroute +``` + +### DevTools + +```bash +playwright-cli console +playwright-cli console warning +playwright-cli requests +playwright-cli request 5 +playwright-cli run-code "async page => await page.context().grantPermissions(['geolocation'])" +playwright-cli run-code --filename=script.js +playwright-cli tracing-start +playwright-cli tracing-stop +playwright-cli video-start video.webm +playwright-cli video-chapter "Chapter Title" --description="Details" --duration=2000 +playwright-cli video-stop + +# annotate each subsequent action (click, type, ...) with a callout naming the action and highlighting the target +playwright-cli video-show-actions --duration=600 --position=top-right +playwright-cli video-hide-actions + +# launch the dashboard for UI review / design feedback — user annotates the page, you receive the annotated screenshot, snapshot, and notes +playwright-cli show --annotate + +# generate a Playwright locator for an element from its ref or selector +playwright-cli generate-locator e5 --raw + +# show a persistent highlight overlay for an element, optionally with a custom style +playwright-cli highlight e5 +playwright-cli highlight e5 --style="outline: 3px dashed red" +# hide a single element highlight, or all page highlights when no target is given +playwright-cli highlight e5 --hide +playwright-cli highlight --hide +``` + +## Raw output + +The global `--raw` option strips page status, generated code, and snapshot sections from the output, returning only the result value. Use it to pipe command output into other tools. Commands that don't produce output return nothing. + +```bash +playwright-cli --raw eval "JSON.stringify(performance.timing)" | jq '.loadEventEnd - .navigationStart' +playwright-cli --raw eval "JSON.stringify([...document.querySelectorAll('a')].map(a => a.href))" > links.json +playwright-cli --raw snapshot > before.yml +playwright-cli click e5 +playwright-cli --raw snapshot > after.yml +diff before.yml after.yml +TOKEN=$(playwright-cli --raw cookie-get session_id) +playwright-cli --raw localstorage-get theme +``` + +For structured output wrapping every reply as JSON, pass --json +```bash +playwright-cli list --json +``` + +## Open parameters +```bash +# Use specific browser when creating session +playwright-cli open --browser=chrome +playwright-cli open --browser=firefox +playwright-cli open --browser=webkit +playwright-cli open --browser=msedge + +# Use persistent profile (by default profile is in-memory) +playwright-cli open --persistent +# Use persistent profile with custom directory +playwright-cli open --profile=/path/to/profile + +# Connect to browser via Playwright Extension +playwright-cli attach --extension=chrome + +# Connect to a running Chrome or Edge by channel name +playwright-cli attach --cdp=chrome +playwright-cli attach --cdp=msedge + +# Connect to a running browser via CDP endpoint +playwright-cli attach --cdp=http://localhost:9222 + +# Start with config file +playwright-cli open --config=my-config.json + +# Close the browser +playwright-cli close +# Detach from an attached browser (leaves the external browser running) +playwright-cli -s=msedge detach +# Delete user data for the default session +playwright-cli delete-data +``` + +## URLs with `&` on Windows + +On Windows, `cmd.exe` and PowerShell treat `&` as a command separator, so URLs with multiple query parameters get truncated before `playwright-cli` runs. Escape `&` with `^&` in `cmd.exe`, or use `--%` in PowerShell: + +```batch +playwright-cli goto "https://example.com/?a=1^&b=2" +``` + +```powershell +playwright-cli --% goto "https://example.com/?a=1&b=2" +``` + +## Snapshots + +After each command, playwright-cli provides a snapshot of the current browser state. + +```bash +> playwright-cli goto https://example.com +### Page +- Page URL: https://example.com/ +- Page Title: Example Domain +### Snapshot +[Snapshot](.playwright-cli/page-2026-02-14T19-22-42-679Z.yml) +``` + +You can also take a snapshot on demand using `playwright-cli snapshot` command. All the options below can be combined as needed. + +```bash +# default - save to a file with timestamp-based name +playwright-cli snapshot + +# save to file, use when snapshot is a part of the workflow result +playwright-cli snapshot --filename=after-click.yaml + +# snapshot an element instead of the whole page +playwright-cli snapshot "#main" + +# limit snapshot depth for efficiency, take a partial snapshot afterwards +playwright-cli snapshot --depth=4 +playwright-cli snapshot e34 + +# include each element's bounding box as [box=x,y,width,height] +playwright-cli snapshot --boxes +``` + +## Targeting elements + +By default, use refs from the snapshot to interact with page elements. + +```bash +# get snapshot with refs +playwright-cli snapshot + +# interact using a ref +playwright-cli click e15 +``` + +You can also use css selectors or Playwright locators. + +```bash +# css selector +playwright-cli click "#main > button.submit" + +# role locator +playwright-cli click "getByRole('button', { name: 'Submit' })" + +# test id +playwright-cli click "getByTestId('submit-button')" +``` + +## Browser Sessions + +```bash +# create new browser session named "mysession" with persistent profile +playwright-cli -s=mysession open example.com --persistent +# same with manually specified profile directory (use when requested explicitly) +playwright-cli -s=mysession open example.com --profile=/path/to/profile +playwright-cli -s=mysession click e6 +playwright-cli -s=mysession close # stop a named browser +playwright-cli -s=mysession delete-data # delete user data for persistent session + +playwright-cli list +# Close all browsers +playwright-cli close-all +# Forcefully kill all browser processes +playwright-cli kill-all +``` + +## Installation + +If global `playwright-cli` command is not available, try a local version via `npx playwright-cli`: + +```bash +npx --no-install playwright-cli --version +``` + +When local version is available, use `npx playwright-cli` in all commands. Otherwise, install `playwright-cli` as a global command: + +```bash +npm install -g @playwright/cli@latest +``` + +## Example: Form submission + +```bash +playwright-cli open https://example.com/form +playwright-cli snapshot + +playwright-cli fill e1 "user@example.com" +playwright-cli fill e2 "password123" +playwright-cli click e3 +playwright-cli snapshot +playwright-cli close +``` + +## Example: Multi-tab workflow + +```bash +playwright-cli open https://example.com +playwright-cli tab-new https://example.com/other +playwright-cli tab-list +playwright-cli tab-select 0 +playwright-cli snapshot +playwright-cli close +``` + +## Example: Debugging with DevTools + +```bash +playwright-cli open https://example.com +playwright-cli click e4 +playwright-cli fill e7 "test" +playwright-cli console +playwright-cli requests +playwright-cli close +``` + +```bash +playwright-cli open https://example.com +playwright-cli tracing-start +playwright-cli click e4 +playwright-cli fill e7 "test" +playwright-cli tracing-stop +playwright-cli close +``` + +## Example: Interactive session + +Ask the user for UI review or design feedback. The user draws boxes on the live page and types comments; you receive the annotated screenshot, the snapshot of the marked region, and the user's notes. Use this whenever the user asks for "UI review", "design feedback", or to "ask the user what they think / want / mean": + +```bash +playwright-cli open https://example.com +playwright-cli show --annotate +``` + +## Specific tasks + +* **Running and Debugging Playwright tests** [references/playwright-tests.md](references/playwright-tests.md) +* **Request mocking** [references/request-mocking.md](references/request-mocking.md) +* **Running Playwright code** [references/running-code.md](references/running-code.md) +* **Browser session management** [references/session-management.md](references/session-management.md) +* **Spec-driven testing (plan / generate / heal)** [references/spec-driven-testing.md](references/spec-driven-testing.md) +* **Storage state (cookies, localStorage)** [references/storage-state.md](references/storage-state.md) +* **Test generation** [references/test-generation.md](references/test-generation.md) +* **Tracing** [references/tracing.md](references/tracing.md) +* **Video recording** [references/video-recording.md](references/video-recording.md) +* **Inspecting element attributes** [references/element-attributes.md](references/element-attributes.md) diff --git a/.claude/skills/playwright-cli/references/element-attributes.md b/.claude/skills/playwright-cli/references/element-attributes.md new file mode 100644 index 000000000..4e9fa6b99 --- /dev/null +++ b/.claude/skills/playwright-cli/references/element-attributes.md @@ -0,0 +1,23 @@ +# Inspecting Element Attributes + +When the snapshot doesn't show an element's `id`, `class`, `data-*` attributes, or other DOM properties, use `eval` to inspect them. + +## Examples + +```bash +playwright-cli snapshot +# snapshot shows a button as e7 but doesn't reveal its id or data attributes + +# get the element's id +playwright-cli eval "el => el.id" e7 + +# get all CSS classes +playwright-cli eval "el => el.className" e7 + +# get a specific attribute +playwright-cli eval "el => el.getAttribute('data-testid')" e7 +playwright-cli eval "el => el.getAttribute('aria-label')" e7 + +# get a computed style property +playwright-cli eval "el => getComputedStyle(el).display" e7 +``` diff --git a/.claude/skills/playwright-cli/references/playwright-tests.md b/.claude/skills/playwright-cli/references/playwright-tests.md new file mode 100644 index 000000000..bec2ec90e --- /dev/null +++ b/.claude/skills/playwright-cli/references/playwright-tests.md @@ -0,0 +1,39 @@ +# Running Playwright Tests + +To run Playwright tests, use the `npx playwright test` command, or a package manager script. To avoid opening the interactive html report, use `PLAYWRIGHT_HTML_OPEN=never` environment variable. + +```bash +# Run all tests +PLAYWRIGHT_HTML_OPEN=never npx playwright test + +# Run all tests through a custom npm script +PLAYWRIGHT_HTML_OPEN=never npm run special-test-command +``` + +# Debugging Playwright Tests + +To debug a failing Playwright test, run it with `--debug=cli` option. This command will pause the test at the start and print the debugging instructions. + +**IMPORTANT**: run the command in the background and check the output until "Debugging Instructions" is printed. Make sure to stop the command after you have finished. + +Once instructions containing a session name are printed, use `playwright-cli` to attach the session and explore the page. + +```bash +# Run the test +PLAYWRIGHT_HTML_OPEN=never npx playwright test --debug=cli +# ... +# ... debugging instructions for "tw-abcdef" session ... +# ... + +# Attach to the test +playwright-cli attach tw-abcdef +``` + +Keep the test running in the background while you explore and look for a fix. +The test is paused at the start, so you should step over or pause at a particular location +where the problem is most likely to be. + +Every action you perform with `playwright-cli` generates corresponding Playwright TypeScript code. +This code appears in the output and can be copied directly into the test. Most of the time, a specific locator or an expectation should be updated, but it could also be a bug in the app. Use your judgement. + +After fixing the test, stop the background test run. Rerun to check that test passes. diff --git a/.claude/skills/playwright-cli/references/request-mocking.md b/.claude/skills/playwright-cli/references/request-mocking.md new file mode 100644 index 000000000..9005fda67 --- /dev/null +++ b/.claude/skills/playwright-cli/references/request-mocking.md @@ -0,0 +1,87 @@ +# Request Mocking + +Intercept, mock, modify, and block network requests. + +## CLI Route Commands + +```bash +# Mock with custom status +playwright-cli route "**/*.jpg" --status=404 + +# Mock with JSON body +playwright-cli route "**/api/users" --body='[{"id":1,"name":"Alice"}]' --content-type=application/json + +# Mock with custom headers +playwright-cli route "**/api/data" --body='{"ok":true}' --header="X-Custom: value" + +# Remove headers from requests +playwright-cli route "**/*" --remove-header=cookie,authorization + +# List active routes +playwright-cli route-list + +# Remove a route or all routes +playwright-cli unroute "**/*.jpg" +playwright-cli unroute +``` + +## URL Patterns + +``` +**/api/users - Exact path match +**/api/*/details - Wildcard in path +**/*.{png,jpg,jpeg} - Match file extensions +**/search?q=* - Match query parameters +``` + +## Advanced Mocking with run-code + +For conditional responses, request body inspection, response modification, or delays: + +### Conditional Response Based on Request + +```bash +playwright-cli run-code "async page => { + await page.route('**/api/login', route => { + const body = route.request().postDataJSON(); + if (body.username === 'admin') { + route.fulfill({ body: JSON.stringify({ token: 'mock-token' }) }); + } else { + route.fulfill({ status: 401, body: JSON.stringify({ error: 'Invalid' }) }); + } + }); +}" +``` + +### Modify Real Response + +```bash +playwright-cli run-code "async page => { + await page.route('**/api/user', async route => { + const response = await route.fetch(); + const json = await response.json(); + json.isPremium = true; + await route.fulfill({ response, json }); + }); +}" +``` + +### Simulate Network Failures + +```bash +playwright-cli run-code "async page => { + await page.route('**/api/offline', route => route.abort('internetdisconnected')); +}" +# Options: connectionrefused, timedout, connectionreset, internetdisconnected +``` + +### Delayed Response + +```bash +playwright-cli run-code "async page => { + await page.route('**/api/slow', async route => { + await new Promise(r => setTimeout(r, 3000)); + route.fulfill({ body: JSON.stringify({ data: 'loaded' }) }); + }); +}" +``` diff --git a/.claude/skills/playwright-cli/references/running-code.md b/.claude/skills/playwright-cli/references/running-code.md new file mode 100644 index 000000000..98b541fb5 --- /dev/null +++ b/.claude/skills/playwright-cli/references/running-code.md @@ -0,0 +1,241 @@ +# Running Custom Playwright Code + +Use `run-code` to execute arbitrary Playwright code for advanced scenarios not covered by CLI commands. + +## Syntax + +```bash +playwright-cli run-code "async page => { + // Your Playwright code here + // Access page.context() for browser context operations +}" +``` + +You can also load the function from a file: + +```bash +playwright-cli run-code --filename=./my-script.js +``` + + +The code must be a single function expression, it is wrapped in `(...)` and evaluated. +import/export/require syntax is not supported. + +## Geolocation + +```bash +# Grant geolocation permission and set location +playwright-cli run-code "async page => { + await page.context().grantPermissions(['geolocation']); + await page.context().setGeolocation({ latitude: 37.7749, longitude: -122.4194 }); +}" + +# Set location to London +playwright-cli run-code "async page => { + await page.context().grantPermissions(['geolocation']); + await page.context().setGeolocation({ latitude: 51.5074, longitude: -0.1278 }); +}" + +# Clear geolocation override +playwright-cli run-code "async page => { + await page.context().clearPermissions(); +}" +``` + +## Permissions + +```bash +# Grant multiple permissions +playwright-cli run-code "async page => { + await page.context().grantPermissions([ + 'geolocation', + 'notifications', + 'camera', + 'microphone' + ]); +}" + +# Grant permissions for specific origin +playwright-cli run-code "async page => { + await page.context().grantPermissions(['clipboard-read'], { + origin: 'https://example.com' + }); +}" +``` + +## Media Emulation + +```bash +# Emulate dark color scheme +playwright-cli run-code "async page => { + await page.emulateMedia({ colorScheme: 'dark' }); +}" + +# Emulate light color scheme +playwright-cli run-code "async page => { + await page.emulateMedia({ colorScheme: 'light' }); +}" + +# Emulate reduced motion +playwright-cli run-code "async page => { + await page.emulateMedia({ reducedMotion: 'reduce' }); +}" + +# Emulate print media +playwright-cli run-code "async page => { + await page.emulateMedia({ media: 'print' }); +}" +``` + +## Wait Strategies + +```bash +# Wait for network idle +playwright-cli run-code "async page => { + await page.waitForLoadState('networkidle'); +}" + +# Wait for specific element +playwright-cli run-code "async page => { + await page.locator('.loading').waitFor({ state: 'hidden' }); +}" + +# Wait for function to return true +playwright-cli run-code "async page => { + await page.waitForFunction(() => window.appReady === true); +}" + +# Wait with timeout +playwright-cli run-code "async page => { + await page.locator('.result').waitFor({ timeout: 10000 }); +}" +``` + +## Frames and Iframes + +```bash +# Work with iframe +playwright-cli run-code "async page => { + const frame = page.locator('iframe#my-iframe').contentFrame(); + await frame.locator('button').click(); +}" + +# Get all frames +playwright-cli run-code "async page => { + const frames = page.frames(); + return frames.map(f => f.url()); +}" +``` + +## File Downloads + +```bash +# Handle file download +playwright-cli run-code "async page => { + const downloadPromise = page.waitForEvent('download'); + await page.getByRole('link', { name: 'Download' }).click(); + const download = await downloadPromise; + await download.saveAs('./downloaded-file.pdf'); + return download.suggestedFilename(); +}" +``` + +## Clipboard + +```bash +# Read clipboard (requires permission) +playwright-cli run-code "async page => { + await page.context().grantPermissions(['clipboard-read']); + return await page.evaluate(() => navigator.clipboard.readText()); +}" + +# Write to clipboard +playwright-cli run-code "async page => { + await page.evaluate(text => navigator.clipboard.writeText(text), 'Hello clipboard!'); +}" +``` + +## Page Information + +```bash +# Get page title +playwright-cli run-code "async page => { + return await page.title(); +}" + +# Get current URL +playwright-cli run-code "async page => { + return page.url(); +}" + +# Get page content +playwright-cli run-code "async page => { + return await page.content(); +}" + +# Get viewport size +playwright-cli run-code "async page => { + return page.viewportSize(); +}" +``` + +## JavaScript Execution + +```bash +# Execute JavaScript and return result +playwright-cli run-code "async page => { + return await page.evaluate(() => { + return { + userAgent: navigator.userAgent, + language: navigator.language, + cookiesEnabled: navigator.cookieEnabled + }; + }); +}" + +# Pass arguments to evaluate +playwright-cli run-code "async page => { + const multiplier = 5; + return await page.evaluate(m => document.querySelectorAll('li').length * m, multiplier); +}" +``` + +## Error Handling + +```bash +# Try-catch in run-code +playwright-cli run-code "async page => { + try { + await page.getByRole('button', { name: 'Submit' }).click({ timeout: 1000 }); + return 'clicked'; + } catch (e) { + return 'element not found'; + } +}" +``` + +## Complex Workflows + +```bash +# Login and save state +playwright-cli run-code "async page => { + await page.goto('https://example.com/login'); + await page.getByRole('textbox', { name: 'Email' }).fill('user@example.com'); + await page.getByRole('textbox', { name: 'Password' }).fill('secret'); + await page.getByRole('button', { name: 'Sign in' }).click(); + await page.waitForURL('**/dashboard'); + await page.context().storageState({ path: 'auth.json' }); + return 'Login successful'; +}" + +# Scrape data from multiple pages +playwright-cli run-code "async page => { + const results = []; + for (let i = 1; i <= 3; i++) { + await page.goto(\`https://example.com/page/\${i}\`); + const items = await page.locator('.item').allTextContents(); + results.push(...items); + } + return results; +}" +``` diff --git a/.claude/skills/playwright-cli/references/session-management.md b/.claude/skills/playwright-cli/references/session-management.md new file mode 100644 index 000000000..bf39acd5f --- /dev/null +++ b/.claude/skills/playwright-cli/references/session-management.md @@ -0,0 +1,225 @@ +# Browser Session Management + +Run multiple isolated browser sessions concurrently with state persistence. + +## Named Browser Sessions + +Use `-s` flag to isolate browser contexts: + +```bash +# Browser 1: Authentication flow +playwright-cli -s=auth open https://app.example.com/login + +# Browser 2: Public browsing (separate cookies, storage) +playwright-cli -s=public open https://example.com + +# Commands are isolated by browser session +playwright-cli -s=auth fill e1 "user@example.com" +playwright-cli -s=public snapshot +``` + +## Browser Session Isolation Properties + +Each browser session has independent: +- Cookies +- LocalStorage / SessionStorage +- IndexedDB +- Cache +- Browsing history +- Open tabs + +## Browser Session Commands + +```bash +# List all browser sessions +playwright-cli list + +# Stop a browser session (close the browser) +playwright-cli close # stop the default browser +playwright-cli -s=mysession close # stop a named browser + +# Stop all browser sessions +playwright-cli close-all + +# Forcefully kill all daemon processes (for stale/zombie processes) +playwright-cli kill-all + +# Delete browser session user data (profile directory) +playwright-cli delete-data # delete default browser data +playwright-cli -s=mysession delete-data # delete named browser data +``` + +## Environment Variable + +Set a default browser session name via environment variable: + +```bash +export PLAYWRIGHT_CLI_SESSION="mysession" +playwright-cli open example.com # Uses "mysession" automatically +``` + +## Common Patterns + +### Concurrent Scraping + +```bash +#!/bin/bash +# Scrape multiple sites concurrently + +# Start all browsers +playwright-cli -s=site1 open https://site1.com & +playwright-cli -s=site2 open https://site2.com & +playwright-cli -s=site3 open https://site3.com & +wait + +# Take snapshots from each +playwright-cli -s=site1 snapshot +playwright-cli -s=site2 snapshot +playwright-cli -s=site3 snapshot + +# Cleanup +playwright-cli close-all +``` + +### A/B Testing Sessions + +```bash +# Test different user experiences +playwright-cli -s=variant-a open "https://app.com?variant=a" +playwright-cli -s=variant-b open "https://app.com?variant=b" + +# Compare +playwright-cli -s=variant-a screenshot +playwright-cli -s=variant-b screenshot +``` + +### Persistent Profile + +By default, browser profile is kept in memory only. Use `--persistent` flag on `open` to persist the browser profile to disk: + +```bash +# Use persistent profile (auto-generated location) +playwright-cli open https://example.com --persistent + +# Use persistent profile with custom directory +playwright-cli open https://example.com --profile=/path/to/profile +``` + +## Attaching to a Running Browser + +Use `attach` to connect to a browser that is already running, instead of launching a new one. + +### Attach by channel name + +Connect to a running Chrome or Edge instance by its channel name. The browser must have remote debugging enabled — navigate to `chrome://inspect/#remote-debugging` in the target browser and check "Allow remote debugging for this browser instance". + +```bash +# Attach to Chrome +playwright-cli attach --cdp=chrome + +# Attach to Chrome Canary +playwright-cli attach --cdp=chrome-canary + +# Attach to Microsoft Edge +playwright-cli attach --cdp=msedge + +# Attach to Edge Dev +playwright-cli attach --cdp=msedge-dev +``` + +Supported channels: `chrome`, `chrome-beta`, `chrome-dev`, `chrome-canary`, `msedge`, `msedge-beta`, `msedge-dev`, `msedge-canary`. + +When `--session` is not provided, the session is named after the channel (e.g. `--cdp=msedge` creates a session called `msedge`), so parallel attaches to Chrome and Edge don't collide on `default`. Pass `--session=<name>` to override. + +### Attach via CDP endpoint + +Connect to a browser that exposes a Chrome DevTools Protocol endpoint: + +```bash +playwright-cli attach --cdp=http://localhost:9222 +``` + +### Attach via browser extension + +Connect to a browser with the Playwright extension installed: + +```bash +playwright-cli attach --extension +``` + +### Detach + +Tear down an attached session without affecting the external browser: + +```bash +# Detach the default attached session +playwright-cli detach + +# Detach a specific attached session +playwright-cli -s=msedge detach +``` + +`detach` only works on sessions created via `attach`. For sessions created via `open`, use `close`. + +## Default Browser Session + +When `-s` is omitted, commands use the default browser session: + +```bash +# These use the same default browser session +playwright-cli open https://example.com +playwright-cli snapshot +playwright-cli close # Stops default browser +``` + +## Browser Session Configuration + +Configure a browser session with specific settings when opening: + +```bash +# Open with config file +playwright-cli open https://example.com --config=.playwright/my-cli.json + +# Open with specific browser +playwright-cli open https://example.com --browser=firefox + +# Open in headed mode +playwright-cli open https://example.com --headed + +# Open with persistent profile +playwright-cli open https://example.com --persistent +``` + +## Best Practices + +### 1. Name Browser Sessions Semantically + +```bash +# GOOD: Clear purpose +playwright-cli -s=github-auth open https://github.com +playwright-cli -s=docs-scrape open https://docs.example.com + +# AVOID: Generic names +playwright-cli -s=s1 open https://github.com +``` + +### 2. Always Clean Up + +```bash +# Stop browsers when done +playwright-cli -s=auth close +playwright-cli -s=scrape close + +# Or stop all at once +playwright-cli close-all + +# If browsers become unresponsive or zombie processes remain +playwright-cli kill-all +``` + +### 3. Delete Stale Browser Data + +```bash +# Remove old browser data to free disk space +playwright-cli -s=oldsession delete-data +``` diff --git a/.claude/skills/playwright-cli/references/spec-driven-testing.md b/.claude/skills/playwright-cli/references/spec-driven-testing.md new file mode 100644 index 000000000..336dbfc13 --- /dev/null +++ b/.claude/skills/playwright-cli/references/spec-driven-testing.md @@ -0,0 +1,305 @@ +# Spec-driven testing (plan → generate → heal) + +End-to-end workflow for authoring and maintaining Playwright tests using `playwright-cli`. The three sections below can be used independently: + +- **Planning** — explore the app, produce a spec file describing what to test. +- **Generate** — turn a spec into Playwright test files. Update the spec if it's vague or stale. +- **Heal** — diagnose failing tests, fix the code, reconcile the spec with reality. + +All three lean on the same mechanic: run `npx playwright test --debug=cli` in the background, then `playwright-cli attach tw-XXXX` to drive the paused page interactively. See [playwright-tests.md](playwright-tests.md) for the debug/attach mechanics and [test-generation.md](test-generation.md) for how every `playwright-cli` action emits Playwright TypeScript. + +--- + +## 1. Planning + +Goal: produce a spec file (e.g. `specs/<feature>.plan.md`) that enumerates the scenarios to test. **Always** write the spec to a file. + +### 1.1 Prerequisite: workspace + +Check the workspace has Playwright installed before anything else: + +```bash +# Either of these confirms a workspace: +test -f playwright.config.ts || test -f playwright.config.js +npx --no-install playwright --version +``` + +If there is no Playwright install, bootstrap one and let the user pick the defaults: + +```bash +npm init playwright@latest +``` + +### 1.2 Prerequisite: seed test + +A **seed test** is a minimal test that lands the page in the state every scenario starts from: navigation to the app, any required login, feature flags, etc. Scenarios assume a fresh start *after* the seed. `--debug=cli` pauses *inside* this test, so the seed is where every planning and generation session begins. + +Minimum viable seed: + +```ts +// tests/seed.spec.ts +import { test } from '@playwright/test'; + +test('seed', async ({ page }) => { + await page.goto('https://example.com/'); +}); +``` + +Preferred — push navigation into a fixture so scenario tests reuse it: + +```ts +// tests/fixtures.ts +import { test as baseTest } from '@playwright/test'; +export { expect } from '@playwright/test'; + +export const test = baseTest.extend({ + page: async ({ page }, use) => { + await page.goto('https://example.com/'); + await use(page); + }, +}); +``` + +```ts +// tests/seed.spec.ts +import { test } from './fixtures'; + +test('seed', async ({ page }) => { + // Fixture already navigates. This empty body tells agents where to start. +}); +``` + +If no seed exists, create one that at least navigates to the app. + +### 1.3 Explore the app + +Launch the app via the seed in the background and attach: + +```bash +PLAYWRIGHT_HTML_OPEN=never npx playwright test tests/seed.spec.ts --debug=cli +# wait for "Debugging Instructions" and the session name tw-XXXX +playwright-cli attach tw-XXXX +``` + +Resume so the seed runs, then probe the app: + +```bash +playwright-cli resume # resume so that seed test runs fully +playwright-cli snapshot # inventory of interactive elements +playwright-cli click e5 # follow a flow +playwright-cli eval "location.href" # read URL / state +playwright-cli show --annotate # ask the user to point at something +``` + +Map out: + +- Interactive surfaces (forms, buttons, lists, filters, modals). +- Primary user journeys end-to-end. +- Edge cases: empty states, validation errors, very long input, boundary values. +- Persistence: reload, local/session storage, URL fragments. +- Navigation: which controls change the URL, back/forward behaviour. + +**Important**: Do not just open the app url with playwright-cli, always go through the test to capture any custom setup done there. +**Important**: Stop the background test when done exploring. + +### 1.4 Write the spec file + +Save under `specs/<feature>.plan.md`. Use this structure: + +```markdown +# <Feature> Test Plan + +## Application Overview + +<One paragraph describing what the feature does and why it matters.> + +## Test Scenarios + +### 1. <Group Name> + +**Seed:** `tests/seed.spec.ts` + +#### 1.1. <kebab-case-scenario-name> + +**File:** `tests/<group>/<kebab-case-scenario-name>.spec.ts` + +**Steps:** + 1. <Concrete user step> + - expect: <observable outcome> + - expect: <another observable outcome> + 2. <Next step> + - expect: <outcome> + +#### 1.2. <next-scenario> +... + +### 2. <Next Group> + +**Seed:** `tests/seed.spec.ts` +... +``` + +Guidelines: + +- Each scenario is independent and starts from the seed's fresh state — never chain scenarios. +- Scenario names are kebab-case and match the test file name (`should-add-single-todo` → `should-add-single-todo.spec.ts`). +- Cover happy path, edge cases, validation, negative flows, persistence. +- Write steps at the user level ("Type 'Buy milk' into the input"), not the API level ("call `fill`"). +- Put observable outcomes in `- expect:` bullets; each becomes an assertion during generation. + +--- + +## 2. Generate + +Goal: take a spec file and produce Playwright test files. Optionally update the spec if it has drifted. + +### 2.1 Inputs + +- **Spec file**, e.g. `specs/basic-operations.plan.md`. +- **Target**: either a single scenario (e.g. `1.2`), a whole group (`1`), or all. +- **Seed file**, read from the `**Seed:**` line of the scenario's group. + +### 2.2 Generate one scenario + +For each target scenario, in sequence (never in parallel — scenarios share the seed session): + +```bash +PLAYWRIGHT_HTML_OPEN=never npx playwright test <seed-file> --debug=cli # background +playwright-cli attach tw-XXXX +# resume +``` + +**Do not** just open the app url with playwright-cli, always go through the test to capture any custom setup done there. + +Walk the scenario's `Steps:` one by one with `playwright-cli`, treating the spec as the plan and the live app as the source of truth. If a step is vague ("click the button" — which button?), references an element that no longer exists, or contradicts the app's actual behaviour, use your judgement: update the spec to match what the app really does, then keep going. Editing the spec mid-generation is expected. + +Every action prints the equivalent Playwright TypeScript (see [test-generation.md](test-generation.md)): + +```bash +playwright-cli snapshot # find refs +playwright-cli fill e3 "John Doe" # -> page.getByRole('textbox', {...}).fill(...) +playwright-cli press Enter +playwright-cli click e7 +``` + +For each `- expect:` bullet, add an explicit assertion. See [test-generation.md](test-generation.md) for details. + +Collect the generated code and write the test file at the path given in the spec: + +```ts +// spec: specs/basic-operations.plan.md +// seed: tests/seed.spec.ts +import { test, expect } from './fixtures'; // or '@playwright/test' if no fixtures file + +test.describe('Signing in and out', () => { + test('should sign in', async ({ page }) => { + // 1. Navigate to the application + // (handled by the seed fixture) + + // 2. Type 'John Doe' into the username field + await page.getByRole('textbox', { name: 'username' }).fill('John Doe'); + + // 3. Type password + await page.getByRole('textbox', { name: 'password' }).fill('TestPassword'); + + // 4. Press Enter to submit + await page.getByRole('textbox', { name: 'password' }).press('Enter'); + + await expect(page.getByRole('heading')).toContainText('Welcome, John Doe!'); + }); +}); +``` + +Rules: + +- **One test per file.** File path, describe name, and test name come verbatim from the spec (minus the ordinal). +- Prefix each numbered step with a `// N. <step text>` comment before its actions. +- Use the describe group name verbatim from the spec (no `1.` ordinal). +- Import from `./fixtures` if the project has one; otherwise `@playwright/test`. +- **Important**: close the CLI session and stop the background test before moving to the next scenario. + +### 2.3 Generate multiple scenarios + +Loop 2.2 over the targeted scenarios one at a time, restarting the seed between each so every test starts from a clean page. This is safe to parallelise due to unique generated session names - just make sure each test run is stopped. + +### 2.4 Run generated tests + +After generation, run the new tests once: + +```bash +PLAYWRIGHT_HTML_OPEN=never npx playwright test tests/<group>/<scenario>.spec.ts +``` + +Any failure goes to Section 3. + +--- + +## 3. Heal + +Goal: fix failing tests, and update the spec if the app's intended behaviour changed. + +### 3.1 Find failing tests + +```bash +PLAYWRIGHT_HTML_OPEN=never npx playwright test +``` + +Record the list of failing `<file>:<line>` entries and process them one at a time. Do not attempt parallel fixes — shared state and the single CLI session make that fragile. + +### 3.2 Debug one failure + +Run the single failing test in debug mode in the background, then attach: + +```bash +PLAYWRIGHT_HTML_OPEN=never npx playwright test tests/<group>/<scenario>.spec.ts:<line> --debug=cli +# wait for "Debugging Instructions" and the tw-XXXX session name +playwright-cli attach tw-XXXX +``` + +The test is paused at the start. Step forward or run to until just before the failing action or assertion, then diagnose: + +```bash +playwright-cli snapshot # did the element change / move / rename? +playwright-cli console # app-side errors? +playwright-cli requests # failed request? wrong payload? +playwright-cli show --annotate # ask the user to point somewhere +``` + +Common causes: selector drift, new wrapper element, label/ARIA rename, timing (transition, async load), assertion text updated in the app, test data leaking between runs. + +Rehearse the corrected interaction with `playwright-cli` — the generated code in the output is what you paste back into the test. + +### 3.3 Apply the fix + +Edit the test file: update the locator, assertion, step order, or inputs to match the corrected behaviour. Stop the background debug run. Rerun the single test to confirm green. + +Never skip hooks or add sleeps as a fix. Never use `networkidle`. + +### 3.4 Reconcile with the spec + +Open the spec referenced by the `// spec:` header in the test file and locate the scenario that matches the test. + +- **Fix was purely technical** (locator drift, better assertion shape) and the spec's user-level behaviour still matches the app → leave the spec alone. +- **Fix changed user-visible steps, inputs, order, or expected outcomes** that the spec describes → update the spec to match reality. Keep the scenario id and file path stable; only the step / expect lines change. +- **Unclear whether the app change is intentional** (spec is stale) **or a regression** (test was right, app is wrong) → **stop and ask the user**. Provide: + - the scenario id (e.g. `2.3`), + - the spec lines that no longer match, + - the observed app behaviour (quote a snapshot excerpt or a concrete outcome). + +Only after the user answers, either update the spec (intentional change) or file/flag the test as covering a bug (regression). + +### 3.5 Iteration and giving up + +- Fix failures one at a time; rerun after each. +- If after thorough investigation you are confident the test is correct but the app is wrong *and* the user has confirmed it's a bug: mark the test `test.fixme(...)` with a comment pointing at the user's decision or issue link. Never silently skip. + +--- + +## Cross-references + +| For... | See | +|---|---| +| `--debug=cli` / attach mechanics | [playwright-tests.md](playwright-tests.md) | +| How `playwright-cli` actions become TS | [test-generation.md](test-generation.md) | +| Mocking requests during exploration/generation | [request-mocking.md](request-mocking.md) | +| Managing the CLI browser session | [session-management.md](session-management.md) | diff --git a/.claude/skills/playwright-cli/references/storage-state.md b/.claude/skills/playwright-cli/references/storage-state.md new file mode 100644 index 000000000..bb5021a09 --- /dev/null +++ b/.claude/skills/playwright-cli/references/storage-state.md @@ -0,0 +1,275 @@ +# Storage Management + +Manage cookies, localStorage, sessionStorage, and browser storage state. + +## Storage State + +Save and restore complete browser state including cookies and storage. + +### Save Storage State + +```bash +# Save to auto-generated filename (storage-state-{timestamp}.json) +playwright-cli state-save + +# Save to specific filename +playwright-cli state-save my-auth-state.json +``` + +### Restore Storage State + +```bash +# Load storage state from file +playwright-cli state-load my-auth-state.json + +# Reload page to apply cookies +playwright-cli open https://example.com +``` + +### Storage State File Format + +The saved file contains: + +```json +{ + "cookies": [ + { + "name": "session_id", + "value": "abc123", + "domain": "example.com", + "path": "/", + "expires": 1893456000, + "httpOnly": true, + "secure": true, + "sameSite": "Lax" + } + ], + "origins": [ + { + "origin": "https://example.com", + "localStorage": [ + { "name": "theme", "value": "dark" }, + { "name": "user_id", "value": "12345" } + ] + } + ] +} +``` + +## Cookies + +### List All Cookies + +```bash +playwright-cli cookie-list +``` + +### Filter Cookies by Domain + +```bash +playwright-cli cookie-list --domain=example.com +``` + +### Filter Cookies by Path + +```bash +playwright-cli cookie-list --path=/api +``` + +### Get Specific Cookie + +```bash +playwright-cli cookie-get session_id +``` + +### Set a Cookie + +```bash +# Basic cookie +playwright-cli cookie-set session abc123 + +# Cookie with options +playwright-cli cookie-set session abc123 --domain=example.com --path=/ --httpOnly --secure --sameSite=Lax + +# Cookie with expiration (Unix timestamp) +playwright-cli cookie-set remember_me token123 --expires=1893456000 +``` + +### Delete a Cookie + +```bash +playwright-cli cookie-delete session_id +``` + +### Clear All Cookies + +```bash +playwright-cli cookie-clear +``` + +### Advanced: Multiple Cookies or Custom Options + +For complex scenarios like adding multiple cookies at once, use `run-code`: + +```bash +playwright-cli run-code "async page => { + await page.context().addCookies([ + { name: 'session_id', value: 'sess_abc123', domain: 'example.com', path: '/', httpOnly: true }, + { name: 'preferences', value: JSON.stringify({ theme: 'dark' }), domain: 'example.com', path: '/' } + ]); +}" +``` + +## Local Storage + +### List All localStorage Items + +```bash +playwright-cli localstorage-list +``` + +### Get Single Value + +```bash +playwright-cli localstorage-get token +``` + +### Set Value + +```bash +playwright-cli localstorage-set theme dark +``` + +### Set JSON Value + +```bash +playwright-cli localstorage-set user_settings '{"theme":"dark","language":"en"}' +``` + +### Delete Single Item + +```bash +playwright-cli localstorage-delete token +``` + +### Clear All localStorage + +```bash +playwright-cli localstorage-clear +``` + +### Advanced: Multiple Operations + +For complex scenarios like setting multiple values at once, use `run-code`: + +```bash +playwright-cli run-code "async page => { + await page.evaluate(() => { + localStorage.setItem('token', 'jwt_abc123'); + localStorage.setItem('user_id', '12345'); + localStorage.setItem('expires_at', Date.now() + 3600000); + }); +}" +``` + +## Session Storage + +### List All sessionStorage Items + +```bash +playwright-cli sessionstorage-list +``` + +### Get Single Value + +```bash +playwright-cli sessionstorage-get form_data +``` + +### Set Value + +```bash +playwright-cli sessionstorage-set step 3 +``` + +### Delete Single Item + +```bash +playwright-cli sessionstorage-delete step +``` + +### Clear sessionStorage + +```bash +playwright-cli sessionstorage-clear +``` + +## IndexedDB + +### List Databases + +```bash +playwright-cli run-code "async page => { + return await page.evaluate(async () => { + const databases = await indexedDB.databases(); + return databases; + }); +}" +``` + +### Delete Database + +```bash +playwright-cli run-code "async page => { + await page.evaluate(() => { + indexedDB.deleteDatabase('myDatabase'); + }); +}" +``` + +## Common Patterns + +### Authentication State Reuse + +```bash +# Step 1: Login and save state +playwright-cli open https://app.example.com/login +playwright-cli snapshot +playwright-cli fill e1 "user@example.com" +playwright-cli fill e2 "password123" +playwright-cli click e3 + +# Save the authenticated state +playwright-cli state-save auth.json + +# Step 2: Later, restore state and skip login +playwright-cli state-load auth.json +playwright-cli open https://app.example.com/dashboard +# Already logged in! +``` + +### Save and Restore Roundtrip + +```bash +# Set up authentication state +playwright-cli open https://example.com +playwright-cli eval "() => { document.cookie = 'session=abc123'; localStorage.setItem('user', 'john'); }" + +# Save state to file +playwright-cli state-save my-session.json + +# ... later, in a new session ... + +# Restore state +playwright-cli state-load my-session.json +playwright-cli open https://example.com +# Cookies and localStorage are restored! +``` + +## Security Notes + +- Never commit storage state files containing auth tokens +- Add `*.auth-state.json` to `.gitignore` +- Delete state files after automation completes +- Use environment variables for sensitive data +- By default, sessions run in-memory mode which is safer for sensitive operations diff --git a/.claude/skills/playwright-cli/references/test-generation.md b/.claude/skills/playwright-cli/references/test-generation.md new file mode 100644 index 000000000..a045c55d6 --- /dev/null +++ b/.claude/skills/playwright-cli/references/test-generation.md @@ -0,0 +1,134 @@ +# Test Generation + +Generate Playwright test code automatically as you interact with the browser. + +## How It Works + +Every action you perform with `playwright-cli` generates corresponding Playwright TypeScript code. +This code appears in the output and can be copied directly into your test files. + +## Example Workflow + +```bash +# Start a session +playwright-cli open https://example.com/login + +# Take a snapshot to see elements +playwright-cli snapshot +# Output shows: e1 [textbox "Email"], e2 [textbox "Password"], e3 [button "Sign In"] + +# Fill form fields - generates code automatically +playwright-cli fill e1 "user@example.com" +# Ran Playwright code: +# await page.getByRole('textbox', { name: 'Email' }).fill('user@example.com'); + +playwright-cli fill e2 "password123" +# Ran Playwright code: +# await page.getByRole('textbox', { name: 'Password' }).fill('password123'); + +playwright-cli click e3 +# Ran Playwright code: +# await page.getByRole('button', { name: 'Sign In' }).click(); +``` + +## Building a Test File + +Collect the generated code into a Playwright test: + +```typescript +import { test, expect } from '@playwright/test'; + +test('login flow', async ({ page }) => { + // Generated code from playwright-cli session: + await page.goto('https://example.com/login'); + await page.getByRole('textbox', { name: 'Email' }).fill('user@example.com'); + await page.getByRole('textbox', { name: 'Password' }).fill('password123'); + await page.getByRole('button', { name: 'Sign In' }).click(); + + // Add assertions + await expect(page).toHaveURL(/.*dashboard/); +}); +``` + +## Best Practices + +### 1. Use Semantic Locators + +The generated code uses role-based locators when possible, which are more resilient: + +```typescript +// Generated (good - semantic) +await page.getByRole('button', { name: 'Submit' }).click(); + +// Avoid (fragile - CSS selectors) +await page.locator('#submit-btn').click(); +``` + +### 2. Explore Before Recording + +Take snapshots to understand the page structure before recording actions: + +```bash +playwright-cli open https://example.com +playwright-cli snapshot +# Review the element structure +playwright-cli click e5 +``` + +### 3. Add Assertions Manually + +Generated code captures actions but not assertions. Add expectations in your test using one of the recommended matchers: + +- `toBeVisible()` — element is rendered and visible +- `toHaveText(text)` — element text content matches +- `toHaveValue(value) / toBeEmpty()` — input/select value matches +- `toBeChecked() / toBeUnchecked()` — checkbox state matches +- `toMatchAriaSnapshot(snapshot)` — page (or locator) matches a partial accessibility snapshot + +Use `playwright-cli generate-locator <target>` to produce the locator expression for the assertion, and the snapshot/eval commands to capture the expected value. + +When asserting text content, make sure that generated locator does not contain text from the element itself. `getByTestId()` or `getByLabel()` usually work well with asserting text. When locator is text-based, prefer `toBeVisible()` instead. + +Snapshot to be matched does not have to contain all the information - only capture what's necessary for the assertion. You can use regular expressions for unstable values. + +```bash +# Get a stable locator for an element ref to use in the assertion +playwright-cli --raw generate-locator e5 +# getByRole('button', { name: 'Submit' }) + +# Capture expected text content for toHaveText +playwright-cli --raw eval "el => el.textContent" e5 + +# Capture expected input value for toHaveValue/toBeEmpty +playwright-cli --raw eval "el => el.value" e5 + +# Capture expected aria snapshot for toMatchAriaSnapshot/toBeChecked +# (whole page, or use a ref to scope to a region) +playwright-cli --raw snapshot +playwright-cli --raw snapshot e5 +``` + +```typescript +// Generated action +await page.getByRole('button', { name: 'Submit' }).click(); + +// Manual assertions using the outputs above: +await expect(page.getByRole('alert', { name: 'Success' })).toBeVisible(); +await expect(page.getByTestId('main-header')).toHaveText('Welcome, user'); +await expect(page.getByRole('textbox', { name: 'Email' })).toHaveValue('user@example.com'); +await expect(page.getByRole('checkbox', { name: 'Enable notifications' })).toBeChecked(); + +// toMatchAriaSnapshot on the whole page, finds a matching region +await expect(page).toMatchAriaSnapshot(` + - heading "Welcome, user" + - link /\\d+ new messages?/ + - button "Sign out" +`); + +// toMatchAriaSnapshot scoped to a region +await expect(page.getByRole('navigation')).toMatchAriaSnapshot(` + - link "Home" + - link /\\d+ new messages?/ + - link "Profile" +`); +``` diff --git a/.claude/skills/playwright-cli/references/tracing.md b/.claude/skills/playwright-cli/references/tracing.md new file mode 100644 index 000000000..7ce7babbd --- /dev/null +++ b/.claude/skills/playwright-cli/references/tracing.md @@ -0,0 +1,139 @@ +# Tracing + +Capture detailed execution traces for debugging and analysis. Traces include DOM snapshots, screenshots, network activity, and console logs. + +## Basic Usage + +```bash +# Start trace recording +playwright-cli tracing-start + +# Perform actions +playwright-cli open https://example.com +playwright-cli click e1 +playwright-cli fill e2 "test" + +# Stop trace recording +playwright-cli tracing-stop +``` + +## Trace Output Files + +When you start tracing, Playwright creates a `traces/` directory with several files: + +### `trace-{timestamp}.trace` + +**Action log** - The main trace file containing: +- Every action performed (clicks, fills, navigations) +- DOM snapshots before and after each action +- Screenshots at each step +- Timing information +- Console messages +- Source locations + +### `trace-{timestamp}.network` + +**Network log** - Complete network activity: +- All HTTP requests and responses +- Request headers and bodies +- Response headers and bodies +- Timing (DNS, connect, TLS, TTFB, download) +- Resource sizes +- Failed requests and errors + +### `resources/` + +**Resources directory** - Cached resources: +- Images, fonts, stylesheets, scripts +- Response bodies for replay +- Assets needed to reconstruct page state + +## What Traces Capture + +| Category | Details | +|----------|---------| +| **Actions** | Clicks, fills, hovers, keyboard input, navigations | +| **DOM** | Full DOM snapshot before/after each action | +| **Screenshots** | Visual state at each step | +| **Network** | All requests, responses, headers, bodies, timing | +| **Console** | All console.log, warn, error messages | +| **Timing** | Precise timing for each operation | + +## Use Cases + +### Debugging Failed Actions + +```bash +playwright-cli tracing-start +playwright-cli open https://app.example.com + +# This click fails - why? +playwright-cli click e5 + +playwright-cli tracing-stop +# Open trace to see DOM state when click was attempted +``` + +### Analyzing Performance + +```bash +playwright-cli tracing-start +playwright-cli open https://slow-site.com +playwright-cli tracing-stop + +# View network waterfall to identify slow resources +``` + +### Capturing Evidence + +```bash +# Record a complete user flow for documentation +playwright-cli tracing-start + +playwright-cli open https://app.example.com/checkout +playwright-cli fill e1 "4111111111111111" +playwright-cli fill e2 "12/25" +playwright-cli fill e3 "123" +playwright-cli click e4 + +playwright-cli tracing-stop +# Trace shows exact sequence of events +``` + +## Trace vs Video vs Screenshot + +| Feature | Trace | Video | Screenshot | +|---------|-------|-------|------------| +| **Format** | .trace file | .webm video | .png/.jpeg image | +| **DOM inspection** | Yes | No | No | +| **Network details** | Yes | No | No | +| **Step-by-step replay** | Yes | Continuous | Single frame | +| **File size** | Medium | Large | Small | +| **Best for** | Debugging | Demos | Quick capture | + +## Best Practices + +### 1. Start Tracing Before the Problem + +```bash +# Trace the entire flow, not just the failing step +playwright-cli tracing-start +playwright-cli open https://example.com +# ... all steps leading to the issue ... +playwright-cli tracing-stop +``` + +### 2. Clean Up Old Traces + +Traces can consume significant disk space: + +```bash +# Remove traces older than 7 days +find .playwright-cli/traces -mtime +7 -delete +``` + +## Limitations + +- Traces add overhead to automation +- Large traces can consume significant disk space +- Some dynamic content may not replay perfectly diff --git a/.claude/skills/playwright-cli/references/video-recording.md b/.claude/skills/playwright-cli/references/video-recording.md new file mode 100644 index 000000000..5209d21ee --- /dev/null +++ b/.claude/skills/playwright-cli/references/video-recording.md @@ -0,0 +1,143 @@ +# Video Recording + +Capture browser automation sessions as video for debugging, documentation, or verification. Produces WebM (VP8/VP9 codec). + +## Basic Recording + +```bash +# Open browser first +playwright-cli open + +# Start recording +playwright-cli video-start demo.webm + +# Add a chapter marker for section transitions +playwright-cli video-chapter "Getting Started" --description="Opening the homepage" --duration=2000 + +# Navigate and perform actions +playwright-cli goto https://example.com +playwright-cli snapshot +playwright-cli click e1 + +# Add another chapter +playwright-cli video-chapter "Filling Form" --description="Entering test data" --duration=2000 +playwright-cli fill e2 "test input" + +# Stop and save +playwright-cli video-stop +``` + +## Best Practices + +### 1. Use Descriptive Filenames + +```bash +# Include context in filename +playwright-cli video-start recordings/login-flow-2024-01-15.webm +playwright-cli video-start recordings/checkout-test-run-42.webm +``` + +### 2. Record entire hero scripts. + +When recording a video for the user or as a proof of work, it is best to create a code snippet and execute it with run-code. +It allows inserting appropriate pauses between the actions and annotating the video. There are new Playwright APIs for that. + +1) Perform scenario using CLI and take note of all locators and actions. You'll need those locators to request their bounding boxes for highlight. +2) Create a file with the intended script for video (below). Use pressSequentially w/ delay for nice typing, make reasonable pauses. +3) Use playwright-cli run-code --filename your-script.js + +**Important**: Overlays are `pointer-events: none` — they do not interfere with page interactions. You can safely keep sticky overlays visible while clicking, filling, or performing any actions on the page. + +```js +async page => { + await page.screencast.start({ path: 'video.webm', size: { width: 1280, height: 800 } }); + await page.goto('https://demo.playwright.dev/todomvc'); + + // Show a chapter card — blurs the page and shows a dialog. + // Blocks until duration expires, then auto-removes. + // Use this for simple use cases, but always feel free to hand-craft your own beautiful + // overlay via await page.screencast.showOverlay(). + await page.screencast.showChapter('Adding Todo Items', { + description: 'We will add several items to the todo list.', + duration: 2000, + }); + + // Perform action + await page.getByRole('textbox', { name: 'What needs to be done?' }).pressSequentially('Walk the dog', { delay: 60 }); + await page.getByRole('textbox', { name: 'What needs to be done?' }).press('Enter'); + await page.waitForTimeout(1000); + + // Show next chapter + await page.screencast.showChapter('Verifying Results', { + description: 'Checking the item appeared in the list.', + duration: 2000, + }); + + // Add a sticky annotation that stays while you perform actions. + // Overlays are pointer-events: none, so they won't block clicks. + const annotation = await page.screencast.showOverlay(` + <div style="position: absolute; top: 8px; right: 8px; + padding: 6px 12px; background: rgba(0,0,0,0.7); + border-radius: 8px; font-size: 13px; color: white;"> + ✓ Item added successfully + </div> + `); + + // Perform more actions while the annotation is visible + await page.getByRole('textbox', { name: 'What needs to be done?' }).pressSequentially('Buy groceries', { delay: 60 }); + await page.getByRole('textbox', { name: 'What needs to be done?' }).press('Enter'); + await page.waitForTimeout(1500); + + // Remove the annotation when done + await annotation.dispose(); + + // You can also highlight relevant locators and provide contextual annotations. + const bounds = await page.getByText('Walk the dog').boundingBox(); + await page.screencast.showOverlay(` + <div style="position: absolute; + top: ${bounds.y}px; + left: ${bounds.x}px; + width: ${bounds.width}px; + height: ${bounds.height}px; + border: 1px solid red;"> + </div> + <div style="position: absolute; + top: ${bounds.y + bounds.height + 5}px; + left: ${bounds.x + bounds.width / 2}px; + transform: translateX(-50%); + padding: 6px; + background: #808080; + border-radius: 10px; + font-size: 14px; + color: white;">Check it out, it is right above this text + </div> + `, { duration: 2000 }); + + await page.screencast.stop(); +} +``` + +Embrace creativity, overlays are powerful. + +### Overlay API Summary + +| Method | Use Case | +|--------|----------| +| `page.screencast.showChapter(title, { description?, duration?, styleSheet? })` | Full-screen chapter card with blurred backdrop — ideal for section transitions | +| `page.screencast.showOverlay(html, { duration? })` | Custom HTML overlay — use for callouts, labels, highlights | +| `disposable.dispose()` | Remove a sticky overlay added without duration | +| `page.screencast.hideOverlays()` / `page.screencast.showOverlays()` | Temporarily hide/show all overlays | + +## Tracing vs Video + +| Feature | Video | Tracing | +|---------|-------|---------| +| Output | WebM file | Trace file (viewable in Trace Viewer) | +| Shows | Visual recording | DOM snapshots, network, console, actions | +| Use case | Demos, documentation | Debugging, analysis | +| Size | Larger | Smaller | + +## Limitations + +- Recording adds slight overhead to automation +- Large recordings can consume significant disk space diff --git a/.claude/skills/quality-vector-tests/SKILL.md b/.claude/skills/quality-vector-tests/SKILL.md new file mode 100644 index 000000000..339d6f465 --- /dev/null +++ b/.claude/skills/quality-vector-tests/SKILL.md @@ -0,0 +1,262 @@ +--- +name: quality-vector-tests +description: >- + Write or reformat the **Testing** section inside a constructorfabric/insight feature issue, + grouped by the five quality vectors (Efficiency, Reliability, Performance, + Security, Versatility), with every check written as a Metric / How measured / Target triple — + a short metric name, an exact measurement procedure with a real denominator, and a number to + chase — then edit it into the GitHub issue body. Use this whenever the task is to add, fix, + format, clean up, or standardize the Testing / QA section of an Insight feature or epic — + "format the testing section of #<n>", "add quality-vector tests to this feature", "the testing + block is messy, clean it up", "make the testing section consistent", "put the tests into the + feature", "simplify the testing section", or when a feature's Testing section has loose bullets, + open questions (`coverage?`, `Lighthouse?`), mispaired vectors, vague targets, or broken + numbering that need to become a measurable, vector-grouped block. This is the + *authoring/formatting* counterpart to scope-feature-tests: reach for scope-feature-tests to + REASON OUT what to cover (risk-ordered lean scope); reach for THIS to lay that coverage into the + feature body in the canonical five-vector, measurable-target format the Insight quality program + uses. Trigger even when the user only says "quality vector tests" or "fix the formatting for + testing" without naming the format. +--- + +# Quality-vector Testing sections (Insight features) + +Turn a feature's testing needs into the **standard Testing section** that lives inside the +constructorfabric/insight GitHub issue: grouped by the five quality vectors, every +check named as a metric with a number to chase, written straight into the issue body. This is the +format the Insight quality program uses so that any engineer or QA lead reading a feature sees the +*same shape* of test plan every time. + +## The one idea + +A Testing section earns its place only if each line names **a metric**, **exactly how that metric +gets measured**, and **the value that means pass**. Loose bullets like "UI e2e tests - coverage?" +or "Dashboard loading time. Lighthouse? Playwright?" are questions, not a plan. But so is +"we check that pagination works correctly" — that's a *behaviour*, not a metric, and two people +will score it differently. + +The discipline is to convert intent into a line that reads like a dashboard row: + +> **Page errors** — page through 3,000+ records at page size 500 (7 pages) → **0 duplicates, 0 +> omissions, total exact** + +Not "we verify pagination is honest". The difference is that someone who never read the feature can +run the second one and tell you whether it passed. + +The vectors are not decoration. The canonical set is Efficiency, Reliability, Performance, +Security and Versatility, and that ranking reflects importance to the customer — it is not the +order of the section, which follows risk (see "Order vectors by risk" below). Each metric belongs +to exactly one vector. Getting the assignment right is half the value; see +`references/vector-mapping.md`. + +## Workflow + +### 1. Ground the checks in the real feature +**Not every job needs this step.** When the request is purely cosmetic — regrouping existing checks +under the right vectors, fixing numbering, tightening wording, with no check added, removed or +re-targeted — read the issue body and go straight to step 3. Grounding costs two repos' worth of +reading, and it earns that only when you are changing *what gets tested*. + +Otherwise, don't invent a generic checklist. Read the issue and the actual implementation the same way +`scope-feature-tests` does — pull the issue (`gh issue view <n> --repo constructorfabric/insight +--json title,body,labels,parent`), check for a branch or merged PR (`gh pr list --repo constructorfabric/insight --search "<n>" +--state all`), then read the code — this repo for backend, ingestion and dbt, and the sibling +`../insight-front` for UI. + +Two things this grounding is *for*, beyond correctness: + +- **The denominators.** "API coverage — 100%" is unfalsifiable until you know 100% of what. Count + it: endpoints in the router, cases in the acceptance criteria, connectors under + `src/ingestion/connectors`, metric keys in `metric_definitions/builtin.rs`. Those counts go in + the section. +- **Whether the feature's own framing still holds.** When the shipped code contradicts the issue — + scope was dropped, scope was added, a "table" shipped as a general-purpose component — correct + the checks against reality and say so to the user. + +If the feature is a **port**, **consolidation**, or **rolling migration**, the headline check is +almost always a **differential / parity gate** (old vs new on the same data) — and it is frequently +*missing* from the author's first draft. Add it. + +For the deeper grounding discipline (feature shapes, differential tags, per-source coverage +matrices, deferred-behavior handling), read the sibling skill `scope-feature-tests` — this skill +reuses its reasoning and only differs in the **output format and location**. + +### 2. Verify every tool you are about to name +This is where drafts quietly lie. "Semgrep + Trivy in CI" is a sentence anyone can type; whether +those scanners exist in this repo's pipeline is a fact you can check in about ten seconds. Run only +the line matching a tool the draft actually names — these are lookups, not a survey: + +```sh +grep -rniE "semgrep|trivy|codeql|snyk|grype" .github/ # scanners +grep -rniE "k6|locust|gatling|jmeter|vegeta" . --include='*.yml' --include='*.md' # load harness +grep -rn "lighthouse" ../insight-front/package.json .github/ # page-load tooling +ls ../insight-front/e2e 2>/dev/null || grep -n '"test' ../insight-front/package.json # e2e vs unit +``` + +If the tool is not there, you have found something worth reporting, and the honest line names the +gap rather than pretending: *"Target: 0 critical — **not measurable today**, no load harness is +wired in CI; wiring one is a prerequisite."* A target that silently assumes infrastructure nobody built is +worse than no target, because it will be reported green by default. + +The same applies to numbers with no precedent in the repo. If you propose a 30-minute soak at <5% +memory growth and nothing in the codebase establishes that bar, say it's your proposal and let the +user set it. + +### 3. Assign each check to its one vector +Use `references/vector-mapping.md`. Common miss: "code coverage" and "e2e coverage" belong under +**Reliability** (leading indicators of correctness), not Efficiency — Efficiency is *compute cost +to run*, not test rigor. Pagination correctness is Reliability too, however often it gets filed +under Efficiency because it involves volume. + +### 4. Write the section in the canonical format +Follow the template below. Then edit it into the issue body, replacing the loose Testing block and +preserving everything else (Goal, Scope, Acceptance, Planning). + +### 5. Draft-or-push +Show the checks for review first whenever you are changing what is tested — adding a gate, moving +an item between vectors, or setting numbers the user hasn't seen. A compact table is the fastest +thing to review: + +| # | Vector | Metric | How we measure | Target | +|---|---|---|---|---| + +Push straight through only when the change is purely cosmetic. Edit via a body file, never inline, +so the rest of the body survives verbatim. Write that file **outside this repo** — nothing here is +gitignored for scratch output, so a body file left behind shows up in someone's `git status`: +```sh +BODY="$(mktemp -d)/<n>-body.md" # never a fixed /tmp name: it collides between concurrent + # runs and leaves the last body lying around +gh issue view <n> --repo constructorfabric/insight --json body -q .body > "$BODY" +# replace only the Testing block in that file, then drop the trailing newline `-q` adds — +# without this, every edit appends one more blank line to the end of the issue body +perl -0pi -e 's/\n+\z/\n/' "$BODY" +gh issue edit <n> --repo constructorfabric/insight --body-file "$BODY" +``` +Re-fetch the body immediately before every edit. These issues are actively co-authored, and a body +built from a stale copy silently reverts someone else's work — if the fresh copy differs from what +you last saw, rebuild on the new one and tell the user what changed. + +## The format + +```markdown +## Testing + +<1–3 framing sentences: the feature shape (port / migration / new capability), the fixture threaded +through the non-functional checks (e.g. the reference-org dataset), and what leads the risk. +For a port/migration, name the parity bar here.> + +### <Vector> +1. **<Metric name>** *(optional tag: main gate)* + - Metric: <the quantity — a rate, a count, a percentage, a latency. Two or three words.> + - How measured: <the exact procedure, with the denominator: which fixture, how many items, which + tool's which field.> + - Target: <the number. 100%. 0. < 1s P95. 26/26.> + +### <Next vector> +2. **<Metric name>** + - Metric / How measured / Target … + +### <Vector with nothing to check> +**Not applicable** — <one line saying why, e.g. no new query path of its own; latency is measured +on the shared runtime.> +``` + +### Rules that make the format work + +- **Name a metric, not a behaviour.** "Count match", "Page errors", "Drill coverage", "Latency + (P95)", "Memory growth", "Critical findings". If the name is a sentence, it's a behaviour — find + the quantity underneath it. Two or three plain words; no `snake_case`, no invented coinages like + "reconciliation integrity index". +- **Every How measured carries its denominator.** `n of m`, not "some". *59 of 59 catalog metrics*, + *17 of 17 acceptance criteria*, *26 of 26 connectors*, *7 pages of a 3,000-record fixture*. If you + can't state m, you haven't finished grounding — go count it. +- **Every Target is a value.** A number, a percentage, a ratio, a threshold, or `0`. Never `?`, + never "no regressions", never "works correctly". `Lighthouse?` → `< 10s page load`. `coverage?` → + `17/17 cases`. If the decision isn't yours, ask — don't ship a `?`. + When a target wants to be a promise ("no invalid definition is ever served", "clear error, never + wrong data or a crash"), **count the bad thing and target zero**. Name each failure separately so + each gets its own number: *"**0** invalid definitions served, **0** empty responses during a + reload, **0** conflicts resolved to the wrong definition"* — three failures, three zeros, all + checkable. A prose promise reads as rigorous and scores as nothing; a zero is a number someone can + report against. The same move handles comparisons: "no worse than the old path" → `≤ 100% of the + baseline for both CPU and memory`. +- **One check → one vector.** Never pair vectors (`Efficiency + Versatility`). If a check seems to + span two, it's usually two checks with two different numbers — split it. That's also the tell for + a target doing too much work: "100% reconciliation and 0 leaked records" is two metrics. +- **Number continuously** across the whole section (1..N), not restarting per vector, so people can + refer to "check 4" unambiguously. +- **Order vectors by risk**, headline first. For most Insight features that's **Reliability** (the + data promise — the number is right). +- **Every vector appears.** A vector with nothing to check says **Not applicable** and why, in one + line. Silence reads as an oversight; an explicit N/A reads as a decision, and it survives review. + Don't invent a check to fill a vector — an honest N/A beats a padded one. +- **Plain language, no internal jargon.** Table names, symbol paths, ADR/`@cpt-*` ids, `S1/T4` tags + are how *you* reason; strip them from the filed text. Requirement ids the issue itself defines + (`BR-1`, `AC-7`) are the exception — cite those, they let a reader trace a check to its + requirement. Keep them current if the issue renumbers. +- **"How measured" needs a verb.** The most common failure in these sections is a noun phrase + standing in for a procedure: *"fixture of known-distinct near-duplicate identities — shared names, + shared display-names, recycled logins"* lists ingredients but never says to build anything, run + anything, or compare anything. Write the instruction: *"Build test data with pairs of genuinely + different people who look alike — same full name, same display name, or one reusing another's old + login — resolve each, and check they stay separate."* Longer, and executable by someone who has + never seen the project. Watch especially for a definite article pointing at something the reader + has never seen — "**the** public allowlist", "**the** same request mix", "**the** resolved person + set". If it's `the`, either name it or say where to find it. +- **The framing paragraph orients an outsider.** Its job is to say, in ordinary words, what the + feature does and what could go wrong — not to compress the architecture. If the section's opening + and the issue's title appear to describe two different things (a "weekly git table" above, a + "reusable dimensional timeseries view" below), a newcomer can't tell they're the same feature. + Say that they are. +- **Read it back as a stranger.** Before pushing, reread every line as someone handed this ticket + with no project context, and ask of each one: could I go and do this, or would I have to find the + author first? Anything in the second category isn't finished. An independent reviewer is worth + spending on here — this is exactly the blind spot the author of a section cannot see. +- **Match the issue's heading levels.** If its sections are `##`, use `## Testing` / `### Vector`; + if they're `###`, demote one step. Testing should sit level with Scope and Acceptance. +- **Follow the author's scope.** These sections are drafted by the feature's engineer; your job is + to make their intent measurable, not to replace it. Add at most the one check whose absence would + make the section dishonest — usually the differential/parity gate or the reconciliation gate — + and flag it explicitly rather than slipping it in. When the user says it's too long, cut to their + original items and keep the format. +- **The differential/parity gate is the headline** for ports, consolidations, and migrations, and + is tagged, never blanket zero-diff: `exact` (must match) / `known-diff(direction)` (deliberate + change — assert the direction) / `merge` (siblings collapse — merged == Σ parts). + +### Turning a vague line into a metric + +| Author wrote | Metric | How measured | Target | +|---|---|---|---| +| "API coverage - limits" | **API coverage** | happy path + oversized request + undrillable target, against the 17 acceptance criteria | **17/17**; oversized → 4xx + reason, never a partial 200 | +| "pagination tests" | **Page errors** | page a 3,000-record fixture at page size 500 (7 pages) | **0** duplicates, **0** omissions, total exact | +| "Cover all connector data" | **Connector coverage** | per-connector fixtures driven by the metric catalog | **26/26** connectors, **59/59** metric keys | +| "Latency for drill down requests" | **Latency (P95)** | 200 requests on the reference-org dataset, deepest lineage path | **< 1s** | +| "Resource usage per service" | **Memory growth** | RSS at start vs end of a 30-min paged-request soak | **< 5%** | +| "No critical issues in the ci pipeline" | **Critical findings** | Trivy `--severity CRITICAL` + Semgrep `--severity ERROR` counts, from the workflows in `.github/` | **0** | + +## Counts worth knowing (verify, don't quote from here) +These change; the point is that they are *countable*, and where. + +```sh +find src/ingestion/connectors -maxdepth 2 -mindepth 2 -type d | wc -l # connectors (26) +grep -oE 'metric_key: "[a-z0-9_.]+"' \ + src/backend/services/analytics/src/domain/metric_definitions/builtin.rs \ + | sort -u | wc -l # catalog metrics (59) +grep -c "^CREATE VIEW insight\." \ + src/ingestion/scripts/migrations/20260422000000_gold-views.sql # gold views (28) +ls src/ingestion/tests/e2e/metrics/*.test.yaml | wc -l # metrics with a spec (36) +``` + +Those last two give the coverage ratio for "which metrics have a regression test" — specs over +catalog metrics, 36/59 at the time of writing, not 36/36. The catalog count is the denominator. +It is the natural target for a Reliability coverage check on any metric feature — `metric-test` +authors those specs. + +## Worked examples +Read the one closest to the feature in front of you — they show the format applied end to end: +- `references/example-migration.md` — a rolling **migration platform** (unified metric system). + Reliability-led, differential as the main gate, registry-driven coverage. +- `references/example-port.md` — a C#→Rust **port** (identity resolution). Shows the vector + spread across all five, and the correction that the first draft under-tested core correctness. +- `references/example-lean.md` — a **greenfield epic** trimmed back to the author's own draft. + Shows explicit `Not applicable` vectors, requirement-id citations, and unmeasurable-tooling flags. diff --git a/.claude/skills/quality-vector-tests/references/example-lean.md b/.claude/skills/quality-vector-tests/references/example-lean.md new file mode 100644 index 000000000..8d6ac3d6a --- /dev/null +++ b/.claude/skills/quality-vector-tests/references/example-lean.md @@ -0,0 +1,115 @@ +# Worked example — metric → source drill-down (greenfield, lean) + +Drill from any metric value to the source records behind it. Shape: **new capability, not yet +built** — no branch, no PR. This example shows three things the other two don't: staying inside the +author's own draft scope, explicit `Not applicable` vectors, and how to handle a target whose +tooling doesn't exist. + +## Before (loose author draft) +``` +**Reliability** +API coverage - limits - should return error for too much data requested. + +**Efficiency** +Resource usage per service for drill-down requests - pagination tests. Coverage. + +**Versatility** +Cover all 25 connector data - by metric group + +**Performance** +Latency for drill down requests + +**Security** +No critical issues in the ci pipeline for static analysis +``` +Every vector has something — so this is not a padding problem, it's a *measurability* problem. +Nothing here has a denominator or a number, "pagination tests" and "Coverage" are filed under +Efficiency, and the whole trust invariant the epic exists for (BR-1: if a tile says 42, the +drill-down must account for 42) has no check at all. + +## Grounding that changed the numbers +- `insight.ic_drill` is an **empty stub view** (`SELECT '' … FROM system.one WHERE 0`) reached via + an OData `drill_id eq` filter — the contract exists, the evidence doesn't. So the checks are + written against the issue's requirement ids, not against code. +- Countable denominators found in the repo: **26** connectors, **59** catalog metric keys across 5 + families, **17** acceptance criteria in the issue itself. +- Semgrep, Trivy and CodeQL run in this repo's CI (`.github/workflows/`), so a Security target is + measurable here — but **`insight-front` has no scanner of its own**, and there is **no load + harness anywhere** (no k6, locust, gatling, jmeter). A Performance target naming one would have + to be built first — which is a finding, not a formatting detail. + +## After (canonical format) +```markdown +## Testing + +Drill-down exists to make a number believable, so the headline check is the trust invariant itself +(BR-1): the evidence returned must account for exactly the value it explains. Everything else +guards the honesty of what surrounds it — volume, coverage, speed and exposure. Speed and run-cost +are measured on the reference-org dataset. + +### Reliability +1. **Count match** *(main gate — BR-1, BR-2)* + - Metric: drilled record count ÷ displayed value. + - How measured: for each catalog metric, run the metric query and the drill query with identical + period, person and filters, and compare; derived values compare against their stated inputs. + - Target: **59/59** metrics match, **0** discrepancy. +2. **Excluded records shown** *(BR-3)* + - Metric: records the metric excluded that appear in its drill-down. + - How measured: fixture seeded with bot, automation, migration-artefact and unattributed records. + - Target: **0**. +3. **API coverage** *(BR-10, BR-11)* + - Metric: acceptance criteria covered by an automated test. + - How measured: happy path plus the two refusal cases — oversized request, undrillable target. + - Target: **17/17**; oversized → 4xx + reason, never a partial 200; undrillable → error + distinguishable from an empty result. +4. **Page errors** *(BR-9)* + - Metric: duplicate rows, missing rows, and reported-total accuracy. + - How measured: page a 3,000-record fixture at page size 500 (7 pages) and union the pages. + - Target: **0** duplicates, **0** omissions, total exact (3,000, not 500). + +### Versatility +5. **Connector coverage** *(BR-18, BR-12)* + - Metric: connectors and metrics that return evidence or an explicit lineage gap. + - How measured: per-connector fixtures driven by the metric catalog. + - Target: **26/26** connectors, **59/59** metrics; **0** silently undrillable. + +### Performance +6. **Latency (P95)** + - Metric: drill request latency at reference-org scale. + - How measured: 200 requests on the reference-org dataset, deepest lineage path included. + - Target: **< 1s** — *no load harness exists in the repo today; one is a prerequisite.* + +### Efficiency +7. **Memory growth** + - Metric: RSS of the services a drill touches, start vs end. + - How measured: 30-minute soak driving repeated paged requests on the reference-org dataset. + - Target: **< 5%** growth, CPU back to baseline — *proposed bar, no precedent in the repo.* + +### Security +8. **Critical findings** + - Metric: critical findings in the new drill-down code. + - How measured: Trivy `--severity CRITICAL` + Semgrep `--severity ERROR` counts, from the + workflows already in this repo's CI. + - Target: **0** — *frontend-only findings stay uncovered until `insight-front` gets a scanner.* +``` + +## What the format did +- Renamed behaviours into metrics the author can read at a glance: "pagination tests" → **Page + errors**, "Cover all 25 connector data" → **Connector coverage**, "No critical issues in the ci + pipeline" → **Critical findings**. +- Gave every target a denominator counted from the repo (59, 25, 17, 3,000/500) instead of "all" + or "100%". +- Moved "pagination tests" and "Coverage" from Efficiency to **Reliability** — paging correctness is + a correctness claim; Efficiency kept the author's genuine run-cost item. +- Split "100% reconciliation" from "0 leaked records" into checks 1 and 2, because one target line + can't carry two numbers. +- Added exactly **one** check the author didn't have — the main gate — and flagged it to the user + rather than slipping it in. The epic's own AC-1 demanded it. +- Marked the two unbuildable targets and the one invented bar in italics, so nobody reports them + green by default. + +## Note on co-authored issues +This body was rewritten by the feature's engineer between two edits, dropping three requirements and +renumbering BR-15…21 → BR-12…18. Re-fetch the body immediately before editing, and re-check any +requirement ids you cite — a section built from a stale copy reverts someone else's work and leaves +dangling references. diff --git a/.claude/skills/quality-vector-tests/references/example-migration.md b/.claude/skills/quality-vector-tests/references/example-migration.md new file mode 100644 index 000000000..9bfadfdda --- /dev/null +++ b/.claude/skills/quality-vector-tests/references/example-migration.md @@ -0,0 +1,87 @@ +# Worked example — a unified metric system (rolling migration) + +A generic metrics runtime that the whole catalog migrates onto, wave by wave. Shape: **migration +platform** → the headline check is a reusable differential gate, and coverage must be +registry-driven (the metric list is still settling, so measure the machinery, not a fixed list). + +## Before (loose author draft) +``` +** Testing ** +Single API endpoint: +- Efficiency + Versatility: + 1. Smoke test for metrics. Metrics - coverage of the each metric + coverage of each connector... + 2. UI e2e tests - coverage? +- Performance + Efficiency: + 2. For dataset for organisation [link] - each metric returns <1s Latency... + 3. Dashboard loading time. Lighthouse? Playwright? for same dataset. <10s for team? +- Reliability: + 4. Monitoring for system health. Metric - uptime. + Logging - amount of error messages... +``` +Problems: paired vectors, broken numbering (1,2 → 2,3 → 4), open questions, no differential gate, +no denominators, and `uptime/logging` filed as a check when it needs an observability stack that +isn't wired. + +## After (canonical format) +```markdown +## Testing + +One system serves every metric; each group migrates onto it in turn. So we measure the shared +machinery and the invariants — not a fixed metric list. All speed checks run on the reference-org +dataset so numbers stay comparable over time. + +### Reliability +1. **Old-vs-new diff** *(main gate)* + - Metric: metric values differing between the old per-domain path and the new engine. + - How measured: automated diff harness over the seeded dataset, every catalog metric run on both + paths; each metric tagged `exact`, `known-diff` (assert direction), or `merge` (merged = Σ parts). + - Target: **0** untagged differences. +2. **Definition integrity** + - Metric: invalid definitions served, and empty windows during a reload. + - How measured: unit + integration on the loader/reconciler, including a concurrent-reload test. + - Target: **0** invalid definitions served, **0** empty windows. +3. **Bad-request handling** + - Metric: malformed requests returning a clear error rather than data or a crash. + - How measured: API tests over the 4 rejection cases — unknown metric, bad dimension, oversized + result set, malformed `metric_date` (the YYYY-MM-DD filter guard). + - Target: **4/4** return a specific error; **0** wrong results, **0** crashes. + +### Versatility +4. **Metric × view coverage** + - Metric: catalog metrics served correctly in each of the 4 views (period, peers, over-time, breakdown). + - How measured: registry-driven harness reading the catalog; asserts value + dimensions + peer group. + - Target: **100%** of catalog metrics; a new metric is covered by config, not new test code. +5. **UI group coverage** + - Metric: migrated metric groups rendering correctly on the dashboard. + - How measured: Playwright against the metric-collection renderer; sequenced behind the FE build. + - Target: every migrated group displays; **0** regressions on existing screens. + +### Performance +6. **Latency (P95)** + - Metric: per-metric endpoint latency at reference-org scale. + - How measured: load harness on the reference-org dataset, P95 per endpoint. + - Target: **< 1s**. +7. **Dashboard load** + - Metric: page-load and time-to-interactive for a team. + - How measured: Lighthouse (load) + Playwright (interactive), same dataset — *neither is wired + today: no lighthouse in `insight-front`, and its Playwright is browser-mode unit testing, not + an e2e suite. Wiring one is a prerequisite.* + - Target: **< 10s**. + +### Efficiency +**Not applicable** — the runtime replaces existing query paths rather than adding a service; run-cost +is unchanged and tracked at the platform level. + +### Security +**Not applicable** — no new external surface; the endpoint inherits the gateway's existing authn/authz. +``` + +## What the format did +- De-paired the vectors; one check per vector; continuous 1–7 numbering. +- Turned `coverage?` / `Lighthouse?` into decided targets with denominators (`4/4` rejection cases, + `100%` of catalog metrics, `< 10s`). +- Renamed behaviours into metrics: "Smoke test for metrics" → **Metric × view coverage**; + "Dashboard loading time. Lighthouse?" → **Dashboard load**. +- Added the differential as the `main gate` (missing from the draft) — the real proof the migrated + numbers are right. +- Dropped `uptime/logging`, an un-runnable check blocked on an observability stack that isn't wired, + and made Efficiency/Security explicit `Not applicable` rather than silently absent. diff --git a/.claude/skills/quality-vector-tests/references/example-port.md b/.claude/skills/quality-vector-tests/references/example-port.md new file mode 100644 index 000000000..19f1b6698 --- /dev/null +++ b/.claude/skills/quality-vector-tests/references/example-port.md @@ -0,0 +1,79 @@ +# Worked example — identity resolution (C#→Rust port) + +Port of the C# identity service to Rust, grown into a full identity epic. Shape: **port** → the +core gate is parity (same data in, same people out). This example shows the vector spread across +**all five** vectors, and the correction that grounding forced. + +## The correction grounding forced +The author's first draft was almost all non-functional (soak, monitoring, load, scanning) — it +barely measured the *core* behaviour. For identity, a **wrong merge corrupts every downstream +metric**, so the headline must be resolution correctness + the no-false-merge safety + the C#→Rust +differential. Grounding also confirmed the framing: the person is **email-keyed** by the seed +sources; every other source maps its own key on, and the reviewer namespace (GitHub login / +Bitbucket display-name) ≠ author email (git→HR ≈ 68% today), so cross-namespace resolution is a +deferred, kept-red check. + +## After (canonical format — full five-vector spread) +```markdown +## Testing + +Port of the C# identity service to Rust. The person is **email-keyed** by the seed sources +(HR/Entra/comms/AI); every other source must map its own key onto that person. Speed runs on the +3,000-person demo org. + +### Reliability +1. **People diff** *(main gate)* + - Metric: people whose resolution differs between the C# and Rust services. + - How measured: both services run over the same seeded dataset; compare the resolved person set. + - Target: **0** differences. +2. **False merges** + - Metric: distinct humans auto-merged into one person. + - How measured: fixture with known-distinct near-duplicate identities (shared names, shared + display-names, recycled logins). + - Target: **0**. +3. **API + e2e coverage** + - Metric: identity endpoints and the resolve path covered by automated tests. + - How measured: e2e across MariaDB + ClickHouse + identity; contract/Swagger check. + - Target: **3/3** endpoints; **0** regressions on existing screens. + +### Versatility +4. **Org-chart sync** + - Metric: directory providers whose reporting lines and team model resolve correctly. + - How measured: per-provider fixtures — MS Entra, BambooHR (Workday / AD as they land). + - Target: **2/2** live providers; manager + team at correct depth; root and cycles handled. +5. **Cross-namespace resolution** + - Metric: share of reviewer-namespace identities (GitHub login / Bitbucket display-name) + resolved to a person. + - How measured: fixtures across all 26 connectors. + - Target: resolved wherever evidence allows; **git→HR ≈ 68% today** — kept as a check that stays + red until cross-namespace resolution lands, rather than a silently deferred gap. + +### Performance +6. **Latency (P95)** + - Metric: resolution / lookup latency under load. + - How measured: load test with a baseline first, on the 3,000-person demo org. + - Target: **< 100ms** — *proposed; no existing latency budget in the repo sets this bar.* + +### Efficiency +7. **Memory growth** + - Metric: RSS and CPU of the Rust service under sustained load. + - How measured: soak on the demo dataset, sampled at start vs end. + - Target: **< 5%** growth; CPU returns to baseline — *proposed; no soak precedent in the repo.* + +### Security +8. **Critical findings** + - Metric: critical findings in the new Rust service. + - How measured: Trivy `--severity CRITICAL` + Semgrep `--severity ERROR` counts in CI. + - Target: **0**. +``` + +## Notes +- The author's final call was to keep a leaner set without checks 1 and 2. Those two are what + actually *prove* a port, so if a port's Testing section has no differential and no + no-false-merge check, flag the gap against the issue's own Acceptance Criteria before pushing — + then defer to the author's scope if they still want it out. +- "code coverage with e2e tests", which the author had under *Efficiency*, moved to **Reliability** + — test rigor is a reliability signal, not a run-cost one. +- Before filing check 8, confirm the scanners exist: `grep -rniE "semgrep|trivy" .github/`. In this + repo they do (semgrep, trivy and codeql workflows), so the target is real; `insight-front` has + none, so a frontend-scoped security target there is still "not measurable until one is wired". diff --git a/.claude/skills/quality-vector-tests/references/vector-mapping.md b/.claude/skills/quality-vector-tests/references/vector-mapping.md new file mode 100644 index 000000000..8323ffd4f --- /dev/null +++ b/.claude/skills/quality-vector-tests/references/vector-mapping.md @@ -0,0 +1,31 @@ +# Vector mapping — which test goes under which quality vector + +Every test belongs to **exactly one** vector. When a test seems to fit two, file it under the vector +whose *risk* it addresses, or split it into two tests. + +## The five vectors and what belongs to each + +| Vector | Guiding question | Tests that live here | Common miss | +|---|---|---|---| +| **Efficiency** | What does it cost to *run* Insight, and how does it scale? | Reference-org compute footprint (CPU/mem); resource utilization per service/connector; soak tests for resource growth/leaks. Storage is negligible — lead with compute. | Test *coverage* is **not** Efficiency — it's Reliability. Delivery-pipeline speed is an engineering-process signal, not product Efficiency. | +| **Reliability** | Can the user trust the dashboard — right, current, up? | The **differential/parity gate** (port/migration/consolidation); reconciliation of a number against its evidence; correctness of the headline behavior; data validation; **pagination integrity** (true totals, no duplicates, no omissions); metric-definition loading/precedence/reconcile; fail-clear on invalid/oversized input; source freshness; sync success; **test coverage (unit / API / e2e)** as the leading indicator; service availability. | Don't bury the differential inside a generic "correctness" bullet — it's usually the headline, tag it `main gate`. Pagination gets filed under Efficiency because it involves volume, but "no duplicates, no omissions" is a correctness claim. | +| **Performance** | How fast at the reference org's scale? | Per-endpoint latency budgets (gate P95, track P99/P999); dashboard page-load via Lighthouse; interactive timing via Playwright. Measure on the **same reference-org fixture** as Efficiency. | No global averages — the data endpoints *are* the product, so budget per metric/endpoint. | +| **Security** | Is the surface safe? | Static + dependency scanning (Semgrep, Trivy — no critical); authn/authz; no secret/token leak; the security face of tenant isolation; input-validation guards (e.g. SQLi on a filter param). | The *data* face of tenant isolation (does one tenant's data appear for another) can also read as Reliability — pick one and don't double-list. | +| **Versatility** | How broad is the coverage — across sources, and across the catalog and its surfaces? | Per-source / per-connector coverage (does each source participate correctly?); vendor coverage; API-version currency; connector readiness (bronze→silver→gold, tested); org-chart sync across directory providers; and catalog breadth — every metric resolving across every UI view or surface that should render it. | A connector counts only when production-complete *and tested* — coverage means the metric actually resolves, not just that the connector exists. | + +## Cross-cutting note: "coverage" is not one thing +This mapping deliberately splits coverage by *what it covers*: +- **test-rigor** coverage (unit / API / metric-tests) → **Reliability** +- **breadth** coverage (connector readiness, catalog breadth) → **Versatility** +- **security-surface** coverage (dependency, secrets) → **Security** + +So "code coverage with e2e tests" → Reliability; "coverage of each connector for each metric" → +Versatility; "no critical Trivy findings" → Security. Assign by *what the number measures*, not by +the word "coverage". + +## The shared fixture +Efficiency and Performance are measured on **the same reference-organisation dataset** — a defined +user count, connector set and concurrency profile. Do not invent its size: take it from the perf +target the feature itself cites, name that number in the framing sentence, and reuse it across +both vectors so results compare run to run. Features that predate a shared fixture use their own +demo org (the examples here use a 3,000-person one); say which you measured on. From 89865889db0751240e5e6ff2080b2b27991e9765 Mon Sep 17 00:00:00 2001 From: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech> Date: Thu, 30 Jul 2026 16:13:45 +0800 Subject: [PATCH 2/5] refactor(skills): file-bug-insight reports observations, not root cause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The filer reproduces the defect and attaches what the reproduction produced. Diagnosis — which file, which layer, which expression — moves to whoever picks the issue up. A cause written by someone who reproduced the bug but did not write the code reads as authoritative, and the assignee spends their first hour ruling it out. - `## Root Cause` in the body template becomes `## Additional information`: counts at each layer, the API status and body, the log or dbt error verbatim, the same check on a state where it works — each labelled with what produced it, none of it translated into a claim about the cause. - Drop the "localize the fix from the actual code" section, the fleet layer table, and the code-link and fix-checklist guidance. A list of sites to change is a diagnosis, and naming one wrong costs more than naming none. - The medallion walk stays, reframed: run the same question at all three layers and report all three answers. Empty bronze still ends the report — nothing synced means no product defect. - Filing no longer requires knowing the layer. Attach what was observed at each layer reachable and leave the conclusion open. - Worked example rewritten: the decode error is pasted rather than explained, and the working admin endpoint is offered as a contrast the assignee can use, not as a theory about what differs. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech> --- .claude/skills/file-bug-insight/SKILL.md | 79 +++++++++++------------- 1 file changed, 36 insertions(+), 43 deletions(-) diff --git a/.claude/skills/file-bug-insight/SKILL.md b/.claude/skills/file-bug-insight/SKILL.md index 45f0066ff..dee9663d9 100644 --- a/.claude/skills/file-bug-insight/SKILL.md +++ b/.claude/skills/file-bug-insight/SKILL.md @@ -1,6 +1,6 @@ --- name: file-bug-insight -description: "File an Insight defect as a GitHub issue in constructorfabric/insight — triage against existing issues, gather evidence, localize the fix to the layer that owns it, draft a report that reads in under a minute, confirm, create, and verify the metadata landed. Use whenever the user asks to file/report/raise/log a bug, ticket, defect or issue, and trigger PROACTIVELY once an investigation has converged on 'this is broken and should be recorded' — don't wait for the words 'file a bug'. Also fires on 'log this', 'report it', 'this is broken, make a ticket', 'turn this into an issue', 'we should file two bugs for X and Y'. The repo is PUBLIC, so the default flow is draft → confirm → create and the body must be scrubbed of internal detail. Prefer this over the general `file-bug` skill for anything in the Insight product — dashboards, metrics, connectors, dbt, ClickHouse, identity, the Helm install — since it carries the medallion evidence walk, the layer localization and the live board IDs; the general skill is for a Constructor *platform* defect that belongs in YouTrack." +description: "File an Insight defect as a GitHub issue in constructorfabric/insight — triage against existing issues, gather evidence, collect what the reproduction produced, draft a report that reads in under a minute, confirm, create, and verify the metadata landed. It reports OBSERVATIONS ONLY — no investigation, no root-cause analysis, no naming the file or layer to fix; that is the assignee's job. Use whenever the user asks to file/report/raise/log a bug, ticket, defect or issue, and trigger PROACTIVELY once an investigation has converged on 'this is broken and should be recorded' — don't wait for the words 'file a bug'. Also fires on 'log this', 'report it', 'this is broken, make a ticket', 'turn this into an issue', 'we should file two bugs for X and Y'. The repo is PUBLIC, so the default flow is draft → confirm → create and the body must be scrubbed of internal detail. Prefer this over the general `file-bug` skill for anything in the Insight product — dashboards, metrics, connectors, dbt, ClickHouse, identity, the Helm install — since it carries the medallion evidence walk, the reproduction-data discipline and the live board IDs; the general skill is for a Constructor *platform* defect that belongs in YouTrack." disable-model-invocation: false user-invocable: true allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Skill, Agent, AskUserQuestion @@ -8,7 +8,9 @@ allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Skill, Agent, AskUserQuestio # File an Insight bug -Turn an observed defect into an issue someone else can act on in under a minute, carrying the evidence that proves it and a root cause traced from the real code. +Turn an observed defect into an issue someone else can act on in under a minute, carrying the data the reproduction produced. + +**Report what you saw, not why it happens.** Reproduce, collect, attach. Diagnosis — which file, which layer, which expression — belongs to whoever picks the issue up: they have the context to be right, and a confident wrong cause in the body sends them down it before they start. An observation you can defend beats an explanation you cannot. **The tracker is always `constructorfabric/insight` on GitHub** — inside this repo there is no routing decision to make. (A Constructor *platform* bug — APS, Learn, Proctor, a platform stand's auth or navigation — goes to YouTrack instead; that routing lives in the general `file-bug` skill, not here.) @@ -20,8 +22,8 @@ A finding may arrive already carrying a `verdict`, an `existing_issue` and a `la - **`verdict` must be `CONFIRMED`.** An `UNVERIFIED` finding is a hypothesis, and filing one spends a reader's attention on a maybe. Reproduce it yourself first, or hand it to the `qa-finding-refuter` agent where the fleet is installed. - **`existing_issue` must have been searched.** If it names an issue, comment on that issue instead of filing. If the match is *closed*, say so in the comment — a regression is more urgent than a new bug. -- **`layer: stand` is not a product bug.** A `join_use_nulls` view mismatch, a stale `schema_status` cache, an unseeded connector, a tenant mismatch — these are environment faults. File one only when the deploy path itself is the defect, and then it's a `deploy` bug about the chart or migration, not a metric bug. -- **`layer: unknown` is not filable.** Localize it first — walk the medallion by hand as below, or hand it to `qa-warehouse-analyst` where the fleet is installed — because the assignee and the grooming call both follow the layer. +- **`layer: stand` is not a product bug.** A `join_use_nulls` view mismatch, a stale `schema_status` cache, an unseeded connector, a tenant mismatch — these are environment faults, and the reproduction shows it: the same check passes on a correctly populated instance. Record that and stop. +- **A missing `layer` is not a blocker.** You do not have to know where the fix lands in order to file. Attach what you observed at each layer you could reach and leave the conclusion open. ## Companion skills @@ -31,10 +33,10 @@ Each of these owns a slice of the work. Some are still being built out here, so |---|---|---| | `playwright-cli` | the browser command surface — snapshots, refs, clicks, screenshots, console, network | exploring a stand or reproducing any UI defect | | `drive-ui` | getting an *authenticated* browser on any stand — fakeidp and the `DEV_USER_EMAIL` seed locally, a passkey attach on a remote one — plus the routes and the evidence set | any UI defect, local or remote | -| `metric-parity` | the full bronze → silver → gold walk | localizing a wrong number to a layer | +| `metric-parity` | the full bronze → silver → gold walk | collecting the same query at every layer | | `release-verify` | install and seed health | settling "product bug, or empty instance?" | -One check belongs here rather than in `drive-ui`, because getting it wrong misroutes the bug: before calling a wrong on-screen value a frontend defect, look at the browser console and the API response behind it (`playwright-cli console`, then `requests` and `request <n>`). If the API already returned the wrong number, the layer is `analytics` or below and the UI is only the messenger. +One collection step belongs here rather than in `drive-ui`: whenever a value on screen looks wrong, capture the browser console and the API response behind it (`playwright-cli console`, then `requests` and `request <n>`) and attach both. Whether the wrong number arrived from the API or was rendered wrong is the single most useful fact in the report — and it is an observation, not a diagnosis, as long as you paste what the response actually contained. ## Triage — before you gather @@ -48,11 +50,11 @@ gh issue list --repo constructorfabric/insight --state all --search "<key phrase **Keep the limit high.** Closed issues rank after every open one, so a short window returns open matches only — a search for "threshold" gives 30 open and 0 closed at `--limit 30`, and 57 open plus 43 closed at 100. A closed match is the *more* urgent finding, since it means a regression. -Search more than once with different vocabulary — the metric key, the field name, the group title, the error code, the user-visible label. Same defect → add your evidence to the existing issue. Genuinely different root cause or fix site → file new and cross-link with a one-line `related to #N` (a bare link, not a "how this differs" writeup — that reads as noise). +Search more than once with different vocabulary — the metric key, the field name, the group title, the error code, the user-visible label. Same defect → add your evidence to the existing issue. A genuinely different symptom → file new and cross-link with a one-line `related to #N` (a bare link, not a "how this differs" writeup — that reads as noise). **Product bug, or environment artifact?** A metric that is empty because nothing was seeded or synced is not a product defect. File only what would still be wrong on a correctly populated instance. The cheapest check is the bottom of the medallion: no bronze rows for that connector and window means a seed or sync gap, so stop. (`release-verify` sweeps this for the whole install where it exists.) -**One bug or several?** One issue per distinct root cause and fix site. Split a shared symptom with different causes; use a fix checklist for several touch-points of the *same* fix. +**One bug or several?** One issue per distinct reproduction. Two symptoms that need different steps to trigger are two issues; the same symptom reached by two paths is one issue with both paths in Steps. Where you cannot tell, file one and say what else you saw — merging beats splitting a single defect across two threads. ## Gather evidence — never write from memory @@ -60,7 +62,7 @@ Collect first, write second. The evidence must let someone else reproduce this. **Artifacts do not go in this repo.** Nothing in this tree is gitignored for scratch output — `scratch/`, `tmp/`, `artifacts/` are merely untracked, so a screenshot or a body file left behind surfaces in someone's `git status` and rides along on the next `git add -A`. Write evidence and the issue body to the session scratchpad directory your environment names, or to a fresh `mktemp -d`; that is what the `--body-file` path below assumes. (`../insight-workspace/scratch/` also works when that checkout sits alongside this one.) -- **Data / metric bugs** — trace the medallion to where the value *first* goes wrong. Empty **bronze** means a sync or seed artifact, not a bug. Rows in bronze dropped at **silver** is a staging bug. Rows in silver but wrong in **gold** is a model or view bug. Don't file "gold is broken" when the story is "nothing upstream". +- **Data / metric bugs** — run the same question at all three layers and record all three answers, even the ones that look normal. A reader who sees rows at bronze and silver but not at gold learns more from those three counts than from any sentence you could write about them. Empty **bronze** is the one case that ends the report: nothing was synced or seeded, so there is no product defect to file. ```sh CH=(docker exec insight-clickhouse clickhouse-client -u insight --password "${CLICKHOUSE_PASSWORD:-insight-local}") "${CH[@]}" -q "SELECT … FROM insight.<gold> WHERE …" # gold — served @@ -73,36 +75,11 @@ Collect first, write second. The evidence must let someone else reproduce this. - **Pipeline / config bugs with no UI** — the failure signal itself: the exact error and stack, or a row-count contrast that runs the code's own filter (returns 0) against the unfiltered count (>0). **If the failure is silent** — completes "successfully" with zero effect — say so explicitly. That is the key symptom. - **What the metric is *supposed* to do** lives in `docs/domain/metrics/specs/DESIGN.md` and the model under `src/ingestion/`. Read the intent before calling behaviour wrong. -## Localize the fix from the actual code - -Naming *where the fix lands* is what makes an issue actionable and routes it to an owner. Read and quote the real code; never infer a formula from a metric's name. Use "X-side, not Y-side" when it disambiguates a layer — *"client-side, not API-side: the API correctly returns 403; the SPA renders the menu entry unconditionally."* - -Gold is defined in two places and they are not interchangeable: the dbt models in `src/ingestion/gold/` materialize the measure observation tables, while the `insight.*` views and marts are created by the migrations in `src/ingestion/scripts/migrations/`. Views get redefined across several migrations — grep them all and read the **latest-timestamped** one before quoting a formula. - -Verify every reference against `main` before linking it; your worktree may differ: - -```sh -gh api "repos/constructorfabric/insight/contents/<path>?ref=main" \ - -H "Accept: application/vnd.github.raw" | grep -nE '<pattern>' -``` - ## Type and priority - **Issue Type = `Bug`** — the native type (`--type "Bug"`), never a `bug` label. - **Priority is the Insight #40 project *field*, not a label.** Never add `priority:*`. Options: `Blocker` (blocks the next installable release), `High` (meaningful demo features), `Medium` (default). Suggest a level and confirm it. -- **Don't label.** Component, team, release and planning labels are applied during grooming by the people who own that call, and a wrong one routes the bug to the wrong team. Name the owning layer in Root Cause instead, in words. - -Naming the layer is still your job — it just belongs in Root Cause, in words, not in a label: - -| Fleet `layer` | Symptom shape | -|---|---| -| `frontend` | Correct in ClickHouse, renders wrong — axis, colours, series, formatting, null-vs-zero | -| `analytics` | 500s, wrong filter or bucketing in the serving path, wrong measure binding | -| `ingestion` | Wrong at or before gold: bad dedup, dropped rows, wrong view expression, schema drift | -| `ingestion` (identity) | People or org empty / mis-resolved, producer↔consumer mismatch | -| `stand` (deploy path only) | Broken on a fresh install: missing config or wiring, chart or secret gap | - -The layer follows the *fix*, not the symptom — a wrong number from a gold view is an ingestion bug even though it surfaces in the UI. +- **Don't label.** Component, team, release and planning labels are applied during grooming by the people who own that call, and a wrong one routes the bug to the wrong team. Describe the symptom; the owning team is identified during grooming. ## Body template — four headings @@ -125,22 +102,30 @@ detail, no history, no scope. A reader triaging a list often reads only this lin stops a reviewer waving off a real defect as missing data. If it only reproduces from a given state, name the STATE ("a freshly migrated database"), never the environment.> -## Root Cause -<2–4 sentences. Name the file, view or expression and quote it, each reference linked to `main`.> +## Additional information +<The data the reproduction produced, and nothing you inferred from it. Whatever you ran and what +it returned: counts at each layer, the API status and response body, the log or dbt error, the +same check on a state where it works. Label each one with what produced it. If a value looks +wrong, give the value you saw and the value the spec or the UI led you to expect — not a theory +about where it went wrong.> ## Notes ← optional, one line (e.g. `related to #N`) ``` +**No `## Root Cause` heading.** It used to be in this template, and removing it is the point of the observations-only rule: a cause written by someone who reproduced the bug but did not write the code reads as authoritative, and the assignee spends their first hour ruling it out. What you observed goes under Additional information; what it means is theirs to decide. + **No `## Impact` heading.** It restates the Summary in longer words. Affected instances or states go next to the evidence in Steps; a knock-on effect is one line in Notes. Wanting the heading back means the Summary sentence is not carrying its weight. -Additive when they sharpen the report: an **Examples** table (current-wrong → correct) for a rule, threshold, sign or mapping bug; a **Fix checklist** with each site linked when the fix spans several places. Link code, don't paste strings: `[file.ext#L79](https://github.com/constructorfabric/insight/blob/main/<path>#L79)`. +Additive when it sharpens the report: an **Examples** table (observed → expected) for a rule, threshold, sign or mapping bug. No fix checklist and no code links — a list of sites to change is a diagnosis, and naming one wrong is worse than naming none. ## Write plainly One idea per sentence. Short declarative lines a tired on-call reader parses on the first pass. If a sentence has more than one comma-joined clause plus a dash-aside, split it. State what happens, then why. - ✗ *"Deploy-side, not migration-side: the hook is skipped/lost on a successful fresh install while Helm reports success, leaving the gold layer unbuilt (install-time logs were unavailable — the Job leaves no trace because it never ran)."* -- ✓ *"The post-install hook never runs on a fresh install. Helm still reports success. No hook Job or Pod is ever created, so the gold layer stays unbuilt. The fix is deploy-side — the migration script works when run by hand."* +- ✓ *"On a fresh install the gold layer stays unbuilt. Helm reports success. No hook Job or Pod exists afterwards. Running the migration script by hand builds the layer."* + +The second version is also the shape this skill asks for: four things observed, no claim about which side owns the fix. **Say each fact once.** Every fact lives in exactly one section. Repetition teaches the reader to skim, and skimming is how the one load-bearing line gets missed. @@ -175,13 +160,21 @@ A real filed bug, condensed. Read it for calibration on how little text a comple > > The row is inserted despite the 500 — `SELECT field_name, operator, value FROM thresholds` returns it. The write path works and the read path does not, which is why one successful-looking create disables the endpoint for good. Reproduces on a freshly migrated database with no other data. > -> ## Root Cause -> The `value` column is `DECIMAL(20,6)` ([`m20260414_000001_init.rs#L86`](https://github.com/constructorfabric/insight/blob/main/src/backend/services/analytics/src/migration/m20260414_000001_init.rs#L86)) but the entity maps it to `f64` ([`entities.rs#L39`](https://github.com/constructorfabric/insight/blob/main/src/backend/services/analytics/src/infra/db/entities.rs#L39)), and sqlx-mysql cannot decode `NEWDECIMAL` into `f64` — the `column_type = "Decimal(…)"` annotation on the field does not change how the value is read back. Writes coerce server-side; reads fail during decode, and the handler wraps that as internal ([`handlers.rs#L1070`](https://github.com/constructorfabric/insight/blob/main/src/backend/services/analytics/src/api/handlers.rs#L1070)). The newer admin threshold table reads the same shape safely by casting in raw SQL. +> ## Additional information +> - Every later read of that metric's thresholds returns the same 500: the create's read-back, the list, an update and a delete. +> - The row is in the table — `SELECT field_name, operator, value FROM thresholds` returns it with `value = 1.000000`. +> - The admin threshold endpoints (`metric_threshold`) accept and return the same shape on the same instance, with no error. +> - Server log, verbatim, at the moment of the failed read: +> ``` +> failed to list thresholds error=Query Error: error occurred while decoding column +> "value": mismatched types; Rust type `core::option::Option<f64>` (as SQL type +> `DOUBLE`) is not compatible with SQL type `DECIMAL` +> ``` > > ## Notes -> `metric_threshold` (admin) is unaffected — different read path. +> Found by the endpoint contract suite; the affected tests are currently skipped against this issue. -Three things that example gets right, and they are the ones reports usually miss. The title is a symptom a user could have reported, with the diagnosis left for Root Cause. The "row is inserted despite the 500" line is load-bearing — without it a triager reads a 500 as a flaky write and moves on. And Root Cause stops at *where the defect is*, naming the safe read path as a hint without prescribing the fix. +Three things that example gets right, and they are the ones reports usually miss. The title is a symptom a user could have reported. The "row is in the table" line is load-bearing — without it a triager reads a 500 as a flaky write and moves on. And every line under Additional information is something that was *run and observed*: the decode error is pasted, not paraphrased, and the working admin endpoint is offered as a contrast the assignee can use — not as a theory about what differs. ## Scrub the body From 3dda34b8834b84820fe2e0e6f97a141fd29309e7 Mon Sep 17 00:00:00 2001 From: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech> Date: Thu, 30 Jul 2026 16:18:39 +0800 Subject: [PATCH 3/5] refactor(skills): observations, not conclusions, in drive-ui and quality-vector-tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Carries the file-bug-insight rule across the two skills that feed it. drive-ui - "Before you blame the UI" becomes "Capture what is behind the number". A wrong value on screen may or may not come from the frontend, and the driver does not have to decide: capture the console and the API response, attach the on-screen value beside the returned one, and leave what they imply to whoever picks the issue up. - When no browser is available, say what could not be verified and stop. Reading the frontend source to work out what the page would have shown produces a conclusion, not an observation. "Not visually confirmed" is a complete answer. quality-vector-tests - A Testing section states what gets measured and how, never why something is broken or where a fix would go. A defect found while grounding belongs in its own issue via file-bug-insight — the two artifacts have different readers, and a diagnosis buried in a feature's Testing section reaches neither. - Grounding reports what was found rather than what it might mean: "the endpoint takes 4 filters, the issue lists 2" is checkable; "the filter work was descoped" is a story about people you did not talk to. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech> --- .claude/skills/drive-ui/SKILL.md | 10 +++++----- .claude/skills/quality-vector-tests/SKILL.md | 9 ++++++++- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/.claude/skills/drive-ui/SKILL.md b/.claude/skills/drive-ui/SKILL.md index f6b376341..b24127719 100644 --- a/.claude/skills/drive-ui/SKILL.md +++ b/.claude/skills/drive-ui/SKILL.md @@ -1,6 +1,6 @@ --- name: drive-ui -description: "Drive the Insight web UI in a real browser to see, verify, or capture evidence from any stand — a local compose or kind install, or a shared remote one. Use this whenever the task means LOOKING at the dashboard rather than reading its code: 'check the IC page', 'is that chart still broken', 'screenshot the metrics drilldown', 'reproduce it in the UI', 'grab evidence for a bug', 'open the stand and look at X', or any UI defect you are about to file. Read it BEFORE launching a browser at any remote stand, because the Entra-plus-passkey ones cannot be logged into from a browser you launched, and the wrong acquisition move costs the user a login they cannot complete. Also read it before blaming the UI for a wrong number — the check that settles it is here. The `playwright-cli` skill owns the commands; this skill owns getting an authenticated browser and capturing evidence someone can act on, and hands the issue itself to `file-bug-insight`." +description: "Drive the Insight web UI in a real browser to see, verify, or capture evidence from any stand — a local compose or kind install, or a shared remote one. Use this whenever the task means LOOKING at the dashboard rather than reading its code: 'check the IC page', 'is that chart still broken', 'screenshot the metrics drilldown', 'reproduce it in the UI', 'grab evidence for a bug', 'open the stand and look at X', or any UI defect you are about to file. Read it BEFORE launching a browser at any remote stand, because the Entra-plus-passkey ones cannot be logged into from a browser you launched, and the wrong acquisition move costs the user a login they cannot complete. Also read it before reporting a wrong number as a UI defect — the data that decides it is captured here, and this skill collects observations rather than drawing conclusions from them. The `playwright-cli` skill owns the commands; this skill owns getting an authenticated browser and capturing evidence someone can act on, and hands the issue itself to `file-bug-insight`." disable-model-invocation: false user-invocable: true allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Skill @@ -85,16 +85,16 @@ Group cards expose `aria-label="Open <group title> details"`, which makes them s playwright-cli click "getByRole('button', { name: 'Open Git output details' })" ``` -## Before you blame the UI +## Capture what is behind the number -A wrong number on screen is usually not a frontend bug. Two commands settle it, and skipping them is how a gold-view defect gets filed against the SPA: +A wrong number on screen may or may not come from the frontend, and you do not have to decide which. Two commands capture the evidence that decides it, and skipping them is how a gold-view defect gets filed against the SPA: ```sh playwright-cli console # client-side errors behind a broken or empty widget playwright-cli requests # then `request <n>` for the failing call's status and body ``` -If the API already returned the wrong value, the layer is `analytics` or below and the UI is just the messenger. `metric-parity` walks it the rest of the way down the medallion. +Attach both to the report: the value on screen, and the value the API actually returned, pasted rather than characterised. Those two facts side by side are what a reader needs — leave what they imply to whoever picks the issue up. `metric-parity` collects the same question further down the medallion when you can reach it. ## Capture evidence someone can act on @@ -114,4 +114,4 @@ Annotate before capturing rather than describing the element in prose afterwards ## When you can't get a browser -The user may not have time to flip a toggle or sign in, and that is a normal outcome rather than a blocker. Say plainly what you could not verify, then get what you can from the code: dashboard composition in `insight-front` is data-driven, which often settles "are these two charts really the same" with no browser at all. A finding grounded in code plus an explicit "not visually confirmed" is honest and useful. A finding that implies visual confirmation it never got is neither. +The user may not have time to flip a toggle or sign in, and that is a normal outcome rather than a blocker. Say plainly what you could not verify, and stop there. Reading the frontend source to work out what the page *would* have shown produces a conclusion, not an observation — and this skill exists to produce observations. "Not visually confirmed" is a complete and useful answer; a claim that implies visual confirmation it never got is neither honest nor useful. diff --git a/.claude/skills/quality-vector-tests/SKILL.md b/.claude/skills/quality-vector-tests/SKILL.md index 339d6f465..da5e2b3c7 100644 --- a/.claude/skills/quality-vector-tests/SKILL.md +++ b/.claude/skills/quality-vector-tests/SKILL.md @@ -71,7 +71,9 @@ Two things this grounding is *for*, beyond correctness: the section. - **Whether the feature's own framing still holds.** When the shipped code contradicts the issue — scope was dropped, scope was added, a "table" shipped as a general-purpose component — correct - the checks against reality and say so to the user. + the checks against reality and say so to the user. Report what you found, not what you think it + means: "the endpoint takes 4 filters, the issue lists 2" is checkable; "the filter work was + descoped" is a story about people you did not talk to. If the feature is a **port**, **consolidation**, or **rolling migration**, the headline check is almost always a **differential / parity gate** (old vs new on the same data) — and it is frequently @@ -112,6 +114,11 @@ under Efficiency because it involves volume. Follow the template below. Then edit it into the issue body, replacing the loose Testing block and preserving everything else (Goal, Scope, Acceptance, Planning). +The section states what gets measured and how — never why something is broken or where a fix would +go. Grounding often turns up a real defect; that belongs in its own issue via `file-bug-insight`, +not in a paragraph here. Two different readers use these two artifacts, and a diagnosis buried in a +feature's Testing section reaches neither. + ### 5. Draft-or-push Show the checks for review first whenever you are changing what is tested — adding a gate, moving an item between vectors, or setting numbers the user hasn't seen. A compact table is the fastest From cbb5322bd9826166a7dc6678171b8b4a03060e07 Mon Sep 17 00:00:00 2001 From: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech> Date: Thu, 30 Jul 2026 16:21:45 +0800 Subject: [PATCH 4/5] =?UTF-8?q?fix(skills):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20session=20propagation,=20auth=20state,=20closed-issue=20sear?= =?UTF-8?q?ch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drive-ui - `attach --cdp=chrome` creates a session named after the channel, not `default` (session-management.md documents this). Unqualified `goto` or `snapshot` afterwards talks to a different browser or none, so export PLAYWRIGHT_CLI_SESSION or prefix with `-s=chrome`. - Saved auth state moves out of $EVIDENCE to ~/.playwright-auth (mode 700). $EVIDENCE is what gets dragged into a public issue, and a state file is live session cookies. file-bug-insight - Search open and closed separately. In a combined `--state all` query, closed matches rank after every open one and fall off the end of the window; a higher limit only postpones it. Two queries guarantee both. - Worked example no longer says the same thing twice: the server log was pasted in both Steps and Additional information, and two facts appeared in both the prose paragraph and the bullets — in the example that is supposed to demonstrate "say each fact once". The template now sends verbatim output to Additional information and keeps Steps to actions and outcomes. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech> --- .claude/skills/drive-ui/SKILL.md | 10 +++++++--- .claude/skills/file-bug-insight/SKILL.md | 18 +++++++----------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.claude/skills/drive-ui/SKILL.md b/.claude/skills/drive-ui/SKILL.md index b24127719..ddcdfa98f 100644 --- a/.claude/skills/drive-ui/SKILL.md +++ b/.claude/skills/drive-ui/SKILL.md @@ -44,21 +44,25 @@ A passkey is bound to the user's real Chrome profile and its platform authentica ```sh playwright-cli attach --cdp=chrome # preferred: one checkbox, no restart playwright-cli attach --extension=chrome # when CDP is blocked or the user prefers it +export PLAYWRIGHT_CLI_SESSION=chrome # every later command now targets that session ``` +**Attach names the session after the channel, not `default`.** `--cdp=chrome` creates a session called `chrome`, so an unqualified `goto` or `snapshot` afterwards talks to a different browser — or none. Either export `PLAYWRIGHT_CLI_SESSION` as above, or prefix each command (`playwright-cli -s=chrome snapshot`). + If CDP fails with `Could not connect to chrome: DevToolsActivePort file not found`, the user has to open `chrome://inspect/#remote-debugging` and tick **"Allow remote debugging for this browser instance"**. Open the page for them if you like — `open -a "Google Chrome" "chrome://inspect/#remote-debugging"` — but the click is theirs. Their windows, tabs and session all survive, which is why this beats every alternative. **Never suggest restarting Chrome with `--remote-debugging-port`.** Chrome 136+ refuses remote debugging on the *default* profile, so the restart either fails or hands you an empty logged-out profile — which is how you end up asking for a passkey you cannot get. **Detach, don't close.** While attached you are holding the user's own browser, and `playwright-cli close` closes *their* windows. Use `playwright-cli -s=<session> detach`. `playwright-cli list` tells you which sessions you launched yourself; those are the only ones safe to kill. -Bank the session once you have it, so later runs skip auth entirely: +Bank the session once you have it, so later runs skip auth entirely — but keep it out of `$EVIDENCE`. That directory gets attached to a public issue, and a saved state file is live session cookies: ```sh -playwright-cli state-save "$EVIDENCE/stand-state.json" # then state-load next time +mkdir -p ~/.playwright-auth && chmod 700 ~/.playwright-auth +playwright-cli -s=chrome state-save ~/.playwright-auth/stand-state.json # state-load next time ``` -Re-save when it goes stale — the symptom is landing back on a login page. +Re-save when it goes stale — the symptom is landing back on a login page — and delete it when the investigation ends. ## Know when the page is actually ready diff --git a/.claude/skills/file-bug-insight/SKILL.md b/.claude/skills/file-bug-insight/SKILL.md index dee9663d9..265218d9c 100644 --- a/.claude/skills/file-bug-insight/SKILL.md +++ b/.claude/skills/file-bug-insight/SKILL.md @@ -45,10 +45,11 @@ Three checks that routinely change the plan. **Search first.** Never file a blind duplicate: ```sh -gh issue list --repo constructorfabric/insight --state all --search "<key phrase>" --limit 100 +gh issue list --repo constructorfabric/insight --state open --search "<key phrase>" --limit 50 +gh issue list --repo constructorfabric/insight --state closed --search "<key phrase>" --limit 50 ``` -**Keep the limit high.** Closed issues rank after every open one, so a short window returns open matches only — a search for "threshold" gives 30 open and 0 closed at `--limit 30`, and 57 open plus 43 closed at 100. A closed match is the *more* urgent finding, since it means a regression. +**Search the two states separately.** In a combined `--state all` search, closed issues rank after every open one, so they fall off the end of the window: "threshold" returns 30 open and 0 closed at `--limit 30`, and raising the limit only postpones the problem — enough open matches still crowd them out. Two queries guarantee you see both. A closed match is the *more* urgent finding, since it means a regression. Search more than once with different vocabulary — the metric key, the field name, the group title, the error code, the user-visible label. Same defect → add your evidence to the existing issue. A genuinely different symptom → file new and cross-link with a one-line `related to #N` (a bare link, not a "how this differs" writeup — that reads as noise). @@ -91,8 +92,8 @@ detail, no history, no scope. A reader triaging a list often reads only this lin ## Steps to Reproduce 1. <UI path, or the fastest isolated check — one query or command> 2. <what to observe> -3. <When the failure emits anything — exception, stack, HTTP status and body, dbt or ClickHouse - error — paste it verbatim in a fenced block, trimmed to the lines that identify the defect.> +3. <what the call or query returned — the status, the visible result. The verbatim error text goes + under Additional information, so this stays a list of actions and outcomes.> **Expected:** <one line> **Actual:** <one line — the failure at that step, NOT a restatement of Summary> @@ -148,17 +149,12 @@ A real filed bug, condensed. Read it for calibration on how little text a comple > ## Steps to Reproduce > 1. Create a metric, then `POST /v1/metrics/{id}/thresholds` with any valid body. > 2. Read them back: `GET /v1/metrics/{id}/thresholds`. -> 3. Both calls return 500 `application/problem+json`, and the log names the decode: -> ``` -> failed to list thresholds error=Query Error: error occurred while decoding column -> "value": mismatched types; Rust type `core::option::Option<f64>` (as SQL type -> `DOUBLE`) is not compatible with SQL type `DECIMAL` -> ``` +> 3. Both calls return 500 `application/problem+json` (server log below). > > **Expected:** 201 with the created threshold, then 200 with the list. > **Actual:** 500 on the create and on every later read of that metric's thresholds. > -> The row is inserted despite the 500 — `SELECT field_name, operator, value FROM thresholds` returns it. The write path works and the read path does not, which is why one successful-looking create disables the endpoint for good. Reproduces on a freshly migrated database with no other data. +> Reproduces on a freshly migrated database with no other data. > > ## Additional information > - Every later read of that metric's thresholds returns the same 500: the create's read-back, the list, an update and a delete. From cc56575a86d73a819439cffbd7ad7dfd742d6ad1 Mon Sep 17 00:00:00 2001 From: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech> Date: Thu, 30 Jul 2026 17:46:18 +0800 Subject: [PATCH 5/5] chore(skills): drop the test-generation reference from playwright-cli Removes references/test-generation.md and the five pointers to it (one in SKILL.md, four in spec-driven-testing.md, including its cross-reference table row). The remaining nine references stand alone; spec-driven-testing still points at playwright-tests.md for the debug/attach mechanics. Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech> --- .claude/skills/playwright-cli/SKILL.md | 1 - .../references/spec-driven-testing.md | 7 +- .../references/test-generation.md | 134 ------------------ .claude/skills/quality-vector-tests/SKILL.md | 4 +- 4 files changed, 5 insertions(+), 141 deletions(-) delete mode 100644 .claude/skills/playwright-cli/references/test-generation.md diff --git a/.claude/skills/playwright-cli/SKILL.md b/.claude/skills/playwright-cli/SKILL.md index 252e62ce4..7a0a73d38 100644 --- a/.claude/skills/playwright-cli/SKILL.md +++ b/.claude/skills/playwright-cli/SKILL.md @@ -398,7 +398,6 @@ playwright-cli show --annotate * **Browser session management** [references/session-management.md](references/session-management.md) * **Spec-driven testing (plan / generate / heal)** [references/spec-driven-testing.md](references/spec-driven-testing.md) * **Storage state (cookies, localStorage)** [references/storage-state.md](references/storage-state.md) -* **Test generation** [references/test-generation.md](references/test-generation.md) * **Tracing** [references/tracing.md](references/tracing.md) * **Video recording** [references/video-recording.md](references/video-recording.md) * **Inspecting element attributes** [references/element-attributes.md](references/element-attributes.md) diff --git a/.claude/skills/playwright-cli/references/spec-driven-testing.md b/.claude/skills/playwright-cli/references/spec-driven-testing.md index 336dbfc13..43ffb9b57 100644 --- a/.claude/skills/playwright-cli/references/spec-driven-testing.md +++ b/.claude/skills/playwright-cli/references/spec-driven-testing.md @@ -6,7 +6,7 @@ End-to-end workflow for authoring and maintaining Playwright tests using `playwr - **Generate** — turn a spec into Playwright test files. Update the spec if it's vague or stale. - **Heal** — diagnose failing tests, fix the code, reconcile the spec with reality. -All three lean on the same mechanic: run `npx playwright test --debug=cli` in the background, then `playwright-cli attach tw-XXXX` to drive the paused page interactively. See [playwright-tests.md](playwright-tests.md) for the debug/attach mechanics and [test-generation.md](test-generation.md) for how every `playwright-cli` action emits Playwright TypeScript. +All three lean on the same mechanic: run `npx playwright test --debug=cli` in the background, then `playwright-cli attach tw-XXXX` to drive the paused page interactively. See [playwright-tests.md](playwright-tests.md) for the debug/attach mechanics. --- @@ -173,7 +173,7 @@ playwright-cli attach tw-XXXX Walk the scenario's `Steps:` one by one with `playwright-cli`, treating the spec as the plan and the live app as the source of truth. If a step is vague ("click the button" — which button?), references an element that no longer exists, or contradicts the app's actual behaviour, use your judgement: update the spec to match what the app really does, then keep going. Editing the spec mid-generation is expected. -Every action prints the equivalent Playwright TypeScript (see [test-generation.md](test-generation.md)): +Every action prints the equivalent Playwright TypeScript: ```bash playwright-cli snapshot # find refs @@ -182,7 +182,7 @@ playwright-cli press Enter playwright-cli click e7 ``` -For each `- expect:` bullet, add an explicit assertion. See [test-generation.md](test-generation.md) for details. +For each `- expect:` bullet, add an explicit assertion. Collect the generated code and write the test file at the path given in the spec: @@ -300,6 +300,5 @@ Only after the user answers, either update the spec (intentional change) or file | For... | See | |---|---| | `--debug=cli` / attach mechanics | [playwright-tests.md](playwright-tests.md) | -| How `playwright-cli` actions become TS | [test-generation.md](test-generation.md) | | Mocking requests during exploration/generation | [request-mocking.md](request-mocking.md) | | Managing the CLI browser session | [session-management.md](session-management.md) | diff --git a/.claude/skills/playwright-cli/references/test-generation.md b/.claude/skills/playwright-cli/references/test-generation.md deleted file mode 100644 index a045c55d6..000000000 --- a/.claude/skills/playwright-cli/references/test-generation.md +++ /dev/null @@ -1,134 +0,0 @@ -# Test Generation - -Generate Playwright test code automatically as you interact with the browser. - -## How It Works - -Every action you perform with `playwright-cli` generates corresponding Playwright TypeScript code. -This code appears in the output and can be copied directly into your test files. - -## Example Workflow - -```bash -# Start a session -playwright-cli open https://example.com/login - -# Take a snapshot to see elements -playwright-cli snapshot -# Output shows: e1 [textbox "Email"], e2 [textbox "Password"], e3 [button "Sign In"] - -# Fill form fields - generates code automatically -playwright-cli fill e1 "user@example.com" -# Ran Playwright code: -# await page.getByRole('textbox', { name: 'Email' }).fill('user@example.com'); - -playwright-cli fill e2 "password123" -# Ran Playwright code: -# await page.getByRole('textbox', { name: 'Password' }).fill('password123'); - -playwright-cli click e3 -# Ran Playwright code: -# await page.getByRole('button', { name: 'Sign In' }).click(); -``` - -## Building a Test File - -Collect the generated code into a Playwright test: - -```typescript -import { test, expect } from '@playwright/test'; - -test('login flow', async ({ page }) => { - // Generated code from playwright-cli session: - await page.goto('https://example.com/login'); - await page.getByRole('textbox', { name: 'Email' }).fill('user@example.com'); - await page.getByRole('textbox', { name: 'Password' }).fill('password123'); - await page.getByRole('button', { name: 'Sign In' }).click(); - - // Add assertions - await expect(page).toHaveURL(/.*dashboard/); -}); -``` - -## Best Practices - -### 1. Use Semantic Locators - -The generated code uses role-based locators when possible, which are more resilient: - -```typescript -// Generated (good - semantic) -await page.getByRole('button', { name: 'Submit' }).click(); - -// Avoid (fragile - CSS selectors) -await page.locator('#submit-btn').click(); -``` - -### 2. Explore Before Recording - -Take snapshots to understand the page structure before recording actions: - -```bash -playwright-cli open https://example.com -playwright-cli snapshot -# Review the element structure -playwright-cli click e5 -``` - -### 3. Add Assertions Manually - -Generated code captures actions but not assertions. Add expectations in your test using one of the recommended matchers: - -- `toBeVisible()` — element is rendered and visible -- `toHaveText(text)` — element text content matches -- `toHaveValue(value) / toBeEmpty()` — input/select value matches -- `toBeChecked() / toBeUnchecked()` — checkbox state matches -- `toMatchAriaSnapshot(snapshot)` — page (or locator) matches a partial accessibility snapshot - -Use `playwright-cli generate-locator <target>` to produce the locator expression for the assertion, and the snapshot/eval commands to capture the expected value. - -When asserting text content, make sure that generated locator does not contain text from the element itself. `getByTestId()` or `getByLabel()` usually work well with asserting text. When locator is text-based, prefer `toBeVisible()` instead. - -Snapshot to be matched does not have to contain all the information - only capture what's necessary for the assertion. You can use regular expressions for unstable values. - -```bash -# Get a stable locator for an element ref to use in the assertion -playwright-cli --raw generate-locator e5 -# getByRole('button', { name: 'Submit' }) - -# Capture expected text content for toHaveText -playwright-cli --raw eval "el => el.textContent" e5 - -# Capture expected input value for toHaveValue/toBeEmpty -playwright-cli --raw eval "el => el.value" e5 - -# Capture expected aria snapshot for toMatchAriaSnapshot/toBeChecked -# (whole page, or use a ref to scope to a region) -playwright-cli --raw snapshot -playwright-cli --raw snapshot e5 -``` - -```typescript -// Generated action -await page.getByRole('button', { name: 'Submit' }).click(); - -// Manual assertions using the outputs above: -await expect(page.getByRole('alert', { name: 'Success' })).toBeVisible(); -await expect(page.getByTestId('main-header')).toHaveText('Welcome, user'); -await expect(page.getByRole('textbox', { name: 'Email' })).toHaveValue('user@example.com'); -await expect(page.getByRole('checkbox', { name: 'Enable notifications' })).toBeChecked(); - -// toMatchAriaSnapshot on the whole page, finds a matching region -await expect(page).toMatchAriaSnapshot(` - - heading "Welcome, user" - - link /\\d+ new messages?/ - - button "Sign out" -`); - -// toMatchAriaSnapshot scoped to a region -await expect(page.getByRole('navigation')).toMatchAriaSnapshot(` - - link "Home" - - link /\\d+ new messages?/ - - link "Profile" -`); -``` diff --git a/.claude/skills/quality-vector-tests/SKILL.md b/.claude/skills/quality-vector-tests/SKILL.md index da5e2b3c7..73a1ba1a9 100644 --- a/.claude/skills/quality-vector-tests/SKILL.md +++ b/.claude/skills/quality-vector-tests/SKILL.md @@ -58,8 +58,8 @@ re-targeted — read the issue body and go straight to step 3. Grounding costs t reading, and it earns that only when you are changing *what gets tested*. Otherwise, don't invent a generic checklist. Read the issue and the actual implementation the same way -`scope-feature-tests` does — pull the issue (`gh issue view <n> --repo constructorfabric/insight ---json title,body,labels,parent`), check for a branch or merged PR (`gh pr list --repo constructorfabric/insight --search "<n>" +`scope-feature-tests` does — pull the issue (`gh issue view n --repo constructorfabric/insight +--json title,body,labels,parent`), check for a branch or merged PR (`gh pr list --repo constructorfabric/insight --search "n" --state all`), then read the code — this repo for backend, ingestion and dbt, and the sibling `../insight-front` for UI.