feat: Add unified CLI for run_guidance and fix missing args bug

[Abdelsalam-Abbas]

Summary

• Fixes run_guidance when run with scripts exits non-zero because the protein, model, and guidance_type args are never added #181: run_guidance scripts crash with AttributeError: 'Namespace' object has no attribute 'protein' because protein, model, and guidance_type were never added to the argparse parser. The error occurs in get_job_result() after guidance completes (or fails), making it impossible to get a clean exit from standalone scripts.
• Fixes Add a CLI interface for run_guidance() in guidance_script_utils.py #154: Adds sampleworks-guidance as a unified CLI entrypoint (registered via console_scripts) that replaces the need for 8 separate model+guidance scripts.
• Fixes Document how to use the run_guidance method in guidance_script_utils #153: Documents the new CLI in the README Quick Start section with a reference table of required arguments.

What changed

• Added GuidanceConfig.from_cli() classmethod that handles two-pass argument parsing: first resolves --model and --guidance-type, then builds the full parser with model-specific and guidance-type-specific args dynamically.
• Created src/sampleworks/cli/guidance.py as the unified entrypoint.
• Simplified all 8 standalone scripts to thin wrappers calling from_cli() with pre-set model/guidance_type.
• Removed the 8 parse_*_args() functions (replaced by from_cli()).
• Added --protein as a required argument (must match naming used in grid search/evaluation).
• Registered sampleworks-guidance console_scripts in pyproject.toml.
• Propagate run_guidance exit code in all 8 wrapper scripts via sys.exit(main()) so CI and automation see non-zero exits on failure (previously always exited 0).
• Stop registering --model/--guidance-type on the argparse parser when preset by wrapper scripts, so conflicting CLI input (e.g. rf3_fk_steering.py --model boltz2) is rejected instead of silently ignored.

Test plan

• 22 new tests in tests/cli/test_guidance_cli.py covering unified CLI, legacy script pattern, validation errors, and defaults
• All 678 existing tests pass (164 skipped)
• Manually reproduced run_guidance when run with scripts exits non-zero because the protein, model, and guidance_type args are never added #181 on main (AttributeError: 'Namespace' object has no attribute 'protein', EXIT CODE: 1)
• Verified fix on branch: same script exits cleanly (EXIT CODE: 0) with GuidanceConfig properly populating protein, model, guidance_type
• Run GPU end-to-end test with a valid CCD/checkpoint setup to verify full guidance completion

Summary by CodeRabbit

• New Features

  • Added a unified CLI command for guidance workflows with explicit model, guidance type, protein, and experimental-data inputs; registered as a console entrypoint.

• Refactor

  • Consolidated legacy script entrypoints into a single CLI-based approach with centralized configuration parsing.

• Documentation

  • Updated Quick Start to use the unified command and added a CLI reference with required arguments and discoverable options.

• Tests

  • Added comprehensive CLI parsing tests covering modes, validation, and edge cases.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Replaces many per-model per-strategy script entrypoints with a single unified CLI sampleworks-guidance backed by GuidanceConfig.from_cli(). Adds a console script entrypoint, deletes legacy scripts, updates docs/README/AGENTS, centralizes CLI parsing logic, and adds tests for the new parsing behavior.

Changes

