v0.7.0: cleanup + perf (--tool both removal, probe_hardware cache, pytest-xdist)#129
Merged
Conversation
…spec §7) Single source of truth: CODING_TOOLS tuple lives in intake.py; cli.py re-exports as a list for argparse choices= and derives INSTALLABLE_TOOLS from it; VALID_TOOLS in intake.py becomes frozenset(CODING_TOOLS). The drift-detection test is replaced with a derivation invariant test.
Honors the v0.6.0 deprecation promise. Removed: `both` from CODING_TOOLS / INSTALLABLE_TOOLS / VALID_TOOLS, _BOTH_EXPANSION, _BOTH_WARNING_FIRED, reset_deprecation_state, the both-expansion branch in normalize_tools, _normalize_persisted_intake helper, IntakeAnswers.agent property. Programmatic callers passing "both" now hit a CliError naming the replacement. CLI users hit a CliError (rc=1) from normalize_tools since setup run --tool uses action=append (no choices=). Legacy project.json files with singular `tool` are silently ignored — re-run setup to regenerate.
…tring Spec review caught: --tool on setup run uses action=append without choices=, so argparse doesn't reject 'both'. The CliError from normalize_tools is the actual rejection path. Update both text strings to reflect reality.
…gial loop Important from review: - installers.py:50 still branched on 'both' to expand to opencode+openclaude. Dead code post-removal; the caller always passes a canonical single tool name. Simplified to direct '[_plan_for(selection)]' with the manual check. - cli.py:2219 still treated 'both' as an opencode-adapter trigger for knowledge setup. Same dead branch; cleaned up. Minor from review: - normalize_tools had a vestigial 'expanded' list that was always equal to 'flat' after the 'both' branch became an early raise. Collapsed to a single 'if "both" in flat: raise' + dedupe directly on flat.
Real user-visible perf win: doctor/pilot warm calls 243ms → ~78ms (3x+). Cache at $XDG_CACHE_HOME/coding-scaffold/hardware.json keyed on OS/arch/Python so it self-invalidates when any of those change. TTL = 1h balances staleness (new ollama install) against re-probe cost. --no-probe-cache flag on doctor / pilot / probe forces a fresh probe. Uses asdict() for cache serialization so HardwareProfile field names are preserved and reconstruction via HardwareProfile(**profile) works cleanly. Test uses sys.executable so the correct venv Python is used in subprocess.
- hardware.py: inline comment in _write_cache explaining asdict vs to_dict asymmetry (prevents future 'fix me' regressions that would silently poison the cache via os_name → os field rename). - test_hardware_cache.py: replace subprocess-based perf gate with an in-process measurement that counts probes. Removes subprocess-startup noise and pytest-xdist scheduling jitter (Task 4 is about to enable parallel runs). Asserts the cache invariant (1 cold probe + N warm hits) + a sanity check that warm < cold. Wall-clock numbers go in the PR description, not in CI. - test_hardware_cache.py: drop spurious 'arch' key from wrong-key fixture (HardwareProfile has no arch field; the architecture portion lives in _cache_key, not the profile dict). Comment explains. - test_hardware_cache.py: switch test_expired_cache_re_probes from manual try/finally to monkeypatch.setattr for pattern consistency.
Dev-loop improvement: 635 tests sequential ~22s → parallel ~3s with -n auto. The perf gate test in test_hardware_cache.py is in-process (no subprocess timing) so it's stable under worker scheduling, and the autouse XDG_CACHE_HOME fixture in that file gives each worker its own tmp_path-backed cache. Parallel-safety audit confirmed no other module writes to fixed external paths during tests.
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
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
Implements v0.7.0 spec. Four bundles, each behind a separate commit:
CODING_TOOLS/VALID_TOOLS— single source of truth inintake.py.--tool both+ back-fill helpers — honors v0.6.0 deprecation promise.probe_hardware()— real user-visible perf win.Measured perf delta
doctorpilotBreaking changes
--tool bothremoved. Replace with--tool opencode,openclaude. CLI prints the three-line error block on attempted use.project.jsonfiles with singulartool/agentkeys (written by 0.5.x and never updated through 0.6.x) silently fall back toDEFAULT_TOOLS; re-runsetup runto regenerate.IntakeAnswers.agentproperty removed. UseIntakeAnswers.tools[0].See Upgrading wiki for the full migration story.
What's the cache?
A small JSON file at
$XDG_CACHE_HOME/coding-scaffold/hardware.json(default~/.cache/coding-scaffold/hardware.json). 1-hour TTL, keyed on OS / arch / Python version (self-invalidates on any change). Corrupt / expired / unwritable cases all degrade to re-probe.New
--no-probe-cacheflag ondoctor/pilot/probeforces a fresh probe — handy right after installing a new local runtime likeollama.Test plan
uv run pytest -q— 637 tests pass in ~3s with pytest-xdist.uv run ruff check src/ tests/— clean.coding-scaffold setup run --tool both --target /tmp --non-interactive→ exits 1 with the three-line error.doctor(243ms) → warmdoctor(78ms) confirmed via subprocess timing.--no-probe-cachere-probes (verified via cache-mtime).Execution notes
"both"guards ininstallers.pyandcli.pyoutside the original Task 2 scope; fixed in the same task.