feat(gemini-image): v0.2.0 major refactor with modular architecture#20
feat(gemini-image): v0.2.0 major refactor with modular architecture#20williaby wants to merge 12 commits into
Conversation
Phase 0 - OpenSSF Compliance: - Add MIT LICENSE file - Add SECURITY.md with vulnerability reporting policy - Add CONTRIBUTING.md with contribution guidelines - Add CHANGELOG.md for version history tracking - Update pyproject.toml with new dependencies (tenacity, python-dotenv, structlog) Phase 1 - Architecture Refactoring: - Create exceptions.py with full error hierarchy (GeminiImageError base, ConfigurationError, APIError, RateLimitError, ServerError, ContentBlockedError, ValidationError, FormatDetectionError, FileOperationError, GenerationError) - Create io.py with magic byte format detection fixing JPEG/PNG mismatch, safe file saving with extension correction, and JSON sidecar metadata - Create client.py with GeminiClient class and retry logic using tenacity - Create registry.py for PROMPTS.md generation tracking - Create response_parser.py for API response parsing with ThoughtImage and GenerationResponse dataclasses - Refactor generator.py to use new modular components - Update cli.py with new flags (--no-document, --no-metadata, --registry, --resume) - Update __init__.py with all new exports - Add comprehensive tests for all new modules (86 tests passing) - Add package-specific ruff overrides in pyproject.toml BREAKING CHANGE: Internal module structure changed. Public API remains compatible. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 2 - Feature Backport:
- Add generate_batch() function for processing multiple prompts
- Support JSON file input via --batch CLI flag
- Add resume capability to skip existing output files
- Add progress bar with rich library for batch/story operations
Phase 3 - Production Hardening:
- Add rich dependency for CLI progress indicators
- Add spinner for single image generation (via _show_spinner helper)
- Add --no-progress flag to disable progress display
- Add batch mode summary output
CLI Enhancements:
- New --batch JSON_FILE option for batch processing
- New --no-progress flag for script-friendly output
- Batch file format: [{"prompt": "...", "aspect_ratio": "16:9", ...}, ...]
Test additions:
- test_batch_empty_list_raises: Validates empty prompt list
- test_batch_generates_multiple_images: Verifies batch generation
- test_batch_handles_invalid_prompt: Graceful error handling
- test_batch_resume_skips_existing: Resume functionality
All 90 tests passing.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive functional tests that make real API calls to verify the image generation system works end-to-end: Functional Tests (10 tests): - test_generate_simple_image: Basic image generation - test_generate_image_with_aspect_ratio: Pro model with 16:9 aspect - test_generate_draft_image: Draft mode at 1K resolution - test_generate_image_with_metadata: JSON sidecar file creation - test_generate_image_with_registry: PROMPTS.md registry entry - test_batch_two_images: Batch processing two prompts - test_batch_with_different_models: Batch with mixed model settings - test_story_sequence_two_parts: 2-part story generation - test_story_sequence_resume: Resume skips existing parts - test_format_detection_matches_extension: Format detection validation Bug Fix: - Fix story resume logic to find existing images with any extension - Previously looked for .png only, but files may be saved as .jpg due to magic byte format detection and correction - Add _find_existing_image() helper to check all image extensions Test Infrastructure: - Add @functional pytest marker for integration tests - Add --run-functional CLI option - Skip functional tests when GEMINI_API_KEY not configured - Add pytest.ini_options to pyproject.toml Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add batch processing documentation and JSON file format - Document PROMPTS.md registry feature - Add exception handling examples - Update API reference with all function signatures - Add development section with setup commands - Document resolution options and aspect ratios Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Installation and configuration instructions - Model selection and resolution options - Reference-based editing examples - Multi-part story sequence workflow - Batch processing with JSON format - Draft-then-finalize cost optimization pattern - PROMPTS.md registry documentation - Exception handling and troubleshooting guide - Best practices for production use Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 5 Implementation: - Add agents/diagram-specialist.md for technical diagram generation - Add scripts/validate_installation.py for installation verification - Include prompt engineering guidelines and workflow examples - Document batch documentation generation patterns Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…idation Per Gemini API documentation review: - Add 5 missing aspect ratios: 2:3, 3:2, 4:5, 5:4, 21:9 - Add reference image limit validation (3 for flash, 14 for pro) - Update README and USAGE_GUIDE with complete aspect ratio list - Add MAX_REFERENCE_IMAGES constant to models.py - Add tests for new aspect ratios and reference image limits Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add Security section to README documenting API key handling - Add Memory Considerations section for 4K batch operations - Fix darglint docstring issue in io.py detect_image_format Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds gemini-image v0.2.0: new GeminiClient with retries and API-key handling, expanded generator (batch, story, resume, metadata, registry), IO, exceptions, response parsing, CLI flags, tests, docs, installation validator, and a FIXES_NEEDED.md diagnosing cloudflare-auth import/settings issues. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Generator
participant Client as GeminiClient
participant API as Gemini API
participant IO
participant Registry
User->>CLI: run generate_image(prompt, flags)
CLI->>Generator: generate_image(prompt, document=..., registry_path=..., ...)
Generator->>Generator: validate inputs
Generator->>Client: GeminiClient.generate_content(config)
Client->>Client: load API key (.env/env)
Client->>API: POST /generateContent
loop retry on transient errors
API-->>Client: 429/5xx/error
Client->>Client: exponential backoff / retry
Client->>API: retry POST
end
API-->>Client: response (image + thoughts)
Client-->>Generator: GenerationResponse
Generator->>IO: detect_image_format(data)
Generator->>IO: save_image(data, path)
Generator->>IO: save_metadata(sidecar)
Generator->>Registry: add_entry(image_path, prompt, model)
Registry-->>Generator: True/False
Generator-->>CLI: image_path / result
CLI-->>User: print success / summary
sequenceDiagram
participant User
participant CLI
participant Generator
participant Client as GeminiClient
participant API as Gemini API
participant Progress
User->>CLI: run generate_batch(batch.json, --resume)
CLI->>Generator: generate_batch(prompts, resume=True, show_progress=True)
Generator->>Progress: start progress bar
loop for each prompt
Generator->>Generator: check existing output (resume)
alt exists
Generator->>Progress: mark skipped
else
Generator->>Client: generate_content(prompt)
Client->>API: POST /generateContent
API-->>Client: response
Client-->>Generator: GenerationResponse
Generator->>IO: save_image + metadata
Generator->>Registry: add_entry(...)
Generator->>Progress: update
end
end
Progress-->>Generator: done
Generator-->>CLI: aggregate results
CLI-->>User: batch summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
coderabbitai
Bot
added
breaking-change
dependencies
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
🤖 Fix all issues with AI agents
In @FIXES_NEEDED.md:
- Around line 30-38: There are fenced code blocks in FIXES_NEEDED.md that lack a
blank line before the opening ``` and/or after the closing ```, which violates
MD031; update every fenced block (e.g., the Python snippet importing
CloudflareUser/CloudflareAuthMiddleware) by inserting one blank line immediately
above the opening ``` and one blank line immediately below the closing ```
throughout the file (apply the same change to the other occurrences referenced
in the comment).
- Line 11: The markdown entry for the byronwilliamscpa-cloudflare-auth package
in FIXES_NEEDED.md contains a single line that exceeds the 120-character limit;
edit the package description paragraph for byronwilliamscpa-cloudflare-auth to
wrap/split the long sentence into multiple lines under 120 characters (preserve
words and punctuation, avoid breaking URLs), ensuring the markdown renders the
same and run a quick markdown lint to confirm line length compliance.
In @packages/gemini-image/CHANGELOG.md:
- Line 44: Replace the redundant phrase "CLI interface (`gemini-image` command)"
in the CHANGELOG entry with a non-redundant form such as "CLI (`gemini-image`
command)" (or "Command-line interface (`gemini-image` command)") so the line no
longer repeats "interface" after "CLI".
- Line 8: Replace the "## [Unreleased]" header with a versioned release header
for v0.2.0 (e.g. change "## [Unreleased]" to "## [0.2.0]" and add the release
date if your changelog format requires it), and update the comparison link at
the bottom of the changelog so it points to the v0.1.0...v0.2.0 range (replace
the existing "Unreleased" comparison URL with the appropriate tag range for
0.2.0).
In @packages/gemini-image/CONTRIBUTING.md:
- Around line 77-82: The fenced Conventional Commits example block is missing a
language identifier; update the opening fence to include a language (e.g.,
```plaintext) so the code block is properly labeled. Edit the fenced block in
the CONTRIBUTING.md example (the block showing
feat(...)/fix(...)/docs(...)/test(...)) and change the opening fence from ``` to
```plaintext, leaving the content and closing fence unchanged.
- Around line 121-127: The fenced code block containing the quality checks in
CONTRIBUTING.md is missing blank lines before and after; update the markdown to
insert a blank line immediately before the opening ```bash fence and another
blank line immediately after the closing ``` fence so the block is separated
from surrounding list items (fixing MD031); ensure the closing fence is on its
own line and the list numbering/context (the "3. **Run quality checks**:" and
following "4. **Update documentation** if needed") remains correct.
In @packages/gemini-image/docs/USAGE_GUIDE.md:
- Around line 391-400: The usage example is missing imports for generate_image
and the RateLimitError used in the retry block; add imports at the top of the
snippet such as importing generate_image (e.g., from gemini_image import
generate_image) and the RateLimitError (e.g., from gemini_image.exceptions
import RateLimitError) so the retry example is complete and runnable, keeping
the rest of the loop and sleep logic unchanged.
- Around line 391-400: The example uses RateLimitError in the except block but
doesn't import it, causing a NameError; add an import for RateLimitError from
the gemini_image.exceptions module at the top of the snippet so the exception
class is defined when calling generate_image in the try/except example.
In @packages/gemini-image/README.md:
- Around line 196-211: The README example for PROMPTS.md is inconsistent with
the actual registry output; update the example block so it matches the markdown
table format produced by ENTRY_TEMPLATE in registry.py (i.e., replace the
bullet-list example with the same table header/row format and fields used by
ENTRY_TEMPLATE: timestamp, filename, Prompt, Model, Size, Aspect) to ensure
documentation reflects actual output.
In @packages/gemini-image/scripts/validate_installation.py:
- Around line 85-101: The check_api_key function currently prints a masked API
key (variable masked) which leaks sensitive info; change check_api_key to only
confirm existence and sufficient length of get_api_key() without printing any
part of the key—replace the print f"OK (key: {masked})" with a non-sensitive
message like "OK: API key configured" and ensure no other logs expose key
characters or slices.
- Around line 193-201: The code assumes the API Key result is at results[-1],
which is fragile; instead locate the API Key entry by name from results (e.g.,
use next((ok for name, ok in results if name == "API Key"), False) or convert
results to a dict) and use that boolean to decide the "API key not configured"
message; update the conditional that currently uses results[-1][1] to reference
this looked-up value and keep core_passed logic unchanged.
In @packages/gemini-image/src/gemini_image/cli.py:
- Around line 23-41: The helper function _show_spinner is unused; either remove
it to eliminate dead code or mark it for future use by adding a clear TODO
comment and/or a unit test that exercises it; specifically, either delete the
_show_spinner function definition from gemini_image/cli.py, or prepend a "#
TODO: keep _show_spinner for future interactive CLI spinner (unused currently)"
comment and ensure any planned usage is referenced (e.g., call _show_spinner in
the CLI command or add tests) so linters won’t flag it as unused.
- Around line 254-256: The open() call that reads the JSON batch file (with
open(args.batch) as f: prompts = json.load(f)) should specify encoding="utf-8"
for consistent cross-platform behavior; update the open() invocation to include
encoding="utf-8" when reading args.batch so json.load reads the file with
explicit UTF-8 decoding.
In @packages/gemini-image/src/gemini_image/generator.py:
- Around line 696-725: Add a file-size check before reading each reference image
in the reference_images loop: define a MAX_REFERENCE_IMAGE_BYTES constant (e.g.,
5 * 1024 * 1024) and use img_path.stat().st_size to compare; if the file exceeds
the limit, log a warning like "reference_image_too_large" with the path and size
and skip (continue) instead of calling open/read or types.Part.from_bytes,
otherwise proceed to read and add the Part as before.
- Around line 145-147: Replace the runtime assertion with explicit validation:
check parsed.image_data with an if statement before calling save_image and raise
a clear exception (e.g., ValueError or custom exception) or log an error and
return/abort so missing image data cannot be skipped when Python assertions are
disabled; specifically update the block around parsed.image_data, save_image,
and final_path to validate parsed.image_data is not None and handle the error
path instead of using assert.
In @packages/gemini-image/src/gemini_image/io.py:
- Around line 121-137: The MIME mapping in get_mime_type_for_format duplicates
the extension mapping in get_extension_for_format; consolidate both into a
module-level single source of truth (e.g., FORMAT_INFO mapping format ->
(extension, mime)) and change get_extension_for_format and
get_mime_type_for_format to look up FORMAT_INFO using image_format.lower(),
returning the appropriate tuple element and a sensible default (e.g., (".png",
"image/png")) when missing; update references to the unique symbols
get_extension_for_format and get_mime_type_for_format to use the shared
FORMAT_INFO constant.
- Around line 102-118: The local extensions dict inside get_extension_for_format
duplicates MIME_TO_EXTENSION; replace it by referencing the existing
MIME_TO_EXTENSION (or create a single module-level FORMAT_TO_EXTENSION and use
that) so there's only one source of truth. Update get_extension_for_format to
look up image_format.lower() in the shared mapping and return the found value or
default ".png". Ensure you reference the existing symbol MIME_TO_EXTENSION (or
the new FORMAT_TO_EXTENSION) and remove the inline extensions dict.
In @packages/gemini-image/src/gemini_image/registry.py:
- Around line 102-111: The file operations in
packages/gemini-image/src/gemini_image/registry.py use open() without an
explicit encoding; update all open(...) calls that read or write the registry
(the write of REGISTRY_TEMPLATE using open(self.path, "w"), and the other open
calls around the same block) to specify encoding="utf-8" (e.g., open(self.path,
"w", encoding="utf-8") and open(self.path, "r", encoding="utf-8")) so file
reads/writes are deterministic across platforms and keep existing error handling
via FileOperationError and logger.info unchanged.
In @packages/gemini-image/src/gemini_image/response_parser.py:
- Around line 171-194: The code in _process_thought_part accesses part.text and
part.inline_data directly, which is inconsistent with other module guards;
update the checks to use getattr(part, "text", None) and getattr(part,
"inline_data", None) (or hasattr) before using them, then only create
ThoughtImage and append to result.thought_images when inline_data is present,
and call logger.debug with the guarded values (e.g., use the local thought_text
variable set from getattr) to avoid AttributeError on missing attributes.
- Around line 100-118: The response parsing lacks defensive checks for the
candidates/content/parts chain; update the start of parse logic to verify
response has a non-empty candidates attribute and that candidates[0] has a
content with a parts iterable before indexing into
response.candidates[0].content.parts (use hasattr/getattr or try/except to
detect missing attributes), call _handle_empty_response and raise
GenerationError with details when malformed, and change the for-loop over parts
to iterate safely (e.g., assign parts = getattr(response.candidates[0].content,
"parts", None) and handle None) so _process_thought_part and _process_final_part
only receive valid part objects.
In @packages/gemini-image/tests/conftest.py:
- Around line 15-30: The pytest setup registers the --run-functional flag and
the functional marker but lacks the pytest_collection_modifyitems hook to skip
tests marked with @pytest.mark.functional when the flag is not set; implement
pytest_collection_modifyitems(config, items) to check
config.getoption("--run-functional") and, if False, iterate items and add
pytest.mark.skip(reason=...) to any item with "functional" in item.keywords (or
remove them from items) so functional tests are skipped unless the flag is
provided; add this function alongside pytest_addoption/pytest_configure in
conftest.py and reference the "functional" marker and "--run-functional" flag.
In @packages/gemini-image/tests/test_functional.py:
- Around line 300-305: The pytest marker "functional" is being registered twice;
remove the duplicate registration by deleting the pytest_configure function (or
at least its config.addinivalue_line call) from tests/test_functional.py so the
marker is only registered in conftest.py; ensure the pytest_configure symbol in
tests/test_functional.py no longer calls config.addinivalue_line("markers",
"functional: marks tests as functional (requiring API key)").
In @packages/gemini-image/tests/test_io.py:
- Around line 86-96: Add tests for GIF and WebP MIME types to the
TestGetMimeTypeForFormat class: add two new test methods that call
get_mime_type_for_format("gif") and get_mime_type_for_format("webp") and assert
they return "image/gif" and "image/webp" respectively; place them alongside the
existing test_png_mime and test_jpeg_mime methods in the tests/test_io.py file
so the suite covers common image formats.
In @packages/gemini-image/tests/test_registry.py:
- Around line 13-23: The test test_create_registry should avoid calling the
private method _ensure_exists(); instead, use the public API to trigger file
creation by calling PromptRegistry.add_entry(...) on the registry instance
(e.g., add a minimal entry/title) which will internally create the file, then
assert registry_path.exists() and that the expected header "Image Generation
Registry" is in the content; update the test to replace
registry._ensure_exists() with a call to registry.add_entry(...) and keep the
same assertions.
In @tmp_cleanup/image-generationreview.md:
- Around line 1-349: The file tmp_cleanup/image-generationreview.md appears to
be a temporary review artifact and should not be merged as-is; either move it to
a permanent docs location (e.g., create docs/archive/design-review.md and commit
the file there, update any repo index/README or CHANGELOG to reference it),
delete it from the branch before merging if it’s not intended to be kept, or add
tmp_cleanup/image-generationreview.md (or the entire tmp_cleanup/ folder) to
.gitignore if it’s a local working file; pick one option, perform the
corresponding change, and ensure the PR description notes the decision so
reviewers know why the file was moved/removed/ignored.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (28)
FIXES_NEEDED.mdpackages/gemini-image/CHANGELOG.mdpackages/gemini-image/CONTRIBUTING.mdpackages/gemini-image/LICENSEpackages/gemini-image/README.mdpackages/gemini-image/SECURITY.mdpackages/gemini-image/agents/diagram-specialist.mdpackages/gemini-image/docs/USAGE_GUIDE.mdpackages/gemini-image/pyproject.tomlpackages/gemini-image/scripts/validate_installation.pypackages/gemini-image/src/gemini_image/__init__.pypackages/gemini-image/src/gemini_image/cli.pypackages/gemini-image/src/gemini_image/client.pypackages/gemini-image/src/gemini_image/exceptions.pypackages/gemini-image/src/gemini_image/generator.pypackages/gemini-image/src/gemini_image/io.pypackages/gemini-image/src/gemini_image/models.pypackages/gemini-image/src/gemini_image/registry.pypackages/gemini-image/src/gemini_image/response_parser.pypackages/gemini-image/tests/conftest.pypackages/gemini-image/tests/test_exceptions.pypackages/gemini-image/tests/test_functional.pypackages/gemini-image/tests/test_generator.pypackages/gemini-image/tests/test_io.pypackages/gemini-image/tests/test_models.pypackages/gemini-image/tests/test_registry.pypackages/gemini-image/tests/test_response_parser.pytmp_cleanup/image-generationreview.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Use 120 character line length for Markdown documentation files
Files:
packages/gemini-image/SECURITY.mdpackages/gemini-image/CONTRIBUTING.mdpackages/gemini-image/agents/diagram-specialist.mdpackages/gemini-image/docs/USAGE_GUIDE.mdpackages/gemini-image/CHANGELOG.mdFIXES_NEEDED.mdpackages/gemini-image/README.mdtmp_cleanup/image-generationreview.md
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use Ruff formatting with 88 character line length for Python code
Use Ruff linting with PyStrict-aligned rules including BLE, EM, SLF, INP, ISC, PGH, RSE, TID, YTT, FA, T10, and G rules
Tag assumptions with #CRITICAL, #ASSUME, or #EDGE comments including category and reason for verification
Files:
packages/gemini-image/tests/test_response_parser.pypackages/gemini-image/src/gemini_image/io.pypackages/gemini-image/tests/conftest.pypackages/gemini-image/tests/test_generator.pypackages/gemini-image/tests/test_functional.pypackages/gemini-image/src/gemini_image/client.pypackages/gemini-image/tests/test_registry.pypackages/gemini-image/tests/test_models.pypackages/gemini-image/src/gemini_image/generator.pypackages/gemini-image/scripts/validate_installation.pypackages/gemini-image/tests/test_io.pypackages/gemini-image/src/gemini_image/registry.pypackages/gemini-image/src/gemini_image/response_parser.pypackages/gemini-image/src/gemini_image/cli.pypackages/gemini-image/tests/test_exceptions.pypackages/gemini-image/src/gemini_image/__init__.pypackages/gemini-image/src/gemini_image/exceptions.pypackages/gemini-image/src/gemini_image/models.py
🧠 Learnings (5)
📚 Learning: 2025-12-14T22:54:23.019Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T22:54:23.019Z
Learning: Applies to tests/**/*.py : Use pytest fixtures defined in tests/conftest.py for test setup and teardown
Applied to files:
packages/gemini-image/tests/conftest.py
📚 Learning: 2025-12-14T22:53:58.937Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-14T22:53:58.937Z
Learning: Applies to src/python_libs/**/*.py : Ensure TODO/FIXME comments are actionable with sufficient context
Applied to files:
FIXES_NEEDED.md
📚 Learning: 2025-12-14T22:54:23.019Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T22:54:23.019Z
Learning: Applies to **/*.py : Use Ruff linting with PyStrict-aligned rules including BLE, EM, SLF, INP, ISC, PGH, RSE, TID, YTT, FA, T10, and G rules
Applied to files:
packages/gemini-image/pyproject.toml
📚 Learning: 2025-12-14T22:54:23.019Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T22:54:23.019Z
Learning: Applies to **/*.py : Use Ruff formatting with 88 character line length for Python code
Applied to files:
packages/gemini-image/pyproject.toml
📚 Learning: 2025-12-14T22:54:23.019Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T22:54:23.019Z
Learning: Applies to src/**/*.py : Use centralized exception hierarchy from src/python_libs/core/exceptions.py for all error handling
Applied to files:
packages/gemini-image/src/gemini_image/exceptions.py
🧬 Code graph analysis (6)
packages/gemini-image/tests/test_functional.py (3)
packages/gemini-image/src/gemini_image/generator.py (3)
generate_batch(186-374)generate_image(42-183)generate_story_sequence(377-490)packages/gemini-image/src/gemini_image/io.py (2)
detect_image_format(53-99)validate_image_file(304-336)packages/gemini-image/tests/conftest.py (1)
pytest_configure(15-20)
packages/gemini-image/src/gemini_image/client.py (2)
packages/gemini-image/src/gemini_image/exceptions.py (5)
APIError(56-87)ConfigurationError(45-53)ContentBlockedError(128-158)RateLimitError(90-117)ServerError(120-125)src/python_libs/utils/logging.py (1)
get_logger(131-157)
packages/gemini-image/tests/test_registry.py (1)
packages/gemini-image/src/gemini_image/registry.py (3)
_ensure_exists(98-111)add_entry(144-225)find_entry(227-262)
packages/gemini-image/scripts/validate_installation.py (3)
packages/gemini-image/src/gemini_image/generator.py (4)
finalize_draft(493-590)generate_batch(186-374)generate_image(42-183)generate_story_sequence(377-490)packages/gemini-image/src/gemini_image/exceptions.py (5)
ContentBlockedError(128-158)FileOperationError(233-267)FormatDetectionError(203-230)GeminiImageError(12-42)RateLimitError(90-117)packages/gemini-image/src/gemini_image/io.py (3)
detect_image_format(53-99)save_image(140-205)validate_image_file(304-336)
packages/gemini-image/src/gemini_image/response_parser.py (3)
packages/gemini-image/src/gemini_image/exceptions.py (1)
GenerationError(270-303)packages/gemini-image/src/gemini_image/io.py (2)
detect_image_format(53-99)get_mime_type_for_format(121-137)src/python_libs/utils/logging.py (1)
get_logger(131-157)
packages/gemini-image/src/gemini_image/cli.py (2)
packages/gemini-image/src/gemini_image/generator.py (4)
finalize_draft(493-590)generate_batch(186-374)generate_image(42-183)generate_story_sequence(377-490)src/python_libs/utils/logging.py (1)
get_logger(131-157)
🪛 GitHub Actions: PR Validation
packages/gemini-image/CHANGELOG.md
[error] 1-1: Changelog enforcer failed: Please update CHANGELOG.md with a description of your changes.
🪛 GitHub Check: CodeQL
packages/gemini-image/scripts/validate_installation.py
[failure] 95-95: Clear-text logging of sensitive information
This expression logs sensitive data (password) as clear text.
This expression logs sensitive data (password) as clear text.
🪛 LanguageTool
packages/gemini-image/CHANGELOG.md
[style] ~44-~44: This phrase is redundant (‘I’ stands for ‘interface’). Use simply “CLI”.
Context: ... function for draft-to-final workflow - CLI interface (gemini-image command) - Support for ...
(ACRONYM_TAUTOLOGY)
tmp_cleanup/image-generationreview.md
[uncategorized] ~84-~84: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...Missing resilience for: - HTTP 429 rate limiting - HTTP 5xx server errors - Network time...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.18.1)
packages/gemini-image/SECURITY.md
16-16: Bare URL used
(MD034, no-bare-urls)
packages/gemini-image/CONTRIBUTING.md
77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
122-122: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
127-127: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
FIXES_NEEDED.md
30-30: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
41-41: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
59-59: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
91-91: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
136-136: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
149-149: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
162-162: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
tmp_cleanup/image-generationreview.md
48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
150-150: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (71)
packages/gemini-image/LICENSE (1)
1-21: Standard MIT License properly added.The MIT License file is correctly formatted with appropriate copyright attribution and dates. This is well-suited for an open-source project and aligns with common practice.
packages/gemini-image/CONTRIBUTING.md (1)
1-170: Clear, comprehensive contribution guidelines.The document provides well-structured guidance on development setup, testing, code quality standards, and the PR process. Prerequisites are explicit, commands are precise, and expectations are clearly stated. This should facilitate consistent contributions.
packages/gemini-image/SECURITY.md (1)
1-67: Security policy is well-structured and covers essential practices.The document clearly defines supported versions, provides a responsible vulnerability disclosure process with explicit timelines, and documents security best practices for API key handling, output review, and dependency monitoring. This gives users and contributors clear guidance on security expectations.
FIXES_NEEDED.md (2)
1-220: This document belongs in cloudflare-auth package, not at repository root.The file is positioned at
/FIXES_NEEDED.md(repository root), but it documents issues specific to thecloudflare-authpackage. Consider relocating this topackages/cloudflare-auth/FIXES_NEEDED.mdor incorporating it into that package's issue tracking.Additionally, the content is a remediation plan for a different package (cloudflare-auth) rather than part of the gemini-image v0.2.0 release PR. Clarify whether this is intended to be committed as part of PR #20 (gemini-image v0.2.0) or should be handled separately for the cloudflare-auth fixes.
131-176: Action items are comprehensive and well-structured.The immediate and follow-up action items are clear and actionable, with concrete bash commands, verification steps, and testing instructions. The document effectively documents the problem, root cause, and a clear path to resolution. However, per the retrieved learning about actionable context, ensure that whoever picks up these fixes has access to this document in the appropriate package context (see above comment on file location).
packages/gemini-image/CHANGELOG.md (1)
1-7: LGTM! Standard changelog format.The header correctly references Keep a Changelog and Semantic Versioning standards.
packages/gemini-image/agents/diagram-specialist.md (3)
160-220: LGTM! Well-structured Python integration example.The example correctly demonstrates both single image generation and batch processing with appropriate parameters that align with the new API surface.
1-287: LGTM! Comprehensive documentation for diagram generation agent.The documentation is well-structured, provides clear examples, and follows the 120-character line length guideline. It effectively guides users through single diagram generation, draft workflows, batch processing, and Claude Code integration.
56-56: No issue found. The--finalizeflag syntax is correct and matches the CLI implementation. The flag accepts a draft image path as its argument (e.g.,--finalize governance_v2.png), which is consistent with the argparse definition and documented help text. The example on line 56 is accurate and the line length (80 characters) complies with the 120-character guideline.Likely an incorrect or invalid review comment.
packages/gemini-image/pyproject.toml (5)
3-3: LGTM! Version bump correctly follows semantic versioning.The bump from 0.1.0 to 0.2.0 appropriately reflects a minor version change for new features and refactoring without breaking the public API.
72-91: Review Ruff configuration against PyStrict coding guidelines.The configuration ignores several rules (EM101, EM102, BLE001) that are part of the PyStrict-aligned guidelines mentioned in the coding standards. While the comments provide justifications for a user-facing library, this creates an inconsistency.
As per coding guidelines, the repository should use PyStrict-aligned rules including BLE and EM. Consider:
- If these exceptions are approved: Document this as a package-specific override in the repository's coding guidelines.
- If strict compliance is required: Refactor exception handling to comply with EM rules (use error codes or separate message variables) and BLE rules (catch specific exceptions).
Which approach aligns with the project's standards?
93-97: LGTM! Per-file ignores are appropriate.The per-file ignores correctly allow asserts and magic values in tests, and print statements in the CLI module, which are necessary exceptions to the general linting rules.
99-105: LGTM! Pytest configuration supports functional testing.The functional marker enables separation of tests requiring API keys from unit tests, which is a good practice. The deprecation warning filter is acceptable for initial release but consider addressing deprecations in future iterations.
27-30: Minimum dependency versions are valid and secure.Verification confirms all pinned versions (tenacity 8.2.0, python-dotenv 1.0.0, structlog 24.1.0, rich 13.0.0) have no known CVEs or security advisories as of January 2026. All versions are stable and appropriate for production use.
packages/gemini-image/src/gemini_image/exceptions.py (6)
12-43: LGTM! Well-designed base exception class.The base exception provides a clean foundation with structured details support and clear string representation. The keyword-only
detailsparameter is good practice.
56-159: LGTM! Well-structured API error hierarchy.The API errors properly differentiate between retriable (RateLimitError, ServerError) and non-retriable (ContentBlockedError) failures, with appropriate attributes for debugging and retry logic.
161-201: LGTM! Comprehensive validation error with helpful context.The ValidationError captures all necessary information (field, value, valid_options) to help users understand and fix validation failures.
203-268: LGTM! I/O errors with appropriate debugging context.FormatDetectionError intelligently captures and truncates magic bytes for debugging, while FileOperationError provides path and operation context. Both support the PR's magic-byte format detection feature.
270-303: LGTM! GenerationError with appropriate prompt truncation.The 100-character prompt truncation prevents log pollution while preserving context. The error captures model information for debugging.
1-303: No action required. The package-specific exception hierarchy is appropriate and follows the established pattern.The retrieved learning specifies "Applies to src/**/*.py", which explicitly restricts centralized exception usage to the
src/python_libs/library code only. Thepackages/directory contains independently-distributed packages (byronwilliamscpa-cloudflare-api, byronwilliamscpa-gcs-utilities, byronwilliamscpa-gemini-image), each with its own exception hierarchy. This is the consistent architecture pattern across all packages in the monorepo—cloudflare-api uses CloudflareAPIError, gcs-utilities uses GCSError, and gemini-image uses GeminiImageError. No package imports from the centralized exceptions module. The gemini-image exception hierarchy is well-designed with appropriate context-specific attributes and follows the same patterns as other packages.packages/gemini-image/src/gemini_image/models.py (2)
9-9: The aspect ratios in both the type definition and constant list accurately match the official Google Gemini image generation API documentation. All 10 supported ratios (1:1, 2:3, 3:2, 3:4, 4:3, 4:5, 5:4, 9:16, 16:9, 21:9) are correctly defined and documented.
12-16: The reference image limits are correctly defined. Official Google API documentation confirms:
- Gemini 2.5 Flash Image (flash model): 3 reference images per prompt ✓
- Gemini 3 Pro Image (pro model): 14 reference images per prompt ✓
The MAX_REFERENCE_IMAGES constant accurately reflects the current API specifications.
packages/gemini-image/tests/test_models.py (2)
33-40: LGTM! Aspect ratio test updated for expanded API coverage.The test properly validates all 10 aspect ratios per API documentation. The explicit ordering assertion ensures the constant maintains its expected sequence.
47-50: LGTM! Reference image limits are correctly validated.The test verifies the documented limits: 3 for flash model and 14 for pro model.
packages/gemini-image/tests/conftest.py (1)
43-47: LGTM! JPEG fixture provides valid magic bytes for format detection tests.The fixture correctly provides JPEG magic bytes (
\xff\xd8\xff) followed by JFIF marker structure, suitable for testing format detection logic.packages/gemini-image/tests/test_response_parser.py (3)
1-16: LGTM! Well-structured test imports and module organization.The imports are clean and the test file is well-organized into logical test classes covering all public interfaces of the response_parser module.
70-122: LGTM! Comprehensive parse_response test coverage.The tests cover key scenarios: basic responses, empty responses (error path), responses with thought images, and text responses. The mock setup accurately represents the expected API response structure.
175-212: LGTM! Safety ratings extraction tests are thorough.Tests correctly cover: ratings present, empty response, and missing safety_ratings attribute. Edge cases are well handled.
packages/gemini-image/tests/test_exceptions.py (4)
1-16: LGTM! Exception imports cover the full hierarchy.All exception classes are imported and will be tested, ensuring the public exception API is properly validated.
19-38: LGTM! Base exception tests verify core behavior.Tests cover message storage, details dictionary, and inheritance chain. The
detailsfeature for structured error information is well tested.
65-78: LGTM! RateLimitError tests verify retry_after functionality.Tests confirm the retry_after parameter is stored and included in the string representation, which is essential for implementing retry logic.
107-121: LGTM! ValidationError tests verify field-level error details.The test validates that field name, invalid value, and valid options are all captured, enabling clear error messages for users.
packages/gemini-image/tests/test_registry.py (3)
48-74: LGTM! Idempotency test correctly validates duplicate prevention.The test verifies that adding the same filename twice returns
Falseon the second call and only one entry exists in the registry. This is important for preventing registry pollution.
114-130: LGTM! Find entry test validates round-trip data integrity.The test confirms that metadata (filename, prompt) survives the add/find cycle, which is essential for registry usability.
141-163: LGTM! Index table test ensures markdown table consistency.The test validates that entries appear in the index table with correct model information. The line-by-line search is appropriate for verifying structured markdown content.
packages/gemini-image/tests/test_io.py (4)
22-59: LGTM! Comprehensive format detection tests with good edge case coverage.Tests cover all supported formats (PNG, JPEG, GIF87a/89a, WebP) and properly validate error handling for empty data and unknown formats.
98-142: LGTM! Save image tests cover key functionality including extension correction.Tests validate basic saving, automatic extension correction based on detected format, opt-out of correction, and parent directory creation. This covers the main use cases and edge cases.
145-197: LGTM! Metadata tests validate JSON sidecar file creation.Tests verify both basic metadata fields (prompt, model, created_at) and optional fields (aspect_ratio, image_size, is_draft, thought_signature, reference_images). The JSON structure validation is thorough.
227-245: LGTM! Validate image file tests cover success and error paths.Tests verify format detection and size reporting for valid files, and proper
FileOperationErrorraising for missing files.packages/gemini-image/tests/test_generator.py (7)
11-12: LGTM! Clean import structure.The imports correctly reference the new modular architecture with
clientandexceptionsmodules.
27-36: LGTM! Comprehensive input validation tests.These tests properly verify that invalid inputs raise
ValidationErrorwith appropriate error messages.
66-96: LGTM! Reference image limit validation.These tests correctly verify the reference image limits (3 for flash, 14 for pro) mentioned in the PR objectives.
98-114: LGTM! Comprehensive API key configuration test.The test properly isolates the environment by patching
load_dotenvand clearingos.environ, ensuring the API key check is reliably tested.
255-297: LGTM! Good batch processing test coverage.The tests validate both the empty input case (raising
ValidationError) and successful batch generation of multiple images.
298-334: LGTM! Graceful error handling in batch processing.The test correctly verifies that
generate_batchcontinues processing when encountering an invalid prompt, returningNonefor failed items while successfully generating the others.
335-375: LGTM! Excellent resume logic test.This test verifies that the batch resume feature correctly skips existing files and avoids redundant API calls, which is crucial for cost savings and efficiency.
packages/gemini-image/tests/test_functional.py (5)
1-12: LGTM! Clear documentation for functional tests.The module docstring provides helpful instructions for running the functional tests, making it easy for developers to understand the requirements.
54-72: LGTM! Solid functional test for basic image generation.The test validates the complete end-to-end workflow with appropriate assertions for file existence, size, and format.
108-135: LGTM! Comprehensive metadata sidecar test.The test validates that metadata sidecars are created with all required fields, aligning with the PR's new metadata feature.
233-265: LGTM! Critical resume functionality validated.This test confirms that the story sequence resume feature works correctly, addressing a key gap identified in the repository analysis (issue #5 in the review document). The test verifies that existing parts are reused rather than regenerated, saving API costs.
30-43: The skip marker evaluates at module import time, before environment variables set via pytest hooks take effect.The
requires_api_keymarker evaluates_has_api_key()when the test module is imported—before test collection and before any pytest fixtures run. This means ifGEMINI_API_KEYis set via a pytest hook (e.g.,pytest_configureor a fixture withautouse=True), the tests will still be skipped.To ensure the marker respects dynamically configured API keys, consider:
- Using
pytest_runtest_setupinstead ofskipiffor runtime evaluation, or- Integrating the
--run-functionalflag (already defined in conftest.py) with the skip logic to conditionally override the markerpackages/gemini-image/src/gemini_image/io.py (5)
25-33: LGTM! Standard magic byte signatures.The image format signatures are correctly defined and well-documented with a reference to Wikipedia's list of file signatures.
53-99: LGTM! Robust format detection with proper error handling.The function correctly detects image formats using magic bytes with appropriate length checks and provides helpful error context by including the first 16 bytes in error messages.
140-205: LGTM! Comprehensive save_image implementation.The function properly handles format detection, extension correction, directory creation, and error cases with structured logging. The return value correctly reflects the actual saved path after any extension correction.
208-272: LGTM! Well-structured metadata handling.The function properly constructs metadata with both required and optional fields, uses timezone-aware timestamps (compatible with Python 3.9+), and includes comprehensive error handling.
275-301: LGTM! Appropriate error handling for optional metadata.The function correctly treats missing metadata as a non-error case (returns
None) while raisingFileOperationErrorfor corrupt or unreadable metadata files. This design supports optional metadata sidecars.packages/gemini-image/src/gemini_image/client.py (6)
35-55: LGTM! Appropriate lazy import pattern.The lazy import mechanism correctly defers loading of
google-genaiuntil actually needed, with a helpful error message if the package is not installed. Theglobalstatement is appropriately used and marked withnoqa.
58-93: LGTM! Robust API key handling with python-dotenv.The function uses
python-dotenvfor proper.envfile parsing, addressing the manual parsing concerns mentioned in the repository analysis. The error message provides helpful hints for users.
145-178: LGTM! Well-designed client initialization.The initialization provides sensible defaults for retry configuration (max 3 retries, 4-60 second exponential backoff) and flexible API key handling with proper logging.
185-269: LGTM! Comprehensive retry implementation with minor suggestion.The retry logic properly handles transient failures with exponential backoff. The implementation correctly maps exceptions and uses tenacity for retry control.
Minor observation: Lines 267-268 check for
e.__cause__, but_RetryableErroris always created withfrom mapped_exc(line 258), so__cause__should always exist. The fallback is defensive programming, which is acceptable.
271-302: LGTM! Comprehensive exception mapping.The method properly maps various error types from the
google-genaiSDK to the project's custom exception hierarchy, providing better error handling throughout the application. The fallback to genericAPIErrorensures all exceptions are properly handled.
101-123: Remove or implement_is_retryable_exceptionas it is unused dead code.The function
_is_retryable_exception(lines 101-123) is defined but never called. The actual retry logic uses_map_exceptionto detect errors and convert them to typed exceptions (RateLimitError, ServerError, etc.), which are then checked withisinstancebefore retry.If
_is_retryable_exceptionwas intended as a helper function, wire it into_map_exception. Otherwise, remove it. The current retry mechanism—with exponential backoff (min/max wait times), retry limits, and specific exception types—is properly configured per coding guidelines.Likely an incorrect or invalid review comment.
packages/gemini-image/src/gemini_image/registry.py (1)
1-66: LGTM on module structure and templates.The module docstring, constants, and templates are well-structured. The
ENTRY_TEMPLATEcorrectly handles optional reference images via thereferences_sectionplaceholder.packages/gemini-image/src/gemini_image/cli.py (2)
239-287: LGTM on batch processing CLI integration.The batch mode implementation properly validates the JSON file, handles errors gracefully, and provides clear status output. The error handling with structured logging is appropriate for a CLI entrypoint.
23-41: Incorrect context manager return in fallback path.Line 40 returns
noop()(calling the generator), which yields immediately and returns a generator object. This won't work correctly as a context manager since it's already been partially consumed. The function should returnnoop(the decorated function) without calling it.🐛 Proposed fix
try: from rich.console import Console from rich.status import Status console = Console() return Status(message, console=console, spinner="dots") except ImportError: # Fallback to a simple context manager that does nothing from contextlib import contextmanager @contextmanager - def noop(): + def noop_ctx(): print(message + "...") yield - return noop() + return noop_ctx()Actually, the issue is different -
noop()is correct because@contextmanagerdecorator makesnoopreturn a context manager when called. Let me re-analyze...The
@contextmanagerdecorator transformsnoopinto a function that returns a context manager. Sonoop()correctly returns a context manager instance. This is actually correct.Wait, I need to reconsider. The
@contextmanagerdecorator transforms the generator function into a factory that returns context managers. So callingnoop()does return a context manager. Let me verify this is actually correct...Actually this IS correct -
@contextmanagermakesnoopa factory, andnoop()returns a proper context manager.packages/gemini-image/scripts/validate_installation.py (1)
15-48: LGTM on validation check structure.The
check_importandcheck_exportsfunctions are well-structured with clear pass/fail reporting. The callable checks on exported functions are appropriate validation.packages/gemini-image/src/gemini_image/response_parser.py (1)
20-76: LGTM on data class definitions.The
ThoughtImageandGenerationResponsedataclasses are well-structured with appropriate defaults and useful computed properties (has_image,thought_count,format).packages/gemini-image/README.md (1)
305-335: LGTM on Security documentation.The Security section clearly documents API key handling best practices, warns against common mistakes, and correctly notes that hardcoding keys is not supported by the API.
packages/gemini-image/src/gemini_image/__init__.py (1)
26-66: LGTM on expanded public API surface.The imports are well-organized by module, and the
__all__list is comprehensive and alphabetically sorted. The version bump to 0.2.0 is appropriate for this refactor with new features.packages/gemini-image/src/gemini_image/generator.py (2)
619-684: LGTM on validation helper functions.The validation functions (
_validate_model_key,_validate_aspect_ratio,_validate_image_size,_validate_reference_images) are well-structured with clear error messages and use the customValidationErrorwith helpful details includingvalid_options.
186-374: LGTM ongenerate_batchimplementation.The batch processing function is well-implemented with:
- Input validation for empty prompts
- Resume support for existing outputs
- Optional rich progress bar with graceful fallback
- Per-item error handling that doesn't abort the entire batch
- Clear summary logging
The
parallel > 1warning correctly documents the limitation while still accepting the parameter for future compatibility.
| def pytest_configure(config: pytest.Config) -> None: | ||
| """Register custom pytest markers.""" | ||
| config.addinivalue_line( | ||
| "markers", | ||
| "functional: marks tests as functional (requiring GEMINI_API_KEY)", | ||
| ) | ||
|
|
||
|
|
||
| def pytest_addoption(parser: pytest.Parser) -> None: | ||
| """Add custom command line options.""" | ||
| parser.addoption( | ||
| "--run-functional", | ||
| action="store_true", | ||
| default=False, | ||
| help="Run functional tests that require GEMINI_API_KEY", | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if pytest_collection_modifyitems is defined in the gemini-image package
rg -n "pytest_collection_modifyitems" packages/gemini-image/Repository: ByronWilliamsCPA/python-libs
Length of output: 54
🏁 Script executed:
# Read the full conftest.py file to see if the hook is defined elsewhere
cat -n packages/gemini-image/tests/conftest.pyRepository: ByronWilliamsCPA/python-libs
Length of output: 3048
🏁 Script executed:
# Check if there are any functional tests using the marker
rg -n "@pytest.mark.functional" packages/gemini-image/Repository: ByronWilliamsCPA/python-libs
Length of output: 54
Missing pytest_collection_modifyitems hook to skip functional tests.
The --run-functional option is registered, but there's no hook to skip tests marked with @pytest.mark.functional when the flag is not provided. While no functional tests currently exist in the codebase, this infrastructure should be complete to prevent issues when functional tests are added.
🔧 Suggested fix
def pytest_addoption(parser: pytest.Parser) -> None:
"""Add custom command line options."""
parser.addoption(
"--run-functional",
action="store_true",
default=False,
help="Run functional tests that require GEMINI_API_KEY",
)
+
+
+def pytest_collection_modifyitems(
+ config: pytest.Config, items: list[pytest.Item]
+) -> None:
+ """Skip functional tests unless --run-functional is passed."""
+ if config.getoption("--run-functional"):
+ return
+ skip_functional = pytest.mark.skip(reason="need --run-functional option to run")
+ for item in items:
+ if "functional" in item.keywords:
+ item.add_marker(skip_functional)🤖 Prompt for AI Agents
In @packages/gemini-image/tests/conftest.py around lines 15 - 30, The pytest
setup registers the --run-functional flag and the functional marker but lacks
the pytest_collection_modifyitems hook to skip tests marked with
@pytest.mark.functional when the flag is not set; implement
pytest_collection_modifyitems(config, items) to check
config.getoption("--run-functional") and, if False, iterate items and add
pytest.mark.skip(reason=...) to any item with "functional" in item.keywords (or
remove them from items) so functional tests are skipped unless the flag is
provided; add this function alongside pytest_addoption/pytest_configure in
conftest.py and reference the "functional" marker and "--run-functional" flag.
| # Pytest configuration for custom markers | ||
| def pytest_configure(config): | ||
| """Register custom markers.""" | ||
| config.addinivalue_line( | ||
| "markers", "functional: marks tests as functional (requiring API key)" | ||
| ) |
There was a problem hiding this comment.
Remove duplicate marker registration.
The functional marker is already registered in packages/gemini-image/tests/conftest.py (lines 14-19, as mentioned in the relevant code snippets). Duplicate registration can cause pytest warnings or unexpected behavior.
🔧 Proposed fix
-# Pytest configuration for custom markers
-def pytest_configure(config):
- """Register custom markers."""
- config.addinivalue_line(
- "markers", "functional: marks tests as functional (requiring API key)"
- )🤖 Prompt for AI Agents
In @packages/gemini-image/tests/test_functional.py around lines 300 - 305, The
pytest marker "functional" is being registered twice; remove the duplicate
registration by deleting the pytest_configure function (or at least its
config.addinivalue_line call) from tests/test_functional.py so the marker is
only registered in conftest.py; ensure the pytest_configure symbol in
tests/test_functional.py no longer calls config.addinivalue_line("markers",
"functional: marks tests as functional (requiring API key)").
| class TestGetMimeTypeForFormat: | ||
| """Tests for get_mime_type_for_format function.""" | ||
|
|
||
| def test_png_mime(self) -> None: | ||
| """Test PNG format returns image/png.""" | ||
| assert get_mime_type_for_format("png") == "image/png" | ||
|
|
||
| def test_jpeg_mime(self) -> None: | ||
| """Test JPEG format returns image/jpeg.""" | ||
| assert get_mime_type_for_format("jpeg") == "image/jpeg" | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding tests for GIF and WebP MIME types.
Only PNG and JPEG MIME types are tested. For completeness, consider adding tests for GIF (image/gif) and WebP (image/webp).
💡 Additional MIME type tests
def test_gif_mime(self) -> None:
"""Test GIF format returns image/gif."""
assert get_mime_type_for_format("gif") == "image/gif"
def test_webp_mime(self) -> None:
"""Test WebP format returns image/webp."""
assert get_mime_type_for_format("webp") == "image/webp"🤖 Prompt for AI Agents
In @packages/gemini-image/tests/test_io.py around lines 86 - 96, Add tests for
GIF and WebP MIME types to the TestGetMimeTypeForFormat class: add two new test
methods that call get_mime_type_for_format("gif") and
get_mime_type_for_format("webp") and assert they return "image/gif" and
"image/webp" respectively; place them alongside the existing test_png_mime and
test_jpeg_mime methods in the tests/test_io.py file so the suite covers common
image formats.
| def test_create_registry(self, tmp_path: Path) -> None: | ||
| """Test registry file is created when it doesn't exist.""" | ||
| registry_path = tmp_path / "PROMPTS.md" | ||
| registry = PromptRegistry(registry_path) | ||
|
|
||
| # Access should create the file | ||
| registry._ensure_exists() | ||
|
|
||
| assert registry_path.exists() | ||
| content = registry_path.read_text() | ||
| assert "Image Generation Registry" in content |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using public API instead of private _ensure_exists() method.
Directly calling _ensure_exists() couples the test to implementation details. The public add_entry() method internally calls _ensure_exists(), so this test could use that instead. However, this is acceptable for explicit initialization testing.
💡 Alternative approach using public API
def test_create_registry(self, tmp_path: Path) -> None:
"""Test registry file is created when it doesn't exist."""
registry_path = tmp_path / "PROMPTS.md"
+ image_path = tmp_path / "test.png"
+ image_path.touch()
registry = PromptRegistry(registry_path)
- # Access should create the file
- registry._ensure_exists()
+ # Any operation should create the file
+ registry.add_entry(image_path=image_path, prompt="test", model="pro")
assert registry_path.exists()
content = registry_path.read_text()
assert "Image Generation Registry" in content🤖 Prompt for AI Agents
In @packages/gemini-image/tests/test_registry.py around lines 13 - 23, The test
test_create_registry should avoid calling the private method _ensure_exists();
instead, use the public API to trigger file creation by calling
PromptRegistry.add_entry(...) on the registry instance (e.g., add a minimal
entry/title) which will internally create the file, then assert
registry_path.exists() and that the expected header "Image Generation Registry"
is in the content; update the test to replace registry._ensure_exists() with a
call to registry.add_entry(...) and keep the same assertions.
| # Image Generation Repository Analysis Report | ||
|
|
||
| > **Analysis Date**: 2026-01-09 | ||
| > **Methodology**: Multi-Model Consensus Analysis | ||
| > **Models Consulted**: 5 frontier AI models with varying stances | ||
|
|
||
| ## Executive Summary | ||
|
|
||
| This report presents a comprehensive analysis of the `image-generation` repository, a toolkit for generating images using Google's Gemini AI models (Gemini 2.5 Flash and Gemini 3 Pro "Nano Banana"). The analysis was conducted using a multi-model consensus approach with five frontier AI models to identify practical and functional gaps. | ||
|
|
||
| **Overall Assessment**: The repository provides a **robust foundation** with thoughtful workflows and excellent documentation, but has **significant gaps** in testing, error resilience, and production-grade features that impact reliability for automated or batch workflows. | ||
|
|
||
| --- | ||
|
|
||
| ## Models Consulted | ||
|
|
||
| | Model | Stance | Confidence | Key Focus | | ||
| |-------|--------|------------|-----------| | ||
| | Google Gemini 2.5 Pro | For | 9/10 | Strengths advocacy | | ||
| | Google Gemini 3 Pro Preview | Against | 10/10 | Critical gap identification | | ||
| | OpenAI GPT-5.2 | Neutral | 8/10 | Balanced analysis | | ||
| | DeepSeek R1-0528 | Against | 9/10 | Security & code quality | | ||
| | xAI Grok-4 | Neutral | 8/10 | User experience focus | | ||
|
|
||
| --- | ||
|
|
||
| ## Consensus Findings | ||
|
|
||
| ### Universal Agreement (5/5 Models) | ||
|
|
||
| These issues were identified by **all five models** and represent the highest-priority gaps: | ||
|
|
||
| #### 1. Monolithic Script Structure | ||
| **Location**: [generate_image.py](scripts/generate_image.py) (941 lines) | ||
|
|
||
| The single-file structure mixes: | ||
|
|
||
| - CLI argument parsing | ||
| - Gemini API interaction | ||
| - File I/O operations | ||
| - Markdown registry formatting | ||
| - Story sequence logic | ||
|
|
||
| **Impact**: Makes unit testing nearly impossible, increases maintenance burden, and limits extensibility. | ||
|
|
||
| **Recommendation**: Refactor into modules: | ||
|
|
||
| ``` | ||
| scripts/ | ||
| ├── generate_image.py # CLI entry point | ||
| ├── client.py # GeminiClient class | ||
| ├── io.py # Format detection, file saving | ||
| ├── registry.py # PROMPTS.md handling | ||
| └── utils.py # Shared utilities | ||
| ``` | ||
|
|
||
| #### 2. Zero Test Coverage | ||
| **Current State**: No unit tests, integration tests, or CI/CD configuration. | ||
|
|
||
| **Critical paths lacking tests**: | ||
|
|
||
| - `detect_image_format()` - Magic byte detection (lines 104-119) | ||
| - Output path routing rules (draft/final/story) | ||
| - PROMPTS.md insertion logic and idempotency | ||
| - API response parsing (thoughts vs final parts) | ||
| - Multi-part story sequencing | ||
|
|
||
| **Recommendation**: Add pytest suite with minimum coverage for: | ||
|
|
||
| - Format detection edge cases | ||
| - Path routing logic | ||
| - Registry table updates | ||
|
|
||
| #### 3. No Retry/Backoff for API Failures | ||
| **Location**: [generate_image.py:558-562](scripts/generate_image.py#L558-L562) | ||
|
|
||
| ```python | ||
| except Exception as e: | ||
| print(f"Error generating image: {e}") | ||
| ``` | ||
|
|
||
| **Missing resilience for**: | ||
|
|
||
| - HTTP 429 rate limiting | ||
| - HTTP 5xx server errors | ||
| - Network timeouts/resets | ||
| - Transient connection failures | ||
|
|
||
| **Recommendation**: Add `tenacity` or similar retry decorator: | ||
|
|
||
| ```python | ||
| @retry( | ||
| stop=stop_after_attempt(3), | ||
| wait=wait_exponential(multiplier=1, min=4, max=60), | ||
| retry=retry_if_exception_type((RateLimitError, NetworkError)) | ||
| ) | ||
| def generate_image(...): | ||
| ``` | ||
|
|
||
| #### 4. Claude Agent Integration is Conceptual Only | ||
| **Location**: [diagram-specialist.md](agents/diagram-specialist.md) | ||
|
|
||
| The agent configuration file documents a validation workflow but: | ||
|
|
||
| - No programmatic integration with `generate_image.py` | ||
| - No script to invoke validation automatically | ||
| - No structured metadata export (JSON sidecar) for agents | ||
| - `/diagram` commands referenced but not implemented in CLI | ||
|
|
||
| **Recommendation**: Create `scripts/validate_image.py` that: | ||
|
|
||
| - Invokes Claude API with diagram-specialist prompts | ||
| - Accepts generated image path as input | ||
| - Outputs validation report | ||
| - Optionally blocks finalization on validation failure | ||
|
|
||
| --- | ||
|
|
||
| ### Strong Agreement (4/5 Models) | ||
|
|
||
| #### 5. Story Mode Lacks Resume Capability | ||
| **Location**: [generate_image.py:565-659](scripts/generate_image.py#L565-L659) | ||
|
|
||
| The `generate_story_sequence()` function iterates 1 to N without checking if files exist. If a 10-part story fails on part 9, re-running regenerates parts 1-8. | ||
|
|
||
| **Impact**: Wasted API costs and time on already-generated content. | ||
|
|
||
| **Recommendation**: | ||
|
|
||
| ```python | ||
| # Check if part already exists | ||
| output_path = output_prefix.parent / f"{output_prefix.stem}_part{part_num}.png" | ||
| if output_path.exists(): | ||
| print(f"Skipping part {part_num} (already exists)") | ||
| previous_image_path = output_path | ||
| continue | ||
| ``` | ||
|
|
||
| #### 6. `--finalize` Doesn't Parse PROMPTS.md for Original Prompt | ||
| **Location**: [generate_image.py:827-847](scripts/generate_image.py#L827-L847) | ||
|
|
||
| The code claims to read the draft's PROMPTS.md entry but: | ||
|
|
||
| - Only performs a naive substring search | ||
| - Falls back to generic upscaling prompt | ||
| - Doesn't actually extract and reuse original parameters | ||
|
|
||
| **Recommendation**: Store metadata in JSON sidecar per image: | ||
|
|
||
| ``` | ||
| output/drafts/ | ||
| ├── draft_20260109_123456.png | ||
| └── draft_20260109_123456.json # Contains prompt, params, model | ||
| ``` | ||
|
|
||
| #### 7. Output Path Behavior is Surprising | ||
| **Location**: [generate_image.py:501-513](scripts/generate_image.py#L501-L513) | ||
|
|
||
| ```python | ||
| if not str(output_path).startswith("output"): | ||
| output_path = script_dir / "output" / output_path.name | ||
| ``` | ||
|
|
||
| **Issues**: | ||
|
|
||
| - `-o /tmp/myimage.png` gets rewritten to `output/myimage.png` | ||
| - Violates CLI conventions where user-specified paths are honored | ||
| - `startswith("output")` is brittle across platforms | ||
| - Path traversal possible via `output/../../somewhere` | ||
|
|
||
| **Recommendation**: | ||
|
|
||
| - Honor absolute paths as-is | ||
| - Add explicit `--output-dir` flag | ||
| - Use `pathlib.Path.resolve()` for safe path handling | ||
|
|
||
| --- | ||
|
|
||
| ### Moderate Agreement (3/5 Models) | ||
|
|
||
| #### 8. Python 3.11+ Requirement Not Documented | ||
| **Location**: Multiple occurrences of `datetime.UTC` | ||
|
|
||
| Lines 205, 423-425, 479-480, 594-595 use `datetime.UTC` which was added in Python 3.11. | ||
|
|
||
| **Impact**: Script fails silently or with cryptic errors on Python 3.10. | ||
|
|
||
| **Recommendation**: | ||
|
|
||
| ```python | ||
| # Compatible with Python 3.9+ | ||
| from datetime import datetime, timezone | ||
| datetime.now(tz=timezone.utc) # Instead of datetime.UTC | ||
| ``` | ||
|
|
||
| Or document Python 3.11+ requirement in README. | ||
|
|
||
| #### 9. Manual .env Parsing Should Use python-dotenv | ||
| **Location**: [generate_image.py:85-93](scripts/generate_image.py#L85-L93) | ||
|
|
||
| Current implementation: | ||
|
|
||
| - May fail on complex quoted values | ||
| - Doesn't handle comments | ||
| - No whitespace/newline robustness | ||
|
|
||
| **Recommendation**: Add `python-dotenv` to dependencies and use standard loading. | ||
|
|
||
| #### 10. No Batch Processing Support | ||
| **Current State**: Only single prompt or story mode supported. | ||
|
|
||
| **User expectation**: Generate multiple images from CSV/JSON/text file of prompts. | ||
|
|
||
| **Recommendation**: Add `--batch prompts.json` flag: | ||
|
|
||
| ```json | ||
| [ | ||
| {"prompt": "Network diagram", "aspect": "16:9", "size": "2K"}, | ||
| {"prompt": "Server rack layout", "aspect": "9:16", "size": "2K"} | ||
| ] | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Additional Findings by Category | ||
|
|
||
| ### Security Considerations | ||
|
|
||
| | Issue | Severity | Location | Recommendation | | ||
| |-------|----------|----------|----------------| | ||
| | No `--no-document` flag for privacy | Medium | Line 917-929 | Add flag to disable PROMPTS.md logging | | ||
| | Verbose mode may log sensitive data | Medium | Line 446 | Scrub API keys from output | | ||
| | Path traversal possible | Low | Line 501-513 | Use `Path.resolve()` with validation | | ||
| | .env commit not warned | Low | README.md | Add explicit warning about .env security | | ||
| | No Gemini safety_settings configured | Medium | Line 376 | Handle content blocks gracefully | | ||
|
|
||
| ### Documentation Gaps | ||
|
|
||
| | Issue | Location | Recommendation | | ||
| |-------|----------|----------------| | ||
| | Model ID mismatch in docstring | Lines 5-8 vs 58-69 | Sync docstring with MODELS dict | | ||
| | Story output location mismatch | IMAGE_GENERATION_GUIDE.md | Clarify output/ prepending behavior | | ||
| | PROMPTS.md template is empty | examples/PROMPTS.md | Add example entries | | ||
| | Max 14 ref images buried | IMAGE_GENERATION_GUIDE.md line 37 | Highlight in main features | | ||
| | Windows incompatibility of `file` command | README.md line 217 | Document alternatives | | ||
|
|
||
| ### Code Quality Issues | ||
|
|
||
| | Issue | Location | Recommendation | | ||
| |-------|----------|----------------| | ||
| | Broad exception handling | Line 558 | Handle specific API errors | | ||
| | Thought images use MIME substring | Line 411-415 | Use `detect_image_format()` consistently | | ||
| | Inefficient base64 round-trip | Lines 133-161, 327-333 | Keep raw bytes, detect MIME directly | | ||
| | Only first candidate processed | Line 395-398 | Handle multi-candidate responses | | ||
| | Hardcoded path separators | Line 420 | Use `pathlib` consistently | | ||
|
|
||
| ### Platform Compatibility | ||
|
|
||
| | Issue | Affected Platform | Recommendation | | ||
| |-------|-------------------|----------------| | ||
| | `datetime.UTC` | Python < 3.11 | Use `timezone.utc` | | ||
| | `startswith("output")` | Windows | Use `pathlib` methods | | ||
| | Hardcoded `/` separators | Windows | Use `Path` objects | | ||
| | `file` command | Windows | Document `python-magic` alternative | | ||
|
|
||
| --- | ||
|
|
||
| ## Prioritized Recommendations | ||
|
|
||
| ### Critical (Address Before Sharing Widely) | ||
|
|
||
| 1. **Fix Python compatibility**: Replace `datetime.UTC` with `timezone.utc` | ||
| 2. **Add retry logic**: Implement exponential backoff for API calls | ||
| 3. **Add story resume**: Skip existing parts in story mode | ||
| 4. **Fix finalize workflow**: Actually parse PROMPTS.md or use JSON sidecar | ||
|
|
||
| ### High Priority (Production Readiness) | ||
|
|
||
| 5. **Add unit tests**: Focus on `detect_image_format`, path routing, registry updates | ||
| 6. **Refactor script**: Split into modules for testability | ||
| 7. **Honor output paths**: Let users specify absolute paths | ||
| 8. **Add `--no-document`**: Privacy option for prompt logging | ||
|
|
||
| ### Medium Priority (Enhanced Usability) | ||
|
|
||
| 9. **Add progress indicators**: Spinner or progress bar for long operations | ||
| 10. **Add batch processing**: CSV/JSON prompt file support | ||
| 11. **Automate Claude validation**: Create `validate_image.py` script | ||
| 12. **Use python-dotenv**: Standard .env file handling | ||
|
|
||
| ### Low Priority (Polish) | ||
|
|
||
| 13. **Add CI/CD**: GitHub Actions for linting, tests | ||
| 14. **Add pyproject.toml**: Modern Python packaging | ||
| 15. **Add type checking**: Enable strict basedpyright | ||
| 16. **Warn on model-specific flags**: Error when `--search` used with flash | ||
|
|
||
| --- | ||
|
|
||
| ## Strengths Acknowledged | ||
|
|
||
| All models recognized these as **significant strengths**: | ||
|
|
||
| 1. **Draft-then-finalize workflow**: Cost-effective iteration pattern | ||
| 2. **Automatic format detection**: Magic bytes solve API MIME mismatches | ||
| 3. **Comprehensive documentation**: README, guide, and agent config | ||
| 4. **Minimal dependencies**: Single `google-genai` dependency | ||
| 5. **CLI-first design**: Enables automation and scripting | ||
| 6. **PROMPTS.md registry**: Automatic documentation of generations | ||
| 7. **Multi-part story generation**: Unique continuity feature | ||
| 8. **Reference image support**: Up to 14 images for editing/style transfer | ||
|
|
||
| --- | ||
|
|
||
| ## Conclusion | ||
|
|
||
| The image-generation repository is a **well-designed toolkit** with thoughtful features for its target use case of technical diagram generation. The draft-finalize workflow, automatic format detection, and comprehensive documentation demonstrate mature design thinking. | ||
|
|
||
| However, the repository is **not production-ready** due to: | ||
|
|
||
| - Zero test coverage | ||
| - Missing error resilience (no retries) | ||
| - Python version compatibility issues | ||
| - Surprising output path behavior | ||
|
|
||
| Addressing the **Critical** and **High Priority** recommendations would transform this from a personal utility into a sharable, reliable tool suitable for broader adoption. | ||
|
|
||
| --- | ||
|
|
||
| ## Appendix: Model Verdicts | ||
|
|
||
| ### Gemini 2.5 Pro (For) | ||
| > "This is a thoughtfully designed and highly practical toolkit that excels in its specific niche of technical diagram generation, with robust workflows and excellent documentation that address real-world API limitations." | ||
|
|
||
| ### Gemini 3 Pro Preview (Against) | ||
| > "The repository offers a functional and creative toolkit for simplified image generation, but its reliance on brittle text scraping for state management and lack of production-grade resilience severely limits its reliability for batch or automated workflows." | ||
|
|
||
| ### GPT-5.2 (Neutral) | ||
| > "Strong practical foundation for a 'power-user' Gemini image CLI, but it has several functional expectation gaps plus maintainability/test/security hardening needed before broader adoption." | ||
|
|
||
| ### DeepSeek R1 (Against) | ||
| > "The repository provides a robust foundation for Gemini-powered image generation but has significant gaps in validation, error resilience, testing, and security hardening that impact production readiness." | ||
|
|
||
| ### Grok-4 (Neutral) | ||
| > "Robust toolkit with strong core features for Gemini-based image generation, but gaps in testing, modularity, and advanced user functionalities limit long-term maintainability and scalability." | ||
|
|
||
| --- | ||
|
|
||
| *Report generated via multi-model consensus analysis using PAL MCP Server* |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify the purpose and retention of this review document.
This file is located in tmp_cleanup/, suggesting it's a temporary artifact. Since it contains valuable context about the design decisions that led to the v0.2.0 refactor, consider either:
- Moving it to a permanent location (e.g.,
docs/archive/design-review.md) if it has archival value - Removing it before merge if it was only meant for development reference
- Adding it to
.gitignoreif it's a local working file
Would you like me to help determine the appropriate location for this document, or should it be removed?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~84-~84: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...Missing resilience for: - HTTP 429 rate limiting - HTTP 5xx server errors - Network time...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.18.1)
48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
150-150: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @tmp_cleanup/image-generationreview.md around lines 1 - 349, The file
tmp_cleanup/image-generationreview.md appears to be a temporary review artifact
and should not be merged as-is; either move it to a permanent docs location
(e.g., create docs/archive/design-review.md and commit the file there, update
any repo index/README or CHANGELOG to reference it), delete it from the branch
before merging if it’s not intended to be kept, or add
tmp_cleanup/image-generationreview.md (or the entire tmp_cleanup/ folder) to
.gitignore if it’s a local working file; pick one option, perform the
corresponding change, and ensure the PR description notes the decision so
reviewers know why the file was moved/removed/ignored.
There was a problem hiding this comment.
Pull request overview
This is a major v0.2.0 release that refactors the gemini-image library from a monolithic structure into a modular architecture, addressing key recommendations from the multi-model consensus analysis documented in tmp_cleanup/image-generationreview.md.
Key Changes:
- Modular architecture with separated concerns (client, io, registry, response_parser, exceptions)
- Batch processing support with resume capability via
generate_batch() - Story sequence resume functionality to skip existing parts
- Retry logic with exponential backoff using tenacity
- Comprehensive exception hierarchy for better error handling
- PROMPTS.md registry and JSON metadata sidecars for generation tracking
- Security hardening and documentation (SECURITY.md, best practices)
Reviewed changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Version bump to 0.2.0, added dependencies (tenacity, python-dotenv, structlog, rich) |
| client.py | New GeminiClient with retry logic and exception mapping |
| io.py | Magic byte format detection and file operations |
| registry.py | PROMPTS.md generation logging system |
| response_parser.py | Structured API response parsing |
| exceptions.py | Custom exception hierarchy |
| generator.py | Refactored with modular architecture, added batch/resume features |
| cli.py | Enhanced with batch processing, --no-document, --resume flags |
| models.py | Expanded aspect ratios (10 total), reference image limits |
| test_*.py | Comprehensive test coverage (22+ tests) |
| docs/* | USAGE_GUIDE.md, SECURITY.md, CONTRIBUTING.md, CHANGELOG.md |
| using=1, | ||
| message="Parallel batch processing not yet implemented", | ||
| ) | ||
| parallel = 1 |
There was a problem hiding this comment.
Variable parallel is not used.
| parallel = 1 |
| """Check if the package can be imported.""" | ||
| print("Checking package import...", end=" ") | ||
| try: | ||
| import gemini_image # noqa: F401 |
There was a problem hiding this comment.
Module 'gemini_image' is imported with both 'import' and 'import from'.
| import gemini_image # noqa: F401 | |
| from gemini_image import generate_image # noqa: F401 |
CI Fixes: - Add SPDX license header to validate_installation.py (REUSE compliance) - Remove API key logging to fix CodeQL security warning - Change [Unreleased] to [0.2.0] with release date (changelog check) CodeRabbit Review Fixes: - FIXES_NEEDED.md: split long line, add blank lines around code blocks - CHANGELOG.md: remove redundant "CLI interface" -> "CLI" - CONTRIBUTING.md: add language to code block, fix formatting - USAGE_GUIDE.md: add missing imports to RateLimitError example - README.md: update PROMPTS.md example to match actual table format Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/gemini-image/README.md (2)
236-298: Document keyword-only parameter separators in function signatures.The documented function signatures are missing the
*,separator that marks keyword-only parameters in the actual implementation. Ingenerate_image(), parametersdocument,registry_path, andsave_metadata_fileare keyword-only. Ingenerate_batch(), parametersparallel,resume,document, andshow_progressare keyword-only. Ingenerate_story_sequence(), parametersresumeanddocumentare keyword-only. Infinalize_draft(), parameterdocumentis keyword-only. This omission makes the documented API incomplete and prevents users from calling these functions correctly using the documented signatures.
3-3: Update model names in the Models table to match line 3 branding.The Models table (lines 130–131) shows "Gemini 2.5 Flash" and "Gemini 3 Pro," but line 3 introduces them as "Nano Banana / Nano Banana Pro." Update the table to include the "Nano Banana" branding for consistency with both the introduction and the actual model configuration in the codebase.
Suggested fix:
flash: "Nano Banana (Gemini 2.5 Flash)"pro: "Nano Banana Pro (Gemini 3 Pro)"docs/secure.md (2)
297-303: Extract email allowlist logic to a dedicated service class.The
is_email_allowed()method provides utility functionality but belongs with settings configuration. For better separation of concerns and testability, consider moving this to a dedicated validator or authorization service:# Example refactor class CloudflareEmailValidator: def __init__(self, allowed_domains: list[str]): self.allowed_domains = allowed_domains def is_allowed(self, email: str) -> bool: # Logic hereThis allows settings to focus on configuration, while authorization logic resides in its own module.
228-321: Module path is incorrect; referenceconfig.pyinstead ofsettings.py.The CloudflareSettings implementation is located in
packages/cloudflare-auth/src/cloudflare_auth/config.py, notsettings.py. Additionally, CloudflareSettings andget_cloudflare_settings()are not exported in the package's__init__.py, which limits public accessibility.The thread-safety concern for the module-level
_settings_instancesingleton with lazy initialization remains valid: this pattern lacks explicit locking and is not guaranteed safe for non-CPython runtimes or async frameworks. Consider usingfunctools.lru_cache(which is already imported but unused) withmaxsize=1for thread-safe caching, or add explicit locking withthreading.Lock()if non-CPython support is required.
🤖 Fix all issues with AI agents
In @FIXES_NEEDED.md:
- Around line 139-189: Add the missing import test and CI check: create
tests/test_imports.py that imports CloudflareAuthMiddleware, CloudflareUser, and
setup_cloudflare_auth_enhanced from the cloudflare_auth package and asserts the
main symbol(s) are not None (to validate public exports), and update
.github/workflows/ci.yml to add a step (e.g., "Test package imports") that runs
a one-liner import command (python -c "from cloudflare_auth import
CloudflareUser") to catch import-time errors in CI.
In @packages/gemini-image/docs/USAGE_GUIDE.md:
- Line 33: Replace the outdated Google AI Studio URL string
"https://makersuite.google.com/app/apikey" with the current canonical URL
"https://aistudio.google.com/apikey" in the docs (USAGE_GUIDE.md) so the API key
link points to the correct Google Gemini API key page; ensure the visible link
text and the href are both updated and leave the --list-models CLI reference
unchanged.
In @packages/gemini-image/scripts/validate_installation.py:
- Around line 87-102: The check_api_key function currently catches the broad
Exception; change it to catch only specific expected exceptions (for example
ConfigurationError from gemini_image.client, plus ValueError, KeyError and
ImportError as appropriate) and import ConfigurationError (or the correct
specific exception type) at the top of the module; replace the bare except
Exception as e with except (ConfigurationError, ValueError, KeyError,
ImportError) as e (or the minimal subset relevant to get_api_key) so you only
handle known error cases when calling get_api_key().
- Around line 195-201: The code currently checks the API key result using a
hardcoded index results[-1][1], which is fragile; instead locate the tuple by
name in the results list (e.g., use next((ok for name, ok in results if name ==
"API Key"), False) or build a dict like dict(results).get("API Key", False)) and
use that boolean to decide whether to print the API key note—replace the
results[-1][1] reference with a name-based lookup on results.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
FIXES_NEEDED.mddocs/secure.mdpackages/gemini-image/CHANGELOG.mdpackages/gemini-image/CONTRIBUTING.mdpackages/gemini-image/README.mdpackages/gemini-image/docs/USAGE_GUIDE.mdpackages/gemini-image/scripts/validate_installation.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Use 120 character line length for Markdown documentation files
Files:
FIXES_NEEDED.mdpackages/gemini-image/CHANGELOG.mdpackages/gemini-image/CONTRIBUTING.mdpackages/gemini-image/README.mddocs/secure.mdpackages/gemini-image/docs/USAGE_GUIDE.md
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use Ruff formatting with 88 character line length for Python code
Use Ruff linting with PyStrict-aligned rules including BLE, EM, SLF, INP, ISC, PGH, RSE, TID, YTT, FA, T10, and G rules
Tag assumptions with #CRITICAL, #ASSUME, or #EDGE comments including category and reason for verification
Files:
packages/gemini-image/scripts/validate_installation.py
🧠 Learnings (6)
📚 Learning: 2025-12-14T22:53:58.937Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-14T22:53:58.937Z
Learning: Applies to src/python_libs/**/*.py : Ensure TODO/FIXME comments are actionable with sufficient context
Applied to files:
FIXES_NEEDED.md
📚 Learning: 2025-12-14T22:54:23.019Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T22:54:23.019Z
Learning: Applies to **/*.md : Use 120 character line length for Markdown documentation files
Applied to files:
FIXES_NEEDED.md
📚 Learning: 2025-12-14T22:54:23.019Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T22:54:23.019Z
Learning: Use conventional commit messages with appropriate type prefixes: feat:, fix:, docs:, refactor:, perf:, test:, chore:
Applied to files:
packages/gemini-image/CONTRIBUTING.md
📚 Learning: 2025-12-14T22:53:58.937Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-14T22:53:58.937Z
Learning: Applies to README.md : Verify README documentation is consistent with actual code behavior
Applied to files:
packages/gemini-image/README.md
📚 Learning: 2025-12-14T22:54:23.019Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T22:54:23.019Z
Learning: Applies to docs/template_feedback.md : Add template feedback to docs/template_feedback.md when identifying issues that should be addressed in the cookiecutter-python-template
Applied to files:
packages/gemini-image/README.md
📚 Learning: 2025-12-14T22:53:58.937Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-14T22:53:58.937Z
Learning: Applies to src/python_libs/**/*.py : Ensure exceptions are specific and appropriate for the failure mode; verify error messages are actionable and user-friendly; check retry mechanisms have proper backoff and limits
Applied to files:
packages/gemini-image/docs/USAGE_GUIDE.md
🧬 Code graph analysis (1)
packages/gemini-image/scripts/validate_installation.py (2)
packages/gemini-image/src/gemini_image/exceptions.py (5)
ContentBlockedError(128-158)FileOperationError(233-267)FormatDetectionError(203-230)GeminiImageError(12-42)RateLimitError(90-117)packages/gemini-image/src/gemini_image/io.py (3)
detect_image_format(53-99)save_image(140-205)validate_image_file(304-336)
🪛 markdownlint-cli2 (0.18.1)
FIXES_NEEDED.md
17-17: Bare URL used
(MD034, no-bare-urls)
36-36: Bare URL used
(MD034, no-bare-urls)
98-98: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (23)
packages/gemini-image/CONTRIBUTING.md (1)
1-172: Excellent contribution guidelines document.The CONTRIBUTING.md file is comprehensive, well-organized, and provides clear guidance for contributors. Coverage includes setup (uv and pip), testing with coverage, code quality checks (ruff, basedpyright), branch naming conventions, conventional commit examples, code style requirements with a Python docstring example, PR process with checklist, and issue reporting guidance. All content aligns with project tooling and the PR's expanded documentation footprint. Line lengths comply with the 120-character markdown guideline.
packages/gemini-image/CHANGELOG.md (1)
1-52: Well-maintained changelog with accurate content.The CHANGELOG.md file follows Keep a Changelog format and Semantic Versioning conventions correctly. The [0.2.0] entry (dated 2026-01-09, matching the PR creation date) accurately documents all major features from the PR objectives: new modular architecture (exceptions, client with tenacity retry logic, io with magic-byte detection, registry, response_parser), structured logging, CLI enhancements, metadata sidecars, Python 3.10 compatibility, and bug fixes. The [0.1.0] section provides appropriate historical context. All comparison and release links are properly formatted. Line lengths are compliant with the 120-character markdown guideline.
packages/gemini-image/scripts/validate_installation.py (7)
1-10: LGTM!The SPDX headers and module docstring are well-structured and provide clear usage instructions.
17-27: LGTM!The import check is straightforward and correctly handles the import validation.
30-49: LGTM!The export checks correctly validate the main API functions introduced in v0.2.0.
105-118: LGTM!The CLI entry point validation is correct and follows the same pattern as other checks.
121-134: LGTM!The model configuration checks correctly validate the expected 'flash' and 'pro' models introduced in v0.2.0.
137-154: LGTM!The I/O function checks correctly validate the new I/O utilities (detect_image_format, save_image, validate_image_file) from the modular architecture.
52-84: Add missing exception classes to validation checks.The exceptions module defines 10 exception classes, but the validation only checks 8.
ServerErrorandGenerationErrorare missing from the checks and should be added to the exceptions list:exceptions = [ GeminiImageError, ConfigurationError, ValidationError, APIError, RateLimitError, ServerError, ContentBlockedError, FileOperationError, FormatDetectionError, GenerationError, ]⛔ Skipped due to learnings
Learnt from: CR Repo: ByronWilliamsCPA/python-libs PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-12-14T22:53:58.937Z Learning: Applies to src/python_libs/**/*.py : Ensure exceptions are specific and appropriate for the failure mode; verify error messages are actionable and user-friendly; check retry mechanisms have proper backoff and limitsLearnt from: CR Repo: ByronWilliamsCPA/python-libs PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-12-14T22:53:58.937Z Learning: Applies to src/python_libs/**/*.py : Check edge cases and boundary conditions: empty collections, None values, zero-length strings, maximum/minimum values, integer overflow, Unicode handling, timezone edge cases, network timeouts, and file system edge casespackages/gemini-image/README.md (4)
169-227: Batch processing and PROMPTS.md documentation is clear and well-structured.The new Batch Processing section (lines 169–195) provides clear JSON schema examples and field descriptions. The PROMPTS.md Registry section (lines 196–229) illustrates the output format effectively and shows how to disable documentation with the
--no-documentflag.
324-353: Security section provides strong API key handling guidance.The Security section clearly explains best practices for environment-based API key management, explicitly forbids hardcoding keys, and provides
.gitignoreand file permission recommendations. This aligns well with the PR's security improvements (no API key logging).
300-322: Exception imports are properly configured. All eight exception classes (GeminiImageError, ConfigurationError, ValidationError, APIError, RateLimitError, ContentBlockedError, FileOperationError, FormatDetectionError) are defined inpackages/gemini-image/src/gemini_image/exceptions.pyand correctly re-exported in__init__.pyvia the__all__list. The imports shown in the README documentation will work as written.
331-333: Documentation claim verified as accurate.The library does auto-load
.envfiles via python-dotenv as documented. Theget_api_key()function callsload_dotenv()(with or without an explicit env_file path), and this function is automatically invoked duringGeminiClientinitialization when no api_key is explicitly provided. Since the main entry pointgenerate_image()creates aGeminiClient()without passing an explicit api_key, the.envfile loading happens automatically in the normal usage flow. The implementation correctly matches the documentation.packages/gemini-image/docs/USAGE_GUIDE.md (7)
265-272: Verify the chunked_batch helper function signature and return type.The
chunked_batch()helper usesgenerate_batch()which should return iterable results. Confirm the implementation matches this expected behavior and that the helper example is valid.The helper function example follows a sensible pattern for managing memory during batch processing of 4K images, assuming
generate_batch()returns a list-like result.
194-212: [Your rewritten review comment text here]
[Exactly ONE classification tag]
1-120: File complies with 120 character line length limit for Markdown documentation.All 473 lines in USAGE_GUIDE.md are within the required 120 character maximum.
119-125: All 10 aspect ratios align with the implementation and comply with style guidelines.The documentation lists all expected aspect ratios (1:1, 2:3, 3:2, 3:4, 4:3, 4:5, 5:4, 9:16, 16:9, 21:9) and they match the
AspectRatiotype definition andASPECT_RATIOSconstant inmodels.pyexactly. All line lengths comply with the 120-character markdown guideline.
32-33: All referenced API functions (generate_image,generate_batch,generate_story_sequence,finalize_draft) and exception classes (ConfigurationError,RateLimitError,ContentBlockedError,FormatDetectionError,ValidationError,APIError,FileOperationError) exist in the implementation and are properly exported. Parameter names (is_draft,document,save_metadata_file,show_progress,resume) are all correctly defined in their respective functions. Documentation line lengths are compliant (max 90 characters, within the 120-character limit).
157-161: Reference image limits are correctly documented and enforced.The guide accurately specifies "Flash model: Up to 3 reference images" and "Pro model: Up to 14 reference images" per the
MAX_REFERENCE_IMAGESconstant inmodels.py. The validation function_validate_reference_images()enforces these limits. The limits are tested and match the implementation constraints.
317-339: PROMPTS.md registry implementation correctly matches documented behavior.The verification confirms:
generate_image()hasdocument: bool = Trueparameter as documented--no-documentCLI flag correctly maps todocument=not args.no_document- Implementation checks
if document:before adding entries toPROMPTS.md- Both Python API and CLI examples in the guide accurately reflect the actual behavior
docs/secure.md (2)
252-273: Verify Field definitions and environment variable aliases match documentation.The Field definitions use environment variable aliases (e.g.,
CLOUDFLARE_TEAM_DOMAIN,CF_JWT_HEADER). Confirm that:
- Alias naming conventions are consistent and documented
- Upstream code and .env examples use these exact aliases
- Alias casing is correct for the target environment (case_sensitive=False handles this, but verify intended behavior)
The case_sensitive=False setting means environment variable names are matched case-insensitively. Verify this is the intended behavior for your use case. For clarity, consider documenting the canonical (preferred) casing for environment variables.
148-159: Remove incorrect claim aboutnew-package-v*pattern support.The workflow currently only supports
cloudflare-auth-v*andgcs-utilities-v*tag patterns. Thenew-package-v*pattern is documented in Part 5 as instructions for adding future packages, but has not yet been implemented in.github/workflows/publish.yml. The parse step correctly extractspackage_dir,package_name, andversionfor the two existing patterns.Likely an incorrect or invalid review comment.
FIXES_NEEDED.md (1)
1-50: Fix line length compliance in markdown documentation.Line 120 exceeds the 120-character limit required for Markdown files:
120: The package appears to have been copied from a different project structure where it was nested under `src/` with a separate `src/config/` module. When moved to the python-libs monorepo structure, imports were not updated to reflect the new package structure.Wrap this line at or below 120 characters. Regarding URLs: the link on line 235 is already properly formatted in markdown syntax and requires no changes.
Likely an incorrect or invalid review comment.
| @field_validator("allowed_email_domains", "allowed_tunnel_ips", mode="before") | ||
| @classmethod | ||
| def parse_comma_separated(cls, v): | ||
| if isinstance(v, str): | ||
| return [item.strip() for item in v.split(",") if item.strip()] if v.strip() else [] | ||
| return v or [] | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Ensure field validator is robust against edge cases.
The parse_comma_separated validator handles comma-separated strings and strips whitespace. Verify:
- Empty strings and None values are handled safely (currently checks
if v.strip()) - Behavior matches expectations when fields are passed as lists directly (currently returns
v or []) - The validator applies to both
allowed_email_domainsandallowed_tunnel_ipsas intended
Consider adding a docstring to clarify the expected input formats and return behavior.
| ### Immediate (Required for PR #54) | ||
|
|
||
| 1. **Commit local import fixes**: | ||
|
|
||
| ```bash | ||
| cd packages/cloudflare-auth | ||
| git add src/cloudflare_auth/*.py | ||
| git commit -m "fix: correct all module imports from src.cloudflare_auth to cloudflare_auth" | ||
| ``` | ||
|
|
||
| 2. **Create CloudflareSettings module**: | ||
| - Create `src/cloudflare_auth/settings.py` | ||
| - Define `CloudflareSettings` and `get_cloudflare_settings` | ||
| - Update imports in affected files | ||
| - Test imports work correctly | ||
|
|
||
| 3. **Add import tests**: | ||
|
|
||
| ```python | ||
| # tests/test_imports.py | ||
| def test_public_imports(): | ||
| """Verify all public imports work correctly.""" | ||
| from cloudflare_auth import ( | ||
| CloudflareAuthMiddleware, | ||
| CloudflareUser, | ||
| setup_cloudflare_auth_enhanced, | ||
| ) | ||
| assert CloudflareAuthMiddleware is not None | ||
| ``` | ||
|
|
||
| 4. **Add CI check**: | ||
|
|
||
| ```yaml | ||
| # .github/workflows/ci.yml | ||
| - name: Test package imports | ||
| run: | | ||
| uv run python -c "from cloudflare_auth import CloudflareUser" | ||
| ``` | ||
|
|
||
| ### Follow-up (Before PyPI Publication) | ||
|
|
||
| 5. **Publish to PyPI**: | ||
| - Once fixes are complete and tests pass | ||
| - Follow semantic versioning (v0.1.1 for patch) | ||
| - Update homelab-infra to use PyPI package instead of path dependency | ||
|
|
||
| 6. **Update template**: | ||
| - Add import tests to cookiecutter-python-template | ||
| - Ensure packages in workspace members have validated imports | ||
|
|
||
| --- |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the repository structure
git ls-files | head -20Repository: ByronWilliamsCPA/python-libs
Length of output: 766
🏁 Script executed:
# Verify the cloudflare-auth package directory structure
if [ -d "packages/cloudflare-auth" ]; then
echo "✓ cloudflare-auth package exists"
find packages/cloudflare-auth -type f -name "*.py" | head -20
else
echo "✗ cloudflare-auth package not found"
fiRepository: ByronWilliamsCPA/python-libs
Length of output: 1204
🏁 Script executed:
# Check for settings.py and examine FIXES_NEEDED.md
if [ -f "FIXES_NEEDED.md" ]; then
echo "✓ FIXES_NEEDED.md exists"
echo "Checking lines 139-189:"
sed -n '139,189p' FIXES_NEEDED.md
fiRepository: ByronWilliamsCPA/python-libs
Length of output: 1526
🏁 Script executed:
# Check if docs/secure.md exists and examine it
if [ -f "docs/secure.md" ]; then
echo "✓ docs/secure.md exists"
grep -n "CloudflareSettings" docs/secure.md | head -10
else
echo "✗ docs/secure.md not found"
fiRepository: ByronWilliamsCPA/python-libs
Length of output: 481
🏁 Script executed:
# Check the actual package structure
if [ -d "packages/cloudflare-auth/src/cloudflare_auth" ]; then
echo "Contents of packages/cloudflare-auth/src/cloudflare_auth:"
ls -la packages/cloudflare-auth/src/cloudflare_auth/
else
echo "✗ Expected directory not found"
fiRepository: ByronWilliamsCPA/python-libs
Length of output: 1194
🏁 Script executed:
# Check what's exported from __init__.py
echo "=== __init__.py contents ==="
cat -n packages/cloudflare-auth/src/cloudflare_auth/__init__.pyRepository: ByronWilliamsCPA/python-libs
Length of output: 3720
🏁 Script executed:
# Check if test_imports.py exists
if [ -f "packages/cloudflare-auth/tests/test_imports.py" ]; then
echo "✓ test_imports.py exists"
cat -n packages/cloudflare-auth/tests/test_imports.py
else
echo "✗ test_imports.py does not exist (action item 3 proposes creating it)"
fiRepository: ByronWilliamsCPA/python-libs
Length of output: 141
🏁 Script executed:
# Check settings.py to verify CloudflareSettings and get_cloudflare_settings are defined
echo "=== settings.py first 50 lines ==="
head -50 packages/cloudflare-auth/src/cloudflare_auth/settings.pyRepository: ByronWilliamsCPA/python-libs
Length of output: 1714
🏁 Script executed:
# Check existing CI workflows
if [ -d ".github/workflows" ]; then
echo "=== CI Workflow files ==="
ls -la .github/workflows/
else
echo "✗ .github/workflows not found"
fiRepository: ByronWilliamsCPA/python-libs
Length of output: 1379
🏁 Script executed:
# Check if the CI check already exists for imports
if [ -f ".github/workflows/ci.yml" ]; then
echo "=== Checking for 'Test package imports' in ci.yml ==="
grep -n "Test package imports" .github/workflows/ci.yml || echo "Not found"
fiRepository: ByronWilliamsCPA/python-libs
Length of output: 135
🏁 Script executed:
# Check ci.yml to understand the current test structure and find where the import test should be added
cat -n .github/workflows/ci.yml | head -100Repository: ByronWilliamsCPA/python-libs
Length of output: 3968
🏁 Script executed:
# Verify that CloudflareSettings and get_cloudflare_settings are actually exported and importable
grep -n "get_cloudflare_settings" packages/cloudflare-auth/src/cloudflare_auth/settings.pyRepository: ByronWilliamsCPA/python-libs
Length of output: 128
🏁 Script executed:
# Check if CloudflareSettings and get_cloudflare_settings are exported from __init__.py
grep "CloudflareSettings\|get_cloudflare_settings" packages/cloudflare-auth/src/cloudflare_auth/__init__.pyRepository: ByronWilliamsCPA/python-libs
Length of output: 54
🏁 Script executed:
# Verify test_settings.py already exists and what it covers
if [ -f "packages/cloudflare-auth/tests/test_settings.py" ]; then
echo "✓ test_settings.py exists"
wc -l packages/cloudflare-auth/tests/test_settings.py
head -40 packages/cloudflare-auth/tests/test_settings.py
fiRepository: ByronWilliamsCPA/python-libs
Length of output: 1422
🏁 Script executed:
# Check docs/secure.md to see what the intended migration path is for CloudflareSettings
echo "=== docs/secure.md sections about CloudflareSettings ==="
grep -B2 -A2 "CloudflareSettings" docs/secure.md | head -50Repository: ByronWilliamsCPA/python-libs
Length of output: 859
🏁 Script executed:
# Check if settings should be in __all__ by looking at the full settings.py
tail -20 packages/cloudflare-auth/src/cloudflare_auth/settings.pyRepository: ByronWilliamsCPA/python-libs
Length of output: 705
🏁 Script executed:
# Verify the actual line count of FIXES_NEEDED.md to understand the context
wc -l FIXES_NEEDED.mdRepository: ByronWilliamsCPA/python-libs
Length of output: 91
🏁 Script executed:
# Check the markdown line length guideline compliance for FIXES_NEEDED.md around lines 139-189
sed -n '139,189p' FIXES_NEEDED.md | awk '{print length, $0}' | sort -rn | head -10Repository: ByronWilliamsCPA/python-libs
Length of output: 723
🏁 Script executed:
# Check if the proposed test imports would actually work - verify CloudflareSettings is NOT exported
echo "=== Checking __all__ exports from __init__.py ==="
grep -A 50 "__all__" packages/cloudflare-auth/src/cloudflare_auth/__init__.pyRepository: ByronWilliamsCPA/python-libs
Length of output: 899
🏁 Script executed:
# Try to verify if importing the proposed items would work
python3 << 'EOF'
import sys
import os
# Simulate what the test would do
try:
# This should work
from cloudflare_auth import CloudflareUser
print("✓ Can import CloudflareUser")
except ImportError as e:
print(f"✗ Cannot import CloudflareUser: {e}")
try:
# This should work
from cloudflare_auth import CloudflareAuthMiddleware
print("✓ Can import CloudflareAuthMiddleware")
except ImportError as e:
print(f"✗ Cannot import CloudflareAuthMiddleware: {e}")
try:
# This should work
from cloudflare_auth import setup_cloudflare_auth_enhanced
print("✓ Can import setup_cloudflare_auth_enhanced")
except ImportError as e:
print(f"✗ Cannot import setup_cloudflare_auth_enhanced: {e}")
try:
# This might NOT work - check if CloudflareSettings is exported
from cloudflare_auth import CloudflareSettings
print("✓ Can import CloudflareSettings from cloudflare_auth")
except ImportError as e:
print(f"✗ Cannot import CloudflareSettings from cloudflare_auth: {e}")
try:
# This will work
from cloudflare_auth.settings import CloudflareSettings, get_cloudflare_settings
print("✓ Can import CloudflareSettings from cloudflare_auth.settings")
except ImportError as e:
print(f"✗ Cannot import from cloudflare_auth.settings: {e}")
EOFRepository: ByronWilliamsCPA/python-libs
Length of output: 158
🏁 Script executed:
# Look more carefully at the CI section to see if there's any import testing
grep -n "import\|Import\|test.*import" .github/workflows/ci.ymlRepository: ByronWilliamsCPA/python-libs
Length of output: 54
🏁 Script executed:
# Check if there are any remaining "src.cloudflare_auth" imports that should be "cloudflare_auth"
grep -r "from src\.cloudflare_auth\|from src\.config" packages/cloudflare-auth/src/ 2>/dev/null | grep -v ".pyc" || echo "No src. imports found"Repository: ByronWilliamsCPA/python-libs
Length of output: 93
🏁 Script executed:
# Check middleware.py and middleware_enhanced.py for import issues
head -30 packages/cloudflare-auth/src/cloudflare_auth/middleware.py | grep "^from\|^import"Repository: ByronWilliamsCPA/python-libs
Length of output: 54
Action items 1–2 appear complete; items 3–4 (test_imports.py and CI check) remain pending.
The steps correctly reference the cloudflare-auth package structure and align with docs/secure.md. Import fixes appear done (no src.cloudflare_auth imports remain), and CloudflareSettings module exists in packages/cloudflare-auth/src/cloudflare_auth/settings.py.
However:
- test_imports.py (item 3) does not yet exist; the proposed test is valid—all three imports are exported in
__all__ - CI check (item 4) is not present in
.github/workflows/ci.yml; the proposed command is correct
All referenced file paths and import statements match the actual module layout. Sequencing is sound and ready to implement.
🤖 Prompt for AI Agents
In @FIXES_NEEDED.md around lines 139 - 189, Add the missing import test and CI
check: create tests/test_imports.py that imports CloudflareAuthMiddleware,
CloudflareUser, and setup_cloudflare_auth_enhanced from the cloudflare_auth
package and asserts the main symbol(s) are not None (to validate public
exports), and update .github/workflows/ci.yml to add a step (e.g., "Test package
imports") that runs a one-liner import command (python -c "from cloudflare_auth
import CloudflareUser") to catch import-time errors in CI.
| core_passed = all(ok for name, ok in results if name != "API Key") | ||
|
|
||
| if core_passed: | ||
| print() | ||
| if not results[-1][1]: # API key failed | ||
| print("Note: API key not configured. Set GEMINI_API_KEY to enable generation.") | ||
| print("Installation is valid!") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace hardcoded index with name-based lookup.
Line 199 uses hardcoded index [-1] to check if the API key check failed. This is fragile—if the checks list order changes, this logic breaks. Use name-based lookup instead.
♻️ Proposed fix
# API key is optional for installation validation
core_passed = all(ok for name, ok in results if name != "API Key")
+ api_key_result = next((ok for name, ok in results if name == "API Key"), True)
if core_passed:
print()
- if not results[-1][1]: # API key failed
+ if not api_key_result: # API key failed
print("Note: API key not configured. Set GEMINI_API_KEY to enable generation.")
print("Installation is valid!")
return 0🤖 Prompt for AI Agents
In @packages/gemini-image/scripts/validate_installation.py around lines 195 -
201, The code currently checks the API key result using a hardcoded index
results[-1][1], which is fragile; instead locate the tuple by name in the
results list (e.g., use next((ok for name, ok in results if name == "API Key"),
False) or build a dict like dict(results).get("API Key", False)) and use that
boolean to decide whether to print the API key note—replace the results[-1][1]
reference with a name-based lookup on results.
REUSE Compliance: - Remove SPDX header from validate_installation.py (use REUSE.toml) - Add packages/**/scripts/**/*.py pattern to REUSE.toml SonarCloud: - Fix sonar.tests config: wildcards not allowed, use explicit paths Code Quality: - Replace assert with proper error handling in generator.py (Bandit B101) - Import GenerationError for the new error handling - Fix bare URL in SECURITY.md (MD034) - Update outdated Google AI Studio URL in USAGE_GUIDE.md Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In @packages/gemini-image/docs/USAGE_GUIDE.md:
- Around line 341-363: Update the documentation examples to match actual output:
include the is_draft boolean in the metadata JSON produced by generate_image
when save_metadata_file=True (so the sample JSON includes "is_draft": true/false
and retains prompt, model, aspect_ratio, image_size, created_at), and replace
the simple bullet-list registry example with the actual markdown table format
the implementation emits (columns: File, Model, Date, Type, Aspect Ratio, Size,
Prompt) so the registry example matches the real output.
In @packages/gemini-image/scripts/validate_installation.py:
- Around line 85-100: In check_api_key, avoid the broad "except Exception"
around importing and calling get_api_key; instead catch specific failure modes
such as ImportError/ModuleNotFoundError and AttributeError for import/name
issues, plus the specific configuration exception that gemini_image.client may
raise (e.g., ConfigurationError or ValueError) when the key is missing/invalid,
and log those error messages the same way; update the except block to list these
specific exceptions (ImportError, ModuleNotFoundError, AttributeError, and the
package's ConfigurationError or ValueError fallback) so unexpected errors are
not silently swallowed.
- Around line 155-204: In main(), avoid using the hardcoded index results[-1][1]
to check the API Key result; instead look up the API Key entry by name (e.g.,
build a dict from results or iterate to find the tuple where name == "API Key")
and use that value to decide the note about API key configuration; update the
code around the core_passed check and the conditional that prints the API key
note to reference the name-based lookup (and remove the implicit
assumption/#ASSUME).
In @packages/gemini-image/src/gemini_image/generator.py:
- Around line 18-19: Move the local import of FileOperationError out of
finalize_draft() and add it to the module-level imports alongside GeminiClient,
_get_genai, GenerationError, and ValidationError; then remove the redundant
local import inside the finalize_draft function so all code uses the single
top-level FileOperationError import consistently.
- Around line 541-565: The code currently defaults final_aspect to "16:9"
regardless of draft metadata; change it to prefer a stored aspect ratio from
metadata when aspect_ratio is not provided by loading metadata (via
load_metadata(draft_path)) and setting final_aspect = aspect_ratio or
(metadata.get("aspect_ratio") if metadata else None) or "16:9"; also add a
verbose logger.info branch similar to original_prompt to indicate when the
original aspect ratio from metadata is being used (referencing symbols metadata,
aspect_ratio, final_aspect, load_metadata, draft_path).
- Around line 699-702: The code currently logs and continues when a path in
reference_images is missing (the loop that calls
logger.warning("reference_image_not_found", path=str(img_path))); change this so
missing user-supplied reference images raise an exception (e.g.,
FileNotFoundError or ValueError) with a clear message including str(img_path)
inside the generate_image function (or the method that contains the
reference_images loop) instead of silently continuing; if skipping missing
references is intentional, update the generate_image() docstring to explicitly
document that behavior rather than leaving it implicit.
- Around line 347-353: The catch-all except Exception in the batch loop
(surrounding logger.error and batch_results.append(None)) hides
configuration/validation problems; change it to only catch expected
API/generation/transient errors (e.g., import and catch APIError from
gemini_image.exceptions and any other known transient exceptions) and allow
ValidationError/ConfigurationError to propagate (or explicitly re-raise them if
accidentally caught). Update the exception clause around the block that
currently references idx, logger.error("batch_item_failed", ...) and
batch_results.append(None) to target the specific exception types and remove the
broad Exception handler.
- Around line 710-719: The code in generator.py determines MIME type from the
file extension (using mime_types and img_path.suffix) which can disagree with
actual image bytes; change it to read the image bytes and call the shared
detect_image_format function (from io.py) to get the real format, then map that
result to the appropriate MIME string instead of relying on suffix. Update the
logic around mime_types/mime_type to use detect_image_format(img_bytes) (or
equivalent) and fallback to "image/png" only if detection fails.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
REUSE.tomlpackages/gemini-image/SECURITY.mdpackages/gemini-image/docs/USAGE_GUIDE.mdpackages/gemini-image/scripts/validate_installation.pypackages/gemini-image/src/gemini_image/generator.pysonar-project.properties
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Use 120 character line length for Markdown documentation files
Files:
packages/gemini-image/docs/USAGE_GUIDE.mdpackages/gemini-image/SECURITY.md
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use Ruff formatting with 88 character line length for Python code
Use Ruff linting with PyStrict-aligned rules including BLE, EM, SLF, INP, ISC, PGH, RSE, TID, YTT, FA, T10, and G rules
Tag assumptions with #CRITICAL, #ASSUME, or #EDGE comments including category and reason for verification
Files:
packages/gemini-image/scripts/validate_installation.pypackages/gemini-image/src/gemini_image/generator.py
🧠 Learnings (1)
📚 Learning: 2025-12-14T22:53:58.937Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-14T22:53:58.937Z
Learning: Applies to src/python_libs/**/*.py : Ensure exceptions are specific and appropriate for the failure mode; verify error messages are actionable and user-friendly; check retry mechanisms have proper backoff and limits
Applied to files:
packages/gemini-image/docs/USAGE_GUIDE.mdpackages/gemini-image/scripts/validate_installation.py
🧬 Code graph analysis (2)
packages/gemini-image/scripts/validate_installation.py (2)
packages/gemini-image/src/gemini_image/exceptions.py (5)
ContentBlockedError(128-158)FileOperationError(233-267)FormatDetectionError(203-230)GeminiImageError(12-42)RateLimitError(90-117)packages/gemini-image/src/gemini_image/io.py (3)
detect_image_format(53-99)save_image(140-205)validate_image_file(304-336)
packages/gemini-image/src/gemini_image/generator.py (6)
packages/gemini-image/src/gemini_image/client.py (4)
client(181-183)GeminiClient(126-302)_get_genai(39-55)generate_content(185-269)packages/gemini-image/src/gemini_image/exceptions.py (3)
GenerationError(270-303)ValidationError(161-200)FileOperationError(233-267)packages/gemini-image/src/gemini_image/io.py (4)
get_extension_for_format(102-118)load_metadata(275-301)save_image(140-205)save_metadata(208-272)packages/gemini-image/src/gemini_image/registry.py (2)
PromptRegistry(68-282)add_entry(144-225)packages/gemini-image/src/gemini_image/response_parser.py (4)
GenerationResponse(44-75)parse_response(78-138)has_image(68-70)format(38-40)src/python_libs/utils/logging.py (1)
get_logger(131-157)
🔇 Additional comments (27)
packages/gemini-image/SECURITY.md (1)
1-67: Well-structured security policy with clear guidance and proper formatting.The document effectively communicates security practices across all required areas: supported versions, responsible vulnerability disclosure with response SLAs, and actionable best practices for API key management, output safety, and dependency monitoring. All lines comply with the 120-character markdown guideline, and the hierarchical structure with clear sections enhances readability for users implementing the library securely.
sonar-project.properties (1)
18-19: Add missing test directory to sonar.tests configuration.The explicit test directory listing is incomplete.
packages/cloudflare-api/tests/exists in the project but is missing from thesonar.testsproperty, which means these tests will not be scanned by SonarCloud. Add this directory to the configuration:sonar.tests=tests/,packages/cloudflare-api/tests/,packages/cloudflare-auth/tests/,packages/gcs-utilities/tests/,packages/gemini-image/tests/Additionally, the original concern about maintainability remains valid—future test directories will require manual updates to this configuration file.
⛔ Skipped due to learnings
Learnt from: CR Repo: ByronWilliamsCPA/python-libs PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-14T22:54:23.019Z Learning: Applies to tests/**/*.py : Maintain minimum 80% test coverage as enforced by CI/CD pipelineREUSE.toml (1)
11-27: Good coverage expansion for package scripts (MIT annotation).Adding
"packages/**/scripts/**/*.py"closes a gap for Python scripts living under package directories. Verification confirms no files currently exist underpackages/**/scripts/, so this is a preemptive annotation; if scripts are added there in the future, they will be automatically covered by this pattern.packages/gemini-image/scripts/validate_installation.py (9)
1-8: LGTM!Clear documentation with practical usage examples for both direct Python execution and uv run.
10-12: LGTM!Minimal imports at module level allow the script to run even if gemini_image has import issues, enabling proper error reporting.
15-26: LGTM!Properly validates package importability with appropriate noqa annotation for the intentionally unused import.
28-47: LGTM!Validates the main public API functions are both importable and callable, covering the key entry points.
50-82: LGTM!Validates the exception hierarchy is properly structured, ensuring all custom exceptions inherit from Exception.
103-116: LGTM!Properly validates the CLI entry point is importable and callable.
119-132: LGTM!Validates required model configurations with helpful output showing all available models.
135-152: LGTM!Validates essential I/O functions are importable and callable, covering format detection, saving, and validation.
207-208: LGTM!Standard entry point pattern with proper exit code propagation.
packages/gemini-image/docs/USAGE_GUIDE.md (8)
1-473: Line length compliant with 120-character guideline.All lines are within the 120-character limit specified for Markdown documentation files.
375-429: Well-designed error handling and troubleshooting section.The troubleshooting section provides actionable guidance for each exception type with clear solutions and code examples. The error handling approach aligns well with best practices, particularly the RateLimitError example showing proper retry patterns. Based on learnings, this exemplifies specific, user-friendly exception messaging.
1-10: Comprehensive and well-structured usage documentation.The guide provides clear coverage of installation, configuration, basic and advanced features, workflow patterns, and troubleshooting. Documentation structure with table of contents, code examples for both Python API and CLI, and clear explanations of new features (batch processing, story sequences, draft-finalize, metadata, registry) makes this a valuable reference. The troubleshooting and best practices sections are practical and actionable.
157-161: The reference image limits are correctly documented and match the implementation in models.py (Flash: 3, Pro: 14). All line lengths comply with the 120-character Markdown guideline. No changes needed.
255-272: The memory guidance and code example are accurate and properly formatted.All lines comply with the 120-character markdown guideline. The chunked_batch example correctly aligns with the generate_batch API signature, and the chunking approach is valid for managing memory usage during batch operations. The memory estimates (~50-100MB per 4K image) are reasonable for uncompressed RGB data and serve as helpful guidance for developers.
276-303: Remove unsubstantiated cost savings claim or substantiate with Google Gemini pricing documentation.The draft-finalize workflow is correctly implemented (is_draft parameter and finalize_draft function match the documentation), and all line lengths comply with the 120-character markdown guideline. However, the "~75% on costs during iteration" claim cannot be verified from the code alone—it requires documented current pricing from Google Gemini API. Either remove this specific percentage estimate or provide a link to the pricing documentation that supports it.
79-90: All documented exception types, CLI flags, and API functions align with the implementation. The 7 exception types (ConfigurationError, ValidationError, APIError, RateLimitError, ContentBlockedError, FileOperationError, FormatDetectionError) all exist in exceptions.py. CLI flags including --batch, --resume, --draft-mode, --finalize, --no-document, --list-models, --verbose, --save-thoughts, --story-parts, and shorthand options (-r, -o, -d) are all defined in cli.py. The API functions (generate_image, generate_batch, generate_story_sequence, finalize_draft) are correctly documented with accurate signatures in generator.py. Model names "Gemini 2.5 Flash" and "Gemini 3 Pro" match the implementation definitions. Code examples at lines 84-87 use the correct API with model_key parameter. No line length violations detected.
79-90: No issues found. The documented model names "Gemini 2.5 Flash" and "Gemini 3 Pro" are accurate and current as of January 2026. Their stated capabilities—fast iterations and drafts for Flash, and high quality text rendering with 4K support for Pro—align with the latest Google Gemini API specifications. All lines in the snippet comply with the 120-character limit for Markdown documentation.Likely an incorrect or invalid review comment.
packages/gemini-image/src/gemini_image/generator.py (7)
42-185: Well-structured image generation flow.The function properly validates inputs, builds API requests, parses responses, and handles output/metadata/registry concerns in a clean, modular fashion. The defensive check at lines 146-148 with the pragma comment is appropriate for guarding against edge cases.
379-492: LGTM!The story sequence generation logic is well-structured with proper resume support via
_find_existing_image(), consistent UTC timestamps, and appropriate break-on-failure behavior to maintain visual continuity.
601-618: LGTM!Clean helper for finding existing images regardless of extension. The approach handles format correction scenarios where the saved extension differs from the requested one.
621-685: Well-structured validation helpers.Consistent use of
ValidationErrorwithfield,value, andvalid_optionsprovides excellent error messages for debugging. The reference image limit validation correctly enforces the per-model limits (3 for flash, 14 for pro).
734-768: LGTM!Clean configuration builder that correctly handles model-specific options (image config for pro model) and optional Google Search grounding.
771-788: LGTM!Output path determination correctly distinguishes absolute vs. relative paths and generates consistent timestamped filenames with appropriate draft/generated prefixes.
791-837: LGTM!The thought image saving helper properly handles indexing and extension detection. The story prompt builder provides clear narrative context for opening, middle, and concluding scenes.
| ### Metadata Sidecar Files | ||
|
|
||
| Each image can have a JSON metadata file: | ||
|
|
||
| ```python | ||
| # Enable metadata file (default) | ||
| result = generate_image( | ||
| "Test prompt", | ||
| save_metadata_file=True, # Creates result.json alongside result.png | ||
| ) | ||
| ``` | ||
|
|
||
| The metadata file contains: | ||
|
|
||
| ```json | ||
| { | ||
| "prompt": "Test prompt", | ||
| "model": "pro", | ||
| "aspect_ratio": "16:9", | ||
| "image_size": "2K", | ||
| "created_at": "2026-01-09T14:30:22Z" | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check metadata structure in io.py
echo "=== Metadata Implementation ==="
rg -A 15 "def save_metadata" packages/gemini-image/src/gemini_image/io.py
# Check PROMPTS.md registry implementation
echo -e "\n=== Registry Implementation ==="
rg -A 20 "class PromptRegistry|def add|PROMPTS" packages/gemini-image/src/gemini_image/registry.py | head -60
# Check document parameter behavior
echo -e "\n=== Document Parameter Usage ==="
rg "document" packages/gemini-image/src/gemini_image/ --type py -B 2 -A 2 | head -50
# Check line lengths in the documentation
echo -e "\n=== Line Length Check ==="
awk 'length > 120 {print NR ": " length " chars: " $0}' packages/gemini-image/docs/USAGE_GUIDE.md | head -20Repository: ByronWilliamsCPA/python-libs
Length of output: 6091
🏁 Script executed:
#!/bin/bash
# Check actual metadata JSON structure being created
echo "=== Metadata JSON Creation ==="
rg -A 30 "save_metadata" packages/gemini-image/src/gemini_image/io.py | grep -A 30 "def save_metadata"
# Check how metadata dict is built
echo -e "\n=== Metadata Dict Structure ==="
rg "created_at|metadata\[|metadata =" packages/gemini-image/src/gemini_image/io.py -B 2 -A 2
# Check PROMPTS.md entry template and what fields are added
echo -e "\n=== Registry Entry Template and Fields ==="
rg -A 20 "ENTRY_TEMPLATE|add_entry" packages/gemini-image/src/gemini_image/registry.py
# Verify document parameter control flow
echo -e "\n=== Document Parameter Control Flow ==="
rg -B 3 -A 8 "if document:" packages/gemini-image/src/gemini_image/generator.pyRepository: ByronWilliamsCPA/python-libs
Length of output: 4539
🏁 Script executed:
#!/bin/bash
# Check the exact lines 317-340 and 341-363 in USAGE_GUIDE.md
echo "=== Lines 317-340 (Registry section) ==="
sed -n '317,340p' packages/gemini-image/docs/USAGE_GUIDE.md
echo -e "\n=== Lines 341-363 (Metadata section) ==="
sed -n '341,363p' packages/gemini-image/docs/USAGE_GUIDE.md
# Check if there's a discrepancy in aspect_ratio/image_size being optional vs always shown
echo -e "\n=== Check metadata always has is_draft and created_at ==="
rg "metadata\[|{\"" packages/gemini-image/src/gemini_image/io.py -A 1 | head -15Repository: ByronWilliamsCPA/python-libs
Length of output: 1219
🏁 Script executed:
#!/bin/bash
# Let me get the actual metadata structure more carefully
echo "=== Full metadata dict construction ==="
rg -B 5 -A 25 'metadata = {' packages/gemini-image/src/gemini_image/io.py
# Check registry entry formatting more carefully
echo -e "\n=== Registry add_entry full implementation ==="
rg -A 50 'def add_entry' packages/gemini-image/src/gemini_image/registry.py | head -80Repository: ByronWilliamsCPA/python-libs
Length of output: 1997
🏁 Script executed:
#!/bin/bash
# Read the io.py file directly and find metadata dict construction
echo "=== Metadata dict construction in io.py ==="
cat packages/gemini-image/src/gemini_image/io.py | grep -A 20 "metadata ="
# Look at the full save_metadata function
echo -e "\n=== Full save_metadata function ==="
awk '/^def save_metadata/,/^def / {print NR": "$0}' packages/gemini-image/src/gemini_image/io.py | head -60Repository: ByronWilliamsCPA/python-libs
Length of output: 176
🏁 Script executed:
#!/bin/bash
# Read the io.py file directly to find the save_metadata function
echo "=== Reading io.py to find metadata structure ==="
wc -l packages/gemini-image/src/gemini_image/io.py
# Get lines around save_metadata
sed -n '208,280p' packages/gemini-image/src/gemini_image/io.pyRepository: ByronWilliamsCPA/python-libs
Length of output: 2464
Fix documented metadata JSON and registry format to match implementation.
The documented metadata JSON structure is incomplete—it omits the is_draft field, which is always present in the actual output. Additionally, the documented registry format (lines 317-340) shows a simple bullet-point structure, but the implementation uses a markdown table format with File, Model, Date, Type, Aspect Ratio, Size, and Prompt fields. Update both examples to reflect the actual implementation behavior.
🤖 Prompt for AI Agents
In @packages/gemini-image/docs/USAGE_GUIDE.md around lines 341 - 363, Update the
documentation examples to match actual output: include the is_draft boolean in
the metadata JSON produced by generate_image when save_metadata_file=True (so
the sample JSON includes "is_draft": true/false and retains prompt, model,
aspect_ratio, image_size, created_at), and replace the simple bullet-list
registry example with the actual markdown table format the implementation emits
(columns: File, Model, Date, Type, Aspect Ratio, Size, Prompt) so the registry
example matches the real output.
| def check_api_key() -> bool: | ||
| """Check if API key is configured.""" | ||
| print("Checking API key configuration...", end=" ") | ||
| try: | ||
| from gemini_image.client import get_api_key | ||
|
|
||
| key = get_api_key() | ||
| if key and len(key) > 10: | ||
| # Key found and valid length - don't log any part of the key | ||
| print("OK (API key configured)") | ||
| return True | ||
| print("FAILED: API key too short or empty") | ||
| return False | ||
| except Exception as e: | ||
| print(f"FAILED: {e}") | ||
| return False |
There was a problem hiding this comment.
Use specific exception types instead of bare except Exception.
Line 98 catches all exceptions broadly, which can mask unexpected errors and makes debugging harder. Catch specific exceptions like ImportError or ConfigurationError to handle known failure modes while allowing unexpected errors to surface.
Based on learnings, exceptions should be specific and appropriate for the failure mode.
🔒 Proposed fix to use specific exceptions
def check_api_key() -> bool:
"""Check if API key is configured."""
print("Checking API key configuration...", end=" ")
try:
from gemini_image.client import get_api_key
key = get_api_key()
if key and len(key) > 10:
# Key found and valid length - don't log any part of the key
print("OK (API key configured)")
return True
print("FAILED: API key too short or empty")
return False
- except Exception as e:
+ except (ImportError, AttributeError) as e:
print(f"FAILED: {e}")
return False🤖 Prompt for AI Agents
In @packages/gemini-image/scripts/validate_installation.py around lines 85 -
100, In check_api_key, avoid the broad "except Exception" around importing and
calling get_api_key; instead catch specific failure modes such as
ImportError/ModuleNotFoundError and AttributeError for import/name issues, plus
the specific configuration exception that gemini_image.client may raise (e.g.,
ConfigurationError or ValueError) when the key is missing/invalid, and log those
error messages the same way; update the except block to list these specific
exceptions (ImportError, ModuleNotFoundError, AttributeError, and the package's
ConfigurationError or ValueError fallback) so unexpected errors are not silently
swallowed.
| def main() -> int: | ||
| """Run all validation checks.""" | ||
| print("=" * 50) | ||
| print("Gemini Image Installation Validation") | ||
| print("=" * 50) | ||
| print() | ||
|
|
||
| checks = [ | ||
| ("Package Import", check_import), | ||
| ("Function Exports", check_exports), | ||
| ("Exception Classes", check_exceptions), | ||
| ("CLI Entry Point", check_cli), | ||
| ("Model Configurations", check_models), | ||
| ("I/O Functions", check_io_functions), | ||
| ("API Key", check_api_key), # Last since it's optional for basic install | ||
| ] | ||
|
|
||
| results: list[tuple[str, bool]] = [] | ||
| for name, check_fn in checks: | ||
| result = check_fn() | ||
| results.append((name, result)) | ||
|
|
||
| print() | ||
| print("=" * 50) | ||
| print("Summary") | ||
| print("=" * 50) | ||
|
|
||
| passed = sum(1 for _, ok in results if ok) | ||
| total = len(results) | ||
|
|
||
| for name, ok in results: | ||
| status = "PASS" if ok else "FAIL" | ||
| print(f" {name}: {status}") | ||
|
|
||
| print() | ||
| print(f"Result: {passed}/{total} checks passed") | ||
|
|
||
| # API key is optional for installation validation | ||
| core_passed = all(ok for name, ok in results if name != "API Key") | ||
|
|
||
| if core_passed: | ||
| print() | ||
| if not results[-1][1]: # API key failed | ||
| print("Note: API key not configured. Set GEMINI_API_KEY to enable generation.") | ||
| print("Installation is valid!") | ||
| return 0 | ||
| else: | ||
| print() | ||
| print("Installation has issues. Please check the errors above.") | ||
| return 1 |
There was a problem hiding this comment.
Replace hardcoded index with name-based lookup for robustness.
Line 197 uses results[-1][1] assuming the API Key check is always last. This assumption is fragile and will break silently if checks are reordered.
As per coding guidelines, assumptions should be tagged with #ASSUME comments, or better yet, eliminated. Use name-based lookup consistent with line 193 to make the code resilient to reordering.
🔧 Proposed fix to use name-based lookup
# API key is optional for installation validation
core_passed = all(ok for name, ok in results if name != "API Key")
if core_passed:
print()
- if not results[-1][1]: # API key failed
+ # Check if API key check failed
+ api_key_failed = any(name == "API Key" and not ok for name, ok in results)
+ if api_key_failed:
print("Note: API key not configured. Set GEMINI_API_KEY to enable generation.")
print("Installation is valid!")
return 0🤖 Prompt for AI Agents
In @packages/gemini-image/scripts/validate_installation.py around lines 155 -
204, In main(), avoid using the hardcoded index results[-1][1] to check the API
Key result; instead look up the API Key entry by name (e.g., build a dict from
results or iterate to find the tuple where name == "API Key") and use that value
to decide the note about API key configuration; update the code around the
core_passed check and the conditional that prints the API key note to reference
the name-based lookup (and remove the implicit assumption/#ASSUME).
| from gemini_image.client import GeminiClient, _get_genai | ||
| from gemini_image.exceptions import GenerationError, ValidationError |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Import FileOperationError at module level for consistency.
FileOperationError is imported locally at line 532 inside finalize_draft(), but it's already used by other functions via the exception hierarchy. Moving it to the top-level imports keeps the import structure consistent and avoids redundant local imports.
Proposed fix
-from gemini_image.exceptions import GenerationError, ValidationError
+from gemini_image.exceptions import (
+ FileOperationError,
+ GenerationError,
+ ValidationError,
+)Then remove the local import at line 532.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from gemini_image.client import GeminiClient, _get_genai | |
| from gemini_image.exceptions import GenerationError, ValidationError | |
| from gemini_image.client import GeminiClient, _get_genai | |
| from gemini_image.exceptions import ( | |
| FileOperationError, | |
| GenerationError, | |
| ValidationError, | |
| ) |
🤖 Prompt for AI Agents
In @packages/gemini-image/src/gemini_image/generator.py around lines 18 - 19,
Move the local import of FileOperationError out of finalize_draft() and add it
to the module-level imports alongside GeminiClient, _get_genai, GenerationError,
and ValidationError; then remove the redundant local import inside the
finalize_draft function so all code uses the single top-level FileOperationError
import consistently.
| if parallel > 1: | ||
| logger.warning( | ||
| "parallel_not_implemented", | ||
| requested=parallel, | ||
| using=1, | ||
| message="Parallel batch processing not yet implemented", | ||
| ) | ||
| parallel = 1 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider documenting the parallel limitation in the docstring.
The warning for parallel > 1 is appropriate for a v0.2.0 release, but the docstring at line 211 says "currently only 1 supported" which may be missed. Consider adding a note that parallel > 1 is a no-op for now or raising ValidationError if the feature isn't implemented.
| except Exception as e: | ||
| logger.error( | ||
| "batch_item_failed", | ||
| index=idx, | ||
| error=str(e), | ||
| ) | ||
| batch_results.append(None) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Broad exception handling may mask configuration errors.
Catching all Exception types allows batch processing to continue, but it also silently swallows ValidationError and ConfigurationError that indicate problems with the prompt configuration itself—not transient API failures.
Consider catching only expected API/generation exceptions and letting validation errors propagate:
Proposed fix
try:
result = generate_image(
prompt=prompt_text,
model_key=model_key, # type: ignore[arg-type]
reference_images=reference_images, # type: ignore[arg-type]
output_path=output_path,
output_dir=output_dir,
aspect_ratio=aspect_ratio, # type: ignore[arg-type]
image_size=image_size, # type: ignore[arg-type]
document=document,
)
batch_results.append(result)
- except Exception as e:
+ except (GenerationError, APIError) as e:
logger.error(
"batch_item_failed",
index=idx,
error=str(e),
)
batch_results.append(None)This would require importing APIError from gemini_image.exceptions. Alternatively, document that all errors are caught and logged to justify the current behavior.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @packages/gemini-image/src/gemini_image/generator.py around lines 347 - 353,
The catch-all except Exception in the batch loop (surrounding logger.error and
batch_results.append(None)) hides configuration/validation problems; change it
to only catch expected API/generation/transient errors (e.g., import and catch
APIError from gemini_image.exceptions and any other known transient exceptions)
and allow ValidationError/ConfigurationError to propagate (or explicitly
re-raise them if accidentally caught). Update the exception clause around the
block that currently references idx, logger.error("batch_item_failed", ...) and
batch_results.append(None) to target the specific exception types and remove the
broad Exception handler.
| # Try to load original metadata | ||
| metadata = load_metadata(draft_path) | ||
| original_prompt = None | ||
|
|
||
| if verbose: | ||
| print(f"Finalizing draft image: {draft_path}") # noqa: T201 | ||
| print(f"Target resolution: {final_size} ({final_aspect})") # noqa: T201 | ||
| if metadata: | ||
| original_prompt = metadata.get("prompt") | ||
| if verbose and original_prompt: | ||
| logger.info("using_original_prompt", preview=original_prompt[:50]) | ||
|
|
||
| # Use provided prompt or default upscaling prompt | ||
| final_prompt = prompt or ( | ||
| # Determine final resolution | ||
| final_size = image_size or "2K" | ||
| final_aspect = aspect_ratio or "16:9" | ||
|
|
||
| logger.info( | ||
| "finalizing_draft", | ||
| draft=str(draft_path), | ||
| target_size=final_size, | ||
| target_aspect=final_aspect, | ||
| ) | ||
|
|
||
| # Use provided prompt, original prompt, or default | ||
| final_prompt = prompt or original_prompt or ( | ||
| "Recreate this image at higher resolution with the same " | ||
| "composition, style, and details" | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider loading aspect_ratio from metadata when not provided.
When aspect_ratio is not explicitly provided, the function defaults to "16:9" (line 552). However, if the draft was generated with a different aspect ratio (e.g., "1:1"), this could produce an unexpected cropped or stretched result.
Consider extracting the original aspect ratio from metadata similarly to how original_prompt is handled:
Proposed fix
# Try to load original metadata
metadata = load_metadata(draft_path)
original_prompt = None
+ original_aspect_ratio = None
if metadata:
original_prompt = metadata.get("prompt")
+ original_aspect_ratio = metadata.get("aspect_ratio")
if verbose and original_prompt:
logger.info("using_original_prompt", preview=original_prompt[:50])
# Determine final resolution
final_size = image_size or "2K"
- final_aspect = aspect_ratio or "16:9"
+ final_aspect = aspect_ratio or original_aspect_ratio or "16:9"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Try to load original metadata | |
| metadata = load_metadata(draft_path) | |
| original_prompt = None | |
| if verbose: | |
| print(f"Finalizing draft image: {draft_path}") # noqa: T201 | |
| print(f"Target resolution: {final_size} ({final_aspect})") # noqa: T201 | |
| if metadata: | |
| original_prompt = metadata.get("prompt") | |
| if verbose and original_prompt: | |
| logger.info("using_original_prompt", preview=original_prompt[:50]) | |
| # Use provided prompt or default upscaling prompt | |
| final_prompt = prompt or ( | |
| # Determine final resolution | |
| final_size = image_size or "2K" | |
| final_aspect = aspect_ratio or "16:9" | |
| logger.info( | |
| "finalizing_draft", | |
| draft=str(draft_path), | |
| target_size=final_size, | |
| target_aspect=final_aspect, | |
| ) | |
| # Use provided prompt, original prompt, or default | |
| final_prompt = prompt or original_prompt or ( | |
| "Recreate this image at higher resolution with the same " | |
| "composition, style, and details" | |
| ) | |
| # Try to load original metadata | |
| metadata = load_metadata(draft_path) | |
| original_prompt = None | |
| original_aspect_ratio = None | |
| if metadata: | |
| original_prompt = metadata.get("prompt") | |
| original_aspect_ratio = metadata.get("aspect_ratio") | |
| if verbose and original_prompt: | |
| logger.info("using_original_prompt", preview=original_prompt[:50]) | |
| # Determine final resolution | |
| final_size = image_size or "2K" | |
| final_aspect = aspect_ratio or original_aspect_ratio or "16:9" | |
| logger.info( | |
| "finalizing_draft", | |
| draft=str(draft_path), | |
| target_size=final_size, | |
| target_aspect=final_aspect, | |
| ) | |
| # Use provided prompt, original prompt, or default | |
| final_prompt = prompt or original_prompt or ( | |
| "Recreate this image at higher resolution with the same " | |
| "composition, style, and details" | |
| ) |
🤖 Prompt for AI Agents
In @packages/gemini-image/src/gemini_image/generator.py around lines 541 - 565,
The code currently defaults final_aspect to "16:9" regardless of draft metadata;
change it to prefer a stored aspect ratio from metadata when aspect_ratio is not
provided by loading metadata (via load_metadata(draft_path)) and setting
final_aspect = aspect_ratio or (metadata.get("aspect_ratio") if metadata else
None) or "16:9"; also add a verbose logger.info branch similar to
original_prompt to indicate when the original aspect ratio from metadata is
being used (referencing symbols metadata, aspect_ratio, final_aspect,
load_metadata, draft_path).
| for img_path in reference_images: | ||
| if not img_path.exists(): | ||
| logger.warning("reference_image_not_found", path=str(img_path)) | ||
| continue |
There was a problem hiding this comment.
Consider raising an error for missing reference images.
Currently, a missing reference image only logs a warning and continues. This could lead to unexpected results if the user explicitly provided a reference image path and expects it to influence generation. The silent skip may be surprising.
Proposed fix
for img_path in reference_images:
if not img_path.exists():
- logger.warning("reference_image_not_found", path=str(img_path))
- continue
+ raise FileOperationError(
+ f"Reference image not found: {img_path}",
+ path=str(img_path),
+ operation="read",
+ )Alternatively, if skipping is intentional, document this behavior in the generate_image() docstring.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @packages/gemini-image/src/gemini_image/generator.py around lines 699 - 702,
The code currently logs and continues when a path in reference_images is missing
(the loop that calls logger.warning("reference_image_not_found",
path=str(img_path))); change this so missing user-supplied reference images
raise an exception (e.g., FileNotFoundError or ValueError) with a clear message
including str(img_path) inside the generate_image function (or the method that
contains the reference_images loop) instead of silently continuing; if skipping
missing references is intentional, update the generate_image() docstring to
explicitly document that behavior rather than leaving it implicit.
| # Detect MIME type from extension (will be validated by API) | ||
| suffix = img_path.suffix.lower() | ||
| mime_types = { | ||
| ".png": "image/png", | ||
| ".jpg": "image/jpeg", | ||
| ".jpeg": "image/jpeg", | ||
| ".gif": "image/gif", | ||
| ".webp": "image/webp", | ||
| } | ||
| mime_type = mime_types.get(suffix, "image/png") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
MIME type detection from extension may be inconsistent with actual format.
The MIME type is determined from the file extension, but io.py uses magic byte detection (detect_image_format) which handles cases where the extension doesn't match the actual format. Consider using the same approach here for consistency:
Proposed fix
+from gemini_image.io import detect_image_format, get_mime_type_for_format
# In _build_contents:
# Load image data
with open(img_path, "rb") as f:
data = f.read()
- # Detect MIME type from extension (will be validated by API)
- suffix = img_path.suffix.lower()
- mime_types = {
- ".png": "image/png",
- ".jpg": "image/jpeg",
- ".jpeg": "image/jpeg",
- ".gif": "image/gif",
- ".webp": "image/webp",
- }
- mime_type = mime_types.get(suffix, "image/png")
+ # Detect MIME type from magic bytes for accuracy
+ detected_format = detect_image_format(data)
+ mime_type = get_mime_type_for_format(detected_format)🤖 Prompt for AI Agents
In @packages/gemini-image/src/gemini_image/generator.py around lines 710 - 719,
The code in generator.py determines MIME type from the file extension (using
mime_types and img_path.suffix) which can disagree with actual image bytes;
change it to read the image bytes and call the shared detect_image_format
function (from io.py) to get the real format, then map that result to the
appropriate MIME string instead of relying on suffix. Update the logic around
mime_types/mime_type to use detect_image_format(img_bytes) (or equivalent) and
fallback to "image/png" only if detection fails.
Replace unsupported [[plugin.ignore]] and [[smells.ignore]] syntax with the correct [[triage]] blocks to ignore Bandit B101 (assert_used) and B108 (hardcoded_tmp_directory) in test files. This properly excludes pytest assert statements and test fixture /tmp usage from security scanning while keeping other security checks active. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.qlty/qlty.toml:
- Around line 97-102: Replace the blanket use of exclude_patterns for
gemini_image/generator.py by adding a [[triage]] block that targets that file
and the "smells" plugin: create a [[triage]] entry with match.file_patterns set
to ["gemini_image/generator.py"], match.plugins set to ["smells"], and
set.ignored set to true so complexity warnings for that file are suppressed
without removing it from other Qlty analyses.
- Around line 45-69: The triage entries in .qlty/qlty.toml use incorrect flat
keys (e.g., match.plugins, set.ignored) and therefore won't match Qlty's schema,
and even if fixed they are ineffective because Bandit is already configured to
exclude the tests directory in pyproject.toml; update each [[triage]] block to
the nested schema (use [triage.match] with plugins and rules arrays and
[triage.match.file_patterns], and [triage.set] with ignored = true) and then
either remove these test-specific triage rules or remove/adjust the Bandit
exclude_dirs setting in pyproject.toml (whichever policy you want) so the triage
rules can actually apply.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.qlty/qlty.toml
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-14T22:54:23.019Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T22:54:23.019Z
Learning: Applies to src/**/*.py : Run security scans with Bandit for code security and Safety for dependency vulnerabilities
Applied to files:
.qlty/qlty.toml
📚 Learning: 2025-12-14T22:54:23.019Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T22:54:23.019Z
Learning: Applies to **/*.py : Use Ruff linting with PyStrict-aligned rules including BLE, EM, SLF, INP, ISC, PGH, RSE, TID, YTT, FA, T10, and G rules
Applied to files:
.qlty/qlty.toml
📚 Learning: 2025-12-14T22:53:58.937Z
Learnt from: CR
Repo: ByronWilliamsCPA/python-libs PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-14T22:53:58.937Z
Learning: Applies to src/python_libs/**/*.py : Ensure code is self-documenting through clear naming; replace magic numbers with named constants; verify complexity is appropriate and could not be simplified
Applied to files:
.qlty/qlty.toml
| # ============================================================================ | ||
| # Issue Triage - Rule-level Ignores | ||
| # ============================================================================ | ||
| # Ignore Bandit B101 (assert_used) in test files - pytest uses assert statements | ||
| [[triage]] | ||
| match.plugins = ["bandit"] | ||
| match.rules = ["bandit:B101"] | ||
| match.file_patterns = [ | ||
| "**/tests/**", | ||
| "**/test_*.py", | ||
| "**/*_test.py", | ||
| ] | ||
| set.ignored = true | ||
|
|
||
| # Ignore Bandit B108 (hardcoded_tmp_directory) in test files - test fixtures use /tmp | ||
| [[triage]] | ||
| match.plugins = ["bandit"] | ||
| match.rules = ["bandit:B108"] | ||
| match.file_patterns = [ | ||
| "**/tests/**", | ||
| "**/test_*.py", | ||
| "**/*_test.py", | ||
| ] | ||
| set.ignored = true | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Qlty qlty.toml triage schema [[triage]] match.plugins match.rules match.file_patterns set.ignored
💡 Result:
Short summary — meaning and usage of the qlty.toml triage keys you asked about:
- [[triage]]: a rule block that matches issues and then overrides properties for matched issues (evaluated sequentially; first match wins). [1]
- match.plugins: list of plugin names whose reported issues should be matched (e.g., ["eslint", "rubocop"]). [1]
- match.rules: list of plugin-specific rule identifiers to match (e.g., ["eslint:react/prop-types"]). Wildcards are supported. [1]
- match.file_patterns: list of glob patterns to limit matches to files (default "*" matches all files). [1]
- set.ignored (boolean): when set to true the matched issues are ignored (suppressed at generation time and won’t appear in the web app). Use set.ignored = true under the [triage.set] block. [1][2]
Minimal example:
[[triage]]
[triage.match]
rules = ["eslint:react/prop-types"]
file_patterns = ["frontend/components/ui/**"]
[triage.set]
ignored = true
References:
- Qlty project config / triage docs. [1]
- Qlty CLI ignoring/triage examples (includes note that ignored issues are suppressed). [2]
🏁 Script executed:
# Search for pyproject.toml to check Bandit configuration
find . -name "pyproject.toml" -type f | head -5Repository: ByronWilliamsCPA/python-libs
Length of output: 250
🏁 Script executed:
# Check full qlty.toml for context around Bandit configuration
fd "qlty.toml" -type f | head -5Repository: ByronWilliamsCPA/python-libs
Length of output: 242
🏁 Script executed:
# Find and read .qlty/qlty.toml
find . -path "./.qlty/qlty.toml" -o -path "*/qlty.toml" | grep -E "\.qlty/qlty\.toml|qlty\.toml$" | head -1 | xargs -I {} sh -c 'wc -l {} && cat -n {}'Repository: ByronWilliamsCPA/python-libs
Length of output: 7460
🏁 Script executed:
# Check pyproject.toml for Bandit configuration
cat -n ./pyproject.toml | grep -A 10 -B 2 -i "bandit\|exclude"Repository: ByronWilliamsCPA/python-libs
Length of output: 4215
Triage configuration has two issues: incorrect TOML syntax and ineffective rules due to Bandit excluding tests.
The [[triage]] blocks use flat dot-notation keys (match.plugins, set.ignored) that don't match Qlty's documented schema, which requires nested [triage.match] and [triage.set] blocks. Additionally, these rules won't apply because Bandit is already configured in pyproject.toml to exclude the tests directory entirely (line 688: exclude_dirs = ["tests", ...]).
Corrected triage syntax
-# Ignore Bandit B101 (assert_used) in test files - pytest uses assert statements
-[[triage]]
-match.plugins = ["bandit"]
-match.rules = ["bandit:B101"]
-match.file_patterns = [
- "**/tests/**",
- "**/test_*.py",
- "**/*_test.py",
-]
-set.ignored = true
-
-# Ignore Bandit B108 (hardcoded_tmp_directory) in test files - test fixtures use /tmp
-[[triage]]
-match.plugins = ["bandit"]
-match.rules = ["bandit:B108"]
-match.file_patterns = [
- "**/tests/**",
- "**/test_*.py",
- "**/*_test.py",
-]
-set.ignored = true
+# Note: Bandit already excludes tests via exclude_dirs in pyproject.toml;
+# these triage rules are unnecessary. If test scanning is needed, remove
+# "tests" from tool.bandit.exclude_dirs in pyproject.toml and use:
+#
+# [[triage]]
+# [triage.match]
+# plugins = ["bandit"]
+# rules = ["bandit:B101", "bandit:B108"]
+# file_patterns = ["**/tests/**", "**/test_*.py", "**/*_test.py"]
+# [triage.set]
+# ignored = true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # ============================================================================ | |
| # Issue Triage - Rule-level Ignores | |
| # ============================================================================ | |
| # Ignore Bandit B101 (assert_used) in test files - pytest uses assert statements | |
| [[triage]] | |
| match.plugins = ["bandit"] | |
| match.rules = ["bandit:B101"] | |
| match.file_patterns = [ | |
| "**/tests/**", | |
| "**/test_*.py", | |
| "**/*_test.py", | |
| ] | |
| set.ignored = true | |
| # Ignore Bandit B108 (hardcoded_tmp_directory) in test files - test fixtures use /tmp | |
| [[triage]] | |
| match.plugins = ["bandit"] | |
| match.rules = ["bandit:B108"] | |
| match.file_patterns = [ | |
| "**/tests/**", | |
| "**/test_*.py", | |
| "**/*_test.py", | |
| ] | |
| set.ignored = true | |
| # ============================================================================ | |
| # Issue Triage - Rule-level Ignores | |
| # ============================================================================ | |
| # Note: Bandit already excludes tests via exclude_dirs in pyproject.toml; | |
| # these triage rules are unnecessary. If test scanning is needed, remove | |
| # "tests" from tool.bandit.exclude_dirs in pyproject.toml and use: | |
| # | |
| # [[triage]] | |
| # [triage.match] | |
| # plugins = ["bandit"] | |
| # rules = ["bandit:B101", "bandit:B108"] | |
| # file_patterns = ["**/tests/**", "**/test_*.py", "**/*_test.py"] | |
| # [triage.set] | |
| # ignored = true | |
🤖 Prompt for AI Agents
In @.qlty/qlty.toml around lines 45 - 69, The triage entries in .qlty/qlty.toml
use incorrect flat keys (e.g., match.plugins, set.ignored) and therefore won't
match Qlty's schema, and even if fixed they are ineffective because Bandit is
already configured to exclude the tests directory in pyproject.toml; update each
[[triage]] block to the nested schema (use [triage.match] with plugins and rules
arrays and [triage.match.file_patterns], and [triage.set] with ignored = true)
and then either remove these test-specific triage rules or remove/adjust the
Bandit exclude_dirs setting in pyproject.toml (whichever policy you want) so the
triage rules can actually apply.
| [smells] | ||
| # Mode: comment (add PR comments) vs block (fail CI) | ||
| mode = "comment" | ||
|
|
||
| # Ignore complexity warnings for files with documented intentional complexity | ||
| [[smells.ignore]] | ||
| rules = [ | ||
| "function-complexity", | ||
| "function-parameters", | ||
| "nested-control-flow", | ||
| ] | ||
| file_patterns = [ | ||
| "**/gemini_image/generator.py", | ||
| ] | ||
| # Note: Complexity warnings for gemini_image/generator.py are intentional | ||
| # and documented in the file header. Use exclude_patterns at top level if needed. | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Qlty configuration suppress smells complexity rules triage exclude_patterns
💡 Result:
Below is a concise recipe for configuring Qlty to exclude files, suppress/adjust smell (complexity) rules, and triage specific issues in .qlty/qlty.toml.
- Exclude files/paths
- Top-level key: exclude_patterns (glob list). These files are not processed by any analysis.
- Example:
exclude_patterns = ["/node_modules/", "/generated/", "**/*.min.js"]. [1][3]
- Global smell / complexity settings
- Top-level smells block controls overall behavior (mode) and thresholds (file_complexity, function_complexity).
- Example:
[smells]
mode = "comment" # or "block", "monitor", "disabled"
file_complexity = 50
function_complexity = 15. [1]
- Per-language smell tuning
- Use language..smells to override thresholds or duplication settings for a language.
- Example:
[language.rust.smells]
function_complexity.threshold = 20
duplication.nodes_threshold = 40. [1]
- Disable/ignore plugin checks on specific files
- Use [[exclude]] blocks to stop plugins from running on matching files (evaluated sequentially; first match wins).
- Example:
[[exclude]]
plugins = ["eslint"]
file_patterns = ["frontend/components/ui/**"]. [1][2]
- Triaging (suppress or change issue properties)
-
Use [[triage]] entries with match and set sub-blocks. Can match by plugins, rules (plugin-specific names), levels, file_patterns. Use set.ignored = true to suppress.
-
Example:
[[triage]]
match.rules = ["eslint:react/prop-types"]
match.file_patterns = ["frontend/components/ui/**"]
set.ignored = true[[triage]]
match.plugins = ["rubocop"]
match.levels = ["low"]
set.ignored = true. [1][2]
- Duplication settings
- smells.duplication and language..smells.duplication support threshold and nodes_threshold to tune duplicate-code detection.
- Example:
[smells.duplication]
threshold = 10
nodes_threshold = 30. [1]
References
- Qlty qlty.toml project config (exclude_patterns, smells, language, [[exclude]], [[triage]]). [1]
- Qlty docs: Excluding files and triage/ignore examples. [2][3]
If you want, I can produce a ready-to-drop .qlty/qlty.toml example tailored to your repo (languages, complexity thresholds, and globs).
Use [[triage]] to suppress complexity warnings without excluding the entire file.
Line 101–102 suggests using exclude_patterns if needed; that excludes the file from all Qlty analysis. If the goal is only to silence complexity-related smells for gemini_image/generator.py, use a [[triage]] block instead:
[[triage]]
match.file_patterns = ["gemini_image/generator.py"]
match.plugins = ["smells"]
set.ignored = trueThis suppresses smells for that file without removing it from other analyses.
🤖 Prompt for AI Agents
In @.qlty/qlty.toml around lines 97 - 102, Replace the blanket use of
exclude_patterns for gemini_image/generator.py by adding a [[triage]] block that
targets that file and the "smells" plugin: create a [[triage]] entry with
match.file_patterns set to ["gemini_image/generator.py"], match.plugins set to
["smells"], and set.ignored set to true so complexity warnings for that file are
suppressed without removing it from other Qlty analyses.
|
Summary
Major v0.2.0 release of the gemini-image library with comprehensive refactoring and new features:
generate_batch()function with rich progress indicatorsis_draftandfinalize_draft()Changes
New Features
generate_batch()for processing multiple prompts from JSONgenerate_story_sequence()for multi-part narrativesfinalize_draft()for upscaling draft imagesArchitecture
client.py: Gemini API client with retry logic and exception mappingio.py: File I/O with magic byte format detectionregistry.py: PROMPTS.md generation loggingresponse_parser.py: API response parsing and validationexceptions.py: Comprehensive exception hierarchyDocumentation
Tests
Test plan
uv run pytest tests/test_models.py tests/test_generator.py -v)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.