fix(extract): reject invalid CLI arguments#2026
Conversation
5f293e1 to
ec6a901
Compare
| print(f"error: path not found: {target}", file=sys.stderr) | ||
| sys.exit(1) | ||
|
|
||
| backend: str | None = None |
There was a problem hiding this comment.
The argument parsing logic is easy to follow. Would using argparse simplify maintenance as the number of CLI options continues to grow?
There was a problem hiding this comment.
Thanks. argparse could make sense as a broader CLI refactor. This PR is intentionally focused on validating the existing extract arguments.
| "cli_cargo": False, | ||
| "cli_allow_partial": False, | ||
| "no_cluster": False, | ||
| "dedup_llm": False, |
There was a problem hiding this comment.
Nice validation for the target path. Should we also validate that out_dir is writable before proceeding to avoid failing later in the execution?
There was a problem hiding this comment.
Good point. I would handle this in a follow-up by catching the actual OSError, since a pre-check can become stale.
| code_only: bool = parsed["code_only"] | ||
| no_gitignore: bool = parsed["no_gitignore"] | ||
| global_repo_tag: str | None = parsed["global_repo_tag"] | ||
| cli_max_workers: int | None = parsed["cli_max_workers"] |
There was a problem hiding this comment.
I like the dedicated _parse_int() helper. Would it make sense to follow the same pattern for float(args[i + 1]) (e.g. --exclude-hubs) to keep error handling consistent across numeric arguments?
There was a problem hiding this comment.
Yes. This PR already routes --exclude-hubs through _parse_percentile() for both spaced and = forms, with consistent conversion and range errors.
ec6a901 to
dd9ec20
Compare
2 participants
Summary
graphify extractarguments from one declarative option schema--exclude-hubsrange during parsing instead of returning a conversion tracebackResult
Tests
python -m pytest tests/ -q --tb=short— 3511 passed, 33 skippedpython -m pytest tests/test_extract_cli_args.py tests/test_extract_cli.py -q --tb=short— 116 passedpython -m ruff check graphify/cli.py tests/test_extract_cli_args.pytools.skillgenchecksgraphify update .Closes #2025.
Related to #2004.