Repository navigation
feat(generate-video): JSON-schema driven hybrid architecture - #37
Conversation
Codex APPROVE (Score B+) を取得したgenerate-video品質向上計画。
Key decisions:
- JSON Schema as SSOT with Zod auto-generation
- Task tool parallel JSON generation (not direct TSX)
- 3-layer validation: scene → scenario → E2E
- Determinism: SHA-256 hashes, seed parameter, package-lock.json
- Merge strategy: scenario.sections[] index as primary key
Pipeline I/O Contract:
- out/video-{YYYYMMDD}-{id}/ directory structure
- scene-{NNN}.json naming convention
- assets/manifest.json for hash management
8 Phases, 24 Tasks total at cc:TODO status.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Phase 4: Direction system (transition, emphasis, background) - Phase 5: Visual components (EmphasisBox, TransitionWrapper, etc.) - Phase 6: Image generation patterns (comparison, concept, flow, highlight) - Phase 7: Asset foundation (backgrounds, sounds, user override) - Reorganized to 10 phases total for complete quality pipeline Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Complete implementation of generate-video skill quality improvements: ## Architecture - JSON-schema driven pipeline: scenario → scenes → merge → validate → render - 3-layer validation: scene → scenario → E2E gate - Determinism: SHA-256 hashes, seed parameter, package-lock pinning ## Schemas (10 files) - scenario/scene/video-script schemas for core data - direction/animation/emphasis schemas for visual effects - visual-patterns/character schemas for extensions ## Scripts (8 files) - generate-schemas.js: JSON Schema → Zod conversion - validate-scene/scenario/video.js: 3-layer validation - merge-scenes.js: Scene merging with conflict detection - render-video.js: Remotion CLI wrapper - load-assets.js: 3-tier asset loader - template-registry.js: Template management ## Components (4 files) - EmphasisBox: 3-level emphasis with animations - TransitionWrapper: 4 transition types (fade/slideIn/zoom/cut) - ProgressIndicator: Section progress display - BackgroundLayer: 5 background types ## Tests - 92 tests across 6 test files - Determinism tests, schema validation, E2E render tests ## Codex Review Fixes - Fixed merge-scenes.js schema compliance (total_duration_ms, output_settings) - Fixed validate-video.js early return on schema failure - Fixed transition enum consistency between schema and component Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Security: - render-video.js: Use shell: false with spawn to prevent command injection - render-video.js: Write props to temp file instead of CLI args (avoids length limits) Quality: - validate-video.js: Guard ajv.addSchema against duplicate registration - generate-schemas.js: Fix Zod schema naming (NameSchema suffix) - generate-schemas.js: Remove duplicate import statements in generated files All 92 tests passing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Refactor getRemotionCli() to return { command, extraArgs }
- Handle npx case separately from local binary path
- Fixes issue where paths with spaces would break on split
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
## 11.1 $ref Resolution - Add json-schema-deref-sync for dereferencing JSON Schema $ref - Resolve $ref before Zod conversion for type safety ## 11.2 Naming Standardization - All time units now use _ms suffix - Transition enums unified to: fade, slide_in, zoom, cut - All properties use snake_case - Added references/naming-conventions.md ## 11.3 Merge Determinism - Add scene_id tiebreaker for identical section+order - Detect and warn on duplicate orders - Unknown sections now cause Critical errors ## 11.4 Validation Integration - merge-scenes.js validates scenario.json by default - render-video.js validates video-script.json by default - Add --skip-validation flag for explicit bypass ## 11.5 Component/Schema Sync - Add src/types/components.ts with TypeScript interfaces - Add src/utils/converters.ts for ms<->frames conversion ## 11.6 Test Improvements - Export sortScenes from merge-scenes.js - Add merge-scenes.test.js (11 unit tests) - Add merge-e2e.test.js (7 E2E tests) - Add converters.test.js (54 tests) - Add validation-integration.test.js (11 tests) Total: 184 tests passing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ments - Add json-schema-deref-sync for $ref resolution in Zod generation - Fix naming conventions: snake_case for properties, kebab-case for IDs - Align alignment enums: topLeft → top_left across schemas - Fix transition enums: "slide" → "slide_in" in fixtures and examples - Clarify components.ts as Remotion-specific props (not schema mapping) - Regenerate all Zod schemas with proper $ref dereferencing - Update tests to use snake_case naming Codex Review Results: - Architect: C → A (no issues) - Security: B (1 Medium, 3 Low) - Performance: B (1 Medium, 1 Low) - Quality: B (2 Medium, 1 Low) All 184 tests passing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. Walkthroughgenerate-video スキルを大規模に追加・拡張:JSON Schema と Zod 型、Remotion 用 React コンポーネント群、スキーマ生成/検証/マージ/レンダリングスクリプト、資産定義、テンプレート、包括的なユニット・統合・E2E テスト、およびルートプラグインのバージョンバンプ(2.17.3 → 2.17.6)。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TaskTool as Task Tool (agent)
participant SceneGen as Scene Generator (per-scene job)
participant AssetLoader as Asset Loader
participant Validator as Schema Validator
participant Merger as Merger (merge-scenes.js)
participant Renderer as Remotion Renderer (render-video.js)
User->>TaskTool: シナリオ要求 (scenario.json)
TaskTool->>SceneGen: 並列ジョブを開始 -> out/.../scenes/{scene_id}.json
SceneGen->>AssetLoader: アセット参照/ロード
SceneGen->>Validator: シーン単位のスキーマ検証
Validator-->>SceneGen: PASS / FAIL
SceneGen-->>TaskTool: 出力 (scenes/{scene_id}.json, validation status)
TaskTool->>Merger: 集めた scenes/*.json を渡す
Merger->>Validator: マージ後の video-script.schema.json を検証(E2E)
Merger-->>Renderer: video-script.json(合格時にレンダリングへ)
Renderer->>AssetLoader: アセット解決(存在確認・パス解決)
Renderer->>Validator: 最終チェック(オプション)
Renderer-->>User: 完成ビデオを出力
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60分 Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@skills/generate-video/remotion/components/TransitionWrapper.tsx`:
- Around line 11-13: The TransitionType union in TransitionWrapper.tsx is using
camelCase 'slideIn' but schemas use snake_case 'slide_in'; update the
TransitionType definition to include 'slide_in' instead of 'slideIn' (export
type TransitionType = 'fade' | 'slide_in' | 'zoom' | 'cut') and check any usage
of TransitionType within TransitionWrapper (e.g., props, switch/case,
comparisons) to use the snake_case value or map incoming schema values to the
updated enum consistently.
In `@skills/generate-video/schemas/character.schema.json`:
- Around line 122-255: The schema allows appearance.type to be
"avatar"/"icon"/"image" etc. without requiring the corresponding configuration
object, which can lead to missing data at render time; add JSON Schema
conditional rules (if/then) under the "appearance" schema so that when "type" is
"avatar" the "avatar" property is required, when "type" is "icon" the "icon"
property is required, when "type" is "image" the "image" property is required
(and optionally require other fields like avatar.style or icon.icon_path if
desired); update the "appearance" object to include these if/then blocks
referencing the existing "type", "avatar", "icon", and "image" property names so
callers must supply the matching config for each type.
In `@skills/generate-video/schemas/direction.schema.json`:
- Around line 10-14: The scene_id property pattern allows uppercase letters and
underscores which is inconsistent with the other schemas; update the "scene_id"
JSON Schema (property name "scene_id") to use the same pattern as the other
schemas (only lowercase letters, digits and hyphens) by replacing the pattern
with one that enforces ^[a-z0-9-]+$ and keep the description consistent if
needed so downstream validation won't fail.
In `@skills/generate-video/schemas/examples/video-script-example.json`:
- Around line 125-128: The example's emphasis object uses invalid fields
("effect" and "timing"); update the example to match emphasis.schema.json by
removing "effect" and "timing" under emphasis, add an animation object with
animation.pulse set to true to represent a pulse effect (e.g.,
emphasis.animation.pulse = true), and if voice timing is needed move that data
under sound.timing using one of the allowed enum values ("start", "end", or
"peak"); ensure references to emphasis, animation.pulse, and sound.timing in the
example match the schema names exactly.
In `@skills/generate-video/schemas/examples/visual-patterns/flow-example.json`:
- Around line 5-10: JSON property names use camelCase but the schema expects
snake_case; rename the keys in the example: change "colorScheme" to
"color_scheme" (the object with primary/secondary/accent/background), change
"arrowStyle" to "arrow_style", and change "showNumbers" to "show_numbers",
updating every occurrence and preserving their values/structure so the example
conforms to visual-patterns.schema.json.
In `@skills/generate-video/scripts/load-assets.js`:
- Around line 110-140: loadAssetFile is vulnerable to path traversal via the
category and filename inputs; fix by validating/sanitizing inputs before
filesystem access: ensure category and filename are not absolute (reject
path.isAbsolute), normalize them (path.normalize) and reject if they contain
'..' segments or resolve outside the intended base directories, then resolve the
candidate paths with path.resolve against USER_ASSETS_DIR and ASSETS_DIR and
only accept them if the resolved path startsWith the corresponding resolved base
directory (USER_ASSETS_DIR or ASSETS_DIR); if validation fails, log a warning
and return null instead of accessing the filesystem. Ensure checks reference
loadAssetFile, USER_ASSETS_DIR, ASSETS_DIR, category, and filename.
In `@skills/generate-video/scripts/template-registry.js`:
- Around line 87-112: loadTemplateFile currently allows path-traversal via
templateName; validate and restrict access by either ensuring templateName is a
known template key or resolving the path and verifying it stays under
TEMPLATES_DIR. Specifically, inside loadTemplateFile (and callers like the CLI
load/variables commands), check that templateName exists in TEMPLATE_METADATA
keys before using it OR compute const templatePath = path.resolve(TEMPLATES_DIR,
`${templateName}.json`) and assert
templatePath.startsWith(path.resolve(TEMPLATES_DIR) + path.sep) (or use
path.relative and ensure it does not start with '..') and throw an error if the
check fails; then continue reading/parsing as before.
In `@skills/generate-video/scripts/validate-scenario.js`:
- Around line 271-272: The current filtering treats semanticErrors as those with
keyword === 'semantic', but validateSemantics emits specific keywords like
'uniqueness', 'order-sequence', 'order-duplicate', 'duration-negative', etc., so
semanticErrors will always be empty; update the filtering logic for
semanticErrors to check against the actual semantic keywords emitted by
validateSemantics (e.g., create a SEMANTIC_KEYWORDS array containing
'uniqueness','order-sequence','order-duplicate','duration-negative' and then set
semanticErrors = result.errors.filter(e =>
SEMANTIC_KEYWORDS.includes(e.keyword)), leaving schemaErrors as the complement
or filtering by !SEMANTIC_KEYWORDS.includes(e.keyword)); reference
result.errors, semanticErrors, schemaErrors and validateSemantics when making
the change.
In `@skills/generate-video/src/schemas/video-script.ts`:
- Around line 15-33: The actions item currently allows an empty object because
"type", "selector", and "value" are all optional; make "type" required by
removing .optional() on the "type" enum inside the actions z.object, then add
conditional validation (e.g., in the same actions z.object or its existing
superRefine) to require "selector" for action types that need a selector (click,
hover, scroll, type, keypress) and to require "value" for action types that need
a value (type requires a string, wait requires a number), ensuring type-specific
shapes are enforced; also update the source JSON Schema
(skills/generate-video/schemas/scene.schema.json) to add the appropriate
"required" entries for actions items and then run npm run generate:schemas to
regenerate the Zod schema.
In `@skills/generate-video/templates/intro-3min.json`:
- Around line 79-146: The template intro-3min.json currently sets
content.actions as a string ("actions": "{{DEMO_PART1_ACTIONS}}"), which
violates the scene.schema.json that requires actions to be an array of objects;
fix by making DEMO_PART1_ACTIONS / DEMO_PART2_ACTIONS / DEMO_PART3_ACTIONS
provide a JSON array (e.g., [{"type": "...","selector":"...","value":"..."}]) or
change the template substitution logic to parse the variable as JSON and inject
the resulting array into content.actions (ensure the substitution code
recognizes and inserts a parsed array rather than a quoted string), updating all
occurrences (content.actions) in the file to match the schema.
In `@skills/generate-video/tests/visual-patterns.test.js`:
- Around line 196-278: The example flow JSON uses camelCase keys that don't
match the schema; update the example file (flow-example.json) so the flow object
uses snake_case keys arrow_style and show_numbers (instead of arrowStyle and
showNumbers) to match the schema used by the tests/validator (see the validFlow
/ flow object in visual-patterns.test.js).
🟡 Minor comments (28)
skills/generate-video/references/image-patterns.md-39-49 (1)
39-49:⚠️ Potential issue | 🟡 Minorフェンス付きコードブロックに言語指定を追加
markdownlintのMD040に該当しています。ASCII図のブロックは ```text などの言語指定を入れてください(同様の他ブロックにも適用)。
🛠️ 修正案
-``` +```text ┌──────────────────────────────────────────┐ │ │ │ [悪い例/Before] 🠖 [良い例/After] │ │ │ │ ❌ 問題点1 ✅ 改善点1 │ │ ❌ 問題点2 ✅ 改善点2 │ │ ❌ 問題点3 ✅ 改善点3 │ │ │ └──────────────────────────────────────────┘ -``` +```skills/generate-video/references/asset-customization.md-474-484 (1)
474-484:⚠️ Potential issue | 🟡 MinorVIDEO_ASSETS_DIR の例が実装に存在しません
現状の load-assets.js は環境変数を読んでいないため、この手順どおりには切替できません。実装追加か注記を入れると混乱が減ります。skills/generate-video/references/asset-customization.md-334-351 (1)
334-351:⚠️ Potential issue | 🟡 MinorJSON内の部分上書きが実装と一致していません
load-assets.js はユーザー backgrounds.json を見つけた時点で全置換しており、未記載 ID がデフォルトから補完されません。説明を「完全上書き」に修正するか、コード側でマージ処理を追加してください。skills/generate-video/references/asset-customization.md-11-15 (1)
11-15:⚠️ Potential issue | 🟡 Minorコードフェンスに言語指定を追加
markdownlint の MD040 が出ているので、text /json /bash /markdown などを付けると警告が解消します。Also applies to: 25-43, 375-389, 488-504
skills/generate-video/schemas/character.schema.json-58-64 (1)
58-64:⚠️ Potential issue | 🟡 Minorlanguage の正規表現が説明より狭い
説明では BCP 47 も許容とありますが、現状はxx/xx-YYのみ許可します。許容範囲を広げるか説明を合わせてください。✏️ 例: パターン緩和
- "pattern": "^[a-z]{2}(-[A-Z]{2})?$", + "pattern": "^[a-z]{2,3}(-[A-Za-z0-9]{2,8})*$",Plans.md-28-29 (1)
28-29:⚠️ Potential issue | 🟡 MinorPlans Boardで未完了扱いにならないようタスク表記を調整
PlansParser は-で始まる行をタスク扱いにするため、ここが pending として表示される可能性があります。完了済みとして扱いたい場合は[x]形式やcc:DONEを付けるか、箇条書きを段落にするのが安全です。skills/generate-video/src/types/components.ts-21-44 (1)
21-44:⚠️ Potential issue | 🟡 Minorspring 設定があるのに type で選択できません
コメントと spring オプションがある一方、
typeにspringが含まれていません。また、isTransitionConfigの type guard (210行目) でも'spring'が検証リストに追加されていません。使うなら union と type guard に追加、不要なら spring プロパティを削除してください。🛠️ 修正案(spring を許可)
-export interface TransitionConfig { - /** Transition type */ - type: 'fade' | 'slide_in' | 'zoom' | 'cut'; +export interface TransitionConfig { + /** Transition type */ + type: 'fade' | 'slide_in' | 'zoom' | 'cut' | 'spring'; ... - ['fade', 'slide_in', 'zoom', 'cut'].includes(config.type) && + ['fade', 'slide_in', 'zoom', 'cut', 'spring'].includes(config.type) &&skills/generate-video/src/schemas/animation.ts-12-15 (1)
12-15:⚠️ Potential issue | 🟡 MinorJSON スキーマソースで
type === "spring"の条件付き検証を追加してください。
springパラメータの説明では「typeがspringの場合のみ適用」と記載されていますが、現在のスキーマ定義では他のタイプでも提供可能です。animation.schema.jsonで JSON Schema のif/thenまたはoneOfを用いて、type が "spring" の場合のみ spring パラメータを適用する条件付き制約を追加してください。skills/generate-video/schemas/validate-schemas-basic.js-15-19 (1)
15-19:⚠️ Potential issue | 🟡 Minor「全スキーマ検証」とは言い切れない点が気になります。
新規スキーマが多数あるため、固定3件だと見落としが出ます。*.schema.jsonを自動収集するか、出力文言を限定的にしてください。修正案(自動収集)
-const schemas = [ - { name: 'scenario.schema.json', file: 'scenario.schema.json' }, - { name: 'scene.schema.json', file: 'scene.schema.json' }, - { name: 'video-script.schema.json', file: 'video-script.schema.json' } -]; +const schemas = fs.readdirSync(__dirname) + .filter((file) => file.endsWith('.schema.json')) + .map((file) => ({ name: file, file }));skills/generate-video/references/naming-conventions.md-51-57 (1)
51-57:⚠️ Potential issue | 🟡 MinorEnum表記ルールが矛盾しています(snake_case vs hyphen)。
transition.typeのみ snake_case とする等、例外を明記して整合を取ってください。✏️ 例(例外を明記)
-**Enum values MUST use lowercase with hyphens for multi-word values** +**Enum values MUST use lowercase with hyphens for multi-word values** +(ただし `transition.type` は snake_case を採用)Also applies to: 123-125
skills/generate-video/schemas/examples/visual-patterns/concept-example.json-5-10 (1)
5-10:⚠️ Potential issue | 🟡 Minor命名規約(snake_case)と例のフィールド名が不一致です。
colorScheme/aspectRatioはcolor_scheme/aspect_ratioへ統一するか、例外として明記してください。✏️ 例(snake_caseへ統一)
- "colorScheme": { + "color_scheme": { "primary": "#6366F1", "secondary": "#8B5CF6", "accent": "#EC4899", "background": "#0F172A" }, @@ - "dimensions": { - "width": 1920, - "height": 1080, - "aspectRatio": "16:9" - }, + "dimensions": { + "width": 1920, + "height": 1080, + "aspect_ratio": "16:9" + },Also applies to: 74-78
skills/generate-video/remotion/components/README.md-213-221 (1)
213-221:⚠️ Potential issue | 🟡 Minor表は前後に空行を入れてください(Markdownlint MD058)。
✏️ 例
-**Background Types**: -| Type | Primary | Secondary | Use Case | +**Background Types**: + +| Type | Primary | Secondary | Use Case | |------|---------|-----------|----------| | `neutral` | Dark gray | Light gray | General content, demos | | `highlight` | Cyan | Magenta | Intros, CTAs, highlights | | `dramatic` | Black | Red | Hooks, problem statements | | `tech` | Dark blue | Navy | Architecture, technical content | | `warm` | Orange | Yellow | Conclusions, warm CTAs | +skills/generate-video/schemas/examples/scene-example.json-20-23 (1)
20-23:⚠️ Potential issue | 🟡 Minor
emphasis.timingが ms ルールに沿っていません。
命名規約では時間は_msが必須なので、timing_ms: 2000などに統一するか、例外を明記してください。✏️ 例
- "emphasis": { - "effect": "glitch", - "timing": "2s" - }, + "emphasis": { + "effect": "glitch", + "timing_ms": 2000 + },skills/generate-video/remotion/components/README.md-267-272 (1)
267-272:⚠️ Potential issue | 🟡 Minor「60fps」は「60 fps」などの表記に揃えると読みやすいです。
✏️ 例
-- **Smooth**: 60fps at 1920x1080 +- **Smooth**: 60 fps at 1920×1080skills/generate-video/templates/image-prompts/concept.txt-11-19 (1)
11-19:⚠️ Potential issue | 🟡 Minorテンプレートのキー名がスキーマ/例と不一致です。
parentId/relationshipTypeはparent_id/typeに合わせるか、テンプレート生成前の変換処理を明記してください。✏️ 例(テンプレート側で統一)
-{{order}}. {{label}} ({{icon}} icon, emphasis: {{emphasis}}) - {{`#description`}}- Description: {{.}}{{/description}} - {{`#level`}}- Level: {{.}}{{/level}} - {{`#parentId`}}- Parent: {{.}}{{/parentId}} +{{order}}. {{label}} ({{icon}} icon, emphasis: {{emphasis}}) + {{`#description`}}- Description: {{.}}{{/description}} + {{`#level`}}- Level: {{.}}{{/level}} + {{`#parent_id`}}- Parent: {{.}}{{/parent_id}} @@ -- {{from}} {{relationshipType}} {{to}}{{`#label`}} ({{.}}){{/label}} +- {{from}} {{type}} {{to}}{{`#label`}} ({{.}}){{/label}}skills/generate-video/tests/e2e/render.test.js-127-134 (1)
127-134:⚠️ Potential issue | 🟡 Minorコメントの正確性を確認してください
Line 133のコメント「100ms rounds up」は不正確です。
msToFramesはMath.round()を使用しており、100ms @ 30fps = 3フレーム(Math.round(0.1 * 30) = 3)は四捨五入の結果であり、切り上げではありません。関連ファイル:
skills/generate-video/src/utils/converters.ts(lines 23-32)ではMath.round()を使用。📝 コメント修正案
expect(msToFrames(1000, fps)).toBe(30); // 1 second expect(msToFrames(3000, fps)).toBe(90); // 3 seconds expect(msToFrames(5000, fps)).toBe(150); // 5 seconds - expect(msToFrames(100, fps)).toBe(3); // 100ms rounds up + expect(msToFrames(100, fps)).toBe(3); // 100ms = 3 frames (Math.round)skills/generate-video/scripts/README.md-339-347 (1)
339-347:⚠️ Potential issue | 🟡 MinorREADME の「Future Scripts」セクションから
merge-scenes.jsを削除してください
merge-scenes.jsは既にskills/generate-video/scripts/に実装済みですが、README の 343 行目で「今後追加予定」として記載されています。ドキュメントを最新の状態に更新してください。skills/generate-video/scripts/generate-schemas.js-162-166 (1)
162-166:⚠️ Potential issue | 🟡 Minorメタデータの description エスケープが不完全
Line 165 でシングルクォートのみエスケープしていますが、description に改行やバックスラッシュが含まれる場合、生成される TypeScript ファイルで構文エラーが発生します。
🛡️ より安全なエスケープ処理
export const ${schemaName}Meta = { version: '${version}', title: '${title}', - description: '${description.replace(/'/g, "\\'")}', + description: '${description.replace(/\\/g, '\\\\').replace(/'/g, "\\'").replace(/\n/g, '\\n').replace(/\r/g, '\\r')}', } as const;または
JSON.stringifyを使用:description: ${JSON.stringify(description)},skills/generate-video/scripts/generate-schemas.js-95-96 (1)
95-96:⚠️ Potential issue | 🟡 Minor失敗したスキーマも index.ts に含まれる可能性
generateIndexFileは全てのschemaFilesを受け取りますが、生成に失敗したスキーマも index.ts の export に含まれてしまいます。これにより、存在しないモジュールを import しようとしてビルドエラーが発生する可能性があります。🐛 成功したスキーマのみ index に含める
+ // 成功したスキーマファイルのみを抽出 + const successfulFiles = results + .filter(r => r.success) + .map(r => r.file); + // インデックスファイルを生成 - generateIndexFile(schemaFiles); + generateIndexFile(successfulFiles);skills/generate-video/scripts/validate-scenario.js-244-249 (1)
244-249:⚠️ Potential issue | 🟡 Minor
metadataがundefinedの場合にエラーが発生する可能性があります。
scenarioData.metadata.video_typeへの直接アクセスは、metadataが存在しない場合にTypeErrorをスローします。オプショナルチェイニングを使用してください。- if (scenarioData.metadata.video_type) { - console.log(` Type: ${scenarioData.metadata.video_type}`); + if (scenarioData.metadata?.video_type) { + console.log(` Type: ${scenarioData.metadata?.video_type}`); } - if (scenarioData.metadata.target_funnel) { - console.log(` Funnel: ${scenarioData.metadata.target_funnel}`); + if (scenarioData.metadata?.target_funnel) { + console.log(` Funnel: ${scenarioData.metadata?.target_funnel}`); }skills/generate-video/tests/determinism.test.js-91-116 (1)
91-116:⚠️ Potential issue | 🟡 Minorテストヘルパーの
mergeScenesが実際の実装と異なっています。テストヘルパーが
section_idとorderでのみソートしているのに対して、実装のソート関数(merge-scenes.js)は以下の複数の点で異なります:
- タイブレーカー欠落: 同一
orderの場合、実装ではscene_id.localeCompare(scene_id)をタイブレーカーとして使用していますが、テストヘルパーにはこのロジックがありません- シナリオの段階順序未使用: 実装は
scenario.sectionsから段階の順序を参照してソートしていますが、テストヘルパーはシナリオを参照せずsection_idの文字列比較を行っています実装の
sortScenes関数をインポートして再利用することで、テストの信頼性が向上します。skills/generate-video/tests/merge-scenes.test.js-355-375 (1)
355-375:⚠️ Potential issue | 🟡 MinorlocaleCompare の明示的なロケール指定で決定論的なソート順序を保証する
現在の実装(line 186)では
localeCompare()にロケール引数がなく、テスト環境のシステムロケール設定に依存しています。テストコメント(lines 369-370)で'_'が space として扱われることを前提としていますが、このソート順序は環境によって異なる可能性があります。CI/CD 環境やローカル開発環境で異なるロケール設定を使用している場合、テストが失敗することがあります。localeCompare('ja-JP')のように明示的なロケールを指定することで、すべての環境で一貫した結果を保証してください。skills/generate-video/scripts/render-video.js-239-241 (1)
239-241:⚠️ Potential issue | 🟡 Minor
msToFramesの入力検証と丸めを統一してください
--skip-validation時に負値やfps <= 0が入ると NaN/負値になり、ログや計算が壊れます。skills/generate-video/src/utils/converters.tsの挙動に合わせると一貫性が出ます。✅ 修正案
function msToFrames(ms, fps) { - return Math.ceil((ms / 1000) * fps); + if (ms < 0) { + throw new Error(`msToFrames: milliseconds must be non-negative, got ${ms}`); + } + if (fps <= 0) { + throw new Error(`msToFrames: fps must be positive, got ${fps}`); + } + return Math.round((ms / 1000) * fps); }skills/generate-video/scripts/render-video.js-195-227 (1)
195-227:⚠️ Potential issue | 🟡 MinorURL アセットが誤って “見つからない” 警告になります
asset.sourceが URL の場合も相対解決に入り、存在チェックで警告が出ます。URL は解決せずそのまま通す分岐を追加してください(スキーマでも URL が許可されています)。✅ 修正案
if (!asset.source) { return asset; } + const isUrl = + /^https?:\/\//i.test(asset.source) || + asset.source.startsWith('data:') || + asset.source.startsWith('file://'); + if (isUrl) { + return asset; + } + // Check if asset path is absolute if (path.isAbsolute(asset.source)) { return asset; }skills/generate-video/remotion/components/ProgressIndicator.tsx-230-238 (1)
230-238:⚠️ Potential issue | 🟡 Minor
createSectionsの endFrame が 1 フレーム長くなります
endFrame = startFrame + durationFramesだと inclusive 判定(<=)で 1 フレーム余分になり、次セクションと重複します。-1するか判定条件を変更してください。✅ 修正案
id: scene.id, name: scene.name, startFrame: scene.startFrame, - endFrame: scene.startFrame + scene.durationFrames, + endFrame: scene.startFrame + scene.durationFrames - 1, })); };skills/generate-video/remotion/components/EmphasisBox.tsx-164-175 (1)
164-175:⚠️ Potential issue | 🟡 Minorswitch文内での変数宣言にブロックスコープを追加してください。
case 'glitch'内でconst glitchOffsetが宣言されていますが、ブロックスコープ(波括弧)で囲まれていません。これは他のcase節から誤ってアクセスされる可能性があり、また一部のリンターでエラーとして検出されます。🐛 ブロックスコープを追加する修正案
- case 'glitch': + case 'glitch': { // Glitch effect with text shadow const glitchOffset = enablePulse ? Math.sin(relativeFrame * 0.5) * 2 : 0; return { ...baseStyle, textShadow: ` ${glitchOffset}px 0 0 `#ff00ff`, ${-glitchOffset}px 0 0 `#00ffff`, 0 0 ${effectiveGlowIntensity}px ${color} `, }; + }skills/generate-video/tests/asset-loader.test.js-215-231 (1)
215-231:⚠️ Potential issue | 🟡 Minor
setTimeoutを使用したテストは非同期処理を正しく待機していません。このテストは
setTimeout内でアサーションを実行していますが、テスト関数自体は同期的であるため、setTimeoutコールバック内のアサーションはテスト完了後に実行される可能性があり、テスト結果が不安定になります。🐛 async/awaitまたはPromiseを使用した修正案
- test('updates generated_at timestamp', () => { + test('updates generated_at timestamp', async () => { const manifestPath = path.join(TEST_USER_DIR, 'manifest3.json'); const asset = { id: 'test', path: 'test.png', type: 'image', hash: 'xyz', size: 500 }; updateManifest(manifestPath, asset); const manifest1 = JSON.parse(fs.readFileSync(manifestPath, 'utf-8')); const timestamp1 = manifest1.generated_at; - // Wait a bit - setTimeout(() => { - updateManifest(manifestPath, { id: 'test2', path: 'test2.png', type: 'image', hash: 'xyz2', size: 600 }); - const manifest2 = JSON.parse(fs.readFileSync(manifestPath, 'utf-8')); - const timestamp2 = manifest2.generated_at; - - expect(timestamp2).not.toBe(timestamp1); - }, 100); + // Wait a bit using Promise + await new Promise(resolve => setTimeout(resolve, 100)); + + updateManifest(manifestPath, { id: 'test2', path: 'test2.png', type: 'image', hash: 'xyz2', size: 600 }); + const manifest2 = JSON.parse(fs.readFileSync(manifestPath, 'utf-8')); + const timestamp2 = manifest2.generated_at; + + expect(timestamp2).not.toBe(timestamp1); });skills/generate-video/scripts/validate-video.js-217-219 (1)
217-219:⚠️ Potential issue | 🟡 Minor
output_settingsが未定義の場合にランタイムエラーが発生する可能性があります。
videoData.output_settingsが存在しない場合、widthとheightへのアクセスでTypeErrorがスローされます。スキーマ検証が先に行われているため通常は問題ありませんが、防御的プログラミングとしてnullチェックを追加することを推奨します。🛡️ 防御的チェックを追加する修正案
// 6. Resolution validation + if (!videoData.output_settings) { + warnings.push({ + path: '/output_settings', + message: 'Missing output_settings', + keyword: 'missing-field', + }); + return { errors, warnings }; + } + const { width, height } = videoData.output_settings;
🧹 Nitpick comments (31)
skills/generate-video/remotion/tsconfig.json (2)
14-18:noEmit: trueとdeclaration設定の整合性を確認してください。
noEmit: trueが設定されている場合、declaration: true、declarationMap: true、sourceMap: trueは実質的に無効になります。Remotion のバンドラーが処理するため意図的かもしれませんが、混乱を避けるためにコメントを追加するか、不要な設定を削除することを検討してください。♻️ 不要な設定を削除する場合
"isolatedModules": true, "noEmit": true, "allowSyntheticDefaultImports": true, - "declaration": true, - "declarationMap": true, - "sourceMap": true, "types": ["node"]
5-5: Remotion の TypeScript プロジェクトで"jsx": "react-jsx"への移行を検討してください。現在
"jsx": "react"を使用していますが、React 17 以降では"react-jsx"が推奨されます。この設定に変更することで、import React from 'react'の明示的なインポートが不要になり、コンポーネント内でも削除可能になります。さらにreact/jsx-runtimeを自動的に利用でき、バンドルサイズの若干の改善が期待できます。Remotion の TypeScript テンプレートで推奨されている設定です。skills/generate-video/src/utils/converters.ts (1)
24-139: NaN/Infinity がすり抜けるのでガードを共通化
< 0チェックだけだと NaN/Infinity が通過します。Number.isFiniteとisValidFpsを使った共通チェックを入れると安全です。♻️ 例: 共通ガード導入
export const DEFAULT_FPS = 30; + +function assertFiniteNumber(value: number, name: string): void { + if (!Number.isFinite(value)) { + throw new Error(`${name} must be a finite number, got ${value}`); + } +} export function msToFrames(ms: number, fps: number = DEFAULT_FPS): number { + assertFiniteNumber(ms, 'ms'); if (ms < 0) { throw new Error(`msToFrames: milliseconds must be non-negative, got ${ms}`); } - if (fps <= 0) { - throw new Error(`msToFrames: fps must be positive, got ${fps}`); - } + if (!isValidFps(fps)) { + throw new Error(`msToFrames: fps must be positive, got ${fps}`); + }skills/generate-video/scripts/template-registry.js (1)
123-149: 継承プロパティを拾わないループに寄せたいです。
for...inだとプロトタイプ由来のキーが混入し得るので、Object.entriesで限定するほうが安全です。♻️ 提案パッチ
if (obj !== null && typeof obj === 'object') { const result = {}; - for (const key in obj) { + for (const [key, value] of Object.entries(obj)) { // Skip _template_variables field if (key === '_template_variables') { continue; } - result[key] = replaceVariables(obj[key], variables); + result[key] = replaceVariables(value, variables); } return result; }skills/generate-video/src/types/README.md (1)
133-136: 使用例の import 先を新しいバレルに合わせてください。
index.tsを用意したので、サンプルでもそれを使う方が一貫します。修正案
-import { TransitionConfig } from '../src/types/components'; +import { TransitionConfig } from '../src/types';skills/generate-video/schemas/validate-schemas.js (1)
23-37: ファイル読み込みのエラーハンドリングを検討してくださいスキーマファイルが存在しない場合、
JSON.parseまたはfs.readFileSyncで例外がスローされますが、エラーメッセージがユーザーフレンドリーではありません。💡 エラーハンドリングの追加案
+function loadSchema(filename) { + const schemaPath = path.join(__dirname, filename); + if (!fs.existsSync(schemaPath)) { + console.error(`❌ Schema file not found: ${filename}`); + process.exit(1); + } + return JSON.parse(fs.readFileSync(schemaPath, 'utf8')); +} + // Load schemas -const scenarioSchema = JSON.parse( - fs.readFileSync(path.join(__dirname, 'scenario.schema.json'), 'utf8') -); +const scenarioSchema = loadSchema('scenario.schema.json');skills/generate-video/tests/schema-validation.test.js (1)
15-18: AJV設定の不整合に注意してくださいこのテストでは
strict: trueを使用していますが、validate-schemas.jsではstrict: falseを使用しています。意図的な違いであれば問題ありませんが、スキーマの動作が環境によって異なる可能性があります。関連ファイル:
skills/generate-video/schemas/validate-schemas.js(lines 16-20)ではstrict: falseを使用。💡 設定を統一する場合の提案
beforeAll(() => { // Initialize AJV validator - ajv = new Ajv({ allErrors: true, strict: true }); + ajv = new Ajv({ allErrors: true, strict: false }); addFormats(ajv);または、意図的な違いであればコメントで理由を明記してください。
skills/generate-video/tests/e2e/render.test.js (1)
100-105: テストフィクスチャのクリーンアップが不完全です
afterAllではoutputVideoPathのみクリーンアップしていますが、beforeAllで作成したminimalScriptPathはクリーンアップされていません。CI環境では問題になりませんが、ローカルでテストを繰り返し実行すると古いフィクスチャが残る可能性があります。💡 完全なクリーンアップの提案
afterAll(() => { // Cleanup test files if (fs.existsSync(outputVideoPath)) { fs.unlinkSync(outputVideoPath); } + if (fs.existsSync(minimalScriptPath)) { + fs.unlinkSync(minimalScriptPath); + } });skills/generate-video/src/schemas/scenario.ts (1)
15-15: 生成スクリプトにコード整形ツール(Prettier等)を統合してスキーマの可読性を改善してください現在、
scripts/generate-schemas.jsで生成されるZodスキーマは1行で出力されており、デバッグやレビュー時に可読性が低下しています。生成時にjson-schema-to-zodから出力された後、Prettierなどのフォーマッタを適用することで、読みやすく保守しやすいコードが実現できます。skills/generate-video/scripts/generate-schemas.js (1)
198-203:toPascalCaseのエッジケース考慮現在の実装は
visual-patterns→VisualPatternsのような一般的なケースでは正しく動作しますが、以下のエッジケースで予期しない結果になる可能性があります:
- 連続する区切り文字(例:
foo--bar)→ 空文字列の単語が生成される- 大文字を含む入力(例:
FOO_BAR)→FoobarになりFooBarにならない現在のスキーマファイル名が規約に従っている場合は問題ありませんが、将来の拡張性のために考慮してください。
🔧 より堅牢な実装案
function toPascalCase(str) { return str .split(/[-_.]/) + .filter(word => word.length > 0) - .map(word => word.charAt(0).toUpperCase() + word.slice(1).toLowerCase()) + .map(word => word.charAt(0).toUpperCase() + word.slice(1)) .join(''); }skills/generate-video/schemas/scene.schema.json (2)
73-96: Playwright actions のtypeフィールドを required にすることを検討
actions配列内のオブジェクトでtypeが required になっていません。typeがないとアクションの種類が不明になり、実行時エラーの原因になる可能性があります。♻️ type を required に追加
"items": { "type": "object", + "required": ["type"], "properties": { "type": {
183-200: camera のstart/endプロパティに型定義がない
camera.startとcamera.endは単に"type": "object"で定義されており、どのようなプロパティを持つべきか不明確です。これにより、不正なデータが渡される可能性があります。将来的にスキーマを拡張する場合は、
x,y,z,rotationなどの座標/回転プロパティを定義することを検討してください。skills/generate-video/schemas/video-script.schema.json (2)
81-85:total_duration_msの minimum を 1 以上にすることを検討
minimum: 0は 0ms の動画を許容しますが、実際には有効な動画には最低限の長さが必要です。content.duration_msがminimum: 100であることと整合性を取るなら、こちらも正の値にすべきかもしれません。♻️ minimum を調整
"total_duration_ms": { "type": "integer", "description": "Total duration of the video in milliseconds", - "minimum": 0 + "minimum": 100 },
202-225:branding.colorsに色フォーマットの pattern を追加することを検討
visual-patterns.schema.jsonでは色に"pattern": "^#[0-9A-Fa-f]{6}$"が定義されていますが、こちらのbranding.colorsには定義がありません。一貫性のために同様のパターンを追加するか、またはコメントで許容フォーマット(hex, rgb, named colors等)を明記することを検討してください。skills/generate-video/schemas/scenario.schema.json (1)
22-67: sections 配列に maximum を追加することを検討
sections配列にminItems: 1はありますが、maxItemsがありません。極端に大きなシナリオを防ぐために上限を設定することを検討してください(例:maxItems: 50)。skills/generate-video/tests/validation-integration.test.js (2)
26-28:/tmpのハードコードはクロスプラットフォーム互換性がありません。
os.tmpdir()を使用することで、Windows環境でもテストが動作するようになります。♻️ 提案する修正
+const os = require('os'); const MERGE_SCRIPT = path.join(__dirname, '../scripts/merge-scenes.js'); const RENDER_SCRIPT = path.join(__dirname, '../scripts/render-video.js'); describe('Validation Integration (Task 11.4)', () => { let tempDir; beforeEach(() => { - tempDir = fs.mkdtempSync(path.join('/tmp', 'validation-test-')); + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'validation-test-')); });
113-125:execSyncでの文字列連結によるコマンド構築について。セキュリティノートで説明されている通り、これはテストコードであり入力は制御されていますが、スペースを含むパスがある場合に問題が発生する可能性があります。
tempDirのパスにスペースが含まれる場合(例:macOS の一部の環境)、コマンドが正しく解析されない可能性があります。♻️ より堅牢なアプローチ
function runMerge(options = {}) { - const skipFlag = options.skipValidation ? ' --skip-validation' : ''; + const args = [MERGE_SCRIPT, tempDir]; + if (options.skipValidation) { + args.push('--skip-validation'); + } try { - const output = execSync(`node ${MERGE_SCRIPT} ${tempDir}${skipFlag}`, { + const output = execSync(`node ${args.map(a => `"${a}"`).join(' ')}`, { encoding: 'utf-8', stdio: 'pipe' });または
child_process.spawnSyncを使用:const { spawnSync } = require('child_process'); // ... const result = spawnSync('node', args, { encoding: 'utf-8' });skills/generate-video/remotion/components/BackgroundLayer.tsx (2)
95-96: 未使用の変数widthがあります。
useVideoConfig()からwidthを分割代入していますが、使用されていません。♻️ 提案する修正
- const { width, height, fps } = useVideoConfig(); + const { height, fps } = useVideoConfig();
187-221: パーティクルエフェクトのパフォーマンス考慮。フレームごとに20個のパーティクルが生成されます。長時間の動画や低スペック環境では、パーティクル数を減らすか、メモ化を検討してください。現状では許容範囲内ですが、将来的なスケーラビリティのために留意しておくと良いでしょう。
skills/generate-video/scripts/validate-scene.js (2)
22-28: AJV のstrict: false設定について。このスクリプトでは
strict: falseを使用していますが、character-schema.test.jsではstrict: trueを使用しています。一貫性のため、同じ設定を使用することを検討してください。strict: trueは潜在的なスキーマの問題を早期に検出するのに役立ちます。♻️ 提案する修正
const ajv = new Ajv({ - strict: false, + strict: true, allErrors: true, verbose: true, });ただし、既存のスキーマが
strict: trueで警告を出す場合は、スキーマの修正が必要になる可能性があります。
151-158:sceneData.contentの null チェックがありません。バリデーションが成功した場合でも、
sceneData.contentが存在することを前提としています。スキーマでcontentが必須の場合は問題ありませんが、防御的なチェックを追加するとより堅牢になります。🛡️ 防御的なチェックの追加
console.log('Scene details:'); console.log(` ID: ${sceneData.scene_id}`); console.log(` Type: ${sceneData.type}`); console.log(` Section: ${sceneData.section_id}`); - console.log(` Duration: ${sceneData.content.duration_ms}ms`); - if (sceneData.content.title) { - console.log(` Title: ${sceneData.content.title}`); + if (sceneData.content) { + console.log(` Duration: ${sceneData.content.duration_ms}ms`); + if (sceneData.content.title) { + console.log(` Title: ${sceneData.content.title}`); + } }skills/generate-video/src/schemas/scene.ts (1)
15-33: 自動生成されたスキーマの可読性について。このファイルは自動生成されており、
z.object()の定義が1行に圧縮されているため、デバッグやレビューが困難です。jsonSchemaToZodライブラリがこのフォーマットで出力しており、現在のジェネレータスクリプトには書式設定オプションがありません。scripts/generate-schemas.jsで生成後に Prettier を適用して、出力をより読みやすくすることを検討してください。skills/generate-video/schemas/visual-patterns.schema.json (2)
6-6:versionフィールドはJSON Schema標準キーワードではありません。Line 6の
"version": "1.0.0"はJSON Schema draft-07の標準キーワードではないため、AJVなどのバリデータでは無視されます。バージョン情報を含める場合は、$idにバージョンを含めるか、descriptionに記載することを検討してください。- "version": "1.0.0", + "$id": "https://claude-code-harness.com/schemas/visual-patterns.schema.json/v1.0.0",
400-404:seedの最大値の制限を検討してください。
seedフィールドにminimum: 0のみが設定されていますが、整数オーバーフローを防ぐためにmaximumも設定することを推奨します。"seed": { "type": "integer", "description": "決定性シード値(再現性のため)", - "minimum": 0 + "minimum": 0, + "maximum": 2147483647 },skills/generate-video/tests/merge-scenes.test.js (1)
22-23: クロスプラットフォーム対応のためos.tmpdir()の使用を推奨します。
/tmpはUnix系システム固有のパスです。Windows環境でもテストが実行できるよう、os.tmpdir()を使用することを検討してください。+const os = require('os'); + describe('merge-scenes.js', () => { let tempDir; beforeEach(() => { - tempDir = fs.mkdtempSync(path.join('/tmp', 'merge-test-')); + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'merge-test-')); });skills/generate-video/schemas/assets.manifest.schema.json (1)
65-70: ハッシュパターンが小文字のみを許可しています。SHA-256ハッシュは通常、大文字と小文字の両方で表現されます。現在のパターン
^[a-f0-9]{64}$は小文字のみを許可しています。これが意図的な正規化要件であれば問題ありませんが、外部システムから大文字のハッシュを受け取る可能性がある場合は、パターンを調整するか、入力時に小文字に正規化する処理を追加してください。"hash": { "type": "string", "description": "SHA-256 hash of the asset file", - "pattern": "^[a-f0-9]{64}$", + "pattern": "^[a-fA-F0-9]{64}$",skills/generate-video/tests/determinism.test.js (1)
11-12: 未使用のインポートがあります。
fsとpathがインポートされていますが、このファイルでは使用されていません。-const fs = require('fs'); -const path = require('path'); const crypto = require('crypto');skills/generate-video/tests/e2e/merge-e2e.test.js (1)
38-40: 一時ディレクトリはos.tmpdir()を使うと移植性が上がります
/tmp固定だと Windows 等で失敗します。os.tmpdir()を使うとテストが環境依存しにくくなります。✅ 修正案
const fs = require('fs'); const path = require('path'); const crypto = require('crypto'); +const os = require('os'); const { execSync } = require('child_process'); // Safe in tests with controlled inputs @@ - tempDir = fs.mkdtempSync(path.join('/tmp', 'merge-e2e-')); + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'merge-e2e-'));skills/generate-video/scripts/validate-video.js (1)
209-214: ナレーションを持つすべてのシーンに警告が出力されるのはノイズが多い可能性があります。ナレーションファイルを持つすべてのシーンに対して
audio-verify警告が無条件に追加されます。これは警告出力を大量に生成し、実際に注意が必要な警告を見逃す原因になる可能性があります。この警告を削除するか、
verboseフラグでオプション化することを検討してください。♻️ 警告を削除または条件付きにする修正案
// Check if narration start offset is reasonable if (startOffset > sceneDuration / 2) { warnings.push({ path: `/scenes/${index}/audio/narration/start_offset_ms`, message: `Narration starts too late (${startOffset}ms in ${sceneDuration}ms scene)`, keyword: 'audio-sync', }); } - - // Recommend checking audio duration - warnings.push({ - path: `/scenes/${index}/audio`, - message: `Audio sync should be verified with ffprobe for scene "${scene.scene_id}"`, - keyword: 'audio-verify', - }); }skills/generate-video/scripts/merge-scenes.js (2)
114-130: スキーマのコンパイルがシーンごとに繰り返されています。
ajv.compile(sceneSchema)がvalidateSceneの各呼び出しで実行されています。シーン数が多い場合、パフォーマンスに影響する可能性があります。スキーマを一度だけコンパイルして再利用することを推奨します。♻️ スキーマコンパイルを関数外に移動する修正案
+// Compiled validator cache +let compiledSceneValidator = null; + /** * Validate scene against schema */ function validateScene(scene, sceneSchema, fileName) { - const validate = ajv.compile(sceneSchema); + // Compile schema once and cache + if (!compiledSceneValidator) { + compiledSceneValidator = ajv.compile(sceneSchema); + } + const validate = compiledSceneValidator; const valid = validate(scene); if (!valid) {
41-59:loadJsonFile関数がvalidate-video.jsと重複しています。この関数は
validate-video.jsにも同一の実装が存在します。共有ユーティリティモジュールに抽出することで、コードの重複を削減できます。
| "appearance": { | ||
| "type": "object", | ||
| "description": "Visual appearance configuration (for character dialogue videos)", | ||
| "required": ["type"], | ||
| "properties": { | ||
| "type": { | ||
| "type": "string", | ||
| "description": "Visual representation type", | ||
| "enum": ["avatar", "icon", "image", "video", "none"], | ||
| "default": "avatar" | ||
| }, | ||
| "avatar": { | ||
| "type": "object", | ||
| "description": "Avatar configuration (for type: 'avatar')", | ||
| "properties": { | ||
| "style": { | ||
| "type": "string", | ||
| "enum": ["realistic", "cartoon", "minimalist", "pixel-art", "abstract"], | ||
| "default": "minimalist" | ||
| }, | ||
| "base_image": { | ||
| "type": "string", | ||
| "description": "Path to base avatar image" | ||
| }, | ||
| "expressions": { | ||
| "type": "object", | ||
| "description": "Expression image paths (for different emotions)", | ||
| "properties": { | ||
| "neutral": { | ||
| "type": "string", | ||
| "description": "Path to neutral expression image" | ||
| }, | ||
| "happy": { | ||
| "type": "string" | ||
| }, | ||
| "sad": { | ||
| "type": "string" | ||
| }, | ||
| "surprised": { | ||
| "type": "string" | ||
| }, | ||
| "thinking": { | ||
| "type": "string" | ||
| }, | ||
| "speaking": { | ||
| "type": "string" | ||
| } | ||
| } | ||
| }, | ||
| "color_scheme": { | ||
| "type": "object", | ||
| "properties": { | ||
| "primary": { | ||
| "type": "string", | ||
| "description": "Primary color (hex or rgb)" | ||
| }, | ||
| "secondary": { | ||
| "type": "string" | ||
| }, | ||
| "background": { | ||
| "type": "string" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "icon": { | ||
| "type": "object", | ||
| "description": "Simple icon representation (for type: 'icon')", | ||
| "properties": { | ||
| "icon_path": { | ||
| "type": "string", | ||
| "description": "Path to icon file (SVG/PNG recommended)" | ||
| }, | ||
| "shape": { | ||
| "type": "string", | ||
| "enum": ["circle", "square", "rounded-square", "hexagon"], | ||
| "default": "circle" | ||
| }, | ||
| "size": { | ||
| "type": "integer", | ||
| "description": "Icon size in pixels", | ||
| "minimum": 32, | ||
| "maximum": 512, | ||
| "default": 128 | ||
| }, | ||
| "background_color": { | ||
| "type": "string", | ||
| "description": "Background color (hex or rgb)" | ||
| } | ||
| } | ||
| }, | ||
| "image": { | ||
| "type": "object", | ||
| "description": "Static image representation (for type: 'image')", | ||
| "properties": { | ||
| "image_path": { | ||
| "type": "string", | ||
| "description": "Path to character image" | ||
| }, | ||
| "crop": { | ||
| "type": "object", | ||
| "description": "Image cropping configuration", | ||
| "properties": { | ||
| "x": { | ||
| "type": "integer" | ||
| }, | ||
| "y": { | ||
| "type": "integer" | ||
| }, | ||
| "width": { | ||
| "type": "integer" | ||
| }, | ||
| "height": { | ||
| "type": "integer" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "position": { | ||
| "type": "string", | ||
| "description": "Position on screen (for dialogue scenes)", | ||
| "enum": ["left", "right", "center", "top_left", "top_right", "bottom_left", "bottom_right"], | ||
| "default": "left" | ||
| }, | ||
| "size_preset": { | ||
| "type": "string", | ||
| "description": "Size preset for character display", | ||
| "enum": ["small", "medium", "large", "full-screen"], | ||
| "default": "medium" | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
appearance.type と対応設定の必須化が不足
type が icon 等でも該当オブジェクト無しで通過してしまい、描画時に欠落し得ます。type に応じて必須化するのが安全です。
🧩 修正案(if/thenで必須化)
"size_preset": {
"type": "string",
"description": "Size preset for character display",
"enum": ["small", "medium", "large", "full-screen"],
"default": "medium"
}
- }
+ },
+ "allOf": [
+ { "if": { "properties": { "type": { "const": "icon" } } }, "then": { "required": ["icon"] } },
+ { "if": { "properties": { "type": { "const": "avatar" } } }, "then": { "required": ["avatar"] } },
+ { "if": { "properties": { "type": { "const": "image" } } }, "then": { "required": ["image"] } }
+ ]
},🤖 Prompt for AI Agents
In `@skills/generate-video/schemas/character.schema.json` around lines 122 - 255,
The schema allows appearance.type to be "avatar"/"icon"/"image" etc. without
requiring the corresponding configuration object, which can lead to missing data
at render time; add JSON Schema conditional rules (if/then) under the
"appearance" schema so that when "type" is "avatar" the "avatar" property is
required, when "type" is "icon" the "icon" property is required, when "type" is
"image" the "image" property is required (and optionally require other fields
like avatar.style or icon.icon_path if desired); update the "appearance" object
to include these if/then blocks referencing the existing "type", "avatar",
"icon", and "image" property names so callers must supply the matching config
for each type.
| "scene_id": { | ||
| "type": "string", | ||
| "description": "対象シーンのID(シナリオ内のシーンと一致する必要がある)", | ||
| "pattern": "^[a-zA-Z0-9_-]+$" | ||
| }, |
There was a problem hiding this comment.
scene_id のパターンが他スキーマと不一致です
scene / video-script では小文字 + ハイフンに限定されているため、ここだけ大文字や _ を許可すると単体検証は通っても後段で落ちる可能性があります。ID パターンを統一してください。
✅ 修正案
- "pattern": "^[a-zA-Z0-9_-]+$"
+ "pattern": "^[a-z0-9-]+$"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "scene_id": { | |
| "type": "string", | |
| "description": "対象シーンのID(シナリオ内のシーンと一致する必要がある)", | |
| "pattern": "^[a-zA-Z0-9_-]+$" | |
| }, | |
| "scene_id": { | |
| "type": "string", | |
| "description": "対象シーンのID(シナリオ内のシーンと一致する必要がある)", | |
| "pattern": "^[a-z0-9-]+$" | |
| }, |
🤖 Prompt for AI Agents
In `@skills/generate-video/schemas/direction.schema.json` around lines 10 - 14,
The scene_id property pattern allows uppercase letters and underscores which is
inconsistent with the other schemas; update the "scene_id" JSON Schema (property
name "scene_id") to use the same pattern as the other schemas (only lowercase
letters, digits and hyphens) by replacing the pattern with one that enforces
^[a-z0-9-]+$ and keep the description consistent if needed so downstream
validation won't fail.
| "emphasis": { | ||
| "effect": "pulse", | ||
| "timing": "3s" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check emphasis schema for timing field format and complete definition
cat skills/generate-video/schemas/emphasis.schema.jsonRepository: Chachamaru127/claude-code-harness
Length of output: 8146
🏁 Script executed:
#!/bin/bash
# Check the actual example JSON at lines 120-135 for context
sed -n '120,135p' skills/generate-video/schemas/examples/video-script-example.jsonRepository: Chachamaru127/claude-code-harness
Length of output: 432
emphasisオブジェクトの構造がスキーマと一致していません。
例で使用されている "effect": "pulse" と "timing": "3s" はemphasis.schema.jsonに存在しないフィールドです。スキーマでは以下のように定義されています:
"effect"フィールドは存在しません。パルス効果はanimation.pulse(真偽値) で制御します"timing"フィールドは emphasis直下には存在しません。音声タイミングが必要な場合はsound.timingを使用し、値は["start", "end", "peak"]の列挙値で指定します
例を修正して、スキーマの実際の構造に合わせてください。パルス効果を表現するには animation オブジェクトを正しく使用する必要があります。
🤖 Prompt for AI Agents
In `@skills/generate-video/schemas/examples/video-script-example.json` around
lines 125 - 128, The example's emphasis object uses invalid fields ("effect" and
"timing"); update the example to match emphasis.schema.json by removing "effect"
and "timing" under emphasis, add an animation object with animation.pulse set to
true to represent a pulse effect (e.g., emphasis.animation.pulse = true), and if
voice timing is needed move that data under sound.timing using one of the
allowed enum values ("start", "end", or "peak"); ensure references to emphasis,
animation.pulse, and sound.timing in the example match the schema names exactly.
| "colorScheme": { | ||
| "primary": "#F59E0B", | ||
| "secondary": "#EF4444", | ||
| "accent": "#10B981", | ||
| "background": "#111827" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the schema property names
if [ -f "skills/generate-video/schemas/visual-patterns.schema.json" ]; then
echo "=== Schema properties ==="
cat skills/generate-video/schemas/visual-patterns.schema.json | jq '.properties | keys' 2>/dev/null || cat skills/generate-video/schemas/visual-patterns.schema.json | head -50
else
echo "Schema file not found, searching for it..."
find . -name "*visual-patterns*.schema.json" -type f
fiRepository: Chachamaru127/claude-code-harness
Length of output: 255
🏁 Script executed:
#!/bin/bash
# Search for colorScheme vs color_scheme in schema files
echo "=== Searching for colorScheme/color_scheme ==="
rg -n "colorScheme|color_scheme" --type json
echo "=== Searching for arrowStyle/arrow_style ==="
rg -n "arrowStyle|arrow_style" --type json
echo "=== Searching for showNumbers/show_numbers ==="
rg -n "showNumbers|show_numbers" --type jsonRepository: Chachamaru127/claude-code-harness
Length of output: 1236
🏁 Script executed:
#!/bin/bash
# Check the flow-example.json file content
echo "=== flow-example.json content ==="
if [ -f "skills/generate-video/schemas/examples/visual-patterns/flow-example.json" ]; then
cat -n skills/generate-video/schemas/examples/visual-patterns/flow-example.json | head -80
else
echo "File not found at expected path"
find . -name "flow-example.json" -type f
fiRepository: Chachamaru127/claude-code-harness
Length of output: 2509
スキーマに合わせてsnake_caseに修正してください
スキーマファイル(visual-patterns.schema.json)ではsnake_case(color_scheme、arrow_style、show_numbers)を使用していますが、このサンプルファイルではcamelCase(colorScheme、arrowStyle、showNumbers)を使用しており、スキーマ検証に失敗します。
修正が必要なプロパティ:
colorScheme→color_scheme(5-10行目)arrowStyle→arrow_style(60行目)showNumbers→show_numbers(61行目)
🤖 Prompt for AI Agents
In `@skills/generate-video/schemas/examples/visual-patterns/flow-example.json`
around lines 5 - 10, JSON property names use camelCase but the schema expects
snake_case; rename the keys in the example: change "colorScheme" to
"color_scheme" (the object with primary/secondary/accent/background), change
"arrowStyle" to "arrow_style", and change "showNumbers" to "show_numbers",
updating every occurrence and preserving their values/structure so the example
conforms to visual-patterns.schema.json.
| export const VideoScriptSchema = z.object({ "metadata": z.object({ "title": z.string().min(1).max(200).describe("Video title"), "description": z.string().describe("Video description").optional(), "version": z.string().regex(new RegExp("^[0-9]+\\.[0-9]+\\.[0-9]+$")).describe("Script version"), "created_at": z.string().datetime({ offset: true }).describe("ISO 8601 timestamp of script creation"), "updated_at": z.string().datetime({ offset: true }).describe("ISO 8601 timestamp of last update").optional(), "author": z.string().describe("Script author or generator").optional(), "project": z.string().describe("Project name or identifier").optional(), "video_type": z.enum(["lp-teaser","intro-demo","release-notes","architecture","onboarding","custom"]).describe("Type of video").optional(), "tags": z.array(z.string()).describe("Categorization tags").optional(), "scenario_id": z.string().describe("Reference to the scenario this script is based on").optional() }).describe("Metadata about the video script"), "scenes": z.array(z.object({ "scene_id": z.string().regex(new RegExp("^[a-z0-9-]+$")).describe("Unique identifier for the scene"), "section_id": z.string().regex(new RegExp("^[a-z0-9-]+$")).describe("ID of the section this scene belongs to"), "order": z.number().int().gte(0).describe("Order of the scene within its section (0-indexed)"), "type": z.enum(["intro","ui-demo","architecture","code-highlight","changelog","cta","feature-highlight","problem-promise","workflow","objection","custom"]).describe("Type of scene content"), "content": z.object({ "text": z.string().describe("Primary text content for the scene").optional(), "image": z.string().describe("Path to image asset (relative or absolute)").optional(), "duration_ms": z.number().int().gte(100).describe("Duration of the scene in milliseconds"), "title": z.string().describe("Scene title").optional(), "subtitle": z.string().describe("Scene subtitle or tagline").optional(), "url": z.string().url().describe("URL for Playwright capture or external resource").optional(), "actions": z.array(z.object({ "type": z.enum(["click","type","wait","scroll","hover","keypress"]).optional(), "selector": z.string().describe("CSS selector or data-testid").optional(), "value": z.any().superRefine((x, ctx) => { | ||
| const schemas = [z.string(), z.number()]; | ||
| const errors = schemas.reduce<z.ZodError[]>( | ||
| (errors, schema) => | ||
| ((result) => | ||
| result.error ? [...errors, result.error] : errors)( | ||
| schema.safeParse(x), | ||
| ), | ||
| [], | ||
| ); | ||
| if (schemas.length - errors.length !== 1) { | ||
| ctx.addIssue({ | ||
| path: ctx.path, | ||
| code: "invalid_union", | ||
| unionErrors: errors, | ||
| message: "Invalid input: Should pass single schema", | ||
| }); | ||
| } | ||
| }).describe("Value for type actions or wait duration").optional() })).describe("Playwright actions for UI demo scenes").optional(), "mermaid": z.string().describe("Mermaid diagram definition for architecture scenes").optional(), "code": z.object({ "language": z.string().optional(), "snippet": z.string().optional(), "highlights": z.array(z.number().int()).describe("Line numbers to highlight").optional() }).describe("Code snippet for code-highlight scenes").optional() }).describe("Scene content configuration"), "direction": z.object({ "transition": z.object({ "in": z.enum(["fade","slide_in","zoom","cut"]).default("fade"), "out": z.enum(["fade","slide_in","zoom","cut"]).default("fade"), "duration_ms": z.number().int().gte(0).lte(2000).describe("Transition duration in milliseconds").default(500) }).describe("Transition effects").optional(), "emphasis": z.object({ "effect": z.enum(["glitch","pulse","shake","highlight","none"]).optional(), "timing": z.string().describe("When to apply effect (e.g., '2s', '50%')").optional() }).describe("Emphasis and highlighting").optional(), "background": z.object({ "type": z.enum(["solid","gradient","image","video","particles"]).optional(), "value": z.string().describe("Color, gradient definition, or asset path").optional(), "opacity": z.number().gte(0).lte(1).default(1) }).describe("Background configuration").optional(), "camera": z.object({ "movement": z.enum(["static","pan","zoom","rotate","parallax"]).optional(), "start": z.record(z.any()).describe("Starting position/rotation").optional(), "end": z.record(z.any()).describe("Ending position/rotation").optional() }).describe("Camera movement (3D effects)").optional() }).describe("Visual direction and effects for the scene").optional(), "assets": z.array(z.object({ "type": z.enum(["image","video","audio","font","data"]), "source": z.string().describe("Path or URL to the asset"), "generated": z.boolean().describe("Whether this asset was AI-generated").default(false), "metadata": z.object({ "width": z.number().int().optional(), "height": z.number().int().optional(), "duration_ms": z.number().int().optional(), "format": z.string().optional() }).describe("Additional metadata about the asset").optional() })).describe("Assets used in this scene").optional(), "audio": z.object({ "narration": z.object({ "text": z.string().describe("Narration text").optional(), "file": z.string().describe("Path to narration audio file").optional(), "start_offset_ms": z.number().int().describe("Delay before narration starts (typically 1000ms)").default(1000) }).optional(), "sfx": z.array(z.object({ "file": z.string().optional(), "timing_ms": z.number().int().optional(), "volume": z.number().gte(0).lte(1).optional() })).describe("Sound effects").optional() }).describe("Audio configuration for the scene").optional(), "template": z.string().describe("Template name if using a predefined template").optional(), "notes": z.string().describe("Internal notes for this scene").optional() }).describe("Defines an individual video scene with content, direction, and assets")).min(1).describe("Ordered list of video scenes"), "total_duration_ms": z.number().int().gte(0).describe("Total duration of the video in milliseconds"), "output_settings": z.object({ "width": z.number().int().gte(320).lte(7680).describe("Video width in pixels"), "height": z.number().int().gte(240).lte(4320).describe("Video height in pixels"), "fps": z.union([z.literal(24), z.literal(25), z.literal(30), z.literal(50), z.literal(60)]).describe("Frames per second").default(30), "codec": z.enum(["h264","h265","vp8","vp9","av1"]).describe("Video codec").default("h264"), "format": z.enum(["mp4","webm","mov","gif"]).describe("Output format").default("mp4"), "quality": z.enum(["low","medium","high","ultra"]).describe("Output quality preset").default("high"), "bitrate": z.string().describe("Video bitrate (e.g., '5M', '10M')").optional(), "preset": z.enum(["240p","360p","480p","720p","1080p","1440p","4k","custom"]).describe("Encoding preset for resolution").optional() }).describe("Video output configuration"), "audio_settings": z.object({ "bgm": z.object({ "file": z.string().describe("Path to BGM file").optional(), "volume": z.number().gte(0).lte(1).describe("BGM volume (0.0 - 1.0)").default(0.3), "fade_in_ms": z.number().int().gte(0).describe("Fade in duration in milliseconds").default(1000), "fade_out_ms": z.number().int().gte(0).describe("Fade out duration in milliseconds").default(2000), "loop": z.boolean().describe("Whether to loop the BGM").default(true) }).describe("Background music configuration").optional(), "master_volume": z.number().gte(0).lte(1).describe("Master volume (0.0 - 1.0)").default(1) }).describe("Global audio settings").optional(), "branding": z.object({ "logo": z.string().describe("Path to logo file").optional(), "colors": z.object({ "primary": z.string().describe("Primary brand color (hex or rgb)").optional(), "secondary": z.string().describe("Secondary brand color").optional(), "accent": z.string().describe("Accent color").optional(), "background": z.string().describe("Default background color").optional(), "text": z.string().describe("Default text color").optional() }).optional(), "fonts": z.object({ "primary": z.string().describe("Primary font family or path to font file").optional(), "secondary": z.string().describe("Secondary font family").optional(), "monospace": z.string().describe("Monospace font for code").optional() }).optional() }).describe("Branding configuration").optional(), "transitions": z.object({ "default_duration_ms": z.number().int().gte(0).lte(2000).describe("Default transition duration in milliseconds").default(500), "overlap_ms": z.number().int().gte(0).lte(1000).describe("Overlap duration between scenes (for cross-fade)").default(450), "type": z.enum(["fade","slide_in","zoom","cut"]).describe("Default transition type").default("fade") }).describe("Global transition settings").optional(), "notes": z.string().describe("General notes about the video script").optional() }).describe("Complete video script with metadata, scenes, and output settings") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n skills/generate-video/src/schemas/video-script.ts | head -60Repository: Chachamaru127/claude-code-harness
Length of output: 10041
🏁 Script executed:
rg -A 10 -B 5 "actions" skills/generate-video/src/schemas/video-script.tsRepository: Chachamaru127/claude-code-harness
Length of output: 9185
🏁 Script executed:
find . -name "*.json" -o -name "*.ts" | xargs grep -l "actions" | head -10Repository: Chachamaru127/claude-code-harness
Length of output: 497
🏁 Script executed:
find . -name "video-script.schema.json" -exec head -200 {} +Repository: Chachamaru127/claude-code-harness
Length of output: 5764
🏁 Script executed:
rg -A 20 '"actions"' skills/generate-video/schemas/Repository: Chachamaru127/claude-code-harness
Length of output: 3682
🏁 Script executed:
cat skills/generate-video/schemas/scene.schema.json | grep -A 30 '"actions"'Repository: Chachamaru127/claude-code-harness
Length of output: 1092
🏁 Script executed:
grep -B 5 -A 25 '"type": "object"' skills/generate-video/schemas/scene.schema.json | grep -A 25 '"actions"'Repository: Chachamaru127/claude-code-harness
Length of output: 890
🏁 Script executed:
cat skills/generate-video/schemas/scene.schema.json | jq '.properties.content.properties.actions.items' | head -50Repository: Chachamaru127/claude-code-harness
Length of output: 599
🏁 Script executed:
rg -B 5 -A 35 '"actions".*:.*{' skills/generate-video/schemas/scene.schema.jsonRepository: Chachamaru127/claude-code-harness
Length of output: 170
🏁 Script executed:
cat skills/generate-video/schemas/scene.schema.json | jq '.properties.content.properties.actions.items | keys'Repository: Chachamaru127/claude-code-harness
Length of output: 105
🏁 Script executed:
rg -A 5 -B 5 'actions\[' skills/generate-video/src/ --type tsRepository: Chachamaru127/claude-code-harness
Length of output: 59
🏁 Script executed:
find skills/generate-video -name "*.ts" -type f | xargs grep -l "action" | head -5Repository: Chachamaru127/claude-code-harness
Length of output: 169
🏁 Script executed:
cat skills/generate-video/schemas/scene.schema.json | jq '.properties.content.properties.actions.items | has("required")'Repository: Chachamaru127/claude-code-harness
Length of output: 82
🏁 Script executed:
cat skills/generate-video/schemas/scene.schema.json | jq '.properties.content.properties.actions.items'Repository: Chachamaru127/claude-code-harness
Length of output: 599
🏁 Script executed:
rg -A 50 'properties.*content' skills/generate-video/schemas/scene.schema.json | head -100Repository: Chachamaru127/claude-code-harness
Length of output: 59
🏁 Script executed:
cat skills/generate-video/schemas/scene.schema.json | jq '.properties.content.properties.actions'Repository: Chachamaru127/claude-code-harness
Length of output: 751
🏁 Script executed:
rg -i "downstream\|action" skills/generate-video/src/ --type ts -A 3 | head -40Repository: Chachamaru127/claude-code-harness
Length of output: 59
🏁 Script executed:
find skills/generate-video -name "*.ts" -o -name "*.js" | xargs grep -l "actions" | grep -v node_modules | head -10Repository: Chachamaru127/claude-code-harness
Length of output: 219
🏁 Script executed:
find skills/generate-video -name "*.ts" -type f | xargs grep -B 3 -A 8 "\.actions" | head -80Repository: Chachamaru127/claude-code-harness
Length of output: 59
🏁 Script executed:
cat skills/generate-video/schemas/validate-schemas.jsRepository: Chachamaru127/claude-code-harness
Length of output: 6787
🏁 Script executed:
find skills/generate-video -name "*.ts" -type f | xargs grep -B 5 -A 8 "action" | grep -v "z\.object\|z\.enum\|z\.string\|z\.number" | head -100Repository: Chachamaru127/claude-code-harness
Length of output: 3577
actions 配列の要素がすべてオプションで、空オブジェクト {} が妥当として通ります
type、selector、value がすべてオプションのため、検証段階では空オブジェクトが許可され、後続の処理で解釈不能になります。少なくとも type を必須化し、必要に応じて type 値に応じた条件付き必須フィールドを追加して、生成元の JSON Schema(skills/generate-video/schemas/scene.schema.json)の actions items 定義に "required" を追加してから npm run generate:schemas で再生成してください。
🤖 Prompt for AI Agents
In `@skills/generate-video/src/schemas/video-script.ts` around lines 15 - 33, The
actions item currently allows an empty object because "type", "selector", and
"value" are all optional; make "type" required by removing .optional() on the
"type" enum inside the actions z.object, then add conditional validation (e.g.,
in the same actions z.object or its existing superRefine) to require "selector"
for action types that need a selector (click, hover, scroll, type, keypress) and
to require "value" for action types that need a value (type requires a string,
wait requires a number), ensuring type-specific shapes are enforced; also update
the source JSON Schema (skills/generate-video/schemas/scene.schema.json) to add
the appropriate "required" entries for actions items and then run npm run
generate:schemas to regenerate the Zod schema.
| "scene_id": "demo-part1", | ||
| "section_id": "demo", | ||
| "order": 2, | ||
| "type": "ui-demo", | ||
| "content": { | ||
| "text": "{{DEMO_PART1_DESCRIPTION}}", | ||
| "duration_ms": 40000, | ||
| "url": "{{APP_URL}}/{{DEMO_PATH}}", | ||
| "actions": "{{DEMO_PART1_ACTIONS}}" | ||
| }, | ||
| "direction": { | ||
| "transition": { | ||
| "in": "fade", | ||
| "out": "fade", | ||
| "duration_ms": 500 | ||
| } | ||
| }, | ||
| "assets": [], | ||
| "audio": { | ||
| "narration": { | ||
| "text": "{{DEMO_PART1_NARRATION}}", | ||
| "file": "{{AUDIO_DIR}}/demo-part1.mp3", | ||
| "start_offset_ms": 1000 | ||
| } | ||
| }, | ||
| "template": "ui-demo", | ||
| "notes": "Demo Part 1: First part of the workflow. 40 seconds." | ||
| }, | ||
| { | ||
| "scene_id": "demo-part2", | ||
| "section_id": "demo", | ||
| "order": 3, | ||
| "type": "ui-demo", | ||
| "content": { | ||
| "text": "{{DEMO_PART2_DESCRIPTION}}", | ||
| "duration_ms": 40000, | ||
| "url": "{{APP_URL}}/{{DEMO_PATH}}", | ||
| "actions": "{{DEMO_PART2_ACTIONS}}" | ||
| }, | ||
| "direction": { | ||
| "transition": { | ||
| "in": "fade", | ||
| "out": "fade", | ||
| "duration_ms": 500 | ||
| } | ||
| }, | ||
| "assets": [], | ||
| "audio": { | ||
| "narration": { | ||
| "text": "{{DEMO_PART2_NARRATION}}", | ||
| "file": "{{AUDIO_DIR}}/demo-part2.mp3", | ||
| "start_offset_ms": 1000 | ||
| } | ||
| }, | ||
| "template": "ui-demo", | ||
| "notes": "Demo Part 2: Middle part showing key interactions. 40 seconds." | ||
| }, | ||
| { | ||
| "scene_id": "demo-part3", | ||
| "section_id": "demo", | ||
| "order": 4, | ||
| "type": "ui-demo", | ||
| "content": { | ||
| "text": "{{DEMO_PART3_DESCRIPTION}}", | ||
| "duration_ms": 30000, | ||
| "url": "{{APP_URL}}/{{DEMO_PATH}}", | ||
| "actions": "{{DEMO_PART3_ACTIONS}}" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find schema files related to generate-video
find skills/generate-video -name "*schema*" -type f | head -20Repository: Chachamaru127/claude-code-harness
Length of output: 816
🏁 Script executed:
# Check the actual template file content at the specified lines
sed -n '79,146p' skills/generate-video/templates/intro-3min.json | head -100Repository: Chachamaru127/claude-code-harness
Length of output: 1897
🏁 Script executed:
# Also check lines 280-288 mentioned in "Also applies to"
wc -l skills/generate-video/templates/intro-3min.json
sed -n '280,288p' skills/generate-video/templates/intro-3min.jsonRepository: Chachamaru127/claude-code-harness
Length of output: 675
🏁 Script executed:
# Search for schema definitions related to actions field
rg '"actions"' skills/generate-video --type json -C 3Repository: Chachamaru127/claude-code-harness
Length of output: 3532
🏁 Script executed:
# Check the complete schema definition for actions field
cat skills/generate-video/schemas/scene.schema.json | grep -A 20 '"actions"'Repository: Chachamaru127/claude-code-harness
Length of output: 804
🏁 Script executed:
# Check the example file to see the correct structure
cat skills/generate-video/schemas/examples/video-script-example.json | grep -A 10 '"actions"'Repository: Chachamaru127/claude-code-harness
Length of output: 327
🏁 Script executed:
# Check if there's any template substitution or validation code
rg 'DEMO_PART.*_ACTIONS' skills/generate-video --type json -B 2 -A 2Repository: Chachamaru127/claude-code-harness
Length of output: 2458
content.actions がスキーマと一致していない型構造になっている。
スキーマ(scene.schema.json)では actions は配列型で、各要素がオブジェクト(type, selector, value などを持つ)として定義されていますが、テンプレート(intro-3min.json)では "actions": "{{DEMO_PART1_ACTIONS}}" として文字列で定義されています。テンプレート変数の値("DEMO_PART1_ACTIONS": "Playwright actions for first part")も単なる文字列であるため、置換後はスキーマバリデーション失敗となります。
DEMO_PART1/2/3_ACTIONS をJSON配列形式で定義するか、テンプレート置換時に JSON 文字列を再パース(または JSON 断片を直接注入)する仕組みが必要です。
🤖 Prompt for AI Agents
In `@skills/generate-video/templates/intro-3min.json` around lines 79 - 146, The
template intro-3min.json currently sets content.actions as a string ("actions":
"{{DEMO_PART1_ACTIONS}}"), which violates the scene.schema.json that requires
actions to be an array of objects; fix by making DEMO_PART1_ACTIONS /
DEMO_PART2_ACTIONS / DEMO_PART3_ACTIONS provide a JSON array (e.g., [{"type":
"...","selector":"...","value":"..."}]) or change the template substitution
logic to parse the variable as JSON and inject the resulting array into
content.actions (ensure the substitution code recognizes and inserts a parsed
array rather than a quoted string), updating all occurrences (content.actions)
in the file to match the schema.
| describe('Flow Pattern', () => { | ||
| test('validates valid flow pattern', () => { | ||
| const validate = ajv.compile(schema); | ||
| const validFlow = { | ||
| type: 'flow', | ||
| topic: 'Video Generation Flow', | ||
| style: 'modern', | ||
| flow: { | ||
| steps: [ | ||
| { | ||
| id: 'analyze', | ||
| label: 'Analyze Codebase', | ||
| description: 'Detect project structure', | ||
| order: 1, | ||
| type: 'start', | ||
| icon: 'circle', | ||
| duration: '10 seconds' | ||
| }, | ||
| { | ||
| id: 'plan', | ||
| label: 'Generate Scenario', | ||
| order: 2, | ||
| type: 'process', | ||
| icon: 'square', | ||
| duration: '20 seconds' | ||
| }, | ||
| { | ||
| id: 'render', | ||
| label: 'Render Video', | ||
| order: 3, | ||
| type: 'end', | ||
| icon: 'hexagon', | ||
| duration: '30 seconds' | ||
| } | ||
| ], | ||
| direction: 'horizontal', | ||
| arrow_style: 'solid', | ||
| show_numbers: true | ||
| } | ||
| }; | ||
|
|
||
| const result = validate(validFlow); | ||
| if (!result) { | ||
| console.error('Validation errors:', validate.errors); | ||
| } | ||
| expect(result).toBe(true); | ||
| }); | ||
|
|
||
| test('rejects flow with less than 2 steps', () => { | ||
| const validate = ajv.compile(schema); | ||
| const invalid = { | ||
| type: 'flow', | ||
| topic: 'Test', | ||
| flow: { | ||
| steps: [ | ||
| { id: 'single', label: 'Single Step', order: 1 } | ||
| ] | ||
| } | ||
| }; | ||
|
|
||
| expect(validate(invalid)).toBe(false); | ||
| }); | ||
|
|
||
| test('validates different flow directions', () => { | ||
| const validate = ajv.compile(schema); | ||
|
|
||
| ['horizontal', 'vertical', 'zigzag'].forEach(direction => { | ||
| const valid = { | ||
| type: 'flow', | ||
| topic: 'Test', | ||
| flow: { | ||
| steps: [ | ||
| { id: 'a', label: 'A', order: 1 }, | ||
| { id: 'b', label: 'B', order: 2 } | ||
| ], | ||
| direction | ||
| } | ||
| }; | ||
|
|
||
| expect(validate(valid)).toBe(true); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check flow properties in the schema file
echo "=== Checking schema file ==="
rg -n "arrow_style|arrowStyle|show_numbers|showNumbers" skills/generate-video/schemas/visual-patterns.schema.json
echo -e "\n=== Looking for flow-example.json ==="
fd "flow-example" skills/generate-video/
echo -e "\n=== Checking example file content ==="
if [ -f "skills/generate-video/examples/flow-example.json" ]; then
cat skills/generate-video/examples/flow-example.json
fiRepository: Chachamaru127/claude-code-harness
Length of output: 315
🏁 Script executed:
#!/bin/bash
# Read the flow-example.json file
cat -n skills/generate-video/schemas/examples/visual-patterns/flow-example.json
echo -e "\n=== Checking for camelCase properties in the example ==="
rg -n "arrowStyle|showNumbers" skills/generate-video/schemas/examples/visual-patterns/flow-example.json
echo -e "\n=== Checking for snake_case properties in the example ==="
rg -n "arrow_style|show_numbers" skills/generate-video/schemas/examples/visual-patterns/flow-example.jsonRepository: Chachamaru127/claude-code-harness
Length of output: 2649
flow-example.jsonの命名規則がスキーマと一致していません
スキーマ定義ではarrow_style、show_numbers(snake_case)を使用していますが、flow-example.jsonではarrowStyle、showNumbers(camelCase)が使用されています。例ファイルをスキーマの定義に合わせて修正してください。
🤖 Prompt for AI Agents
In `@skills/generate-video/tests/visual-patterns.test.js` around lines 196 - 278,
The example flow JSON uses camelCase keys that don't match the schema; update
the example file (flow-example.json) so the flow object uses snake_case keys
arrow_style and show_numbers (instead of arrowStyle and showNumbers) to match
the schema used by the tests/validator (see the validFlow / flow object in
visual-patterns.test.js).
- Add path traversal validation in loadAssetFile (load-assets.js) - Add whitelist + path character check in loadTemplateFile (template-registry.js) - Fix semantic error filtering in validate-scenario.js (was always empty) - Align TransitionType enum to snake_case (slideIn → slide_in) All 184 tests passing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Resolved version conflict: 2.16.21 (branch) vs 2.17.3 (main) → 2.17.4 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@skills/generate-video/scripts/load-assets.js`:
- Around line 286-324: The updateManifest function is vulnerable to path
traversal via the manifestPath parameter and can throw if the loaded manifest
lacks an assets array; sanitize and constrain manifestPath the same way
loadAssetFile does by resolving it and verifying it sits inside an allowed base
directory (e.g., join a known assets root and ensure
path.startsWith(rootResolved)), reject or throw on invalid paths, then ensure
the loaded manifest has an assets array before pushing (e.g., if
(!Array.isArray(manifest.assets)) manifest.assets = []), and keep the existing
behavior of creating the directory and writing the file.
🧹 Nitpick comments (3)
skills/generate-video/remotion/components/TransitionWrapper.tsx (2)
79-79: 未使用の変数widthとheightを削除してください。
useVideoConfig()から分割代入されたwidthとheightはこのコンポーネント内で使用されていません。不要な変数は削除することでコードの明確さが向上します。♻️ 修正案
- const { fps, width, height } = useVideoConfig(); + const { fps } = useVideoConfig();
19-20:useSpring使用時にdurationが無視される点をドキュメントに追記してください。
useSpring: trueの場合、durationプロパティは無視され、springConfigの物理パラメータによってアニメーション時間が決定されます。この挙動を JSDoc に明記することで、API 利用者の混乱を防げます。📝 修正案
- /** Duration of the transition in frames */ + /** Duration of the transition in frames (ignored when useSpring is true) */ duration?: number;skills/generate-video/scripts/template-registry.js (1)
135-164:hasOwnPropertyを安全な呼び出しにして堅牢化を推奨します。
variablesにhasOwnPropertyが無い/上書きされているケースで例外になり得るため、Object.prototype.hasOwnProperty.callへ置き換えると安全です。♻️ 修正案
- if (variables.hasOwnProperty(varName)) { + if (Object.prototype.hasOwnProperty.call(variables, varName)) {
| function updateManifest(manifestPath, assets) { | ||
| try { | ||
| console.log('📝 Updating asset manifest...'); | ||
|
|
||
| // Load existing manifest or create new | ||
| let manifest = loadJSON(manifestPath) || { | ||
| version: '1.0.0', | ||
| generated_at: new Date().toISOString(), | ||
| project: { | ||
| name: 'Unknown', | ||
| video_id: 'video-' + Date.now() | ||
| }, | ||
| assets: [] | ||
| }; | ||
|
|
||
| // Add new assets | ||
| if (Array.isArray(assets)) { | ||
| manifest.assets.push(...assets); | ||
| } else { | ||
| manifest.assets.push(assets); | ||
| } | ||
|
|
||
| // Update timestamp | ||
| manifest.generated_at = new Date().toISOString(); | ||
|
|
||
| // Write manifest | ||
| const manifestDir = path.dirname(manifestPath); | ||
| if (!fs.existsSync(manifestDir)) { | ||
| fs.mkdirSync(manifestDir, { recursive: true }); | ||
| } | ||
| fs.writeFileSync(manifestPath, JSON.stringify(manifest, null, 2)); | ||
|
|
||
| console.log(' ✅ Manifest updated:', manifestPath); | ||
| return true; | ||
| } catch (error) { | ||
| console.error(' ❌ Failed to update manifest:', error.message); | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
manifestPath にパストラバーサル対策が必要です。
loadAssetFile と同様に、manifestPath パラメータが外部から渡される可能性があり、パストラバーサル攻撃により任意のファイルパスへの書き込みが可能になります。また、既存のマニフェストに assets プロパティがない場合、Line 303 で実行時エラーが発生します。
🔒️ 修正案
function updateManifest(manifestPath, assets) {
try {
console.log('📝 Updating asset manifest...');
+ // Path traversal protection - ensure path is within expected directories
+ const resolvedPath = path.resolve(manifestPath);
+ const skillBase = path.resolve(SKILL_ROOT) + path.sep;
+ const userBase = path.resolve(os.homedir(), '.harness') + path.sep;
+ if (!resolvedPath.startsWith(skillBase) && !resolvedPath.startsWith(userBase)) {
+ console.error('❌ Manifest path must be within skill or user directory');
+ return false;
+ }
+
// Load existing manifest or create new
let manifest = loadJSON(manifestPath) || {
version: '1.0.0',
generated_at: new Date().toISOString(),
project: {
name: 'Unknown',
video_id: 'video-' + Date.now()
},
assets: []
};
+ // Ensure assets array exists
+ if (!Array.isArray(manifest.assets)) {
+ manifest.assets = [];
+ }
+
// Add new assets
if (Array.isArray(assets)) {
manifest.assets.push(...assets);🤖 Prompt for AI Agents
In `@skills/generate-video/scripts/load-assets.js` around lines 286 - 324, The
updateManifest function is vulnerable to path traversal via the manifestPath
parameter and can throw if the loaded manifest lacks an assets array; sanitize
and constrain manifestPath the same way loadAssetFile does by resolving it and
verifying it sits inside an allowed base directory (e.g., join a known assets
root and ensure path.startsWith(rootResolved)), reject or throw on invalid
paths, then ensure the loaded manifest has an assets array before pushing (e.g.,
if (!Array.isArray(manifest.assets)) manifest.assets = []), and keep the
existing behavior of creating the directory and writing the file.
Run build-opencode.js to sync with latest skill changes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Added to .gitignore: - **/skills/**/node_modules/ - **/skills/**/coverage/ - **/skills/**/.claude/state/ - **/skills/**/.claude/logs/ - **/skills/**/.claude/sessions/ Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Added exclusion patterns for: - node_modules/ - coverage/ - .claude/ (state, logs, sessions) - IMPLEMENTATION_*.md - TASK_*.md Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🎯 What's Changed for You
/generate-videoが「ワークフロー記述」から「実行可能な基盤」に進化しましたBefore → After
Added
Phase 0-3: 基盤・スキーマ・バリデーション
scripts/generate-schemas.jsPhase 4-5: 演出・コンポーネント
EmphasisBox- グロー、パルス、アウトライン効果TransitionWrapper- fade, slide_in, zoom, cutBackgroundLayer- solid, gradient, image, video, particlesProgressIndicator- bar, circle, dotsPhase 6-7: 画像パターン・アセット
templates/image-prompts/Phase 8-9: レンダリング・テンプレート
Phase 10-11: 将来拡張・アーキテクチャ改善
Codex Review Results
Test Coverage
Requirements
/remotion-setup)🤖 Generated with Claude Code
Summary by CodeRabbit
新機能
改善
テスト
Chores