[dbolser]

Valid BinaryCIF numeric columns failed to decode in two cases: interval quantization looked up num_steps instead of the standard numSteps, and delta decoding tried to modify the read-only array returned by ByteArray decoding.

Read numSteps, calculate the quantization step as (max - min) / (numSteps - 1), and copy delta input before modifying it. An interval with min=1, max=2, and three steps now decodes [0, 1, 2] to [1, 1.5, 2]. The metadata and arithmetic follow the BinaryCIF encoding specification.

Regression tests start with encoded byte payloads and exercise the complete decoding chains. They cover both floating-point output types and zero, positive, and negative delta origins. All five new subcases fail on the original code with KeyError: 'num_steps' or ValueError: assignment destination is read-only, and pass after the fix.

Validation on Linux, CPython 3.14.6:

• python -m pytest --offline -q Tests/test_PDB_binary_cif.py: 18 passed, 25 subtests passed, including the existing public-parser structure comparisons.
• Full offline suite: 4,439 passed, 116 skipped, 2,947 subtests passed. Skips include network tests and unavailable optional dependencies.
• pre-commit run --all-files: passed, including both mypy checks.

Other platforms and Python versions were not tested locally; CI is pending. This fixes the two BinaryCIF findings from the review. The alignment safety fixes are independently covered by #264; neither PR depends on the other.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 56 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: eda2641f-0422-4185-a719-d771ce7c86fb

📥 Commits

Reviewing files that changed from the base of the PR and between 13d83b9 and 9538951.

📒 Files selected for processing: 3

• Bio/PDB/binary_cif.py
• NEWS.rst
• Tests/test_PDB_binary_cif.py

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

[gemini-code-assist]

Code Review

This pull request fixes issues in the BinaryCIF decoder by correcting the interval quantization step size calculation, reading the standard numSteps field, and ensuring delta decoding copies its input to avoid modifying a read-only buffer. It also adds corresponding unit tests and updates the release notes. The review feedback suggests adding robust error handling to prevent a potential ZeroDivisionError when numSteps is 1 or less, and an IndexError when decoding empty columns.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.55%. Comparing base (13d83b9) to head (9538951).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #265      +/-   ##
==========================================
+ Coverage   87.53%   87.55%   +0.01%     
==========================================
  Files         294      294              
  Lines       60497    60497              
==========================================
+ Hits        52956    52967      +11     
+ Misses       7541     7530      -11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dbolser]

Independent review

Correct, and ready to merge.

• Both fixes match the BinaryCIF encoding spec: it reads numSteps, the step is (max - min) / (numSteps - 1), and Delta decoding copies the read-only buffer from ByteArray decoding.
• The tests start from encoded bytes, so they exercise the whole decoding chain.

On Gemini's two points (neither blocks the merge):

• numSteps <= 1 raises ZeroDivisionError, not ValueError. This only happens with a malformed file. A short guard would match TestMalformedEncodings, if wanted.
• Empty Delta column raises IndexError. This bug predates the PR, and real atom_site columns are not empty. It is out of scope here.

🤖 Generated with Claude Code