-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[AI] Finish Reasons #15931
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: pb-image-config
Are you sure you want to change the base?
[AI] Finish Reasons #15931
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -233,4 +233,88 @@ final class APITests: XCTestCase { | |
| XCTAssertEqual(cacheDetail?.modality, .text) | ||
| XCTAssertEqual(cacheDetail?.tokenCount, 50) | ||
| } | ||
|
|
||
| func testFinishReason_decoding() throws { | ||
| let decoder = JSONDecoder() | ||
|
|
||
| // Test LANGUAGE | ||
| var json = createResponseJSON(finishReason: "LANGUAGE") | ||
| var response = try decoder.decode(GenerateContentResponse.self, from: json) | ||
| XCTAssertEqual(response.candidates.first?.finishReason, .language) | ||
|
|
||
| // Test UNEXPECTED_TOOL_CALL | ||
| json = createResponseJSON(finishReason: "UNEXPECTED_TOOL_CALL") | ||
| response = try decoder.decode(GenerateContentResponse.self, from: json) | ||
| XCTAssertEqual(response.candidates.first?.finishReason, .unexpectedToolCall) | ||
|
|
||
| // Test TOO_MANY_TOOL_CALLS | ||
| json = createResponseJSON(finishReason: "TOO_MANY_TOOL_CALLS") | ||
| response = try decoder.decode(GenerateContentResponse.self, from: json) | ||
| XCTAssertEqual(response.candidates.first?.finishReason, .tooManyToolCalls) | ||
|
|
||
| // Test MISSING_THOUGHT_SIGNATURE | ||
| json = createResponseJSON(finishReason: "MISSING_THOUGHT_SIGNATURE") | ||
| response = try decoder.decode(GenerateContentResponse.self, from: json) | ||
| XCTAssertEqual(response.candidates.first?.finishReason, .missingThoughtSignature) | ||
|
|
||
| // Test MALFORMED_RESPONSE | ||
| json = createResponseJSON(finishReason: "MALFORMED_RESPONSE") | ||
| response = try decoder.decode(GenerateContentResponse.self, from: json) | ||
| XCTAssertEqual(response.candidates.first?.finishReason, .malformedResponse) | ||
|
|
||
| // Test IMAGE_SAFETY | ||
| json = createResponseJSON(finishReason: "IMAGE_SAFETY") | ||
| response = try decoder.decode(GenerateContentResponse.self, from: json) | ||
| XCTAssertEqual(response.candidates.first?.finishReason, .imageSafety) | ||
|
|
||
| // Test IMAGE_PROHIBITED_CONTENT | ||
| json = createResponseJSON(finishReason: "IMAGE_PROHIBITED_CONTENT") | ||
| response = try decoder.decode(GenerateContentResponse.self, from: json) | ||
| XCTAssertEqual(response.candidates.first?.finishReason, .imageProhibitedContent) | ||
|
|
||
| // Test IMAGE_OTHER | ||
| json = createResponseJSON(finishReason: "IMAGE_OTHER") | ||
| response = try decoder.decode(GenerateContentResponse.self, from: json) | ||
| XCTAssertEqual(response.candidates.first?.finishReason, .imageOther) | ||
|
|
||
| // Test NO_IMAGE | ||
| json = createResponseJSON(finishReason: "NO_IMAGE") | ||
| response = try decoder.decode(GenerateContentResponse.self, from: json) | ||
| XCTAssertEqual(response.candidates.first?.finishReason, .noImage) | ||
|
|
||
| // Test IMAGE_RECITATION | ||
| json = createResponseJSON(finishReason: "IMAGE_RECITATION") | ||
| response = try decoder.decode(GenerateContentResponse.self, from: json) | ||
| XCTAssertEqual(response.candidates.first?.finishReason, .imageRecitation) | ||
| } | ||
|
Comment on lines
+237
to
+289
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test To improve readability and maintainability, I suggest refactoring this test to be data-driven. You can create an array of test cases (tuples containing the reason string and the expected func testFinishReason_decoding() throws {
let decoder = JSONDecoder()
let testCases: [(String, FinishReason)] = [
("LANGUAGE", .language),
("UNEXPECTED_TOOL_CALL", .unexpectedToolCall),
("TOO_MANY_TOOL_CALLS", .tooManyToolCalls),
("MISSING_THOUGHT_SIGNATURE", .missingThoughtSignature),
("MALFORMED_RESPONSE", .malformedResponse),
("IMAGE_SAFETY", .imageSafety),
("IMAGE_PROHIBITED_CONTENT", .imageProhibitedContent),
("IMAGE_OTHER", .imageOther),
("NO_IMAGE", .noImage),
("IMAGE_RECITATION", .imageRecitation),
]
for (reasonString, expectedReason) in testCases {
let json = createResponseJSON(finishReason: reasonString)
let response = try decoder.decode(GenerateContentResponse.self, from: json)
XCTAssertEqual(
response.candidates.first?.finishReason,
expectedReason,
"Failed to decode finishReason '\(reasonString)'"
)
}
} |
||
|
|
||
| // Helper function to create JSON for GenerateContentResponse with a specific finish reason | ||
| private func createResponseJSON(finishReason: String) -> Data { | ||
| return """ | ||
| { | ||
| "candidates": [ | ||
| { | ||
| "content": { | ||
| "parts": [ | ||
| { "text": "Test reply" } | ||
| ], | ||
| "role": "model" | ||
| }, | ||
| "finishReason": "\(finishReason)", | ||
| "index": 0, | ||
| "safetyRatings": [] | ||
| } | ||
| ], | ||
| "usageMetadata": { | ||
| "promptTokenCount": 10, | ||
| "cachedContentTokenCount": 0, | ||
| "candidatesTokenCount": 5, | ||
| "totalTokenCount": 15, | ||
| "promptTokensDetails": [], | ||
| "cacheTokensDetails": [], | ||
| "candidatesTokensDetails": [] | ||
| } | ||
| } | ||
| """.data(using: .utf8)! | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test appears to have a logical contradiction. It expects the call to
model.generateImages(prompt: imagePrompt)to succeed, but then asserts thatresponse.images.isEmpty.Based on the implementation of
ImagenGenerationResponse.init(from:), anImagenImagesBlockedErroris thrown if theimagesarray is empty. Therefore, this test should expect an error to be thrown rather than a successful response.Please consider refactoring this test to use
await #expect { ... } throws: { ... }to correctly handle and verify the expected error, similar to thegenerateImage_allImagesFilteredOuttest.