Feature/style system consolidation#5
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors and enhances the prompt generation capabilities by introducing a consolidated and extensible style system. The changes centralize style definitions, expand the range of available prompt styles, and improve the robustness of style loading and error reporting. This makes the prompt generation nodes more flexible and easier to manage for future additions and modifications. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request consolidates the style system for the ComfyUI Prompt Generator by moving style definitions out of the code and into configuration files and dedicated modules, enhancing extensibility. However, a potential Denial of Service vulnerability was identified in the streaming generation logic, specifically due to the use of unmanaged threads for fetching chunks from the Ollama API, which could lead to thread exhaustion if the API hangs. It is recommended to use a more robust streaming implementation, such as a thread pool with a fixed size or an asynchronous approach, to prevent resource depletion. Furthermore, the review includes suggestions to improve maintainability by reducing data duplication across style implementations and addresses a minor issue with exception handling.
| "still_image": { | ||
| "name": "Still Image (Photography)", | ||
| "description": "Sharp, realistic photography with technical camera specifications", | ||
| "template": """Write a detailed Stable Diffusion prompt for: {{ description }} | ||
|
|
||
| Style: Generate a professional photography still image with sharp focus. | ||
| {% if emphasis %}Focus particularly on: {{ emphasis }}{% endif %} | ||
| {% if mood %}Mood/Atmosphere: {{ mood }}{% endif %} | ||
|
|
||
| Include specific details about: | ||
| - Camera settings (ISO 100, f/2.8 aperture, sharp optics) | ||
| - Natural or studio lighting | ||
| - Realistic textures and fine details | ||
| - Clean, balanced composition | ||
| - Professional photography qualities | ||
|
|
||
| Format the response as a single, detailed photography prompt.""", | ||
| }, |
There was a problem hiding this comment.
While adding this style to the DEFAULT_STYLES fallback is necessary with the current design, this large dictionary duplicates all the templates from config/templates.yaml. This creates a maintenance burden, as any changes to templates.yaml must be manually synchronized here. This is error-prone and goes against the goal of consolidating the style system. Consider reducing this duplication. One approach is to embed the default YAML content as a single multi-line string and parse it, ensuring there's only one source of truth for the default templates. Another is to make templates.yaml a required file and raise an error if it's missing or unreadable, removing the need for a large fallback dictionary.
| StyleMode.ANIME: StyleDefinition( | ||
| name="Anime", | ||
| description="Vibrant anime-style illustration with dynamic colors", | ||
| keywords=StyleKeywords( | ||
| primary=[ | ||
| "anime style", | ||
| "manga illustration", | ||
| "cel shading", | ||
| "vibrant colors", | ||
| ], | ||
| lighting=[ | ||
| "soft glow", | ||
| "dramatic backlighting", | ||
| "ambient light", | ||
| "bloom effect", | ||
| ], | ||
| technical=[ | ||
| "clean linework", | ||
| "flat color areas", | ||
| "high contrast", | ||
| "detailed eyes", | ||
| ], | ||
| composition=[ | ||
| "dynamic pose", | ||
| "expressive composition", | ||
| "layered background", | ||
| ], | ||
| texture=["smooth gradients", "soft skin tones", "vivid saturation"], | ||
| ), | ||
| ), |
There was a problem hiding this comment.
This PR consolidates styles, which is great. However, it maintains two separate sources of truth for style definitions: the keyword lists here in STYLE_DEFINITIONS, and the LLM prompt templates in config/templates.yaml (with a fallback in prompt_generator_node.py). While they serve different nodes, they represent the same conceptual styles. This means adding or modifying a style requires changes in multiple places, increasing maintenance overhead and risk of them becoming out of sync. For true consolidation, consider deriving one from the other, or defining them together in a single source file (e.g., a more structured YAML) from which both the keyword lists and the LLM templates are generated.
1 participant
No description provided.