Conversation
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7e96b9406
ℹ️ 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 files = (await fs.readdir(params.outputDir)) | ||
| .filter((entry) => /^frame-\d+\.png$/i.test(entry)) | ||
| .sort((left, right) => left.localeCompare(right, undefined, { numeric: true })); |
There was a problem hiding this comment.
Clear stale frame artifacts before reading extracted frames
When diff video is run with an existing --out directory, this code reads every frame-*.png in that folder after extraction, but the folder is never cleaned first. If a previous run produced more frames than the current run, leftover PNGs are included in files, so transition analysis uses stale frames and reports incorrect timing/content. This is reproducible by running diff video twice against the same output dir with different --max-frames or video lengths.
Useful? React with 👍 / 👎.
Summary
Add transition summaries for visual frame sequences and recordings via
diff framesanddiff video.diff framesaccepts a PNG directory or explicit PNG frame paths, whilediff videouses externalffmpeg/ffprobeto sample recordings into frames without adding JS dependencies. The summarizer reuses screenshot diff region/OCR hints only on selected transition boundaries, supports gesture telemetry labels such asafter tap, and writes keyframe/diff artifacts under--out.Example from a synthetic Settings-like frame sequence:
diff videoprerequisite behavior when FFmpeg is missing:Touched-file count: 10. Scope stayed within the diff/media command family plus docs/skill guidance.
Validation
pnpm formatpnpm check:quickpnpm vitest run src/utils/__tests__/transition-summary.test.ts src/__tests__/cli-diff.test.ts src/utils/__tests__/cli-option-schema.test.tspnpm test:smokepnpm buildgit diff --checkpnpm ad diff frames <frames-dir> --out <out-dir> --telemetry <telemetry.json> --threshold 0pnpm ad diff video /tmp/missing.mp4 --out /tmp/transition-video-outin an environment without FFmpeg/FFprobe to verify the prerequisite errorKnown gap: full
pnpm check:unit/all-unit Vitest runs were attempted, but the tool session was terminated before Vitest printed a final summary; targeted unit/CLI tests, smoke tests, typecheck/lint, and build passed.