Repository navigation
feat: add prepare image cli - #58
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR introduces a new Possibly related PRs
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/integration/prepare_image.integration.test.ts`:
- Around line 19-36: The test "exports a PNG as a JPEG and prints the actual
output path" runs FFmpeg/FFprobe and needs an explicit 30s timeout; update the
test to use a 30_000 ms timeout by passing 30000 as the third argument to the
test(...) call (or call jest.setTimeout(30000) at the top of the file) so that
the async test using runPrepareImageCli and inspectMedia has a 30-second limit
instead of the default 5s.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 48e400f8-5c82-4867-9842-4d336ed4b783
📒 Files selected for processing (7)
docs/plans/v1-prepare-image-cli.mdpackage.jsonsrc/cli/prepare_image.tssrc/domain/prepare_image.tstests/helpers/cli.tstests/integration/cli.integration.test.tstests/integration/prepare_image.integration.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{js,ts,jsx,tsx}: Prioritize using plain JavaScript/TypeScript instead of libraries for fundamental algorithms (e.g., use Array methods instead of lodash)
Use functional programming patterns and immutable data structures in JavaScript/TypeScript code
Files:
tests/integration/cli.integration.test.tssrc/cli/prepare_image.tstests/integration/prepare_image.integration.test.tstests/helpers/cli.tssrc/domain/prepare_image.ts
tests/**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Use explicit 30 second timeout for ffmpeg-heavy tests instead of relying on Bun's default 5 second per-test timeout
Files:
tests/integration/cli.integration.test.tstests/integration/prepare_image.integration.test.ts
src/cli/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Organize code into
src/clifor command entry points and argument parsing
Files:
src/cli/prepare_image.ts
src/domain/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Place deterministic policy, analysis, export, report, benchmark, and validation logic in
src/domain
Files:
src/domain/prepare_image.ts
🔇 Additional comments (16)
src/domain/prepare_image.ts (8)
1-23: LGTM!
24-61: LGTM!
63-91: LGTM!
109-115: LGTM!
117-124: LGTM!
126-142: LGTM!
144-154: LGTM!
93-107: ⚡ Quick winNo action needed. The
buildPrepareImageFilterfunction is safe from FFmpeg filter injection because allPrepareImageLayoutproperties are strictly typed asnumber. TypeScript ensures that non-numeric values cannot reach the filter string construction, making injection impossible. No additional sanitization is required.> Likely an incorrect or invalid review comment.src/cli/prepare_image.ts (4)
1-14: LGTM!
50-66: LGTM!
68-80: LGTM!
24-41:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix the argument parsing loop to avoid skipping arguments.
There's a bug in the argument parsing loop. On lines 29 and 36, you increment
indexand thencontinue, but theforloop at line 24 will incrementindexagain at the end of the iteration. This causes the parser to skip the next argument after--outor--border-px.For example, if the args are
["input.png", "--out", "output.jpg", "--border-px", "10"], after processing--out,indexis incremented from 1 to 2 (line 29), then the loop increments it to 3, skipping"--border-px".🐛 Proposed fix to remove manual index increments
if (flag === "--out") { outputPath = readFlagValue(args, index, "--out"); - index += 1; continue; } if (flag === "--border-px") { const value = readFlagValue(args, index, "--border-px"); borderPx = parseBorderPx(value); - index += 1; continue; }The
forloop's increment expression already handles advancing to the next argument.> Likely an incorrect or invalid review comment.tests/helpers/cli.ts (1)
23-24: LGTM!Also applies to: 39-39, 110-112
tests/integration/cli.integration.test.ts (1)
28-28: LGTM!package.json (1)
16-16: LGTM!Also applies to: 26-26
docs/plans/v1-prepare-image-cli.md (1)
1-21: LGTM!
|
@coderabbitai review\n\nAll active review threads have been addressed or replied to and resolved. Please re-review the current stack state. |
|
✅ Actions performedFull review triggered. |
1ae3a8c to
b00efbf
Compare
b1e6ea8 to
9e86a92
Compare
Dismissed after addressed review thread was resolved and fresh CodeRabbit status passed.
Merge activity
|
9e86a92 to
f5e6a40
Compare
1 participant
Note
Add
prepare-imageCLI to export images as JPEG with white background compositingprepare-imageCLI (src/cli/prepare_image.ts) accepting<input>,--out,--border-px, and--helpflags; exits nonzero on invalid usage or processing errors.prepareImagedomain function (src/domain/prepare_image.ts) that uses ffprobe to detect source dimensions (respecting rotation metadata), computes layout, then runs ffmpeg to composite the source onto a white background canvas and exports a single-frame JPEG with metadata stripped.computePrepareImageLayout(src/domain/prepare_image_layout.ts) to deterministically select landscape (3:2) or portrait (3:4) output targets, avoid upscaling, and compute render size, offsets, and optional centered cover crop.resolvePrepareImageOutputPath(src/domain/output_path.ts) to normalize the output to.jpg, create parent directories, reject directory paths, and auto-suffix (-1,-2, …) to avoid overwriting existing files.--border-pxis omitted.Macroscope summarized 9e86a92.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores