Mdc/issue 231 dont delete structure file

[marcuscollins]

This makes a small change to make sure we do not delete original input structure files, and adds a test to ensure this behavior remains correct. This scenario arose because we started checking for unusual CIF files that include different sequences at altloc positions (when there is compositional heterogeneity in the crystal structure, in our case because some fraction of the protein in the crystal is chemically modified). If there were no changes needed, we returned the original file path. We wanted to delete any temporarily created files, but checked that the file was temporary by testing that the file was not the original, but only tested that the file name object was different than the original--these could be Path and str objects which were logically the same but not strictly the same type, and therefore we could accidentally delete the original file by unlinking it.

Resolves #231

Summary by CodeRabbit

• New Features

  • Added macOS ARM (osx-arm64) support alongside existing Linux environments

• Bug Fixes

  • Prevented accidental deletion of original structure files when invoking reward-generation with path strings

• Chores

  • Reorganized dependency and feature configuration for clearer environment management
  • Moved toolchain packages into platform-specific declarations and expanded workspace platforms
  • Added environment wiring for the new macOS feature

[coderabbitai]

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)

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

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b422f968-730b-4204-ad8f-40f44282b082

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

📝 Walkthrough

Walkthrough

Fixes a Path-vs-string comparison in get_reward_function_and_structure to prevent deletion of caller files, adds a unit test verifying preservation of the original CIF, and restructures pyproject.toml Pixi configs to add an osx-arm64 feature/environment and move platform-specific toolchain deps.

Changes

Pixi Platform and Dependency Configuration

Layer / File(s)	Summary
Feature definitions and environment wiring pyproject.toml	Replaces dependency-groups with Pixi feature tables for boltz, boltz-osx, and updates protenix to *; wires a boltz-osx environment.
Dependency target split and workspace platforms pyproject.toml	Moves general packages to tool.pixi.dependencies, places Linux toolchain packages under tool.pixi.target.linux-64.dependencies, and adds osx-arm64 to tool.pixi.workspace.platforms.

Path Comparison Bug Fix and Test Coverage

Layer / File(s)	Summary
String-based path comparison fix src/sampleworks/utils/guidance_script_utils.py	Compares str(safe_structure_path) and str(structure_path) in the cleanup guard to prevent unlinking the caller's original file.
Test for path comparison fix tests/utils/test_guidance_script_utils.py	Adds imports and test_get_reward_function_keeps_original_structure_file, which mocks altloc resolution and parsing to assert the original input CIF is not deleted when the function receives a string path.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nudged the paths to wear the same coat,

str(safe) and str(given) now speak as one;
No more lost CIFs on the testing road,
Pixi learned a platform, and spring's begun.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The pyproject.toml changes introduce platform/feature configuration not mentioned in issue #231 requirements.	Remove pyproject.toml changes or explain how they relate to fixing the file deletion bug in issue #231.

✅ 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 indicates the PR prevents unwanted deletion of original structure files, which matches the main change objective.
Linked Issues check	✅ Passed	The PR fully addresses issue #231 by comparing Path objects consistently and adding a test to verify original files are not deleted.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ 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 mdc/issue-231-dont-delete-structure-file

---

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]

🧹 Nitpick comments (2)

pyproject.toml (2)

121-121: 💤 Low value

Consider whether boltz-osx should be included in cross-environment testing.

The test-all task runs tests across boltz-dev, protenix-dev, and rf3-dev environments but does not include the newly added boltz-osx environment. If boltz-osx is intended for development use on macOS, consider whether it should be tested alongside the other environments (e.g., as boltz-osx or a hypothetical boltz-osx-dev).

This may be intentional if boltz-osx is only for local development and not CI, or if the test matrix is handled differently for macOS.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyproject.toml` at line 121, The test-all command in pyproject.toml currently
runs environments boltz-dev, protenix-dev, and rf3-dev but omits the new
boltz-osx environment; update the test-all task command string (the cmd value in
pyproject.toml for the test-all task) to include running the boltz-osx
environment (or boltz-osx-dev if you created a dev variant) alongside the
others, ensuring the command preserves the existing OR-fail logic (|| f=1) and
final exit $f behavior so failures are aggregated the same way.

---

20-31: Reproducibility concern is mitigated by pixi.lock, but '*' ranges still affect future lock refreshes

pyproject.toml pins boltz, cuequivariance-torch, cuequivariance-ops-torch-cu12, protenix, einx, and triton with '*' in [tool.pixi.feature.*] (lines 20-31). However, pixi.lock resolves them to concrete versions (e.g., boltz==2.2.1, cuequivariance-torch==0.6.1, protenix==0.7.3, einx==0.3.0, triton==3.3.1), so builds using the lockfile remain reproducible/stable.

If lock regeneration is expected to happen frequently, consider replacing '*' with bounded ranges (at least lower bounds) to reduce the risk of breaking updates when pixi lock is run.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pyproject.toml` around lines 20 - 31, The pyproject.toml feature sections
[tool.pixi.feature.boltz], [tool.pixi.feature.boltz-osx], and
[tool.pixi.feature.protenix] use '*' for pypi-dependencies (keys: "boltz",
"cuequivariance-torch", "cuequivariance-ops-torch-cu12", "rdkit", "protenix",
"einx", "triton"), which relies on pixi.lock for reproducibility but allows
risky version changes when the lock is regenerated; update those
pypi-dependencies to include at least explicit lower bounds or bounded ranges
(e.g., replace "*" with ">=<min_version>" or a caret/range) for each named
package to limit unexpected upgrades and document the chosen bounds in a
comment.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pyproject.toml`:
- Line 121: The test-all command in pyproject.toml currently runs environments
boltz-dev, protenix-dev, and rf3-dev but omits the new boltz-osx environment;
update the test-all task command string (the cmd value in pyproject.toml for the
test-all task) to include running the boltz-osx environment (or boltz-osx-dev if
you created a dev variant) alongside the others, ensuring the command preserves
the existing OR-fail logic (|| f=1) and final exit $f behavior so failures are
aggregated the same way.
- Around line 20-31: The pyproject.toml feature sections
[tool.pixi.feature.boltz], [tool.pixi.feature.boltz-osx], and
[tool.pixi.feature.protenix] use '*' for pypi-dependencies (keys: "boltz",
"cuequivariance-torch", "cuequivariance-ops-torch-cu12", "rdkit", "protenix",
"einx", "triton"), which relies on pixi.lock for reproducibility but allows
risky version changes when the lock is regenerated; update those
pypi-dependencies to include at least explicit lower bounds or bounded ranges
(e.g., replace "*" with ">=<min_version>" or a caret/range) for each named
package to limit unexpected upgrades and document the chosen bounds in a
comment.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 50420851-c55f-4a54-b4d5-d00a3a357eb7

📥 Commits

Reviewing files that changed from the base of the PR and between 984401a and fb6ab1f.

⛔ Files ignored due to path filters (1)

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

📒 Files selected for processing (3)

• pyproject.toml
• src/sampleworks/utils/guidance_script_utils.py
• tests/utils/test_guidance_script_utils.py

[k-chrispens]

Looks good to me! Thanks for the fix.