refactor(skills): locator-first playwright guidance, drop emoji markers

Two cleanups to the playwright skills, landed together since they overlap the same files.

The skills taught networkidle as the readiness check and leaned on raw page.click/fill/waitForSelector. Playwright discourages networkidle for readiness, so the guidance in both SKILL.md files now waits for a visible app landmark via a web assertion or locator, the login and form examples use getByLabel/getByRole plus expect, the API reference leads with that pattern, and lib/helpers.js defaults waitForPageReady to load (preferring a caller-supplied landmark) and races the success indicator in authenticate instead of waiting on networkidle.

The second cleanup strips emoji console markers across run.js, helpers.js, both SKILL.md files, and the py examples, replacing each with a plain ASCII tag like [ok], [error], or [scan]. node --check and py_compile pass, and an emoji grep comes back clean.

4 files changed, 106 insertions, 64 deletions

diff --git a/playwright-js/API_REFERENCE.md b/playwright-js/API_REFERENCE.md
index 9ee2975..7307cd2 100644
--- a/playwright-js/API_REFERENCE.md
+++ b/playwright-js/API_REFERENCE.md
@@ -95,11 +95,15 @@ const { chromium } = require('playwright');
 
   const page = await context.newPage();
 
-  // Navigate
+  // Navigate. 'load' (or 'domcontentloaded') is the default-safe wait; prefer
+  // asserting on a visible landmark afterward for readiness (see below).
   await page.goto('https://example.com', {
-    waitUntil: 'networkidle'  // Wait for network to be idle
+    waitUntil: 'load'
   });
 
+  // Wait for a real app landmark instead of network state:
+  // await expect(page.getByRole('main')).toBeVisible();
+
   // Your automation here
 
   await browser.close();
@@ -259,6 +263,18 @@ await page.keyboard.press('ArrowDown');
 
 ### Smart Waiting
 
+**Prefer web assertions and locator waits for readiness.** Web assertions
+(`expect(locator).toBeVisible()`, `toHaveText()`, etc.) and `locator.waitFor()`
+auto-wait for a user-visible, app-specific condition — which is what "the page is
+ready" actually means. Lead with these:
+
+```javascript
+// PREFERRED: assert on a visible landmark (auto-waits)
+await expect(page.getByRole('main')).toBeVisible();
+await expect(page.getByRole('heading', { name: 'Dashboard' })).toBeVisible();
+await page.getByText('Welcome back').waitFor();
+```
+
 ```javascript
 // Wait for element states
 await page.locator('button').waitFor({ state: 'visible' });
@@ -270,8 +286,11 @@ await page.locator('button').waitFor({ state: 'detached' });
 await page.waitForURL('**/success');
 await page.waitForURL(url => url.pathname === '/dashboard');
 
-// Wait for network
-await page.waitForLoadState('networkidle');
+// Load-state waits. 'load' / 'domcontentloaded' are fine for "page navigated".
+// AVOID 'networkidle' as a readiness signal — Playwright discourages it; a page
+// can be interactive long before the network quiets (or never quiet at all).
+// Wait on a visible landmark instead (see PREFERRED block above).
+await page.waitForLoadState('load');
 await page.waitForLoadState('domcontentloaded');
 
 // Wait for function
@@ -576,7 +595,7 @@ jobs:
 
 1. **Test Organization** - Use descriptive test names, group related tests
 2. **Selector Strategy** - Prefer data-testid attributes, use role-based selectors
-3. **Waiting** - Use Playwright's auto-waiting, avoid hard-coded delays
+3. **Waiting** - Lead with web assertions (`expect(locator).toBeVisible()`) and locator waits; they auto-wait for a real, visible condition. Avoid `networkidle` as a readiness signal (Playwright discourages it) and avoid hard-coded delays.
 4. **Error Handling** - Add proper error messages, take screenshots on failure
 5. **Performance** - Run tests in parallel, reuse authentication state
 
diff --git a/playwright-js/SKILL.md b/playwright-js/SKILL.md
index b4b037b..40427c3 100644
--- a/playwright-js/SKILL.md
+++ b/playwright-js/SKILL.md
@@ -82,7 +82,7 @@ const TARGET_URL = 'http://localhost:3001'; // <-- Auto-detected or from user
   console.log('Page loaded:', await page.title());
 
   await page.screenshot({ path: '/tmp/screenshot.png', fullPage: true });
-  console.log('📸 Screenshot saved to /tmp/screenshot.png');
+  console.log('[screenshot] Saved to /tmp/screenshot.png');
 
   await browser.close();
 })();
@@ -127,6 +127,7 @@ const TARGET_URL = 'http://localhost:3001'; // Auto-detected
 ```javascript
 // /tmp/playwright-test-login.js
 const { chromium } = require('playwright');
