feat(dry-run): re-land the plan work that never reached main (#24) - #26
Merged
Conversation
--- a/dry-run reported nothing usable
summarise() only accumulated bytes for results with status "compressed", and
every dry-run result is a skip, so the summary came back all zeros:
$ imgvidcompress . --dry-run --json | jq .summary
{ "totalFiles": 1, "skipped": 1, "inputBytes": 0, "savedBytes": 0, ... }
The README recipe built on that printed zeros. Its premise was also wrong:
savings cannot be known without encoding. The summary gains additive `planned`
and `plannedInputBytes` counts, the existing byte fields keep meaning actual
encoded totals, and no savings figure is ever invented for a plan.
--- b/it could not tell you the run would fail
run() skipped ffmpeg resolution entirely when dryRun was set, so a machine
with no ffmpeg, or an ffmpeg missing the requested codec, reported a clean
plan and then died on the real run. Dry runs now resolve ffmpeg and go through
the codec preflight whenever video is planned.
This is an intentional behaviour change: a dry run that cannot be executed now
exits 3 instead of 0. A plan that cannot run is not a passing plan.
--- c/it misreported what would be skipped
executeJob() checked dryRun before the output-exists branch, so every file came
back reason "dry-run" even when a real run would have skipped it as
"output-exists". Output-exists is now determined first. Both values were
already in the documented SkipReason enum.
Results also carry the resolved targetFormat, videoCodec and audioCodec, and
dry runs now probe each source so the plan reports the same stream drops the
real run would emit. Probing makes a video dry run slower; accuracy is the
whole point of a plan, and "will this lose my subtitles" is the most valuable
thing it can answer before anything is written.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-lands #24, which never reached
main.What happened
#24 was a stacked PR with its base set to
feat/codec-parityrather thanmain. Merging #23 into main squash-createde51b06f, and #24 was merged17 seconds later — into
feat/codec-parity, its own base branch, not intomain. Both PRs show MERGED, but only #23's content actually landed.
The dry-run work has been sitting on
feat/codec-parityat86476b2eversince:
What this PR is
86476b2cherry-picked onto current main.main's tree was alreadybyte-identical to
6501f38(#23 squashed cleanly), so the replay wasconflict-free, and the resulting tree is identical to what was merged into
feat/codec-parity:No new work, no re-review of the substance needed — the content is exactly
what was approved in #24. Its description still applies in full.
Verified on this branch
202 tests pass.
typecheck(both packages) /lint/format:check/build/build:mcpclean.After merging
feat/codec-paritycan be deleted — everything on it is then on main.🤖 Generated with Claude Code