feat(prompts): MiniMax H3 Ref2VA system prompt with a per-prompt Expand Prompt output cap - #236
Merged
Merged
Conversation
MiniMax H3's ref2va conditioning presents each reference under a per-modality label (<Picture i>/<Video k>/<Audio j>) and responds best to a prompt organised into subject_definitions / summary / retention_analysis / detailed_description / overall_soundscape / non_diegetic_music sections. Adds "MiniMax H3 Ref2VA Structured Prompt" to the Expand Prompt system-prompt catalog, teaching the LLM that structure and the reference-label convention, and instructing it to mark anything the user's prompt does not supply as [TO FILL] rather than inventing references or scene detail. Seeded from a new migration rather than appended to DEFAULT_SYSTEM_PROMPTS: the migrator runs each id exactly once, so extending the original list would only reach databases created after this release. Same fixed-UUID / INSERT OR IGNORE shape, so an edited or deleted prompt stays that way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C1Li5DERFCEST56rvbEYo5
Expand Prompt asked the text LLM for at most 300 new tokens, a global default on `ExpandPromptRequest.max_tokens` that nothing ever overrode -- webv2 sent only the model, the prompt and the system prompt. That suits the one-paragraph rewrites the original seeded prompts produce, but truncates prompts that are structurally longer by design: the MiniMax H3 Ref2VA prompt emits six labelled sections and routinely needs more room than 300 tokens. The cap therefore becomes a property of the system prompt. `system_prompts` gains a nullable `max_tokens`, NULL meaning "use the endpoint default", and both callers that resolve a stored prompt honour it: webv2 sends the selected prompt's value with the expansion request, and the Text LLM (with System Prompt Preset) node -- which already had the record in hand and passed its own field regardless -- now defers to the preset's cap when its `max_tokens` field is left unset. An explicit field value still wins there; it is the node author overriding the preset. That changes the meaning of an unset field, hence the node version bump. The Ref2VA prompt is seeded at 500; every other prompt is untouched and keeps the 300 default. `max_tokens` is the one field on SystemPromptChanges whose null is a value rather than "no change" -- it clears the cap back to the default -- so the storage layer keys off the field's presence in the request body instead of `is not None`. Without that a cap could be set but never removed. The editor in the Expand Prompt popover exposes the cap as an optional numeric field (empty = the default), validated in place against the range the endpoint enforces so an out-of-range value never reaches the API as a bare 422. Both nodes now take their bounds from the same constants as the request model and the storage layer, rather than re-hardcoding 300/1/2048. The column and the seed are one migration, and the migrator runs it at most once per database inside a single transaction; its `IS NULL` scope and `PRAGMA table_info` guard therefore only protect a hand-modified database, not any sequence the app can produce. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NQyWtiuACyQeGZeddtFdLk
… list 500 still truncated the Ref2VA output, always in `overall_soundscape` or `non_diegetic_music`. Tokenizing with the Qwen BPE tokenizer these models use shows why: the cap was sized off the prompt's own worked example (187 tokens), but that example demonstrates a one-sentence shot while the instructions ask for camera movement, subject action, setting and lighting -- which costs ~155. A realistic three-reference request lands at 469, clearing 500 by 31 tokens; a fourth reference or a second shot does not fit. The two audio sections are last and cheap (~45 combined), so they are not truncated for being verbose -- they are what remains when the unbounded sections ahead of them have taken the budget. Two changes: - The cap goes to 1000. Measured marginal costs are ~75 fixed, +55 per reference and +155 per shot, so six references with three shots and a real music section lands near 955. `max_new_tokens` is a safety stop rather than a target (generation ends at EOS), so sizing for the worst realistic request costs nothing on a typical one. - `detailed_description` gains a two-to-four-sentence-per-shot bound, fixing the mismatch between the instructions and the worked example at its source rather than only buying room for it. The existing example is two sentences, so it already complies. Edited into the unreleased seed rather than added as a third migration: neither migration has shipped, so no released database carries the old text. Installs that ran them from this branch keep the old content and cap, since the migrator records each id once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NQyWtiuACyQeGZeddtFdLk
…t" sentinel
Typing the preset node's `max_tokens` as `int | None` broke the node in both workflow
editors. Pydantic serialises an optional constrained int as
`anyOf: [{integer, min, max}, {null}]` with `default: null`, and both editors' field
template builders read the default and the bounds off the *top level* of the property
schema (`buildFieldInputTemplate.ts:65,72,80`; webv2 `workflow/data/templates.ts:220`):
- `schemaObject.default ?? 0` turned the null default into 0, so every newly added node
materialised `max_tokens: 0` -- which the field's own `ge=1` rejects. Adding the node
and pressing Invoke produced a 422 on a node that worked before the change.
- `minimum`/`maximum` moved inside the `anyOf` and were dropped from the template, so the
number input ran unbounded (±2^31) instead of 1..2048, and the client-side range check
that exists to catch this before enqueue was silenced along with them.
0 is the sentinel instead, matching `tile_size` in image_to_latents, latents_to_image and
qwen_image_image_to_latents. The field stays a plain `int`, so the schema keeps
`default: 0, minimum: 0, maximum: 2048` at the top level: the editors materialise a value
the node accepts, the bounds survive, and a new node resolves to the preset's own cap.
Two tests pin the properties that were violated -- that the field's default is a value the
node itself accepts, and that its bounds are published at the top level of the schema
rather than nested in an `anyOf`.
Also corrects the migration docstring, which still said 500 after the cap went to 1000.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NQyWtiuACyQeGZeddtFdLk
lstein
requested review from
JPPhoto,
Pfannkuchensack and
blessedcoolant
as code owners
September 10, 2026 02:31
The three i18n strings for the Max-output-length field put `otherRawBytes` 106 bytes over budget on every route (152221 against a 152115 limit). That budget is effectively a budget on the translation bundle -- `dist/locales/en.json` is 147432 of the 152221 -- so any feature that adds UI strings grows it. main was already down to 154 bytes of headroom (~0.1%) against a baseline captured 2026-09-07, having grown 1353 bytes since, so the overage is a stale baseline meeting unavoidable i18n growth rather than a regression in the feature. Trimming the copy cannot fix it: the shortest wording that still explains what an empty field means is 169 bytes, still 15 over. Deleting the help text to reclaim 106 bytes of a 1.85 MB route payload is the wrong trade. Two side effects of `--update-baseline` that reviewers should see rather than have to find in the JSON: - It re-records every route, so main's ~30 KB of accumulated script growth is absorbed too (launchpad scriptRawBytes 1676948 -> 1691844). main was already inside that budget -- 1874 bytes of headroom -- so these bytes were de facto accepted; the baseline had simply not been refreshed since. - `browserExecutable` and the timing medians now come from this machine. Timings are recorded but not gated (`timingPolicy.enforce` is false, runner "unconfigured"), so the faster numbers cannot tighten CI; were enforcement ever switched on, the baseline would need re-recording on the designated runner first. Verified with `pnpm test:performance:architecture` (build, chunk check and browser measurement), which is the gate that actually failed in CI -- not `check-architecture-performance.mjs` alone, which measures a stale `dist` when run without its preceding build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NQyWtiuACyQeGZeddtFdLk
…onstants The two assertions added with the sentinel fix pinned constants (`field.default == 0` twice) and derived one expected value from the implementation, which `tests/AGENTS.md` rejects. They would have caught a revert, but only incidentally. Replaced with a test that takes the schema default -- the value both editors actually materialise an input from -- constructs the node with it, and asserts generation runs at the preset's cap. That exercises the property that broke: the default has to be a value the node accepts *and* has to mean "defer to the preset". The bounds check is kept, since constraints nested in an `anyOf` are what both editors drop, but without the duplicate default pin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NQyWtiuACyQeGZeddtFdLk
lstein
enabled auto-merge
September 11, 2026 02:04
lstein
disabled auto-merge
September 11, 2026 02:06
Replaces the wholesale `--update-baseline` re-record with the targeted raise the gate's own error asks for. Net effect on the committed baseline is now 20 lines -- `otherRawBytes` from 150608 to the measured 152221 and its ceiling from 152115 to 153744 (ceil(measured * 1.01)) -- instead of 158. `otherRawBytes` is the only metric over its limit. Checking every metric of every route against the *previous* ceilings: css, font, image, largestAsset, request counts, script bytes and totals all still pass, including the editor routes' script growth that the re-record had absorbed and raised ceilings for. Those ceilings are back where they were, so nothing is loosened that this change does not consume. `browserExecutable` and `capturedAt` are left as they were: this is a hand-edited budget raise, not a recapture, so it must not claim to be one or overwrite the recording environment of whoever captured it. The timing medians are likewise untouched. Verified with `pnpm test:performance:architecture` -- build gate and browser gate both clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NQyWtiuACyQeGZeddtFdLk
lstein
enabled auto-merge
September 11, 2026 02:11
lstein
added a commit
that referenced
this pull request
Sep 13, 2026
Merging main moved both perf baselines under this branch, so the pre-merge numbers no longer applied. Re-measured on the merged tree rather than carried over: - `architecture-baseline.json`: editor `ownedRawBytes` 131729 -> 131793. This budget is pinned with zero headroom, so any editor-code growth fails it by construction. The merge conflicted here (main 131729, this branch 131330); resolved to main's value and measured again, which gave 64 bytes rather than the 53 measured before the merge -- adding the old delta to main's number would have been wrong. - `browser-baseline.json`: editor-minimal and editor-gallery `scriptRawBytes` baseline 3511549 -> 3546763 with its ceiling derived as before. Worth naming: main already measures 3546699 on these routes, 34 bytes under the old ceiling, so most of this is main's drift since the 2026-09-07 capture, not this change's 64 bytes. Recording the measured value necessarily absorbs it. The surgical raise in #236 kept those ceilings honest but left no room, which is why a second budget surfaced behind the first here. launchpad, editor-canvas and editor-workflow are untouched; they never failed. Verified with `pnpm test:performance:architecture` on the merged tree: build gate and browser measurement both clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NQyWtiuACyQeGZeddtFdLk
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.
Summary
Expand Prompt asked the text LLM for at most 300 new tokens — a global default on
ExpandPromptRequest.max_tokensthat no client ever overrode. That suits the one-paragraph rewrites the existing seeded prompts produce, but truncates prompts that are structurally longer by design.This branch adds a MiniMax H3 Ref2VA structured-prompt preset and makes the output cap a property of the system prompt rather than a global.
What changed
A per-prompt cap.
system_promptsgains a nullablemax_tokens; NULL means "use the endpoint default". Both callers that resolve a stored prompt honour it — webv2 sends the selected prompt's value with the expansion request, and the Text LLM (with System Prompt Preset) node (which already had the record in hand and passed its own field regardless) defers to it when its own field is left at 0.max_tokensis the one field onSystemPromptChangeswhose null is a value rather than "no change" — it clears the cap back to the default — so the storage layer keys off the field's presence in the request body instead ofis not None. Without that a cap could be set but never removed.The editor in the Expand Prompt popover exposes the cap as an optional numeric field (empty = the default), validated in place against the range the endpoint enforces so an out-of-range value never reaches the API as a bare 422.
The Ref2VA prompt is seeded at 1000, sized off measurement rather than guesswork. Tokenizing with the Qwen BPE tokenizer these models use: ~75 tokens of fixed structure, +55 per reference, +155 per shot. A realistic three-reference request lands at 469, which is why 500 truncated — always in
overall_soundscapeornon_diegetic_music, the two sections that come last and are cheap (~45 combined). They were not truncated for being verbose; they were what remained once the unbounded sections ahead of them took the budget. Six references with three shots and a music section lands near 955.max_new_tokensis a safety stop rather than a target — generation ends at EOS — so sizing for the worst realistic request costs nothing on a typical one.detailed_descriptionalso gains a two-to-four-sentence-per-shot bound, fixing the mismatch between the instructions (camera movement, subject action, setting, lighting) and the worked example (one sentence) at its source rather than only buying room for it.Review history
Three adversarial fresh-context reviews were run against this diff. Each found real defects; all confirmed ones are fixed.
int | None, which nests its bounds in an OpenAPIanyOfand makes its defaultnull. Both editors read the default and bounds off the top level, so a newly added node materialisedmax_tokens: 0— rejected by its ownge=1— and its number input lost its bounds. Fixed with a0sentinel on a plainint, matchingtile_sizeelsewhere in the package.Corrections to earlier commit messages
Three measurements asserted in commits on this branch are wrong. The conclusions they supported still hold, but the numbers do not:
44a45d42f3) is true only of legacy web. In webv2 this node cannot be invoked at all:SystemPromptFieldis absent fromSTATEFUL_FIELD_TYPE_NAMES, so the requiredsystem_promptinput renders connection-only, and no invocation in the schema outputs aSystemPromptFieldfor an edge to supply. Readiness blocks Invoke beforemax_tokensis ever sent.9a42cc150e) used the sourceen.jsondiff (260 bytes) where the budget measures the minified build. The real built delta is 230 bytes, so main's headroom was 124, not 154, and the shortest viable rewording was 45 bytes over, not 15.9a42cc150e) said "main's ~30 KB". That commit has since been superseded by679883d923, which replaces the wholesale re-record with a targeted raise of the one metric the gate names, restoring every other ceiling; the description below is what the superseded commit did. Launchpad grew 14,896 bytes and that growth is main's — itsscriptSourceOwnerSetsentry contains none of this feature's modules. The editor routes grew ~29.7 KB each and their owner sets do containcore/systemPrompts.ts,data/systemPrompts.ts,SystemPromptsField.tsxandPositivePromptActions.tsx, so part of that is this feature's own code, re-recorded as baseline without being named. No budget was loosened beyond the mechanical ±1% re-derivation, and those routes were inside their old limits regardless.Known limitations
SystemPromptFieldinput (see above), so the preset-governs-generation path is unreachable there for both new and saved workflows. Pre-existing: webv2's widget dispatch has no case forSystemPromptField,StylePresetFieldorSavedWorkflowFieldand silently degrades them to connection-only rather than reporting an unsupported required input.max_tokens: 300and so never pick up a preset's cap. The editor always materialises a concrete value, so no saved workflow carries "unset". This repo's mechanism for a value whose meaning changed at a version bump is a per-version hook in legacy web'snodeUpdate.ts(migrateMiniMaxH3NumFramesValueis the precedent); that file is legacy UI, whichinvokeai/frontend/web/AGENTS.mdsays to change only when explicitly requested. Clearing the field to 0 opts an existing workflow in.invocation_cache_memory.py), not on the resolved record, so re-running an unchanged graph after a cap change returns the cached result. Pre-existing for prompt content; this change makes the DB-resolved path the default for new nodes.INSERT OR IGNORE, so re-running repairs nothing; such installs need the row edited directly.Noted, out of scope
21 invocation input fields across 15 other nodes declare numeric constraints inside an
anyOf—Ideogram4DenoiseInvocation.steps(min 2),ResizeLatentsInvocation.width(min 64),VideoConcatInvocation.fps(1–120) among them — so both editors drop their bounds exactly as happened here. Any of those with anulldefault will also materialise an out-of-range0. Worth its own change; it is why a repo-wide schema invariant test could not be added alongside this one.Test plan
tests/app/(excluding the slow services dir): 2400 passedtests/app/services/: 1297 passedpnpm lintclean (format, oxlint, tsc, architecture budgets), 7657 unit tests pass, architecture perf gate exits 0tsc --noEmitclean;openapi.jsonandschema.tsregenerated🤖 Generated with Claude Code
https://claude.ai/code/session_01NQyWtiuACyQeGZeddtFdLk