Cohort / File(s)	Summary
Documentation & Packaging README.md, AGENTS.md, pyproject.toml	README and AGENTS updated to document sampleworks-guidance usage and CLI reference; pyproject.toml adds sampleworks-guidance = "sampleworks.cli.guidance:main" entrypoint.
Unified CLI entrypoint src/sampleworks/cli/guidance.py	New `main(argv: list[str]
Centralized argument parsing src/sampleworks/utils/guidance_script_arguments.py	Introduces GuidanceConfig.from_cli(...), _DYNAMIC_ATTRS, dynamic _MODEL_ARG_ADDERS/_GUIDANCE_ARG_ADDERS, two-pass parsing and removal of legacy per-script parse_*_args() functions.
Removed legacy scripts scripts/*_pure_guidance.py, scripts/*_fk_steering.py (e.g., scripts/boltz2_pure_guidance.py, scripts/rf3_fk_steering.py, ...)	Deletes per-model/per-strategy standalone scripts in favor of the unified CLI; removed their main() functions and __main__ invocation logic.
Tests tests/cli/test_guidance_cli.py	New pytest suite validating GuidanceConfig.from_cli() behavior: required args, model/guidance-specific flags, presets, defaults, boolean flags, error cases, and a smoke --help check.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant CLI as "sampleworks-guidance\n(CLI)"
  participant Config as "GuidanceConfig\n.from_cli()"
  participant Model as "get_model_and_device"
  participant Runner as "run_guidance"

  User->>CLI: invoke sampleworks-guidance [--model ... --guidance-type ... + args]
  CLI->>Config: GuidanceConfig.from_cli(argv)
  Config-->>CLI: GuidanceConfig instance
  CLI->>Model: get_model_and_device(config.device, getattr(config,'model_checkpoint',None), config.model, getattr(config,'method',None))
  Model-->>CLI: model_wrapper, device
  CLI->>Runner: run_guidance(config, config.guidance_type, model_wrapper, device)
  Runner-->>CLI: result (exit_code)
  CLI-->>User: process exit (exit_code)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• feat: create Enum classes for guidance constants (closes #49) #62: Modifies CLI/config parsing and guidance argument handling; touches the same guidance parsing utilities.
• Sampleworks refactoring - separate guidance from sampling, and improve ModelWrapper protocols #73: Related refactor centralizing samplers/scalers and guidance argument wiring that aligns with the unified CLI changes.

Suggested reviewers

• marcuscollins
• k-chrispens

Poem

🐰 I hopped through flags and parsed each line,
One carrot CLI now makes the runs align.
Models gathered neat, guidance types in a row,
I nudged the config — then watch the samplers go! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.62% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main changes: adding a unified CLI for run_guidance and fixing a missing arguments bug that was causing crashes.
Linked Issues check	✅ Passed	The PR successfully addresses all three linked issues: #181 (protein/model/guidance_type args now populated), #154 (unified CLI added via GuidanceConfig.from_cli and sampleworks-guidance entrypoint), and #153 (documentation added to README/AGENTS.md).
Out of Scope Changes check	✅ Passed	All changes directly support the stated objectives. The removal of eight legacy scripts, refactoring of argument parsing, addition of the CLI module, and documentation updates are all necessary components of the unified CLI implementation.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch unified-guidance-cli

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (3)

src/sampleworks/cli/guidance.py (1)

11-11: Add a NumPy-style docstring to main().

This new public CLI function is missing the required NumPy-style docstring.

As per coding guidelines "Always include NumPy-style docstrings for every function and class."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/sampleworks/cli/guidance.py` at line 11, The public function main(argv:
list[str] | None = None) lacks a NumPy-style docstring; add one directly above
the def main(...) that briefly describes the function purpose, documents the
parameters (argv: list[str] | None — what it represents), the return value (int
— exit code), and any raised exceptions or side effects (e.g., calls to sys.exit
or logging), following the NumPy docstring sections (Parameters, Returns,
Raises) and using concise, clear prose so automated doc linters will accept it.

scripts/boltz2_fk_steering.py (1)

7-7: Add a NumPy-style docstring to main().

This function is missing the required NumPy-style docstring.

