Skip to content

fix: disable broken clawloop run/eval with truthful redirect#49

Merged
bordeauxred merged 1 commit into
mainfrom
fix/truthful-cli-run
Apr 19, 2026
Merged

fix: disable broken clawloop run/eval with truthful redirect#49
bordeauxred merged 1 commit into
mainfrom
fix/truthful-cli-run

Conversation

@bordeauxred
Copy link
Copy Markdown
Contributor

Summary

  • clawloop run and clawloop eval only wired 2 of 5+ env adapters and failed with Unknown benchmark on benches the README implies. Per issue Make public CLI and example configs truthful #30, this is a launch-credibility blocker.
  • Disable both subcommands: print a truthful redirect to examples/train_runner.py (the unified TrainConfig runner that covers every supported env) and uv run clawloop demo math --dry-run (the no-key hero demo). Exit code 2.
  • Intercept before argparse so legacy muscle memory (clawloop run --bench entropic ...) gets the redirect instead of an opaque "unrecognized arguments" error.
  • Gut ~150 LoC of dead helpers (ADAPTER_REGISTRY, _get_adapter, _load_config, _build_evolver, _ensure_output_dir); they had no callers outside cmd_run/cmd_eval.

Post-launch fast-follow: reintroduce clawloop run as a thin wrapper: clawloop run config.jsontrain(TrainConfig(**json)). Tracked in a separate issue.

Closes #30.

Test plan

  • pytest tests/test_cli.py — 6 passes: run no-args / with legacy flags / with -v / with --help, eval no-args, demo math --dry-run still works.
  • Full suite: +4 passes vs main, 0 new failures, 0 new errors. Pre-existing failures are unrelated (server fixtures, reflector JSON parsing).
  • Codex reviewed twice: NEEDS WORK → SHIP after tightening the redirect message and adding -v run / run --help regression tests.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request disables the legacy run and eval subcommands in the CLI, redirecting users to the unified TrainConfig runner and removing associated legacy logic. It introduces an early interception mechanism in the main function to provide helpful error messages for these disabled commands, supported by new smoke tests. Feedback was provided regarding the fragility of the subcommand detection logic, which may fail if global flags with arguments are added in the future.

Comment thread clawloop/cli.py Outdated
# so stale muscle memory (`clawloop run --bench entropic ...`) gets the
# redirect instead of an opaque "unrecognized arguments" error.
argv = sys.argv[1:]
leading = next((a for a in argv if not a.startswith("-")), None)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for detecting the subcommand by finding the first non-flag argument is a bit fragile. While it works for the current CLI structure where all global flags are boolean (like -v), it would fail if a global flag that takes an argument (e.g., --config file.json) were added in the future. In that case, leading would incorrectly capture the flag's argument instead of the subcommand. Consider using a more robust approach or adding a comment to warn future maintainers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in e926eac. Replaced the argv-scan with parser.parse_known_args() + disabled subparsers declared with add_help=False. Legacy flags (including a hypothetical future value-taking global, or run --config foo.json) now land in unknown and are ignored; the redirect runs based on args.command, which argparse resolves correctly regardless of what globals are defined.

Added test_run_redirect_ignores_unknown_subparser_flags_with_values as a regression guard for the exact class of failure you flagged.

@bordeauxred bordeauxred force-pushed the fix/truthful-cli-run branch from 6b54c26 to 8e25e15 Compare April 19, 2026 21:59
@bordeauxred bordeauxred force-pushed the fix/truthful-cli-run branch from 8e25e15 to e926eac Compare April 19, 2026 22:17
@bordeauxred bordeauxred merged commit 424ad79 into main Apr 19, 2026
5 checks passed
@bordeauxred bordeauxred deleted the fix/truthful-cli-run branch April 19, 2026 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make public CLI and example configs truthful

1 participant