fix: add RNGadvance support to legacy Python CVM

[qartik]

Summary

• add RNGadvance(delta) support to the legacy Python CVM RNG path, including backward replay through historical bound sequences
• preserve shot-local rewind correctness by resetting replay state at shot boundaries instead of mutating count directly
• keep existing RNGseed, RNGindex, and RNGnum behavior while tightening RNG argument validation and stream bookkeeping
• include the CI hardening and Julia formatter-only updates already present on this branch

Reviewer Notes

• The behavior change is in python/quantum-pecos/src/pecos/engines/cvm/rng_model.py and the legacy caller in python/quantum-pecos/src/pecos/engines/hybrid_engine_old.py.
• Backward RNGadvance now replays from an explicit replay base: reseeds reset that base, and each shot snapshots the current RNG state so rewinds stay correct even when the stream continues across shots.
• Bound history is stored in run-length encoded form to reduce memory growth for long runs with stable bounds.
• The Rust binding adds RngPcg.clone() only to snapshot the current generator state for shot-local replay.
• Workflow and Julia file changes are CI fixes, not product behavior changes.

Validation

• uv run maturin develop --manifest-path python/pecos-rslib/Cargo.toml
• uv run --frozen pytest python/quantum-pecos/tests/pecos/unit/test_rng.py
• uv run --frozen ruff check python/quantum-pecos/src/pecos/engines/cvm/rng_model.py python/quantum-pecos/src/pecos/engines/hybrid_engine_old.py python/quantum-pecos/tests/pecos/unit/test_rng.py

[Copilot]

Pull request overview

Adds relative RNG stream movement (RNGadvance(delta)) to the Python PECOS CVM RNG model, including deterministic rewind behavior by reconstructing from the active seed, and expands unit coverage to validate forward/backward advances and reseed/count semantics.

Changes:

• Implement RNGadvance via set_relative_index(delta) with support for negative deltas (rewind by reseeding + replaying to target index).
• Fix RNGModel initialization to avoid overwriting seed with None, and reset count on reseed to keep stream position tracking consistent.
• Extend Python unit tests to cover forward advance, backward advance, reseed semantics, and count tracking.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
python/quantum-pecos/src/pecos/engines/cvm/rng_model.py	Adds relative index movement (RNGadvance) and updates seed/count handling + argument parsing for signed integers.
python/quantum-pecos/tests/pecos/unit/test_rng.py	Adds unit tests validating forward/backward relative movement, reseed behavior, and count consistency.
julia/PECOS.jl/src/Simulator.jl	Adjusts field docstring formatting for MeasurementResult.
julia/PECOS.jl/src/Decoder.jl	Adjusts field docstring formatting for DecodingResult.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

[ciaranra]

Claude's review:

Reviewed the RNG changes. The Python RNGModel logic is sound — the reseed-and-replay rewind is correct precisely because pcg32_boundedrand_r uses rejection sampling (variable underlying steps), so replaying the historical bound sequence is necessary; tests pass; it also fixes a latent self.seed = None init bug.

One scoping note for follow-up: RNGadvance lands only on the legacy Python cvm RNGModel (reached via hybrid_engine_old.py). The newer workflows — hybrid_engine.py (RustPhirClassicalInterpreter), sim() (pecos-qasm / phir-json), and sim_neo() — don't route program RNG through it, and the Rust RNGModel has no advance. Filed #332 to track landing program RNG once in the canonical Rust RNGModel that the pecos-neo path will consume, rather than back-porting into the engines slated for retirement.

Minor: in set_relative_index, the rewind loop's if remaining == 0: break should be <= 0 — when the target lands mid-run, remaining goes negative and the loop iterates the rest of the bound history doing empty draws (harmless today, just wasteful).

[ciaranra]

Consolidated review (Claude + Codex)

Two-model review of the RNG changes. Net: the core algorithm is sound, but the
PR introduces a real correctness bug on its one live path, and the scope is
narrower than the title. Recommend merge as a scoped legacy-path fix after two
fixes + a rename.

What's correct

• The reseed-and-replay rewind is the right mechanism for this generator:
  pcg32_boundedrand_r uses rejection sampling (crates/pecos-random/src/rng_pcg.rs:58-66),
  so a bounded draw consumes a variable number of underlying PCG steps and there
  is no jump-ahead — replaying the exact historical bound sequence is necessary.
  The run-length _draw_bound_runs design holds up: adversarial sequences
  (bound changes, high-rejection bounds, RNGindex, mid-run rewinds,
  run-boundary rewinds, rewind-to-0, reseed, rewind-then-forward, plus 100
  randomized op sequences) all matched a fresh seeded PCG replay.
• Also fixes a latent init bug (self.seed = self.set_seed(...) previously
  stored None).

Blocking: per-shot count reset desyncs the replay state (live default engine)

hybrid_engine_old.py:144 resets self.rng_model.count = 0 at each shot start
without clearing _draw_bound_runs or reseeding. Before this PR count was a
bare index counter; this PR couples it to _draw_bound_runs and the replay
base. So a multi-shot program that calls RNGadvance(-k) without an in-shot
RNGseed rewinds against stale shot-1 bound history and the seed-reset PCG, and
diverges.

This is not a dead path: pecos/__init__.py:269 imports the top-level public
pecos.HybridEngine from hybrid_engine_old, so this is the default engine.

Repro — invariant: draw forward, advance(-1), redraw should reproduce the
value just drawn. Verified independently by both reviewers:

