Repository navigation
docs(sim): explain backend selection - #596
Conversation
heyong4725
left a comment
There was a problem hiding this comment.
Review: docs-only, two sentence fixes before merge
Checked the guide's claims against the code. These hold:
--sim-engineis on rollout, fleet, monolith run, fault calibrate, skill register (src/aisle/harness/cli.py:94,163,181,317,376).- Installer flags
--feature,--nexus,--rapier,--determinismexist; Nexus defaults tometalon macOS,webgpuelsewhere. AISLE_SIM_BACKENDaccepts metal/webgpu/cuda/cpu for Nexus (src/aisle/sim/__init__.py:37).- A graph that declares its engine refuses a conflicting
--sim-engine(src/aisle/harness/rollout.py:1393); the bridge readsAISLE_SIM_ENGINE, so the directdora runadvice is right. - A missing, malformed, or dirty receipt stops the run before launch (
tools/env_hash.py:352,590->rollout.py:670).
1. uv run does not remove the optional wheels
docs/simulation-backends.md, "Keep optional wheels installed": "Plain uv sync, and uv run without --no-sync, ... remove the out-of-lock Nexus and Rapier wheels."
Tested with uv 0.11.29 in a scratch project: a package installed outside the lock survived uv run --locked; only uv sync removed it. uv run syncs inexactly by default (it adds/changes what the lock needs, never removes extras).
--no-sync is still a sensible recommendation (it also avoids reverting any locked dependency the wheel install changed), but the stated reason is wrong. Suggest: "uv sync removes the out-of-lock wheels; use uv run --no-sync ... so no sync step runs." docs/getting-started.md:244 carries the same claim from before this PR; worth fixing in the same edit.
2. README "install once after uv sync" is ambiguous
The README quickstart just above warns that plain uv sync REMOVES the sim extras. A reader who runs plain uv sync here loses them, and every following --no-sync command keeps the env that way. Suggest "after uv sync --extra sim --locked", matching docs/simulation-backends.md.
Minor
The README hero now links the guide instead of ADR-67/68 directly; the guide links both, so nothing is lost.
Verdict: fix 1 and 2, then merge.
🤖 Generated with Claude Code
|
Addressed both requested fixes in c9c4b5c:
Rechecked |
Summary
uv run --no-syncVerification
uv run ruff format --check .uv run ruff check .uv run python tools/trace_check.pyuv run python tools/docs_inventory.py --checkuv run python tools/claim_evidence.py --checkuv run pytest -q tests/unit/test_docs_inventory.py tests/unit/test_harness_sim_engine.py tests/unit/test_research_contract.py tests/unit/test_conformance_research_contract.py(65 passed)The full local
pytest -m unitgate reached 780 passing tests before reproducing the existing macOS dynamic monolithic infrastructure classification failure intests/unit/test_dynamic_monolithic_journal.py. The changed files are documentation only; CI will run the complete platform matrix.Related: ADR-67, ADR-68