Skip to content

Fix double-call bug for custom compounds with analytical programs#105

Merged
grzanka merged 2 commits into
mainfrom
claude/libdedx-104-comments-RmoCF
May 29, 2026
Merged

Fix double-call bug for custom compounds with analytical programs#105
grzanka merged 2 commits into
mainfrom
claude/libdedx-104-comments-RmoCF

Conversation

@grzanka
Copy link
Copy Markdown
Contributor

@grzanka grzanka commented May 29, 2026

dedx_internal_evaluate_compound was called twice in
dedx_internal_validate_config when both program >= 100 and target == 0:
once to prepare mass fractions before I-potential calculation, and again
in the generic custom-compound path. The second call hit the final else
branch (DEDX_ERR_INCONSISTENT_COMPOUND) because elements_mass_fraction
was already populated, silently breaking Bethe calculations for custom
compounds.

Add an idempotency guard: when elements_mass_fraction is already set and
elements_atoms is provided, return success immediately. Add explanatory
comments at both call sites in dedx_internal_validate_config and at the
new guard branch to make the two-call pattern and its safety obvious to
future readers.

Fixes #104

https://claude.ai/code/session_01CRVeFp8umURnbExVFxMCQ1

dedx_internal_evaluate_compound was called twice in
dedx_internal_validate_config when both program >= 100 and target == 0:
once to prepare mass fractions before I-potential calculation, and again
in the generic custom-compound path. The second call hit the final else
branch (DEDX_ERR_INCONSISTENT_COMPOUND) because elements_mass_fraction
was already populated, silently breaking Bethe calculations for custom
compounds.

Add an idempotency guard: when elements_mass_fraction is already set and
elements_atoms is provided, return success immediately. Add explanatory
comments at both call sites in dedx_internal_validate_config and at the
new guard branch to make the two-call pattern and its safety obvious to
future readers.

Fixes #104

https://claude.ai/code/session_01CRVeFp8umURnbExVFxMCQ1
@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

❌ Patch coverage is 94.59459% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.94%. Comparing base (9f632e6) to head (3b6ed97).

Files with missing lines Patch % Lines
src/dedx_validate.c 71.42% 2 Missing ⚠️
tests/test_validate_internal.c 97.01% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #105      +/-   ##
==========================================
+ Coverage   75.44%   75.94%   +0.49%     
==========================================
  Files          30       30              
  Lines        2769     2843      +74     
  Branches      396      400       +4     
==========================================
+ Hits         2089     2159      +70     
- Misses        593      595       +2     
- Partials       87       89       +2     

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a configuration-validation bug affecting custom compounds (target == 0) when using analytical programs (program >= 100). The validation flow could call dedx_internal_evaluate_compound() twice, causing the second call to return DEDX_ERR_INCONSISTENT_COMPOUND after mass fractions had already been derived, which breaks Bethe-type calculations for custom compounds.

Changes:

  • Added an early-return “idempotency” branch in dedx_internal_evaluate_compound() to avoid failing on repeated calls after mass fractions are already populated.
  • Documented the required call order for analytical programs in dedx_internal_validate_config() and clarified the intentional second call for custom compounds.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/dedx_validate.c Outdated
Comment thread src/dedx_validate.c
Comment thread src/dedx_validate.c
…tests

Follow-up to PR review on the custom-compound double-call fix:

- Accept any caller-provided elements_mass_fraction in
  dedx_internal_evaluate_compound, not only the atoms-derived case. The
  public API documents elements_mass_fraction as an alternative to
  elements_atoms, so a compound given as elements_id + mass fractions
  (elements_atoms == NULL) must validate instead of hitting
  DEDX_ERR_INCONSISTENT_COMPOUND. Still reject an empty element list.

- Reset *err to DEDX_OK at the start of dedx_internal_validate_config.
  The helper validators only set *err on failure, so a stale non-zero
  value from the caller could otherwise be mistaken for an error.

- Add regression tests covering the Bethe (program >= 100) custom-compound
  paths that were previously uncovered: atom-count compounds, directly
  supplied mass fractions, and stale-err reset.

https://claude.ai/code/session_01CRVeFp8umURnbExVFxMCQ1
@grzanka grzanka merged commit 6313d91 into main May 29, 2026
24 checks passed
@grzanka grzanka deleted the claude/libdedx-104-comments-RmoCF branch May 29, 2026 21:00
grzanka added a commit to APTG/dedx_web that referenced this pull request May 30, 2026
… calc (#662) (#667)

* chore: bump libdedx submodule to merged fix for custom-compound Bethe calc

Updates the libdedx submodule from ae1f23d to 6313d91, picking up
APTG/libdedx#105 ("Fix double-call bug for custom compounds with
analytical programs"). This resolves #662, where Bethe / Bethe ext
(programs 100/101) returned no values for custom compounds because
dedx_internal_validate_config invoked dedx_internal_evaluate_compound
twice and the second call failed with DEDX_ERR_INCONSISTENT_COMPOUND.
PSTAR (tabulated, id 2) was unaffected, matching the reported symptom.

Also fixes the stale submodule tracking branch (master -> main); the
master branch no longer exists upstream.

Requires a WASM rebuild (build-wasm) to ship the fix.

https://claude.ai/code/session_01Li8whPdRMx4ZT6Z1Khv5wD

* test(e2e): add #662 regression for custom compound with Bethe program

Adds a Playwright regression test that loads the CR39 custom compound
(H:18, C:12, O:7) with the analytical Bethe program (id 100) and asserts
both stopping power and CSDA range resolve to numeric values. This is the
exact scenario from #662 that the libdedx#105 submodule bump fixes; the
test guards against the double-call regression at the app level.

https://claude.ai/code/session_01Li8whPdRMx4ZT6Z1Khv5wD

* fix: resolve double free in custom compound wrapper and fix e2e tests for Bethe validation

* Apply suggestions from code review

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

* Potential fix for pull request finding 'Unused variable, import, function or class'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

* fix: remove outdated documentation for Bethe custom compound bug

* test: fix possible undefined object errors in bethe e2e tests

* test: add Custom Water validation and table stability tests for PR #667

* docs: add AI session logs for PR #667 testing

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
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.

Bug Report: Analytical programs (Bethe) fail for custom compounds due to double-call of dedx_internal_evaluate_compound

3 participants