Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
162 changes: 162 additions & 0 deletions docs/superpowers/plans/2026-07-26-render-tooling-reliability.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
# Render Tooling Reliability Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.

**Goal:** Make missing Playwright Chromium failures actionable and prevent canonical Z-up view labels from being mistaken for product-semantic exterior views.

**Architecture:** Keep the current Playwright renderer and camera transforms. Add a narrow launch-error classifier at the shared browser bootstrap, then clarify the MCP view descriptions. Verify the existing Rollup optional-dependency representation rather than changing it without evidence.

**Tech Stack:** TypeScript, Playwright, Vitest, npm lockfile, KernelCAD headless renderer.

---

### Task 1: Missing Chromium Diagnostic

**Files:**
- Create: `tests/unit/render/playwrightLaunchDiagnostic.test.ts`
- Modify: `src/agent/render/headlessRender.ts`

- [ ] **Step 1: Write failing classifier tests**

Add tests proving:

```typescript
expect(formatPlaywrightLaunchError(
new Error("browserType.launch: Executable doesn't exist at /cache/chrome\nPlease run: npx playwright install"),
)).toMatch(/npx playwright install chromium/);

const unrelated = new Error('browserType.launch: permission denied');
expect(formatPlaywrightLaunchError(unrelated)).toBe(unrelated);
```

- [ ] **Step 2: Run the focused test and confirm RED**

Run:

```bash
npx vitest run tests/unit/render/playwrightLaunchDiagnostic.test.ts
```

Expected: failure because `formatPlaywrightLaunchError` is not exported.

- [ ] **Step 3: Implement the narrow classifier**

Export a helper from `headlessRender.ts` that:

- returns unrelated errors unchanged;
- recognizes only Playwright’s missing-executable message;
- returns an `Error` containing `npx playwright install chromium`;
- preserves the original failure as `cause`.

Wrap only the fresh `chromium.launch(...)` call with this helper. Do not relabel CDP, navigation, timeout, or permission errors.

- [ ] **Step 4: Run the focused test and confirm GREEN**

Run:

```bash
npx vitest run tests/unit/render/playwrightLaunchDiagnostic.test.ts
```

Expected: all tests pass.

### Task 2: Canonical View Semantics

**Files:**
- Modify: `tests/unit/mcp/renderPreviewTool.test.ts`
- Modify: `src/agent/mcp/tools/renderPreview.ts`

- [ ] **Step 1: Add failing description assertions**

Assert that:

```typescript
expect(VIEW_DESCRIPTIONS.top).toMatch(/geometric \+Z/i);
expect(VIEW_DESCRIPTIONS.top).toMatch(/not necessarily.*exterior/i);
expect(VIEW_DESCRIPTIONS.iso).toMatch(/model orientation/i);
```

- [ ] **Step 2: Run the focused test and confirm RED**

Run:

```bash
npx vitest run tests/unit/mcp/renderPreviewTool.test.ts
```

Expected: the new semantic-label assertions fail against current descriptions.

- [ ] **Step 3: Clarify descriptions without changing camera math**

Update `VIEW_DESCRIPTIONS.top` and `.iso` to state that canonical views are geometric Z-up views and do not infer which side of an imported part is its product exterior.

- [ ] **Step 4: Run the focused test and confirm GREEN**

Run:

```bash
npx vitest run tests/unit/mcp/renderPreviewTool.test.ts
```

Expected: all tests pass.

### Task 3: Dependency and Merge Verification

**Files:**
- Verify: `package.json`
- Verify: `package-lock.json`

- [ ] **Step 1: Verify Rollup’s platform packages are already optional**

Run:

```bash
node -e "const l=require('./package-lock.json'); const r=l.packages['node_modules/rollup']; if(!r?.optionalDependencies?.['@rollup/rollup-darwin-arm64']) process.exit(1)"
```

Expected: exit 0. No package change is warranted because the lockfile already uses Rollup’s portable optional-dependency model; the observed failure followed an interrupted install.

- [ ] **Step 2: Run focused validation**

Run:

