Fix QA reliability issues from code review

docs/profile-schema.md

@@ -36,11 +36,12 @@ Required selectors for chatbot-style profiles:
 
 ```json
 {
-  "chatInput": { "role": "textbox", "nameIncludes": "..." },
-  "answerDoneText": "..."
+  "chatInput": { "role": "textbox", "nameIncludes": "..." }
 }
 ```
 
+`answerDoneText` is optional. When set, `question` scenarios wait for an extra occurrence of this text to confirm the answer finished; when omitted, they wait only for the submitted text to appear.
+
 Optional consent selectors:
 
 ```json
@@ -66,7 +67,7 @@ Finds the consent button, optionally fills a label input, clicks agree, then wai
 
 ### `question`
 
-Fills `selectors.chatInput`, presses Enter, waits for submitted text and an additional `selectors.answerDoneText` occurrence.
+Fills `selectors.chatInput`, presses Enter, then waits for the submitted text to appear. If `selectors.answerDoneText` is set, it additionally waits for one more occurrence of that marker (used to detect that the answer finished streaming). When `answerDoneText` is omitted, the scenario waits only for the submitted text — it does not hang.
 
 ```json
 {
@@ -79,14 +80,17 @@ Fills `selectors.chatInput`, presses Enter, waits for submitted text and an addi
 
 ### `empty-input`
 
-Attempts to submit empty/blank input and asserts the accessibility tree does not materially change.
+Attempts to submit empty/blank input and asserts the accessibility tree does not materially change (i.e. no new answer was produced).
+
+`maxNodeDelta` (default `2`) is the number of additional accessibility nodes tolerated after submitting blank input — raise it if the UI legitimately shows an aria-live validation hint on empty submit.
 
 ```json
 {
   "type": "empty-input",
   "name": "empty-input-guard",
   "text": "   ",
-  "waitMs": 500
+  "waitMs": 500,
+  "maxNodeDelta": 2
 }
 ```
 

src/core/artifacts.mjs

@@ -31,11 +31,19 @@ export class ArtifactStore {
     if (item) item.screenshots.push(rel);
   }
 
+  // In-memory snapshot for polling. Unlike snapshot(), it does not write a file
+  // or advance the artifact counter, so poll iterations don't flood the output
+  // directory or scramble the numbering of persisted artifacts.
+  async pollSnapshot() {
+    const result = await this.client.run('snapshot-poll', ['take_snapshot']);
+    return result.snapshot || result;
+  }
+
   async waitForSnapshot(predicate, timeout) {
     const deadline = Date.now() + timeout;
     let last;
     while (Date.now() < deadline) {
-      last = await this.snapshot(`poll-${Date.now()}`);
+      last = await this.pollSnapshot();
       if (predicate(last)) return last;
       await sleep(1000);
     }

src/core/quality.mjs

@@ -3,7 +3,7 @@ export function analyzeQuality(r, quality = {}) {
   const warnings = [];
 
   for (const msg of r.consoleMessages?.consoleMessages || []) {
-    if (msg.type === 'error') warnings.push(`Console error/warning: ${msg.text}`);
+    if (msg.type === 'error') warnings.push(`Console error: ${msg.text}`);
   }
 
   for (const req of r.networkRequests?.networkRequests || []) {

src/core/utils.mjs

@@ -27,5 +27,7 @@ export function parseJsonOutput(stdout) {
       // CLI startup notices can precede JSON; keep scanning.
     }
   }
-  return { raw: stdout };
+  // Surface the real CLI failure instead of letting an unparseable response flow
+  // downstream as missing `.snapshot`, which masquerades as "element not found".
+  throw new Error(`chrome-devtools did not return parseable JSON output:\n${stdout.slice(0, 500)}`);
 }

src/scenarios/chatbot.mjs

@@ -25,10 +25,16 @@ async function runConsentScenario({ spec, item, profile, client, artifacts }) {
 
 async function runQuestionScenario({ spec, item, profile, client, artifacts, timeoutMs }) {
   const before = await artifacts.snapshot(`${item.name}-before`, item);
-  const beforeCount = countTextOccurrences(before, profile.selectors?.answerDoneText || '');
+  const answerDoneText = profile.selectors?.answerDoneText || '';
+  const beforeCount = countTextOccurrences(before, answerDoneText);
   await askFromSnapshot({ snap: before, text: spec.text || '', profile, client });
   const done = await artifacts.waitForSnapshot((snap) => {
-    return hasText(snap, spec.text || '') && countTextOccurrences(snap, profile.selectors?.answerDoneText || '') >= beforeCount + 1;
+    if (!hasText(snap, spec.text || '')) return false;
+    // When no answerDoneText marker is configured, degrade to waiting only for
+    // the submitted text. countTextOccurrences('') is always 0, so requiring the
+    // marker here would otherwise guarantee a timeout.
+    if (!answerDoneText) return true;
+    return countTextOccurrences(snap, answerDoneText) >= beforeCount + 1;
   }, spec.timeoutMs || timeoutMs);
   assert(hasText(done, spec.text || ''), 'submitted text should appear in snapshot');
   await artifacts.snapshot(`${item.name}-after`, item);
@@ -41,7 +47,13 @@ async function runEmptyInputScenario({ spec, item, profile, client, artifacts })
   await askFromSnapshot({ snap: before, text: spec.text || '   ', profile, client });
   await sleep(spec.waitMs || 500);
   const after = await artifacts.snapshot(`${item.name}-after`, item);
-  assert(flatten(after).length <= beforeTextCount + 2, 'empty input should not materially change the accessibility tree');
+  // Empty input should not produce a new answer. A few extra nodes (e.g. an
+  // aria-live validation hint) are tolerated; tune via spec.maxNodeDelta.
+  const maxNodeDelta = Number.isInteger(spec.maxNodeDelta) ? spec.maxNodeDelta : 2;
+  assert(
+    flatten(after).length <= beforeTextCount + maxNodeDelta,
+    `empty input should not materially change the accessibility tree (added ${flatten(after).length - beforeTextCount} nodes, allowed ${maxNodeDelta})`,
+  );
   await artifacts.screenshot(`${item.name}-after`, item);
 }
 

test/quality.test.mjs

@@ -0,0 +1,47 @@
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import { analyzeQuality } from '../src/core/quality.mjs';
+import { parseJsonOutput } from '../src/core/utils.mjs';
+
+test('analyzeQuality flags 5xx as failure and 4xx as warning', () => {
+  const result = analyzeQuality({
+    networkRequests: {
+      networkRequests: [
+        { method: 'GET', url: 'https://x/api', status: '502' },
+        { method: 'GET', url: 'https://x/missing', status: '404' },
+        { method: 'GET', url: 'https://x/ok', status: '200' },
+      ],
+    },
+  });
+  assert.equal(result.status, 'fail');
+  assert.equal(result.failures.length, 1);
+  assert.match(result.failures[0], /502/);
+  assert.equal(result.warnings.length, 1);
+  assert.match(result.warnings[0], /404/);
+});
+
+test('analyzeQuality reports console errors as warnings', () => {
+  const result = analyzeQuality({
+    consoleMessages: { consoleMessages: [{ type: 'error', text: 'boom' }] },
+  });
+  assert.equal(result.status, 'warning');
+  assert.equal(result.warnings[0], 'Console error: boom');
+});
+
+test('analyzeQuality honors ignoreFavicon404', () => {
+  const result = analyzeQuality(
+    { networkRequests: { networkRequests: [{ method: 'GET', url: 'https://x/favicon.ico', status: '404' }] } },
+    { ignoreFavicon404: true },
+  );
+  assert.equal(result.status, 'pass');
+  assert.equal(result.warnings.length, 0);
+});
+
+test('parseJsonOutput returns the last JSON object after notices', () => {
+  const stdout = 'startup notice\nupdate available\n{"snapshot":{"role":"RootWebArea"}}';
+  assert.deepEqual(parseJsonOutput(stdout), { snapshot: { role: 'RootWebArea' } });
+});
+
+test('parseJsonOutput throws on unparseable output instead of masking it', () => {
+  assert.throws(() => parseJsonOutput('Error: daemon failed to start'), /did not return parseable JSON/);
+});