As per coding guidelines "Always include NumPy-style docstrings for every function and class."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/boltz2_fk_steering.py` at line 7, Add a NumPy-style docstring to the
main() function: insert a triple-quoted docstring immediately under the def
main(): that follows NumPy conventions including a short summary line, a blank
line, sections for Parameters (list any args or none), Returns (what main
returns, e.g., None), and Examples if relevant; reference the function name
main() when editing and ensure formatting matches other files (short summary,
expanded description, sections titled "Parameters", "Returns", and optional
"Examples") to satisfy the project's docstring guidelines.

src/sampleworks/utils/guidance_script_arguments.py (1)

173-186: Document from_cli() in the repo’s required docstring format.

This is the new public parsing entrypoint, but its docstring still skips the NumPy-style Parameters / Returns / Raises sections. That makes the CLI contract harder to consume than the rest of the config surface.

As per coding guidelines, **/*.py: "Always include NumPy-style docstrings for every function and class."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/sampleworks/utils/guidance_script_arguments.py` around lines 173 - 186,
The docstring for the classmethod from_cli (returning GuidanceConfig) is missing
NumPy-style sections; update its docstring to include Parameters (describe argv:
list[str]|None, model: str|None, guidance_type: str|None and their behavior),
Returns (describe the GuidanceConfig returned) and Raises (list any exceptions
raised during CLI parsing), keeping the existing summary and noting that
model/guidance_type make CLI flags optional; ensure the sections follow the
repo's NumPy-style formatting conventions so downstream docs and linters
recognize them.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/boltz2_fk_steering.py`:
- Around line 15-19: The wrapper currently calls run_guidance(config, ...) but
ignores the returned JobResult and then unconditionally calls main(), causing
the process to always exit 0; update each wrapper entrypoint (the main()
function and the script-level invocation that calls main()) to capture the
JobResult from run_guidance (e.g., result = run_guidance(...)), return or
sys.exit(result.exit_code) from main, and propagate that exit_code back to the
process so CI sees failures; also add a NumPy-style docstring to each main()
describing parameters and return (including that it returns an int exit code) to
satisfy coding guidelines.

In `@src/sampleworks/utils/guidance_script_arguments.py`:
- Around line 211-236: The parser currently adds --model/--guidance-type with
preset defaults but then ignores any CLI overrides when building the config; fix
by validating CLI values after parsing: in the from_cli function (where parser
is built and args = parser.parse_args(argv) is called), compare args.model and
args.guidance_type to the preset model and guidance_type variables and call
parser.error(...) if they differ (reject conflicting CLI input), or
alternatively use args.model and args.guidance_type when constructing the config
(replace model=model, guidance_type=guidance_type with model=args.model,
guidance_type=args.guidance_type) so CLI values are honored — implement one of
these two approaches in guidance_script_arguments.py around the parser/args and
the cls(...) config construction.

---

Nitpick comments:
In `@scripts/boltz2_fk_steering.py`:
- Line 7: Add a NumPy-style docstring to the main() function: insert a
triple-quoted docstring immediately under the def main(): that follows NumPy
conventions including a short summary line, a blank line, sections for
Parameters (list any args or none), Returns (what main returns, e.g., None), and
Examples if relevant; reference the function name main() when editing and ensure
formatting matches other files (short summary, expanded description, sections
titled "Parameters", "Returns", and optional "Examples") to satisfy the
project's docstring guidelines.

In `@src/sampleworks/cli/guidance.py`:
- Line 11: The public function main(argv: list[str] | None = None) lacks a
NumPy-style docstring; add one directly above the def main(...) that briefly
describes the function purpose, documents the parameters (argv: list[str] | None
— what it represents), the return value (int — exit code), and any raised
exceptions or side effects (e.g., calls to sys.exit or logging), following the
NumPy docstring sections (Parameters, Returns, Raises) and using concise, clear
prose so automated doc linters will accept it.

In `@src/sampleworks/utils/guidance_script_arguments.py`:
- Around line 173-186: The docstring for the classmethod from_cli (returning
GuidanceConfig) is missing NumPy-style sections; update its docstring to include
Parameters (describe argv: list[str]|None, model: str|None, guidance_type:
str|None and their behavior), Returns (describe the GuidanceConfig returned) and
Raises (list any exceptions raised during CLI parsing), keeping the existing
summary and noting that model/guidance_type make CLI flags optional; ensure the
sections follow the repo's NumPy-style formatting conventions so downstream docs
and linters recognize them.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c43adbe7-7184-4d4a-98bb-5c3f4f361106

📥 Commits

Reviewing files that changed from the base of the PR and between 41512b1 and 4fa3e39.

⛔ Files ignored due to path filters (1)

• pixi.lock is excluded by !**/*.lock

📒 Files selected for processing (15)

• README.md
• pyproject.toml
• scripts/boltz1_fk_steering.py
• scripts/boltz1_pure_guidance.py
• scripts/boltz2_fk_steering.py
• scripts/boltz2_pure_guidance.py
• scripts/protenix_fk_steering.py
• scripts/protenix_pure_guidance.py
• scripts/rf3_fk_steering.py
• scripts/rf3_pure_guidance.py
• src/sampleworks/cli/__init__.py
• src/sampleworks/cli/guidance.py
• src/sampleworks/utils/guidance_script_arguments.py
• tests/cli/__init__.py
• tests/cli/test_guidance_cli.py

[coderabbitai]

🧹 Nitpick comments (2)

src/sampleworks/utils/guidance_script_arguments.py (1)

176-182: Use a NumPy-style docstring for GuidanceConfig.from_cli.

The new method docstring is clear, but it doesn’t follow the repository’s required NumPy docstring format (e.g., Parameters / Returns sections).

As per coding guidelines: "Always include NumPy-style docstrings for every function and class."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/sampleworks/utils/guidance_script_arguments.py` around lines 176 - 182,
Update the docstring for GuidanceConfig.from_cli to use NumPy-style formatting:
add a "Parameters" section detailing parameters (e.g., model, guidance_type and
any CLI-related args or flags), and a "Returns" section describing the
GuidanceConfig instance returned (including type). Keep the existing explanatory
text but reorganize it into the NumPy sections and ensure parameter names and
return type match the method signature in GuidanceConfig.from_cli.

