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

[petlenz]

Summary

Three hardening items from REVIEW-pr-58.md so TimeIntegrationPass (next Phase 2.2 work) can rely on a stable IR:

• Alignment invariant — ConstitutiveModel::validate_state_variable_symbol_alignment() walks m_state_variables and verifies every entry's current_symbol_idx / old_symbol_idx points at a SymbolDecl of matching name (<sv> / <sv>_old), category, kind, and dim/rank. Runs from both validate() and emit_compute_function() before the pass pipeline. Today no public API can produce a violation; this guards against Phase 2.2+ mutating passes that synthesise / replace state variables without keeping the pair aligned.
• pass_tags::state_variables_declared — SymbolValidationPass advertises it unconditionally (the tag means "checked," not "non-empty"). Phase 2.2's TimeIntegrationPass will declare it as a precondition.
• find_state_variable_by_name(PassContext, name) -> StateVariable const * — declared in passes/pass.h, defined in recipe.h after StateVariable is complete (mirrors find_tensor_symbol). Nullable return rather than std::expected per docs/workflow.md §6.1 — single failure mode.

Test plan

• 3 new StateVariablePhase22Prep tests covering each addition
  • happy-path alignment validation on a recipe mixing scalar + tensor state vars with non-trivial index offsets
  • SymbolValidationPass postconditions contain the new tag; recipes with zero state vars still satisfy it
  • find_state_variable_by_name resolves declared names, returns nullptr for misses, _old suffix, and empty string
• All previously-passing 135 tests still green (now 138/138)
• Branch builds clean on local gcc-14

Why this PR exists

TimeIntegrationPass will (a) look up Dt(α) → its StateVariable record, (b) expect the index-pointed SymbolDecls to be aligned, (c) declare a precondition on state vars being declared. Doing these three together now is much cheaper than threading them into the pass later.

Refs: issue #59, REVIEW-pr-58.md m1/m2/m3, epic #28.

[petlenz]

Three-lens critical review (architect / cpp-pro / code-reviewer) on this PR. One CRITICAL and five MAJOR findings worth addressing before merge — most are small surgical fixes (~30 LOC + 1 friend-based negative test). None invalidate the design; all are about closing the gap between what the doc-comments promise and what the code/tests actually enforce.

No CRITICAL findings would block merging if (a) the negative-path test gap is acknowledged with a TODO + a follow-up issue, and (b) the enum-mismatch messages are improved in this PR. The other MAJOR items can ship as either fixups here or a follow-up before TimeIntegrationPass starts.

Severity legend: CRITICAL = invariant defect / dead-code logic; MAJOR = contract mismatch or significant gap; everything else omitted.

[petlenz]

Round-2 three-lens review on fixup 16ac2b9. All six round-1 findings are addressed in substance — verification confirmed via reading the actual code paths + running the tests (143/143 green). This round surfaces 8 NEW or RESIDUAL MAJOR items, most introduced by the very mechanism that fixed round-1. None are blockers; all are small wording or test-shape tightenings (~50 LOC total).

The most consequential one is #3 (state-flag stale across reuse / throw paths) — a real correctness gap that 2-of-3 lenses independently flagged. The rest are documentation/test-quality items.

Severity legend unchanged. No CRITICAL findings this round.

[petlenz]

Round-2 three-lens review on fixup 16ac2b9. All six round-1 findings are addressed in substance — verification confirmed via reading the actual code paths + running the tests (143/143 green). This round surfaces 8 NEW or RESIDUAL MAJOR items, most introduced by the very mechanism that fixed round-1. None are blockers; all are small wording or test-shape tightenings (~50 LOC total).

The most consequential one is #3 (state-flag stale across reuse / throw paths) — a real correctness gap that 2-of-3 lenses independently flagged. The rest are documentation/test-quality items.

Severity legend unchanged. No CRITICAL findings this round.

[petlenz]

Round-3 three-lens review on fixup a278fee. All 8 round-2 findings + 2 OOB items addressed in substance; 143/143 tests pass. This round surfaces 7 inline MAJOR + 1 architecture MAJOR in a file this PR doesn't touch (#3 below).

One reviewer flagged a CRITICAL — FAIL() inside try{} returning before the substring asserts. On careful re-read this is correct test behaviour: the negative-substring asserts only need to fire when validation DOES throw (to pin which arm); when validation doesn't throw, FAIL() correctly catches the regression and the substring asserts are correctly skipped (no what to inspect). Net: 0 CRITICAL, 8 MAJOR.

---

#3 (out of diff scope but worth raising) — CodeEmitPass doesn't require state_variables_checked, leaving the bypass caveat real.

include/numsim_codegen/passes/code_emit_pass.h isn't touched by this PR, so I can't inline-comment it. But: verify_state_variable_symbol_alignment's bypass-caveat paragraph says "the convention is any pipeline that emits code... registers SymbolValidationPass first." That convention is codified via precondition tags for symbols_declared / identifiers_valid / tensor_space_declarations_checked (CodeEmitPass requires these → only SVP advertises → emit pipelines must register SVP). But state_variables_checked — which SVP advertises unconditionally — is NOT in CodeEmitPass's preconditions list. A custom pipeline that stubs the three identifier tags via a no-op pass and skips SVP could emit code without the alignment check. Fix (one line): add pass_tags::state_variables_checked to CodeEmitPass::preconditions(). The tag already exists; SVP already advertises it; the precondition graph then enforces what the doc only states.

---

The 7 inline items below are mostly small fixes (comment wording, diagram step, +1 test). Most concrete cross-lens findings: #1 (mode-c misattribution), #2 (lifecycle MUST-language), #5 (reuse-test gap).