fix(cli): accept auto/mps devices in pixelrag index build#73
Conversation
|
@codysj is attempting to deploy a commit to the andylizf's projects Team on Vercel. A member of the Team first needs to authorize it. |
andylizf
left a comment
There was a problem hiding this comment.
Thanks — the --device choices fix is correct and needed (auto/mps work downstream but argparse rejects them).
However, this PR is based on an older main and its diff reverts 10+ recently-merged PRs (~680 lines of unrelated regressions: removes --cdp-url, --user-data-dir isolation, pre-commit hooks, non-numeric ID fix, agent timeouts, version bump, etc.).
Could you rebase onto the latest origin/main? The fix itself is just pipelines.py (the choices line) + the new test — everything else should drop away after rebase.
git fetch upstream main
git rebase upstream/main
git push --force-with-lease0861720 to
2779740
Compare
|
Rebased onto latest main, diff is now just the --device choices line + the CLI test. Thanks for the catch! |
|
CI is failing because Fix: wrap the test with a skip if the index stage isn't installed: def test_index_build_device_choices():
r = _run("pixelrag", "index", "build", "--help")
if r.returncode != 0 and "not installed" in (r.stdout + r.stderr):
pytest.skip("index stage not installed")
assert r.returncode == 0
assert "{auto,cpu,mps,cuda}" in r.stdoutOr add |
Adds 'auto' and 'mps' to --device choices for pixelrag index build. The test gracefully skips in CI where the index stage (pyyaml) isn't installed, preventing the ModuleNotFoundError that broke PR StarTrail-org#73. Supersedes StarTrail-org#73 (same fix + CI-compatible test).
Summary
pixelrag index build --deviceonly acceptscpu/cuda, but the rest of thestack supports
auto/mpstoo:build()routesdevice in ("cpu","mps","auto")to the local embedder, and
embed_cpudeclareschoices=["auto","cpu","mps","cuda"].So
--device auto(the smart default) and--device mpsfail at argparse with"invalid choice" even though they'd work.
Config (
device: autoin pixelrag.yaml) is unaffected, this only fixes the CLIoverride flag, which currently can't express the values the config can.
Changes
--devicechoices toauto, cpu, mps, cudato match build()/embed_cpu.Testing