fix: person_generation smart auto-routing to Imagen models - #14
Conversation
The PERSON_GENERATION_SDK_MAP had "block" mapped to "ALLOW_NONE", which is not a valid member of the PersonGeneration enum in the google-genai SDK. The correct value is "DONT_ALLOW". Updated the field description to clarify that person_generation is only supported on Imagen models and will be ignored with a warning on Gemini models.
…outing to Imagen The Gemini API (generate_content) does not support person_generation — the SDK raises a ValueError in _ImageConfig_to_mldev(). This commit removes the parameter from the Gemini ImageConfig construction entirely. Added GEMINI_TO_IMAGEN_MAP to map each Gemini model to its closest Imagen equivalent. When person_generation is set on a Gemini model and no conflicting Gemini-exclusive parameters (temperature, reference_images, thinking_budget) are present, the request is transparently auto-routed to the equivalent Imagen model and the result includes an auto_routed metadata block. When conflicts exist, the request falls through to the Gemini path and a warning is included in the result data explaining the conflict. Updated model capability metadata to correctly report person_generation as False for all Gemini models.
format_image_result now propagates warnings and auto_routed keys from result.data directly to the top-level MCP response dict, making them visible to MCP clients without requiring them to dig into the nested details object. Updated the generate_image tool docstring to describe the person_generation auto-routing behavior, and updated get_server_info to reflect that person_generation auto-routes to Imagen when applicable.
test_validation.py: - Validates PERSON_GENERATION_SDK_MAP values against the actual PersonGeneration enum members from google-genai SDK - Asserts "block" maps to DONT_ALLOW (regression guard for the ALLOW_NONE → DONT_ALLOW fix) test_api_client.py: - test_list_models_person_generation_capability: updated to assert Gemini models report False and Imagen models report True - TestPersonGenerationAutoRouting suite (4 new tests): - auto_routes_to_imagen_when_no_conflicts - no_auto_route_when_temperature_set (conflict path → warning) - auto_route_model_mapping (GEMINI_TO_IMAGEN_MAP contract) - no_auto_route_for_imagen_models (direct Imagen path unchanged) test_server.py: - warnings_propagated_to_response - auto_routed_propagated_to_response - no_warnings_when_not_in_data
Patch version bump for the person_generation routing bugfix release. Updated in both pyproject.toml and src/pixelforge_mcp/config.py.
Ahmed-aleryani
added
bug
|
/gemini-review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
The pull request introduces smart auto-routing for the person_generation parameter, allowing users to specify child safety or person blocks on Gemini models by routing them to an equivalent Imagen model when no conflicts occur.
This is a great improvement that overcomes a limitation of the Gemini generate_content API while providing clear feedback with warnings when routing is not possible.
However, I've noted a few issues:
safety_settingshould be included in the conflict list for auto-routing as it is currently ignored if auto-routed to Imagen.imagen-4.0-ultra-generate-001is missing from thelist_modelsresponse even though it's used in the auto-routing map.- The update of
PERSON_GENERATION_SDK_MAPto useDONT_ALLOWshould be confirmed to match the intended SDK version support.
Overall, the logic is solid and the test coverage is good. Recommendation is to address the conflict check and model listing for consistency.
| conflicts.append("temperature") | ||
| if reference_images: | ||
| conflicts.append("reference_images") | ||
| if thinking_budget is not None and thinking_budget > 0: |
There was a problem hiding this comment.
High Severity: The safety_setting parameter is not checked for conflicts here. If a user provides both person_generation and safety_setting, the safety settings will be lost when auto-routing to Imagen (since _generate_via_imagen4 does not yet support it). It should be added to the conflicts list.
There was a problem hiding this comment.
Addressing all review feedback — fixes incoming:
1. safety_setting conflict (line 632) — Fixed.
Added safety_setting to the conflict check, but only when non-default (!= "preset:strict"). The default is always set by Pydantic, so checking the raw value would break auto-routing entirely. Non-default values (e.g., "preset:relaxed") now correctly trigger a conflict. Imagen has its own safety filters (block_medium_and_above by default), so the default case is safe.
2. Warning message (line 478) — Updated.
Warning now mentions all conflict sources including safety_setting and aspect_ratio.
3. imagen-4.0-ultra-generate-001 in list_models (line 114) — Fixed.
Added the ultra model to list_models() output with correct specs. Since auto-routing actively routes to it, it should be discoverable.
4. DONT_ALLOW SDK compatibility (line 48) — Verified.
ALLOW_NONE was never a valid PersonGeneration enum member. Confirmed against installed SDK (v1.62.0) and official docs. DONT_ALLOW is correct across our pinned range (>=1.62.0,<2.0.0).
Additional fix discovered during review: Also added Imagen aspect ratio compatibility checking — ratios like 21:9, 1:4, 8:1 work on Gemini but not Imagen. These now correctly trigger a conflict instead of producing a confusing Imagen API error.
| f"person_generation='{person_generation}' was ignored" | ||
| " — could not auto-route to Imagen because" | ||
| " Gemini-exclusive parameters (temperature," | ||
| " reference_images, or thinking_budget) were also" |
There was a problem hiding this comment.
Low Severity: The warning message should be updated to mention safety_setting as well, if it's added to the conflict list in generate.
| # so we map each Gemini model to its closest Imagen equivalent. | ||
| GEMINI_TO_IMAGEN_MAP = { | ||
| "gemini-2.5-flash-image": "imagen-4.0-fast-generate-001", | ||
| "gemini-3.1-flash-image-preview": "imagen-4.0-generate-001", |
There was a problem hiding this comment.
Medium Severity: imagen-4.0-ultra-generate-001 is used in this map but is not included in the list_models output (line 1137 onwards). This could be confusing to users who see they are auto-routed to a model that isn't listed as available. It should probably be added to list_models for consistency.
| "allow": "ALLOW_ALL", | ||
| "adults_only": "ALLOW_ADULT", | ||
| "block": "ALLOW_NONE", | ||
| "block": "DONT_ALLOW", |
There was a problem hiding this comment.
Low Severity: Mapping 'block' to 'DONT_ALLOW' (previously 'ALLOW_NONE') matches the current google-genai SDK enum member for PersonGeneration. This seems to be a fix for an issue where the SDK would fail with the previous value. Worth double-checking for consistency with older SDK versions if they are still supported.
…ect ratios - Add IMAGEN_ASPECT_RATIOS constant (1:1, 3:4, 4:3, 9:16, 16:9) to document which ratios Imagen's generate_images API supports vs Gemini's broader set (panoramic etc.) - Fix temperature conflict check: was using `is not None` which always triggered because temperature defaults to 0.7 in the MCP tool; now checks `!= 0.7` so auto-routing works for the common case - Add safety_setting conflict check: non-default safety_setting (anything other than "preset:strict") now blocks auto-routing to Imagen - Add aspect_ratio conflict check: ratios outside IMAGEN_ASPECT_RATIOS (e.g. 21:9 panoramic) now block auto-routing - Update warning message to accurately describe all conflict conditions - Add imagen-4.0-ultra-generate-001 to list_models output (was missing; list returned 5 models instead of the correct 6)
- Refactor TestPersonGenerationAutoRouting with _make_imagen_mock_client and _make_gemini_mock_client helpers to eliminate repeated setup code - Add test_auto_routes_with_default_temperature: verifies temperature=0.7 (the default) does NOT block auto-routing to Imagen - Rename test_no_auto_route_when_temperature_set → test_no_auto_route_when_nondefault_temperature (temperature=0.5 blocks) - Add test_no_auto_route_when_nondefault_safety: safety_setting="preset:relaxed" blocks auto-routing - Add test_no_auto_route_when_incompatible_aspect_ratio: aspect_ratio=21:9 blocks auto-routing - Add test_auto_route_with_imagen_compatible_ratio: aspect_ratio=16:9 allows auto-routing - Add test_imagen_aspect_ratios_defined: verifies the constant covers common ratios and excludes panoramic ones (21:9, 1:4) - Update model count assertion 5 → 6 to account for added ultra model - Update version assertion 0.5.0 → 0.5.1 in test_config.py
* fix: revert person_generation auto-routing, use warn-only approach Removed GEMINI_TO_IMAGEN_MAP, IMAGEN_ASPECT_RATIOS, and the entire auto-routing block from generate(). The auto-routing silently swapped Gemini models for Imagen when person_generation was set, degrading quality (especially Arabic/RTL text rendering) without user consent. Updated the warning message to accurately explain that Gemini blocks photorealistic identifiable people by default but still generates fictional/illustrated people, and suggests Imagen models for strict API-level enforcement. * fix: remove auto_routed from server responses, update descriptions Removed auto_routed propagation from format_image_result() since the field no longer exists in generation results. Updated the generate_image docstring and server_info to reflect the warn-only behavior for person_generation on Gemini models. Updated the person_generation field description in validation.py to clarify enforcement scope. * test: replace auto-routing tests with warn-only tests Replaced TestPersonGenerationAutoRouting class (10 tests) with TestPersonGenerationWarnOnly class (2 focused tests) that verify: - person_generation on Gemini stays on Gemini (no silent model swap) - person_generation on Gemini produces a warning pointing to Imagen Removed auto_routed propagation test from test_server.py since the field is no longer present in generation responses. All 242 unit tests passing.
1 participant
Summary
Fixes
person_generationfailing for all Gemini models by adding smart auto-routing to equivalent Imagen models. Also corrects a wrong SDK enum mapping (ALLOW_NONE→DONT_ALLOW).Closes #13
What changed
PERSON_GENERATION_SDK_MAPnow maps"block"toDONT_ALLOW(the correctPersonGenerationenum member).ALLOW_NONEdoes not exist in the SDK and caused silent failures.person_generationfrom GeminiImageConfig: The Geminigenerate_contentAPI does not support this parameter — the SDK raisesValueErrorin_ImageConfig_to_mldev(). The parameter is now stripped from the Gemini code path entirely.person_generationis set on a Gemini model and no Gemini-exclusive parameters (temperature,reference_images,thinking_budget) conflict, the request is transparently auto-routed to the equivalent Imagen model viaGEMINI_TO_IMAGEN_MAP. The result includes anauto_routedmetadata block identifying the source and target models.person_generation, the request proceeds on the Gemini path and the response includes an actionablewarningslist explaining what to remove to enable person generation.person_generation: False.format_image_resultnow propagateswarningsandauto_routedto the top-level response so MCP clients see them without inspecting nesteddetails.Test plan
pytest tests/unit/ -v)test_person_generation_block_maps_to_dont_allow— regression guard for the enum fixTestPersonGenerationAutoRouting— 4 tests for the routing logicTestFormatImageResult— 3 new tests for warnings/auto_routed propagationVersion
0.5.0 → 0.5.1