Added CLI Support - #11
Closed
CyrixJD115 wants to merge 5 commits into
Closed
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe project replaces legacy image models with Flash and Illustration, updates generation and CLI download behavior, adds modern packaging metadata and entrypoint loading, and revises installation and usage documentation. ChangesMAI model and CLI update
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as bingart.cli
participant Client as BingArt
participant API as Remote API
participant Downloader as download_results
User->>CLI: Run bingart with prompt and options
CLI->>Client: Generate with model and aspect
Client->>API: Submit creation request
API-->>Client: Return redirect and generation result
Client-->>CLI: Return image URLs
CLI->>Downloader: Download result URLs
Downloader-->>User: Report formatted output and download status
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
bingart/cli.py (1)
78-94: Unnecessary mutually exclusive groups for single arguments.The
model_groupandaspect_groupeach contain only one argument, making the mutually exclusive wrapper unnecessary. These could be simplified to directparser.add_argument()calls.♻️ Suggested simplification
- model_group = parser.add_mutually_exclusive_group() - model_group.add_argument( + parser.add_argument( "-m", "--model", choices=list(MODEL_MAP.keys()), default="dalle", help="AI model to use (default: dalle).", ) - aspect_group = parser.add_mutually_exclusive_group() - aspect_group.add_argument( + parser.add_argument( "-a", "--aspect", choices=list(ASPECT_MAP.keys()), default="square", help="Aspect ratio (default: square).", )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bingart/cli.py` around lines 78 - 94, The parser uses unnecessary mutually exclusive groups model_group and aspect_group each wrapping a single add_argument; remove the calls to parser.add_mutually_exclusive_group() and register the options directly via parser.add_argument for --model and --aspect (keeping choices=list(MODEL_MAP.keys())/list(ASPECT_MAP.keys()), defaults "dalle" and "square", and the same help texts) so you no longer reference model_group or aspect_group and simplify the CLI setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bingart/cli.py`:
- Around line 165-182: The async function download_file currently calls blocking
APIs (urllib.request.urlretrieve and curl_cffi.requests.get) and silences all
exceptions, so convert the blocking calls to run in the event loop's executor
(e.g., loop.run_in_executor) or replace them with an async HTTP client (aiohttp)
inside download_file; capture exceptions instead of using bare except: pass and
log debug-level details (when verbose or logger is available) including the URL
and exception info to aid troubleshooting; update the branches that use
urllib.request.urlretrieve and the curl_cffi get call accordingly and ensure the
function still returns True/False based on success.
In `@README.MD`:
- Around line 33-37: Update the README clone command to use the main repository
URL instead of the contributor's fork: replace the git clone URL
`https://github.com/CyrixJD115/bingart.git` with
`https://github.com/DedInc/bingart.git` in the README.MD example so users clone
the canonical repo (the surrounding commands `cd bingart` and `python bingart.py
--help` remain unchanged).
---
Nitpick comments:
In `@bingart/cli.py`:
- Around line 78-94: The parser uses unnecessary mutually exclusive groups
model_group and aspect_group each wrapping a single add_argument; remove the
calls to parser.add_mutually_exclusive_group() and register the options directly
via parser.add_argument for --model and --aspect (keeping
choices=list(MODEL_MAP.keys())/list(ASPECT_MAP.keys()), defaults "dalle" and
"square", and the same help texts) so you no longer reference model_group or
aspect_group and simplify the CLI setup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dc4638ad-ee48-497c-b1e1-2624fd625ae1
📒 Files selected for processing (5)
README.MDbingart.pybingart/__init__.pybingart/cli.pysetup.py
…mplify arg parser - Run urllib and curl_cffi blocking calls in run_in_executor to avoid blocking the event loop in download_file - Log debug-level details on download failure instead of silently swallowing exceptions - Update README clone URL to point to canonical DedInc/bingart repo - Remove unnecessary mutually exclusive groups for --model and --aspect
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
bingart/cli.py (1)
275-280: Avoid stacking duplicate log handlers across repeatedrun()calls.If
run(args)is invoked multiple times in one process, this adds a new handler each time and duplicates logs.♻️ Suggested guard
if args.verbose: logger.setLevel(logging.DEBUG) - handler = logging.StreamHandler(sys.stderr) - handler.setFormatter(logging.Formatter("%(levelname)s: %(message)s")) - logger.addHandler(handler) + if not logger.handlers: + handler = logging.StreamHandler(sys.stderr) + handler.setFormatter(logging.Formatter("%(levelname)s: %(message)s")) + logger.addHandler(handler)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bingart/cli.py` around lines 275 - 280, The verbose logging branch in run (conditional on args.verbose) unconditionally creates and adds a StreamHandler to logger, causing duplicate handlers and repeated log output when run() is called multiple times; update the run function to check for an existing equivalent handler before adding (e.g., inspect logger.handlers for a StreamHandler with the same formatter/level) or clear/add handlers idempotently so logger.addHandler(handler) only runs once, ensuring the logging setup (handler creation, formatter assignment, and logger.setLevel/logging.debug call) is guarded against duplicate registration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bingart/cli.py`:
- Around line 163-172: download_file currently passes the raw URL into
urllib.request.urlretrieve which allows non-http(s) schemes; before calling
loop.run_in_executor (and before urlretrieve), parse the URL (e.g.,
urllib.parse.urlparse) and explicitly check that parsed.scheme is "http" or
"https", and if not, raise a ValueError or return an error/log via the existing
logger; ensure the validation happens early in the download_file function
(before the lambda using urllib.request.urlretrieve) so only allowed schemes
reach urlretrieve.
In `@README.MD`:
- Around line 46-47: Update the primary CLI examples in README.MD to use the
installed entrypoint `bingart` instead of `python bingart.py` (e.g., change
`python bingart.py "sunset over mountains" -c YOUR_U_COOKIE` to `bingart "sunset
over mountains" -c YOUR_U_COOKIE`), and keep the `python bingart.py ...` form
only in the "From Source (Portable)" section; apply this replacement
consistently for all occurrences referenced (examples around the shown snippets
and the ranges noted).
- Line 63: Several fenced code blocks lack language identifiers causing
markdownlint MD040; update each triple-backtick block that contains plain text
examples (e.g., the block starting with "usage: bingart [-h]...", blocks showing
"Model: DALLE", and the block with "https://th.bing.com/th/id/OIG...") to
include a language tag such as ```text (or ```bash for CLI examples) so every
fenced code block has an explicit language identifier.
---
Nitpick comments:
In `@bingart/cli.py`:
- Around line 275-280: The verbose logging branch in run (conditional on
args.verbose) unconditionally creates and adds a StreamHandler to logger,
causing duplicate handlers and repeated log output when run() is called multiple
times; update the run function to check for an existing equivalent handler
before adding (e.g., inspect logger.handlers for a StreamHandler with the same
formatter/level) or clear/add handlers idempotently so
logger.addHandler(handler) only runs once, ensuring the logging setup (handler
creation, formatter assignment, and logger.setLevel/logging.debug call) is
guarded against duplicate registration.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
…e blocks, guard duplicate handler - Validate URL scheme (http/https only) before download to prevent non-http URLs reaching urllib - Replace python bingart.py with bingart entrypoint in all CLI examples outside the From Source section - Add text language identifiers to bare fenced code blocks in README - Guard StreamHandler registration against duplicate accumulation in run()
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
…MAI-Image-2e - Replace setup.py with PEP 621 pyproject.toml (uv-compatible) - Cap requires-python at <3.13 so rookiepy installs from wheels - Rewrite CLI help to be plain-language and readable - Update models: DALL-E/GPT-4O removed; add FLASH (MAI-Image-2.5-Flash) and ILLUSTRATION (MAI-Image-2e) with correct mdl/body mappings - Drop obsolete GPT-4O streaming poll logic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added CLI for easy command-line execution and scripting.
Summary by CodeRabbit
New Features
bingartcommand.Documentation
Chores