Add advisory changed-file coverage guardrails and finalize batch operation contract - PR_26126_076-coverage-and-batch-guardrails

DavidQ · DavidQ · commit 55089d459eda · 2026-05-05T23:21:48.000-04:00
diff --git a/docs/dev/PROJECT_INSTRUCTIONS.md b/docs/dev/PROJECT_INSTRUCTIONS.md
@@ -589,8 +589,11 @@ No PR is complete with:
 ## BATCH OPERATION RULES
 
 - Batch operations must log per item.
+- Each item must log `OK`, `WARN`, `FAIL`, or `SKIP`.
 - One failed item must not stop the batch unless the failure is global.
 - Summary must include written, failed, skipped, and warnings.
+- Long-running batches must support a stop or cancel pattern when applicable.
+- Batch operations must discover real files and directories and must not assume numeric folder sequences.
 
 ## PLAYWRIGHT DEPTH AND COVERAGE REQUIREMENT
 
@@ -626,6 +629,8 @@ When runtime JavaScript changes, Codex must produce a Playwright V8 coverage rep
 
 The coverage report must list changed runtime JavaScript files.
 
+Missing changed runtime JavaScript files in coverage must be reported as `WARN`, not `FAIL`.
+
 Coverage report lines must start with coverage percentage in this format:
 
 `(xx%) <file-path> - <details>`
diff --git a/docs/dev/reports/batch_guardrail_contract.txt b/docs/dev/reports/batch_guardrail_contract.txt
@@ -0,0 +1,26 @@
+PR_26126_076 Batch Guardrail Contract
+
+Scope:
+- Added Tool Template V2 batch guardrail documentation.
+- Linked the guardrail contract from the template README and control/service contracts.
+- Updated PROJECT_INSTRUCTIONS.md with the finalized batch guardrail wording.
+
+Documented guardrails:
+- Batch operations log every item.
+- Each item logs OK, WARN, FAIL, or SKIP.
+- One item failure does not stop the batch unless the failure is global.
+- Summary includes written, failed, skipped, and warnings.
+- Long-running batches support stop/cancel when applicable.
+- Batch discovery uses real files/directories and never assumes numeric folder sequences.
+
+Runtime impact:
+- No tool runtime behavior changed.
+
+Validation:
+- node --check tests/helpers/playwrightV8CoverageReporter.mjs passed.
+- git diff --check passed.
+- npm run test:workspace-v2 passed.
+
+Manual test notes:
+- Review tools/templates-v2/docs/BATCH_GUARDRAIL_CONTRACT.md for the batch contract wording.
+- Review docs/dev/reports/coverage_changed_js_guardrail.txt to confirm missing changed runtime JS coverage is advisory WARN-only.
diff --git a/docs/dev/reports/coverage_changed_js_guardrail.txt b/docs/dev/reports/coverage_changed_js_guardrail.txt
@@ -0,0 +1,12 @@
+Changed Runtime JS Coverage Guardrail
+
+Status: advisory only.
+Thresholds: none enforced.
+Missing changed runtime JS files are WARN, not FAIL.
+Source: Playwright/Chromium built-in V8 coverage from npm run test:workspace-v2.
+
+Changed runtime JS files considered:
+(100%) none changed - no changed runtime JS files
+
+Guardrail warnings:
+(100%) none changed - no changed runtime JS files
diff --git a/docs/dev/reports/playwright_v8_coverage_report.txt b/docs/dev/reports/playwright_v8_coverage_report.txt
@@ -6,6 +6,7 @@ Coverage scope: all repo-relative browser JavaScript collected by Playwright/Chr
 Dependencies: no new npm packages.
 Thresholds: none enforced.
 Note: coverage is an advisory baseline only for this PR.
+Note: missing changed runtime JS coverage is reported as WARN, not FAIL.
 Note: line counts are V8 range-based and advisory; function counts show partial module exercise where available.
 Note: entry percentages use function coverage when available, otherwise line coverage.
 Note: coverage entries are aggregated across every page/tool where coverageReporter.start(page) and coverageReporter.stop(page) ran.
