Install picker: skip pip when the selection is unchanged#71
Merged
Conversation
Re-running start.sh/.ps1 always invoked `pip install -e ".[…]"`, which uninstalls, rebuilds, and reinstalls the editable project package on every launch even when nothing changed — noisy and slow for no benefit. Stamp each successful install (resolved extras + a pyproject.toml fingerprint) inside the venv and skip pip on the next run when neither changed. The stamp lives under sys.prefix so recreating .venv forces a fresh install, and a dependency bump in pyproject.toml (e.g. after a git pull) re-triggers it even when the family/backend selection is identical. Editable installs already pick up source edits with no reinstall, so this only suppresses genuine no-ops. https://claude.ai/code/session_017UhPmXQq4nUxsbB4gMLdiV
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
A current install stamp can survive the package it describes — the operator ran `pip uninstall tapscribe`, or recreated the venv with a tool other than start.sh. Skipping pip on the strength of the stamp alone would then leave a broken or missing install with no pip run, a regression versus the old always-install behaviour. Gate the skip on package_is_installed() (an importlib find_spec probe; the picker runs from tools/ with repo root off sys.path, so it only succeeds when an install actually placed TapScribe on the path). https://claude.ai/code/session_017UhPmXQq4nUxsbB4gMLdiV
Replace the shallow main()-level checks (which monkeypatched the fingerprint and the install-presence probe to constants) with tests that exercise the real machinery: - pyproject_fingerprint against a real temp pyproject.toml: SHA-256 match, stability, change-on-edit, and the missing-file sentinel. - package_is_installed against a real importable vs absent module. - An end-to-end main() run where an actual pyproject.toml edit (no fingerprint mock) forces a reinstall and the fresh stamp re-stabilises. - The stamp written by an install equals resolve_extras + the real fingerprint and round-trips through install_is_current. - The skip path actually prints "skipping pip" (operator-visible). - STAMP_FILE location is pinned under sys.prefix so a refactor can't move it to a venv-surviving path and re-introduce the stale-stamp bug. https://claude.ai/code/session_017UhPmXQq4nUxsbB4gMLdiV
The unit suite mocks run_install, so it never exercises a real pip install, a real on-disk stamp, and the real find_spec presence probe together. Add a `real_pip`-marked e2e test that stands up a throwaway venv + trivial dependency-free package and drives the real install_picker.main in a fresh subprocess per run (mirroring repeated start.sh launches), asserting pip-ran vs pip-skipped across: clean install, unchanged re-run (skip), pyproject edit (reinstall), unchanged again (skip), and package-removed-but-stamp-current (reinstall via the find_spec guard). Self-skips when a trivial editable build isn't possible (no network for the build backend) so it can't flake CI. https://claude.ai/code/session_017UhPmXQq4nUxsbB4gMLdiV
CodeQL's py/unnecessary-lambda rule flagged the
monkeypatch.setattr(install_picker, "detect_caps",
lambda *, force_no_mlx=False: _caps())
stub I added in every main()-level skip-install test, plus the
constant-return lambdas for `package_is_installed` and the pip-failure
stub. Replace all of them with named module-level helpers
(_detect_caps_cpu, _package_present, _package_missing,
_pip_install_fails) — they have to keep the real signature anyway,
so a `def` is the right shape.
No behaviour change; full picker suite still passes.
https://claude.ai/code/session_017UhPmXQq4nUxsbB4gMLdiV
starlette 1.x emits StarletteDeprecationWarning at `starlette.testclient` import time, nagging that `httpx` should be replaced by `httpx2`. fastapi's TestClient pulls that import in, so every test module that uses TestClient (test_routes, test_tap_endpoint) fails collection under `filterwarnings = error`. The warning class is starlette's own UserWarning subclass (NOT a DeprecationWarning), so the filter has to name it by FQCN — a `:DeprecationWarning` category match doesn't catch it. Drop once starlette finalises the httpx2 migration. https://claude.ai/code/session_017UhPmXQq4nUxsbB4gMLdiV
Adds .claude/hooks/pre-push.sh (wired in via .claude/settings.json as a PreToolUse hook on Bash) that inspects every Bash command Claude runs. When the command actually invokes `git push`, the hook runs ruff check + the full pytest suite — mirroring ci.yml's `Lint (ruff)` and `Run tests` steps — and exits 2 to block the push if either is red. Everything that isn't a push is passed through with exit 0, so the rest of the session pays no overhead. Bypass with CLAUDE_SKIP_PRE_PUSH=1 for genuine emergencies. This closes the gap from PR #71: the lambda-fix commit there pushed without a full `pytest tests` run; the failure that surfaced was unrelated dep drift, but the discipline of running the same checks CI runs before pushing is the right default. Documented in CLAUDE.md under "Verify before claiming green". Smoke-tested locally: non-git commands pass through silently, the gate fires on bare and chained `git push`, the bypass env var works, and a real run with the gate blocks on a red ruff/pytest and allows on green. https://claude.ai/code/session_017UhPmXQq4nUxsbB4gMLdiV
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Re-running
start.sh/start.ps1always invokedpip install -e ".[…]", and an editable reinstall uninstalls, rebuilds, and reinstalls the project package on every launch even when nothing changed. That's the noisy "Uninstalling tapscribe… / Successfully installed tapscribe…" churn that looks bad on an otherwise no-op re-run.This keeps the per-run picker (you still choose what to install each time) but skips pip entirely when there's nothing to do:
pyproject.toml.sys.prefix), so blowing away.venvnaturally forces a fresh install.pyproject.toml(e.g. aftergit pull) changes the fingerprint and re-triggers the install even when the family/backend selection is byte-for-byte identical.--dry-runstays read-only.Editable installs already reflect source-tree edits without a reinstall, so this only suppresses genuine no-ops. The fix is in the shared
tools/install_picker.py, so both start scripts benefit.Test plan
pytest tests/test_install_picker.py -q(existing + new tests pass)ruff check/ruff format --checkclean on changed filesinstall_is_currentmatching/mismatch, skip on unchanged re-run, reinstall on changed selection, reinstall on changed pyproject fingerprint, no stamp on pip failure, no stamp on--dry-run.https://claude.ai/code/session_017UhPmXQq4nUxsbB4gMLdiV
Generated by Claude Code