Low-priority quality-of-life follow-ups from PR #58 review (REVIEW-pr-58.md m4, n6, n7). Defer until Phase 2.2-2.4 surface the actual API friction.
Items
1. Lift Handle structs out of `ConstitutiveModel`
Source: REVIEW-pr-58.md m4 (cpp-pro #4).
File: `include/numsim_codegen/recipe.h:243-252`
`ConstitutiveModel::ScalarStateVariableHandle` and `::TensorStateVariableHandle` are nested types. `auto` deduction at user call sites hides the spelling, but Phase 2.2's `TimeIntegrationPass` will write helper functions that accept handles by value — the qualified name `ConstitutiveModel::ScalarStateVariableHandle` is cumbersome in those signatures.
Possible fix: lift to free types in `numsim::codegen`, or use a common templated `StateVar`. Wait until the second consumer that wants to pass handles around (likely Phase 2.4's `LocalNewtonSystem`) before committing to one shape.
2. Handle equality + name accessor
Source: REVIEW-pr-58.md n7 (architect Q9).
Phase 2.2 lowering will want to ask "is this expression's leaf the same symbol as `α.current`?". The cas layer supports that via pointer identity on the underlying expression. Exposing `Handle::name()` (returning the registered string) and `operator==` would make pattern-matching code in the lowering pass clearer.
Possible fix: add `name()` returning the bare state-var name (so `alpha.current.name()` and `alpha.previous.name()` could resolve to "alpha" and "alpha_old" respectively). Equality: defer to the underlying cas `expression_holder` identity.
3. `add_*_state_variable` accepts a `Role` parameter
Source: REVIEW-pr-58.md n6 (architect Q9).
Today's API is `add_scalar_state_variable(name, initial_value, doc)` — no Role. Future state variables may want semantic tagging (`roles::Hardening`, `roles::Damage`) for backend-specific storage decisions (e.g. some backends might want hardening variables in a single contiguous buffer).
Possible fix: add an optional `Role role = roles::Other` parameter to both add methods. Backends switch on `Category::StateVariable*` for the wiring distinction (current vs old); the Role is purely a semantic hint. Easy to add when the first consuming use case lands.
4. Variant-vs-Kind invariant check pass
Source: REVIEW-pr-58.md m7 (cpp-pro #3 + architect Q7).
`StateVariable` has a `Kind` field AND a `std::variant<scalar_holder, tensor_holder> initial_value`. Today the `add_*_state_variable` method signatures enforce alignment via overload resolution. A future internal codepath that constructs `StateVariable` directly (e.g. Phase 2.2 synthesising a state var during lowering) could mismatch.
Possible fix: add `assert(std::holds_alternative<scalar_holder>(sv.initial_value))` (etc.) inside both `add_*_state_variable` methods (one line), plus a real `SymbolValidationPass`-side check that walks `state_variables()` and verifies kind-vs-variant alignment. Defer the latter until Phase 2.2 introduces a synthetic-state-var pass.
Why low priority
Today's API works. These items address friction that surfaces only when:
- Phase 2.2-2.4 write helper functions consuming Handle types (item 1)
- Phase 2.2 lowering passes pattern-match by symbol identity (item 2)
- Backends specialise on state-var semantic role (item 3)
- A codepath constructs `StateVariable` outside the add methods (item 4)
Bundle into one PR if/when two of the above triggers fire.
Refs: `REVIEW-pr-58.md`, PR #58, epic #28.
Low-priority quality-of-life follow-ups from PR #58 review (REVIEW-pr-58.md m4, n6, n7). Defer until Phase 2.2-2.4 surface the actual API friction.
Items
1. Lift Handle structs out of `ConstitutiveModel`
Source: REVIEW-pr-58.md m4 (cpp-pro #4).
File: `include/numsim_codegen/recipe.h:243-252`
`ConstitutiveModel::ScalarStateVariableHandle` and `::TensorStateVariableHandle` are nested types. `auto` deduction at user call sites hides the spelling, but Phase 2.2's `TimeIntegrationPass` will write helper functions that accept handles by value — the qualified name `ConstitutiveModel::ScalarStateVariableHandle` is cumbersome in those signatures.
Possible fix: lift to free types in `numsim::codegen`, or use a common templated `StateVar`. Wait until the second consumer that wants to pass handles around (likely Phase 2.4's `LocalNewtonSystem`) before committing to one shape.
2. Handle equality + name accessor
Source: REVIEW-pr-58.md n7 (architect Q9).
Phase 2.2 lowering will want to ask "is this expression's leaf the same symbol as `α.current`?". The cas layer supports that via pointer identity on the underlying expression. Exposing `Handle::name()` (returning the registered string) and `operator==` would make pattern-matching code in the lowering pass clearer.
Possible fix: add `name()` returning the bare state-var name (so `alpha.current.name()` and `alpha.previous.name()` could resolve to "alpha" and "alpha_old" respectively). Equality: defer to the underlying cas `expression_holder` identity.
3. `add_*_state_variable` accepts a `Role` parameter
Source: REVIEW-pr-58.md n6 (architect Q9).
Today's API is `add_scalar_state_variable(name, initial_value, doc)` — no Role. Future state variables may want semantic tagging (`roles::Hardening`, `roles::Damage`) for backend-specific storage decisions (e.g. some backends might want hardening variables in a single contiguous buffer).
Possible fix: add an optional `Role role = roles::Other` parameter to both add methods. Backends switch on `Category::StateVariable*` for the wiring distinction (current vs old); the Role is purely a semantic hint. Easy to add when the first consuming use case lands.
4. Variant-vs-Kind invariant check pass
Source: REVIEW-pr-58.md m7 (cpp-pro #3 + architect Q7).
`StateVariable` has a `Kind` field AND a `std::variant<scalar_holder, tensor_holder> initial_value`. Today the `add_*_state_variable` method signatures enforce alignment via overload resolution. A future internal codepath that constructs `StateVariable` directly (e.g. Phase 2.2 synthesising a state var during lowering) could mismatch.
Possible fix: add `assert(std::holds_alternative<scalar_holder>(sv.initial_value))` (etc.) inside both `add_*_state_variable` methods (one line), plus a real `SymbolValidationPass`-side check that walks `state_variables()` and verifies kind-vs-variant alignment. Defer the latter until Phase 2.2 introduces a synthetic-state-var pass.
Why low priority
Today's API works. These items address friction that surfaces only when:
Bundle into one PR if/when two of the above triggers fire.
Refs: `REVIEW-pr-58.md`, PR #58, epic #28.