Prevent specifying a commodity of type OTH in agent_commodity_portions

[AdrianDAlessandro]

Description

This PR adds a check to validate_agent_commodity_portions to ensure that none of the commodities specified in the agent_commodity_portions.csv are of type OTH.

Note that I had to shorten the test function because clips was unhappy with the length of it. So I pulled out the make_commodity function (which was copilot's suggestion).

Fixes #1295

Type of change

• Bug fix (non-breaking change to fix an issue)
• New feature (non-breaking change to add functionality)
• Refactoring (non-breaking, non-functional change to improve maintainability)
• Optimization (non-breaking change to speed up the code)
• Breaking change (whatever its nature)
• Documentation (improve or add documentation)

Key checklist

• All tests pass: $ cargo test
• The documentation builds and looks OK: $ cargo doc
• Update release notes for the latest release if this PR adds a new feature or fixes a bug
  present in the previous release

Further checks

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.44%. Comparing base (4044358) to head (ac655b2).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1327      +/-   ##
==========================================
- Coverage   89.51%   89.44%   -0.08%     
==========================================
  Files          58       58              
  Lines        8537     8468      -69     
  Branches     8537     8468      -69     
==========================================
- Hits         7642     7574      -68     
  Misses        580      580              
+ Partials      315      314       -1     

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

[tsmbland]

Looks good!

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[tsmbland]

A couple of comments but looks good

[tsmbland]

It might be worth creating a new macro for this kind of thing like assert_error_contains (I thought we already had something like this but can't find it)

That said, we ideally want error messages to be deterministic. The problem is that in validate_agent_commodity_portions we're iterating over a HashMap which stores entries in a non-deterministic order. I can't think of a simple way to address this without storing commodity portions as an IndexMap instead, which would require more changes throughout the code, and possibly doesn't seem worth it if it's only for this reason.

Maybe leave it and see what @alexdewar thinks when he's back