[jcode] Add H3 camera movement preset chips - #9
Conversation
📝 WalkthroughWalkthroughThe PR adds six shared camera-motion presets, editable prompt overrides in the composer, selection-aware token insertion, reset behavior, server validation, idempotency hashing, persistence, retry restoration, tests, styles, and documentation. ChangesPrompt customization
Sequence Diagram(s)sequenceDiagram
participant Composer
participant SharedPresets
participant GenerationService
participant PayloadHash
Composer->>SharedPresets: Insert camera token at selection
SharedPresets-->>Composer: Return effective prompt and caret
Composer->>GenerationService: Submit prompt override
GenerationService->>PayloadHash: Include prompt in normalized hash input
PayloadHash-->>GenerationService: Return payload hash
GenerationService-->>Composer: Create or reuse generation job
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@packages/client/src/features/Composer.tsx`:
- Around line 168-180: In packages/client/src/features/Composer.tsx lines
168-180, update insertCameraPreset so it returns without setting promptOverride
or promptTouched when rendered.text is null; in lines 306-346, disable preset
buttons and prompt editing until rendered.text exists. In
packages/client/src/features/Composer.test.tsx lines 155-228, add a regression
test covering preset selection before subject entry and verify the submitted
prompt retains “A film of a car”.
In `@packages/shared/src/__tests__/schemas.test.ts`:
- Around line 93-103: Update the prompt validation used by
createGenerationSchema so whitespace-only overrides normalize to an empty
string, while nonblank prompts retain all leading and trailing characters. Apply
the 7000-character H3 limit to the generated prompt value without trimming
nonblank input, and extend the existing test with ' a cat, pan left '
expecting the identical parsed value.
In `@packages/shared/src/schemas.ts`:
- Around line 80-87: Update the prompt schema definition so the optional prompt
override preserves leading and trailing whitespace when passed to
GenerationService.create(). Keep the existing string type, maximum-length
validation, and optional behavior, but remove trimming from the prompt field.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 89a41e65-82c9-4bc2-acfa-4153a99b5046
📒 Files selected for processing (13)
README.mdpackages/client/src/features/Composer.test.tsxpackages/client/src/features/Composer.tsxpackages/client/src/styles.csspackages/server/src/__tests__/services.test.tspackages/server/src/__tests__/util.test.tspackages/server/src/services/generationService.tspackages/server/src/util.tspackages/shared/src/__tests__/cameraPresets.test.tspackages/shared/src/__tests__/schemas.test.tspackages/shared/src/cameraPresets.tspackages/shared/src/index.tspackages/shared/src/schemas.ts
| function insertCameraPreset(preset: CameraPreset) { | ||
| const el = promptRef.current; | ||
| const text = effectivePrompt; | ||
| // Before the user has placed the cursor, append at the end (intuitive | ||
| // default) instead of an uninitialized 0 cursor. | ||
| const fallback = text.length; | ||
| const start = el && promptInteractedRef.current ? el.selectionStart : fallback; | ||
| const end = el && promptInteractedRef.current ? el.selectionEnd : fallback; | ||
| const result = insertTokenAtSelection(text, start, end, preset.token); | ||
| pendingSelectionRef.current = { start: result.selectionStart, end: result.selectionEnd }; | ||
| setPromptOverride(result.text); | ||
| setPromptTouched(true); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not freeze an empty prompt override before template rendering succeeds.
If a user selects a preset before filling subject, effectivePrompt is ''. Lines 176-179 then freeze only the camera token. After the user fills subject, submission sends that token and discards the rendered template.
packages/client/src/features/Composer.tsx#L168-L180: Ifrendered.textis null, do not setpromptOverrideorpromptTouched.packages/client/src/features/Composer.tsx#L306-L346: Disable preset buttons and prompt editing untilrendered.textexists.packages/client/src/features/Composer.test.tsx#L155-L228: Add a regression test that selects a preset before fillingsubject, then verifies the final prompt retainsA film of a car.
📍 Affects 2 files
packages/client/src/features/Composer.tsx#L168-L180(this comment)packages/client/src/features/Composer.tsx#L306-L346packages/client/src/features/Composer.test.tsx#L155-L228
🤖 Prompt for 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.
In `@packages/client/src/features/Composer.tsx` around lines 168 - 180, In
packages/client/src/features/Composer.tsx lines 168-180, update
insertCameraPreset so it returns without setting promptOverride or promptTouched
when rendered.text is null; in lines 306-346, disable preset buttons and prompt
editing until rendered.text exists. In
packages/client/src/features/Composer.test.tsx lines 155-228, add a regression
test covering preset selection before subject entry and verify the submitted
prompt retains “A film of a car”.
| it('accepts an optional rendered-prompt override and caps it at the H3 limit', () => { | ||
| expect(() => | ||
| createGenerationSchema.parse({ ...base, prompt: 'a car, pan left' }), | ||
| ).not.toThrow(); | ||
| const at = createGenerationSchema.parse({ ...base, prompt: 'x'.repeat(7000) }); | ||
| expect(at.prompt).toHaveLength(7000); | ||
| expect(() => | ||
| createGenerationSchema.parse({ ...base, prompt: 'x'.repeat(7001) }), | ||
| ).toThrow(); | ||
| // Blank trims to empty (treated as "no override" by the service). | ||
| expect(createGenerationSchema.parse({ ...base, prompt: ' ' }).prompt).toBe(''); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve nonblank prompt whitespace.
Line 102 validates blank normalization, but the route schema also trims nonblank overrides. This removes leading and trailing text that the composer preserved before the request reaches GenerationService.
Collapse only whitespace-only input to ''. Preserve every nonblank input character and apply the H3 length limit to the text that will be generated. Add a test for ' a cat, pan left ' that expects the same value after parsing.
🤖 Prompt for 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.
In `@packages/shared/src/__tests__/schemas.test.ts` around lines 93 - 103, Update
the prompt validation used by createGenerationSchema so whitespace-only
overrides normalize to an empty string, while nonblank prompts retain all
leading and trailing characters. Apply the 7000-character H3 limit to the
generated prompt value without trimming nonblank input, and extend the existing
test with ' a cat, pan left ' expecting the identical parsed value.
| /** | ||
| * Optional fully-rendered prompt override. When a non-empty string is | ||
| * supplied (e.g. by the composer after inserting camera-motion cues), the | ||
| * server uses it verbatim as the prompt text item instead of rendering the | ||
| * immutable prompt version with `values`. It is still subject to the H3 | ||
| * rendered-character limit. Omit/leave blank to render from the version. | ||
| */ | ||
| prompt: z.string().trim().max(H3_MAX_PROMPT_CHARS).optional(), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline packages/server/src/services/generationService.ts \
--match resolveRenderedPrompt --view expanded
rg -n -C 4 'createGenerationSchema|safeParse\(|\.parse\(|resolveRenderedPrompt' \
packages/server packages/sharedRepository: cnjack/minimax-video-prompt-library
Length of output: 26897
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## schemas:"
sed -n '76,90p' packages/shared/src/schemas.ts
echo "## generationService create and resolveRenderedPrompt:"
sed -n '30,55p' packages/server/src/services/generationService.ts
sed -n '320,370p' packages/server/src/services/generationService.ts
echo "## shared prompt schema tests around override behavior:"
sed -n '93,104p' packages/shared/src/__tests__/schemas.test.ts
echo "## behavioral probe for Zod trim transform semantics if dependency is present:"
node - <<'JS'
try {
const z = require('zod');
const schema = z.string().trim().max(7000).optional();
for (const input of [' a ', ' a ', ' a ', '']) {
const parsed = schema.parse(input);
console.log(JSON.stringify({ input: input, parsed: parsed, length: parsed.length }));
}
} catch (error) {
console.log(`zod dependency probe skipped: ${error.message}`);
}
JSRepository: cnjack/minimax-video-prompt-library
Length of output: 4248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## package dependency declarations:"
python3 - <<'PY'
import json, pathlib
for p in pathlib.Path('.').glob('*'):
if p.is_file() and p.name in ('package.json', 'package-lock.json', 'pnpm-lock.yaml', 'yarn.lock'):
print(f'\n== {p} ==')
data = p.read_text(errors='ignore')
for line in data.splitlines()[:220]:
if 'zod' in line.lower() or '"dependencies"' in line or '"devDependencies"' in line:
print(line)
PY
echo "## shared scripts/config:"
fd -a 'package.json|tsconfig|vite|vitest' packages/shared packages/server | sed 's#^\./##'
for f in packages/shared/package.json packages/shared/vite.config.ts packages/shared/vitest.config.ts; do
test -f "$f" && echo "===== $f =====" && sed -n '1,160p' "$f"
doneRepository: cnjack/minimax-video-prompt-library
Length of output: 1601
🌐 Web query:
Zod v3 string trim transform parsed string removes leading and trailing whitespace
💡 Result:
In Zod v3, the.trim method is a built-in string transformation that removes leading and trailing whitespace from a string [1][2]. It functions by applying the JavaScript String.prototype.trim operation to the input [3]. Key points regarding.trim: 1. Behavior: It strictly removes whitespace from both the beginning and the end of the string [3][4]. It does not affect whitespace located within the middle of the string [3]. 2. Chaining: Because.trim is a built-in method of the ZodString class, it returns a ZodString instance [5]. This allows you to chain other string-specific validation methods (such as.min,.max, or.email) after it [5]. 3. Execution Order: Built-in transformations like.trim are typically executed as part of the schema's validation/transformation pipeline [5][6]. If you require whitespace removal before other validations (such as a minimum length check that should ignore padded spaces),.trim is designed to be used directly in the chain [7]. 4. Alternatives for Deep Whitespace Removal: If you need to remove all whitespace (including internal whitespace), you must use the.transform method with a regular expression (e.g.,.transform(val => val.replace(/\s+/g, ''))) [5][4]. Note that because.transform returns a ZodEffects wrapper rather than a ZodString, you may need to use.pipe if you wish to chain further string-specific validations after such a transformation [5]. For scenarios requiring complex preprocessing before validation, Zod also provides z.preprocess, which allows you to modify the input data before it reaches the Zod schema validation logic [1][7].
Citations:
- 1: https://zod.dev/api
- 2: https://v3.zod.dev/?id=or
- 3: https://stackoverflow.com/questions/77594561/how-to-remove-whitespace-from-within-a-string-using-zod
- 4: https://stackoverflow.com/questions/77134910/how-can-i-remove-all-whitespace-in-zod
- 5: Zod transformations understanding colinhacks/zod#5734
- 6: https://github.com/colinhacks/zod/blob/main/packages/docs-v3/MIGRATION.md
- 7: how to trim string before do validation? colinhacks/zod#614
Preserve the submitted prompt text.
Line 87 trims request.prompt before GenerationService.create() receives it, so submitted leading or trailing spaces are discarded instead of being used verbatim as documented for the prompt override.
Proposed fix
- prompt: z.string().trim().max(H3_MAX_PROMPT_CHARS).optional(),
+ prompt: z.string().max(H3_MAX_PROMPT_CHARS).optional(),📝 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.
| /** | |
| * Optional fully-rendered prompt override. When a non-empty string is | |
| * supplied (e.g. by the composer after inserting camera-motion cues), the | |
| * server uses it verbatim as the prompt text item instead of rendering the | |
| * immutable prompt version with `values`. It is still subject to the H3 | |
| * rendered-character limit. Omit/leave blank to render from the version. | |
| */ | |
| prompt: z.string().trim().max(H3_MAX_PROMPT_CHARS).optional(), | |
| /** | |
| * Optional fully-rendered prompt override. When a non-empty string is | |
| * supplied (e.g. by the composer after inserting camera-motion cues), the | |
| * server uses it verbatim as the prompt text item instead of rendering the | |
| * immutable prompt version with `values`. It is still subject to the H3 | |
| * rendered-character limit. Omit/leave blank to render from the version. | |
| */ | |
| prompt: z.string().max(H3_MAX_PROMPT_CHARS).optional(), |
🤖 Prompt for 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.
In `@packages/shared/src/schemas.ts` around lines 80 - 87, Update the prompt
schema definition so the optional prompt override preserves leading and trailing
whitespace when passed to GenerationService.create(). Keep the existing string
type, maximum-length validation, and optional behavior, but remove trimming from
the prompt field.
|
Superseded by the cumulative, adversarially reviewed and merged #11. |
1 participant
Draft PR opened by jcode Cloud Agent for run
34055d9b85459e1c16ece4a52616202a.Triggered by a jtype kanban card.
Task
Add H3 camera movement preset chips
Context
MiniMax's official H3 guide recommends camera-motion cues such as pan, zoom,
tracking, and static shots. Creators should not have to remember or repeatedly
type the supported syntax. This request was produced from external product/API
research and intentionally enters Backlog before implementation approval.
Sources:
Requested outcome
Add accessible camera movement preset chips to the generation composer. A user
can insert a preset at the current prompt cursor without losing surrounding text.
At minimum include: Pan left, Pan right, Push in, Pull out, Tracking shot, and
Static shot. Keep the preset data in a small shared/pure module so labels and
inserted tokens are testable and are not duplicated across the UI.
Acceptance criteria
accessible names.
preserves surrounding prompt text.
request policy before submission.
a selection.
Delivery
Create a ready-for-review PR. Do not call the paid MiniMax API. Use mock mode for
all automated validation.
Branch
jcode/run-34055d9b@4e62adaa0ed6c57b9c6ea38c2df029e4eb41a20c.Not auto-merged and CI is not auto-triggered — review and iterate.
Summary by CodeRabbit