Conversation
📝 WalkthroughWalkthroughThis PR adds Playwright CLI reference documentation, revises UI investigation and observation-only bug filing guidance, and introduces quality-vector testing instructions with worked examples for measurable Insight issue criteria. ChangesPlaywright CLI Documentation
Insight UI Investigation and Bug Filing
Quality-Vector Testing Documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (10)
.claude/skills/playwright-cli/references/running-code.md-39-42 (1)
39-42: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the geolocation cleanup example.
clearPermissions()revokes the grantedgeolocationpermission but does not clear a geolocation coordinate override set withsetGeolocation(). UsesetGeolocation(null)for location cleanup, or rename this to “Clear all permissions” if permission revoked state is the intended example.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/playwright-cli/references/running-code.md around lines 39 - 42, Update the geolocation cleanup example to call setGeolocation(null) so it clears the coordinate override created by setGeolocation(); keep clearPermissions() only if the example is renamed to describe clearing permissions rather than location state..claude/skills/quality-vector-tests/references/example-lean.md-96-101 (1)
96-101: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the stale connector denominator.
This explanation says the denominator is 25 connectors, but the grounded count and canonical target are 26. Change
25to26so the rationale does not contradict the example it explains.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/quality-vector-tests/references/example-lean.md around lines 96 - 101, Update the connector coverage explanation in “What the format did” to use the canonical denominator of 26 connectors instead of 25, keeping the surrounding rationale unchanged..claude/skills/quality-vector-tests/references/vector-mapping.md-10-11 (1)
10-11: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the contradictory pagination guidance.
The table classifies pagination integrity as Reliability, but the common-miss text says pagination gets filed under Efficiency. Change that sentence to say it is often mistakenly filed under Efficiency, matching
.claude/skills/quality-vector-tests/SKILL.md.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/quality-vector-tests/references/vector-mapping.md around lines 10 - 11, Update the common-miss guidance in the vector-mapping table to say pagination is often mistakenly filed under Efficiency, while preserving its classification under Reliability and the existing explanation that no duplicates or omissions are correctness claims..claude/skills/quality-vector-tests/SKILL.md-40-41 (1)
40-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the 3,000-record pagination arithmetic.
A fixture containing exactly 3,000 records at page size 500 has 6 pages. Change the examples to 6 pages, or change the fixture size so that 7 pages is mathematically accurate.
.claude/skills/quality-vector-tests/SKILL.md#L40-L41: correct the illustrative pagination row..claude/skills/quality-vector-tests/SKILL.md#L231-L232: correct the “3,000-record fixture” table entry..claude/skills/quality-vector-tests/references/example-lean.md#L67-L68: correct the worked example.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/quality-vector-tests/SKILL.md around lines 40 - 41, Correct the pagination arithmetic for the exactly 3,000-record, page-size-500 example by changing the page count from 7 to 6 at .claude/skills/quality-vector-tests/SKILL.md lines 40-41, .claude/skills/quality-vector-tests/SKILL.md lines 231-232, and .claude/skills/quality-vector-tests/references/example-lean.md lines 67-68; keep the zero-duplicates, zero-omissions, and exact-total assertions unchanged..claude/skills/file-bug-insight/SKILL.md-165-170 (1)
165-170: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language to the fenced error block.
markdownlintreports MD040 for the fence at Line 167. Usetextorconsoleon the opening fence so the documentation passes the configured lint rule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/file-bug-insight/SKILL.md around lines 165 - 170, Add a language identifier such as text or console to the opening fenced block containing the error output in the file-bug-insight documentation, leaving the error contents unchanged.Source: Linters/SAST tools
.claude/skills/file-bug-insight/SKILL.md-223-230 (1)
223-230: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winVerify rendered Markdown, not only
body.
gh issue view --json ... bodyreturns raw issue Markdown, so this step cannot confirm the public issue renders correctly. Inspect the issue in the browser or fetch the rendered body via the GitHub REST HTML media type.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/file-bug-insight/SKILL.md around lines 223 - 230, Update the “Verify what landed” instructions to validate rendered Markdown rather than relying only on the raw body returned by gh issue view. In addition to checking the existing type, labels, and content requirements, instruct the agent to inspect the issue in a browser or fetch its rendered body using GitHub’s REST HTML media type before reporting the URL and summary..claude/skills/playwright-cli/references/request-mocking.md-30-35 (1)
30-35: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSpecify the fenced-block language.
Use
textfor the URL-pattern example so MD040 passes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/playwright-cli/references/request-mocking.md around lines 30 - 35, Specify the fenced code block language for the URL-pattern examples by adding the text language tag to the existing fence around the patterns. Do not alter the patterns or surrounding documentation.Source: Linters/SAST tools
.claude/skills/playwright-cli/references/request-mocking.md-44-53 (1)
44-53: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAwait or return every
route.fulfill()call.These handlers discard the fulfillment promise, so fulfillment failures can become unhandled. Use
return route.fulfill(...)in the conditional branches andawait route.fulfill(...)in the delayed handler.Also applies to: 81-85
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/playwright-cli/references/request-mocking.md around lines 44 - 53, Update the request route handlers in the examples around the login mock and the delayed handler so every route.fulfill call is awaited or returned. In the conditional branches within the login route, return each fulfillment promise; in the delayed handler, await the fulfillment call, preserving the existing response behavior..claude/skills/playwright-cli/references/video-recording.md-124-129 (1)
124-129: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the unsupported
styleSheetoption from the API table.
page.screencast.showChapter(title, options)supportsdescriptionandduration;styleSheetis not a supported option here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/playwright-cli/references/video-recording.md around lines 124 - 129, Remove the unsupported styleSheet option from the page.screencast.showChapter API table entry, leaving only the supported description and duration options while preserving the rest of the table..claude/skills/playwright-cli/references/video-recording.md-95-114 (1)
95-114: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle a nullable bounding box before constructing the overlay.
locator.boundingBox()can returnnullwhen the element is not visible, so the interpolated overlay HTML can throw while readingbounds.y. Add a visibility wait/null check before using the bounds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/playwright-cli/references/video-recording.md around lines 95 - 114, Handle the nullable result from boundingBox() before constructing the overlay in the Walk the dog example: wait for the locator to be visible, then check that bounds is non-null before reading its coordinates. Only call screencast.showOverlay when valid bounds are available.
🧹 Nitpick comments (1)
.claude/skills/playwright-cli/references/session-management.md (1)
31-49: 🩺 Stability & Availability | 🔵 TrivialWarn before using global cleanup commands.
close-alland especiallykill-allcan affect attached or user-owned browsers. Require checkingplaywright-cli listand closing only sessions owned by the workflow; reserve global cleanup for isolated environments. This matches the detach/ownership guidance in.claude/skills/drive-ui/SKILL.md:53-59.Also applies to: 206-217
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/playwright-cli/references/session-management.md around lines 31 - 49, Update the Browser Session Commands guidance for close-all and kill-all to require running playwright-cli list first, closing only workflow-owned sessions, and reserving global cleanup for isolated environments. Apply the same ownership warning to the additional cleanup section, while leaving targeted close commands unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/drive-ui/SKILL.md:
- Around line 44-59: Update the browser workflow documentation around the attach
and state-save examples to propagate the attached session name to every
subsequent Playwright command, including goto, console, requests, snapshot,
screenshot, state-save, and state-load. Use the explicit -s=chrome session form
consistently, or clearly document an intentional default-session behavior.
- Around line 55-61: Update the authentication state-saving instructions in the
drive UI skill to store the Playwright session file in a private path outside
$EVIDENCE, such as ~/.playwright-auth/stand-state.json, and use that same path
for later state-load operations. Note that the auth state should be ignored and
deleted when no longer needed.
In @.claude/skills/file-bug-insight/SKILL.md:
- Around line 43-51: Update the issue-search guidance in the “Search first”
section to run separate searches for open and closed issues for each vocabulary
variant, rather than relying on a mixed --state all search capped at 100
results. Preserve the existing duplicate-checking and cross-linking guidance,
while ensuring closed regression matches cannot be excluded by abundant open
results.
In @.claude/skills/playwright-cli/references/spec-driven-testing.md:
- Around line 164-165: Resolve the conflicting execution guidance around the
target-scenario workflow and its later parallelism statement: retain a
sequential, one-at-a-time policy because scenarios share state and file writes,
and remove or revise the claim that unique session names make parallel execution
safe. Ensure both referenced sections consistently require sequential execution
unless complete isolation is explicitly established.
- Around line 39-69: Add await page.pause() immediately after page.goto() in the
seed test and the reusable page fixture shown in the walkthroughs, including
equivalent ephemeral command examples. Keep the pause after navigation setup so
resume leaves the browser session open for subsequent snapshot and click
exploration.
In @.claude/skills/playwright-cli/references/storage-state.md:
- Around line 269-275: Update the Security Notes storage-state guidance to cover
every documented filename pattern, including auth.json, my-auth-state.json,
my-session.json, and storage-state-{timestamp}.json. Recommend storing state
outside the repository, and if local files are permitted, provide explicit
ignore rules that match all documented examples rather than only
*.auth-state.json.
- Around line 223-227: Update the page.evaluate example around
indexedDB.deleteDatabase to await a Promise that resolves on the request’s
onsuccess and rejects on onerror, ensuring run-code completes only after
database deletion finishes.
In @.claude/skills/playwright-cli/references/test-generation.md:
- Around line 12-18: Update the example workflow in the test-generation
reference to use the required seed test generation flow, including seed, debug,
and attach steps before interacting with the application. Apply the same change
to the related example at the referenced section, or clearly label both examples
as standalone sessions without test setup if they are not intended to use
fixtures, authentication, and custom setup.
In @.claude/skills/playwright-cli/references/tracing.md:
- Around line 20-50: Update the “Trace Output Files” section to document that
playwright-cli tracing-stop produces .playwright-cli/trace.zip rather than a
timestamped traces directory, include the npx playwright show-trace
.playwright-cli/trace.zip inspection command, and change any cleanup guidance to
target the actual trace.zip artifact.
In @.claude/skills/playwright-cli/SKILL.md:
- Around line 328-337: Update the local Playwright CLI guidance in the
installation and invocation instructions to use the agent-focused
`@playwright/cli`@latest package, installed with npm install -D and invoked as npx
playwright-cli. Keep the npx --no-install playwright-cli --version probe and
ensure the global fallback uses the same package and command path.
In @.claude/skills/quality-vector-tests/SKILL.md:
- Around line 173-185: Retain the governing one-metric/one-target rules in
.claude/skills/quality-vector-tests/SKILL.md:173-185. Update the worked examples
in .claude/skills/quality-vector-tests/references/example-lean.md:60-74,
example-migration.md:42-56, and example-port.md:40-49 so every outcome is a
separate check with an explicit denominator or zero-count numeric target; split
combined API, connector, metric, bad-request, UI-regression, and org-chart
outcomes, and replace the open-ended cross-namespace target with independently
measurable checks.
---
Minor comments:
In @.claude/skills/file-bug-insight/SKILL.md:
- Around line 165-170: Add a language identifier such as text or console to the
opening fenced block containing the error output in the file-bug-insight
documentation, leaving the error contents unchanged.
- Around line 223-230: Update the “Verify what landed” instructions to validate
rendered Markdown rather than relying only on the raw body returned by gh issue
view. In addition to checking the existing type, labels, and content
requirements, instruct the agent to inspect the issue in a browser or fetch its
rendered body using GitHub’s REST HTML media type before reporting the URL and
summary.
In @.claude/skills/playwright-cli/references/request-mocking.md:
- Around line 30-35: Specify the fenced code block language for the URL-pattern
examples by adding the text language tag to the existing fence around the
patterns. Do not alter the patterns or surrounding documentation.
- Around line 44-53: Update the request route handlers in the examples around
the login mock and the delayed handler so every route.fulfill call is awaited or
returned. In the conditional branches within the login route, return each
fulfillment promise; in the delayed handler, await the fulfillment call,
preserving the existing response behavior.
In @.claude/skills/playwright-cli/references/running-code.md:
- Around line 39-42: Update the geolocation cleanup example to call
setGeolocation(null) so it clears the coordinate override created by
setGeolocation(); keep clearPermissions() only if the example is renamed to
describe clearing permissions rather than location state.
In @.claude/skills/playwright-cli/references/video-recording.md:
- Around line 124-129: Remove the unsupported styleSheet option from the
page.screencast.showChapter API table entry, leaving only the supported
description and duration options while preserving the rest of the table.
- Around line 95-114: Handle the nullable result from boundingBox() before
constructing the overlay in the Walk the dog example: wait for the locator to be
visible, then check that bounds is non-null before reading its coordinates. Only
call screencast.showOverlay when valid bounds are available.
In @.claude/skills/quality-vector-tests/references/example-lean.md:
- Around line 96-101: Update the connector coverage explanation in “What the
format did” to use the canonical denominator of 26 connectors instead of 25,
keeping the surrounding rationale unchanged.
In @.claude/skills/quality-vector-tests/references/vector-mapping.md:
- Around line 10-11: Update the common-miss guidance in the vector-mapping table
to say pagination is often mistakenly filed under Efficiency, while preserving
its classification under Reliability and the existing explanation that no
duplicates or omissions are correctness claims.
In @.claude/skills/quality-vector-tests/SKILL.md:
- Around line 40-41: Correct the pagination arithmetic for the exactly
3,000-record, page-size-500 example by changing the page count from 7 to 6 at
.claude/skills/quality-vector-tests/SKILL.md lines 40-41,
.claude/skills/quality-vector-tests/SKILL.md lines 231-232, and
.claude/skills/quality-vector-tests/references/example-lean.md lines 67-68; keep
the zero-duplicates, zero-omissions, and exact-total assertions unchanged.
---
Nitpick comments:
In @.claude/skills/playwright-cli/references/session-management.md:
- Around line 31-49: Update the Browser Session Commands guidance for close-all
and kill-all to require running playwright-cli list first, closing only
workflow-owned sessions, and reserving global cleanup for isolated environments.
Apply the same ownership warning to the additional cleanup section, while
leaving targeted close commands unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 39bcfb26-4adf-4aac-af3e-acdf26b27114
📥 Commits
Reviewing files that changed from the base of the PR and between 7129aff and 051751c808956d25421d43ba09b4c540a8ee837d.
📒 Files selected for processing (18)
.claude/skills/drive-ui/SKILL.md.claude/skills/file-bug-insight/SKILL.md.claude/skills/playwright-cli/SKILL.md.claude/skills/playwright-cli/references/element-attributes.md.claude/skills/playwright-cli/references/playwright-tests.md.claude/skills/playwright-cli/references/request-mocking.md.claude/skills/playwright-cli/references/running-code.md.claude/skills/playwright-cli/references/session-management.md.claude/skills/playwright-cli/references/spec-driven-testing.md.claude/skills/playwright-cli/references/storage-state.md.claude/skills/playwright-cli/references/test-generation.md.claude/skills/playwright-cli/references/tracing.md.claude/skills/playwright-cli/references/video-recording.md.claude/skills/quality-vector-tests/SKILL.md.claude/skills/quality-vector-tests/references/example-lean.md.claude/skills/quality-vector-tests/references/example-migration.md.claude/skills/quality-vector-tests/references/example-port.md.claude/skills/quality-vector-tests/references/vector-mapping.md
| ```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. | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'spec-driven-testing.md' . || true
echo "== git status/stat =="
git status --short || true
git diff --stat || true
echo "== file excerpt =="
if [ -f ".claude/skills/playwright-cli/references/spec-driven-testing.md" ]; then
nl -ba .claude/skills/playwright-cli/references/spec-driven-testing.md | sed -n '1,120p'
fi
echo "== search related commands/docs =="
rg -n "resume|snapshot|page\.pause|playwright-cli|seed\(spec|fixtures" .claude playwright 2>/dev/null || trueRepository: constructorfabric/insight
Length of output: 263
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
find . -path '*/.claude/skills/playwright-cli/references/spec-driven-testing.md' -print 2>/dev/null || true
FILE=$(find . -path '*/.claude/skills/playwright-cli/references/spec-driven-testing.md' -print 2>/dev/null | head -n 1 || true)
echo "FILE=$FILE"
if [ -n "$FILE" ] && [ -f "$FILE" ]; then
echo "== file excerpt =="
awk '{printf "%6d：%s\n", NR, $0}' "$FILE" | sed -n '1,130p'
echo "== git info for file =="
git diff -- "$FILE" | sed -n '1,220p' || true
fi
echo "== search related text in repo =="
find . -maxdepth 6 -type f \( -name '*.md' -o -name '*.ts' -o -name '*.js' \) -print 2>/dev/null | head -n 200 | xargs -r grep -E "resume|snapshot|page\.pause|playwright-cli|seed\(spec|fixtures" 2>/dev/null || trueRepository: constructorfabric/insight
Length of output: 50383
🌐 Web query:
Playwright debug cli pause after test completes page.pause interactive inspection
💡 Result:
To pause your Playwright test for interactive inspection after it completes or at a specific point, you can use the following methods. Note that there is currently no native CLI flag (e.g., --leave-browser-open) to automatically pause after a test finishes [1][2]. 1. Using page.pause The most direct way to pause execution is to call await page.pause in your test code [3][4]. This stops execution and opens the Playwright Inspector, where you can inspect selectors, perform manual actions, and resume the test [3][4][5]. This requires the test to be run in headed mode (e.g., using --headed) [3][6]. 2. Using UI Mode For a better debugging experience, run your tests using Playwright's UI Mode [7]. npx playwright test --ui UI Mode provides a time-travel experience, allowing you to hover over each test action to see exactly what happened in the DOM, network requests, and console logs at that specific moment [7][8]. You can also pop out DOM snapshots for deeper inspection in browser DevTools [7][8]. 3. Workarounds to Keep Browser Open If you need to keep the browser open after a test completes, you can use one of these approaches: - Environment Variable: Use a conditional pause in your test code that is triggered by an environment variable [1][2]. if (process.env.KEEP_OPEN) { await page.pause; } Run it with: KEEP_OPEN=1 npx playwright test --headed - Test Fixture: You can create a custom fixture that automatically triggers a pause if an environment variable is set, preventing the need to modify every test file [1][2]. 4. Recommended Tooling For the best debugging experience, Playwright's official VS Code extension is highly recommended [5][9]. It allows you to set breakpoints directly in your IDE, see error messages, live-debug with "Show Browsers," and analyze test execution step-by-step using the built-in Trace Viewer [9].
Citations:
- 1: [Feature]: Add CLI option to leave headed browser open after running test microsoft/playwright#31329
- 2: Add CLI support to keep the browser opened microsoft/playwright#20130
- 3: https://playwright.dev/docs/api/class-page
- 4: https://playwright.dev/docs/debug
- 5: https://playwright.dev/docs/next/debug
- 6: https://playwright.dev/python/docs/api/class-page
- 7: https://playwright.dev/docs/test-ui-mode
- 8: https://playwright.dev/docs/next/test-ui-mode
- 9: https://playwright.dev/docs/getting-started-vscode
🌐 Web query:
site:playwright.dev/docs test debugging playwright-cli resume
💡 Result:
In Playwright, "resuming" test execution during debugging is primarily handled through the Playwright Inspector or the programmatic Debugger API. When debugging tests using the Playwright Inspector (triggered via the --debug CLI flag), you control execution using the toolbar at the top of the Inspector window [1][2]. The "Resume" (or "Play") button allows you to continue test execution until the next breakpoint or the end of the test [1][2]. Adding an await page.pause statement in your test code creates a manual breakpoint; clicking "Resume" in the Inspector while paused at this point will run the test and pause again only at the next page.pause call [1][3]. For more granular control, Playwright provides a Debugger API available on the browser context (browserContext.debugger) [4]: - debugger.resume: Resumes script execution. This throws an error if the debugger is not currently paused [4]. - debugger.runTo: Resumes execution and pauses again when an action originates from a specified source location [4]. While the --debug CLI flag provides a standard interactive workflow, you can also use UI Mode (npx playwright test --ui) for a time-travel debugging experience, which allows you to inspect test states and actions without needing to manually "resume" in the traditional sense, as it provides a visual timeline of the test execution [2][5].
Citations:
- 1: https://playwright.dev/docs/debug
- 2: https://playwright.dev/docs/running-tests
- 3: https://playwright.dev/docs/next/debug
- 4: https://playwright.dev/docs/api/class-debugger
- 5: https://playwright.dev/docs/test-ui-mode
🌐 Web query:
Playwright Playwright CLI resume detach browser context closing when test finishes
💡 Result:
Playwright does not have a native CLI flag to "detach" or keep a browser context open after a standard test run finishes. By design, Playwright closes the browser context and the browser itself upon the completion of a test to ensure environment isolation and resource cleanup [1][2][3]. However, you can achieve this behavior using several established workarounds depending on your goal: 1. Using page.pause The most direct way to keep the browser open is to call page.pause at the end of your test [4][5]. This pauses test execution, opens the Playwright Inspector, and keeps the browser window active, allowing you to interact with the page manually [5]. 2. Using Environment Variables and Fixtures You can create a custom fixture or add a conditional check to your test code that triggers a pause only when a specific environment variable is set [6][4]. This keeps your production test runs clean while allowing local debugging: // Example fixture approach export const test = base.extend({ context: async ({ context }, use) => { await use(context); if (process.env.KEEP_OPEN) { await expect( => { expect(context.pages.every((page) => page.isClosed)).toBe(true); }).toPass({ intervals: [2_000] }); } }, }); Run this with: KEEP_OPEN=1 npx playwright test --headed [6]. 3. Playwright Inspector (--debug) Running your tests with the --debug flag (or setting PWDEBUG=1) automatically runs in headed mode, limits workers to 1, and opens the Playwright Inspector [7][5]. While it still closes the browser after the test completes, it provides the built-in control interface for stepping through actions [5]. 4. Distinction for Playwright Agent CLI If you are using the separate Playwright Agent CLI (which manages persistent browser sessions), the behavior is different from the standard test runner. In that context, sessions can be kept alive explicitly using --persistent and managed via commands like open, close, and kill-all [8][9]. These sessions are intended for long-lived, interactive, or multi-step manual workflows rather than standard unit/integration testing [8][10].
Citations:
- 1: https://playwright.dev/docs/browser-contexts
- 2: https://playwright.dev/docs/api/class-browsercontext
- 3: https://dev.to/johnnyv5g/playwright-browsercontext-what-it-is-why-it-matters-and-how-to-configure-it-3gi8
- 4: [Feature]: Add CLI option to leave headed browser open after running test microsoft/playwright#31329
- 5: https://playwright.dev/docs/running-tests
- 6: Add CLI support to keep the browser opened microsoft/playwright#20130
- 7: https://playwright.dev/docs/test-cli
- 8: https://playwright.dev/agent-cli/sessions
- 9: https://github.com/microsoft/playwright-cli/blob/main/skills/playwright-cli/references/session-management.md
- 10: https://qaskills.sh/blog/playwright-cli-debug-tests-traces-agents-guide-2026
Keep the seed session paused after setup.
resume lets the seed run to completion, so the documented snapshot/click interaction commands can run after the Playwright test has already closed the browser. Add await page.pause() after page.goto() (and apply the same pause in seeded fixture walkthroughs, for example in an ephemeral command) so the session stays open for exploration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/playwright-cli/references/spec-driven-testing.md around lines
39 - 69, Add await page.pause() immediately after page.goto() in the seed test
and the reusable page fixture shown in the walkthroughs, including equivalent
ephemeral command examples. Keep the pause after navigation setup so resume
leaves the browser session open for subsequent snapshot and click exploration.
| For each target scenario, in sequence (never in parallel — scenarios share the seed session): | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Resolve the generation parallelism contradiction.
The workflow first says scenarios must never run in parallel because they share a seed session, then says parallel execution is safe because session names are unique. Choose one policy; the surrounding stateful, one-at-a-time workflow should remain sequential unless all shared state and file writes are explicitly isolated.
Also applies to: 221-223
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/playwright-cli/references/spec-driven-testing.md around lines
164 - 165, Resolve the conflicting execution guidance around the target-scenario
workflow and its later parallelism statement: retain a sequential, one-at-a-time
policy because scenarios share state and file writes, and remove or revise the
claim that unique session names make parallel execution safe. Ensure both
referenced sections consistently require sequential execution unless complete
isolation is explicitly established.
| ## 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 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Cover every documented storage-state filename.
The suggested *.auth-state.json ignore rule does not cover the examples auth.json, my-auth-state.json, my-session.json, or the auto-generated storage-state-{timestamp}.json. Recommend storing state outside the repository and, if local files are allowed, ignore all documented patterns explicitly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/playwright-cli/references/storage-state.md around lines 269 -
275, Update the Security Notes storage-state guidance to cover every documented
filename pattern, including auth.json, my-auth-state.json, my-session.json, and
storage-state-{timestamp}.json. Recommend storing state outside the repository,
and if local files are permitted, provide explicit ignore rules that match all
documented examples rather than only *.auth-state.json.
| ## 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 | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
echo "Repo files matching tracing.md:"
git ls-files | rg '(^|/)tracing\.md$|traces/.*|playwright|claude' | head -200 || true
echo
echo "Target file excerpt:"
if [ -f .claude/skills/playwright-cli/references/tracing.md ]; then
nl -ba .claude/skills/playwright-cli/references/tracing.md | sed -n '1,170p'
else
echo "target missing"
fi
echo
echo "Search trace-related text:"
rg -n "trace|traces|show-trace|artifact|resources" .claude/skills/playwright-cli/references/tracing.md .claude 2>/dev/null || trueRepository: constructorfabric/insight
Length of output: 13125
🏁 Script executed:
#!/bin/bash
set -e
echo "Tracing file:"
cat .claude/skills/playwright-cli/references/tracing.md
echo
echo "Playwright CLI skill outline/search:"
rg -n "trace|traces|show-trace|record|start|stop|cleanup|find" .claude/skills/playwright-cli -S || trueRepository: constructorfabric/insight
Length of output: 14225
🌐 Web query:
Playwright CLI tracing start-stop show-trace trace.zip artifacts
💡 Result:
In Playwright, tracing is used to capture browser operations, network activity, screenshots, and snapshots for debugging [1][2]. The process differs depending on whether you are using the Playwright Test runner or the low-level Playwright CLI/API [3][4]. ### 1. Recording Traces There are two primary ways to record traces: * Via Playwright Test: You can enable tracing automatically when running tests by setting the --trace flag [3][5]. * Example: npx playwright test --trace on [3][6] * Other options include on-first-retry, on-all-retries, and retain-on-failure [3][5]. * Via Playwright CLI / API (Manual/Agent): For custom browser sessions or automated workflows, you can explicitly start and stop tracing using the tracing command set [7][1]. * playwright-cli tracing-start: Begins recording [7][1]. * (Perform your browser actions) * playwright-cli tracing-stop: Ends the recording and saves the artifact to a file (default: trace.zip) [7][1]. When using the programmatic API (e.g., in a Node.js script), use context.tracing.start() and context.tracing.stop({ path: 'trace.zip' }) [2][6]. ### 2. Viewing Traces Once you have a trace.zip file, you can analyze it in two ways [3][8]: * GUI (Interactive): Use the show-trace command to launch the visual Trace Viewer application [3][8]. * Command: npx playwright show-trace path/to/trace.zip [3][6][8] * Alternatively, you can drag and drop your trace.zip directly into the web-based viewer at trace.playwright.dev [3][8][9]. * CLI (Non-Interactive/Scriptable): Introduced in later versions (e.g., 1.60+), the npx playwright trace command allows you to inspect trace contents programmatically, which is useful for CI pipelines or AI agents [8]. * List actions: npx playwright trace --list trace.zip [8] * Filter for errors: npx playwright trace --list --errors-only trace.zip [8] * Export network data: npx playwright trace --network --json trace.zip > network.json [8] ### Summary Table | Goal | Command / Method | |:--- |:--- | | Record Test Trace | npx playwright test --trace on [3][6] | | Record Manual Flow | playwright-cli tracing-start... playwright-cli tracing-stop [7][1] | | Visual Debugging | npx playwright show-trace <file.zip> [3][8] | | Automated/CLI Analysis | npx playwright trace <file.zip> [options] [8] | Traces saved to a trace.zip file capture the state of the browser, network requests, and console logs, serving as the primary artifact for post-mortem analysis [1][2][4].
Citations:
- 1: https://github.com/microsoft/playwright-cli/blob/main/skills/playwright-cli/references/tracing.md
- 2: https://playwright.dev/docs/api/class-tracing
- 3: https://playwright.dev/docs/trace-viewer
- 4: https://qaskills.sh/blog/playwright-cli-debug-tests-traces-agents-guide-2026
- 5: https://playwright.dev/docs/test-cli
- 6: https://github.com/microsoft/playwright/blob/main/docs/src/trace-viewer.md
- 7: https://playwright.dev/agent-cli/commands/tracing
- 8: https://qaskills.sh/blog/playwright-trace-cli-npx-guide
- 9: https://playwright.dev/dotnet/docs/next/trace-viewer
Document the actual trace zip artifact path.
playwright-cli tracing-stop saves the trace as .playwright-cli/trace.zip, not a traces/trace-{timestamp}.* directory tree. Also show how to inspect it (npx playwright show-trace .playwright-cli/trace.zip) and update the cleanup path to the actual artifact.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/playwright-cli/references/tracing.md around lines 20 - 50,
Update the “Trace Output Files” section to document that playwright-cli
tracing-stop produces .playwright-cli/trace.zip rather than a timestamped traces
directory, include the npx playwright show-trace .playwright-cli/trace.zip
inspection command, and change any cleanup guidance to target the actual
trace.zip artifact.
| 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 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate file =="
fd -a 'SKILL\.md$' . | sed 's#^\./##' | rg 'playwright|SKILL.md' | head -50
echo "== target lines =="
nl -ba .claude/skills/playwright-cli/SKILL.md | sed -n '300,350p'
echo "== nearby playwright-cli commands in target file =="
rg -n "playwright-cli|npx playwright|--no-install|install -g|playwright cli" .claude/skills/playwright-cli/SKILL.md
echo "== package manifests mentioning playwright dependencies/packges =="
rg -n '"@?playwright/?cli"?|\"playwright-cli"|npx playwright-cli|npx playwright cli|npx --no-install' -S . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' | head -200Repository: constructorfabric/insight
Length of output: 184
🌐 Web query:
Microsoft Playwright CLI package playwright-cli npx playwright cli installation documentation
💡 Result:
There are two distinct types of CLI tools associated with Playwright, which are often confused: the standard Playwright Test CLI and the specialized Playwright CLI designed for coding agents [1][2][3]. Standard Playwright Test CLI This is the command-line interface used for running tests, generating code, and debugging in standard Playwright projects [3][4]. It is bundled with the standard playwright package and typically invoked via npx [5]. Usage: npx playwright test [options] npx playwright codegen [options] [url] npx playwright install [options] Playwright CLI for Coding Agents (@playwright/cli) This is a separate, dedicated tool designed specifically for use by coding agents (such as Claude Code or GitHub Copilot) to perform browser automation tasks [1][6][2]. The old package name playwright-cli is deprecated; you should use @playwright/cli [7][8]. Installation: For global access: npm install -g @playwright/cli@latest For local project use: npm install -D @playwright/cli@latest Usage: If installed globally, run it directly: playwright-cli --help If installed locally, use npx: npx playwright-cli --help Important Note on "npx playwright cli" Some older documentation or workflows might reference npx playwright cli. Current documentation for the agent-focused tool directs users to run playwright-cli directly if installed globally, or npx playwright-cli if installed locally [1][6][7]. Always check the official documentation at playwright.dev for the most current commands [1][3].
Citations:
- 1: https://playwright.dev/agent-cli/installation
- 2: https://www.npmjs.com/package/@playwright/cli
- 3: https://playwright.dev/docs/test-cli
- 4: https://playwright.dev/docs/next/running-tests
- 5: https://playwright.dev/docs/next/intro
- 6: https://playwright.dev/docs/next/getting-started-cli
- 7: https://registry.npmjs.org/%40playwright%2Fcli
- 8: https://www.npmjs.com/package/playwright-cli
🌐 Web query:
npm package @playwright/cli npx playwright-cli
💡 Result:
The @playwright/cli npm package is a dedicated command-line interface designed specifically for coding agents (such as Claude Code or GitHub Copilot) to interact with Playwright in a token-efficient manner [1][2]. It allows these agents to perform browser automation tasks—like navigating pages, clicking elements, taking screenshots, and typing text—using concise commands that avoid the overhead of large tool schemas [2][3]. To use the package locally, you can install it as a dependency in your project [2]: npm install -D @playwright/cli@latest Once installed, you can execute commands using npx: npx playwright-cli --help While some documentation mentions the usage of npx playwright-cli [2][4], other sections suggest npx playwright cli [1][5] depending on the specific installation and local setup. The official recommendation for agents is often to install it globally (npm install -g @playwright/cli@latest) to ensure the playwright-cli command is available directly [2][6]. The tool also supports installing "skills," which are specific capabilities for agents, via the command: playwright-cli install --skills [2][3] This package is separate from the standard Playwright library used for test automation and is optimized for LLM-based agentic workflows [2][6].
Citations:
- 1: https://www.npmjs.com/package/@playwright/cli
- 2: https://playwright.dev/docs/getting-started-cli
- 3: https://playwright.dev/docs/next/getting-started-cli
- 4: https://github.com/microsoft/playwright/blob/4d289016/docs/src/getting-started-cli.md
- 5: https://github.com/microsoft/playwright-cli
- 6: https://github.com/microsoft/playwright-cli/
🌐 Web query:
playwright.dev agent-cli installation npx playwright cli npx playwright-cli
💡 Result:
It is important to distinguish between the two different command-line tools provided by Playwright, as they serve different purposes and use different packages [1][2][3]. 1. Playwright CLI for Coding Agents (@playwright/cli) This is a specialized CLI designed for use with coding agents (like Claude Code or GitHub Copilot) to perform browser automation [1][4][3]. It is token-efficient and supports installing "skills" that allow agents to interact with web pages effectively [1][3]. Installation and Usage: - Global Installation: You can install it globally via npm: npm install -g @playwright/cli@latest [1][5][6] - Running it: Once installed, use the command: playwright-cli [command] [1][5][6] - Local Execution: If you prefer not to install it globally, you can execute it via npx: npx playwright-cli [command] [1][6] 2. Standard Playwright CLI (playwright) This is the standard command-line interface included with the main playwright package, used for running tests, generating code (codegen), and debugging [2][7]. Usage: - Execution: Always run this via npx to ensure you are using the version pinned to your project's dependencies: npx playwright [command] [2][7] Examples include: npx playwright test npx playwright codegen npx playwright install Summary Table Package: @playwright/cli | Purpose: Browser automation for AI coding agents | Command: playwright-cli [1][3] Package: playwright | Purpose: Test execution, debugging, and generation | Command: npx playwright [2][7] Do not confuse these two; if you are trying to run standard test suites, use npx playwright. If you are configuring a coding agent to control a browser, use the @playwright/cli tool [2][3].
Citations:
- 1: https://playwright.dev/agent-cli/installation
- 2: https://playwright.dev/docs/test-cli
- 3: https://playwright.dev/agent-cli/introduction
- 4: https://github.com/microsoft/playwright-cli
- 5: https://www.npmjs.com/package/@playwright/cli
- 6: https://registry.npmjs.org/%40playwright%2Fcli
- 7: https://playwright.dev/docs/running-tests
🌐 Web query:
"playwright-cli" "npx --no-install playwright"
💡 Result:
The command npx --no-install playwright runs the Playwright command-line interface (CLI) using a locally installed version of Playwright within your project, explicitly instructing npx not to download or install the package if it is not found [1][2][3]. Breakdown of the command components: 1. npx: A tool bundled with Node.js that executes Node.js binaries [1][4]. It typically searches for a package in the local node_modules directory first and, by default, will attempt to download and install the package from the registry if it is not found. 2. --no-install: This flag forces npx to only use a version of the package that is already installed locally in your project [1][2][3]. If the package is not found locally, the command will fail rather than attempting to download it. 3. playwright: The specific command being executed. This refers to the Playwright CLI tool, which is used for tasks such as running tests, installing browsers, and debugging [4][5]. Usage and Context: - Local vs. Global: This command is primarily used to ensure you are executing the version of Playwright pinned to your project's dependencies, preventing accidental execution of a different global version or unnecessary network downloads [1][3]. - Troubleshooting: If you receive an error when running npx --no-install playwright, it indicates that the Playwright package is not installed in your local node_modules folder [1][2]. In such cases, you should first install the necessary dependencies, typically by running npm install [6][5]. - Playwright-cli: Note that there is a distinction between the standard Playwright testing CLI (usually accessed via npx playwright) and the newer, specialized playwright-cli designed for coding agents [7][4][8]. Ensure you are using the correct package name for your specific use case [1][3].
Citations:
- 1: https://github.com/microsoft/playwright-cli/
- 2: https://www.npmjs.com/package/@playwright/cli
- 3: https://registry.npmjs.org/%40playwright%2Fcli
- 4: https://playwright.dev/docs/test-cli
- 5: https://playwright.dev/docs/browsers
- 6: https://playwright.dev/docs/intro
- 7: https://playwright.dev/agent-cli/installation
- 8: https://playwright.dev/docs/getting-started-cli
🌐 Web query:
"playwright.dev/agent-cli/installation" "npx playwright-cli"
💡 Result:
The Playwright CLI is a command-line interface designed to enable coding agents, such as Claude Code and GitHub Copilot, to perform browser automation [1][2]. ### Installation Methods You can install the Playwright CLI either globally or locally [3][4]. - Global installation (recommended for most users): bash npm install -g `@playwright/cli`@latest After installation, you can verify it by running playwright-cli --help [3][4]. - Local installation (using npx): If you prefer not to install the package globally, you can use it as a local dependency or invoke it directly via npx [3][4]: bash npm install -D `@playwright/cli`@latest npx playwright-cli --help ### Setting Up Skills To provide coding agents with richer context about available commands and workflows, you should install "skills" locally after installing the CLI [3][5]: bash playwright-cli install --skills ### Prerequisites - Node.js 20 or newer [3][4] - A coding agent (e.g., Claude Code, GitHub Copilot) [3][4] For further details and advanced usage, you can refer to the official Playwright CLI documentation [2].
Citations:
- 1: https://playwright.dev/agent-cli/introduction
- 2: https://github.com/microsoft/playwright/
- 3: https://playwright.dev/agent-cli/installation
- 4: https://playwright.dev/docs/getting-started-cli
- 5: https://playwright.dev/agent-cli/skills
🌐 Web query:
"playwright.dev/getting-started-cli" "npx playwright cli"
💡 Result:
The npx playwright command serves as the primary interface for interacting with the standard Playwright testing library, while playwright-cli refers to a distinct tool designed specifically for browser automation in agentic workflows [1][2][3]. Standard Playwright CLI The command npx playwright [command] is used to execute standard testing and utility tasks provided by the Playwright library installed in your project [1]. Common commands include: - npx playwright test: Runs your test suite [1]. - npx playwright codegen: Generates tests by recording your browser interactions [1]. - npx playwright show-report: Opens the HTML report for your tests [1]. - npx playwright install: Installs necessary browser binaries [1]. - npx playwright --help: Lists all available commands for the installed version [1]. Agent-Focused Playwright CLI playwright-cli (often installed via @playwright/cli) is a separate, specialized tool built for coding agents that require token-efficient, concise browser control [2][3]. It allows agents to perform actions such as: - playwright-cli open [url]: Opens a browser page for automation [2][3]. - playwright-cli click [ref]: Performs clicks based on element references [2][3]. - playwright-cli screenshot: Captures the page state [2][3]. - playwright-cli install --skills: Installs specialized agent skills [2][3]. Distinction and Usage While both use "Playwright" in their names, they serve different purposes. The standard npx playwright command is integrated into your project's development workflow for testing [1], whereas playwright-cli is typically installed globally or locally as a dedicated utility for browser automation agents [2][4][3]. If you encounter a situation where you need to use a local version of the agent-focused CLI but the global command is unavailable, documentation suggests using npx playwright cli or npx --no-install playwright to invoke it if available within your environment [4][5]. Always check npx playwright --help or playwright-cli --help to see the specific capabilities available in your current installation [1][2].
Citations:
- 1: https://playwright.dev/docs/test-cli
- 2: https://playwright.dev/docs/getting-started-cli
- 3: https://playwright.dev/agent-cli/introduction
- 4: https://github.com/microsoft/playwright-cli
- 5: https://www.npmjs.com/package/@playwright/cli
Align the local CLI instructions with the upstream installation path.
The agent-focused Playwright CLI is installed locally via npm install -D @playwright/cli@latest and invoked via npx playwright-cli, not npx playwright cli; use npx --no-install playwright-cli --version for the local probe unless this file intentionally targets the standard Playwright CLI instead.
🧰 Tools
🪛 SkillSpector (2.4.4)
[warning] 328: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 334: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/playwright-cli/SKILL.md around lines 328 - 337, Update the
local Playwright CLI guidance in the installation and invocation instructions to
use the agent-focused `@playwright/cli`@latest package, installed with npm install
-D and invoked as npx playwright-cli. Keep the npx --no-install playwright-cli
--version probe and ensure the global fallback uses the same package and command
path.
Source: Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.claude/skills/file-bug-insight/SKILL.md (1)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine one owner for wrong-number evidence collection.
Both skills currently document the same console/API capture workflow while assigning ownership differently.
.claude/skills/file-bug-insight/SKILL.md#L39-L39: make this the sole owner, or change it to consume thedrive-uiworkflow..claude/skills/drive-ui/SKILL.md#L92-L99: remove the duplicate instructions or make this the canonical evidence-collection section.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/file-bug-insight/SKILL.md at line 39, Make .claude/skills/file-bug-insight/SKILL.md the sole owner of the wrong-number console/API evidence workflow, retaining the `playwright-cli console`, `requests`, and `request <n>` capture guidance. Update .claude/skills/drive-ui/SKILL.md lines 92-99 to remove the duplicate workflow or explicitly defer to file-bug-insight, so ownership is defined in only one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/file-bug-insight/SKILL.md:
- Line 13: Update the writing rules in SKILL.md to remove any instruction that
asks agents to explain why an issue happens, including the “State what happens,
then why” guidance. Replace it with a requirement to describe the observable
consequence or expected behavior, while preserving the existing “Report what you
saw, not why it happens” rule.
- Line 66: Require redaction of every artifact before attaching evidence to a
public issue: update the evidence guidance in
.claude/skills/file-bug-insight/SKILL.md at lines 66-66 to cover raw API
responses, logs, screenshots, snapshots, and query results, and update
.claude/skills/drive-ui/SKILL.md at lines 92-99 to explicitly prohibit attaching
raw headers, cookies, tokens, or user/tenant data.
---
Nitpick comments:
In @.claude/skills/file-bug-insight/SKILL.md:
- Line 39: Make .claude/skills/file-bug-insight/SKILL.md the sole owner of the
wrong-number console/API evidence workflow, retaining the `playwright-cli
console`, `requests`, and `request <n>` capture guidance. Update
.claude/skills/drive-ui/SKILL.md lines 92-99 to remove the duplicate workflow or
explicitly defer to file-bug-insight, so ownership is defined in only one place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0dbf2f73-85d7-4479-be76-930633cba309
📥 Commits
Reviewing files that changed from the base of the PR and between 051751c808956d25421d43ba09b4c540a8ee837d and c619daa54a000bd2468fdf5e6e20c1a3b3d33b8f.
📒 Files selected for processing (5)
.claude/skills/drive-ui/SKILL.md.claude/skills/file-bug-insight/SKILL.md.claude/skills/playwright-cli/SKILL.md.claude/skills/playwright-cli/references/spec-driven-testing.md.claude/skills/quality-vector-tests/SKILL.md
💤 Files with no reviewable changes (1)
- .claude/skills/playwright-cli/SKILL.md
|
|
||
| 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. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not reintroduce causal explanations into the writing rules.
“Report what you saw, not why it happens” conflicts with “State what happens, then why.” Replace the latter with observable consequence or expected behavior so agents are not instructed to add diagnosis.
Also applies to: 124-124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/file-bug-insight/SKILL.md at line 13, Update the writing
rules in SKILL.md to remove any instruction that asks agents to explain why an
issue happens, including the “State what happens, then why” guidance. Replace it
with a requirement to describe the observable consequence or expected behavior,
while preserving the existing “Report what you saw, not why it happens” rule.
|
|
||
| **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** — 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. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Require redaction of every artifact attached to a public issue.
The workflow captures raw API responses, logs, screenshots, snapshots, and query results, but the public-repo guidance only clearly scrubs issue-body text.
.claude/skills/file-bug-insight/SKILL.md#L66-L66: add artifact redaction before evidence is attached..claude/skills/drive-ui/SKILL.md#L92-L99: explicitly prohibit attaching raw headers, cookies, tokens, or user/tenant data.
📍 Affects 2 files
.claude/skills/file-bug-insight/SKILL.md#L66-L66(this comment).claude/skills/drive-ui/SKILL.md#L92-L99
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/file-bug-insight/SKILL.md at line 66, Require redaction of
every artifact before attaching evidence to a public issue: update the evidence
guidance in .claude/skills/file-bug-insight/SKILL.md at lines 66-66 to cover raw
API responses, logs, screenshots, snapshots, and query results, and update
.claude/skills/drive-ui/SKILL.md at lines 92-99 to explicitly prohibit attaching
raw headers, cookies, tokens, or user/tenant data.
…cli, quality-vector-tests 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 <Konstantin.Tursunov@constructor.tech>
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>
…ity-vector-tests 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>
…-issue search 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>
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>
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
.claude/skills/drive-ui/SKILL.md (1)
60-65: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winShow the corresponding auth-state restore command.
This documents
state-savebut only mentionsstate-loadin a comment, leaving the later-run workflow incomplete. Add the exact restore command and verify its session/path syntax.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/drive-ui/SKILL.md around lines 60 - 65, Update the authentication workflow documentation around the state-save example to include the exact corresponding playwright-cli state-load command using the same Chrome session and ~/.playwright-auth/stand-state.json path. Keep the stale-state guidance and cleanup instruction unchanged, and ensure the restore syntax matches the documented save syntax.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/file-bug-insight/SKILL.md:
- Around line 164-168: Update the fenced server-log block in the relevant
documentation section to include an appropriate language identifier, such as
text or console, while preserving the log content unchanged.
In @.claude/skills/playwright-cli/references/request-mocking.md:
- Around line 30-35: Label the URL-pattern fenced code block in the
request-mocking documentation as text by adding the text language identifier to
its opening fence; leave the pattern examples unchanged.
In @.claude/skills/quality-vector-tests/references/example-lean.md:
- Line 9: Update the opening fenced code blocks at
.claude/skills/quality-vector-tests/references/example-lean.md:9-9 and
.claude/skills/quality-vector-tests/references/example-migration.md:8-8 to
include the markdown language identifier, changing each unannotated fence to a
markdown-labeled fence.
- Around line 100-101: Update the summary’s connector denominator from 25 to 26
in the referenced coverage guidance, keeping it consistent with the grounding
and canonical target statements.
- Around line 67-68: Correct the pagination example’s arithmetic in the
measurement description: update the page count for 3,000 records at a page size
of 500 from 7 to 6, while preserving the existing completeness targets of zero
duplicates, zero omissions, and exactly 3,000 records.
In @.claude/skills/quality-vector-tests/references/example-migration.md:
- Around line 79-81: Update the vector-count summary in the migration example to
state that each vector has one or more checks, or one section per vector,
instead of claiming one check per vector; leave the other summary points
unchanged.
In @.claude/skills/quality-vector-tests/references/example-port.md:
- Around line 44-49: Clarify the “Cross-namespace resolution” check by defining
the eligible fixture/evidence denominator and an explicit pass threshold above
the current 68% baseline, or reclassify it as a deferred observation instead of
a canonical test check. Update the metric, measurement, and target text
consistently within the cross-namespace section.
In @.claude/skills/quality-vector-tests/SKILL.md:
- Around line 40-41: Update the worked examples in the quality-vector test
guidance to use an exact record fixture and calculate the corresponding page
count explicitly, rather than saying “3,000+” and “7 pages.” Replace “total
exact” with a concrete numeric assertion, and split combined outcomes into
separate checks so API coverage, refusal behavior, connector count, and
metric-key count are independently measurable.
- Around line 60-64: Update the GitHub CLI examples in the quality-vector-tests
instructions to use a clearly defined issue-number variable or consistent
placeholder instead of the literal n. Apply it consistently to both gh issue
view and gh pr list commands, while preserving their existing repository, JSON
fields, search, and state options.
---
Nitpick comments:
In @.claude/skills/drive-ui/SKILL.md:
- Around line 60-65: Update the authentication workflow documentation around the
state-save example to include the exact corresponding playwright-cli state-load
command using the same Chrome session and ~/.playwright-auth/stand-state.json
path. Keep the stale-state guidance and cleanup instruction unchanged, and
ensure the restore syntax matches the documented save syntax.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c213d26b-55ff-4e66-b990-286d39470833
📥 Commits
Reviewing files that changed from the base of the PR and between c619daa54a000bd2468fdf5e6e20c1a3b3d33b8f and cc56575.
📒 Files selected for processing (17)
.claude/skills/drive-ui/SKILL.md.claude/skills/file-bug-insight/SKILL.md.claude/skills/playwright-cli/SKILL.md.claude/skills/playwright-cli/references/element-attributes.md.claude/skills/playwright-cli/references/playwright-tests.md.claude/skills/playwright-cli/references/request-mocking.md.claude/skills/playwright-cli/references/running-code.md.claude/skills/playwright-cli/references/session-management.md.claude/skills/playwright-cli/references/spec-driven-testing.md.claude/skills/playwright-cli/references/storage-state.md.claude/skills/playwright-cli/references/tracing.md.claude/skills/playwright-cli/references/video-recording.md.claude/skills/quality-vector-tests/SKILL.md.claude/skills/quality-vector-tests/references/example-lean.md.claude/skills/quality-vector-tests/references/example-migration.md.claude/skills/quality-vector-tests/references/example-port.md.claude/skills/quality-vector-tests/references/vector-mapping.md
🚧 Files skipped from review as they are similar to previous changes (4)
- .claude/skills/playwright-cli/references/playwright-tests.md
- .claude/skills/playwright-cli/references/session-management.md
- .claude/skills/playwright-cli/references/storage-state.md
- .claude/skills/playwright-cli/references/spec-driven-testing.md
| > ``` | ||
| > 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` | ||
| > ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify a language for the fenced log block.
The verbatim server-log fence lacks a language identifier and triggers Markdownlint MD040. Use an appropriate label such as text or console.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 164-164: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/file-bug-insight/SKILL.md around lines 164 - 168, Update the
fenced server-log block in the relevant documentation section to include an
appropriate language identifier, such as text or console, while preserving the
log content unchanged.
Source: Linters/SAST tools
| ``` | ||
| **/api/users - Exact path match | ||
| **/api/*/details - Wildcard in path | ||
| **/*.{png,jpg,jpeg} - Match file extensions | ||
| **/search?q=* - Match query parameters | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify a language for the URL-pattern code fence.
SkillSpector reports markdownlint MD040 for this unlabeled fenced block. Use text for the pattern reference block.
Proposed fix
-```
+```text
**/api/users - Exact path match
**/api/*/details - Wildcard in path
**/*.{png,jpg,jpeg} - Match file extensions
**/search?q=* - Match query parameters</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 30-30: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/playwright-cli/references/request-mocking.md around lines 30
- 35, Label the URL-pattern fenced code block in the request-mocking
documentation as text by adding the text language identifier to its opening
fence; leave the pattern examples unchanged.
Source: Linters/SAST tools
| tooling doesn't exist. | ||
|
|
||
| ## Before (loose author draft) | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to the fenced code blocks.
Both fences trigger Markdownlint MD040. Use markdown (or the appropriate language) after the opening backticks:
.claude/skills/quality-vector-tests/references/example-lean.md#L9-L9: changetomarkdown`..claude/skills/quality-vector-tests/references/example-migration.md#L8-L8: changetomarkdown`.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
📍 Affects 2 files
.claude/skills/quality-vector-tests/references/example-lean.md#L9-L9(this comment).claude/skills/quality-vector-tests/references/example-migration.md#L8-L8
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/quality-vector-tests/references/example-lean.md at line 9,
Update the opening fenced code blocks at
.claude/skills/quality-vector-tests/references/example-lean.md:9-9 and
.claude/skills/quality-vector-tests/references/example-migration.md:8-8 to
include the markdown language identifier, changing each unannotated fence to a
markdown-labeled fence.
Source: Linters/SAST tools
| - 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). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the pagination arithmetic.
A 3,000-record fixture with page size 500 produces 6 pages, not 7. The current instruction can make a valid implementation fail the example’s completeness check.
Proposed fix
- - How measured: page a 3,000-record fixture at page size 500 (7 pages) and union the pages.
+ - How measured: page a 3,000-record fixture at page size 500 (6 pages) and union the pages.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - 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). | |
| - How measured: page a 3,000-record fixture at page size 500 (6 pages) and union the pages. | |
| - Target: **0** duplicates, **0** omissions, total exact (3,000, not 500). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/quality-vector-tests/references/example-lean.md around lines
67 - 68, Correct the pagination example’s arithmetic in the measurement
description: update the page count for 3,000 records at a page size of 500 from
7 to 6, while preserving the existing completeness targets of zero duplicates,
zero omissions, and exactly 3,000 records.
| - Gave every target a denominator counted from the repo (59, 25, 17, 3,000/500) instead of "all" | ||
| or "100%". |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the connector denominator consistent.
The grounding and canonical target both state 26 connectors, but this summary says 25. Change the summary to 26 to avoid teaching conflicting coverage requirements.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/quality-vector-tests/references/example-lean.md around lines
100 - 101, Update the summary’s connector denominator from 25 to 26 in the
referenced coverage guidance, keeping it consistent with the grounding and
canonical target statements.
| - 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`). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the vector-count summary.
The canonical example has multiple checks in Reliability, Versatility, and Performance. Replace “one check per vector” with “one or more checks per vector” (or “one section per vector”) so the summary matches the example.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/quality-vector-tests/references/example-migration.md around
lines 79 - 81, Update the vector-count summary in the migration example to state
that each vector has one or more checks, or one section per vector, instead of
claiming one check per vector; leave the other summary points unchanged.
| 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. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the cross-namespace target measurable.
The metric is a share, but “resolved wherever evidence allows” defines neither the eligible denominator nor a pass threshold; 68% is only a current baseline. Add an explicit denominator and target, or mark this as a deferred observation rather than a canonical test check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/quality-vector-tests/references/example-port.md around lines
44 - 49, Clarify the “Cross-namespace resolution” check by defining the eligible
fixture/evidence denominator and an explicit pass threshold above the current
68% baseline, or reclassify it as a deferred observation instead of a canonical
test check. Update the metric, measurement, and target text consistently within
the cross-namespace section.
| > **Page errors** — page through 3,000+ records at page size 500 (7 pages) → **0 duplicates, 0 | ||
| > omissions, total exact** |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep worked examples aligned with the measurable-target contract.
3,000+ records does not imply exactly seven pages at page size 500, and total exact is not a numeric target. The table also combines independent outcomes such as API coverage plus refusal behavior and connector count plus metric-key count. Use exact fixtures and split each independently reportable outcome into its own check.
Also applies to: 237-242
🧰 Tools
🪛 SkillSpector (2.4.4)
[warning] 251: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/quality-vector-tests/SKILL.md around lines 40 - 41, Update
the worked examples in the quality-vector test guidance to use an exact record
fixture and calculate the corresponding page count explicitly, rather than
saying “3,000+” and “7 pages.” Replace “total exact” with a concrete numeric
assertion, and split combined outcomes into separate checks so API coverage,
refusal behavior, connector count, and metric-key count are independently
measurable.
| 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. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not pass the literal n to GitHub CLI commands.
As written, gh issue view n and --search "n" inspect the literal string n, not the requested issue. Use a clearly defined variable or consistent issue-number placeholder before users copy these commands.
🧰 Tools
🪛 SkillSpector (2.4.4)
[warning] 251: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/quality-vector-tests/SKILL.md around lines 60 - 64, Update
the GitHub CLI examples in the quality-vector-tests instructions to use a
clearly defined issue-number variable or consistent placeholder instead of the
literal n. Apply it consistently to both gh issue view and gh pr list commands,
while preserving their existing repository, JSON fields, search, and state
options.
2 participants
Four skills for the QA workflow, under
.claude/skills/. Docs only — no product code, no CI changes.What is here
file-bug-insight— file an Insight defect as a GitHub issue in this repo. Triage against existing issues, gather evidence down the medallion, localize the fix to the layer that owns it, draft a report that reads in under a minute, confirm, create, verify. Issues are created with the native Type=Bug and no labels — component, team and planning labels are a grooming call, and a wrong one routes the bug to the wrong team.The name matters: a user-level skill called
file-bugshadows a project skill of the same name, and the project copy then never loads with no warning. Hencefile-bug-insight.drive-ui— get an authenticated browser on any stand and capture evidence someone can act on. Covers the local path (fakeidp default, theDEV_USER_EMAILseed behind403/caller_unresolved, ports and the kind port-forward) and the remote one (attach over CDP, never launch — a passkey cannot be completed in a browser you started; detach, never close). Includes the check that stops a gold-view defect being filed against the SPA: read the console and the API response before calling a wrong number a frontend bug.playwright-cli— the browser command surface, vendored sodrive-uihas its commands in-repo.quality-vector-tests— author the Testing section of a feature issue, grouped by the five quality vectors, every check written as Metric / How measured / Target. Provenance and specific feature numbers were removed so it stays usable as the issues it was derived from move on.Verification
Every factual claim was checked against the live repo, the project board and the installed tooling rather than written from memory. That pass caught, and this branch fixes:
handlers.rsL992 → L1070,entities.rsL38 → L39)silver.prefix, and all three need the password compose sets--limit 20never reaches a regressionBoard and priority field IDs, every
ghflag, all referenced repo paths and theinsight-frontroutes andaria-labelselectors were confirmed live.Notes for review
metric-parity,release-verify, a finding contract) are deliberately hedged — a missing sibling degrades to the hand-run commands rather than failing..claude/is not wired into CI, so nothing here runs automatically.Summary by CodeRabbit