scripts/protenix_fk_steering.py (1)

9-9: Add a NumPy-style docstring to main().

Line 9 introduces/updates the function without the required NumPy-style docstring.

As per coding guidelines: "Always include NumPy-style docstrings for every function and class."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/protenix_fk_steering.py` at line 9, The function main() is missing a
NumPy-style docstring; add a docstring immediately under the def main():
declaration following NumPy conventions that includes a short summary,
Parameters (if any), Returns (if any), Raises (if any), and a brief Examples
section when applicable; ensure the docstring uses triple quotes, proper section
headers (Parameters, Returns, Raises, Examples) and concise descriptions so
linters and docs tools recognize it for the main() function.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@scripts/protenix_fk_steering.py`:
- Line 9: The function main() is missing a NumPy-style docstring; add a
docstring immediately under the def main(): declaration following NumPy
conventions that includes a short summary, Parameters (if any), Returns (if
any), Raises (if any), and a brief Examples section when applicable; ensure the
docstring uses triple quotes, proper section headers (Parameters, Returns,
Raises, Examples) and concise descriptions so linters and docs tools recognize
it for the main() function.

In `@src/sampleworks/utils/guidance_script_arguments.py`:
- Around line 176-182: Update the docstring for GuidanceConfig.from_cli to use
NumPy-style formatting: add a "Parameters" section detailing parameters (e.g.,
model, guidance_type and any CLI-related args or flags), and a "Returns" section
describing the GuidanceConfig instance returned (including type). Keep the
existing explanatory text but reorganize it into the NumPy sections and ensure
parameter names and return type match the method signature in
GuidanceConfig.from_cli.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ae3bc552-cd58-43cb-bdf1-274d9a7620f7

📥 Commits

Reviewing files that changed from the base of the PR and between 5dd3c4b and 1296e9b.

📒 Files selected for processing (10)

• scripts/boltz1_fk_steering.py
• scripts/boltz1_pure_guidance.py
• scripts/boltz2_fk_steering.py
• scripts/boltz2_pure_guidance.py
• scripts/protenix_fk_steering.py
• scripts/protenix_pure_guidance.py
• scripts/rf3_fk_steering.py
• scripts/rf3_pure_guidance.py
• src/sampleworks/utils/guidance_script_arguments.py
• tests/cli/test_guidance_cli.py

🚧 Files skipped from review as they are similar to previous changes (8)

• scripts/rf3_pure_guidance.py
• scripts/boltz1_pure_guidance.py
• scripts/boltz2_fk_steering.py
• scripts/boltz1_fk_steering.py
• scripts/rf3_fk_steering.py
• scripts/boltz2_pure_guidance.py
• tests/cli/test_guidance_cli.py
• scripts/protenix_pure_guidance.py

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/sampleworks/utils/guidance_script_arguments.py (1)

115-136: Avoid keeping _DYNAMIC_ATTRS in manual lockstep with every adder.

This whitelist is easy to drift. If a new CLI option gets added in any add_*_args() helper and _DYNAMIC_ATTRS is not updated, argparse will accept the flag but GuidanceConfig will silently drop it here. Consider deriving the extra fields from vars(args) minus the known base fields, or moving these fields onto the dataclass explicitly so the parser and config stay in sync.