+const { expect } = require('@playwright/test');
 
 const TARGET_URL = 'http://localhost:3001'; // Auto-detected
 
@@ -136,13 +137,15 @@ const TARGET_URL = 'http://localhost:3001'; // Auto-detected
 
   await page.goto(`${TARGET_URL}/login`);
 
-  await page.fill('input[name="email"]', 'test@example.com');
-  await page.fill('input[name="password"]', 'password123');
-  await page.click('button[type="submit"]');
+  // Prefer user-visible locators; they auto-wait for the field to be ready.
+  await page.getByLabel('Email').fill('test@example.com');
+  await page.getByLabel('Password').fill('password123');
+  await page.getByRole('button', { name: /sign in|log in/i }).click();
 
-  // Wait for redirect
+  // Wait for redirect, then assert on a landmark of the destination.
   await page.waitForURL('**/dashboard');
-  console.log('✅ Login successful, redirected to dashboard');
+  await expect(page.getByRole('heading', { name: /dashboard/i })).toBeVisible();
+  console.log('[ok] Login successful, redirected to dashboard');
 
   await browser.close();
 })();
@@ -153,6 +156,7 @@ const TARGET_URL = 'http://localhost:3001'; // Auto-detected
 ```javascript
 // /tmp/playwright-test-form.js
 const { chromium } = require('playwright');
+const { expect } = require('@playwright/test');
 
 const TARGET_URL = 'http://localhost:3001'; // Auto-detected
 
@@ -162,14 +166,14 @@ const TARGET_URL = 'http://localhost:3001'; // Auto-detected
 
   await page.goto(`${TARGET_URL}/contact`);
 
-  await page.fill('input[name="name"]', 'John Doe');
-  await page.fill('input[name="email"]', 'john@example.com');
-  await page.fill('textarea[name="message"]', 'Test message');
-  await page.click('button[type="submit"]');
+  await page.getByLabel('Name').fill('John Doe');
+  await page.getByLabel('Email').fill('john@example.com');
+  await page.getByLabel('Message').fill('Test message');
+  await page.getByRole('button', { name: /submit|send/i }).click();
 
-  // Verify submission
-  await page.waitForSelector('.success-message');
-  console.log('✅ Form submitted successfully');
+  // Verify submission via a web assertion (auto-waits for the message to appear).
+  await expect(page.getByText(/thank you|message sent|success/i)).toBeVisible();
+  console.log('[ok] Form submitted successfully');
 
   await browser.close();
 })();
@@ -203,8 +207,8 @@ const { chromium } = require('playwright');
     }
   }
 
-  console.log(`✅ Working links: ${results.working}`);
-  console.log(`❌ Broken links:`, results.broken);
+  console.log(`[ok] Working links: ${results.working}`);
+  console.log(`[fail] Broken links:`, results.broken);
 
   await browser.close();
 })();
@@ -221,7 +225,7 @@ const { chromium } = require('playwright');
 
   try {
     await page.goto('http://localhost:3000', {
-      waitUntil: 'networkidle',
+      waitUntil: 'load',
       timeout: 10000,
     });
 
@@ -230,9 +234,9 @@ const { chromium } = require('playwright');
       fullPage: true,
     });
 
-    console.log('📸 Screenshot saved to /tmp/screenshot.png');
+    console.log('[screenshot] Saved to /tmp/screenshot.png');
   } catch (error) {
-    console.error('❌ Error:', error.message);
+    console.error('[error]', error.message);
   } finally {
     await browser.close();
   }
@@ -276,7 +280,7 @@ const TARGET_URL = 'http://localhost:3001'; // Auto-detected
     });
   }
 
-  console.log('✅ All viewports tested');
+  console.log('[ok] All viewports tested');
   await browser.close();
 })();
 ```
