fix(api_cc): size remapped atomic parameters by stride

[njzjz]

Summary

• allocate remapped atomic-parameter buffers for every frame, retained atom, and daparam component;
• keep the allocation count aligned with the stride passed to select_map;
• add typed C++ regressions for float and double, two frames, local and ghost virtual atoms, and both local-only and all-atom aparam layouts.

Impact

The previous allocation omitted the daparam factor. For models with dim_aparam greater than one, select_map wrote more scalar components than the vector contained, causing undefined behavior and potential heap memory corruption in shared neighbor-list API paths.

Why existing tests missed this

Existing multiframe and neighbor-list API tests use models whose daparam is one. The aparam_nall=true fixtures also use one component per atom, so the missing multiplier was numerically hidden.

The new direct helper tests exercise the exact missing combination: daparam=2 with virtual-atom remapping, multiple frames, local parameters, and local-plus-ghost parameters.

Validation

• backend-neutral C++ unit-test target compiled and linked successfully;
• TestSelectMap typed suite: 12 passed;
• the four new cases fail with the pre-fix allocation and pass with the corrected size;
• clang-format applied to changed C++ files;
• ruff format .;
• ruff check .;
• git diff --check.

Closes #5618.

Coding agent: Codex
Codex version: codex-cli 0.144.1
Model: gpt-5.6-sol
Reasoning effort: xhigh

Summary by CodeRabbit

• Bug Fixes

  • Fixed handling of multi-component atom parameters when filtering virtual atoms.
  • Ensured remapped parameter data maintains the correct size and values for local and ghost atoms.

• Tests

  • Added coverage for multi-frame data and virtual-atom filtering scenarios.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

📝 Walkthrough

Walkthrough

select_real_atoms_coord now sizes remapped atomic-parameter buffers using daparam. Typed tests cover local-only and all-atom virtual-atom removal across multiple frames.

Changes

Atomic parameter remapping

Layer / File(s)	Summary
Resize remapped atomic parameters source/api_cc/src/common.cc	aparam is resized for the selected atom count multiplied by daparam before select_map remapping.
Validate selection modes source/api_cc/tests/test_select_map.cc	Typed tests verify remapped aparam_out contents and real-atom counts for local-only and local-plus-ghost selection.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: copilot, wanghan-iapcm

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 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 fix to atomic-parameter buffer sizing in the C++ API.
Linked Issues check	✅ Passed	The code change resizes aparam by daparam and the new tests cover the remapped buffer behavior described in #5618.
Out of Scope Changes check	✅ Passed	The changes stay focused on the buffer-sizing fix and regression tests with no unrelated additions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

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

Inline comments:
In `@source/api_cc/tests/test_select_map.cc`:
- Around line 98-138: Fix both affected tests in
source/api_cc/tests/test_select_map.cc:98-138 and
source/api_cc/tests/test_select_map.cc:140-178 by changing
TYPED_TEST(TestSelectMap, ...) to TEST_F(TestSelectMap, ...), and replace every
TypeParam use with the fixture’s VALUETYPE type. Keep the existing test logic
and assertions unchanged.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 567d2c71-1c2e-4d17-a39d-311e6d570543

📥 Commits

Reviewing files that changed from the base of the PR and between 7d5ad38 and f266482.

📒 Files selected for processing (2)

• source/api_cc/src/common.cc
• source/api_cc/tests/test_select_map.cc

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.58%. Comparing base (7d5ad38) to head (f266482).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5790      +/-   ##
==========================================
- Coverage   79.85%   79.58%   -0.27%     
==========================================
  Files        1022     1022              
  Lines      117351   117387      +36     
  Branches     4313     4310       -3     
==========================================
- Hits        93706    93422     -284     
- Misses      22101    22408     +307     
- Partials     1544     1557      +13     

☔ 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[njzjz]

This piece of codes was created by @Yi-FanLi in 69e712c, so I request @Yi-FanLi to review.

[wanghan-iapcm]

Correct fix. select_map writes out[kknall1stride + to_iistride + dd] with stride=daparam and does not resize out, so the destination needs nframesnall1*daparam elements; the old resize omitted the daparam factor, overflowing the buffer for dim_aparam>1. The new resize matches select_map's indexing exactly. The regressions genuinely fail pre-fix on the size assertion (4!=8, 6!=12) before even hitting the out-of-bounds writes, and they cover both the aparam_nall=false (local) and aparam_nall=true (all) branches, plus float/double and local+ghost virtual atoms. Good explanation of why daparam=1 fixtures hid it. LGTM.

Dismissing to re-review through the /code-review skill per process.

[wanghan-iapcm]

Re-reviewed via the code-review skill. Correct and complete: the new nframesnall1daparam allocation exactly matches select_map's max write index+1 (select_map doesn't resize its output, so the caller's resize is load-bearing), and the sibling dcoord/atype buffers were already correctly stride-sized, so no analogous bug remains. Day-one issue from #2436 where the aparam line never followed the stride-multiply pattern its dcoord neighbor established in the same commit. The two regressions fail pre-fix deterministically (size 4!=8 and 6!=12 via ASSERT_EQ) and cover both aparam_nall branches with local+ghost virtual atoms; pre-sizing aparam_out keeps the failure a clean size-assertion rather than an ASAN crash. The CodeRabbit TYPED_TEST-won't-compile note is a false positive (the fixture is a template). LGTM.