Add throwIfNotFound parameter to AddQuantitiesToSimulationOutputByPath

[Copilot]

Plan for adding throwIfNotFound parameter to AddQuantitiesToSimulationOutputByPath

• Update interface method signature in IContainerTask to add throwIfNotFound parameter
• Update implementation in ContainerTask.AddQuantitiesToSimulationOutputByPath to check if quantities were found and throw/log based on the parameter
• Add test cases for the new behavior (throw when true and path not found, do not throw when false and path not found)
• Update all existing usages of the method to use the new parameter
• Address review feedback: reverse logic to check for non-zero quantities first
• Fix test failure by properly mocking OutputSelections

Summary

Successfully implemented the requested feature to add a throwIfNotFound parameter to AddQuantitiesToSimulationOutputByPath. The implementation:

• Follows the same pattern as other similar methods in the class (SetValueByPath, GetValueByPath, BaseUnitNameByPath, etc.)
• Maintains backward compatibility by updating all existing usages to pass throwIfNotFound: false
• Includes comprehensive test coverage for all scenarios
• Addressed all code review feedback to improve code quality
• Fixed test failure caused by improper mock setup

Original prompt

---

This section details on the original issue you should resolve

<issue_title>AddQuantitiesToSimulationOutputByPath should throw an error if the provided path is not found.</issue_title>
<issue_description>
AddQuantitiesToSimulationOutputByPath in OSPSuite.R.Services.ContainerTask should get an argument throwIfNotFound, as in other similar methods</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes AddQuantitiesToSimulationOutputByPath should throw an error if the provided path is not found. #2775

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[Yuri05]

1 Test fails...

[Copilot]

Pull request overview

This PR aligns AddQuantitiesToSimulationOutputByPath in OSPSuite.R.Services.ContainerTask with other path-based APIs by adding a throwIfNotFound control flag, allowing callers to choose between throwing an exception or logging a warning when no quantities match.

Changes:

• Extended IContainerTask.AddQuantitiesToSimulationOutputByPath to accept throwIfNotFound.
• Implemented not-found handling: throw OSPSuiteException when throwIfNotFound=true, otherwise log a warning and return.
• Updated call sites and added tests for throw / no-throw and successful add behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/OSPSuite.R/Services/ContainerTask.cs	Adds throwIfNotFound parameter to the interface and implements throw/log behavior when no quantities match.
tests/OSPSuite.R.Tests/Services/ContainerTaskSpecs.cs	Adds specs for exception vs no-exception and for adding outputs when a path is found.
tests/OSPSuite.R.Tests/Services/ConcurrentSimulationRunnerSpecs.cs	Updates existing call sites to pass throwIfNotFound: false.

Comments suppressed due to low confidence (2)

src/OSPSuite.R/Services/ContainerTask.cs:78

• Changing the public IContainerTask method signature by removing the old 2-arg overload is a source/binary breaking API change for any downstream consumers compiled against the previous version. Updating call sites in this repo doesn’t preserve external compatibility; consider keeping the original overload (e.g., forwarding to the new method with throwIfNotFound=false) and optionally marking it obsolete to guide migration.

      /// <summary>
      ///    Adds quantities with given path (may contain wildcards) to output selections of the simulation.
      /// </summary>
      /// <param name="simulation">Simulation to use to find the quantity by path</param>
      /// <param name="path">Absolute path of the quantity (may contain wildcards)</param>
      /// <param name="throwIfNotFound">Should an error be thrown if the quantity by path is not found?</param>
      void AddQuantitiesToSimulationOutputByPath(IModelCoreSimulation simulation, string path, bool throwIfNotFound);

src/OSPSuite.R/Services/ContainerTask.cs:189

• The error message for the not-found case is constructed twice (once for the exception and once for the warning). Consider computing it once (e.g., into a local variable) to avoid duplication and keep the throw/log branches consistent if the message ever changes.

         var quantities = AllQuantitiesMatching(simulation, path);
         
         if (quantities.Length != 0)
         {
            quantities.Each(simulation.OutputSelections.AddQuantity);
            return;
         }

         if (throwIfNotFound)

---

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

[Copilot]

1 Test fails...

Fixed in 2421dc0. The test was failing because OutputSelections wasn't properly mocked on the ISimulation fake. It's now set up correctly.

[rwmcintosh]

The tests also don't follow the established AAA pattern

[rwmcintosh]

The tests also don't follow the established AAA pattern

Which is actually the same elsewhere in this file. No shade for that.