Skip to content

docs: WhatStack launch film (landscape + vertical) - #3

Merged
deepu0 merged 1 commit into
mainfrom
launch-video
Sep 26, 2026
Merged

deepu0 merged 1 commit into
mainfrom
launch-video

Conversation

@kiro-agent

@kiro-agent kiro-agent Bot commented Sep 26, 2026

Copy link
Copy Markdown

Adds launch-video/: the 32s, 60fps launch film in 16:9 and 9:16, poster frames, GIF previews and the render source (Skia engine, beat timeline, capture script, generated score). Documentation only — not included in the store package (npm run pack ships only manifest, background, content, popup, shared and icons).

@xhawk-ai xhawk-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📋 Review Summary — Not ready to merge

2 medium findings would be worth fixing before merging.

Findings

  1. Medium Correctness Capture output is written where the engine never reads it ▶
  2. Medium Correctness The render scripts are CommonJS files inside an ESM package ▶
Fix with agent prompt
These are the findings from a code review of this pull request.

## 1. The render scripts are CommonJS files inside an ESM package
Path: launch-video/source/engine.js
Line: 4

Issue: The repository declares `"type": "module"` in `package.json`, so Node treats these `.js` files as ES modules. Running `node launch-video/source/render.js ...` aborts before rendering with `ReferenceError: require is not defined`; the same module-format mismatch exists in `render.js` and `stills.js`, so the added render/still source cannot be executed as written.
Suggested fix:
- Rename the CommonJS render files to `.cjs` and update their `require('./engine')` imports accordingly.
- Convert `engine.js`, `render.js`, and `stills.js` to ESM `import`/`export` syntax.

## 2. Capture output is written where the engine never reads it
Path: launch-video/source/capture.mjs
Line: 6

Issue: The capture script writes generated screenshots and `meta.json` under `/projects/sandbox/whatstack-video/cap/`, while the engine reads captures from `launch-video/source/cap/`. In a clean checkout the committed `cap/` directory only contains `meta.json`, so running capture and then render still leaves `page-*.jpg`, `pop-*.png`, `store.jpg`, and `src-stripe.html` missing from the path consumed by `E.load()`.
Suggested fix:
- Derive `OUT` from the checked-in source directory, for example `new URL('./cap/', import.meta.url)`, so captures are generated into the same directory that `engine.js` loads.

---

For each finding above, determine whether it is valid and should be fixed. If so, fix it directly. Where a finding offers several remedies, pick one and say why. Leave the pull request's own changed files alone unless a fix requires touching them.

Summary

The change adds a new launch-video/ documentation bundle with rendered launch-film assets and the code/timeline used to produce them. The packaged extension remains unaffected, but the checked-in render source is not reproducible in a clean checkout: the Node render scripts use CommonJS in an ESM package, and the capture script writes its generated assets to a different hard-coded sandbox path than the engine reads.

Commands

Re-review the latest changes:

@xhawk-ai review again

Resolve all review threads and post a summary:

@xhawk-ai resolve all

// WhatStack launch film — beat-timed animation engine on Skia (skia-canvas).
// Every element is a pure function of b (time in beats). timeline.json drives picture and score.
// FMT=vertical renders 1080×1920; default 1920×1080.
const {Canvas,FontLibrary,loadImage,Path2D}=require('/projects/sandbox/tools/node_modules/skia-canvas');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Correctness
The render scripts are CommonJS files inside an ESM package

The repository declares "type": "module" in package.json, so Node treats these .js files as ES modules. Running node launch-video/source/render.js ... aborts before rendering with ReferenceError: require is not defined; the same module-format mismatch exists in render.js and stills.js, so the added render/still source cannot be executed as written.

Suggestions

  • Rename the CommonJS render files to .cjs and update their require('./engine') imports accordingly.
  • Convert engine.js, render.js, and stills.js to ESM import/export syntax.

// (a popup opened as a tab would otherwise scan itself). Clipboard writes are recorded so exports are the real output.
import {chromium} from '/projects/sandbox/tools/node_modules/playwright/index.mjs';
import fs from 'fs';import os from 'os';import path from 'path';
const EXT='/projects/sandbox/whatstack';const OUT='/projects/sandbox/whatstack-video/cap/';fs.mkdirSync(OUT,{recursive:true});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Correctness
Capture output is written where the engine never reads it

The capture script writes generated screenshots and meta.json under /projects/sandbox/whatstack-video/cap/, while the engine reads captures from launch-video/source/cap/. In a clean checkout the committed cap/ directory only contains meta.json, so running capture and then render still leaves page-*.jpg, pop-*.png, store.jpg, and src-stripe.html missing from the path consumed by E.load().

Suggestions

Derive OUT from the checked-in source directory, for example new URL('./cap/', import.meta.url), so captures are generated into the same directory that engine.js loads.

@deepu0
deepu0 merged commit f0838fd into main Sep 26, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants