fix: invoke Python scripts with uv run across skills, samples, and docs - #105
fix: invoke Python scripts with uv run across skills, samples, and docs#105bmadcode wants to merge 2 commits into
Conversation
PR #98 standardized on `uv run` for bmad-agent-builder and bmad-workflow-builder, but bmad-bmb-setup, bmad-module-builder, and bmad-eval-runner were missed. This finishes it: 57 invocations across skills/, samples/, and docs/how-to/. Why it matters beyond consistency: a bare `python3` is not guaranteed to be 3.11+, which BMad's shared resolve_customization.py requires for tomllib. `uv run` reads a script's own requires-python and provisions a matching interpreter. Five of the fixes are in assets/setup-skill-template/ and assets/standalone-module-template/ — the templates bmad-module-builder emits — so every module scaffolded from bmb stops inheriting the defect. pytest lines become `uv run --with pytest -m pytest`, matching the convention in bmad-method: those test files carry no PEP 723 block, so uv has nothing else to resolve pytest from. Left deliberately: the two anti-pattern cells in docs/explanation/ (labelled Fragile and "do not modify"), which teach what not to do; the "runs under a bare python3" prose, which describes a script's lack of dependencies rather than invoking anything; and the per-script requires-python floors, which are correct as written. Also corrects the README badge, which advertised >=3.10 — a floor that cannot run the shared resolver.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughThe README now requires Python 3.11. Documentation, sample scripts, skill instructions, and test usage examples now invoke Python tooling through ChangesPython command standardization
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Documents the uv run conversion in this branch, and notes plainly that the 2.1.0 entry's claim about #98 covered only two of the five skills. All three marketplace plugins go 2.1.0 -> 2.2.0.
|
Pushed the v2.2.0 changelog entry and marketplace version bump onto this branch rather than opening a second PR, since the entry documents the work that lives here — splitting them would have left a changelog describing unmerged code. Changelog — a Version bump — all three marketplace plugins Worth copying from bmad-method-test-architecture-enterprise: it has a markdownlint and prettier pass. |
1 participant
#98 standardized on
uv runforbmad-agent-builderandbmad-workflow-builder, butbmad-bmb-setup,bmad-module-builder, andbmad-eval-runnerwere missed. This finishes it — 57 invocations acrossskills/,samples/, anddocs/how-to/.Why it matters beyond consistency: a bare
python3is not guaranteed to be 3.11+, which BMad's sharedresolve_customization.pyrequires fortomllib.uv runreads a script's ownrequires-pythonand provisions a matching interpreter.Breakdown
skills/invocationssamples/.mdinvocations.pyusage strings and docstrings>=3.10→>=3.11Five of the
skills/fixes are in emitted templates —assets/setup-skill-template/andassets/standalone-module-template/— so every module scaffolded bybmad-module-builderstops inheriting the defect.There was a visible seam before this:
init-sanctum-template.pyhad been converted, but the threesamples/*/scripts/init-sanctum.pycopies generated from it still saidpython3. They now agree.pytest lines
python3 -m pytest Xbecomesuv run --with pytest -m pytest X, matching the convention in bmad-method. Those test files carry no PEP 723 block, so uv has nothing else to resolve pytest from. Verified rather than assumed:Deliberately left alone
docs/explanation/—scripts-in-skills.md:110(labelled Fragile) andskill-authoring-best-practices.md:22("do not modify"). They teach what not to do; converting them would invert the lesson.count_tokens.pyandprepass.py— describes a script's lack of third-party dependencies, invokes nothing, still true.requires-pythonfloors (>=3.9/>=3.10) — correct as written. None of these scripts importtomllib; the 3.11 requirement is inherited from bmad-method's shared resolver, not native here.Verification
Every touched
.pyparses (AST check on all 28 files). A converted usage string was smoke-tested end to end —uv run samples/bmad-excalidraw/scripts/validate_excalidraw.pyprints its argparse usage, so the line it now advertises is the line that works. markdownlint and prettier pass.Note for the next release entry
CHANGELOG.md:13claims #98 standardizeduv runacross builder scripts. That was not true when written — it covered two of five skills. I have not rewritten it, since editing shipped release history to hide an inaccuracy is worse than the inaccuracy. As of this branch the statement is finally true; worth a line in the next entry saying the conversion actually completed here.Part of an ecosystem-wide pass — the same fix is going into bmad-method, creative-intelligence-suite, game-dev-studio, and test-architecture-enterprise.
Summary by CodeRabbit
uv.