You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In multiple prompt flows, the returned value may be a Choice or a raw string. Ensure consistent handling and typing (especially when accessing .value or .value.data) to avoid runtime errors if the prompt returns unexpected types.
formatter_choices= [
Choice("⚫ black", data="black"),
Choice("⚡ ruff", data="ruff"),
Choice("🔧 other", data="other"),
Choice("❌ don't use a formatter", data="don't use a formatter"),
]
result=prompts.select("Which code formatter do you use?", choices=formatter_choices, default=formatter_choices[0])
ifresult.commandisNone:
apologize_and_exit()
formatter=result.value.dataifisinstance(result.value, Choice) elseresult.valuegit_remote=""
Several prompts treat result.command is None as a cancel and immediately exit. Verify this matches desired UX in all places (e.g., module/tests selection, GH Actions setup) and won’t surprise users by exiting instead of re-prompting.
default_choice=project_nameifproject_nameinmodule_subdir_optionselsemodule_subdir_options[0]
result=prompts.select(
"Which Python module do you want me to optimize?", choices=module_subdir_options, default=default_choice
)
ifresult.commandisNone:
apologize_and_exit()
module_root_answer=result.valueifmodule_root_answer==curdir_option:
The wrapper relies on inquirer_textual internal widget classes and return structure. Confirm these APIs and the .run(inline=True) return shape (command/value) are stable across versions to prevent breakages.
Use a proper boolean confirm prompt to avoid fragile string comparisons and localization issues. Replace the select with prompts.confirm and branch on its boolean value.
-result = prompts.select("Create pyproject.toml in the current directory?", choices=["Yes", "No"], default="Yes")-if result.command is None or result.value == "No":+result = prompts.confirm("Create pyproject.toml in the current directory?", default=True)+if result.command is None or result.value is False:
apologize_and_exit()
create_empty_pyproject_toml(pyproject_toml_path)
Suggestion importance[1-10]: 7
__
Why: Replacing a Yes/No select with a boolean confirm reduces string-comparison fragility and improves UX; the improved_code aligns with the PR’s new prompt framework (prompts.confirm) and keeps behavior consistent.
Medium
Normalize select default and value handling
Ensure the default passed to select matches the widget’s expected type. Pass the underlying value (label or Choice) consistently and avoid mixing plain strings and Choice objects to prevent selection mismatch.
formatter_choices = [
Choice("⚫ black", data="black"),
Choice("⚡ ruff", data="ruff"),
Choice("🔧 other", data="other"),
Choice("❌ don't use a formatter", data="don't use a formatter"),
]
-result = prompts.select("Which code formatter do you use?", choices=formatter_choices, default=formatter_choices[0])+default_choice = formatter_choices[0]+result = prompts.select("Which code formatter do you use?", choices=formatter_choices, default=default_choice)
if result.command is None:
apologize_and_exit()
-formatter = result.value.data if isinstance(result.value, Choice) else result.value+formatter = result.value.data
Suggestion importance[1-10]: 4
__
Why: Passing a Choice as default is already done and supported; forcing formatter = result.value.data may be slightly cleaner but the current isinstance guard is safe, making the improvement minor.
Low
General
Validate non-empty choices
Guard against empty choices to prevent runtime errors inside the widget and provide a clear failure path. Validate input early and raise a helpful error message.
def select( # noqa: ANN201
message: str, choices: list[str | Choice], default: str | Choice | None = None
): # type: ignore[no-untyped-def]
+ if not choices:+ raise ValueError("select() requires at least one choice")
widget = InquirerSelect(message, choices, default, mandatory=True)
app: CodeflashThemedApp = CodeflashThemedApp(widget, shortcuts=None, show_footer=False)
return app.run(inline=True)
Suggestion importance[1-10]: 6
__
Why: Adding an early check for empty choices prevents runtime errors and clarifies failures; it's a small but useful robustness improvement that accurately updates the provided code block.
Code Review: PR #980 - Replace inquirer with inquirer-textual
Thanks for this refactoring! The migration to inquirer-textual improves the CLI experience. However, I've identified several issues that should be addressed.
🔴 Critical Issues - Must Fix
1. Missing Return Type Annotations (themed_prompts.py)
All functions suppress return type warnings with # noqa: ANN201. This defeats type safety:
# Current (problematic):defselect(message: str, choices: list[str|Choice], ...): # noqa: ANN201# Should be:defselect(message: str, choices: list[str|Choice], ...) ->InquirerResult:
Impact: Makes API unclear to users, prevents type checking from catching bugs.
2. Unsafe Exception Handling (validators.py:85)
exceptException: # noqa: S110pass
Issue: Silently swallows ALL exceptions including KeyboardInterrupt, SystemExit. Fix: Catch specific exceptions:
Risk: No validation that result.value has correct number of elements. Will raise ValueError if structure changes. Fix: Use explicit indexing or a dataclass with named fields.
🟡 Important Issues - Should Fix
6. Type Checker Workarounds
Multiple places have unreachable code for type checker:
ifnotapi_key:
apologize_and_exit()
return# unreachable, satisfies type checker
Fix: Add -> NoReturn type hint to apologize_and_exit():
Extract CSS - The 50+ line CSS string in CodeflashThemedApp makes testing difficult. Consider extracting to a constant.
Add Retry Delays (github_utils.py:88-104) - Immediately retries without backoff could hammer the API.
Reduce API Key Exposure - Keys printed to console (cmd_init.py:202) could be captured in logs. Consider masking more characters.
Break Up Large Functions - init_codeflash() is 200+ lines, install_github_actions() is 110+ lines. These violate single responsibility principle.
✅ Positive Changes
Clean separation of validators into dedicated module
Removal of overly complex text-wrapping logic
Good use of dataclasses for configuration
Improved UX with themed prompts
📊 Overall Assessment
Risk Level: Medium
The refactoring successfully removes the inquirer dependency and introduces cleaner architecture. However, the lack of return type annotations, unsafe exception handling, and the os.pathsep bug need to be addressed before merge. The missing test coverage is also concerning.
Recommendation: Fix critical issues #1-5, add basic test coverage, then merge. Nice-to-haves can be addressed in follow-up PRs.
@aseembits93 both of them together take up 50mb in dependencies, with inquirer taking up most of the bulk, and also I want to open the door towards using Textual bit by bit.
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
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.
PR Type
Enhancement, Other
Description
Replace Inquirer with Textual prompts
Add themed prompt wrapper module
Update CLI flows to new API
Adjust dependencies and mypy config
Diagram Walkthrough
File Walkthrough
cli_common.py
Remove legacy inquirer helpers from CLI commoncodeflash/cli_cmds/cli_common.py
cmd_init.py
Port init workflow to inquirer-textual promptscodeflash/cli_cmds/cmd_init.py
themed_prompts.py
Add themed prompt wrapper around inquirer-textualcodeflash/cli_cmds/themed_prompts.py
pyproject.toml
Update dependencies and mypy config for new promptspyproject.toml