Modernize: std::expected for find_tensor_symbol; deliberate skip of try_mutable_model (issue #62 item 1)

[petlenz]

Item 1 of #62. Converts `find_tensor_symbol` from nullable pointer to `std::expected`, distinguishing its two genuine failure modes. Deliberately skips `try_mutable_model` per the analysis below.

What changed

`find_tensor_symbol` → `std::expected<SymbolDecl const *, LookupError>`

Pre-modernization the helper returned `nullptr` for two semantically distinct cases:

Failure	What it means
Name not in `pctx.symbol_lookup`	The recipe never declared this symbol at all
Name found, but `SymbolDecl::Kind != Tensor`	The recipe declared this symbol, but it's scalar/parameter

The caller couldn't tell them apart without re-inspecting the model. New API:

```cpp
enum class LookupError { NotFound, WrongKind };

[[nodiscard]] inline auto find_tensor_symbol(PassContext const &pctx,
std::string const &name) noexcept
-> std::expected<SymbolDecl const *, LookupError>;
```

The single call site in `TensorSpaceConsistencyPass::run` was updated; the negative-path test in `PassFrameworkTest.FindTensorSymbolResolvesByName` now asserts the specific `LookupError` value rather than just `== nullptr`. Two test sites in `StateVariableTest` updated for the new return-type shape.

`try_mutable_model` deliberately NOT changed

The issue listed both `find_tensor_symbol` and `try_mutable_model` as candidates. I'm doing only the former. Reason:

`try_mutable_model` has exactly one failure mode — the RecipeView was constructed from a const reference. `std::expected<ConstitutiveModel*, RecipeViewError>` would always have `RecipeViewError = ConstView` on failure, providing zero information beyond what `nullptr` already conveyed. It's `std::optional<ConstitutiveModel*>` with extra ceremony.

The `try_*` naming convention also already signals "nullable return; caller checks." And the project already has `require_mutable_model(pass_name)` for callers who want "give me the model or throw." Adding `expected` here would be modernization for its own sake.

I'd note the same reasoning applies to the OTHER nullable-return APIs the codebase already has:

• `find_input_by_role` / `find_output_by_role` — single failure mode (not found)
• `CodeGenContext::find` / `::find_named` — single failure mode (pointer key absent)

These all stay as nullable-pointer returns. `std::expected` is the right tool when failure has structured variants worth distinguishing; for binary present/absent the existing idiom is clearer.

Why this is still worth landing

`find_tensor_symbol` is the only API in the codebase today where a caller might want to react differently to "not found" vs "wrong kind." The `TensorSpaceConsistencyPass` site collapses both into "continue," but a future pass (e.g. Phase 2.2's `TimeIntegrationPass`) might want to throw on `NotFound` while silently skipping on `WrongKind`. The expected type makes that distinction available without rebuilding the lookup.

Also: `std::expected` is C++23's standard answer to Rust's `Result`. Establishing the pattern for one API gives future Phase 2/3 fallible operations a reference point.

Verification

135/135 tests pass. Three test sites updated to consume the new return type:

• `PassFrameworkTest.FindTensorSymbolResolvesByName` — now asserts `LookupError::WrongKind` for the scalar-named case and `LookupError::NotFound` for the unknown-name case (previously both were `== nullptr`).
• `StateVariableTest.FindTensorSymbolResolvesTensorStateVarBothHandles` — switched to `.has_value()` + `(*result)->` access.

Migration notes

Breaking change for any consumer that called `find_tensor_symbol` and got back `SymbolDecl const *`. Migration:

```cpp
// before
if (auto const *decl = find_tensor_symbol(pctx, name)) {
use(*decl);
} else {
// some failure
}

// after
auto result = find_tensor_symbol(pctx, name);
if (result.has_value()) {
use(**result);
} else {
switch (result.error()) {
case LookupError::NotFound: handle_not_found(); break;
case LookupError::WrongKind: handle_wrong_kind(); break;
}
}
```

No external consumers today; this is the right window for the migration.

Stack

`main ← this`. Independent of PRs #63 (std::format) and #64 (std::span); can land in any order.

Issue #62 status after this PR

• ✓ Item 1 (this PR): std::expected for find_tensor_symbol
• ✓ Item 2 (Modernize: convert error-message string concat to std::format (issue #62 item 2) #63): std::format for error messages
• ✓ Item 3 (Modernize: switch read-only accessors from vector const& to std::span (issue #62 item 3) #64): std::span for read-only accessors

#62 closes once all three merge.

[petlenz]

Second-pass review fixup (commit 43482af): addresses five minor items from REVIEW-pr-62-round2.md — all documentation polish, no behaviour change:

• m1 — softened README 'Clang 18 is not supported' to 'not in CI matrix' (technically correct: clang-18 + manually-installed libstdc++-14 would work, but isn't CI-tested) + added a toolchain-verification one-liner
• m2 — added find_* vs try_* naming clarifier to docs/workflow.md §6.1 (acknowledges that find_input_by_role etc. also use nullable-return)
• m3 — added 'AND act on differently' clause to §6.1's distinguishability rule (covers the case where ≥2 modes exist but no caller branches on them)
• m4 — dropped hardcoded clang-19 version from §6.2; cross-reference to build.yml as source of truth so version bumps don't rot the doc
• n1 — tightened try_mutable_model 'ceremony' wording to 'adds machinery without adding distinguishable failure information' (more precise about what std::expected<T, SingleErrorEnum> actually loses)

Earlier fixup (commit 51ade82) addresses round-1 C1 (CI bump to clang-19 + libstdc++-14), M3 (try_mutable_model rationale), m5 (LookupError reuse), m6 (workflow.md §6 convention doc), README compiler baseline.

CI green on the new matrix; 135/135 tests. Full audit trail in REVIEW-pr-62.md (round 1) + REVIEW-pr-62-round2.md (round 2).