# Fresh Codex CLI ultra review — 2026-10-16

A new ephemeral Codex CLI 0.153.4 process reviewed release commit
`7573c893ee92f45791e365a989dbbecda368af69`, using configured model `gpt-6-astra`
and reasoning effort `ultra`. It used no resumed conversation. Upstream Yrs
remained unchanged. The reviewer found four port/adaptation defects and one
inherited Rust defect. The parent investigation also found awareness restoration
and expanded the sparse fix to emitted updates and historical cuts.

| Issue | Correction | Permanent regression evidence |
| --- | --- | --- |
| [#1](https://github.com/bobzhang/yrs/issues/1) | Sparse relay selection, cursor anchors and historical snapshots | `sparse_transport_test.mbt`, `sparse_history_test.mbt`, `sparse_map_history_test.mbt` |
| [#2](https://github.com/bobzhang/yrs/issues/2) | Direct typed v2 transport; independent subdocument descriptors | `v2_typed_values_test.mbt`, `validation/typed_v2.py` |
| [#3](https://github.com/bobzhang/yrs/issues/3) | Prevent subscriptions for conflict-deleted weak items | `weak_conflict_test.mbt` |
| [#4](https://github.com/bobzhang/yrs/issues/4) | Freeze retained weak/subdocument observer transactions | `transaction_retention_test.mbt` |
| [#5](https://github.com/bobzhang/yrs/issues/5) | Notify absent-to-present awareness transitions | `awareness_restore_test.mbt`, `validation/presence.py` |

The original review follows. Line references address the reviewed commit, not the
corrected code. Disposable `_build/review-ultra` probes mentioned by the reviewer
are local diagnostics; the permanent regressions above are committed and portable.
Passing these tests does not prove the absence of all CRDT defects.

## Original reviewer report

Reviewed HEAD `7573c893ee92f45791e365a989dbbecda368af69` against clean, pinned Yrs `ac339600…`. **Found four port defects and one inherited CRDT correctness defect.**

1. **P1 — V2 transport silently corrupts rich-text values. Confirmed compatibility bug.**

   [update_codec.mbt:388](https://github.com/bobzhang/yrs/blob/7573c893ee92f45791e365a989dbbecda368af69/update_codec.mbt#L388) and [update_codec.mbt:396](https://github.com/bobzhang/yrs/blob/7573c893ee92f45791e365a989dbbecda368af69/update_codec.mbt#L396) route v2 decoding and encoding through v1 JSON. V2 embeds actually support binary `Any` values.

   Minimal reproduction: insert an embed, encode the document as v2, and import it into a fresh document. Results on JS and native:

   - `Buffer([1,2])` becomes an ordinary array.
   - `Undefined` becomes `Null`.
   - `Integer(9007199254740993)` becomes `Float(9007199254740992)`.
   - An `Undefined` formatting attribute disappears.

   Unchanged Yrs preserves the embed values. Importing its buffer packet also reproduces the port’s corruption. The loss extends to v2 update utilities using the same conversion path.

   **Fix:** encode and decode typed blocks directly through v2 field codecs, preserving `Any` without intermediate JSON. Cover document transport, update utilities, transaction updates and historical encoding.

   `_build/review-ultra/codecs/moon/probe/main.mbt`, `_build/review-ultra/codecs/rust-output.txt`.

2. **P2 — Differential synchronization omits available sparse items. Confirmed inherited correctness bug.**

   [update_v1.mbt:186](https://github.com/bobzhang/yrs/blob/7573c893ee92f45791e365a989dbbecda368af69/update_v1.mbt#L186), lines 186–188, skips an entire client based on its contiguous state-vector prefix, even when later items are integrated.

   Reproduction: one author inserts `"missing"` at clocks 0–6, map entry `a=true` at 7, and `b=true` at 8. Deliver only `a` to replica A and only `b` to B. Both advertise `{1:0}`. A’s full encoding contains `a`, but encoding against B’s state vector produces `[0,0]`; synchronization never delivers `a`.

   This reproduces in both formats and **also in unchanged Yrs**. It is a production correctness issue, not a port/reference divergence: the receiver’s zero prefix does not justify omitting available later items.

   **Fix:** remove the prefix-based early skip and select blocks using their actual clocks. Removing that condition in a disposable copy makes B receive both entries.

   `_build/review-ultra/codecs/moon-js-output.txt`, `_build/review-ultra/codecs/moon-js-fixed-output.txt`.

3. **P2 — Cursors cannot resolve visible content received sparsely. Confirmed compatibility bug.**

   [sticky_index.mbt:173](https://github.com/bobzhang/yrs/blob/7573c893ee92f45791e365a989dbbecda368af69/sticky_index.mbt#L173) and [sticky_index.mbt:216](https://github.com/bobzhang/yrs/blob/7573c893ee92f45791e365a989dbbecda368af69/sticky_index.mbt#L216) require the referenced identity to precede the contiguous state-vector prefix.

   Reproduction: create an unrelated map operation at clock 0, capture its state vector, then insert `"abc"` into a new text root. Import only the subsequent diff into a fresh peer. The text is visible, but `text.sticky_index(1).unwrap().get_offset(peer)` returns `None`. Unchanged Yrs returns offset `1`. Nested sequence boundary cursors fail similarly. Delivering the unrelated missing prefix makes the same cursor resolve.

   Confirmed on JS/native and v1/v2.

   **Fix:** resolve against actual integrated items and their parent/redone chains; a contiguous-prefix test cannot establish whether a sparse item exists.

   `_build/review-ultra/core/sparse_cursor.mbt`, `_build/review-ultra/core/cursor-js.txt`, `_build/review-ultra/core/cursor-rust.txt`.

4. **P2 — A losing weak-link write leaves ghost deep subscriptions. Confirmed compatibility bug.**

   At [text_store.mbt:642](https://github.com/bobzhang/yrs/blob/7573c893ee92f45791e365a989dbbecda368af69/text_store.mbt#L642), integration materializes weak subscriptions after conflict resolution may already have deleted that item at line 636.

   Reproduction: A quotes a nested map into `links["key"]`; B concurrently writes `"winner"` to that key. B imports A’s losing quotation, subscribes deeply to `links`, then edits the quoted nested map. The winning value remains `"winner"`, but MoonBit emits one unrelated deep event; unchanged Rust emits none.

   Confirmed for both wire formats and both GC settings. This is stale propagation, not resurrection of the losing value.

   **Fix:** require `!item.deleted` before materializing the weak link. That guard eliminated all four reproduced failures in the disposable copy.

   `_build/review-ultra/weak-subdocs/moon/repro/main.mbt`, `_build/review-ultra/weak-subdocs/proposed-fix.diff`.

5. **P2 — Retained weak/subdocument observer transactions absorb later edits. Confirmed adaptation-contract bug.**

   The capture predicate at [transaction.mbt:228](https://github.com/bobzhang/yrs/blob/7573c893ee92f45791e365a989dbbecda368af69/transaction.mbt#L228), lines 228–238, omits `weak_observers` and `subdoc_observers`. Consequently, commit-time freezing at [transaction.mbt:191](https://github.com/bobzhang/yrs/blob/7573c893ee92f45791e365a989dbbecda368af69/transaction.mbt#L191) can be skipped.

   Reproduction: register only a subdocument observer, retain the transaction inserting a child, then insert `later=42`. The retained transaction reports after-state clock `2` instead of its commit’s `1`; its encoded update changes and replays the unrelated later key. Weak-only observation reproduces the same failure.

   This violates the port’s documented stable-after-close transaction contract. Rust borrows callback transactions, so this finding concerns the MoonBit ownership adaptation.

   **Fix:** include both observer families in capture decisions, or freeze every transaction exposed to callbacks.

   `_build/review-ultra/undo-observers/project/probe/main.mbt`, `_build/review-ultra/undo-observers/verified-output.txt`.

Nineteen focused existing tests passed in the isolated build. Additional narrow undo/redo, overlapping-manager GC retention and subdocument undo checks found no further defects. I did not independently exhaust the full randomized undo/network matrices, malformed-input corpus, or every snapshot/weak-range combination; those remain unverified by this review.

Tracked files, the reference checkout and existing release results remain unchanged. All new evidence is under `_build/review-ultra/`.

## Follow-up correction

A second fresh CLI ultra review of the fixes reproduced a sparse snapshot hole:
receive an old map value and an independent later suffix, capture a snapshot,
then receive the missing overwrite. Replaying that snapshot deleted the old value
because integrating even a deleted newer map version hides its predecessor.
Historical encoding now represents excluded versions after the snapshot's visible
winner as inert GC blocks. `sparse_map_history_test.mbt` covers serialized cuts,
both wire formats, before/after gap filling, and replaced shared values with nested
children. The finding is tracked as further evidence on issue #1.

The [full follow-up report](2026-10-16-ultra-followup.txt) independently verified
the snapshot correction (21 selected tests passed in an isolated candidate).
The reviewed patch is committed as `a53e3c2`; “uncommitted” in that report refers
to the review's earlier cutoff. No additional actionable findings were reported.
Its two ownership/retention controls are retained as `subdocument_ownership_test.mbt`.