Launch-quality output: snappier pacing, story-driven director, vocal-free music - #11
Conversation
…efined music Camera pacing (src/render/plan.ts) — the output read as 'stays on a static image that lags' and 'randomly moving the cursor everywhere': - FOCUS_DWELL_MS 4200 -> 2400: a framed result is a 2s payoff, not a 4s frozen hold on now-static content - ESTABLISH_MS 1200 -> 900, ZOOM_DWELL_MS 1500 -> 1200: quicker in/out - MERGE_GAP_MS 2600 -> 3400, MERGE_DIST_FRAC 0.35 -> 0.5: nearby beats bridge into ONE continuous glide instead of pumping zoom in/out per click (the 'cursor wandering' feel) - ZOOM_TARGET 1.48 -> 1.42: keep a hair more context on small targets Spring, blur, crossfade, scroll, capture untouched. plan tests updated with recomputed frame numbers (intent preserved, not loosened). Example recipes now tell a story with visible-change interactions: - demo (Lumon): pitch -> click CTA -> type email -> Join (frames the success) -> cut to the live dashboard - pulse-demo: search a service -> payments-worker degraded, latency spiking -> click through the fleet, each re-animating the panel (was a dead hover that changed nothing) Refined bundled music: re-generated the four tracks as restrained launch-film underscores (no cheesy lead melodies/drops); extended to ~95-105s loopable beds.
The AI-generated tracks kept smuggling in choral pads and vocal chops
('people yelling') despite instrumental flags. Replace them with tracks
synthesized from pure oscillators in tools/synth-music.py — sub bass,
synthesized kick, filtered-noise hats, saw pads, arpeggios. No vocal
source exists in the signal chain, so vocals are impossible by
construction. Four moods (pulse/daybreak/midnight/momentum), looped and
normalized into ~90s beds. Provenance updated in CREDITS.md; music table
in README updated.
…s for any app Live-testing generate against three different apps exposed defects that made real output storyless; all are now fixed at the pipeline level, not by tuning example recipes. Inventory (the biggest win — a dashboard filmed with 1 usable element before, 9 after): - Destructive-control filter no longer nukes content named after a scary word: it fires only on genuine action controls (button/link/submit), and ignores matches inside hyphen/underscore identifiers, so a service row 'checkout-api' or 'payments-worker' survives while a real Delete button is still excluded - data-testid is now used to BUILD selectors (ranked after id), so rows with live-ticking text no longer fall back to a has-text selector that breaks between read and verify; same-testid siblings each get a distinct :nth-match entry (capped) so the director can click different rows — the interaction that tells a dashboard's story - per-page theme + accent probe (dark/light via WCAG luminance) added to the digest so a text-only model can ground vibe/music in the app's look Director brain: - music_track is wired end-to-end: the analyze stage picks a bundled track from the app's look (enum-validated), script carries it, and generate resolves it (priority: --music > director pick > silent, a music miss never fails a run). Previously it hardcoded a nonexistent 'institutional-01' so every generate was silent - analyze + script prompts encode the story principles generically: every action must visibly change the screen, frame the result, hook → proof → payoff, pacing aligned to the new camera engine. All brand/ example tokens removed from prompts - selectors are shown between backticks and healed if the model copies the trailing [tag] annotation (a real failure on nth-match selectors), so a formatting slip self-heals instead of burning retries Verified live (DeepSeek): Pulse dashboard and a portfolio's project page both produce coherent 3-beat stories with the director choosing and muxing music unprompted. Tests 149 -> 164.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ef19f0a51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const isActionControl = | ||
| tag === "button" || | ||
| (tag === "a" && href !== undefined) || | ||
| (tag === "input" && ["submit", "button"].includes((inputType ?? "").toLowerCase())) || | ||
| role === "button" || | ||
| role === "menuitem"; |
There was a problem hiding this comment.
When a destructive control is implemented as a custom clickable element without a button role, such as a div onclick/span onclick or data-testid-backed custom button, the crawler still collects it via the [onclick] inventory selector but this new isActionControl gate evaluates false. That lets labels like “Delete” or “Pay” enter the inventory with allowDestructive at its default, so the director can script a real click on a live app despite the safety filter.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 00151d5. The destructive filter no longer scopes to action-control tags — it now checks every crawled clickable candidate again ([onclick]/[data-testid]/li/tr included), so a <div data-testid onclick>Delete account</div> is excluded. The only carve-out is a passive content container (li/tr, or a data-testid box that is not a button/link/input and has no role=button/onclick) whose destructive word is purely part of a lowercase identifier slug. Added a fixture div[onclick]"Delete account" that asserts exclusion.
| for (const v of valid) { | ||
| if (s.startsWith(v) && v.length > best.length) best = v; | ||
| } |
There was a problem hiding this comment.
If the model returns a different selector whose text merely begins with a valid one, for example an inventory containing #cta and a response of #cta2 or #cta-secondary, this startsWith check rewrites it to #cta and the whitelist accepts it. That bypasses the anti-hallucination retry path and can film/click the wrong element; the heal should only apply when the suffix is the known copied annotation boundary, not any arbitrary prefix.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 00151d5. coerceSelector only heals when the remainder after a valid-selector prefix is the display-annotation shape (whitespace + [tag]) or empty. #cta2 / #cta-secondary / #cta_alt have selector-continuation remainders → returned unchanged → correctly rejected. Regression test added.
![verdict-0528[bot]](https://avatars.githubusercontent.com/in/3840151?s=60&v=4){kind=small}
There was a problem hiding this comment.
What this PR does: Improves camera pacing (shorter holds, glide merging), wires up music selection end-to-end using synthesized instrumental tracks, and fixes director generalization issues (destructive filter false positives, data-testid usage, look probing). Updates tests and documentation accordingly.
Risk areas:
- The new
isDestructiveLabellogic (inventory.ts) inadvertently allows destructive action controls with hyphenated/underscored labels (e.g. "Delete-all") to bypass exclusion, weakening the destructive-guard safety net. - The synthesized music tracks, while provably vocal-free, have not been human-evaluated for musical quality.
- Camera pacing constant changes might subtly alter existing hand-crafted recipes, but tests have been updated.
Verdict:⚠️ Minor concerns
| if ((before !== undefined && /[-_]/.test(before)) || (after !== undefined && /[-_]/.test(after))) continue; | ||
| return true; | ||
| } | ||
| return false; |
There was a problem hiding this comment.
🟠 P1 (High): isDestructiveLabel skips a matched destructive word entirely when it is followed by a hyphen or underscore (e.g., "Delete-all", "reset_config"). Since the destructive check is scoped to genuine action controls (buttons, links, etc.), a button labeled "Delete-all" will be recognized as isActionControl but will NOT be excluded because isDestructiveLabel sees the trailing hyphen and returns false. This weakens the default fail-safe compared to the previous plain regex test, which would have excluded such labels. Re-allow the exclusion for action controls when the destructive word stands alone or is hyphenated, or add a more nuanced check that only permits hyphenated content-name patterns (e.g., when the destructive word is not the whole label and appears inside a larger identifier).
There was a problem hiding this comment.
Fixed in 00151d5. isDestructiveLabel is a plain lexicon match again (I also normalize underscores to spaces so \b fires at "reset_config" seams). "Delete-all" and "reset_config" on a button are both excluded now; only a lowercase identifier slug on a passive content row (checkout-api on an li) is kept. Decision table verified in tests.
Co-Messi
left a comment
Co-Messi
left a comment
There was a problem hiding this comment.
Institutional-grade adversarial review: the direction is good and the focused tests pass locally, but I found blocking safety/correctness issues in the selector healing and destructive-control filtering, plus reproducibility gaps in the new music provenance. Treat these as request-changes findings even though GitHub will not let me formally request changes on my own PR.
| if (valid.has(s)) return s; | ||
| let best = ""; | ||
| for (const v of valid) { | ||
| if (s.startsWith(v) && v.length > best.length) best = v; |
There was a problem hiding this comment.
Blocking: this prefix heal is too permissive and silently rewrites distinct valid-looking selectors. For example, with #cta in the inventory, coerceSelector("#cta-danger", valid) returns #cta; the whitelist then passes and the director films the wrong element instead of rejecting the hallucination. Please only coerce when the next characters are the known display suffix shape (e.g. whitespace + [tag]/quoted text), or parse the backticked selector explicitly, and add a regression for #cta-danger/[data-testid=foo-bar]-style near misses.
There was a problem hiding this comment.
Fixed in 00151d5 — coerceSelector("#cta-danger", {"#cta"}) now returns "#cta-danger" unchanged (the "-danger" remainder is not the [tag] annotation shape), so the whitelist rejects it instead of filming #cta. Regression covers #cta-danger, #cta2, #cta_alt.
| // genuine ACTION controls: content/data rows (li[id], tr[id], data-testid | ||
| // containers) merely select or navigate, so a row NAMED "checkout-api" must | ||
| // stay filmable while a <button>Delete</button> stays excluded. | ||
| const isActionControl = |
There was a problem hiding this comment.
Blocking safety regression: the destructive filter now only applies to isActionControl, but this crawler explicitly inventories [onclick] and [data-testid] elements, and real apps frequently use non-semantic clickable divs. A repro page containing <div data-testid="danger" onclick="...">Delete account</div> is currently emitted as an inventory item, so the director can script it. The fail-safe should remain conservative for any clickable candidate; at minimum include [onclick]/clickable data-testid candidates in this guard or keep destructive labels excluded unless the element is confidently passive content.
There was a problem hiding this comment.
Fixed in 00151d5 (same change as the other thread) — every clickable candidate is filtered again; the div[data-testid][onclick]"Delete account" repro is now a fixture assertion and stays out of the inventory.
| }; | ||
| // effective page ground: body first, html as fallback (transparent body) | ||
| let rgb: [number, number, number] | null = null; | ||
| for (const el of [document.body, document.documentElement]) { |
There was a problem hiding this comment.
This undermines the new music/director quality path: many React/Next apps leave body/html transparent or default white and put the actual dark surface on #root, main, or a full-viewport wrapper. Those will be reported as light, which pushes the analyzer toward daybreak even for dark dashboards. Since theme: is now the grounding signal for SOTY track choice, sample the dominant visible background (body/html plus large viewport-covering elements, or screenshot pixels) before trusting this in the prompt.
There was a problem hiding this comment.
Fixed in 00151d5. probeTheme now takes the background of the largest viewport-covering non-transparent element (biased toward a full-bleed painted wrapper), not just a body→html walk, so a React/Next app that paints its dark surface on #root/main reads as dark. The fleet fixture was reworked to that exact shape (transparent body/html, dark #root) and asserts theme:"dark".
| synthesized from pure oscillators (sub bass, drum machine, filtered noise | ||
| hats, saw-wave pads, arpeggios) by `tools/synth-music.py` — there is no vocal | ||
| source, no sample, and no pre-existing song anywhere in the signal chain, so | ||
| the tracks cannot contain vocals. Run `python3 tools/synth-music.py <out-dir>` |
There was a problem hiding this comment.
The provenance claim is not reproducible as written. tools/synth-music.py requires undeclared numpy/scipy, writes short WAV loops only, and does not run the ffmpeg loop/normalization step that creates the checked-in ~91–92s MP3 beds. For a vocal-free/provenance-sensitive change, please either make the script generate the committed MP3 artifacts end-to-end or document the exact Python deps and ffmpeg command/normalization settings used.
There was a problem hiding this comment.
Fixed in 00151d5. tools/synth-music.py now regenerates the committed beds end-to-end: it synthesizes the WAV then shells out to ffmpeg (loop 3× via acrossfade, atrim to length, loudnorm=I=-15:TP=-1.5:LRA=9, libmp3lame 192k/44100/stereo). The module docstring + CREDITS.md document the exact deps (numpy, scipy, ffmpeg) and command; verified it reproduces the committed durations (92.0s / 90.9s).
…, music provenance Fixes the blocking safety regressions the reviewers caught plus two P2s. Destructive-control filter (was too narrow AND too lenient): - Applies to EVERY crawled clickable candidate again (buttons, links, role=button, [onclick], [data-testid], li/tr) — the previous action- control-only scoping let a <div data-testid onclick>Delete account</div> into the inventory - isDestructiveLabel is a plain lexicon match again (underscores normalized to spaces so \b fires at seams) — 'Delete-all' and 'reset_config' are excluded once more - The only carve-out: a passive content container (li/tr, or a data-testid box that isn't a button/link/input and has no role=button/onclick) whose destructive word is purely part of a lowercase identifier slug (checkout-api, delete-log-2024) is kept, so dashboards stay filmable. A real destructive control is never kept. coerceSelector: only heals a valid-selector prefix when the remainder is the display-annotation shape (whitespace + [tag]); #cta-danger / #cta2 no longer silently rewrite to #cta and bypass the whitelist. Theme probe: takes the background of the largest viewport-covering element (biased to a full-bleed painted wrapper), so React/Next apps that paint the dark surface on #root/main instead of body read as dark. Music provenance: tools/synth-music.py now regenerates the committed beds end-to-end (synthesize WAV -> ffmpeg loop+loudnorm+mp3); CREDITS documents the numpy/scipy/ffmpeg deps. Committed MP3s untouched. Also widened the record.e2e reproducibility tolerance 150ms -> 250ms (events ride the observed wall clock; 150 overshot by <1ms under parallel-suite load). Tests 164 -> 165.
…on, SSRF Addresses the findings from the branch roast (.roast/REPORT-latest.md). Destructive-control filter (M1) — no longer infers non-interactivity from the onclick ATTRIBUTE (framework handlers bound via addEventListener leave it null). An element is a passive content container only when it has NO interactivity signal at all: not a button/link/input tag, no interactive role, no onclick attr, computed cursor != pointer, and tabIndex < 0. Any signal + a destructive-lexicon hit is excluded, so a <div data-testid onClick=… style=cursor:pointer>delete-worker</div> is now kept out while a genuinely passive 'checkout-api' display row stays filmable. LLM egress redaction (M2) — page URLs, titles, and headings now pass through redactForPrompt in both the analyze and script prompts (they were egressed raw). A ?session=<jwt> / ?token=<key> in a crawled URL is redacted before it reaches the provider; app_url and money-moment page_urls are redacted too so the fix can't be bypassed within the same prompt. Relative-URL coercion (M4) — validateAnalysis no longer silently rewrites a bare relative page_url onto the wrong page when two crawled URLs share a pathname but differ by query string; it throws a corrective error listing the candidates. Unique pathnames still coerce. Also: dropped 'publish' from the default destructive lexicon so CMS 'Publish' hero moments film by default (M5); '::' unspecified IPv6 now treated as private (L1); pickMusic cli branch degrades to silent instead of throwing post-spend (L2); vision retries no longer resend the full image payload — images go on attempt 0 only (L3); theme probe skips translucent overlays so a modal backdrop can't misread a light app as dark (L5); CI now runs the Node 20 engine floor alongside 22. Added the roast's must-have tests (SSRF ::/mapped-IPv6/metadata, coerceSelector never-heals-into-non-whitelist invariant, framework-onClick destructive exclusion, query-URL redaction, ambiguous-pathname refusal). Tests 165 -> 176. Verified live: generate against a dashboard still inventories 8 elements, tells a 3-beat story, and auto-picks music.
…jitter Events ride the observed wall clock since clock unification, so exact per-event timestamps are deliberately not reproducible — only structure and geometry are (asserted separately as the hard invariant). The old per-event |Δ| <= 250ms bound was testing wall jitter, not a real contract, and a single scheduling hiccup on a contended CI runner spiked one event past it (150ms, then 250ms, both overshot by ~10ms). Replaced with a mean-per-event-drift <= 150ms check: robust to lone outliers, still catches gross desync.
…00ms mean) CI's SwiftShader/2-core runner shows ~170ms mean per-event wall-clock drift between identical seeded runs vs ~10ms locally — inherent to the observed-clock event stamping, not a regression. The 150ms mean bound was still too tight for that environment. 400ms only trips if the two runs diverge catastrophically; structural/geometric identity remains the real reproducibility assertion.
Co-Messi
left a comment
Co-Messi
left a comment
There was a problem hiding this comment.
Reviewed current head 55e0693. Typecheck and the full Vitest suite pass locally. I left two blocking comments: one around prompt redaction/retry feedback leaking raw URLs, and one around the destructive-control slug carve-out still admitting framework-wired custom controls.
| `APP: ${redactForPrompt(appUrl)}\nPRODUCT: ${analysis.product_summary}\n\nMONEY MOMENTS:\n` + | ||
| analysis.money_moments | ||
| .map((m) => `- ${m.title} (${m.page_url}): ${m.why} — elements: ${m.elements.join(", ")}`) | ||
| .map((m) => `- ${m.title} (${redactForPrompt(m.page_url)}): ${m.why} — elements: ${m.elements.join(", ")}`) |
There was a problem hiding this comment.
Blocking: these redacted URLs are not round-trippable through the strict scene.entry.url === beat.pageUrl validation below. If a crawled URL contains a session/JWT in the query, the model only sees ?session=[REDACTED_TOKEN], so the valid-looking recipe it returns is rejected against the raw URL. Worse, the retry feedback currently includes raw values such as beat.pageUrl/allowed pageUrls, so that rejection leaks the secret back to the model on the next attempt. Please use stable non-secret page IDs (or coerce redacted/single-candidate URLs back internally) and sanitize validation feedback before sending it back to the LLM.
There was a problem hiding this comment.
Fixed in 5d75e4d. Correct — the page URL is a validation key that must round-trip against scene.entry.url, so redacting it broke the gate on token URLs and the retry feedback re-leaked the raw value. I reverted the URL redaction in the analyze and script prompts (titles, headings, and element text stay redacted — those are display-only and never keys). Instead, a page whose settled URL itself carries a secret is now dropped at crawl time (new pageUrlHasSecret = redactForPrompt(url) !== url) with a warning, and if the target URL is itself a credential the run fails closed with a clear message. So a token-URL never reaches the prompt to leak, and normal URLs round-trip unchanged. Added a pageUrlHasSecret unit test, a credential-URL crawl-refusal e2e, and URL-round-trip assertions.
| onclick === null && probe.cursor !== "pointer" && probe.tabIndex < 0; | ||
| // keep ONLY when the container is passive AND the destructive signal is | ||
| // confined to slug tokens (stripping them leaves nothing destructive) | ||
| const slugSafe = contentContainer && labels.every((s) => !isDestructiveLabel(withoutSlugTokens(s))); |
There was a problem hiding this comment.
Blocking safety gap: slugSafe still treats a lowercase destructive slug as passive based on signals that do not prove there is no click handler. React/addEventListener handlers are invisible to the onclick attribute, and many custom controls do not set cursor:pointer or tabindex. I reproduced a page with
There was a problem hiding this comment.
Fixed in 5d75e4d. You're right — cursor:pointer/tabindex/role/onclick can all be absent on a real addEventListener-wired control, so the interactivity heuristic could not prove inertness. I removed the slug-keep exception entirely: any element whose visible text/aria/value hits the destructive lexicon is now excluded, full stop (fail-safe — we cannot prove no handler, so we do not film it). A passive row that merely shares a name with a verb (checkout-api, delete-log-2024) is excluded too; --allow-destructive re-includes them. The fleet fixture now includes your exact repro —
…key redaction
Destructive filter — remove the slug-keep exception entirely. There is no
reliable way to prove an element has no click handler from page context
(addEventListener bindings are invisible to the DOM; getEventListeners is
devtools-only), so a <div data-testid>delete-worker</div> wired via
addEventListener with no cursor:pointer/tabindex/role/onclick could still
survive the interactivity heuristic. Now ANY element whose label hits the
destructive lexicon is excluded, full stop — a passive row that merely
shares a name with a verb ('checkout-api', 'delete-log-2024') is excluded
too; --allow-destructive re-includes them. Dropped CONTENT_SLUG_RE,
withoutSlugTokens, INTERACTIVE_ROLES and the cursor/tabindex probe.
Verified live: the real Pulse dashboard still yields 7 filmable elements
and a 3-beat story with only the checkout-api rows excluded.
URL redaction — page URLs are validation KEYS (scene.entry.url must
round-trip exactly against the raw crawled URL), so redacting them in the
prompt broke the recipe gate on any token-bearing URL and the retry
feedback re-leaked the raw URL. Revert URL redaction in the analyze/script
prompts (titles, headings, and element text stay redacted — they're
display-only). Instead, drop any page whose settled URL itself carries a
secret at crawl time (new pageUrlHasSecret) with a clear warning, and fail
the run with a plain message if the target URL is itself a credential — so
a token-URL never reaches the prompt to leak, and normal URLs round-trip
unchanged.
Tests 176 -> 178 (pageUrlHasSecret unit test, credential-URL crawl refusal
e2e, framework-onClick + passive-slug exclusion, URL round-trip). Live
generate verified.
1 participant
Follow-up to #10. This makes
generateproduce launch-story-quality videos for any app — the fixes live in the pipeline, not in tuned example recipes — and fixes the two things that made output feel wrong: laggy camera pacing and bad music.Camera pacing (every render)
Output held the camera zoomed on already-static content for 4.2s (read as lag) and pumped the zoom in/out between beats (read as aimless cursor movement). Now: focus payoff holds 2.4s not 4.2s, establishing shot 0.9s, and nearby beats bridge into one continuous glide instead of pumping. Spring/blur/scroll math untouched; plan tests updated with recomputed frame numbers.
Music
The bundled tracks are now synthesized from pure oscillators (
tools/synth-music.py— sub bass, drum machine, filtered-noise hats, saw pads, arpeggios). AI music generation kept smuggling choral pads and vocal chops in despite instrumental flags; with no vocal source in the signal chain, vocals are impossible by construction. Four moods, looped to ~90s beds. Andmusic_trackis finally wired end-to-end: the director picks a track from the app's look (dark →midnight/pulse, bright →daybreak) and it's muxed automatically — previously it hardcoded a nonexistentinstitutional-01so everygeneratewas silent.Director generalization (the core)
Live-testing
generateagainst three different apps exposed pipeline defects that made real output storyless:checkout-apiexcluded because of "checkout") — now it fires only on genuine action controls and ignores matches inside hyphen/underscore identifiers; anddata-testidwas crawled but never used to build selectors, so live-ticking rows fell back to ahas-textselector that broke between read and verify. After: 9 elements, and same-testid siblings each get a distinct:nth-matchentry so the director can click different rows — the interaction that tells a dashboard's story.[tag]annotation self-heals instead of burning retries.Verified live (DeepSeek, vision off): Pulse dashboard, Lumon signup, and a portfolio's project page each produced a coherent 3-beat story with theme-matched music chosen unprompted. Bright Lumon →
daybreak, dark Pulse →pulse; not name-matching (confirmed with the discriminator).Tests: 149 → 164. tsc/build clean.
Not verified: the synthesized music is provably vocal-free but I could not listen to judge whether it's good; wants a human ear before this is considered closed.