Skip to content

fix(bootstrap): always try only-system fallback (drop too-strict gate, refs #130) - #142

Merged
debpalash merged 1 commit into
mainfrom
fix/bootstrap-only-system-fallback
May 29, 2026
Merged

fix(bootstrap): always try only-system fallback (drop too-strict gate, refs #130)#142
debpalash merged 1 commit into
mainfrom
fix/bootstrap-only-system-fallback

Conversation

@debpalash

@debpalash debpalash commented May 29, 2026

Copy link
Copy Markdown
Owner

Follow-up from verifying #140 by driving the real uv binary.

Finding: system_python_ge_311() was stricter than uv's own interpreter discovery — it probed only python3/python. On this Mac, python3 is the system 3.9.6 (no python), so the gate returned false and the only-system fallback was skipped — even though UV_PYTHON_PREFERENCE=only-system uv venv immediately resolves the Homebrew CPython 3.14.5. So in the exact worst case the fallback exists for (GitHub and mirror blocked), a user with a perfectly good system Python could be sent to the remediation screen.

Fix: drop the pre-gate (and the now-unused parse_py_version/system_python_ge_311 helpers + the parse test); always add the system-python attempt as the last cascade step. uv's discovery is the authority — with requires-python = ">=3.11" it resolves any compatible system interpreter (3.12/3.13/3.14…) or fails fast → remediation.

Verified live (real uv 0.11.14): only-system uv venv built a venv from system CPython 3.14.5 on a host with no 3.11.x. cargo test + cargo check clean.

Refs #130.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Improved bootstrap virtual environment creation with more reliable system Python fallback strategy. The setup process now consistently attempts system Python as a final option, leveraging built-in dependency version constraints for compatibility validation instead of pre-checks.

Review Change Stack

…gate

Verification of #140 (driving real uv) found system_python_ge_311() was
stricter than uv's own interpreter discovery: it probed only `python3`/`python`,
so on a machine where `python3` is the macOS 3.9 but a Homebrew 3.14 exists, the
gate returned false and the only-system fallback was skipped — even though
`UV_PYTHON_PREFERENCE=only-system uv venv` resolves 3.14 fine.

Fix: drop the pre-gate (and the now-unused parse_py_version/system_python_ge_311
helpers + the parse test) and always add the system-python attempt as the last
resort. uv's discovery is the authority; with `requires-python = ">=3.11"` it
resolves any compatible system interpreter or fails fast → remediation.

Verified live: `only-system uv venv` created a venv from system CPython 3.14.5
on this host (no 3.11.x present). cargo test + cargo check clean.

Refs #130.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The PR simplifies venv bootstrap by removing pre-flight Python version checking and unconditionally adding a system-python fallback strategy that uses uv venv with system-only interpreter discovery. Version compatibility is delegated to uv and project constraints instead of shell-based pre-checks.

Changes

Bootstrap System Python Fallback Simplification

Layer / File(s) Summary
Version pre-check helper removal
frontend/src-tauri/src/bootstrap.rs
Functions parse_py_version and system_python_ge_311 that performed shell-based pre-flight Python >= 3.11 detection are removed, along with their unit test covering version string parsing.
Unconditional system-python fallback strategy
frontend/src-tauri/src/bootstrap.rs
Bootstrap strategy selection now unconditionally appends a final uv venv attempt configured with UV_PYTHON_PREFERENCE=only-system, delegating version compatibility checks to uv interpreter discovery and the project's requires-python constraints instead of pre-checking on PATH.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description provides a comprehensive explanation of the problem, the fix, and verification, but the template structure with Summary/Changes/Type/Testing/Checklist sections is not followed. Reorganize the description to match the template: add a structured Summary section, list Changes as bullet points, check the appropriate Type checkbox (🐛 Bug fix applies here), document Testing steps, and complete the Checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing a strict gate and always attempting the system-python fallback, which aligns with the changeset that removes helper functions and unconditionally adds the fallback strategy.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/bootstrap-only-system-fallback

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes the system_python_ge_311 pre-gate (and its parse_py_version helper and associated test) from the ensure_venv_ready bootstrap cascade, replacing the conditional push with an unconditional one so the UV_PYTHON_PREFERENCE=only-system fallback is always attempted as the last resort.

  • Drops parse_py_version and system_python_ge_311 helper functions along with the parse_py_version_handles_real_and_garbage unit test — the helpers were the only callers, so no dead code remains.
  • The system-python venv attempt is now always appended to venv_attempts, delegating interpreter compatibility checks to uv's own discovery (governed by requires-python = \">=3.11\" in the project manifest) rather than a hand-rolled python3/python probe that missed Homebrew-managed interpreters.

Confidence Score: 4/5

The change is safe to merge — it strictly expands coverage by removing an overly conservative gate, and the worst-case outcome of the new path is the same remediation screen the gate would have shown.

The logic change is small and well-reasoned: delegating interpreter discovery to uv is clearly more reliable than the hand-rolled python3/python probe. The only leftover is a stale comment fragment referencing the removed condition, which has no runtime impact. The deleted test covered only the deleted helpers, so test coverage of remaining code is unaffected.

Only one file changed. The stale '(only if >= 3.11)' fragment in the step-by-step comment on line 454 is worth cleaning up before merging.

Important Files Changed

Filename Overview
frontend/src-tauri/src/bootstrap.rs Removes system_python_ge_311 pre-gate and parse_py_version helpers; always appends the UV_PYTHON_PREFERENCE=only-system venv attempt as a final fallback. One stale comment still references the old '(only if >= 3.11)' condition.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ensure_venv_ready] --> B[resolve_uv]
    B --> C{uv available?}
    C -- No --> Z[fail → remediation]
    C -- Yes --> D[Attempt 1: uv venv --python 3.11 --managed-python\ndefault GitHub host]
    D -- success --> OK[venv_ok = true → proceed to uv sync]
    D -- fail --> E[Attempt 2: uv venv --python 3.11 --managed-python\nUV_PYTHON_INSTALL_MIRROR=gh-proxy]
    E -- success --> OK
    E -- fail --> F[Attempt 3: uv venv\nUV_PYTHON_PREFERENCE=only-system\nalways added — no pre-gate]
    F -- success --> OK
    F -- fail --> Z
    OK --> G[uv sync --frozen --no-dev]
    G -- success --> H[Return venv_python path]
    G -- fail --> Z
Loading

Comments Outside Diff (1)

  1. frontend/src-tauri/src/bootstrap.rs, line 451-454 (link)

    P2 The parenthetical (only if >= 3.11) is a leftover from the removed system_python_ge_311 gate and now describes behavior that no longer exists — the fallback is always added unconditionally.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

    Fix in Claude Code

Fix All in Claude Code

Reviews (1): Last reviewed commit: "fix(bootstrap): always try only-system f..." | Re-trigger Greptile

@debpalash
debpalash merged commit 285e3d8 into main May 29, 2026
17 of 19 checks passed
@debpalash
debpalash deleted the fix/bootstrap-only-system-fallback branch May 29, 2026 06:10
debpalash added a commit that referenced this pull request May 29, 2026
Bring the branch up to date with main and resolve 5 conflicts as
feature-unions so nothing shipped since #133 was opened regresses:

- useTTS.js: take main's #141 validator-safe instruct (buildDesignInstruct);
  #133 held only the stale pre-#141 dedup logic.
- dub_pipeline.py: UNION — keep #133's download-task cancel cleanup AND
  main's plan-04 logging + structured failure event (build_failure).
- dubSlice.ts / useDubWorkflow.js: UNION — keep both #133's dub
  download-progress state (setDubPrepProgress / setDubCurrentSegId) and
  main's pipeline-error-transparency state (setDubFailure).
- bootstrap.rs: take main's shipped plan-03 network-resilience cascade
  (#140/#142); #133's region-based mirror approach was the superseded
  alternative for the same concern. get_effective_region stays live
  (shared via config.rs, used by tools.rs).

Verified: frontend typecheck + build clean; 90 backend tests pass
(dub / failure / timing / onboarding / personalities), 0 failures.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant