2 files changed, 120 insertions, 0 deletions
diff --git a/design/preregistration.md b/design/preregistration.md
index f800461..30f731f 100644
--- a/design/preregistration.md
+++ b/design/preregistration.md
@@ -92,3 +92,55 @@ The interpretation matrix (O/H/P codes) above is NOT amended; only setup details
 - Giannou et al. (2023), "Looped Transformers as Programmable Computers", arXiv:2301.13196
 - Xu et al. (ICLR 2020), "What Can Neural Networks Reason About?", arXiv:1905.13211
 - Xu et al. (2021), "How Neural Networks Extrapolate: From Feedforward to Graph Neural Networks", arXiv:2009.11848
+
+---
+
+## Addendum 2 (2026-08-14, pre-launch — operationalization + correctness fixes)
+
+Trigger: OpenAI Codex code review (experiment repo `design/reviews/codex-review.md`).
+The interpretation matrix is unchanged; this addendum (a) locks the code thresholds that the
+prose left unquantified, and (b) records setup correctness fixes. Everything below is
+pre-launch and pre-run.
+
+### Operationalization of O/H/P codes (thresholds now locked)
+
+- **O1:** train EM ≥ 0.95 for ≥ 10 CONSECUTIVE evals (window start `sat_start`), THEN val EM
+  reaches ≥ 0.9 at eval `hi` with `hi ≥ sat_start + 10` (strictly after the window), where the
+  last eval with val ≤ 0.2 (`lo`) satisfies `hi - lo ≤ 5`.
+- **O2:** train EM ≥ 0.95 sustained AND val EM never reaches 0.9 AND final val EM ≤ 0.3.
+- **O3:** train EM ≥ 0.95 sustained AND val EM reaches 0.9 but not via the O1 pattern.
+- **O4:** train EM never sustained at ≥ 0.95 for 10 evals (setup/optimization failure).
+- **O-PARTIAL** (new — the original matrix left this region unspecified): train saturated,
+  val never reaches 0.9, final val EM in (0.3, 0.9). Reported as measurements, interpreted
+  cautiously as partial in-range generalization; NOT retrofitted into O1/O2/O3.
+- **H1:** mean steps at run end ≤ min_steps + 0.5 (collapse to the floor — see fix 2: with
+  min_steps = 2 the floor is 2, so literal "collapse to 1" is impossible by construction).
+- **H2:** mean steps ≥ K − 0.5 (never learned to halt).
+- **H3:** min_steps + 0.5 < mean < K − 0.5 AND Pearson ρ(gap-to-next-prime, steps) ≥ +0.3
+  (positive correlation = larger gap consumes more compute steps).
+- **H4:** intermediate but ρ < 0.3 (halt signal noisy/unused).
+- **P1:** ≥ 3 of {121, 143, 169, 187} wrong AND ≤ 6 total errors AND all errors within the flagged set.
+- **P2:** scattered errors not matching P1/P3/P4.
+- **P3:** probe accuracy ≥ 85% (checked after P4).
+- **P4:** > 50% of trivial-composite inputs (even or multiple of 5) wrong.
+
+### Correctness fixes (codex review)
+
+1. **Layout invariance (BLOCKER).** Inputs are now LEFT-padded to a fixed global length
+   (3 digits for [2,100]) in EVERY context — training batches, eval, greedy decoding; outputs
+   right-padded to the global length (4 incl. EOS). Previously batch-max padding made an
+   example's representation depend on its batchmates (RNN pad steps transformed the state;
+   transformer logit positions misaligned and absolute position embeddings shifted).
+   RNN pad steps are now exact no-ops. Regression tests: mixed-batch vs singleton logit
+   invariance for both models.
+2. **min_steps off-by-one.** Halting probs forced to 0 only for the first `min_steps − 1`
+   steps (was: first `min_steps`), so the earliest halt is step `min_steps` — matching
+   "execute at least min_steps steps".
+3. **integers vocab EOS alias.** Integers-mode vocab is now `next_prime(range_end) + 2`, so
+   the value token 101 (= next_prime(100)) is NOT aliased with EOS. Boundary test added.
+4. **Rerun protection.** train.py refuses to run if `metrics.csv` exists (append + header
+   would corrupt provenance). Fresh `--out_dir` required per run.
+5. **Checkpoint-selection honesty.** eval reports BOTH `best.pt` (val-selected — flagged as
+   selection-holed) and `last.pt` (unselected); probe + halting analyses use `last.pt`.
+6. **Dead config removed:** `halt_eps` (was never read).
+7. **`run_meta.json`:** python/torch/numpy versions, device, thread count recorded per run.
diff --git a/design/reviews/codex-review.md b/design/reviews/codex-review.md
new file mode 100644
index 0000000..55ca55d
--- /dev/null
+++ b/design/reviews/codex-review.md
@@ -0,0 +1,68 @@
+# Hostile reproducibility review — 2026-08-14
+
+Scope reviewed: `src/`, `tests/`, and `design/preregistration.md`.  This is a code/design audit only; no training artifacts were assumed valid. `pytest` could not be run because it is not installed in the environment.
+
+## Findings
+
+### BLOCKER — Variable-length batches change the input seen by both models
+
+`make_batch` right-pads inputs to the longest input in the batch.  In the tied RNN, `_encode` iterates across *all* padded positions and calls `_cell_step` even when `mask` zeroes the embedding/position contribution ([`src/models/rnn.py`](../../src/models/rnn.py), `_encode`). Thus a one-digit input receives additional tied transformations whenever it is batched with a two- or three-digit input.  At evaluation and greedy decoding, every example is passed as a singleton, so it receives no such transformations.  The same numerical input therefore has a batch-composition-dependent representation during training and a different representation at evaluation.
+
+The transformer has the corresponding, and more severe, alignment error ([`src/models/transformer.py`](../../src/models/transformer.py), `forward`): logits always start at `T_in - 1`. For a shorter example in a mixed-length batch, this is a **padded input position**, not that example's final real input digit. Output positions are also shifted by the batch maximum input length, so their learned absolute positional embeddings differ from singleton evaluation. `key_padding_mask` prevents padding from being a key, but does not move the prediction position or remove its residual/query representation.
+
+This silently makes both reported training behavior and validation/probe behavior depend on batching, and invalidates comparisons. Mask recurrent state updates by each sequence length and gather each transformer's first prediction at `length(x)-1` (or use a packing/layout that places each output immediately after its own input); train and evaluate with the identical layout. Add mixed-vs-singleton invariance tests for logits.
+
+### BLOCKER — The implementation assigns preregistered O/H/P labels using unregistered and incorrect rules
+
+`src/eval.py` claims to implement the locked codes but materially changes them.
+
+* **O1:** `grokking_signature` permits the ten-evaluation train-saturation window to occur anywhere, including after the validation transition. The preregistration requires train EM to be sustained first, then the transition. It also classifies an early `val >= .9` result as O1 based on an unrelated earlier low validation point.
+* **O2:** its stated condition is final validation EM `<= .3`; the code labels *any* run with `max(train) >= .95` and no `val >= .9` as O2, including a final validation EM of .31--.89. That directly contradicts O2 and hides an unclassified/ambiguous outcome.
+* **H1--H4:** `halting_report` evaluates the selected checkpoint rather than the state “by run end,” declares H1 at mean `<= min_steps + .5` (default `<=2.5`) rather than collapse to 1, and declares H3 based solely on `abs(correlation) >= .3`. It neither checks the registered intermediate 2--18 range nor evolution during training; a negative correlation is treated as desired evidence. H2 uses an unregistered `K-.5` threshold. The logged scalar batch mean cannot establish the required per-input evolution either.
+* **P1--P4:** the code invents thresholds absent from the locked matrix (`acc >= .85`, `easy_misses >=5`, and `<=6` errors). In particular P4 requires failure across the probe including easy cases, but five easy misses can be called P4 even at 95% accuracy; P3 is tested first, so it can also override that condition. The P2 catch-all does not test scattered/uniform composite errors. These labels are post-hoc classifications, not the preregistered outcomes.
+
+Do not publish any O/H/P code from this evaluator. Either make each condition faithfully operational with thresholds added in a preregistration amendment before runs, or emit measurements plus `unclassified` where the prose does not supply a testable boundary. Preserve checkpoint/step provenance in every measurement.
+
+### MAJOR — ACT `min_steps` is off by one, making the declared H1 condition impossible
+
+In `_run_compute`, loop index `t=0` represents compute step 1, but halting probability is zeroed when `t < cfg.min_steps`. With the default `min_steps=2`, probabilities for steps 1 *and 2* are forced to zero, so the earliest non-remainder halt is step 3. The returned expected steps can never collapse to 1 (or even below 3), while the preregistration calls H1 “collapse to 1.” The test only asserts `steps >= min_steps`, which does not expose this error.
+
+If `min_steps` means “execute at least two steps,” zero only the first `min_steps - 1` probabilities. Then align H1/H3 code definitions and tests with the chosen indexing convention.
+
+### MAJOR — Validation is repeatedly used for checkpoint selection and then reported as the primary held-out result
+
+There is no train/validation input overlap: `get_splits` uses one seeded shuffle and disjoint slices. Repeated next-prime *labels* across distinct inputs are inherent to the task, not input leakage. However, training evaluates the validation set every 200 updates, stops on it, and saves `best.pt` whenever its validation EM improves ([`src/train.py`](../../src/train.py), `_eval_pass`). `src/eval.py` defaults to that val-selected checkpoint and reports its validation EM as the primary result and its probe outcome. The same holdout therefore controls stopping/checkpoint selection and supplies the headline estimate; it is no longer an unbiased final estimate after hundreds of looks.
+
+The early-stopping rule is preregistered, but the report must call this a validation-selected result, not an untouched held-out test result. For confirmatory performance, freeze a predeclared checkpoint rule (for example, the first qualifying stop) and use an untouched test split or independent seeds/replications; do not select the maximum observed validation EM.
+
+### MAJOR — Rerunning a seed in place corrupts `metrics.csv` and breaks analysis provenance
+
+The output path is deterministic (`runs/<model>/seed<seed>`), but `train.py` opens `metrics.csv` in append mode and unconditionally writes a header on each invocation. A second invocation interleaves another CSV header among old rows; `grokking_signature` then attempts `float("train_em")` and fails. If the new run has no improvement, a prior `best.pt` can also remain in place. Config JSON, metrics, and checkpoints can consequently originate from different invocations.
+
+Require an empty/new run directory, or create a unique run ID and write a manifest atomically. Refuse to resume without explicit resume semantics that restore model, optimizer, step, and all RNG states.
+
+### MAJOR — Determinism is only partial and not documented as CPU-only
+
+`set_seed` seeds Python, NumPy, and PyTorch CPU RNGs, and the data shuffle has its own seeded `Random`; this is a good start. But it neither records/enforces the PyTorch version and threading environment nor enables deterministic algorithms or CUDA/cuDNN deterministic settings. Future device placement, GPU use, or nondeterministic kernels will therefore make same-seed runs diverge without warning. There is also no resume checkpoint for optimizer/RNG state.
+
+Record environment/version/device metadata, explicitly select a device, and enforce deterministic settings appropriate to it. Test two fresh, isolated same-seed runs for byte-equivalent metric rows (or document a bounded tolerance). Keep each run in a fresh directory as above.
+
+### MAJOR — `integers` vocabulary aliases EOS with a valid target and is not range-safe
+
+Although the preregistered arm uses digit tokens, the supported `vocab_mode="integers"` is broken. At the default range end, token 101 is both the valid target `next_prime(100)` and `eos_id` (`vocab-1`); larger ranges can have a next prime beyond `range_end+1`, causing an embedding/target index failure. `decode_tokens` in integers mode ignores EOS entirely, which hides the alias. The existing test checks only output shape.
+
+Reserve separate BOS/EOS/PAD IDs outside the full input/target integer domain, derive that domain from the actual maximum target, and add boundary tests. Until then, reject integer mode rather than silently producing invalid semantics.
+
+### MINOR — ACT aggregation itself is normalized, but `halt_eps` is dead configuration
+
+The ACT aggregation is mathematically normalized: `w_t = remaining * p_t`, `remaining *= 1-p_t`, followed by the final remainder on `h_list[-1]`; returned expected steps and its mean penalty are differentiable. Penalizing `mean(halt_steps)` with the scheduled `lambda` is coherent with that distributional definition and correctly disabled for fixed-K runs. There is no normalization bug here.
+
+However, `Config.halt_eps` is documented as an ACT cumulative threshold but is never read. The code always computes all K states and uses a soft remainder, contrary to that configuration's description. Remove the unused setting or implement and test the specified behavior; do not imply a threshold is active in run configs.
+
+### MINOR — Pad-target clamping is safe only because masking follows it, but lacks a guard/test
+
+`y_safe = y.clamp(max=vocab-1)` maps pad ID to EOS solely to make cross-entropy indexable, then multiplication by `y_mask` removes those terms. With the current `make_batch`, this produces correct token loss and avoids pad embeddings in the output head. It is fragile: any future mask mismatch would silently train padded positions toward EOS. Assert that every masked-in target is `< vocab` and every masked-out target is exactly `pad_id`, or use an ignored-index loss. Add a test that perturbing logits at padded target positions cannot change loss or gradients.
+
+### NIT — Tests do not exercise the reviewed failure modes
+
+The suite checks shapes, a broad halting range, and finite backward passes, but not ACT mass summing to one, penalty gradients, mixed-length versus singleton behavior, evaluator-code boundary cases, fresh-run handling, or deterministic repeatability. These omissions allowed the issues above to pass. Also, `log_examples` orders with `len(str(x))` where `x` is a token list; it is deterministic but obscures the intended numeric ordering.