PHIR classical engine: error-swallowing and fail-fast gaps in secondary execution paths

[ciaranra]

Summary

An independent multi-reviewer audit of PR #339 (fresh-context + cross-model arms) surfaced several pre-existing (present on dev, not introduced by #339) error-handling / fail-fast gaps in the PHIR classical execution engine (pecos-phir-json). None affect the primary interpreter correctness surface — the signed i(S+1) / unsigned u(S) register model and Rust↔Python bit-for-bit parity are sound and covered by tests. Each item below is a latent silent-corruption or guard-bypass confined to a secondary/engine/fallback path. Filing as a follow-up; these did not block #339.

Line numbers reference the binary-str-sign-fix branch head; the same patterns exist on dev at nearby lines. Grep by the named symbol for the current location.

1. Engine swallows variable-definition errors → invalid signed register silently becomes i32 (Medium)

PhirJsonEngine discards handle_variable_definition results with let _ =:

• crates/pecos-phir-json/src/v0_1/engine.rs:157 (constructor) and :284 (main loop)

A rejected definition (e.g. signed size == 64, which Environment::add_variable correctly rejects) is therefore ignored. A later whole-register assignment auto-creates the missing target as DataType::I32 at crates/pecos-phir-json/src/v0_1/operations.rs:1061-1062, so the invalid register silently becomes a signed 32-bit value instead of failing fast.

Repro (engine path):

{"format":"PHIR/JSON","version":"0.1.0","ops":[
  {"data":"cvar_define","data_type":"i64","variable":"x","size":64},
  {"cop":"=","returns":["x"],"args":[9223372036854775807]},
  {"cop":"Result","args":["x"],"returns":["out"]}
]}

Expected: fail fast (invalid signed width). Actual (engine path): define error dropped; x recreated as i32. Only reachable via hand-written PHIR — generators emit unsigned u32/u64. The interpreter path does reject it (tested); the engine path does not.

2. Measurement store logs-and-continues on out-of-range instead of failing fast (Medium)

store_measurement_result (crates/pecos-phir-json/src/v0_1/operations.rs:1672-1702) logs and returns on add_variable failure (:1689) and set_bit failure (:1697) rather than propagating an error. Measure q[0] -> m[70] creates m as U64 size 71 (rejected), set_bit fails, both are logged, and execution continues; if no later Result reads m, the write is silently lost. This contradicts the fail-fast-on-out-of-range behavior that the Result export path does honor (process_result_op propagates via ?).

3. Batch-local measurement index aliases across batch splits (Low-Medium)

handle_measurements (crates/pecos-phir-json/src/v0_1/operations.rs:1713-1758) enumerates with a batch-local index that resets each batch (MAX_BATCH_SIZE = 100, or on a deferral split). The measurement_{index} variables (:1725) and the no-return "m" fallback (:1753) use it as an absolute bit position, so measurements split across batches overwrite measurement_0.. / m[0..]. The primary explicit-return path (:1748-1749) stores to the absolute (var_name, var_idx) from pending_measurements and is correct.

Repro sketch: measure q0 (no return) → ~100 gates → measure q1 (no return); batch 2's index restarts at 0 and overwrites m[0], leaving m[1] unset. No test covers the MAX_BATCH_SIZE boundary.

Minor (not reachable via generated PHIR)

Python PHIR &&/|| do not short-circuit and there is no unary logical !, while Rust expression.rs implements both. Only observable with a hand-written ! op or &&/|| over an erroring/side-effecting operand; generated PHIR uses ~ and pure operands, so shared programs do not diverge.

Provenance

Confirmed against code at each cited file:line and confirmed present on dev. Surfaced while reviewing PR #339; kept out of that PR's scope since they are pre-existing and orthogonal to it.