Code review follow-up: Med & Low findings (umbrella)

[petlenz]

Umbrella for the Medium and Low items from the multi-lens code review (see `REVIEW.md` at the repo root).

Critical/High items are tracked individually as separate issues. The remainder is grouped here for visibility; cherry-pick into PRs as appetite allows.

Medium — correctness & generated-code soundness

• `wrap_if_compound` heuristic is fragile — `is_single_token` accepts leading `-`, enabling future emission of `--x` once a new node type bypasses the CAS simplifier. Switch to an explicit `EmitResult { string text; bool needs_parens; }` (REVIEW.md → CORR-B1). Locations: `scalar_code_emit.h:228-237`, `tensor_code_emit.h:162-168`, `tensor_to_scalar_code_emit.h:177-184`.
• Three byte-identical copies of `register_temp` / `is_single_token` / `wrap_if_compound` across the three emitters. Extract into `code_emit/emit_utils.h` or a CRTP mixin so CORR-B1's fix lives in one place (REVIEW.md → CORR-B2).
• *`void const ` CSE lifetime invariant undocumented at `codegen_context.h:117`. Today it's safe because contexts are stack-local; the next backend author could share one. Document the invariant; consider `[[clang::lifetimebound]]` on `register_symbol_*` (REVIEW.md → CORR-B3).
• Identifier validation missing in `add_*` (`recipe.h:94-150`). Reject C++ keywords, leading digits, names ending in `_out`. Couples naturally with the duplicate-detect work in Generated tN temporaries collide with user symbol names #8 (REVIEW.md → CORR-B4).
• `StandaloneCxxTarget` doesn't include `` but the emitter produces `std::complex` constants when CAS has them. Tests transitively pull it via tmech. Either include unconditionally or track "complex constant emitted" in `CodeGenContext` (REVIEW.md → CORR-B5).
• Error messages don't name the offending output — `TensorCodeEmit::apply called on invalid expression` and stub-throw messages don't say which `OutputDecl` triggered them. Thread the current output name through emitters in `recipe.h:226-237` (REVIEW.md → CORR-B6).

Medium — architecture

• `MooseMaterialTarget` god-class (`moose_material.h:30-279`, ~280 lines, five concerns). Split into `MooseTypeMap`, `MooseHeaderWriter`, `MooseSourceWriter`, `TmechBoundaryAdaptor`. The last one is reusable for future Abaqus/USERMAT backends (REVIEW.md → ARCH-B1).
• `Target::emit()` is the only contract (`target.h:33-45`). Add `validate_recipe(model) const` and `capabilities() const` hooks. Converts late string-synthesis failures into early structural failures and is the home for MOOSE adaptor ignores tensor dim — silent miscompute when dim != 3 #7's dim check (REVIEW.md → ARCH-B2).
• Two parallel symbol stores (`recipe.h:377-387`). `m_symbols` / `m_parameters_cache` / `m_inputs_cache` / `m_scalar_symbols` / `m_tensor_symbols` maintained in lockstep by every `add_*`. One missed push_back desyncs silently. Consolidate (REVIEW.md → ARCH-B3).
• `emit_compute_function` returning `std::string` locks out non-C++ backends (`recipe.h:209-241`). Split into `compile() → CompiledModel` + a C++ render step. Largest refactor; defer until backend Add AnsysUSERMATTarget backend #3 is on the horizon (Add AbaqusVUMATTarget backend (vectorized UMAT) #2, Add AnsysUSERMATTarget backend #3, Add LSDynaUMATTarget backend #4 in the existing backlog) (REVIEW.md → ARCH-C1).
• Flat role enums force every backend to switch (`recipe.h:29-47`). Replace with attribute-bearing `RoleSemantics` records. Defer (REVIEW.md → ARCH-C2).
• `StandaloneCxxTarget` accepts `InputRole::History` silently (`standalone_cxx.h:14-30`). Asymmetric with MOOSE's rejection. Document the Phase A contract or reject symmetrically (REVIEW.md → ARCH-C3).

Low / Nit

