Fix OpenAI-Adapter: extrahiert choices[0].message.content zweistufig
Die OpenAI Chat Completions API liefert den eigentlichen KI-Inhalt als escaped JSON-String in choices[0].message.content, nicht als direktes JSON-Objekt. Der Adapter gab bisher den gesamten Envelope zurück, was dazu führte, dass AiResponseParser das Pflichtfeld 'title' nicht fand. Neues Verhalten: extractContentFromResponse() parst zunächst den äußeren Envelope und gibt choices[0].message.content als AiRawResponse-Inhalt weiter – analog zum AnthropicClaudeHttpAdapter. Bei fehlendem Inhalt (leer, kein choices-Array) oder unparseablem Envelope wird eine technische Failure (NO_CHOICE_CONTENT bzw. UNPARSEABLE_JSON) zurückgegeben. Tests aktualisiert und drei neue Tests für den zweistufigen Parse-Pfad sowie für Fehlerfälle ergänzt. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
+56
-4
@@ -9,6 +9,8 @@ import java.util.Objects;
|
|||||||
|
|
||||||
import org.apache.logging.log4j.LogManager;
|
import org.apache.logging.log4j.LogManager;
|
||||||
import org.apache.logging.log4j.Logger;
|
import org.apache.logging.log4j.Logger;
|
||||||
|
import org.json.JSONArray;
|
||||||
|
import org.json.JSONException;
|
||||||
import org.json.JSONObject;
|
import org.json.JSONObject;
|
||||||
|
|
||||||
import de.gecheckt.pdf.umbenenner.application.config.provider.ProviderConfiguration;
|
import de.gecheckt.pdf.umbenenner.application.config.provider.ProviderConfiguration;
|
||||||
@@ -61,9 +63,11 @@ import de.gecheckt.pdf.umbenenner.domain.model.AiRequestRepresentation;
|
|||||||
* <p>
|
* <p>
|
||||||
* <strong>Response handling:</strong>
|
* <strong>Response handling:</strong>
|
||||||
* <ul>
|
* <ul>
|
||||||
* <li><strong>HTTP 200:</strong> Returns {@link AiInvocationSuccess} with the raw response body,
|
* <li><strong>HTTP 200:</strong> Extracts {@code choices[0].message.content} from the
|
||||||
* even if the body is invalid JSON or semantically problematic. The Application layer
|
* OpenAI Chat Completions response envelope and returns it as the raw response content
|
||||||
* is responsible for parsing and validating content.</li>
|
* via {@link AiInvocationSuccess}. The Application layer then parses and validates
|
||||||
|
* this content as a NamingProposal JSON object. If the envelope cannot be parsed or
|
||||||
|
* the expected content field is absent, a technical failure is returned.</li>
|
||||||
* <li><strong>HTTP non-200:</strong> Treated as a technical failure. The response body may
|
* <li><strong>HTTP non-200:</strong> Treated as a technical failure. The response body may
|
||||||
* contain an error message, but this is logged for debugging; the client treats it as
|
* contain an error message, but this is logged for debugging; the client treats it as
|
||||||
* a transient communication failure.</li>
|
* a transient communication failure.</li>
|
||||||
@@ -77,6 +81,9 @@ import de.gecheckt.pdf.umbenenner.domain.model.AiRequestRepresentation;
|
|||||||
* <li>Endpoint unreachable (connection refused, DNS failure, etc.)</li>
|
* <li>Endpoint unreachable (connection refused, DNS failure, etc.)</li>
|
||||||
* <li>Interrupted IO during HTTP communication</li>
|
* <li>Interrupted IO during HTTP communication</li>
|
||||||
* <li>HTTP response with non-2xx status code</li>
|
* <li>HTTP response with non-2xx status code</li>
|
||||||
|
* <li>HTTP 200 response body that cannot be parsed as JSON ({@code UNPARSEABLE_JSON})</li>
|
||||||
|
* <li>HTTP 200 response with no {@code choices} or no {@code message.content}
|
||||||
|
* ({@code NO_CHOICE_CONTENT})</li>
|
||||||
* <li>Any other transport-level exception</li>
|
* <li>Any other transport-level exception</li>
|
||||||
* </ul>
|
* </ul>
|
||||||
* <p>
|
* <p>
|
||||||
@@ -184,7 +191,7 @@ public class OpenAiHttpAdapter implements AiInvocationPort {
|
|||||||
HttpResponse<String> response = executeRequest(httpRequest);
|
HttpResponse<String> response = executeRequest(httpRequest);
|
||||||
|
|
||||||
if (response.statusCode() == 200) {
|
if (response.statusCode() == 200) {
|
||||||
return new AiInvocationSuccess(request, new AiRawResponse(response.body()));
|
return extractContentFromResponse(request, response.body());
|
||||||
} else {
|
} else {
|
||||||
String reason = "HTTP_" + response.statusCode();
|
String reason = "HTTP_" + response.statusCode();
|
||||||
String message = "AI service returned status " + response.statusCode();
|
String message = "AI service returned status " + response.statusCode();
|
||||||
@@ -220,6 +227,51 @@ public class OpenAiHttpAdapter implements AiInvocationPort {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Extracts the NamingProposal content from a successful (HTTP 200) OpenAI response.
|
||||||
|
* <p>
|
||||||
|
* The OpenAI Chat Completions response wraps the actual AI content inside an envelope:
|
||||||
|
* {@code choices[0].message.content}. This content string is the NamingProposal JSON
|
||||||
|
* that the Application layer expects. This method performs the two-step extraction:
|
||||||
|
* first parses the outer envelope, then returns the inner content string.
|
||||||
|
* <p>
|
||||||
|
* If the envelope JSON cannot be parsed, or if {@code choices} is absent or empty,
|
||||||
|
* or if {@code message.content} is absent or blank, a technical failure is returned.
|
||||||
|
*
|
||||||
|
* @param request the original request (carried through to the result)
|
||||||
|
* @param responseBody the raw HTTP response body (the OpenAI envelope)
|
||||||
|
* @return success with the extracted content string, or a technical failure
|
||||||
|
*/
|
||||||
|
private AiInvocationResult extractContentFromResponse(AiRequestRepresentation request, String responseBody) {
|
||||||
|
try {
|
||||||
|
JSONObject json = new JSONObject(responseBody);
|
||||||
|
JSONArray choices = json.optJSONArray("choices");
|
||||||
|
if (choices == null || choices.isEmpty()) {
|
||||||
|
LOG.warn("OpenAI response contained no choices");
|
||||||
|
return new AiInvocationTechnicalFailure(request, "NO_CHOICE_CONTENT",
|
||||||
|
"OpenAI response contained no choices");
|
||||||
|
}
|
||||||
|
JSONObject firstChoice = choices.getJSONObject(0);
|
||||||
|
JSONObject message = firstChoice.optJSONObject("message");
|
||||||
|
if (message == null) {
|
||||||
|
LOG.warn("OpenAI response choice contained no message");
|
||||||
|
return new AiInvocationTechnicalFailure(request, "NO_CHOICE_CONTENT",
|
||||||
|
"OpenAI response choice contained no message");
|
||||||
|
}
|
||||||
|
String content = message.optString("content", null);
|
||||||
|
if (content == null || content.isBlank()) {
|
||||||
|
LOG.warn("OpenAI response message.content is absent or blank");
|
||||||
|
return new AiInvocationTechnicalFailure(request, "NO_CHOICE_CONTENT",
|
||||||
|
"OpenAI response message.content is absent or blank");
|
||||||
|
}
|
||||||
|
return new AiInvocationSuccess(request, new AiRawResponse(content));
|
||||||
|
} catch (JSONException e) {
|
||||||
|
LOG.warn("OpenAI response could not be parsed as JSON: {}", e.getMessage());
|
||||||
|
return new AiInvocationTechnicalFailure(request, "UNPARSEABLE_JSON",
|
||||||
|
"OpenAI response body is not valid JSON: " + e.getMessage());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Builds an OpenAI Chat Completions API request from the request representation.
|
* Builds an OpenAI Chat Completions API request from the request representation.
|
||||||
* <p>
|
* <p>
|
||||||
|
|||||||
+75
-16
@@ -43,8 +43,10 @@ import de.gecheckt.pdf.umbenenner.domain.model.PromptIdentifier;
|
|||||||
* <p>
|
* <p>
|
||||||
* <strong>Coverage goals:</strong>
|
* <strong>Coverage goals:</strong>
|
||||||
* <ul>
|
* <ul>
|
||||||
* <li>Successful HTTP 200 responses are mapped to {@link AiInvocationSuccess}</li>
|
* <li>HTTP 200 responses: {@code choices[0].message.content} is extracted and returned</li>
|
||||||
* <li>Raw response body is preserved exactly</li>
|
* <li>Two-step parsing: outer envelope parsed first, inner content string returned as-is</li>
|
||||||
|
* <li>HTTP 200 with missing/empty choices is classified as NO_CHOICE_CONTENT failure</li>
|
||||||
|
* <li>HTTP 200 with unparseable envelope JSON is classified as UNPARSEABLE_JSON failure</li>
|
||||||
* <li>HTTP non-2xx responses are mapped to technical failure</li>
|
* <li>HTTP non-2xx responses are mapped to technical failure</li>
|
||||||
* <li>HTTP timeout exceptions are classified as TIMEOUT</li>
|
* <li>HTTP timeout exceptions are classified as TIMEOUT</li>
|
||||||
* <li>Connection failures are classified as CONNECTION_ERROR</li>
|
* <li>Connection failures are classified as CONNECTION_ERROR</li>
|
||||||
@@ -57,8 +59,7 @@ import de.gecheckt.pdf.umbenenner.domain.model.PromptIdentifier;
|
|||||||
* <li>Effective API key is actually used in the Authorization header</li>
|
* <li>Effective API key is actually used in the Authorization header</li>
|
||||||
* <li>Full document text is sent (not truncated)</li>
|
* <li>Full document text is sent (not truncated)</li>
|
||||||
* <li>Null request raises NullPointerException</li>
|
* <li>Null request raises NullPointerException</li>
|
||||||
* <li>Adapter reads all values from ProviderConfiguration (AP-003)</li>
|
* <li>Adapter reads all values from ProviderConfiguration</li>
|
||||||
* <li>Behavioral contracts are unchanged after constructor change (AP-003)</li>
|
|
||||||
* </ul>
|
* </ul>
|
||||||
*/
|
*/
|
||||||
@ExtendWith(MockitoExtension.class)
|
@ExtendWith(MockitoExtension.class)
|
||||||
@@ -84,10 +85,11 @@ class OpenAiHttpAdapterTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@DisplayName("should return AiInvocationSuccess when HTTP 200 is received with raw response")
|
@DisplayName("should extract choices[0].message.content when HTTP 200 is received")
|
||||||
void testSuccessfulInvocationWith200Response() throws Exception {
|
void testSuccessfulInvocationWith200Response() throws Exception {
|
||||||
// Arrange
|
// Arrange
|
||||||
String responseBody = "{\"choices\":[{\"message\":{\"content\":\"test response\"}}]}";
|
String innerContent = "{\"title\":\"Bodenanalyseergebnis\",\"reasoning\":\"Found soil analysis\",\"date\":\"2025-02-23\"}";
|
||||||
|
String responseBody = "{\"choices\":[{\"message\":{\"content\":" + JSONObject.quote(innerContent) + "}}]}";
|
||||||
HttpResponse<String> httpResponse = mockHttpResponse(200, responseBody);
|
HttpResponse<String> httpResponse = mockHttpResponse(200, responseBody);
|
||||||
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
||||||
|
|
||||||
@@ -96,11 +98,11 @@ class OpenAiHttpAdapterTest {
|
|||||||
// Act
|
// Act
|
||||||
AiInvocationResult result = adapter.invoke(request);
|
AiInvocationResult result = adapter.invoke(request);
|
||||||
|
|
||||||
// Assert
|
// Assert: success, and the raw response content is the extracted inner content, not the full envelope
|
||||||
assertThat(result).isInstanceOf(AiInvocationSuccess.class);
|
assertThat(result).isInstanceOf(AiInvocationSuccess.class);
|
||||||
AiInvocationSuccess success = (AiInvocationSuccess) result;
|
AiInvocationSuccess success = (AiInvocationSuccess) result;
|
||||||
assertThat(success.request()).isEqualTo(request);
|
assertThat(success.request()).isEqualTo(request);
|
||||||
assertThat(success.rawResponse().content()).isEqualTo(responseBody);
|
assertThat(success.rawResponse().content()).isEqualTo(innerContent);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -364,7 +366,8 @@ class OpenAiHttpAdapterTest {
|
|||||||
@DisplayName("should preserve request in success result")
|
@DisplayName("should preserve request in success result")
|
||||||
void testSuccessPreservesRequest() throws Exception {
|
void testSuccessPreservesRequest() throws Exception {
|
||||||
// Arrange
|
// Arrange
|
||||||
HttpResponse<String> httpResponse = mockHttpResponse(200, "{\"result\":\"ok\"}");
|
HttpResponse<String> httpResponse = mockHttpResponse(200,
|
||||||
|
"{\"choices\":[{\"message\":{\"content\":\"{\\\"title\\\":\\\"Test\\\",\\\"reasoning\\\":\\\"r\\\"}\"}}]}");
|
||||||
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
||||||
|
|
||||||
AiRequestRepresentation request = createTestRequest("Test prompt", "Test document");
|
AiRequestRepresentation request = createTestRequest("Test prompt", "Test document");
|
||||||
@@ -461,7 +464,9 @@ class OpenAiHttpAdapterTest {
|
|||||||
new ProviderConfiguration(API_MODEL, TIMEOUT_SECONDS, API_BASE_URL, ""),
|
new ProviderConfiguration(API_MODEL, TIMEOUT_SECONDS, API_BASE_URL, ""),
|
||||||
httpClient);
|
httpClient);
|
||||||
|
|
||||||
HttpResponse<String> httpResponse = mockHttpResponse(200, "{}");
|
String innerContent = "{\"title\":\"Test\",\"reasoning\":\"r\"}";
|
||||||
|
HttpResponse<String> httpResponse = mockHttpResponse(200,
|
||||||
|
"{\"choices\":[{\"message\":{\"content\":" + JSONObject.quote(innerContent) + "}}]}");
|
||||||
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
||||||
|
|
||||||
AiRequestRepresentation request = createTestRequest("Test prompt", "Test document");
|
AiRequestRepresentation request = createTestRequest("Test prompt", "Test document");
|
||||||
@@ -525,20 +530,21 @@ class OpenAiHttpAdapterTest {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Verifies that adapter behavioral contracts (success mapping, error classification)
|
* Verifies that adapter behavioral contracts (success mapping, error classification)
|
||||||
* are unchanged after the constructor was changed from StartConfiguration to
|
* are preserved. HTTP 200 must extract {@code choices[0].message.content} as the
|
||||||
* ProviderConfiguration.
|
* raw response; non-200 and exceptions must produce typed technical failures.
|
||||||
*/
|
*/
|
||||||
@Test
|
@Test
|
||||||
@DisplayName("openAiAdapterBehaviorIsUnchanged: HTTP success and error mapping contracts are preserved")
|
@DisplayName("openAiAdapterBehaviorIsUnchanged: HTTP success and error mapping contracts are preserved")
|
||||||
void openAiAdapterBehaviorIsUnchanged() throws Exception {
|
void openAiAdapterBehaviorIsUnchanged() throws Exception {
|
||||||
// Success case: HTTP 200 must produce AiInvocationSuccess with raw body
|
// Success case: HTTP 200 must produce AiInvocationSuccess with extracted content
|
||||||
String successBody = "{\"choices\":[{\"message\":{\"content\":\"result\"}}]}";
|
String innerContent = "{\"title\":\"Rechnung\",\"reasoning\":\"r\"}";
|
||||||
|
String successBody = "{\"choices\":[{\"message\":{\"content\":" + JSONObject.quote(innerContent) + "}}]}";
|
||||||
HttpResponse<String> successResponse = mockHttpResponse(200, successBody);
|
HttpResponse<String> successResponse = mockHttpResponse(200, successBody);
|
||||||
doReturn(successResponse).when(httpClient).send(any(HttpRequest.class), any());
|
doReturn(successResponse).when(httpClient).send(any(HttpRequest.class), any());
|
||||||
|
|
||||||
AiInvocationResult result = adapter.invoke(createTestRequest("p", "d"));
|
AiInvocationResult result = adapter.invoke(createTestRequest("p", "d"));
|
||||||
assertThat(result).isInstanceOf(AiInvocationSuccess.class);
|
assertThat(result).isInstanceOf(AiInvocationSuccess.class);
|
||||||
assertThat(((AiInvocationSuccess) result).rawResponse().content()).isEqualTo(successBody);
|
assertThat(((AiInvocationSuccess) result).rawResponse().content()).isEqualTo(innerContent);
|
||||||
|
|
||||||
// Non-200 case: HTTP 429 must produce AiInvocationTechnicalFailure with HTTP_429 reason
|
// Non-200 case: HTTP 429 must produce AiInvocationTechnicalFailure with HTTP_429 reason
|
||||||
HttpResponse<String> rateLimitedResponse = mockHttpResponse(429, null);
|
HttpResponse<String> rateLimitedResponse = mockHttpResponse(429, null);
|
||||||
@@ -570,7 +576,7 @@ class OpenAiHttpAdapterTest {
|
|||||||
OpenAiHttpAdapter adapterWithPortZero = new OpenAiHttpAdapter(configWithPortZero, httpClient);
|
OpenAiHttpAdapter adapterWithPortZero = new OpenAiHttpAdapter(configWithPortZero, httpClient);
|
||||||
|
|
||||||
HttpResponse<String> httpResponse = mockHttpResponse(200,
|
HttpResponse<String> httpResponse = mockHttpResponse(200,
|
||||||
"{\"choices\":[{\"message\":{\"content\":\"test\"}}]}");
|
"{\"choices\":[{\"message\":{\"content\":\"{\\\"title\\\":\\\"T\\\",\\\"reasoning\\\":\\\"r\\\"}\"}}]}");
|
||||||
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
||||||
|
|
||||||
adapterWithPortZero.invoke(createTestRequest("p", "d"));
|
adapterWithPortZero.invoke(createTestRequest("p", "d"));
|
||||||
@@ -582,6 +588,59 @@ class OpenAiHttpAdapterTest {
|
|||||||
.doesNotContain(":0");
|
.doesNotContain(":0");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@DisplayName("should extract inner JSON from choices[0].message.content (two-step parse)")
|
||||||
|
void testTwoStepParseExtractsInnerContentString() throws Exception {
|
||||||
|
// The OpenAI API wraps the actual AI content as an escaped JSON string inside
|
||||||
|
// choices[0].message.content. The adapter must perform two-step parsing:
|
||||||
|
// 1) parse the outer envelope to extract choices[0].message.content,
|
||||||
|
// 2) return that content string as-is for the Application layer to parse.
|
||||||
|
String innerContent = "{\"title\":\"Bodenanalyseergebnis\",\"reasoning\":\"Soil analysis report\",\"date\":\"2025-02-23\"}";
|
||||||
|
String envelope = "{\"choices\":[{\"message\":{\"content\":" + JSONObject.quote(innerContent) + "}}]}";
|
||||||
|
|
||||||
|
HttpResponse<String> httpResponse = mockHttpResponse(200, envelope);
|
||||||
|
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
||||||
|
|
||||||
|
AiRequestRepresentation request = createTestRequest("Test prompt", "Test document");
|
||||||
|
|
||||||
|
AiInvocationResult result = adapter.invoke(request);
|
||||||
|
|
||||||
|
assertThat(result).isInstanceOf(AiInvocationSuccess.class);
|
||||||
|
AiInvocationSuccess success = (AiInvocationSuccess) result;
|
||||||
|
// The raw response content must be the extracted inner string, not the outer envelope
|
||||||
|
assertThat(success.rawResponse().content())
|
||||||
|
.as("Adapter must return the inner content string, not the outer OpenAI envelope")
|
||||||
|
.isEqualTo(innerContent);
|
||||||
|
// Verify the extracted content is parseable as the expected NamingProposal JSON
|
||||||
|
JSONObject parsed = new JSONObject(success.rawResponse().content());
|
||||||
|
assertThat(parsed.getString("title")).isEqualTo("Bodenanalyseergebnis");
|
||||||
|
assertThat(parsed.getString("date")).isEqualTo("2025-02-23");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@DisplayName("should return NO_CHOICE_CONTENT failure when choices array is empty")
|
||||||
|
void testEmptyChoicesArrayReturnsFailure() throws Exception {
|
||||||
|
HttpResponse<String> httpResponse = mockHttpResponse(200, "{\"choices\":[]}");
|
||||||
|
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
||||||
|
|
||||||
|
AiInvocationResult result = adapter.invoke(createTestRequest("p", "d"));
|
||||||
|
|
||||||
|
assertThat(result).isInstanceOf(AiInvocationTechnicalFailure.class);
|
||||||
|
assertThat(((AiInvocationTechnicalFailure) result).failureReason()).isEqualTo("NO_CHOICE_CONTENT");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@DisplayName("should return UNPARSEABLE_JSON failure when HTTP 200 body is not valid JSON")
|
||||||
|
void testUnparseableEnvelopeReturnsFailure() throws Exception {
|
||||||
|
HttpResponse<String> httpResponse = mockHttpResponse(200, "not-json-at-all");
|
||||||
|
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
||||||
|
|
||||||
|
AiInvocationResult result = adapter.invoke(createTestRequest("p", "d"));
|
||||||
|
|
||||||
|
assertThat(result).isInstanceOf(AiInvocationTechnicalFailure.class);
|
||||||
|
assertThat(((AiInvocationTechnicalFailure) result).failureReason()).isEqualTo("UNPARSEABLE_JSON");
|
||||||
|
}
|
||||||
|
|
||||||
// Helper methods
|
// Helper methods
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user