Conversation
sync next with master
…2780) * fix: images that fail content filter are added to failed directory * refactor: clean up filtering logic 1 * refactor: clean up filtering logic 2 * refactor: cleanup content-filter * fix(storage-resize-images): resize placeholders without overwriting blocked originals * fix(storage-resize-images): reuse moderation setup and correct data URLs * tests: fix tests and use fs instead of fs-extra * refactor(storage-resize-images): drop unused as-any cast and clarify maxOutputTokens comment Type inference from HARM_CATEGORIES.map(...) already matches genkit's SafetySettingsSchema, so the cast was a leftover from an earlier refactor. Restore the maxOutputTokens comment Izaak flagged on PR #2762, reworded to explain why 1 token suffices for the default yes/no answer vs 100 for the JSON-schema custom-prompt path. * fix(storage-resize-images): treat Gemini null-content responses as content-filter blocks Bug: when Gemini 2.5 Flash's input-side safety refuses on a borderline image, the candidate comes back with empty content rather than finishReason="SAFETY". Genkit then runs assertValidSchema on the null output, which throws ValidationError with status "INVALID_ARGUMENT". The existing catch only recognises finishReason="blocked", so the validation error gets retried 3 times (deterministic, same result every time), then propagates as a generic filter error — the image silently passes the filter from the user's perspective. Fix: detect the genkit ValidationError shape (status=INVALID_ARGUMENT + originalMessage matching /Provided data:\s*\n+\s*null/) and treat it as an implicit block, returning false from moderateImageOnce without going through the retry queue. Also adds: - Two unit tests covering the new path: null-content → blocked (no retries), real schema mismatch → still propagates after retries. - An env-var-gated live integration suite under __tests__/integration/ with a Bug 1 regression test that requires LIVE_BORDERLINE_IMAGE_PATH to point at an image the developer/CI supplies (cannot ship borderline imagery in a public repo). - CHANGELOG entry covering the fix and the placeholder/data-URL fixes already on this branch; bump extension.yaml to 0.3.4. Reported by Izaak Gough (#2762). * test(storage-resize-images): tune live content-filter sanity test for real Gemini behaviour Three small fixes after running the suite against a real Vertex project: - Switch the safe-image fixture from test-image.png (a tiny black square) to test-jpg.jpg (the word "test" rendered as text). The black-square fixture has no real content for Gemini to evaluate and was flaking the no-prompt sanity test. - Relax that sanity test from BLOCK_LOW_AND_ABOVE to BLOCK_ONLY_HIGH. The no-prompt path uses maxOutputTokens=1, which leaves the model little room to indicate "this is fine" — under the strictest threshold, even synthetic text can be flagged. The other live tests (which use a custom prompt + JSON schema + 100 tokens) keep BLOCK_LOW_AND_ABOVE and exercise the production-relevant threshold. - Bake NODE_OPTIONS=--experimental-vm-modules into the npm script so genkit's dynamic ESM imports load under Jest's CJS environment. Verified all 3 always-runnable tests pass against izaak-testing. * fix(storage-resize-images): strengthen null-content detection and tighten tests after review - isNullContentSafetyRefusal: also require detail.errors to be a populated array. Previously only the message regex + status guarded against false-positives; the structural check pins the detector to genkit's ValidationError contract (status + detail shape) so unrelated INVALID_ARGUMENT errors don't accidentally read as content blocks. - content-filter.test.ts: switch hand-crafted error mocks to the real ValidationError class imported from @genkit-ai/core/schema. The test now fails loudly if genkit changes the error shape, instead of false- passing against a stale mock. - Bump the negative test (real schema mismatch) from maxAttempts=2 to 3, asserting toHaveBeenCalledTimes(3) — exercises the full retry path rather than just proving "retried once". - CHANGELOG: clarify the bug only affects installs that configure a custom filter prompt (the only path that sets output.schema and so the only path where genkit's parseSchema(null,...) ran). Verified: 14 unit + handler tests pass; 3/3 live tests pass against izaak-testing. * fix(storage-resize-images): broaden moderation-schema-refusal detection to cover empty-object case The previous fix only matched when Gemini's response yielded `null` data (parseSchema's most-documented failure mode). Verified live against a real borderline image, that's incomplete: Gemini 2.5 Flash also produces ValidationError with `data: {}` when input-side safety refuses and emits non-JSON or empty refusal text that genkit's extractJson() coerces to an empty object. Same root cause, different shape — and likely more variants in future model versions. Drop the message-format regex and rely on the structural signature alone (status=INVALID_ARGUMENT + populated detail.errors). We control both the prompt and the schema in this code path, so any ValidationError from `ai.generate` originates from the model's response; treating the whole class as an implicit block is correct because the alternative (propagate as filterErrored) silently fails open on borderline NSFW content, which is the original bug. - Rename `isNullContentSafetyRefusal` → `isModerationSchemaRefusal`. - Drop the `/Provided data:\s*\n+\s*null/` regex. - Add unit tests for: empty-object data variant, type-mismatch (also blocks under the new policy with rationale comment), and a sanity check that non-ValidationError errors still propagate via retries. - Live test file: derive contentType from file extension via a small guessContentType helper, so LIVE_BORDERLINE_IMAGE_PATH works with png/webp/gif/jpeg without forcing the developer to convert. Verified live against izaak-testing with a borderline-NSFW stock photo: regression test resolves to false in 1.87s (single API call, no retries), down from 7.8s with 2 wasted retries before the previous narrow detector. * refactor(storage-resize-images): narrow catch-var types in content-filter Extract isSafetyBlockedResponse() so the safety-block branch narrows the thrown error from unknown the same way isModerationSchemaRefusal() does, instead of dereferencing the raw catch variable. Assert object.contentType in the handler: shouldResize() already rejects objects without a valid image/* contentType, but the handler is exported and unit-tested directly, so make the invariant explicit. * docs(storage-resize-images): note broadened schema-validation block in changelog --------- Co-authored-by: Jacob Cable <jacobcable94@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request addresses issues where Gemini 2.5 Flash safety refusals caused the content filter to fail open or retry unnecessarily. It refactors the filtering logic to treat schema validation errors as implicit blocks, ensures blocked images are routed correctly to the failed-image path, and prevents mutation of original files during placeholder substitution. New unit and live integration tests have been added to verify these fixes. The reviewer feedback identifies opportunities to improve performance by replacing synchronous file system operations with asynchronous ones to avoid blocking the event loop.
| return true; | ||
| } | ||
|
|
||
| const imageBuffer = fs.readFileSync(localOriginalFile); |
Contributor
There was a problem hiding this comment.
Using fs.readFileSync in an asynchronous function blocks the event loop, which can degrade the performance and responsiveness of the Cloud Function, especially when handling multiple concurrent requests or large image files. It is recommended to use the asynchronous fs.promises.readFile instead.
Suggested change
| const imageBuffer = fs.readFileSync(localOriginalFile); | |
| const imageBuffer = await fs.promises.readFile(localOriginalFile); |
| blockedImageStored = true; | ||
|
|
||
| localProcessingFile = `${localOriginalFile}-placeholder`; | ||
| fs.copyFileSync(localOriginalFile, localProcessingFile); |
Contributor
There was a problem hiding this comment.
Using fs.copyFileSync is a synchronous operation that blocks the event loop. Since this code is within an asynchronous handler, it is preferable to use fs.promises.copyFile to avoid blocking the execution of other concurrent operations.
Suggested change
| fs.copyFileSync(localOriginalFile, localProcessingFile); | |
| await fs.promises.copyFile(localOriginalFile, localProcessingFile); |
IzaakGough
approved these changes
May 18, 2026
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
3 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.
No description provided.