-
Notifications
You must be signed in to change notification settings - Fork 642
feat: improve CLI argument parsing with subcommands and enhanced help text #9244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: improve CLI argument parsing with subcommands and enhanced help text #9244
Conversation
- Add subcommands: serve, datasets, fixture, trace-fixture, demo - Improve argument organization with subparsers - Add comprehensive help text and descriptions - Maintain backward compatibility with existing functionality
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
Thank you for this PR! Could you help me understand the motivation for using conditional imports (i.e. using try/except) for the evals package? |
|
I use conditional imports for the evals package to support environments where it isn’t installed (e.g., local dev, lightweight installs). Without this, importing Phoenix can fail because Python resolves phoenix from the local src/phoenix/ directory first, which doesn’t include evals, causing ModuleNotFoundError. The try/except pattern solves this by:
This is a common Python pattern for optional dependencies and keeps the codebase flexible across dev and production environments. |
Thanks for the extra context — that makes sense. The only catch is that evals is still a required dependency, and removing it would break some Phoenix features that rely on shared components. Since it’s not obvious how to make it optional just yet, would you mind reverting the import statements for now? We’d be happy to merge the rest of the PR! |
|
Hey @minhkhoango, we really appreciate your interest in Phoenix and your desire to contribute. I do hear your desire to make the CLI more discoverable and ergonomic. At the moment, only the Perhaps you could help by letting us know what you found unintuitive about the current CLI and where we could improve short of exposing internal, non-user-facing APIs. |
Thanks for clarifying! Totally makes sense about evals being a required dependency. I’ll revert the import changes and keep the rest of the PR intact. Appreciate the context — will push the update shortly. |
Thanks for breaking that down — super helpful context. Before my changes, nearly all CLI help text was hidden via ==SUPPRESS==, so I surfaced it thinking it might improve discoverability. Now that I understand your focus on keeping the CLI minimal and internal, I’ll keep my changes focused on phoenix serve. Quick feedback: the only friction point was seeing hidden commands/options without any hints. Would it be useful if I opened an issue summarizing first-time user pain points (without exposing internal APIs)? |
Summary
This PR improves the Phoenix CLI by reorganizing arguments into logical subcommands and adding comprehensive help text for better user experience.
Changes
serve,datasets,fixture,trace-fixture,demoArgumentParsersubparsers to group related argumentsBenefits
phoenix <command> --helpfor specific helpTesting
Example Usage
Breaking Changes
None - this is a backward-compatible enhancement.