• No SFINAE/concepts on generated template parameters (`recipe.h:299-305`). Wrong-type arguments produce a tmech expression-template error wall. A `concept tmech_tensor_like<T, Dim, Rank>` would give actionable diagnostics. Defer until tmech itself exposes one (REVIEW.md → CPP-D1).
• Dead `return "input";` after exhaustive switch in `input_role_doc` (`moose_material.h:274-275`). Remove and rely on `-Wswitch` (REVIEW.md → CPP-D2).
• CSE counter monotonicity contract is only documented in a comment inside `reset()`. Add a class-level invariant on `CodeGenContext` (REVIEW.md → CPP-D3).
• `tests/generated/CMakeLists.txt` add_custom_command explicitly lists the generator target as DEPENDS; also list the source file path for belt-and-braces (REVIEW.md → CPP-D4).
• `find_input_by_role` / `find_output_by_role` return first match silently (`recipe.h:254-272`). Document "first match wins" or return a vector (REVIEW.md → CORR-D5).
• `examples/linear_elasticity.cpp` is a partial example by design (Lamé form not emittable yet). Move the limitation note from the file header to README, or rename the example (REVIEW.md → CORR-D6).

Refs

• Full review: `REVIEW.md` at repo root
• Severity: Med and below — none of these are silent-wrong-code bugs; cherry-pick into refactor PRs

[petlenz]

ARCH-C2 (flat role enums → attribute-bearing `Role` struct) landed in 4d95600.

The `InputRole` / `OutputRole` enums were replaced with a single `Role` value type carrying name + `is_stateful` / `is_driving` / `is_symmetric` / `expected_rank` attributes. Backends now switch on attributes, not identity, so user-defined custom roles flow through transparently. MOOSE's Phase A guard now triggers on `is_stateful` rather than the specific `InputRole::History` literal.

Remaining items in this umbrella are unchanged.

[petlenz]

Promoted two more items out of this umbrella into dedicated issues:

• ARCH-B1 (god-class `MooseMaterialTarget` split into `MooseTypeMap` / `MooseHeaderWriter` / `MooseSourceWriter` / `TmechBoundaryAdaptor`) → Extract reusable C++ FEM-framework scaffolding (CxxFrameworkTarget + TypeMap + TmechBoundaryAdaptor) #19. Generalised to a `CxxFrameworkTarget` template since the same scaffolding is needed for the planned Abaqus / Ansys / LS-DYNA backends (Add AbaqusVUMATTarget backend (vectorized UMAT) #2, Add AnsysUSERMATTarget backend #3, Add LSDynaUMATTarget backend #4).
• ARCH-C1 (`emit_compute_function` returning `std::string` blocks non-C++ backends) → Split Layer 2 into language-agnostic compile() + C++ render step (unlocks non-C++ backends) #18. Multi-day refactor, deferred until needed.

Also new: a separate user-extensibility issue (#17) for a per-cas-node emitter registry. Out of scope of this umbrella originally because it's an additive flexibility win rather than a refactor of existing code.

Remaining items in this umbrella are unchanged. The architecture section now has only:

• ARCH-B2 — `Target::validate_recipe()` / `capabilities()` hooks (still valuable; partly satisfied by MOOSE adaptor ignores tensor dim — silent miscompute when dim != 3 #7's local dim check)
• ARCH-B3 — two parallel symbol stores (drift risk; nice-to-have)
• ARCH-C3 — `StandaloneCxxTarget` accepts History silently (asymmetry; superseded by MOOSE Phase A guard accepts stateful outputs silently (only inputs checked) #15's stateful-output question)

[petlenz]

Several stub-implementation items in this umbrella (CORR-B1, CORR-B2, CORR-B3, the missing emit cases) are closed by #30 (Phase 1.1 of the NumSim compiler epic #28). The Med architecture items (ARCH-B2 validate_recipe hook, ARCH-B3 single symbol store, ARCH-C3 standalone History symmetry) remain scoped here as standalone refactors.

[petlenz]

Status (evidence sweep 2026-08-02): 5 of 18 items done — CORR-B4 (identifier validation, recipe.h:322 + SymbolValidationPass), ARCH-C2 (Role struct, PR #16), CPP-D2 (dead role_doc removed), CORR-D6 (example retitled), ARCH-B1/C1 promoted to #19/#18. Still undone, verified: CORR-B1 (EmitResult{text,needs_parens} — is_single_token/wrap_if_compound still string heuristics ×3 emitters), CORR-B2 (no code_emit/emit_utils.h — register_temp/is_single_token/wrap_if_compound triplicated), CORR-B3 (CSE pointer-lifetime contract undocumented, codegen_context.h:34-46), CORR-B5 ( never emitted by StandaloneCxx), CORR-B6 (generic emitter error messages), ARCH-B2 (superseded by #137 can_emit — in flight), ARCH-B3 (dual symbol stores — also #144a), ARCH-C3 (StandaloneCxx accepts is_stateful silently — asymmetric with MOOSE's guard), CPP-D1 (no concepts on generated template params), CPP-D3, CPP-D4 (custom_command DEPENDS), CORR-D5 (find_*_by_role first-match undocumented).