Phase 2.2 prep: state-var alignment invariant, declared tag, find_state_variable_by_name (#59)

docs/workflow.md

@@ -64,14 +64,17 @@ sequenceDiagram
     Model->>PM: run(pctx)
 
     PM->>SVP: run(pctx)
+    SVP->>SVP: reset m_state_variables_non_empty = false<br/>(round-3 safe-on-throw guarantee)
     SVP->>SVP: walk symbols, check identifier validity<br/>(ASCII range + C++ keyword reject)
     SVP->>SVP: walk outputs, check no undeclared symbols
-    SVP-->>PM: postconditions:<br/>{symbols-declared, identifiers-valid}
+    SVP->>SVP: verify_state_variable_symbol_alignment<br/>(Phase 2.1+ pairing invariant)
+    SVP->>SVP: set m_state_variables_non_empty<br/>= !state_variables().empty()
+    SVP-->>PM: postconditions:<br/>{symbols-declared, identifiers-valid,<br/>state-variables-checked,<br/>state-variables-non-empty (iff non-empty)}
 
     PM->>TSP: run(pctx)
     TSP->>TSP: cross-check Role.is_symmetric vs<br/>tensor_space.perm (Skew = conflict)
     TSP->>TSP: cross-check Role.expected_rank vs<br/>tensor.rank()
-    TSP-->>PM: postconditions:<br/>{tensor-space-validated}
+    TSP-->>PM: postconditions:<br/>{tensor-space-declarations-checked}
 
     PM->>CEP: run(pctx)
     Note over CEP: Cycle-break via<br/>unique_ptr<T2sCodeEmit> indirection

include/numsim_codegen/passes/code_emit_pass.h

@@ -21,8 +21,17 @@ namespace numsim::codegen {
 //
 // Preconditions: `pass_tags::symbols_declared` +
 // `pass_tags::identifiers_valid` + `pass_tags::tensor_space_declarations_checked`
-// (i.e. SymbolValidationPass + TensorSpaceConsistencyPass must have run
-// first). If you add a pass that transforms expressions (e.g. a future
+// + `pass_tags::state_variables_checked` (i.e. SymbolValidationPass +
+// TensorSpaceConsistencyPass must have run first). The state-var tag
+// (PR #66 round-3 review #3) codifies the bypass-caveat from
+// `verify_state_variable_symbol_alignment` in the precondition graph:
+// SymbolValidationPass advertises it unconditionally, so any emit
+// pipeline transitively requires SVP — a custom pipeline that stubs
+// the identifier tags via a no-op pass and skips SVP would now fail
+// at `run()` rather than silently emit code with an unverified
+// state-var alignment.
+//
+// If you add a pass that transforms expressions (e.g. a future
 // TimeIntegrationPass), register it AFTER the validators but BEFORE
 // CodeEmitPass.
 class CodeEmitPass final : public Pass {
@@ -33,7 +42,8 @@ class CodeEmitPass final : public Pass {
   [[nodiscard]] auto preconditions() const
       -> std::vector<std::string_view> override {
     return {pass_tags::symbols_declared, pass_tags::identifiers_valid,
-            pass_tags::tensor_space_declarations_checked};
+            pass_tags::tensor_space_declarations_checked,
+            pass_tags::state_variables_checked};
   }
   [[nodiscard]] auto postconditions() const
       -> std::vector<std::string_view> override {

include/numsim_codegen/passes/pass.h

@@ -15,6 +15,7 @@
 namespace numsim::codegen {
 
 struct SymbolDecl;
+struct StateVariable;
 
 // Shared state for a single PassManager invocation. Passes read the
 // recipe via `model` (a RecipeView — const-only today, will gain a
@@ -73,6 +74,20 @@ enum class LookupError {
                                              std::string const &name) noexcept
     -> std::expected<SymbolDecl const *, LookupError>;
 
+// Convenience: resolve a name to its StateVariable record. Returns
+// nullable pointer rather than `std::expected` per `docs/workflow.md`
+// §6.1 — only one failure mode ("no state variable with this name")
+// that the caller could distinguish. Linear scan over
+// `pctx.model.state_variables()`; state-variable counts are small
+// (single-digit typical, tens worst-case for multi-surface plasticity)
+// so the scan is fine. Could be promoted to a populated map in
+// PassContext if that ever changes. Issue #59 / REVIEW-pr-58.md m3.
+//
+// Definition lives in recipe.h where `StateVariable` is complete.
+[[nodiscard]] inline auto find_state_variable_by_name(
+    PassContext const &pctx, std::string_view name) noexcept
+    -> StateVariable const *;
+
 // Abstract base for a single codegen pass.
 //
 // Passes advertise their pre/postconditions as string tags. PassManager
@@ -88,10 +103,34 @@ class Pass {
 public:
   virtual ~Pass() = default;
   [[nodiscard]] virtual auto name() const -> std::string_view = 0;
+
+  // Tags this pass requires to be satisfied before `run()` is called.
+  // Stable across the pass's lifetime — `preconditions()` is queried by
+  // `PassManager::run()` *before* `run()`, so the value must not depend
+  // on per-call state.
   [[nodiscard]] virtual auto preconditions() const
       -> std::vector<std::string_view> {
     return {};
   }
+
+  // Tags this pass advertises as satisfied after `run()` returns
+  // successfully. **Lifecycle (PR #66 round-2 review #11, round-3 #2):**
+  // `PassManager::run()` queries `postconditions()` AFTER each call to
+  // `run()`, never before. Implementations are therefore free to return
+  // values that depend on per-call state set during `run()`.
+  //
+  // Most passes (`TensorSpaceConsistencyPass`, `CodeEmitPass`) return
+  // literal initialiser lists from `postconditions()` and don't need to
+  // think about lifecycle. `SymbolValidationPass` uses the pattern to
+  // conditionally advertise `state_variables_non_empty` based on the
+  // recipe — see its `run()` body for the canonical shape.
+  //
+  // **Guidance for passes that DO store per-call postcondition state:**
+  // reset that state at the *start* of `run()`, not the end, so a
+  // throwing `run()` leaves the pass in a coherent observable state and
+  // a pre-`run()` query reports the safe-default shape. This is
+  // convention, not a framework-enforced contract — PassManager has no
+  // way to check it.
   [[nodiscard]] virtual auto postconditions() const
       -> std::vector<std::string_view> {
     return {};

include/numsim_codegen/passes/pass_tags.h

@@ -36,6 +36,38 @@ namespace numsim::codegen::pass_tags {
 inline constexpr std::string_view symbols_declared = "symbols-declared";
 inline constexpr std::string_view identifiers_valid = "identifiers-valid";
 
+// State-variable tags advertised by `SymbolValidationPass` (issue #59 /
+// REVIEW-pr-58.md m2, refined by PR #66 review).
+//
+// Phase 2.2+ passes that touch state variables fall in one of two
+// shapes, and the framework can serve them differently with two tags:
+//
+//   * `state_variables_checked` — ALWAYS advertised. The
+//     SymbolValidationPass body has walked `state_variables()` and
+//     verified the alignment invariant with `symbols()` (via
+//     `verify_state_variable_symbol_alignment`). A pass that runs even
+//     on pure-elasticity recipes (e.g. one that emits no-op stubs for
+//     consistency) takes this as a precondition.
+//
+//   * `state_variables_non_empty` — advertised IFF the recipe actually
+//     has at least one state variable. A pass whose body is a no-op on
+//     empty state vectors (TimeIntegrationPass, KuhnTuckerLoweringPass)
+//     takes this as a precondition; on a pure-elasticity recipe
+//     PassManager then fails at `run()` with a clear
+//     `"pass X requires precondition state-variables-non-empty but no
+//     earlier pass advertised that postcondition"` message — surfacing
+//     the misconfiguration loudly rather than the pass silently doing
+//     nothing. (PassManager checks preconditions at `run()` time, not
+//     at registration; see `pass_manager.h:39-46`.)
+//
+// The split keeps the framework's tag-tracking honest: a passcondition
+// has the same name across every consumer, so a typo or rename surfaces
+// at compile time, not in the form of a pass that quietly never fires.
+inline constexpr std::string_view state_variables_checked =
+    "state-variables-checked";
+inline constexpr std::string_view state_variables_non_empty =
+    "state-variables-non-empty";
+
 // TensorSpaceConsistencyPass postcondition. Renamed from the original
 // `tensor-space-validated` per P6 — the original overpromised; Phase 2's
 // expression-level inference pass will use a separate tag.

include/numsim_codegen/passes/symbol_validation_pass.h

@@ -34,7 +34,17 @@ class SymbolValidationPass final : public Pass {
   }
   [[nodiscard]] auto postconditions() const
       -> std::vector<std::string_view> override {
-    return {pass_tags::symbols_declared, pass_tags::identifiers_valid};
+    // `state_variables_non_empty` is conditionally advertised — PR #66
+    // review #6. `m_state_variables_non_empty` is set by `run()` to
+    // reflect the recipe's actual state-variable count; PassManager
+    // queries postconditions() AFTER run(), so the conditional is safe.
+    std::vector<std::string_view> tags{pass_tags::symbols_declared,
+                                       pass_tags::identifiers_valid,
+                                       pass_tags::state_variables_checked};
+    if (m_state_variables_non_empty) {
+      tags.push_back(pass_tags::state_variables_non_empty);
+    }
+    return tags;
   }
   void run(PassContext &pctx) override; // defined in recipe.h after class.
 
@@ -111,6 +121,13 @@ class SymbolValidationPass final : public Pass {
     }
     return !is_cxx_keyword(s);
   }
+
+private:
+  // Runtime flag set by `run()` to drive the conditional
+  // `state_variables_non_empty` postcondition (declared above).
+  // Default-false so a pre-`run()` query reports the safe shape (no
+  // such query exists today, but defensive).
+  bool m_state_variables_non_empty = false;
 };
 
 } // namespace numsim::codegen

include/numsim_codegen/recipe.h

@@ -190,6 +190,14 @@ struct OutputDecl {
   Role role = roles::Other;
 };
 
+// Forward-declared in a sub-namespace so `ConstitutiveModel`'s
+// `friend struct testing_detail::state_variable_alignment_access;`
+// resolves. Definition lives only in the test TU (see
+// `tests/StateVariableTest.cpp`). PR #66 round-2 review #5.
+namespace testing_detail {
+struct state_variable_alignment_access;
+} // namespace testing_detail
+
 // The constitutive-model registry. Holds declared inputs, parameters,
 // outputs, and their semantic role tags. Target-agnostic — the same
 // recipe can be emitted as standalone C++, MOOSE Material, Abaqus UMAT,
@@ -488,6 +496,27 @@ class ConstitutiveModel {
   }
 
 private:
+  // ─── Test-only friends ──────────────────────────────────────────
+  //
+  // PR #66 round-2 review #5: grants negative-path tests in `tests/`
+  // controlled write access to `m_symbols` and `m_state_variables` so
+  // they can simulate the corruption shapes Phase 2.2+ mutating passes
+  // might produce. Placed under the `private:` label (rather than the
+  // public block) so a reader scanning the class layout doesn't mistake
+  // the line for a public-API affordance.
+  //
+  // Lives in the `testing_detail::` sub-namespace — both as a marker
+  // ("if you're writing production code and naming this, you've taken
+  // a wrong turn") and to keep the friend's reachable surface narrow
+  // from production-namespace TUs.
+  //
+  // The struct is declared here but never defined inside
+  // `numsim::codegen`; tests provide the only definition (see
+  // `tests/StateVariableTest.cpp`). Keeping the surface to a single
+  // friend struct means a single ODR slot in the test binary; if a
+  // future test TU needs the same access, factor the definition to a
+  // shared `tests/internal/` header rather than redefining it.
+  friend struct testing_detail::state_variable_alignment_access;
   // Phase 2.1 (M1+M2 in REVIEW-pr-58.md): reject a duplicate symbol
   // name at add time, with a clear pipeline-misconfiguration message.
   // Two collision modes this catches:
@@ -634,6 +663,107 @@ inline auto RecipeView::tensor_symbol_map() const -> TensorSymbolMap const & {
   return &decl;
 }
 
+[[nodiscard]] inline auto find_state_variable_by_name(
+    PassContext const &pctx, std::string_view name) noexcept
+    -> StateVariable const * {
+  for (auto const &sv : pctx.model.state_variables()) {
+    if (sv.name == name) {
+      return &sv;
+    }
+  }
+  return nullptr;
+}
+
+// Phase 2.2 prep (issue #59 / REVIEW-pr-58.md m1 + PR #66 review):
+// verify the structural alignment between `model.state_variables()` and
+// `model.symbols()`. Throws `std::runtime_error` naming the specific
+// mismatch — silent corruption is worse than a loud throw.
+//
+// Today no public `ConstitutiveModel` API can produce a violation
+// because `add_*_state_variable` writes both vectors atomically. The
+// check earns its keep when Phase 2.2+ mutating passes start touching
+// `m_state_variables` / `m_symbols` independently — at that point this
+// function lives inside `SymbolValidationPass`, so any pipeline that
+// registers `SymbolValidationPass` re-verifies the invariant on every
+// run. Phase 2.2 mutating passes can re-run the pass (or a derived
+// `StructuralIntegrityPass` that calls this) to re-check between
+// mutations.
+//
+// **Bypass caveat:** a caller that constructs a `PassManager` and
+// omits `SymbolValidationPass` skips the check entirely. The framework
+// has no enforcement against that today; the convention is that any
+// pipeline that emits code or mutates state variables registers
+// `SymbolValidationPass` first. `ConstitutiveModel::validate()` and
+// `::emit_compute_function()` both do so.
+//
+// TODO(phase-2.2): this scaffolding fault-injects the assertion arms
+// (see the friend-poison tests in `tests/StateVariableTest.cpp`). When
+// the first mutating pass lands, mutator-driven integration tests
+// (apply mutator → assert throws) will be added alongside this corpus,
+// not replace it — the fault-injection harness covers the assertion
+// arms; the integration tests cover the mutator-to-arm wiring.
+inline void verify_state_variable_symbol_alignment(RecipeView model) {
+  // Hoisted out of the lambda body (PR #66 round-3 #6): the span is
+  // identical across all `check()` invocations within a single call
+  // — fetching once makes that explicit and removes a redundant copy
+  // per state variable.
+  auto const symbols = model.symbols();
+  auto check = [&](std::size_t idx, std::string_view expected_name,
+                   SymbolDecl::Category expected_cat,
+                   StateVariable const &sv, char const *which) {
+    if (idx >= symbols.size()) {
+      throw std::runtime_error(std::format(
+          "ConstitutiveModel '{}': StateVariable '{}' carries {} index "
+          "{} which is out of bounds for symbols() (size {}). State "
+          "variable / symbol vectors are out of sync.",
+          model.name(), sv.name, which, idx, symbols.size()));
+    }
+    auto const &s = symbols[idx];
+    if (s.name != expected_name) {
+      throw std::runtime_error(std::format(
+          "ConstitutiveModel '{}': StateVariable '{}' {} index points "
+          "to SymbolDecl named '{}'; expected '{}' (derived from "
+          "StateVariable.name). The symbols() entry has been renamed or "
+          "the indices have shifted.",
+          model.name(), sv.name, which, s.name, expected_name));
+    }
+    if (s.category != expected_cat) {
+      throw std::runtime_error(std::format(
+          "ConstitutiveModel '{}': StateVariable '{}' {} symbol has "
+          "wrong Category (expected={}, got={}). State variable / symbol "
+          "vectors are out of sync.",
+          model.name(), sv.name, which,
+          static_cast<int>(expected_cat),
+          static_cast<int>(s.category)));
+    }
+    if (s.kind != sv.kind) {
+      throw std::runtime_error(std::format(
+          "ConstitutiveModel '{}': StateVariable '{}' {} symbol has Kind "
+          "mismatched against the state-variable record (expected={}, "
+          "got={}).",
+          model.name(), sv.name, which,
+          static_cast<int>(sv.kind),
+          static_cast<int>(s.kind)));
+    }
+    if (sv.kind == SymbolDecl::Kind::Tensor &&
+        (s.dim != sv.dim || s.rank != sv.rank)) {
+      throw std::runtime_error(std::format(
+          "ConstitutiveModel '{}': StateVariable '{}' {} symbol has "
+          "dim/rank ({}, {}) mismatched against the state-variable "
+          "record ({}, {}).",
+          model.name(), sv.name, which, s.dim, s.rank, sv.dim, sv.rank));
+    }
+  };
+  for (auto const &sv : model.state_variables()) {
+    check(sv.current_symbol_idx, sv.name,
+          SymbolDecl::Category::StateVariableCurrent, sv, "current");
+    auto const old_name =
+        std::string{sv.name} + std::string{state_variable_old_suffix};
+    check(sv.old_symbol_idx, old_name,
+          SymbolDecl::Category::StateVariableOld, sv, "old");
+  }
+}
+
 // ─── Function-frame rendering (free functions) ───────────────────────
 //
 // These render the surrounding function frame after the body statements
@@ -750,6 +880,22 @@ inline auto render_compute_function(
 inline void SymbolValidationPass::run(PassContext &pctx) {
   auto const &model = pctx.model;
 
+  // Reset the conditional-postcondition flag on entry — PR #66 round-2
+  // review #2. Two failure modes leak prior state without this:
+  //   (a) `verify_state_variable_symbol_alignment` below throws → the
+  //       end-of-run assignment never executes → the flag retains its
+  //       previous value;
+  //   (b) the same pass instance is reused across two recipes with
+  //       different state-variable counts → state advertised between
+  //       calls reflects the prior recipe.
+  // (A pre-`run()` query on a pristine pass instance is already covered
+  // by the default-initialiser at `symbol_validation_pass.h:130` — the
+  // reset-at-entry here does not need to address that case.)
+  // Resetting up-front guarantees `m_state_variables_non_empty` always
+  // reflects the *current* run; the real value is computed after
+  // validation succeeds at the bottom of this function.
+  m_state_variables_non_empty = false;
+
   // P4 (post-fixup C1): build the name → index lookup once. Indices
   // survive any future `m_symbols.push_back()` reallocation, whereas
   // the original SymbolDecl pointers would silently dangle the first
@@ -845,6 +991,18 @@ inline void SymbolValidationPass::run(PassContext &pctx) {
            "before referencing a symbol in an output expression.";
     throw std::runtime_error(msg);
   }
+
+  // (3) State-variable alignment invariant (issue #59 / PR #66 review).
+  // Throws if any StateVariable's `current_symbol_idx` / `old_symbol_idx`
+  // points at a mismatched SymbolDecl. Today's public API can't produce
+  // a violation, but running the check here means Phase 2.2+ mutating
+  // passes that touch state variables can re-run SymbolValidationPass
+  // to re-verify between stages — the invariant isn't bypassable by
+  // running passes outside `ConstitutiveModel::validate()`.
+  verify_state_variable_symbol_alignment(model);
+
+  // Drive the conditional `state_variables_non_empty` postcondition.
+  m_state_variables_non_empty = !model.state_variables().empty();
 }
 
 inline void TensorSpaceConsistencyPass::run(PassContext &pctx) {