Skip to content

Add validation for duplicate species within phases#124

Merged
boulderdaze merged 3 commits intomainfrom
copilot/fix-114
Jul 21, 2025
Merged

Add validation for duplicate species within phases#124
boulderdaze merged 3 commits intomainfrom
copilot/fix-114

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jul 18, 2025

This PR adds validation to detect and report duplicate species within individual phases in the v1 parser. Previously, the parser only validated duplicate phase names and duplicate species globally, but allowed duplicate species within the same phase.

Changes Made

Core Implementation:

  • Added DuplicateSpeciesInPhaseDetected to ConfigParseStatus enum
  • Enhanced ParsePhases function in mechanism_parsers.cpp to check for duplicate species within each phase
  • Added corresponding string representation in parse_status.cpp

Testing:

  • Created test configuration files for duplicate species scenarios (both YAML and JSON)
  • Added DetectsDuplicateSpeciesInPhase test case to test_parse_phases.cpp
  • Fixed existing modal test configurations that had unintentional duplicate species

Example

The parser now correctly detects and reports errors like:

# This configuration will now fail validation
phases:
  - name: gas
    species: [O2, N2, O2]  # duplicate O2

Error message:

Duplicate species 'O2' found in phase 'gas'.

Validation

  • ✅ All existing tests continue to pass
  • ✅ New functionality works with both YAML and JSON configurations
  • ✅ Error messages are clear and specify both the duplicate species and phase name
  • ✅ Valid configurations without duplicates parse successfully
  • ✅ Multiple phases are handled correctly (only phases with duplicates report errors)

Fixes #114.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Co-authored-by: K20shores <13836216+K20shores@users.noreply.github.com>
Copilot AI changed the title [WIP] Check for duplicate species in a phase Add validation for duplicate species within phases Jul 18, 2025
Copilot AI requested a review from K20shores July 18, 2025 21:24
@boulderdaze boulderdaze self-requested a review July 18, 2025 21:27
@K20shores K20shores marked this pull request as ready for review July 18, 2025 21:32
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.95%. Comparing base (a3647c4) to head (c4cfabc).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #124   +/-   ##
=======================================
  Coverage   86.95%   86.95%           
=======================================
  Files           3        3           
  Lines          23       23           
=======================================
  Hits           20       20           
  Misses          3        3           

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

Comment on lines +99 to +112
// Check for duplicate species within this phase
for (size_t i = 0; i < species.size(); ++i)
{
for (size_t j = i + 1; j < species.size(); ++j)
{
if (species[i] == species[j])
{
errors.push_back({ ConfigParseStatus::DuplicateSpeciesInPhaseDetected,
"Duplicate species '" + species[i] + "' found in phase '" + name + "'." });
break;
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't want to break. Can you try again?

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.

@copilot, we don't want to break here, we want to collect as many errors as we can. Please remove this break

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed the break statement as requested. The parser now collects all duplicate species errors within a phase rather than stopping at the first one. Verified with testing - configurations with multiple duplicates now report all instances (commit c4cfabc).

Co-authored-by: K20shores <13836216+K20shores@users.noreply.github.com>
@boulderdaze boulderdaze merged commit b4e3b70 into main Jul 21, 2025
15 checks passed
@boulderdaze boulderdaze deleted the copilot/fix-114 branch July 21, 2025 20:51
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.

Check for duplicate species in a phase

4 participants