Consolidate post-contingency balance expressions via target-index dispatch

[luke-kiernan]

Summary

Stacked on #166 (which is itself stacked on #148 / ac/g1-port). Reviewing the outage/security-constrained code alongside the existing _system_expression_type/_balance_expression_targets framework in add_to_expression.jl surfaced a natural consolidation: the system-wide, PTDF-nodal, and AreaBalance-area post-contingency balance/deployment expressions differed only in which extra index component sits between outage_id and t — the surrounding accumulation logic (healthy-device reserve deployment / outaged-generator pre-contingency output) was duplicated three times.

• Introduced _post_contingency_target_axes/_post_contingency_target_index, mirroring the existing _balance_expression_targets pattern: plain multiple dispatch (no closures/higher-order functions), returning a Tuple of 0 or 1 extra index components.
• Dispatch is keyed on the expression type T, not on network_model's runtime type — PostContingencyActivePowerBalance is built unconditionally under every network model (the outer system-wide check), so it must always be axis-less regardless of what network_model happens to be at that call site. Only PostContingencyNodalActivePowerDeployment/PostContingencyAreaActivePowerDeployment are network-model-specific, and their callers already guarantee the matching network model before invoking them. (Caught this the hard way — an earlier version dispatched on network_model directly and broke the PTDF path; fixed and re-verified.)
• Six near-duplicate add_to_expression! methods collapse into two generic ones + the two small dispatch helpers; the three _add_pre_contingency_*_terms_over_time! copies collapse into one.
• As a side effect of unifying the signature, the AreaBalance pre-contingency call site now threads attribute_device_map through instead of recomputing it via a redundant PSY.get_component_supplemental_attribute_pairs call.

Deliberately did not extend this to PostContingencyGenerationBalanceConstraint/PostContingencyCopperPlateBalanceConstraint — on inspection those aren't just an index-shape difference (the AreaBalance one sums in an extra baseline ActivePowerBalance expression the system-wide one doesn't need), so forcing them into the same abstraction would conflate two different concerns for two call sites.

Test plan

• julia --project=test test/runtests.jl test_static_injection_security_constrained_models — 1219/1219 pass
• Test.detect_ambiguities(PowerOperationsModels) — 0 ambiguities
• Formatter run

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR refactors the G-1 security-constrained reserve model’s post-contingency balance/deployment expression construction by consolidating three previously near-duplicate implementations (system-wide balance, PTDF nodal deployment, AreaBalance area deployment) into shared generic methods, with the remaining differences handled via type-based target-index dispatch.

Changes:

• Added _post_contingency_target_axes and _post_contingency_target_index helpers to resolve the “extra axis” (none/bus/area) based on expression type T.
• Collapsed multiple add_to_expression! and _add_pre_contingency_*_terms_over_time! specializations into generic implementations parameterized by T.
• Threaded attribute_device_map through the AreaBalance pre-contingency path to avoid redundant recomputation.

---

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

[acostarelli]

This uses the splat operator a few times, is this a performance concern?

Secondarily, I would really like to cut back on the level of AI comments in this repository. The comments here feel very verbose and seem to assume the reader has the context of what the code was before the change.

Side-thought, my gut feeling is this particular idea for a refactor feels like something that could quickly be over-applied across POM. I would prefer something that feels more "sturdy" -- I'm not sure what I mean by that, but I think it has to do with the splatting.

[acostarelli]

I forget the performance implications here, is using the spread operator bad?

[luke-kiernan]

The refactor here is just to reduce code duplication: the types should be constant-propagated such that _post_contingency_target_axes and the ... are both resolved at compile time. I have not verified that is the case though

[github-actions]

Performance Results

Version	Precompile Time
Main	2.953355781
This Branch	2.942272817

Version	Build Time
Main-Build Time Precompile	83.569085155
Main-Build Time Postcompile	1.327224093
This Branch-Build Time Precompile	82.663658556
This Branch-Build Time Postcompile	1.278365597

Version	Solve Time
Main-Solve Time Precompile	3275.141003642
Main-Solve Time Postcompile	3245.312133551
This Branch-Solve Time Precompile	2079.798172982
This Branch-Solve Time Postcompile	2035.157889006

[luke-kiernan]

Secondarily, I would really like to cut back on the level of AI comments in this repository. The comments here feel very verbose and seem to assume the reader has the context of what the code was before the change.

Agreed: AI loves to add superfluous comments about the evolution of the code ["changed from x to y because..."], when mostly care about the behavior.

Side-thought, my gut feeling is this particular idea for a refactor feels like something that could quickly be over-applied across POM. I would prefer something that feels more "sturdy" -- I'm not sure what I mean by that, but I think it has to do with the splatting.

Hmm. In the case of copperplate type models, we want to call

lazy_container_addition!(container, T, R,
    string.(IS.get_uuid.(associated_outages)),
    time_steps;
    meta = service_name,
)

whereas in the other cases, we want to add another axis between the outages and the time steps (areas for area models, buses for PTDF model). I don't see how to make that happen without the ... splatting operator.

Well, we could make lazy_container_addition! and its like take CartesianAxes-style wrapper, so that we pass (outages, areas, time steps) as one arg instead of as three args. But that's just hiding the splat somewhere else.

[acostarelli]

Yeah that's fair. I think something is off-putting to me about this, but I think it's probably the container constructor that's bothering me. I would probably prefer the axes to be specified in one argument, not in varargs.

[luke-kiernan]

I've diagnosed the CI failure: it's independent of this PR. So I'll merge this.