Fix Collision Inspector V2 template layout and require manifest screen dimensions - PR_26139_006-collision-inspector-template-layout-fixes

Author: DavidQ

docs/dev/reports/PR_26139_006-collision-inspector-template-layout-fixes_report.md

@@ -0,0 +1,65 @@
+# PR_26139_006-collision-inspector-template-layout-fixes Report
+
+## Summary
+
+Updated Collision Inspector V2 to follow the Tool Starter header/menu structure, tightened the layout, and removed hidden screen-dimension defaults. The inspector now hard fails with visible/actionable messaging when a manifest is missing `root.screen.width` or `root.screen.height`.
+
+Playwright impacted: Yes.
+
+## Scope Completed
+
+- Replaced Collision Inspector-specific header/menu markup with Tool Starter-compatible structure:
+  - `tool-starter__header`
+  - `tool-starter__menu tool-starter__tool__menu`
+  - `tool-starter__menu tool-starter__workspace__menu`
+- Kept shared-tool and Workspace Manager launched manifest behavior.
+- Removed the Collision Inspector canvas screen-size default path before manifest acceptance.
+- Removed `DEFAULT_SCREEN_DIMENSIONS` from Asteroids scene setup.
+- Made missing manifest screen dimensions a hard failure with:
+  - visible manifest summary error
+  - `Manifest Error` result badge
+  - actionable collision summary JSON
+  - failure log entry
+  - no fallback render
+- Moved `collisionSummary` into its own accordion.
+- Limited the summary accordion body to available space and kept it scrollable.
+- Kept the canvas below the Zoom control and preserved manifest-driven aspect ratio.
+
+## Guardrails
+
+- Collision Inspector V2 still uses the shared engine collision path.
+- No hardcoded Asteroids geometry was added.
+- No fallback/default vector maps were added.
+- Workspace-launched manifest loading remains automatic.
+- Existing intentional ship flame flicker and asteroid scale tuning were not changed.
+
+## Validation
+
+PASS:
+
+- `npm run build:manifest`
+  - Passed. This repo does not define a plain `npm run build` script.
+- `node --check` on touched Collision Inspector V2 modules, touched Asteroids scene module, and touched tests.
+- `npx playwright test tests/playwright/tools/CollisionInspectorV2.spec.mjs --project=playwright --workers=1 --reporter=list`
+  - 3 passed.
+  - Validated template header/menu structure, canvas below Zoom, aspect ratio, summary accordion placement/scrolling, missing screen-dimension hard failure, and workspace-launched manifest behavior.
+- `node -e "import('./tests/games/AsteroidsManifestScreenDimensions.test.mjs').then(({ run }) => run())"`
+  - Passed. Confirmed manifest screen dimensions reach runtime/scene and missing dimensions fail before engine creation.
+- `npx playwright test tests/playwright/tools/WorkspaceManagerV2.spec.mjs --project=playwright --workers=1 --reporter=list --grep "uses header lifecycle controls and launches tools from fixed Workspace Manager V2 tiles"`
+  - 1 passed.
+- `git diff --check`
+  - Passed with line-ending warnings only.
+- Advisory Playwright V8 coverage report was regenerated by the Collision Inspector V2 Playwright run.
+
+Skipped by request:
+
+- Full regression was not run.
+- Full samples smoke test was not run.
+
+## Manual Validation
+
+1. Open `tools/collision-inspector-v2/index.html?manifestPath=/games/Asteroids/game.manifest.json`.
+2. Confirm the Tool Starter-style header/menu appears, the Game Manifest JSON picker is in the tool menu, and the canvas sits below Zoom.
+3. Confirm Collision Summary is its own scrollable accordion on the right.
+4. Launch Collision Inspector V2 from Workspace Manager V2 and confirm it auto-loads the workspace manifest and shows the workspace return menu.
+5. Try a manifest without `screen.width` and `screen.height`; expected result is a visible `Manifest Error`, no fallback geometry render, and a log entry naming the missing root fields.

games/Asteroids/game/AsteroidsGameScene.js

@@ -32,7 +32,6 @@ const SCORE_TWO_X = 824;
 const LIFE_SPACING = 22;
 const PAUSE_OVERLAY_COLOR = 'rgba(2, 6, 23, 0.58)';
 const INITIALS_OVERLAY_COLOR = 'rgba(1, 6, 19, 0.62)';
-const DEFAULT_SCREEN_DIMENSIONS = Object.freeze({ width: 960, height: 720 });
 const ATTRACT_INPUT_CODES = [
   'Digit1',
   'Digit2',
@@ -61,16 +60,18 @@ function logSceneBootStage(stage, details = null) {
   }
 }
 
