Loading
Review a change to the ffmpeg-skill repository for the failures its own contract makes possible — a claim in a result document that is true at one layer and false at the layer a caller reads, a new fl
The skill file stays in the publisher's repository.
Paste the copied command in a terminal. It installs this SKILL.md into that agent.
npx skills add kajisho5/ffmpeg-skill --skill code-review -a cursor -y
Review a change to the ffmpeg-skill repository for the failures its own contract makes possible — a claim in a result document that is true at one layer and false at the layer a caller reads, a new flag that reaches the code but not the contract/docs/demo surfaces, a "bug" that docs/design-decisions.md already decided with a pinning test, a fix with no regression test, a new runtime dependency or a raw ffmpeg shell call that breaks the scope boundary, a SKILL.md line added without one trimmed, a review that re-derives findings from a summary instead of the tree at the reviewed commit. Use when asked to review a PR or diff in this repository, when authoring code here and needing a pre-commit check, or when triaging an external review or audit report about this codebase.
Review a change to the ffmpeg-skill repository for the failures its own contract makes possible — a claim in a result document that is true at one layer and false at the layer a caller reads, a new flag that reaches the code but not the contract/docs/demo surfaces, a "bug" that docs/design-decisions.md already decided with a pinning test, a fix with no regression test, a new runtime dependency or a raw ffmpeg shell call that breaks the scope boundary, a SKILL.md line added without one trimmed, a review that re-derives findings from a summary instead of the tree at the reviewed commit. Use when asked to review a PR or diff in this repository, when authoring code here and needing a pre-commit check, or when triaging an external review or audit report about this codebase.
This repository has an unusual failure mode, and a review that misses it looks
thorough while catching nothing. The product is a JSON result document that an
agent reads instead of watching the video. A change can be correct at the FFmpeg
layer, correct at the Python layer, and still ship a lie at the layer that
matters: status: "completed" next to a non-zero exit, verified: true for
something the tool never measured, a cut described as lossless when the keyframe
snap moved it 1.24 s. The 0.9.1/0.10.0 "honesty fix" (cut.py's mode /
keyframe_snapped / duration_delta_seconds, check.py's reason, render.py's
check-stage exit code) exists because three such claims shipped and were caught
after the fact. Review the claim, not just the code.
The second failure mode is re-reporting decisions. Three review rounds on
2026-09-12 re-reported entries in docs/design-decisions.md, which is why that
file exists.
Read the tree at the commit under review, not a summary of it and not your own working copy if it has moved.
git fetch origin && git log --oneline -1 origin/main
git diff --stat origin/main...HEAD # what the branch actually changes
git show origin/main:docs/design-decisions.md | head -40
At least one earlier review reported committed media that is not in git and colour flags that were already validated — both came from reading a summary rather than the tree. Before reporting any finding, name the file and line you read it at, and confirm the symbol still exists there.
For every writing tool touched, trace the success document it prints and ask whether each field is something the tool measured.
| field | the review question |
|---|---|
status | can this be completed while the exit code is non-zero? (die() must set failed) |
verified | is it the conjunction of steps the tool actually ran, or an assumption? |
reencodes_* | was a copy fallback taken and reported, or silently assumed not taken? |
dropped_non_av_streams | did the timeline move (track must be dropped, true) or stay (kept, false)? |
keyframe_snapped, duration_delta_seconds | for cut: is the measured divergence reported, not rounded away? |
mode | does it name the path actually taken (copy vs re-encode)? |
error.kind | input / ffmpeg / output / missing_tool / timeout / verification / interrupted — is it the one that happened? |
error.retryable | must stay false; a true here invites a blind retry loop |
--dry-run claims matter too: info() rewrites wrote X to [dry-run] would write X, and analysis_only tools (probe, check, sync, multicam,
scenes, cropdetect, report, silence, loudness, stabilize) do run
FFmpeg to measure. A change that makes a writing tool print a write claim under
dry-run is a defect.
See references/result-honesty.md for the code pointers and the exact test names
that pin each of these.
Grep docs/design-decisions.md before writing any "this looks wrong" sentence.
Each entry names the rationale and the test that pins it. If the change under
review is about one of them, the report must say which sentence there no longer
holds — otherwise it is a re-report and will be closed as one.
Also grep before claiming a gap, because several checks are centralised:
grep -rn "validate_color(" scripts/ # colour flags are validated at tool level, not per call site
grep -rn "apply_common()" scripts/ # --quality range check happens once, here
grep -rn "time_arg(" scripts/ # the single time parser; a tool parsing time itself is the bug
A change that is correct in isolation can still be unshippable here. Verify each applicable rule against the tree:
python3 scripts/_contract.py --json is the source of truth
for tool names, flags, dry-run semantics and error kinds. README.md and
SKILL.md restate it and are tested against it. A new flag that exists only in
a script is half shipped; extend the generator, never hand-duplicate a schema.feat updates README.md (tool
table, contract table, gotchas), SKILL.md, references/scripts.md,
docs/contract.md and CHANGELOG.md. See references/surfaces.md.SKILL.md stays under 30,000 bytes — enforced by
test_skill_md_stays_under_the_30kb_budget, not a convention. Adding a line
means trimming one, and the PR should say which.scripts/ must
appear in some demo's command line (a test asserts it); a feat adds a
before/after entry to demos/build.py and regenerates docs/demos.md with
python3 demos/build.py --docs. Tools whose whole output is a table/JSON/HTML
go in INSPECTION instead.CONTRIBUTING.md is explicit: a
fix without a test that would have caught the original bug is not finished.ffmpeg/ffprobe on PATH.ffmpeg/ffprobe shell invocation outside scripts/*.py, no mutation of input
files, no creative decisions on the caller's behalf.hdr_signal did before 2.0). A PR that changes the
meaning of an existing field in a minor or patch release is a finding.npm test # tests/test_all.py + tests/test_contract.py
npm run release-check # packaging + installer + MCP + doctor + full suite
python3 scripts/_contract.py doctor # what this machine can actually run
CI is a three-OS matrix (Linux/macOS/Windows) across FFmpeg 5.1, 6.1, 7.1 and
8.x/9.x. A change that passes locally on one FFmpeg major is not verified. Report
a red check as its own statement — never a parenthetical under a "done" claim —
and confirm green after the run finishes rather than predicting it. See
reproducing-ci-locally in this repo's .claude/skills/.
Two traps specific to reviewing here:
references/process-pitfalls.md is a maintainer diary and is not in the npm
package; the other reference files are. Packaging claims belong against
package.json files and bin/install.js PAYLOAD.Order findings by what they cost: a false claim in a result document first, then a missing surface or test, then style. For each, give file and line at the reviewed commit, the concrete failure it causes, and the smallest fix. State plainly which checks you ran and what you did not verify — an unrun check reported as passing is the same defect as an overstated result document, one artifact over. Defer deep security audits to a dedicated security pass; this skill reviews quality, scope and claim-honesty.
references/result-honesty.md — the JSON contract, the fields that carry
claims, and the test that pins each one.references/surfaces.md — the full surface inventory a change must touch,
and how to check for drift.references/review-checklist.md — the runnable pre-commit and pre-merge
checklist with exact commands.This skill is repo-local and intentionally duplicates none of
writing-defect-reports, verifying-external-behavior or
reproducing-ci-locally (all in .claude/skills/); it points at them instead.
Its distinguishing subject is the review of a claim — the emit()/die()
document — which is where this codebase's shipped defects have actually come from.