Repository navigation
fix(interactive-turn): cap guard retries and reasoning in completion protocol - #1
Merged
Merged
Conversation
…ize allowlists - Extract imageFormatFromBytes and mimeTypeForFormat into internal/imagegen/image_helpers.go so the OpenAI and Minimax adapters no longer duplicate the format inference logic. - Split the size allowlist in internal/imagegen/service.go by provider: OpenAI keeps the 2K/3K/4K series, Minimax introduces a separate supportedMinimaxImageSizes set. Resolution flows through one shared NormalizeSizeForProvider entry, opening a single seam for future providers. - Bring OpenAI adapter's fetchImageURL-style helper functions (imageFormatFromContentType, imageFormatFromURL, normalizeImageFormat, extensionForFormat) back as OpenAI-specific helpers; the cross-cutting format helpers stay in image_helpers.go. No behavior change for existing OpenAI profiles. New providers will extend NormalizeSizeForProvider with one branch and add their own allowlist slice.
Adds a Minimax image generation provider alongside OpenAI, with a new adapter that translates size to aspect_ratio at the request boundary and validates the response against Minimax's base64 format. This is the first non-OpenAI image provider in the codebase, so the PR also opens two small seams that make the next provider cheap: - Image format sniffing helpers (imageFormatFromBytes, mimeTypeForFormat) are extracted into internal/imagegen/image_helpers.go so the OpenAI and Minimax adapters no longer duplicate them. - The size allowlist in internal/imagegen/service.go becomes provider-aware: OpenAI keeps the 2K/3K/4K series, Minimax uses the 1024 series. Resolution flows through one shared NormalizeSizeForProvider entry point. Changes: - config/image_api_profiles.go: provider string parser accepts 'minimax', default base URL https://api.minimaxi.com/v1, default model image-01 - internal/imagegen/minimax/: new native HTTP client + protocol types - internal/imagegen/minimax_adapter.go: request/response adapter - internal/imagegen/minimax_adapter_test.go: 8 unit tests covering happy path, HTTP 4xx, business errors, empty responses, missing input, invalid base64, output-format inference, and size-allowlist routing - config.minimax.template.toml: copy-paste template for users - scripts/test-minimax-adapter.sh: sanity script - web/src/features/settings/image-profiles.ts and SettingsView.tsx: provider dropdown in the Settings UI with auto-fill for default base URL and model - CHANGELOG.md, README.md, README.en.md: feature entry Minimax is not the default. Adding Minimax to image_api_profiles does not change the default image profile; users opt in via Settings or config.toml. Brand names (OpenAI / Minimax) stay hardcoded in the UI rather than going through i18n; only descriptive strings flow through t(). Tested with go test ./internal/imagegen/... ./config/... and pnpm check:i18n.
…protocol Game-mode turns that lost the submit_interactive_turn submission path caused newInteractiveCompletionGuard to retry with generic feedback. Every retry re-generated a full reasoning pass on the heavy-reasoning Minimax-M3 model, so thinking content reached 7.6x–23.9x of narrative prose on retry turns. Three layered fixes, all in internal/agent/interactive_turn_protocol.go: - A: extract the last submit_interactive_turn receipt from the conversation history and embed field-level diagnostics (Module/Path/Expected/Actual) in the retry feedback so the model self-corrects on the first retry. - D0: guardRetries counter fails fast after 2 guard retries with a typed error, instead of silently burning the shared MaxRetries budget. - B: retry-phase completion budget — inject openai.WithMaxCompletionTokens (8192) + openai.WithReasoningEffort(low) on retry iterations. On Minimax-M3 reasoning_effort is ignored but max_completion_tokens is the only effective provider-side lever to cap thinking+visible output. New tests: receipt history pairing (TestInteractive*Feedback) and TestInteractiveTurnProtocolRetryCapsOpenAICompletionAndDropsReasoning verifying impl-specific OpenAI options fire only on the retry phase. Also adds scripts/turn_metrics.go (stdlib CLI) for before/after comparison of story turn metrics plus scripts/turn_metrics_test.go, and the code-review report under .planning/phases/current-task/. Includes the five WR-01..WR-05 follow-up fixes committed atomically: run_id-to-turn_id attribution in parseRuns, scanner.Err propagation, turn_metrics gofmt.
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.
fix(interactive-turn): cap guard retries and reasoning in completion protocol
Problem
In interactive-story mode, when the model lost the
submit_interactive_turnsubmission path,newInteractiveCompletionGuardretried with generic feedback. Every retry regenerated a full reasoning pass on the heavy-reasoning Minimax-M3 model, so thinking content reached 7.6×–23.9× of narrative prose on retry turns (sampled from.denova/projects/Lyozes RPG/interactive/story/story-st_dkn2ti1p61m0d3704b6f.jsonl).llm_callup to 6× in a single turn.tool_callscount strongly correlates with thinking volume (15 tool calls → 47kB thinking).Fix — three layers in
internal/agent/interactive_turn_protocol.gosubmit_interactive_turnreceipt from conversation history and embed field-level diagnostics (Module / Path / Expected / Actual) in the retry feedback so the model self-corrects on the first retry.lastInteractiveSubmitReceipt+interactiveRetryFeedbackFromReceiptguardRetriesatomic counter fails fast after 2 guard retries with a typed error (ErrInteractiveCompletionRetriesExceeded), instead of silently burning the sharedMaxRetriesbudget.interactiveTurnProtocolRunState+newInteractiveCompletionGuardopenai.WithMaxCompletionTokens(8192)+openai.WithReasoningEffort(low)on retry iterations. On Minimax-M3reasoning_effortis ignored, butmax_completion_tokensis the only effective provider-side lever (thinking + visible output share this limit).interactiveNarrativeBudgetOptionsThe retry feedback remains ephemeral (
PersistModifiedInputMessages=false) — no historical leakage.Provider constraint note
reasoning_effortis silently ignored on the Minimax-M3 OpenAI-compatible endpoint. Thethinkingparameter only supportsenabled/disabled/adaptive(no budget sub-field). Onlymax_completion_tokensconstrains reasoning on Minimax-M3. Seeinternal/providercompat/polyfill.gofor the existing model polyfill layer.Tests
TestInteractiveRetryFeedbackFromReceiptTargetsExactErrorsnow covers real history-pairing path (≥2 submissions, wrong-ID decoy, malformed receipt).TestInteractiveTurnProtocolRetryCapsOpenAICompletionAndDropsReasoningverifiesMaxCompletionTokens=8192andReasoningEffort=lowfire only on retry phase, not on narrative phase.TestInteractiveTurnProtocolAppliesStoryCompletionBudgetOnlyToNarrativeCandidatecontinues to enforce the narrative-phase common-options path.Follow-up tooling
scripts/turn_metrics.go— stdlib Go CLI for per-turn story metrics (narrative_bytes/thinking_bytes/ ratio),--comparemode for before/after,--runsmode aggregatesllm_callviarun_id → turn_idattribution (WR-01 fix).scripts/turn_metrics_test.go— covers parser, ratio edge cases, and oversized-line exit codes.Validation
go build ./...— cleango test ./internal/agent -run 'Interactive.*Guard|Interactive.*Feedback|Interactive.*Protocol' -count=1— all passgo test ./scripts -count=1— passes (new tests included)accepted all result modules completion_requested=true); reasoning budget not exercised on this turn because the model did not hit retry.Code review
Standard-depth review produced 0 critical / 5 warning / 0 info. All 5 warnings applied atomically as separate commits (
fix(REVIEW): WR-01..WR-05). Full report at.planning/phases/current-task/current-task-REVIEW.md.Risk
interactiveRetryCompletionBudget=8192is sized for a complex opening's fullstate_changessubmission. If a future story schema pushes submissions past 8k completion tokens, retry will truncate and the next round will be rejected again — at which pointD0cleanly caps damage at 2 retries.MaxCompletionTokensandReasoningEffortare OpenAI-flavored provider-specific options. Other providers in the registry (go-eino-ext/components/model/openai) accept the same options via the same wrapper; behavior on non-OpenAI providers is a no-op (harmless).