feat(lucebox): autotune sweep + profile/smoke diagnostics (stacked on #335)#5
Open
easel wants to merge 10 commits into
Open
feat(lucebox): autotune sweep + profile/smoke diagnostics (stacked on #335)#5easel wants to merge 10 commits into
easel wants to merge 10 commits into
Conversation
… adapters New lucebox/ Python package exposing the hub CLI (autotune, sweep, profile, smoke, models, config, download, host-check, docker_run) plus the lucebox.sh launcher wrapper and install.sh. Adds the harness/ adapter package wrapping external coding agents (claude_code, codex, hermes, openclaw, opencode, pi) that autotune sweeps drive. Ships scripts/check_lucebox_wrapper_sandbox.sh and scripts/test_lucebox_sh.sh for wrapper validation, full pytest coverage under lucebox/tests/, and the bragi autotune profile-sweep protocol docs. This is the user-facing surface of lucebox-hub: one CLI to launch the image, tune layer-split / pflash settings against a host, run sweeps, and dispatch bench runs. Splitting it out keeps Python-side review independent of the C++ server and Docker stack reviews. - Luce-Org#334 (docker-stack): docker_run.py launches the lucebox-hub image - Luce-Org#337 (lucebench-harness): lucebox bench delegates to luce-bench (workspace dep) - Luce-Org#336 (server-layer-split): autotune presumes layer-split build artifacts
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…to-auto-pick entrypoint.sh:write_host_info() bailed loudly when /opt/lucebox-hub/ did not exist on the host (unit tests, plain docker run without bind mount), because bash refuses the > redirect before the command runs and 2>/dev/null does not suppress the redirect's own error. Guard with an upfront [ -d ] check. test_lucebox_sh.sh:test_entrypoint_multi_target was asserting against the pre-Luce-Org#334 multi-target semantics (auto-pick + warn + exec shim). PR Luce-Org#334 (merged) changed that to refuse-to-auto-pick + exit non-zero. Update the assertion: still drives the auto-detect block (so any DRAFT_FAMILY_GLOB set -u regression trips), but now requires the refuse warn to fire and the shim to NOT exec. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds the missing rc != 0 assertion to test_entrypoint_multi_target. The previous fix updated the test to look for the "Refusing to auto-select" warn and a non-exec'd shim, but didn't check the exit code. A regression where the entrypoint logged the warn but still exited 0 (silently auto-picking under the covers) would have slipped through. The container MUST fail to start on multi-target ambiguity — that is the whole point of the policy added in Luce-Org#334. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…e CI Behavior-preserving cleanup of the lucebox hub CLI surfaced by review: - cli.py: delete dead _pick_variant_from_driver; collapse the six near-identical Typer client subcommands into a factory loop. - harness adapters: rewire claude_code onto _common; add build_base_parser + run_main so the six main() arg-parsers stop duplicating (per-client differences preserved); hoist KNOWN_AREAS in bench.py. - Unify DFLASH_ALLOWLIST in types.py (was duplicated with a silent one-field divergence between cli.py and sweep.py). - config.py: extract _atomic_write_doc. - Shell: shared _append_host_env / _append_scalar_env / _set_tty_flags helpers in lucebox.sh, _trim in entrypoint.sh, sandbox factory in the test runner. - FIX a tty regression introduced while sharing the docker interactive flags: the helper ran behind `< <(...)`, so `[ -t 1 ]` tested the pipe and forced -i even on a real tty (breaking interactive client TUIs). Now runs in caller scope via nameref; added a PTY-based regression test. - Add unit tests for the previously-untested harness adapters + bench, and wire lucebox + harness pytest into CI (it previously never ran pytest). 116 -> 156 tests; bash wrapper 54 -> 55; ruff + mypy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oad core Defer second-order features to follow-up PRs so this PR is just the host wrapper and the CLI needed to install, launch/serve, configure, and download models. Cuts the PR from ~11.2k to ~6.2k added lines. Deferred out of this PR (land as stacked follow-ups): - Agent-client adapters + bench: the entire net-new harness/ package (bench.py, the six clients, run_lucebench.sh) and the client launcher verbs. harness/ reverts to main's loose-scripts state; the root pyproject no longer adds harness as a workspace member/dep. - Autotune sweep + profiles: candidate_configs, the Profile registry and per-arch brackets, sweep.py, and the `autotune` command. The host-derived DFLASH_* heuristic (runtime_from_host) STAYS — config.live_config needs it to bake serve defaults — slimmed into autotune.py. - profile + smoke commands and their modules. Coupling fixes: - recommend_preset moved autotune.py -> download.py (models sub-app uses it). - Wrapper: drop the autotune --sweep exec-routing special case and trim usage/completion/exec-set to the core verbs. Tests/CI follow the surface: deferred-feature tests removed; runtime_from_host heuristic tests kept; new guard asserts the deferred verbs are NOT registered. CI pytest step scoped to lucebox. lucebox 72 passed, wrapper 53 passed, ruff + mypy + shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…utotune) The exhaustive VRAM-tier test for recommend_preset lived in the deferred test_autotune_cli.py; since the function moved to download.py in the split, its direct test belongs alongside it. The models auto-recommend path is also covered indirectly by test_models_cli. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39087d8adb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| local cmd="$1"; shift | ||
| case "$cmd" in | ||
| config|models|check|print-run|print-serve-argv) | ||
| config|smoke|models|check|profile|print-run|print-serve-argv) |
There was a problem hiding this comment.
When the server container is already running, this routes lucebox profile through cmd_exec_in_container. But profile.run_profile immediately shells out to docker inspect/docker exec to run luce-bench in the server container, while the long-running server container is started without the Docker socket mounted. In the normal “server running” case the nested profile command therefore cannot see Docker and exits before taking a snapshot; route profile through the docker-run orchestrator like autotune --sweep.
Useful? React with 👍 / 👎.
| if rc is not None: | ||
| return rc | ||
|
|
||
| cfg = config_mod.load() or config_mod.live_config() |
There was a problem hiding this comment.
When config.toml exists, this raw load ignores the LUCEBOX_PORT, LUCEBOX_CONTAINER, and LUCEBOX_MODELS values that the wrapper exports and the systemd unit uses. With a non-default port/container, the sweep restarts the right service but then waits on cfg.port or profiles cfg.container_name from the stale/default file values, causing every cell to fail readiness or target the wrong container. Use the same env overlay path as the CLI before building candidates and probing the server.
Useful? React with 👍 / 👎.
…eftovers The serve-argv builder (docker_run.py) — the core's whole job — was only 50% covered by one test. Add lucebox/tests/test_docker_run.py pinning the argv/printable contract, every DFLASH_* env (always-on + conditional), target/draft path resolution, host-env forwarding, volume mounts, and docker_pull. docker_run.py 50% -> 96%. Includes a regression guard for the preset-cap analysis: a large preset serves at the conservative DflashRuntime() floor (max_ctx=16384), not a high OOM-prone ctx — the VRAM-tier heuristic's higher caps only apply via `autotune --apply` (which threads cfg.model.preset; separate PR). No bug. Also: - Add lucebox/**/*.py to the ruff include set (was mypy-checked but never ruff-linted) and clear the 3 unused imports it surfaced in cli.py. - Drop an orphan `# autotune` section divider and refresh two docstrings that referenced the deferred autotune subcommand. lucebox 89 passed, wrapper 53 passed, ruff + mypy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
easel
force-pushed
the
feat/lucebox-tuning
branch
from
39087d8 to
9f63eea
Compare
Core carried three bits only the deferred follow-ups use. Evict them so the core CLI is just the basic launch/config/download surface: - types.BASE_DFLASH_ALLOWLIST — no core caller; used by the sweep's per-cell bracket. Moves to the tuning PR. - autotune.runtime_from_host preset-awareness (_preset_approx_gb + the ≥20 GB large-model 32K branch) — core always calls runtime_from_host(host) with no preset, so this was unreachable here. The preset-size cap moves to the tuning PR alongside autotune --apply (which threads cfg.model.preset). Behavior for the core's preset-less call is unchanged. - config.live_config(preset_name=...) — dead across the entire tree (every call site passes no arg). Deleted outright, not moved. Their tests move with them. lucebox 87 passed, wrapper 53, ruff + mypy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Stacked on the core lucebox CLI (Luce-Org#335). Restores the second-order tuning and diagnostics surface deferred from that PR: - autotune.py: the empirical sweep machinery — candidate_configs, the Profile registry + per-arch coding-agent-loop brackets, get_profile. (recommend_preset stays in download.py from the core split.) - sweep.py: the per-cell config sweep driver (config set -> restart -> luce-bench snapshot -> winner pick). - profile.py / smoke.py + the `autotune`, `profile`, `smoke` CLI commands. - Wrapper: the `autotune --sweep` exec-routing special case (sweeps must stay on docker run, not exec into the container they'd restart) and the autotune/profile/smoke entries in usage + completion + the exec set. Tests: sweep / profile / smoke / autotune-cli / candidate-configs suites and the Profile/bracket tests restored; test_cli guards that the client launcher verbs are still deferred while autotune/profile/smoke register. lucebox 122 passed, wrapper 55 passed, ruff + mypy + shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
easel
force-pushed
the
feat/lucebox-tuning
branch
from
9f63eea to
a9a1f30
Compare
easel
force-pushed
the
feat/lucebox-cli
branch
2 times, most recently
from
0ae47c5 to
573570a
Compare
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
1 participant
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.
Stacked on Luce-Org#335 (core lucebox CLI). Base is `feat/lucebox-cli`, so this PR's diff is only the tuning/diagnostics cluster. Review/merge after Luce-Org#335.
Summary
Restores the second-order tuning + diagnostics surface deferred from Luce-Org#335:
Tests
sweep / profile / smoke / autotune-cli / candidate-configs suites + the Profile/bracket tests restored. `test_cli` guards that the client launcher verbs stay deferred while autotune/profile/smoke register.
lucebox 122 passed (123 with the inherited recommend_preset test), wrapper 55 passed, ruff + mypy + shellcheck clean.
🤖 Generated with Claude Code