```bash
npx vitest run \
tests/unit/render/playwrightLaunchDiagnostic.test.ts \
tests/unit/mcp/renderPreviewTool.test.ts \
tests/unit/cli/renderCommand.test.ts
npm run typecheck
npm run build:cli
npm run build:player
```

Expected: all commands pass.

- [ ] **Step 3: Review the branch diff**

Run:

```bash
git diff --check
git status --short
```

Expected: only the approved spec, plan, focused tests, and two focused source files are changed.

- [ ] **Step 4: Commit the implementation**

Run:

```bash
git add \
src/agent/render/headlessRender.ts \
src/agent/mcp/tools/renderPreview.ts \
tests/unit/render/playwrightLaunchDiagnostic.test.ts \
tests/unit/mcp/renderPreviewTool.test.ts \
docs/superpowers/plans/2026-07-26-render-tooling-reliability.md
git commit -m "fix: improve render tooling diagnostics"
```

- [ ] **Step 5: Merge into develop**

From the repository’s `develop` branch, merge `fix/render-tooling-reliability` with a non-fast-forward merge, rerun the focused tests, and report the resulting merge commit.
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Render Tooling Reliability Design

## Objective

Make KernelCAD’s existing render workflow work predictably after installation and fail with precise recovery guidance when Playwright Chromium is absent. Correct misleading canonical preview orientation or labeling so users can distinguish an exterior top view from an open underside.

## Scope

This change is intentionally limited to:

1. Correct installation of the platform-specific Rollup native package required by the existing player build.
2. An actionable `npx playwright install chromium` diagnostic when the render browser executable is missing.
3. Correct canonical preview camera orientation and descriptions for KernelCAD’s Z-up coordinate system.
4. Focused automated regression coverage.

No new setup command, browser bundling, renderer architecture, CAD geometry behavior, STEP semantics, or embossing behavior is included.

## Installation Reliability

Keep the current npm-based installation and build workflow. Ensure the lockfile and dependency declarations allow npm to install the appropriate Rollup native package for the current platform without requiring users to delete `node_modules` or the lockfile.

The fix must remain portable: do not hard-code the macOS ARM package as an unconditional production dependency for other platforms. Use the package manager’s supported optional-platform dependency mechanism or the Rollup version’s established dependency declaration.

## Missing-Browser Diagnostic

When Playwright fails because its Chromium or headless-shell executable is absent, KernelCAD must replace the raw Playwright banner/stack with a concise KernelCAD diagnostic that includes:

```text
npx playwright install chromium
```

The diagnostic must preserve the underlying cause for debugging and must not claim that Studio, a development server, or a full reinstall is required.

Other Playwright failures must retain their existing behavior and must not be mislabeled as a missing-browser installation problem.

## Canonical Preview Orientation

KernelCAD is Z-up. Canonical render names and descriptions must agree with the actual camera:

- `top` views the model from positive Z toward the origin.
- Exterior/underside interpretation must not be guessed from bounding boxes.
- `iso` must use the documented elevation convention consistently.
- Arbitrary pose descriptions must state that positive elevation lifts the camera toward positive Z.

If the current camera is correct but the label is wrong, fix the label. If the label is correct but the camera transform is inverted, fix the transform. The regression test must establish which component was wrong rather than changing both blindly.

## Testing

Add focused tests that prove:

1. The renderer maps only Playwright’s missing-executable failure to the new actionable diagnostic.
2. Unrelated launch errors remain distinguishable.
3. Canonical `top` and `iso` camera vectors/descriptions obey the documented Z-up convention.
4. The package dependency/lockfile representation includes the correct platform-optional Rollup packages through npm’s normal installation model.

Run the smallest relevant unit/integration suites, then the repository’s standard validation required for merging.

## Completion Criteria