# clean public API (no legacy reset): round-trips correctly
control: advance(-1) then redraw == last draw  -> True

# with the legacy per-shot `count = 0` (shot >= 2, no in-shot RNGseed):
shot2 forward draws : 706518008 257766219 1139787578   (count -> 3)
after advance(-1)   : 1868747362
matches last draw?  : False   # reconstructs from seed (PCG idx 2), not the
                              # continued stream the forward draws used (idx 7)

Fix: add a public shot-reset / replay-base API and have the engine call it
instead of mutating count directly — choose one explicit semantic (reseed +
clear history per shot, OR snapshot a per-shot replay base). Add a regression
test for multi-shot RNGadvance without an in-shot RNGseed.

Minor: rewind loop early-exit

set_relative_index subtracts the full run_length (rng_model.py:109) while
breaking only on remaining == 0 (line 104), so a mid-run target drives
remaining negative and the loop iterates the rest of the history doing empty
draws. Harmless today (empty range, current_bound restored), but should be
to_replay = min(run_length, remaining); remaining -= to_replay; break on <= 0.

Scope / cross-engine

• The live sim() (QASM → Rust pecos-qasm, PLATFORM_FUNCTIONS = seed/bound/
  index/num, no advance) and sim_neo() paths do not route through this
  Python CVM RNGModel. So this is not general "RNGadvance support to PECOS" —
  it's the legacy Python CVM path only. Suggest renaming the PR accordingly.
• Cross-engine drift: this PR makes Python set_seed reset count + clear
  history (correct for a platform RNGseed), but Rust RNGModel::set_seed
  (rng_pcg.rs:311-313) does not reset count. Track parity + the canonical
  program-RNG home in Program RNG platform functions (incl. RNGadvance) need a canonical home in the pecos-neo path, not the retiring engines #332.
• (FYI) HUGR has a separate tket.qsystem.random mechanism that already
  includes RandomAdvance (crates/pecos-hugr/src/engine/handlers/qsystem.rs:288);
  unrelated to this OpenQASM-style platform RNG.

Recommendation

Merge as contained legacy Python CVM RNGadvance support after: (1) fixing the
per-shot reset invariant, (2) the <= 0 loop fix, (3) renaming/rewording the PR
to the legacy scope. Track Rust set_seed parity + real neo program-RNG in #332.

Corrections to an earlier review comment on this PR

The earlier comment said hybrid_engine.py uses RustPhirClassicalInterpreter
and framed the RNG path as a dead legacy one. Both are off: hybrid_engine.py
defaults to the Python PhirClassicalInterpreter (Rust is opt-in via
cinterp="rust"), and the RNGadvance path is the default public
pecos.HybridEngine, not dead — which is exactly why the Area 1 bug matters.

---

Reviewed by Claude Opus 4.8 and Codex (adversarial cross-review).

[qartik]

@ciaranra could you review again?

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

[ciaranra]

Sign-off: fix verified, mergeable (Claude + Codex)

Re-reviewed c1a612bc5 ("preserve shot-local RNG rewind state"). Two-model
adversarial pass; both converge: mergeable as the scoped legacy Python-CVM
RNGadvance fix. Thanks for the quick, well-targeted turnaround.

The round-1 blocker is closed

The per-shot count = 0 reset on the default public pecos.HybridEngine no
longer desyncs the rewind: start_shot() now resets count + _draw_bound_runs
and captures a _replay_base_pcg clone at the current stream position, and
backward advance replays from that clone instead of reseeding from self.seed.
Verified on the built tip — the exact failing case now round-trips:

start_shot(1); draw,draw,draw -> [..., 1139787578]
advance(-1); draw            -> 1139787578   (was 1868747362 pre-fix)

A bound-change-within-shot rewind also round-trips. The Area 2 loop fix
(remaining <= 0 + remaining -= replay_count) is exactly right, and the PR
rename to the legacy-CVM scope matches reality.

What was checked

• Full RNG unit suite: 15 passed (incl. the new multi-shot test).
• Replay-base capture points (mid-shot reseed, bound-before-shot, set_index
  after start_shot), clone state-independence through PyO3, single-shot
  equivalence to the old set_seed(self.seed) path, loop arithmetic
  (exact-boundary / mid-run / rewind-to-zero / forward-after-rewind).
• A randomized oracle harness (200 sequences × 180 ops, bounds incl. 0, 65537,
  2147483649) comparing post-rewind state to an independent PCG replay — passed.
• Blast radius: the only other rng_model.* access is a shot_id read; the
  multiprocessing legacy path also routes through eng.run() → start_shot().

Non-blocking follow-ups

• Optional hardening tests worth adding: one combining set_bound +
  start_shot + set_index + backward advance in a single case, and a
  no-start_shot property test locking the single-shot non-regression. A
  one-line doc on the shot-local contract (rewind stays within the current
  shot/seed segment; current_bound carries across shots) would help too.
• Rust RNGModel::set_seed count-reset parity and real program-RNG for the
  surviving sim() / sim_neo() paths remain tracked in Program RNG platform functions (incl. RNGadvance) need a canonical home in the pecos-neo path, not the retiring engines #332 — out of scope
  for this legacy fix.

LGTM.

---

Reviewed by Claude Opus 4.8 and Codex (gpt-5.5) — adversarial cross-review.

[ciaranra]

Looks good. Thanks!

[qartik]

@ciaranra I can't merge, I am seeing:

Commits must have verified signatures.

[ciaranra]

Huh, weird... Maybe it is some added security I added... I'll try merging