Skip to content

feat(rewrite): treat uv run as transparent prefix - #1044

Closed
lgbarn wants to merge 2 commits into
rtk-ai:developfrom
lgbarn:feat/uv-run-transparent-prefix
Closed

lgbarn wants to merge 2 commits into
rtk-ai:developfrom
lgbarn:feat/uv-run-transparent-prefix

Conversation

@lgbarn

@lgbarn lgbarn commented Apr 5, 2026

Copy link
Copy Markdown

Summary

  • Handle uv run as a transparent prefix (like sudo/env) in both classify_command and rewrite_segment
  • The inner command is matched against existing rules; uv run is preserved so the virtualenv stays active
uv run pytest tests/ -v     → uv run rtk pytest tests/ -v
uv run ruff check .         → uv run rtk ruff check .
uv run python -m pytest     → uv run rtk pytest tests/
uv run mypy --strict src/   → uv run rtk mypy --strict src/

Motivation

uv run is the #1 unhandled command in rtk discover — 1,062 occurrences over 30 days. Since uv run is just a virtualenv wrapper, the real command follows it and can be matched by existing RTK rules.

Relationship to #176

PR #176 takes a different approach: a first-class rtk uv command with subcommand dispatch (uv run pytestrtk uv run pytest). This PR is complementary — it treats uv run as a transparent prefix so no new command handler is needed. RTK runs inside uv run and filters output normally. If #176 lands first, this PR's approach can be superseded.

Changes

File Change
src/discover/registry.rs Add strip_uv_run_prefix() in classify_command; add uv_prefix preservation in rewrite_segment format strings

Design decisions

  • Cow return type on strip_uv_run_prefix avoids allocation when no prefix is present (majority of commands)
  • Preserving uv run in output ensures the virtualenv is active when RTK invokes the inner tool. Verified: uv run rtk --version works since RTK is in global PATH
  • No new command handler — RTK runs inside uv run and captures/filters output normally

Test plan

  • cargo test uv_run — 6 new tests pass (classify + rewrite for pytest, ruff, mypy, python -m pytest)
  • cargo test — 1259 passed, 3 ignored, 0 failed
  • Existing uv pip list rewrite unchanged (still rtk pip list)
  • uv run rtk --version confirms RTK is accessible inside uv run

Closes #294 (partial — covers uv run prefix stripping)

Handle `uv run` as a transparent prefix (like `sudo` or `env`) in both
`classify_command` and `rewrite_segment`. The inner command is matched
against existing rules, and `uv run` is preserved in the rewritten
output so the virtualenv stays active:

  uv run pytest tests/ -v     → uv run rtk pytest tests/ -v
  uv run ruff check .         → uv run rtk ruff check .
  uv run python -m pytest     → uv run rtk pytest tests/

This is the #1 unhandled command in `rtk discover` (1,062 occurrences
over 30 days). The approach is complementary to #176 which adds a
first-class `rtk uv` command — this PR requires no new command handler
since rtk runs inside `uv run` and filters output normally.
@CLAassistant

CLAassistant commented Apr 5, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@pszymkowiak pszymkowiak left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lgbarn — good direction, uv run as transparent prefix is the right idea. A few things to fix before merge.

Blocker: retarget to develop, not master. All feature work goes through develop first.

P0 — 3 bugs:

1. uv run --frozen pytest tests/ misclassified as Unsupported
strip_uv_run_prefix only strips the literal "uv run ". Real-world usage often has uv flags (--frozen, --no-dev, --with pkg, --python 3.12). After stripping, --frozen pytest tests/ doesn't match any rule. Guard with:

// Bail if next token is a uv flag, not an inner command
Some(rest) if !rest.starts_with('-') => Cow::Borrowed(rest),

2. uv run alone (no inner command)
No test, fragile path through empty string. Add a test:

assert_eq!(rewrite_command("uv run", &[]), None);

3. PYTHONPATH=. uv run pytest tests/ not rewritten
uv run strip runs before ENV_PREFIX strip. cmd_part starts with PYTHONPATH=, so "uv run " prefix doesn't match. Fix: strip env prefix first, then detect uv run.

P1 — design consideration:
The existing rewrite_prefixes array in rules.rs already handles multi-word prefixes ("python -m pytest"rtk pytest). Adding "uv run pytest", "uv run ruff check", "uv run mypy" to each rule's prefixes would achieve the same result with zero pipeline changes. If you prefer the general approach, consider a data-driven transparent_prefixes field in RewriteRule instead of ad-hoc functions.

Missing tests: uv run --frozen pytest, PYTHONPATH=. uv run pytest, uv run python -m pytest, RTK_DISABLED=1 uv run pytest, uv run pytest 2>&1.

Fix the P0s and retarget to develop, and this is ready.

Address PR #1044 review feedback:

P0-1: strip_uv_run_prefix now skips uv-specific flags (--frozen,
--with pkg, --python ver, etc.) to find the inner command. Previously
"uv run --frozen pytest" was misclassified as Unsupported.

P0-2: Add test for "uv run" alone (no inner command) → returns None.

P0-3: Add test for env prefix + uv run ("PYTHONPATH=. uv run pytest")
confirming correct rewrite ordering.

New helper split_uv_run() extracts (prefix_with_flags, inner_command)
and is shared by both classify_command and rewrite_segment paths.

Added tests: --frozen, --with <pkg>, env prefix, RTK_DISABLED, redirect.
@lgbarn
lgbarn changed the base branch from master to develop April 7, 2026 17:20

@lgbarn lgbarn left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed review @pszymkowiak — all 3 P0s are addressed in the second commit (d08a3c4):

P0-1 ✓split_uv_run() now skips uv flags (--frozen, --with pkg, -p 3.12, etc.) before finding the inner command. Tests: test_classify_uv_run_frozen_pytest, test_rewrite_uv_run_frozen_pytest, test_rewrite_uv_run_with_pkg_pytest.

P0-2 ✓split_uv_run() returns None when no inner command follows the flags. Test: test_rewrite_uv_run_alone.

P0-3 ✓rewrite_segment now strips env prefix before detecting uv run, so PYTHONPATH=. uv run pytest rewrites correctly. Test: test_rewrite_env_prefix_uv_run_pytest.

Retarget ✓ — Base branch is develop.

On the P1 (data-driven transparent_prefixes): I considered adding it to RewriteRule, but uv run is the only transparent prefix today, and generalizing it introduces complexity without a second use case to validate the design. The bespoke split_uv_run is ~40 lines, well-tested, and easy to rip out if #176 lands a first-class rtk uv handler. If another transparent prefix shows up (e.g., poetry run, pipenv run), happy to refactor into a data-driven field at that point.

Ready for re-review when you get a chance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hook rewrite: add coverage for uv run, pnpm exec, Python path variants, and more

3 participants