@@ -18,7 +19,7 @@ Exercised tool entry points detected:
 (0%) Workspace Manager - not exercised by this Playwright run
 
 Changed runtime JS files covered:
-(86%) tools/templates-v2/js/controls/StatusLogControl.js - executed lines 24/24; executed functions 6/7
+(100%) none changed - no changed runtime JS files
 
 Files with executed line/function counts where available:
 (2%) src/engine/input/ActionInputService.js - executed lines 397/397; executed functions 1/51
@@ -156,8 +157,8 @@ Files with executed line/function counts where available:
 (100%) tools/templates-v2/js/services/ToolStateSerializer.js - executed lines 13/13; executed functions 3/3
 
 Uncovered or low-coverage changed JS files:
-(100%) none - no low-coverage changed runtime JS files
+(100%) none changed - no changed runtime JS files
 
 Changed JS files considered:
+(0%) tests/helpers/playwrightV8CoverageReporter.mjs - changed JS file not collected as browser runtime coverage
 (0%) tests/playwright/PreviewGeneratorV2Baseline.spec.mjs - changed JS file not collected as browser runtime coverage
-(86%) tools/templates-v2/js/controls/StatusLogControl.js - changed JS file with browser V8 coverage
diff --git a/docs/dev/reports/template_form_style_match_palette_manager.txt b/docs/dev/reports/template_form_style_match_palette_manager.txt
@@ -0,0 +1,49 @@
+PR_26126_075-template-form-style-match-palette-manager
+
+Scope:
+- Tool Template V2 form/control visual styling only.
+- Existing Tool Template V2 runtime behavior, launch-mode NAV behavior, Status Clear placement, and Status Clear behavior were preserved.
+
+Palette Manager V2 patterns reviewed:
+- `tools/palette-manager-v2/index.html`
+- `tools/palette-manager-v2/paletteManagerV2.css`
+- Structural/style patterns used:
+  - `.palette-manager-v2__field`
+  - `.palette-manager-v2__field--compact`
+  - `.palette-manager-v2__controls`
+  - `.palette-manager-v2__menu-sample`
+  - `.palette-manager-v2__panel`
+  - `.accordion-v2__header`
+  - Palette Manager global surface rules for `button`, `input`, `select`, and `textarea`
+  - Palette Manager text/log surface rules such as `.palette-manager-v2__json-preview`
+
+Changes:
+- Updated `tools/templates-v2/styles/toolStarter.css` to match Palette Manager V2 visual control patterns:
+  - label/control fields use the compact 74px label column and 8px gap
+  - inputs/selects/textareas use Palette Manager surface border, 10px radius, surface-inline background, and text color
+  - buttons use Palette Manager-style 10px radius, compact native button padding, disabled opacity/filter, and focus outline
+  - textarea/output blocks use Palette Manager-style strong surface, 8px radius, and 12px padding
+  - panels use Palette Manager-style border, 8px radius, panel background, 14px padding, and 14px spacing
+  - menu/app shell spacing now follows the Palette Manager V2 shell/menu pattern
+- Added Playwright computed-style assertions for Tool Template V2:
+  - compact field gap and label column
+  - input radius/background/text color
+  - button radius and compact padding
+  - textarea radius/background
+  - panel radius/padding
+
+Playwright impacted: Yes
+- This PR changes visible UI/control styling for Tool Template V2.
+- `npm run test:workspace-v2` passed.
+
+Validation:
+- `node --check tests/playwright/PreviewGeneratorV2Baseline.spec.mjs` passed.
+- `npm run test:workspace-v2` passed: 7 tests passed.
+- `git diff --check` passed.
+
+Manual test notes:
+- Open `tools/templates-v2/index.html`.
+- Confirm default tool-mode NAV is visible and workspace NAV remains hidden.
+- Open `tools/templates-v2/index.html?launch=workspace`.
+- Confirm workspace-mode NAV is visible and tool NAV remains hidden.
+- Confirm template labels, inputs, textarea, buttons, field spacing, panel spacing, and accordion controls visually match Palette Manager V2 patterns.
diff --git a/tests/helpers/playwrightV8CoverageReporter.mjs b/tests/helpers/playwrightV8CoverageReporter.mjs
@@ -7,10 +7,12 @@ export class PlaywrightV8CoverageReporter {
   constructor({
     repoRoot = process.cwd(),
     reportPath = "docs/dev/reports/playwright_v8_coverage_report.txt",
+    guardrailReportPath = "docs/dev/reports/coverage_changed_js_guardrail.txt",
     advisoryLowCoveragePercent = 50
   } = {}) {
     this.repoRoot = repoRoot;
     this.reportPath = path.resolve(repoRoot, reportPath);
+    this.guardrailReportPath = path.resolve(repoRoot, guardrailReportPath);
     this.advisoryLowCoveragePercent = advisoryLowCoveragePercent;
     this.activePages = new WeakSet();
     this.entries = [];
@@ -49,6 +51,7 @@ export class PlaywrightV8CoverageReporter {
       "Dependencies: no new npm packages.",
       "Thresholds: none enforced.",
       "Note: coverage is an advisory baseline only for this PR.",
+      "Note: missing changed runtime JS coverage is reported as WARN, not FAIL.",
       "Note: line counts are V8 range-based and advisory; function counts show partial module exercise where available.",
       "Note: entry percentages use function coverage when available, otherwise line coverage.",
       "Note: coverage entries are aggregated across every page/tool where coverageReporter.start(page) and coverageReporter.stop(page) ran.",
@@ -71,6 +74,7 @@ export class PlaywrightV8CoverageReporter {
 
     await fs.mkdir(path.dirname(this.reportPath), { recursive: true });
     await fs.writeFile(this.reportPath, `${reportLines.join("\n")}\n`, "utf8");
+    await this.writeChangedJsGuardrailReport(changedRuntimeJsFiles, coverageByPath);
   }
 
   buildCoverageByPath() {
@@ -212,10 +216,14 @@ export class PlaywrightV8CoverageReporter {
   }
 
   getChangedJsFiles() {
-    const statusOutput = execFileSync("git", ["status", "--porcelain"], {
-      cwd: this.repoRoot,
-      encoding: "utf8"
-    });
+    return [...new Set([
+      ...this.getStatusChangedJsFiles(),
+      ...this.getHeadChangedJsFiles()
+    ])].sort((left, right) => left.localeCompare(right));
+  }
+
+  getStatusChangedJsFiles() {
+    const statusOutput = this.gitOutput(["status", "--porcelain"]);
     return statusOutput
       .split(/\r?\n/)
       .map((line) => line.trimEnd())
@@ -227,6 +235,28 @@ export class PlaywrightV8CoverageReporter {
       .sort((left, right) => left.localeCompare(right));
   }
 
+  getHeadChangedJsFiles() {
+    const headOutput = this.gitOutput(["diff-tree", "--no-commit-id", "--name-only", "-r", "HEAD"]);
+    return headOutput
+      .split(/\r?\n/)
+      .map((line) => line.trim().replaceAll("\\", "/"))
+      .filter(Boolean)
+      .filter((filePath) => existsSync(path.resolve(this.repoRoot, filePath)))
+      .filter((filePath) => filePath.endsWith(".js") || filePath.endsWith(".mjs"))
+      .sort((left, right) => left.localeCompare(right));
+  }
+
+  gitOutput(args) {
+    try {
+      return execFileSync("git", args, {
+        cwd: this.repoRoot,
+        encoding: "utf8"
+      });
+    } catch {
+      return "";
+    }
+  }
+
   isRuntimeJsFile(filePath) {
     if (filePath.includes("/tests/")) {
       return false;
@@ -258,7 +288,7 @@ export class PlaywrightV8CoverageReporter {
       .map(({ filePath, record }) => (
         record
           ? this.formatCoverageEntry(filePath, record, `${this.lineSummary(record)}; ${this.functionSummary(record)}`)
-          : "(0%) " + filePath + " - not covered"
+          : `(0%) ${filePath} - WARNING: changed runtime JS file was not collected by Playwright V8 coverage; advisory only`
       ));
   }
 
@@ -313,8 +343,8 @@ export class PlaywrightV8CoverageReporter {
     }
     return lowCoverage.map(({ filePath, record }) => (
       record
-        ? this.formatCoverageEntry(filePath, record, `advisory low coverage; ${this.lineSummary(record)}`)
-        : "(0%) " + filePath + " - uncovered"
+        ? this.formatCoverageEntry(filePath, record, `WARNING: advisory low coverage; ${this.lineSummary(record)}`)
+        : `(0%) ${filePath} - WARNING: uncovered changed runtime JS file; advisory only`
     ));
   }
 
@@ -336,6 +366,44 @@ export class PlaywrightV8CoverageReporter {
     return `(${this.coveragePercent(record)}%) ${filePath} - ${details}`;
   }
 
+  async writeChangedJsGuardrailReport(changedRuntimeJsFiles, coverageByPath) {
+    const reportLines = [
+      "Changed Runtime JS Coverage Guardrail",
+      "",
+      "Status: advisory only.",
+      "Thresholds: none enforced.",
+      "Missing changed runtime JS files are WARN, not FAIL.",
+      "Source: Playwright/Chromium built-in V8 coverage from npm run test:workspace-v2.",
+      "",
+      "Changed runtime JS files considered:",
+      ...this.formatChangedRuntimeCoverage(changedRuntimeJsFiles, coverageByPath),
+      "",
+      "Guardrail warnings:",
+      ...this.formatCoverageGuardrailWarnings(changedRuntimeJsFiles, coverageByPath)
+    ];
+
+    await fs.mkdir(path.dirname(this.guardrailReportPath), { recursive: true });
+    await fs.writeFile(this.guardrailReportPath, `${reportLines.join("\n")}\n`, "utf8");
+  }
+
+  formatCoverageGuardrailWarnings(changedRuntimeJsFiles, coverageByPath) {
+    if (!changedRuntimeJsFiles.length) {
+      return ["(100%) none changed - no changed runtime JS files"];
+    }
+    const warnings = changedRuntimeJsFiles
+      .map((filePath) => ({ filePath, record: coverageByPath.get(filePath) }))
+      .filter(({ record }) => !record || this.coveragePercent(record) < this.advisoryLowCoveragePercent)
+      .sort((left, right) => this.compareCoverageEntries(left, right));
+    if (!warnings.length) {
+      return ["(100%) none - no changed runtime JS coverage warnings"];
+    }
+    return warnings.map(({ filePath, record }) => (
+      record
+        ? this.formatCoverageEntry(filePath, record, `WARNING: advisory low coverage below ${this.advisoryLowCoveragePercent}%; ${this.lineSummary(record)}; ${this.functionSummary(record)}`)
+        : `(0%) ${filePath} - WARNING: changed runtime JS file missing from coverage; advisory only`
+    ));
+  }
+
   compareCoverageEntries(left, right) {
     const percentDelta = this.coveragePercent(left.record) - this.coveragePercent(right.record);
     if (percentDelta !== 0) {
diff --git a/tools/templates-v2/README.md b/tools/templates-v2/README.md
@@ -12,6 +12,7 @@ tools/templates-v2/
   playwright.config.mjs
   README.md
   docs/
+    BATCH_GUARDRAIL_CONTRACT.md
     CONTROL_SERVICE_CONTRACTS.md
   styles/
     toolStarter.css
@@ -45,6 +46,7 @@ tools/templates-v2/
 - `js/controls/*.js`: one class per UI control or section.
 - `js/services/*.js`: focused non-UI helper classes when needed.
 - `docs/CONTROL_SERVICE_CONTRACTS.md`: required control, service, app/root, logger, and batch processor contracts.
+- `docs/BATCH_GUARDRAIL_CONTRACT.md`: required batch logging, discovery, stop/cancel, and summary guardrails.
 - `tests/playwright/*.spec.mjs`: starter behavior coverage to copy and rename with the new First-Class Tool V2.
 - `README.md`: tool-specific usage, contracts, and validation notes.
 
@@ -59,13 +61,16 @@ These folders remain under `tools/templates-v2/` as support/reference template a
 
 Read `docs/CONTROL_SERVICE_CONTRACTS.md` before creating or modifying a First-Class Tool V2 from this starter.
 
+Read `docs/BATCH_GUARDRAIL_CONTRACT.md` before adding any batch operation to a First-Class Tool V2.
+
 The contracts define:
 
 - Control responsibilities and method expectations.
 - Service boundaries and result/error return expectations.
 - App/root coordinator boundaries.
 - Logger level requirements: OK, WARN, FAIL, SKIP, INFO.
 - Batch processor discovery, per-item logging, and summary requirements.
+- Batch guardrails for per-item outcomes, stop/cancel behavior, and real file/directory discovery.
 
 ## Architecture Rules
 
diff --git a/tools/templates-v2/docs/BATCH_GUARDRAIL_CONTRACT.md b/tools/templates-v2/docs/BATCH_GUARDRAIL_CONTRACT.md
@@ -0,0 +1,51 @@
+# Tool Template V2 Batch Guardrail Contract
+
+Batch operations process repeated work across real discovered inputs. A First-Class Tool V2 batch flow must be explicit, observable, and safe to resume or diagnose.
+
+## Discovery Rules
+
+- Discover real files and directories.
+- Never assume numeric folder sequences.
+- Missing discovered candidates are logged as `SKIP`, not `FAIL`, unless the missing item is the selected single input.
+- Logs must identify the resolved input path or identifier for every item.
+
+## Per-Item Logging
+
+Every item must log exactly one terminal outcome through the tool logger:
+
+- `OK`: item completed and wrote or updated the expected output.
+- `WARN`: item completed with a recoverable warning.
+- `FAIL`: item failed with an actionable error.
+- `SKIP`: item was intentionally skipped.
+
+Per-item logs must include the item identifier and the reason for `WARN`, `FAIL`, or `SKIP`.
+
+## Failure Isolation
+
+- One item failure must not stop the batch unless the failure is global.
+- Global failures include missing required configuration, unavailable repo access, invalid destination root, or a cancelled run.
+- Batch processors must keep processing remaining discovered items after item-level `FAIL` or `SKIP` outcomes.
+
+## Summary Contract
+
+Every batch run must finish with a summary that includes:
+
+- `written`
+- `failed`
+- `skipped`
+- `warnings`
+
+The summary must match the per-item log outcomes.
+
+## Stop/Cancel Contract
+
+Long-running batches must support a stop or cancel pattern when applicable.
+
+- Stop/cancel requests must prevent new items from starting.
+- Already-running item work should finish or fail safely.
+- Cancelled remaining items should be logged as `SKIP` or another clearly documented non-success outcome.
+- The final summary must make cancellation visible.
+
+## No Silent Fallback
+
+Batch processors must not invent default inputs, substitute fallback targets, or claim success when a fallback or partial result occurred.
diff --git a/tools/templates-v2/docs/CONTROL_SERVICE_CONTRACTS.md b/tools/templates-v2/docs/CONTROL_SERVICE_CONTRACTS.md
@@ -135,15 +135,18 @@ Recommended logger methods:
 
 Batch processors handle repeated work across discovered inputs.
 
+See `BATCH_GUARDRAIL_CONTRACT.md` for the full batch guardrail contract.
+
 Required rules:
 
 - Process real discovered inputs only.
 - Never assume numeric sequences.
-- Log every item through the logger.
+- Log every item through the logger with `OK`, `WARN`, `FAIL`, or `SKIP`.
 - One item failure must not stop the batch unless the failure is global.
 - Missing inputs are `SKIP` during batch processing.
 - Batch failures identify the exact item and underlying error.
 - Batch summaries include written, failed, skipped, and warnings.
+- Long-running batches support stop or cancel when applicable.
 
 Recommended summary shape:
 
