fix: images that fail content filter are added to failed directory#2780
Draft
IzaakGough wants to merge 12 commits intonextfrom
Draft
fix: images that fail content filter are added to failed directory#2780IzaakGough wants to merge 12 commits intonextfrom
IzaakGough wants to merge 12 commits intonextfrom
Conversation
![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 refactors the image moderation and placeholder replacement logic, introducing a new placeholder module and updating the main handler. Several critical issues were identified: a regression prevents resized placeholders from being generated when an image is blocked, and the original blocked image is overwritten by the placeholder before it can be moved to the failed directory for review. Additionally, the content filter logic should be optimized by moving initialization and file reading outside the retry loop, and event metadata should correctly report the filter status.
cabljac
reviewed
Apr 27, 2026
cabljac
reviewed
Apr 27, 2026
cabljac
reviewed
Apr 27, 2026
…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.
…ntent-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).
… 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.
…hten 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.
Contributor
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the storage-resize-images extension to version 0.3.4, introducing several fixes for the content filtering logic. Key changes include handling Gemini safety refusals that previously caused silent failures or unnecessary retries, ensuring blocked images are routed correctly before placeholder substitution, and using the correct content type for moderation requests. Feedback was provided regarding a typo in the model name (gemini-2.5-flash should likely be gemini-1.5-flash) and a suggestion to use a more robust check for ValidationError by inspecting the data property instead of relying on a regex against the error message.
Comment on lines
+67
to
+78
| const e = error as { | ||
| status?: string; | ||
| originalMessage?: string; | ||
| detail?: { errors?: unknown }; | ||
| }; | ||
| if (e.status !== "INVALID_ARGUMENT") return false; | ||
| const errors = e.detail?.errors; | ||
| if (!Array.isArray(errors) || errors.length === 0) return false; | ||
| return ( | ||
| typeof e.originalMessage === "string" && | ||
| /Provided data:\s*\n+\s*null/.test(e.originalMessage) | ||
| ); |
Contributor
There was a problem hiding this comment.
The current implementation relies on a regex check against the originalMessage string to identify safety refusals, which is brittle and may break if the underlying library updates its error message formatting. A more robust approach is to check the data property of the error object, which Genkit's ValidationError sets to null when the model returns empty content due to safety refusals.
const e = error as {
status?: string;
data?: unknown;
detail?: { errors?: unknown };
};
if (e.status !== "INVALID_ARGUMENT") return false;
return (
e.data === null &&
Array.isArray(e.detail?.errors) &&
e.detail.errors.length > 0
);…on 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.
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
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.
Fixes #2769 and #2762.
Refactors content-filter and adds a fix for the silent fail-open on Gemini 2.5 Flash safety refusals reported in #2769.
What changed
moderateImageOncenow recognises the genkitValidationErrorshape Gemini 2.5 Flash produces when its input-side safety refuses on borderline imagery (empty content, nofinishReason: "SAFETY"→parseSchema(null, ...)throwsINVALID_ARGUMENT). Treated as an implicit block, returned directly without going through the retry queue (deterministic — retries just burn API calls).content-filter.ts, blocked-image routing to the failed-image path before placeholder substitution, placeholder swap on a copy of the original (so blocked originals are preserved), data-URL contentType fix.as anyand clarifies themaxOutputTokenscomment per Izaak's review on 🐛 [storage-resize-images] Blocked images by Vertex AI are not moved to the failed path due to bypassed catch block #2762.Tests
__tests__/content-filter.test.ts:ValidationError→ blocks without retrying (assertsmockGenerate.toHaveBeenCalledTimes(1))INVALID_ARGUMENT(real schema mismatch) → still propagates after retries__tests__/integration/content-filter.live.test.ts. Runs against real Vertex AI Gemini 2.5 Flash:BLOCK_ONLY_HIGH→ passesLIVE_BORDERLINE_IMAGE_PATH(image not committed; supply your own)npm run test:live-content-filter --prefix storage-resize-images/functions(env vars:GCLOUD_PROJECT, optionallyLIVE_BORDERLINE_IMAGE_PATH).izaak-testing(~9s).Behaviour change / out of scope
extension.yamlbumped to0.3.4; CHANGELOG updated.Test plan
npx tsc --noEmit -p tsconfig.jsoncleannpx jest __tests__/content-filter.test.ts— 10 pass (8 existing + 2 new)npx jest __tests__/unit/generateResizedImageHandler.test.ts— 4 pass unchangednpm run test:live-content-filteragainstizaak-testing— 3/3 passLIVE_BORDERLINE_IMAGE_PATH)contentFilterBlockedlog + placeholder substitution