Also applies to: 273-279

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/sampleworks/utils/guidance_script_arguments.py` around lines 115 - 136,
The _DYNAMIC_ATTRS whitelist is brittle and can drift from the add_*_args
helpers causing silently dropped flags; instead modify the code that copies
parsed args onto GuidanceConfig (the logic that currently references
_DYNAMIC_ATTRS) to compute dynamic fields at runtime by taking vars(args) and
removing the known base fields (or by using GuidanceConfig's dataclass fields)
so any new CLI options added via add_*_args are automatically included; update
the copy logic to use set(vars(args).keys()) minus set(base_field_names) (or
dataclass.__dataclass_fields__) rather than the hard-coded _DYNAMIC_ATTRS, and
remove or deprecate the static _DYNAMIC_ATTRS list to avoid future maintenance
drift.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/sampleworks/utils/guidance_script_arguments.py`:
- Around line 185-239: The code may index _MODEL_ARG_ADDERS and
_GUIDANCE_ARG_ADDERS with preset values and raise a raw KeyError for a typo; in
from_cli() validate the resolved/preset model and guidance_type against the
allowed enums (StructurePredictor and GuidanceType / model_choices and
guidance_choices) immediately after resolving them (after pre.parse_known_args
and before calling _MODEL_ARG_ADDERS[model] or
_GUIDANCE_ARG_ADDERS[guidance_type]); if a value is invalid raise a clear
ValueError mentioning the invalid value and the allowed choices so caller gets a
friendly error instead of KeyError.

In `@tests/cli/test_guidance_cli.py`:
- Around line 23-505: Add a smoke test that invokes the public entrypoint by
running the package module as a black-box process: spawn
subprocess.run([sys.executable, "-m", "sampleworks.cli.guidance", "--model",
"boltz1", "--guidance-type", "pure_guidance", "--protein", "1VME",
"--structure", "test.cif", "--density", "test.ccp4", "--resolution", "1.8"]) and
assert the process returncode is 0; place this new test alongside the
GuidanceConfig.from_cli tests (referencing GuidanceConfig.from_cli in the test
to mirror realistic args) so we verify the console entrypoint wiring without
mocking.

---

Nitpick comments:
In `@src/sampleworks/utils/guidance_script_arguments.py`:
- Around line 115-136: The _DYNAMIC_ATTRS whitelist is brittle and can drift
from the add_*_args helpers causing silently dropped flags; instead modify the
code that copies parsed args onto GuidanceConfig (the logic that currently
references _DYNAMIC_ATTRS) to compute dynamic fields at runtime by taking
vars(args) and removing the known base fields (or by using GuidanceConfig's
dataclass fields) so any new CLI options added via add_*_args are automatically
included; update the copy logic to use set(vars(args).keys()) minus
set(base_field_names) (or dataclass.__dataclass_fields__) rather than the
hard-coded _DYNAMIC_ATTRS, and remove or deprecate the static _DYNAMIC_ATTRS
list to avoid future maintenance drift.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0f4ebb27-3f83-4321-89a2-727e3bb0b70f

📥 Commits

Reviewing files that changed from the base of the PR and between 1296e9b and b589947.

⛔ Files ignored due to path filters (1)

• pixi.lock is excluded by !**/*.lock

📒 Files selected for processing (15)

• README.md
• pyproject.toml
• scripts/boltz1_fk_steering.py
• scripts/boltz1_pure_guidance.py
• scripts/boltz2_fk_steering.py
• scripts/boltz2_pure_guidance.py
• scripts/protenix_fk_steering.py
• scripts/protenix_pure_guidance.py
• scripts/rf3_fk_steering.py
• scripts/rf3_pure_guidance.py
• src/sampleworks/cli/__init__.py
• src/sampleworks/cli/guidance.py
• src/sampleworks/utils/guidance_script_arguments.py
• tests/cli/__init__.py
• tests/cli/test_guidance_cli.py

✅ Files skipped from review due to trivial changes (1)

• README.md

🚧 Files skipped from review as they are similar to previous changes (8)

• pyproject.toml
• scripts/rf3_pure_guidance.py
• src/sampleworks/cli/guidance.py
• scripts/boltz2_pure_guidance.py
• scripts/protenix_fk_steering.py
• scripts/boltz1_fk_steering.py
• scripts/boltz2_fk_steering.py
• scripts/boltz1_pure_guidance.py

[k-chrispens]

I think the one remaining change to make is to expose the new arguments that Marcus enabled in #205 to the CLI, then it's good to go! It works on my dev node when I run things. I'll let @marcuscollins come back to approve if he things his comments were addressed fully

[Abdelsalam-Abbas]

I think the one remaining change to make is to expose the new arguments that Marcus enabled in #205 to the CLI, then it's good to go! It works on my dev node when I run things. I'll let @marcuscollins come back to approve if he things his comments were addressed fully

exposed both arguments --recycling-steps and --num-diffusion-steps

[marcuscollins]

LGTM. Thanks for the changes @Abdelsalam-Abbas !