-function positiveInteger(value, fallback) {
+function positiveInteger(value) {
   const parsed = Math.floor(Number(value));
-  return Number.isNaN(parsed) || Math.abs(parsed) === Infinity || parsed <= 0 ? fallback : parsed;
+  return Number.isNaN(parsed) || Math.abs(parsed) === Infinity || parsed <= 0 ? 0 : parsed;
 }
 
 function screenDimensionsFromOptions(options) {
-  return {
-    width: positiveInteger(options?.screenDimensions?.width, DEFAULT_SCREEN_DIMENSIONS.width),
-    height: positiveInteger(options?.screenDimensions?.height, DEFAULT_SCREEN_DIMENSIONS.height),
-  };
+  const width = positiveInteger(options?.screenDimensions?.width);
+  const height = positiveInteger(options?.screenDimensions?.height);
+  if (!width || !height) {
+    throw new Error('AsteroidsGameScene requires screenDimensions.width and screenDimensions.height from game.manifest.json.');
+  }
+  return { width, height };
 }
 
 function getBeatInterval(asteroidCount) {

tests/games/AsteroidsManifestScreenDimensions.test.mjs

@@ -69,4 +69,30 @@ export async function run() {
   assert.equal(engineOptions.width, manifestPayload.screen.width);
   assert.equal(engineOptions.height, manifestPayload.screen.height);
   assert.deepEqual(sceneOptions.screenDimensions, manifestPayload.screen);
+
+  const missingScreenManifest = { ...manifestPayload };
+  delete missingScreenManifest.screen;
+  let invalidEngineCreated = false;
+  await assert.rejects(
+    () => bootAsteroids({
+      documentRef: {
+        body: { style: {} },
+        documentElement: { style: {} },
+        getElementById(id) {
+          return id === 'game' ? createCanvas() : null;
+        },
+      },
+      EngineClass: class {
+        constructor() {
+          invalidEngineCreated = true;
+        }
+      },
+      InputServiceClass: class {},
+      ObjectVectorRuntimeClass: class {},
+      SceneClass: class {},
+      manifestPayload: missingScreenManifest,
+    }),
+    /requires screen\.width and screen\.height/
+  );
+  assert.equal(invalidEngineCreated, false);
 }

tests/playwright/tools/CollisionInspectorV2.spec.mjs

@@ -4,6 +4,10 @@ import { join } from "node:path";
 import { startRepoServer } from "../../helpers/playwrightRepoServer.mjs";
 import { workspaceV2CoverageReporter as coverageReporter } from "../../helpers/workspaceV2CoverageReporter.mjs";
 
+test.afterAll(async () => {
+  await coverageReporter.writeReport();
+});
+
 async function dragCanvasPoint(page, from, to) {
   await page.locator("#collisionCanvas").evaluate((canvas, points) => {
     const rect = canvas.getBoundingClientRect();
@@ -39,6 +43,9 @@ test.describe("Collision Inspector V2", () => {
     try {
       await page.goto(`${server.baseUrl}/tools/collision-inspector-v2/index.html?manifestPath=/games/Asteroids/game.manifest.json`, { waitUntil: "networkidle" });
       await expect(page.locator("body.tools-platform-tool-page[data-tool-id='collision-inspector-v2']")).toBeVisible();
+      await expect(page.locator(".tool-starter__header[data-tool-starter-header]")).toBeVisible();
+      await expect(page.locator("nav.tool-starter__menu.tool-starter__tool__menu")).toBeVisible();
+      await expect(page.locator("nav.tool-starter__menu.tool-starter__workspace__menu")).toBeHidden();
       await expect(page.locator("#collisionModeSelect option")).toHaveText(["Bounds", "Vector", "Pixel/Sprite", "Hybrid"]);
       await expect(page.locator("#loadAsteroidsManifestButton")).toHaveCount(0);
       await expect(page.locator("#collisionManifestInput")).toBeVisible();
@@ -53,6 +60,18 @@ test.describe("Collision Inspector V2", () => {
         return rect.width / rect.height;
       });
       expect(Math.abs(aspectRatio - (960 / 720))).toBeLessThan(0.02);
+      const canvasLayout = await page.evaluate(() => {
+        const zoom = document.querySelector(".collision-inspector-v2__zoom-row").getBoundingClientRect();
+        const canvas = document.querySelector("#collisionCanvas").getBoundingClientRect();
+        return {
+          canvasBelowZoom: canvas.top >= zoom.bottom,
+          canvasHeight: canvas.height,
+          canvasWidth: canvas.width
+        };
+      });
+      expect(canvasLayout.canvasBelowZoom).toBe(true);
+      expect(canvasLayout.canvasWidth).toBeGreaterThan(100);
+      expect(canvasLayout.canvasHeight).toBeGreaterThan(100);
       await expect(page.locator("#objectASelect")).toContainText("Asteroids Ship");
       await expect(page.locator("#objectBSelect")).toContainText("Large Asteroid");
       await page.locator("#objectASelect").selectOption("object.asteroids.ship");
@@ -66,6 +85,10 @@ test.describe("Collision Inspector V2", () => {
       await expect(page.locator("#collisionSummary")).toContainText('"objectOrigins"');
       await expect(page.locator("#collisionSummary")).toContainText('"recommendedMode": "vector"');
       await expect(page.locator("#collisionSummary")).toContainText('"transformedPoints"');
+      await expect(page.locator("#resultContent #collisionSummary")).toHaveCount(0);
+      await expect(page.locator("#collisionSummaryContent #collisionSummary")).toBeVisible();
+      const summaryOverflow = await page.locator("#collisionSummaryContent").evaluate((element) => getComputedStyle(element).overflowY);
+      expect(["auto", "scroll"]).toContain(summaryOverflow);
 
       await page.locator("#objectBRotationInput").fill("180");
       await expect(page.locator("#rotationState")).toHaveText("A 0 / B 180");
@@ -128,6 +151,47 @@ test.describe("Collision Inspector V2", () => {
     }
   });
 
+  test("hard fails when manifest screen dimensions are missing", async ({ page }) => {
+    const server = await startRepoServer();
+    const pageErrors = [];
+    page.on("pageerror", (error) => {
+      pageErrors.push(error.message);
+    });
+
+    await coverageReporter.start(page);
+    try {
+      await page.route((url) => url.pathname === "/missing-screen-manifest.json", async (route) => {
+        await route.fulfill({
+          status: 200,
+          contentType: "application/json",
+          body: JSON.stringify({
+            game: { name: "Missing Screen Dimensions" },
+            tools: {
+              "object-vector-studio-v2": {
+                objects: []
+              }
+            }
+          })
+        });
+      });
+      await page.goto(`${server.baseUrl}/tools/collision-inspector-v2/index.html?manifestPath=/missing-screen-manifest.json`, { waitUntil: "networkidle" });
+
+      await expect(page.locator("#manifestSummary")).toContainText("root.screen.width");
+      await expect(page.locator("#manifestSummary")).toContainText("root.screen.height");
+      await expect(page.locator("#collisionResultBadge")).toHaveText("Manifest Error");
+      await expect(page.locator("#collisionSummary")).toContainText('"required": "root.screen.width and root.screen.height"');
+      await expect(page.locator("#collisionLog")).toHaveValue(/FAIL URL manifest path/);
+      await expect(page.locator("#objectASelect option")).toHaveCount(0);
+      await expect(page.locator("#objectBSelect option")).toHaveCount(0);
+      await expect(page.locator("#collisionCanvas")).toHaveAttribute("width", "1");
+      await expect(page.locator("#collisionCanvas")).toHaveAttribute("height", "1");
+      expect(pageErrors).toEqual([]);
+    } finally {
+      await coverageReporter.stop(page);
+      await server.close();
+    }
+  });
+
   test("loads Asteroids Object Vector objects from a Workspace Manager V2 manifest context", async ({ page }) => {
     const server = await startRepoServer();
     const pageErrors = [];
@@ -157,8 +221,8 @@ test.describe("Collision Inspector V2", () => {
       }, workspaceContext);
       await page.goto(`${server.baseUrl}/tools/collision-inspector-v2/index.html?launch=workspace&fromTool=workspace-manager-v2&hostContextId=workspace-manager-v2-collision-test`, { waitUntil: "networkidle" });
 
-      await expect(page.locator('[data-launch-mode-nav="workspace"]')).toBeVisible();
-      await expect(page.locator('[data-launch-mode-nav="tool"]')).toBeHidden();
+      await expect(page.locator("nav.tool-starter__workspace__menu[data-launch-mode-nav='workspace']")).toBeVisible();
+      await expect(page.locator("nav.tool-starter__tool__menu[data-launch-mode-nav='tool']")).toBeHidden();
       await expect(page.locator("#collisionManifestInput")).toBeDisabled();
       await expect(page.locator("#manifestSummary")).toContainText("Asteroids Workspace Manager V2 Context");
       await expect(page.locator("#manifestSummary")).toContainText("screen 960x720");

tools/collision-inspector-v2/index.html

@@ -12,11 +12,11 @@
   </head>
   <body class="tools-platform-tool-page tools-platform-surface hub-page-tools" data-tool-id="collision-inspector-v2">
     <details class="is-collapsible" open>
-      <summary class="is-collapsible__summary" data-collision-inspector-summary>Hide Header and Details</summary>
+      <summary class="is-collapsible__summary" data-tool-starter-summary>Hide Header and Details</summary>
       <div class="is-collapsible__content">
         <div id="shared-theme-header"></div>
-        <div class="collision-inspector-v2__header" data-collision-inspector-header>
-          <section class="tools-platform-frame collision-inspector-v2__local-shell-frame">
+        <div class="tool-starter__header" data-tool-starter-header="">
+          <section class="tools-platform-frame tool-starter__local-shell-frame collision-inspector-v2__local-shell-frame">
             <div class="tools-platform-frame__accordion-content">
               <div class="tools-platform-frame__accordion-summary">
                 <div class="tools-platform-frame__summary-copy">
@@ -29,22 +29,24 @@ <h2 class="tools-platform-frame__eyebrow">Shared Manifest Collision Tool</h2>
               </div>
               <div class="tools-platform-frame__topline">
                 <p class="tools-platform-frame__description">Load a game manifest, select two Object Vector V2 objects, and drag them through live collision checks.</p>
-                <div class="collision-inspector-v2__header-actions">
-                  <label class="collision-inspector-v2__field collision-inspector-v2__header-file" for="collisionManifestInput" data-launch-mode-nav="tool">
-                    <span>Game Manifest JSON</span>
-                    <input id="collisionManifestInput" type="file" accept=".json,application/json">
-                  </label>
-                  <nav class="collision-inspector-v2__workspace-menu" aria-label="Workspace actions" data-launch-mode-nav="workspace" hidden>
-                    <button id="returnToWorkspaceButton" type="button">Return to Workspace</button>
-                  </nav>
-                </div>
               </div>
             </div>
           </section>
         </div>
       </div>
     </details>
 
+    <nav class="tool-starter__menu tool-starter__tool__menu" aria-label="Tool actions" data-launch-mode-nav="tool">
+      <label class="collision-inspector-v2__field collision-inspector-v2__menu-file" for="collisionManifestInput">
+        <span>Game Manifest JSON</span>
+        <input id="collisionManifestInput" type="file" accept=".json,application/json">
+      </label>
+    </nav>
+
+    <nav class="tool-starter__menu tool-starter__workspace__menu" aria-label="Workspace actions" data-launch-mode-nav="workspace" hidden>
+      <button id="returnToWorkspaceButton" type="button">Return to Workspace</button>
+    </nav>
+
     <main class="collision-inspector-v2 app-shell" data-tool-id="collision-inspector-v2">
       <aside class="collision-inspector-v2__panel collision-inspector-v2__panel--left" aria-label="Collision inputs">
         <section class="accordion-v2 collision-inspector-v2__accordion is-open" data-accordion-v2-open="true">
@@ -109,7 +111,9 @@ <h2 class="tools-platform-frame__eyebrow">Shared Manifest Collision Tool</h2>
               </label>
               <output id="zoomState" for="collisionZoomInput">1x</output>
             </div>
-            <canvas id="collisionCanvas" width="960" height="640" aria-label="Collision movement simulation"></canvas>
+            <div class="collision-inspector-v2__canvas-viewport">
+              <canvas id="collisionCanvas" width="1" height="1" aria-label="Collision movement simulation"></canvas>
+            </div>
             <div class="collision-inspector-v2__legend" aria-label="Collision overlay legend">
               <span>Object A</span>
               <span>Object B</span>
@@ -153,11 +157,20 @@ <h2 class="tools-platform-frame__eyebrow">Shared Manifest Collision Tool</h2>
                 <dd id="pointsState">waiting</dd>
               </div>
             </dl>
+          </div>
+        </section>
+
+        <section class="accordion-v2 collision-inspector-v2__accordion collision-inspector-v2__accordion--summary is-open" data-accordion-v2-open="true">
+          <button class="accordion-v2__header" type="button" aria-expanded="true" aria-controls="collisionSummaryContent">
+            <span>Collision Summary</span>
+            <span class="accordion-v2__icon" aria-hidden="true">+</span>
+          </button>
+          <div id="collisionSummaryContent" class="accordion-v2__content collision-inspector-v2__summary-content">
             <pre id="collisionSummary" class="collision-inspector-v2__output">{}</pre>
           </div>
         </section>
 
-        <section class="accordion-v2 collision-inspector-v2__accordion is-open" data-accordion-v2-open="true">
+        <section class="accordion-v2 collision-inspector-v2__accordion collision-inspector-v2__accordion--logs is-open" data-accordion-v2-open="true">
           <div class="accordion-v2__header collision-inspector-v2__status-header" aria-expanded="true" aria-controls="collisionLogContent">
             <span>Collision Logs</span>
             <div class="collision-inspector-v2__status-actions">