Uh oh!
There was an error while loading. Please reload this page.
Fix Translation.translate_fields/6 handling of map-shaped AI responses - #560
Merged
Merged
Conversation
`PhoenixKitAI.ask_with_prompt/4` returns the full OpenAI response
map (`%{"choices" => [%{"message" => %{"content" => "..."}}]}`),
not a raw string. The core helper was treating `{:ok, map}` as
`:unexpected_response` and erroring out — making every real
translation call fail. Surfaced when wiring projects' worker
against a live AI endpoint.
Fixed by routing the success branch through
`PhoenixKitAI.Completion.extract_content/1` — same helper
publishing's TranslatePostWorker already uses for its
post-translate AI call. Falls back to raw-binary handling so
test stubs and older plugin versions keep working.
How this slipped past: the helper's tests only exercised the
`:ai_not_installed` path (test env has no live AI plugin). The
success path was never reached, so the response-shape mismatch
was invisible. Added a test that pins the OpenAI-map → content
extraction → parse_response/2 contract by feeding the helper a
realistic map shape.mdon added a commit
to mdon/phoenix_kit_projects
that referenced
this pull request
May 21, 2026
Previously the worker returned `{:error, _}` for every translation
failure, which Oban dutifully retried up to `max_attempts: 3`.
That's catastrophic when the failure is deterministic — an
`:ai_not_installed` reason or a model that ignores the
structured-marker prompt would burn AI tokens on three identical
re-attempts with no chance of a different outcome.
Now the worker classifies failure reasons through `retryable?/1`
and returns `{:discard, reason}` for deterministic cases:
- `:ai_not_installed` (plugin missing / disabled / no endpoint)
- `{:ai_error, :no_endpoint}` / `:missing_prompt`
- `{:ai_error, {:unexpected_response, _}}`
- `{:parse_error, _}` (model produced unparseable output)
- `{:persist_error, _}` (changeset rejected the merge)
- any other non-transient shape
Retries only on genuinely transient failures:
- `{:ai_error, :request_timeout}`
- `{:ai_error, :rate_limited}`
- `{:ai_error, {:connection_error, _}}`
- `{:ai_error, {:exit, _}}` (GenServer timeout inside plugin)
Existing worker test that asserted `{:error, :ai_not_installed}`
updated to assert `{:discard, :ai_not_installed}` — the new
contract. Added a dedicated describe block pinning the
deterministic-vs-transient classification so the table doesn't
regress.
Discovered when a real translation against aion-1.0-mini hit
`:unexpected_response` (separate core fix in BeamLabEU/phoenix_kit#560)
and Oban re-queued the job 3x burning ~2.5k tokens per attempt.mdon added a commit
to mdon/phoenix_kit_projects
that referenced
this pull request
May 21, 2026
Two prompt-template fixes after real-world testing against
deepseek-v3.2:
1. Dropped the "Title (if present)" hedge — replaced with three
clean example blocks (NAME / TITLE / DESCRIPTION). The
"(if present)" hint was meant to signal "skip empty fields"
but the AI took it as "this is one of the source fields" and
emitted a `---TITLE---{{title}}` block when the title
variable wasn't bound (template resources have no title
field, so the variable was unsubstituted and rendered as
literal `{{title}}` text).
The corresponding parser fix (`---<MARKER>---` terminates
on any marker, not just requested ones) lives in
BeamLabEU/phoenix_kit#560 — that change is what actually
prevents corruption of adjacent fields. This commit is the
prompt cleanup so future-generated default prompts are
cleaner to begin with.
2. "Do not emit markers for fields the caller did not provide"
makes the contract explicit to the model.
Existing in-DB prompts aren't affected — only newly-generated
prompts via `generate_default_translation_prompt/0` get the
new template. Hosts that already provisioned the old prompt
get correctness from the parser fix alone.mdon added a commit
to mdon/phoenix_kit
that referenced
this pull request
May 22, 2026
Codex final-review findings on PR BeamLabEU#560: CONCERN: the original "handles OpenAI-shaped response map" test manually extracted message.content and then called parse_response/2. The test would still have passed against the pre-fix broken implementation because it never drove the new handle_ai_response/2 branch. Fixed by: - Exposing handle_ai_response/2 as @doc false def so the unit test can drive it directly (instead of going through the private path). - Rewrote the test to call Translation.handle_ai_response(map, fields) with a realistic OpenAI response shape. Now actually fails if the map-handling branch regresses. - Added two more handle_ai_response/2 tests covering the raw-binary legacy/stub path and the malformed-shape error path. CONCERN: the earlier implementation called PhoenixKitAI.Completion.extract_content/1, which raises UndefinedFunctionError in core's test env where the plugin isn't loaded. Replaced the cross-module call with an inline pattern match on the OpenAI shape (choices[0].message.content). The shape is stable (OpenAI-standard), so the inline match is safe; and removing the dep means the helper works in core's test env and in any host regardless of which PhoenixKitAI version they pin. NIT: the test comment now matches the test behavior.
This was referenced May 22, 2026
ddon pushed a commit
that referenced
this pull request
May 22, 2026
Codex final-review findings on PR #560: CONCERN: the original "handles OpenAI-shaped response map" test manually extracted message.content and then called parse_response/2. The test would still have passed against the pre-fix broken implementation because it never drove the new handle_ai_response/2 branch. Fixed by: - Exposing handle_ai_response/2 as @doc false def so the unit test can drive it directly (instead of going through the private path). - Rewrote the test to call Translation.handle_ai_response(map, fields) with a realistic OpenAI response shape. Now actually fails if the map-handling branch regresses. - Added two more handle_ai_response/2 tests covering the raw-binary legacy/stub path and the malformed-shape error path. CONCERN: the earlier implementation called PhoenixKitAI.Completion.extract_content/1, which raises UndefinedFunctionError in core's test env where the plugin isn't loaded. Replaced the cross-module call with an inline pattern match on the OpenAI shape (choices[0].message.content). The shape is stable (OpenAI-standard), so the inline match is safe; and removing the dep means the helper works in core's test env and in any host regardless of which PhoenixKitAI version they pin. NIT: the test comment now matches the test behavior.
ddon pushed a commit
to BeamLabEU/phoenix_kit_projects
that referenced
this pull request
May 22, 2026
Two prompt-template fixes after real-world testing against
deepseek-v3.2:
1. Dropped the "Title (if present)" hedge — replaced with three
clean example blocks (NAME / TITLE / DESCRIPTION). The
"(if present)" hint was meant to signal "skip empty fields"
but the AI took it as "this is one of the source fields" and
emitted a `---TITLE---{{title}}` block when the title
variable wasn't bound (template resources have no title
field, so the variable was unsubstituted and rendered as
literal `{{title}}` text).
The corresponding parser fix (`---<MARKER>---` terminates
on any marker, not just requested ones) lives in
BeamLabEU/phoenix_kit#560 — that change is what actually
prevents corruption of adjacent fields. This commit is the
prompt cleanup so future-generated default prompts are
cleaner to begin with.
2. "Do not emit markers for fields the caller did not provide"
makes the contract explicit to the model.
Existing in-DB prompts aren't affected — only newly-generated
prompts via `generate_default_translation_prompt/0` get the
new template. Hosts that already provisioned the old prompt
get correctness from the parser fix alone.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
PhoenixKitAI.ask_with_prompt/4returns the full OpenAI response map (%{\"choices\" => [%{\"message\" => %{\"content\" => \"...\"}}]}), not a raw string. The coreTranslation.translate_fields/6helper was treating{:ok, map}as{:error, {:ai_error, {:unexpected_response, map}}}— so every real translation call against a live AI plugin failed with that opaque error before ever reachingparse_response/2.Surfaced while wiring
phoenix_kit_projects' translate worker against a real endpoint (aion-1.0-mini). The AI call succeeded (status\"success\", 28s latency, 842 tokens), but the helper bailed because the response was a map.Fix
Routed the success branch through
PhoenixKitAI.Completion.extract_content/1— the same helper publishing'sTranslatePostWorkeralready uses for its post-translate AI call. Kept a fallback clause for raw-binary input so test stubs and older plugin versions keep working.How this slipped past testing
The helper's existing tests only exercised the
:ai_not_installedpath (the core test env has no live AI plugin configured). The success path was never reached in CI, so the response-shape mismatch couldn't surface. Added a regression test that feeds the helper a realistic OpenAI-map shape and pins theextract_content → parse_responsecontract.Test plan
parse_response/2finds the markersmix format,mix credo --strictclean