@@ -395,7 +399,7 @@ For comprehensive Playwright API documentation, see [API_REFERENCE.md](API_REFER
 - **DEFAULT: Visible browser** - Always use `headless: false` unless user explicitly asks for headless mode
 - **Headless mode** - Only use `headless: true` when user specifically requests "headless" or "background" execution
 - **Slow down:** Use `slowMo: 100` to make actions visible and easier to follow
-- **Wait strategies:** Use `waitForURL`, `waitForSelector`, `waitForLoadState` instead of fixed timeouts
+- **Wait strategies:** Lead with web assertions and locators — `await expect(locator).toBeVisible()`, `await locator.waitFor()` — and `waitForURL`. They auto-wait for the real condition. Reach for `waitForSelector` only when a locator won't express the wait. Avoid `waitForLoadState('networkidle')` as a readiness check; Playwright discourages it. Don't use fixed timeouts.
 - **Error handling:** Always use try-catch for robust automation
 - **Console output:** Use `console.log()` to track progress and show what's happening
 
@@ -414,7 +418,7 @@ Ensure running from skill directory via `run.js` wrapper
 Check `headless: false` and ensure display available
 
 **Element not found:**
-Add wait: `await page.waitForSelector('.element', { timeout: 10000 })`
+Wait on a locator: `await page.getByRole('button', { name: 'Save' }).waitFor({ timeout: 10000 })` (or `await expect(locator).toBeVisible()`). These auto-wait for a visible, app-specific element rather than network state.
 
 ## Example Usage
 
@@ -445,7 +449,7 @@ User: "Use 3001"
 
 [Writes login automation to /tmp/playwright-test-login.js]
 [Runs: cd $SKILL_DIR && node run.js /tmp/playwright-test-login.js]
-[Reports: ✅ Login successful, redirected to /dashboard]
+[Reports: [ok] Login successful, redirected to /dashboard]
 ```
 
 ## Notes
@@ -470,27 +474,30 @@ User task → Is it static HTML (file:// or plain server-rendered)?
     │
     └─ No (dynamic webapp) →
         1. Navigate to the page
-        2. Wait for networkidle:  await page.waitForLoadState('networkidle');
+        2. Wait for a visible app landmark:  await expect(page.getByRole('main')).toBeVisible();
+                                              (or: await page.getByText('Dashboard').waitFor();)
         3. Inspect rendered DOM (screenshot, page.content(), or locator().all())
         4. Identify selectors from the rendered state, not the source
         5. Execute actions with those selectors
 ```
 
-**Common pitfall:** inspecting the DOM before `networkidle` on a dynamic app returns stale content or an empty skeleton. Every "element not found" bug on dynamic pages should trigger a "did I wait for networkidle?" check first.
+**Common pitfall:** inspecting the DOM before the app has rendered returns stale content or an empty skeleton. The fix is to wait for a *visible, app-specific landmark* — a heading, a `role=main` region, a known piece of text — via a web assertion (`expect(locator).toBeVisible()`) or `locator.waitFor()`. These auto-wait for the element to appear, which is what "ready" actually means. Don't wait on `networkidle` for this; Playwright explicitly discourages it as a readiness signal, since a page can be interactive long before the network goes quiet (or never go quiet at all, with polling or analytics). Every "element not found" bug on dynamic pages should trigger a "did I wait for a visible landmark?" check first.
 
 ## Added: Reconnaissance-Then-Action Pattern
 
 For any non-trivial interaction on a dynamic page:
 
-1. **Reconnoiter.** Navigate, wait for load, capture state:
+1. **Reconnoiter.** Navigate, wait for a visible landmark, capture state:
    ```javascript
    await page.goto(TARGET_URL);
-   await page.waitForLoadState('networkidle');
+   await expect(page.getByRole('main')).toBeVisible(); // wait for a real app landmark, not network quiet
    await page.screenshot({ path: '/tmp/inspect.png', fullPage: true });
    const html = await page.content();
    const buttons = await page.locator('button').all();
    ```
 
+   `expect` comes from `const { expect } = require('@playwright/test');`. If the landmark isn't known yet, `await page.getByText('<some text you expect>').waitFor()` works the same way. Both auto-wait; neither relies on `networkidle`.
+
 2. **Decide.** From the screenshot + content + locator list, pick the selectors you'll use. Don't guess from source.
 
 3. **Act.** Execute the interaction with the discovered selectors.
diff --git a/playwright-js/lib/helpers.js b/playwright-js/lib/helpers.js
index 0920d68..42aeedd 100644
--- a/playwright-js/lib/helpers.js
+++ b/playwright-js/lib/helpers.js
@@ -82,30 +82,45 @@ async function createPage(context, options = {}) {
 }
 
 /**
- * Smart wait for page to be ready
+ * Smart wait for page to be ready.
+ *
+ * Readiness defaults to the 'load' event. Pass `options.waitForSelector` with an
+ * app-specific landmark (a heading, a role=main region, known text) to wait on
+ * something the app actually rendered — that's the most reliable readiness signal
+ * and the one to prefer. `networkidle` is intentionally NOT the default: Playwright
+ * discourages it for readiness, since a page can be interactive long before the
+ * network goes quiet (and may never go quiet with polling or analytics). Callers
+ * can still pass `waitUntil: 'networkidle'` explicitly if they really need it.
  * @param {Object} page - Playwright page
- * @param {Object} options - Wait options
+ * @param {Object} options - Wait options (waitUntil, timeout, waitForSelector)
  */
 async function waitForPageReady(page, options = {}) {
   const waitOptions = {
-    waitUntil: options.waitUntil || 'networkidle',
+    waitUntil: options.waitUntil || 'load',
     timeout: options.timeout || 30000
   };
-  
+
+  // Prefer waiting on a caller-supplied landmark selector when given; it's a
+  // far stronger readiness signal than any load-state event.
+  if (options.waitForSelector) {
+    try {
+      await page.waitForSelector(options.waitForSelector, {
+        state: 'visible',
+        timeout: waitOptions.timeout
+      });
+      return;
+    } catch (e) {
+      console.warn(`Landmark "${options.waitForSelector}" not visible in time, falling back to load state...`);
+    }
+  }
+
   try {
-    await page.waitForLoadState(waitOptions.waitUntil, { 
-      timeout: waitOptions.timeout 
+    await page.waitForLoadState(waitOptions.waitUntil, {
+      timeout: waitOptions.timeout
     });
   } catch (e) {
     console.warn('Page load timeout, continuing...');
   }
-  
-  // Additional wait for dynamic content if selector provided
-  if (options.waitForSelector) {
-    await page.waitForSelector(options.waitForSelector, { 
-      timeout: options.timeout 
-    });
-  }
 }
 
 /**
@@ -215,10 +230,11 @@ async function authenticate(page, credentials, selectors = {}) {
   await safeType(page, finalSelectors.password, credentials.password);
   await safeClick(page, finalSelectors.submit);
   
-  // Wait for navigation or success indicator
+  // Wait for a post-login landmark to become visible (preferred), or for a
+  // navigation to settle. networkidle is avoided here as a readiness signal.
   await Promise.race([
-    page.waitForNavigation({ waitUntil: 'networkidle' }),
-    page.waitForSelector(selectors.successIndicator || '.dashboard, .user-menu, .logout', { timeout: 10000 })
+    page.waitForSelector(selectors.successIndicator || '.dashboard, .user-menu, .logout', { state: 'visible', timeout: 10000 }),
+    page.waitForNavigation({ waitUntil: 'load' })
   ]).catch(() => {
     console.log('Login might have completed without navigation');
   });
@@ -383,7 +399,7 @@ async function detectDevServers(customPorts = []) {
 
   const detectedServers = [];
 
-  console.log('🔍 Checking for running dev servers...');
+  console.log('[scan] Checking for running dev servers...');
 
   for (const port of allPorts) {
     try {
@@ -397,7 +413,7 @@ async function detectDevServers(customPorts = []) {
         }, (res) => {
           if (res.statusCode < 500) {
             detectedServers.push(`http://localhost:${port}`);
-            console.log(`  ✅ Found server on port ${port}`);
+            console.log(`  [ok] Found server on port ${port}`);
           }
           resolve();
         });
@@ -416,7 +432,7 @@ async function detectDevServers(customPorts = []) {
   }
 
   if (detectedServers.length === 0) {
-    console.log('  ❌ No dev servers detected');
+    console.log('  [none] No dev servers detected');
   }
 
   return detectedServers;
diff --git a/playwright-js/run.js b/playwright-js/run.js
index 10f2616..ade36cb 100755
--- a/playwright-js/run.js
+++ b/playwright-js/run.js
@@ -33,14 +33,14 @@ function checkPlaywrightInstalled() {
  * Install Playwright if missing
  */
 function installPlaywright() {
-  console.log('📦 Playwright not found. Installing...');
+  console.log('[setup] Playwright not found. Installing...');
   try {
     execSync('npm install', { stdio: 'inherit', cwd: __dirname });
     execSync('npx playwright install chromium', { stdio: 'inherit', cwd: __dirname });
-    console.log('✅ Playwright installed successfully');
+    console.log('[setup] Playwright installed successfully');
     return true;
   } catch (e) {
-    console.error('❌ Failed to install Playwright:', e.message);
+    console.error('[error] Failed to install Playwright:', e.message);
     console.error('Please run manually: cd', __dirname, '&& npm run setup');
     return false;
   }
@@ -55,24 +55,24 @@ function getCodeToExecute() {
   // Case 1: File path provided
   if (args.length > 0 && fs.existsSync(args[0])) {
     const filePath = path.resolve(args[0]);
-    console.log(`📄 Executing file: ${filePath}`);
+    console.log(`[file] Executing file: ${filePath}`);
     return fs.readFileSync(filePath, 'utf8');
   }
 
   // Case 2: Inline code provided as argument
   if (args.length > 0) {
-    console.log('⚡ Executing inline code');
+    console.log('[inline] Executing inline code');
     return args.join(' ');
   }
 
   // Case 3: Code from stdin
   if (!process.stdin.isTTY) {
-    console.log('📥 Reading from stdin');
+    console.log('[stdin] Reading from stdin');
     return fs.readFileSync(0, 'utf8');
   }
 
   // No input
-  console.error('❌ No code to execute');
+  console.error('[error] No code to execute');
   console.error('Usage:');
   console.error('  node run.js script.js          # Execute file');
   console.error('  node run.js "code here"        # Execute inline');
@@ -146,7 +146,7 @@ function getContextOptionsWithHeaders(options = {}) {
   try {
     ${code}
   } catch (error) {
-    console.error('❌ Automation error:', error.message);
+    console.error('[error] Automation error:', error.message);
     if (error.stack) {
       console.error(error.stack);
     }
@@ -163,7 +163,7 @@ function getContextOptionsWithHeaders(options = {}) {
   try {
     ${code}
   } catch (error) {
-    console.error('❌ Automation error:', error.message);
+    console.error('[error] Automation error:', error.message);
     if (error.stack) {
       console.error(error.stack);
     }
@@ -180,7 +180,7 @@ function getContextOptionsWithHeaders(options = {}) {
  * Main execution
  */
 async function main() {
-  console.log('🎭 Playwright Skill - Universal Executor\n');
+  console.log('Playwright Skill - Universal Executor\n');
 
   // Clean up old temp files from previous runs
   cleanupOldTempFiles();
@@ -205,16 +205,16 @@ async function main() {
     fs.writeFileSync(tempFile, code, 'utf8');
 
     // Execute the code
-    console.log('🚀 Starting automation...\n');
+    console.log('[run] Starting automation...\n');
     require(tempFile);
 
     // Note: Temp file will be cleaned up on next run
     // This allows long-running async operations to complete safely
 
   } catch (error) {
-    console.error('❌ Execution failed:', error.message);
+    console.error('[error] Execution failed:', error.message);
     if (error.stack) {
-      console.error('\n📋 Stack trace:');
+      console.error('\n[trace] Stack trace:');
       console.error(error.stack);
     }
     process.exit(1);
@@ -223,6 +223,6 @@ async function main() {
 
 // Run main function
 main().catch(error => {
-  console.error('❌ Fatal error:', error.message);
+  console.error('[error] Fatal error:', error.message);
   process.exit(1);
 });