tech-debt-backlog §7.10: simplify Q-R scaffolding in below-atol guard test

[jascal]

Summary

• Closes §7.10 in openspec/changes/tech-debt-backlog/tasks.md (PR tech-debt-backlog §7.5: defensive rank-≤2 guard in _apply_cnot #73 review nit: "simplify unused U_oracle/Vh_oracle setup in test 3").
• Replaces the two np.linalg.qr calls and M = U_oracle @ S_oracle @ Vh_oracle product in test_below_atol_discard_does_not_raise (tests/test_concept_gram_mps_contraction.py) with a one-line M = np.diag([1.0, 0.5, 1e-12, 0.0]).astype(complex).
• A diagonal (4, 4) matrix has its diagonal entries as its singular values, so the prescribed [1.0, 0.5, 1e-12, 0] spectrum lands by construction — no randomness, no Q-R unitaries, no sanity assertion needed. The downstream CNOT inverse-permutation is unitary and preserves singular values through to _apply_cnot's SVD step (the existing inline comment already noted this invariance).
• Net: ~10 lines deleted, 1 added. The other two tests in TestApplyCnotBondTruncationGuard are unchanged — rank-2 pass-through and rank-3 raise keep their seeded-Gaussian construction and oracle-SVD magnitude pin respectively.

Surface audit

The task body pointed at test_rank_three_input_raises_with_named_discard, but the Q-R block actually lives in test_below_atol_discard_does_not_raise (lines 336-345). The rank-3 raise test already uses a seeded Gaussian directly and pins the discarded magnitude against an oracle SVD — nothing to simplify there.

Test plan

• pytest tests/test_concept_gram_mps_contraction.py::TestApplyCnotBondTruncationGuard -v — all 3 guard tests pass (rank-2 pass-through, rank-3 raise, below-atol silent).
• pytest tests/test_concept_gram_mps_contraction.py -q — 39 passed, 1 skipped (file-wide regression).
• pytest -q — full suite green at 1282 passed, 8 skipped.

🤖 Generated with Claude Code

[jascal]

Code Review — Claude Sonnet 4.6

Verdict: LGTM — correct, well-scoped refactor. Recommend merge after a one-line comment tweak (non-blocking).

A clean tech-debt simplification: the RNG + dual-QR scaffolding in test_below_atol_discard_does_not_raise is replaced with a single M = np.diag([1.0, 0.5, 1e-12, 0.0]).astype(complex). I traced the math and ran the suite — both confirm equivalence.

Correctness ✅

I verified the round-trip end to end against _apply_cnot (q_orca/compiler/mps_contract.py:129):

1. A diagonal (4,4) matrix has |diagonal entries| as its singular values, so M's spectrum is exactly [1.0, 0.5, 1e-12, 0.0] — identical to the old U_oracle @ S_oracle @ Vh_oracle (unitary · diag · unitary is an SVD). The guard only inspects the spectrum, so the construction is faithful.
2. The test plays M as the post-CNOT tensor T2, reverse-applies the CNOT swap to get T, then SVD-factors T keeping all 4 singular values — so A_c @ A_t reconstructs T exactly.
3. Inside _apply_cnot, the forward CNOT permutation (mps_contract.py:148-151) is the exact inverse of the test's pre-swap, so it lands back on M bit-for-bit (modulo ~1e-15 round-off). The guard at mps_contract.py:163-166 then sees discarded = [1e-12, 0.0], max_discarded = 1e-12 < atol 1e-10 → no raise. The shape[2] == 2 / shape[0] == 2 asserts hold (chi = min(2, 4) = 2).

The pinned third singular value (1e-12) sits ~3 orders above the intermediate-SVD round-off floor (~1e-15) and ~2 orders below the guard's atol (1e-10), so the test keeps a healthy margin on both sides — it remains robust, not brittle, after the change.

One nit — comment precision (non-blocking)

The new comment reads:

The CNOT inverse-permutation below is unitary, so the singular values propagate through to _apply_cnot's SVD step unchanged.

The conclusion is correct (the guard does see M's spectrum), but the stated reason is loose. The inverse-CNOT step is a permutation of tensor entries, i.e. a row-dependent column swap on the reshaped (4,4) — that is not a unitary left/right multiplication and does not preserve the matrix's singular values (indeed T's own spectrum differs from M's). The reason the guard recovers M's spectrum is that the test pre-applies the exact inverse of _apply_cnot's permutation and the intermediate full-rank SVD factoring is exact — so the forward permutation reconstructs M, not that singular values are invariant under the swap. Suggest something like:

# We pre-apply the inverse of _apply_cnot's CNOT swap; the full-rank SVD
# factoring below is exact, so _apply_cnot's forward swap reconstructs M
# and SVDs its [1.0, 0.5, 1e-12, 0] spectrum. (The permutation reshuffles
# entries, so it does NOT itself preserve the matrix's singular values.)

Code is correct either way — this is purely about not leaving a future reader with a wrong mental model.

Test coverage ✅

The behavior under test (sub-atol discard stays silent) is preserved exactly; this is a refactor, not new behavior, so no new edge cases are warranted. Nicely, the two sibling tests (test_rank_two_input_passes_through_cleanly, test_rank_three_input_raises_with_named_discard) are left untouched, keeping their distinct construction styles.

Lint / tests ✅

• .venv/bin/ruff check tests/test_concept_gram_mps_contraction.py → All checks passed. (The 18 repo-wide ruff errors are all pre-existing in other files — test_verifier.py E402, etc. — and are not introduced or touched by this PR.)
• pytest tests/test_concept_gram_mps_contraction.py::TestApplyCnotBondTruncationGuard -v → 3 passed.
• pytest tests/test_concept_gram_mps_contraction.py -q → 39 passed, 1 skipped.
• pytest -q (full suite) → 1282 passed, 8 skipped — matches the PR's claim exactly.

Security / performance

N/A (test-only). Mild plus: the new version drops 4 RNG seeds + 2 QR decompositions, so it's marginally faster and fully deterministic-by-construction rather than deterministic-by-fixed-seed.

What looks good

• The surface audit in tasks.md is exemplary: it caught that the task pointed at the wrong test (test_rank_three_input_raises_with_named_discard) and documented that the Q-R block actually lived in test_below_atol_discard_does_not_raise, then took the simpler path. That's the right way to handle a mislabeled backlog item.
• Net -10 / +1 with a clearer comment block, and the tasks.md [ ] → [x] flip plus rationale keeps the backlog honest.

This review was posted automatically by Claude Sonnet 4.6.