Skip to content

refactor(config): distinguish model names from objects#320

Open
manzuoni-astera wants to merge 2 commits into
diff-use:mdc/protpardellefrom
manzuoni-astera:michaelanzuoni/issue-289-model-name
Open

refactor(config): distinguish model names from objects#320
manzuoni-astera wants to merge 2 commits into
diff-use:mdc/protpardellefrom
manzuoni-astera:michaelanzuoni/issue-289-model-name

Conversation

@manzuoni-astera

@manzuoni-astera manzuoni-astera commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • rename structure-predictor name fields to model_name across guidance, grid-search jobs, and job results
  • preserve the public --model CLI flag with an explicit argparse destination
  • keep wrapper constructor model parameters unchanged because those hold loaded model objects
  • intentionally make the serialized schema change cleanly without legacy pickle/JSON migration shims

Implements #289. This PR is intentionally stacked on #300 because it updates the Protpardelle-integrated guidance files; #289 should remain open until the stack reaches main.

The README CLI examples remain valid because --model is unchanged.

Validation

  • pytest tests/cli/test_guidance_cli.py tests/utils/test_guidance_script_arguments.py tests/utils/test_guidance_script_utils.py -q (110 passed)
  • targeted ty check passes
  • ruff check and ruff format --check on touched Python files
  • git diff --check

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d6fd6941-9659-41b4-a021-b7c37b8f7d44

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

@marcuscollins marcuscollins left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we also need to update the readme? I guess we can still use --model, so maybe not.

Comment thread src/sampleworks/utils/guidance_script_arguments.py
Comment thread src/sampleworks/utils/guidance_script_arguments.py Outdated
Comment thread src/sampleworks/utils/guidance_script_arguments.py
Comment thread src/sampleworks/utils/guidance_script_arguments.py Outdated
Comment thread src/sampleworks/utils/guidance_script_arguments.py Outdated
Comment thread tests/test_run_grid_search.py Outdated
@manzuoni-astera

Copy link
Copy Markdown
Contributor Author

Addressed the review in fa6ac7a:

  • replaced the assertions with a descriptive RuntimeError
  • removed all legacy pickle and results.json migration shims and their compatibility-only tests
  • kept README examples unchanged because the public --model flag is unchanged

Focused CLI/config/metadata tests pass (110 passed) and targeted type/lint checks are clean. Ready for re-review.

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.

3 participants