- A normal dependency installation no longer leaves the existing player build missing its Rollup native package on supported platforms.
- A missing Playwright browser produces the exact recovery command without a misleading renderer diagnosis.
- Canonical camera tests prove that `top` looks from positive Z and `iso` follows the documented elevation convention.
- Existing renderer behavior remains unchanged outside these cases.
- The fixes are reviewed, committed, merged, and verified on the target branch.
4 changes: 2 additions & 2 deletions src/agent/mcp/tools/renderPreview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,9 @@ export const VIEW_DESCRIPTIONS: Record<RenderView, string> = {
right:
'Right side elevation — camera on +X; screen right = +Y, screen up = +Z.',
top:
'Top-down plan — camera on +Z looking down; screen right = +X, screen up = +Y.',
'Geometric +Z top-down plan — camera on +Z looking down; screen right = +X, screen up = +Y. For imported parts, this is not necessarily the product exterior.',
iso:
'Three-quarter overview — camera above the front-right (+X, -Y, +Z octant); best single view for overall shape.',
'Geometric Z-up three-quarter overview — camera in the (+X, -Y, +Z) octant. Product exterior versus underside depends on model orientation.',
};

function poseDescription(az: number, el: number): string {
Expand Down
23 changes: 22 additions & 1 deletion src/agent/render/headlessRender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,23 @@ export interface DemoPlayerPageHandle {
close: () => Promise<void>;
}

/**
* Replace Playwright's verbose missing-browser banner with the exact
* one-command recovery path KernelCAD users need. Keep every unrelated launch
* failure unchanged so permission, sandbox, and CDP errors remain diagnosable.
*/
export function formatPlaywrightLaunchError<T>(error: T): T | Error {
if (!(error instanceof Error)) return error;
const missingExecutable =
error.message.includes("Executable doesn't exist at") &&
error.message.includes('playwright install');
if (!missingExecutable) return error;
return new Error(
'KernelCAD rendering requires Playwright Chromium. Install it with: npx playwright install chromium',
{ cause: error },
);
}

/**
* Shared demo-player browser bootstrap: lazy playwright import, optional
* CDP attach with launch fallback, context + page, `/demo-player?headless=1`
Expand Down Expand Up @@ -241,7 +258,11 @@ export async function openDemoPlayerPage(opts: DemoPlayerPageOpts): Promise<Demo
}
}
if (!browser) {
browser = await chromium.launch({ args: ['--disable-dev-shm-usage'] });
try {
browser = await chromium.launch({ args: ['--disable-dev-shm-usage'] });
} catch (e) {
throw formatPlaywrightLaunchError(e);
}
context = await browser.newContext({ viewport: opts.viewport });
}
page = await context!.newPage();
Expand Down
6 changes: 6 additions & 0 deletions tests/unit/mcp/renderPreviewTool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ describe('render_preview — input validation', () => {
});

describe('render_preview — happy path (mocked render)', () => {
it('describes canonical views as geometric directions, not product-semantic exterior views', () => {
expect(VIEW_DESCRIPTIONS.top).toMatch(/geometric \+Z/i);
expect(VIEW_DESCRIPTIONS.top).toMatch(/not necessarily.*exterior/i);
expect(VIEW_DESCRIPTIONS.iso).toMatch(/model orientation/i);
});

it('writes one PNG per view + pose, with descriptions and metadata', async () => {
const closed = { value: false };
const r = await renderPreviewTool(
Expand Down
28 changes: 28 additions & 0 deletions tests/unit/render/playwrightLaunchDiagnostic.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { describe, expect, it } from 'vitest';
import { formatPlaywrightLaunchError } from '../../../src/agent/render/headlessRender';

describe('formatPlaywrightLaunchError', () => {
it('maps Playwright missing-executable failures to the exact Chromium recovery command', () => {
const cause = new Error(
"browserType.launch: Executable doesn't exist at /cache/chrome-headless-shell\n" +
'Please run the following command to download new browsers:\n' +
' npx playwright install',
);

const result = formatPlaywrightLaunchError(cause);

expect(result).not.toBe(cause);
expect(result.message).toContain('npx playwright install chromium');
expect(result.cause).toBe(cause);
});

it('leaves unrelated launch failures unchanged', () => {
const cause = new Error('browserType.launch: permission denied');

expect(formatPlaywrightLaunchError(cause)).toBe(cause);
});

it('leaves non-Error values unchanged', () => {
expect(formatPlaywrightLaunchError('launch failed')).toBe('launch failed');
});
});
Loading