Skip to content

Conversation

@tsmbland
Copy link
Collaborator

See #1079 (which I accidentally closed)

@tsmbland tsmbland marked this pull request as ready for review January 19, 2026 11:41
Copilot AI review requested due to automatic review settings January 19, 2026 11:41
@codecov
Copy link

codecov bot commented Jan 19, 2026

Codecov Report

❌ Patch coverage is 78.82353% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.25%. Comparing base (83b6347) to head (9236faf).
⚠️ Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
src/simulation/optimisation.rs 0.00% 16 Missing ⚠️
src/asset.rs 88.33% 14 Missing ⚠️
src/simulation/investment.rs 78.57% 3 Missing ⚠️
src/simulation/investment/appraisal.rs 60.00% 2 Missing ⚠️
src/simulation/optimisation/constraints.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1087      +/-   ##
==========================================
+ Coverage   82.15%   82.25%   +0.10%     
==========================================
  Files          55       55              
  Lines        7496     7576      +80     
  Branches     7496     7576      +80     
==========================================
+ Hits         6158     6232      +74     
- Misses       1041     1050       +9     
+ Partials      297      294       -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.

@tsmbland tsmbland mentioned this pull request Jan 19, 2026
11 tasks
@tsmbland tsmbland requested a review from alexdewar January 19, 2026 11:44
Copy link
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 pull request introduces an AssetCapacity enum to represent asset capacity as either continuous or discrete units, replacing the previous approach of using a Capacity type with optional unit_size checks throughout the codebase. This improves type safety and makes the code more explicit about how capacity is represented.

Changes:

  • Introduced AssetCapacity enum with Continuous and Discrete variants, including arithmetic and comparison operations
  • Updated all asset constructors and methods to use AssetCapacity instead of Capacity
  • Modified optimization and appraisal code to handle both capacity types appropriately

Reviewed changes

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

Show a summary per file
File Description
src/asset.rs Added AssetCapacity enum with trait implementations; updated Asset struct and methods to use the new type; added new_candidate_for_dispatch method
src/simulation/optimisation/constraints.rs Updated constraint logic to pattern match on AssetCapacity variants
src/simulation/optimisation.rs Modified iter_capacity to return AssetCapacity; updated add_capacity_variables to handle both capacity types
src/simulation/investment/appraisal/optimisation.rs Updated function signatures and results to use AssetCapacity
src/simulation/investment/appraisal/constraints.rs Simplified capacity constraint logic using pattern matching
src/simulation/investment/appraisal.rs Updated function signatures to use AssetCapacity
src/simulation/investment.rs Updated capacity arithmetic and comparisons to use AssetCapacity methods
src/simulation.rs Changed to use new_candidate_for_dispatch for dispatch candidates
src/output.rs Updated to call total_capacity() on AssetCapacity values
src/fixture.rs Updated test fixture to use AssetCapacity::Continuous
AGENTS.md Added coding style preference for named format arguments

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

Copy link
Collaborator

@alexdewar alexdewar left a comment

Choose a reason for hiding this comment

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

I assume this is just the same as what was in #1079?

@tsmbland
Copy link
Collaborator Author

I assume this is just the same as what was in #1079?

Yeah but I accidentally closed that so had to open another PR. Thanks

@tsmbland tsmbland merged commit 794dbc5 into main Jan 19, 2026
14 checks passed
@tsmbland tsmbland deleted the asset_capacity_enum branch January 19, 2026 12:03
@alexdewar
Copy link
Collaborator

Ok cool. Just wanted to check there wasn't anything new in here too!

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.

3 participants