PINK: E2E trace analysis — Pass 17 unsafe review/dead code/build/protocols (T1-T14)

Seventeenth pass: catch_unwind + AssertUnwindSafe partially mutated state no rollback (T1 High), HazelcastRowWriter bare json.dumps loses Enum/datetime format (T3 High), real_zinc_plane _slot_from_payload direct key access KeyError (T4 High), _build_pink_bodies str.index("]") corrupts SCENARIOS list (T5 High), VenueAdapter protocol missing connect/disconnect AttributeError (T6 High), shared memory writes non-atomic visible-zero window (T7 High), _slot_from_payload duplicated two files schema drift risk (T9 Medium), _backup_20260530 is valid package accidental old-code import (T14 Medium). 319 total flaws across 17 passes. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
2026-06-02 14:10:49 +02:00
parent b0aa91229f
commit 66b403ff7d
10 changed files with 3473 additions and 49 deletions
--- a/PINK_DITAv2_E2E_TRACE_ANALYSIS.md
+++ b/PINK_DITAv2_E2E_TRACE_ANALYSIS.md
@@ -6058,3 +6058,330 @@ Same pattern in `_build_pink_extended.py` templates and other generated test bui
 | R | Pass 15 (Resource Leaks/Trust Boundaries/Security) | 14 | 2 | 6 | 3 | 2 | 1 |
 | S | Pass 16 (Error Handling/Arithmetic/Test Infra) | 16 | 4 | 7 | 5 | 0 | 0 |
 | **Total** | | **305** | **27** | **90** | **87** | **64** | **37** |
+
+---
+
+## PASS 17 — UNSAFE REVIEW, DEAD CODE/BACKUP DEBRIS, BUILD/PLANE PROTOCOLS
+
+### T1: `catch_unwind` + `AssertUnwindSafe` on `&mut KernelCore` — partially mutated heap state persists after caught panic, no rollback
+
+**File:** `_rust_kernel/src/lib.rs:2057-2071`
+
+```rust
+fn with_handle_mut<F, R>(handle: *mut KernelHandle, f: F) -> Result<R, String>
+where F: FnOnce(&mut KernelCore) -> Result<R, String>,
+{
+    let core = unsafe { &mut (*handle).core };
+    match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(core))) {
+        Ok(result) => result,
+        Err(panic_payload) => {
+            let msg = ...;
+            eprintln!("[KERNEL PANIC caught at FFI boundary] {msg}");
+            Err(msg)  // Partially mutated KernelCore still live in heap Box<KernelHandle>
+        }
+    }
+}
+```
+
+`catch_unwind` prevents Rust panics from unwinding across the FFI boundary (which would be UB). But the `KernelCore` behind the raw pointer is **mutated in-place** on the heap. When a panic occurs mid-mutation:
+
+1. `f(core)` calls some kernel function like `process_intent()` or `apply_fill()`
+2. The function panics partway through — e.g., `k_realized_pnl` was incremented but `event_seq` was not bumped; `slots[i]` was replaced but `rebuild_indexes()` was not called
+3. `catch_unwind` catches the panic, returns `Err(msg)` to the Python caller
+4. The `KernelCore` on the heap retains the **partially applied state**
+5. The next FFI call operates on this corrupted state — `k_capital = seed + realized_pnl - fees_paid` is computed with mismatched values
+6. The code comment acknowledges this: "the slot/account mutation that panicked may be partially applied"
+7. The mitigations (reconcile WARN/ERROR → capital frozen) only work if the corruption is detectable — if the panic corrupts `slot.seen_event_ids` such that dedup fails, duplicate fills can process
+
+`AssertUnwindSafe` on `&mut KernelCore` is sound for memory safety (after panic, the reference is still valid, just the data is inconsistent — no use-after-free, no double-free). But it is **logically unsound** — data invariants are violated, and the recovery path relies on a downstream reconcile to detect the issue, which may not catch all corruption patterns.
+
+**Trigger paths:** Any panic inside `process_intent()`, `on_venue_event()`, `reconcile_slots()`, `apply_fill()`, or `save_full_snapshot()` while mutating `KernelCore`. A panic in `HashMap::insert()` (extremely rare, only on OOM) would leave the HashMap in an undefined state.
+
+**Severity: High**
+
+### T2: Empty backup directory `_backup_20260530_105512/` and stale `tea_debug.log` (0 bytes)
+
+**Files:** `_backup_20260530_105512/` (empty directory), `tea_debug.log` (0 bytes)
+
+`_backup_20260530_105512/` is a **completely empty directory** — zero files. Its sibling `_backup_20260530/` contains 22 source files and a `rust_kernel_src/` subdirectory. The `_105512` variant was created during an earlier backup attempt but never populated.
+
+`tea_debug.log` is a **0-byte empty file** in the workspace root. No code writes to it. It's a stale artifact — likely a log file that was opened but never written to, or a debugging aid that was never used.
+
+Both should be deleted to avoid confusion.
+
+**Severity: Low**
+
+### T3: `HazelcastRowWriter.__call__` uses bare `json.dumps(row, default=str)` — Enums and datetimes serialize as Python `str()` representations
+
+**File:** `hazelcast_projection.py:60-63`
+
+```python
+def __call__(self, name: str, row: dict[str, Any]) -> None:
+    if name.endswith("trade_events"):
+        self.client.get_topic(name).publish(
+            json.dumps(row, ensure_ascii=False, sort_keys=True, default=str)
+        )
+```
+
+The `default=str` fallback serializes `Enum` values as `"TradeSide.SHORT"` (Python's `repr()` format) instead of `"SHORT"` (the `.value`). Datetimes become `"2026-01-01 00:00:00"` (Python `str()` format) instead of `"2026-01-01T00:00:00+00:00"` (ISO 8601). Downstream Hazelcast consumers expecting standard formats get unexpected strings.
+
+Compare with `HazelcastProjector.publish_event()` (line 38) which correctly uses `json_safe(payload)` before `json.dumps()`:
+```python
+self.writer(self.trade_events_topic, json_safe(row))  # uses json_safe() first
+```
+
+The inconsistency: `HazelcastProjector` correctly serializes via `json_safe()`, but `HazelcastRowWriter.__call__` (used directly elsewhere) does not. Any code path that calls `HazelcastRowWriter` directly — rather than through `HazelcastProjector` — produces malformed output.
+
+**Severity: High**
+
+### T4: `real_zinc_plane._slot_from_payload()` uses `payload["entry_time"]` direct key access — crashes with `KeyError` if key missing
+
+**File:** `real_zinc_plane.py:116,133`
+
+```python
+entry_time=datetime.fromisoformat(payload["entry_time"]) if payload.get("entry_time") else None,
+# ... yet at line 133:
+last_event_time=datetime.fromisoformat(payload["last_event_time"])  # NO .get() guard!
+```
+
+Line 116 uses `payload.get("entry_time")` — correct. Line 133 uses `payload["last_event_time"]` — **missing `.get()`**, crashes with `KeyError` if the key is absent.
+
+Compare with `rust_backend.py:396-402` (the equivalent function):
+```python
+entry_time=datetime.fromisoformat(payload["entry_time"]) if payload.get("entry_time") else None,
+last_event_time=datetime.fromisoformat(payload["last_event_time"]) if payload.get("last_event_time") else None,
+```
+
+Both fields use `.get()` in `rust_backend.py`. The `real_zinc_plane.py` version has a **copy-paste error** where the guard on `last_event_time` was omitted. If any slot is deserialized via the shared memory path (RealZincPlane) and lacks a `last_event_time` (e.g., a fresh slot that hasn't received a venue event yet), this crashes.
+
+**Severity: High**
+
+### T5: `_build_pink_bodies.py` uses `str.index("]")` to find SCENARIOS list close bracket — corrupts list if any entry contains `]`
+
+**File:** `_build_pink_bodies.py:214`
+
+```python
+close_bracket = with_bodies.index("]", scenarios_open)
+final = with_bodies[:close_bracket] + "\n" + param_block + "\n" + with_bodies[close_bracket:]
+```
+
+`str.index("]")` finds the **first** `]` character after `scenarios_open`. If any SCENARIOS entry contains a `]` inside a string literal (e.g., a diagnostic code like `INVALID_INTENT_PARSE`, a format string, or a nested data structure), the split lands inside the entry — truncating it and injecting the new `param_block` mid-entry.
+
+The resulting file is syntactically incorrect only if the truncation produces unparseable code. If it happens to produce valid (but semantically wrong) code, the build succeeds with silently corrupted test data.
+
+**Fix:** Use `ast` module to parse the list, or count bracket depth.
+
+**Severity: High**
+
+### T6: `VenueAdapter` protocol missing `connect()`/`disconnect()` — `AttributeError` at runtime
+
+**File:** `venue.py` (protocol), `_build_pink_extended.py:31-32` (caller)
+
+```python
+# _build_pink_extended.py — Shim class:
+async def connect(self, initial_capital=0):
+    self.kernel.venue.connect()  # assumes VenueAdapter has connect()
+
+async def disconnect(self):
+    try:
+        self.kernel.venue.disconnect()  # assumes VenueAdapter has disconnect()
+    except:
+        pass
+```
+
+`VenueAdapter` (defined in `venue.py` as a `Protocol`) defines `submit()`, `cancel()`, `snapshot()`, `subscribe()`, `open_positions()`, and `reconcile()` — but **not** `connect()` or `disconnect()`.
+
+`MockVenueAdapter` has both methods (mock_venue.py:160-166). `BingxVenueAdapter` does **not** have them — calling `connect()` on a `BingxVenueAdapter` raises `AttributeError`.
+
+The `Shim` class in `_build_pink_extended.py` is used for live-test infrastructure. If a live test runs with a venue that lacks `connect()`/`disconnect()`, the error is swallowed by the bare `except: pass` in `disconnect()`, but `connect()` propagates uncaught.
+
+**Fix:** Add `connect()`/`disconnect()` to the `VenueAdapter` protocol, or add them as no-ops on `BingxVenueAdapter`.
+
+**Severity: High**
+
+### T7: `real_control_plane.py` and `real_zinc_plane.py` shared memory writes are non-atomic — reader sees partial state
+
+**Files:** `real_control_plane.py:110-114`, `real_zinc_plane.py:252-253`
+
+```python
+# real_control_plane.py _write_region:
+view[:len(packet)] = packet                      # writes new packet
+if len(view) > len(packet):
+    view[len(packet):] = b"\x00" * (len(view) - len(packet))  # zeroes tail
+
+# real_zinc_plane.py _write_region:
+view[:] = b"\x00" * len(view)                    # full zero (visible-zero window)
+view[:len(packet)] = packet                       # writes packet
+```
+
+Both implementations write the shared memory buffer in **multiple non-atomic operations**. A reader process that reads between these operations sees:
+- **`real_control_plane.py`**: The new header with stale tail from a previous larger packet → `_decode_packet()` may return stale data or parse failure
+- **`real_zinc_plane.py`**: All zeros → `_decode_packet()` returns `{}` (empty dict) or parse failure
+
+The visible-zero window in `real_zinc_plane.py` is particularly dangerous — if a reader reads the zeroed buffer, all slot states appear empty, which could trigger a spurious reconcile or incorrect position tracking.
+
+**Fix:** Either:
+1. Write the packet atomically (if the shared memory size supports it — write new data in a single slice assignment)
+2. Use a sequence number in the header that the reader validates (sequence odd while writing, even when complete)
+3. Use an explicit "writing" flag byte set before and cleared after the write
+
+**Severity: High**
+
+### T8: `real_zinc_plane._slot_from_payload()` reconstructs `internal_trade_id` from slot's `trade_id` instead of order's own — data loss on round-trip
+
+**File:** `real_zinc_plane.py:92,106`
+
+```python
+active_entry_order = VenueOrder(
+    internal_trade_id=str(payload.get("trade_id", "")),  # uses SLOT's trade_id
+    ...
+)
+```
+
+`TradeSlot.to_dict()` serializes the order's own `internal_trade_id` inside the `"active_entry_order"` sub-dict. But `_slot_from_payload()` ignores the per-order value and uses the slot-level `trade_id` instead.
+
+If a slot has multiple orders (e.g., an entry order with `trade_id="abc"` and an exit order with `trade_id="def"`), the slot-level `trade_id` is the **current trade's ID** — which may match one of the orders. But after a CANCEL_ACK that clears the entry order, the slot `trade_id` may be empty or changed. The reconstructed order always gets the slot's `trade_id`, losing the distinction between entry-order and exit-order trade IDs.
+
+This only affects the shared-memory round-trip (RealZincPlane). The FFI path (`rust_backend.py`) correctly uses the order's serialized `internal_trade_id`.
+
+**Severity: Medium**
+
+### T9: `_slot_from_payload()` duplicated verbatim between `real_zinc_plane.py` and `rust_backend.py` — double maintenance burden
+
+**Files:** `real_zinc_plane.py:83-138`, `rust_backend.py:379-402`
+
+The slot deserialization function `_slot_from_payload()` (or equivalent inline code) exists in **two separate files** with nearly identical logic. The `real_zinc_plane.py` version is a 55-line function; the `rust_backend.py` version is inline in `_slot_from_payload()`.
+
+Both deserialize `TradeSlot` from the same `to_dict()` output format. Any schema change (field added, removed, renamed, or type-changed) must be updated in both places. T4 (missing `.get()` on `last_event_time`) and T8 (`internal_trade_id` from wrong source) are direct consequences of this duplication — the bug exists in one copy but not the other.
+
+**Fix:** Extract shared `_slot_from_payload()` into `contracts.py` (or `utils.py`).
+
+**Severity: Medium**
+
+### T10: `_build_pink_extended.py` string index math finds first `finally:` — could match nested `try/finally` inside function body
+
+**File:** `_build_pink_extended.py:117-119`
+
+```python
+idx = content.index(old_run_pat)
+run_end = content.index("    finally:", idx)  # finds FIRST "finally:" — could be nested!
+run_end = content.index("\n\n", run_end) + 2  # boundary detection for function end
+```
+
+The search for `"    finally:"` finds the **first** occurrence after `idx`. If the `_run()` function body (or any function it calls, like `_si()` or `_verify()`) contains a nested `try/finally` block — or if the function contains the word "finally:" in a string or comment — the index points to the wrong location. The `"\n\n"` search then terminates inside the function body, producing a truncated replacement that generates syntactically broken output.
+
+The generated `test_pink_bingx_dita_live_e2e.py` is patched with index math that has no validation. A malformed patch silently produces a non-functional test file (syntax error only caught at test import time).
+
+**Fix:** Parse the function boundaries using `ast` module or use a well-defined sentinel comment (e.g., `# END _run`) as anchor points.
+
+**Severity: Medium**
+
+### T11: No workspace-root `.gitignore` — `__pycache__`, backup dirs, context files, build artifacts untracked
+
+**File:** (missing — should be `dita_v2/.gitignore`)
+
+The only `.gitignore` in the workspace is inside `_rust_kernel/` (covers `/target`). There is **no `.gitignore` at the workspace root** (`dita_v2/`). This means:
+
+- `__pycache__/` directories (29 `.pyc` files present) are tracked or untracked depending on global git config
+- `_backup_20260530/` and `_backup_20260530_105512/` are visible to git (the 22 source files in the backup are tracked? may or may not be indexed)
+- `_backup_20260530_105512/` (empty dir) is visible
+- `Codex_CONTEXT_RESTORE__*.txt` context files are visible
+- `tea_debug.log` is visible
+- Any `.pyc` files that end up in the index cause merge conflicts
+
+The git `status` shows `?? 2004` untracked files — many of these would be excluded by a proper `.gitignore`.
+
+**Severity: Low**
+
+### T12: `projection.py` lazy import failure silently swallowed — caller gets `writer=None` with no diagnostic
+
+**File:** `projection.py:75-77`
+
+```python
+try:
+    from .hazelcast_projection import HazelcastRowWriter
+    writer = HazelcastRowWriter(client)
+except Exception:  # catches import errors, constructor errors, everything
+    writer = None
+```
+
+If the `hazelcast_projection` module has a syntax error, `HazelcastRowWriter` doesn't exist, or the constructor raises, the exception is silently swallowed. The caller gets a `HazelcastProjection` with `writer=None`. The `write_transition()` and `write_control()` methods check `if not self.writer:` and silently return — so all Hazelcast writes are silently dropped with no log, no error, no diagnostic.
+
+The `"Hazelcast unavailable — fallback active"` log message is only printed for the **first** import attempt. If the module is later fixed (e.g., a missing dependency is installed), the stale `writer=None` persists because the import is not retried.
+
+**Severity: Medium**
+
+### T13: `Codex_CONTEXT_RESTORE__*.txt` and other AI context files in workspace root — debris
+
+**Files:** `Codex_CONTEXT_RESTORE__2026-06-02-130508-*.txt`, other `.md` analysis documents
+
+The workspace root contains AI-assistant context restore files and 6+ Markdown flaw analysis documents (`PINK_DITAv2_E2E_TRACE_ANALYSIS.md`, `PINK_DITAv2_FLAW_ANALYSIS_2026-05-31.md`, `PINK_DITAv2_THREADING_ATOMICITY.md`, etc.). These are analysis artifacts, not source code.
+
+While the flaw documents are intentional project records, the `Codex_CONTEXT_RESTORE__*.txt` files are ephemeral AI context dumps that should not be in version control. They contain session state information that is meaningless outside the AI session.
+
+**Severity: Low**
+
+### T14: `_backup_20260530/` contains 22 live source files — risk of stale import confusion
+
+**File:** `_backup_20260530/` (22 Python files including `rust_backend.py`, `launcher.py`, `bingx_venue.py`, etc.)
+
+The backup directory contains full copies of all Python source files from May 30. If a developer runs `import` from within the `dita_v2` directory, the backup directory's `__init__.py` makes it a valid Python package. An accidental `from _backup_20260530 import rust_backend` would load the **old** code instead of the current implementation — silently, with no warning.
+
+The backup `rust_backend.py` lacks the Rust FFI integration, has no `_first_invalid_intent_field()`, and uses the old `ExecutionKernel` class. Accidentally importing from the backup would produce hard-to-diagnose errors (missing methods, wrong behavior).
+
+**Fix:** Rename backup directories to non-Python-package names (e.g., `backup_20260530` without the leading underscore), or add `__init__.py` that raises `ImportError` with a clear message.
+
+**Severity: Medium**
+
+---
+
+## Pass 17 Summary
+
+| # | Flaw | Layer | Severity |
+|---|------|-------|----------|
+| T1 | `catch_unwind` + `AssertUnwindSafe` — partially mutated state persists, no rollback | Rust | **High** |
+| T2 | Empty backup dir `_backup_20260530_105512/` and stale `tea_debug.log` | Repo | Low |
+| T3 | `HazelcastRowWriter` uses bare `json.dumps(row, default=str)` — Enums/datetimes wrong format | Bridge | **High** |
+| T4 | `real_zinc_plane._slot_from_payload()` direct key access `payload["last_event_time"]` — `KeyError` crash | Plane | **High** |
+| T5 | `_build_pink_bodies.py` `str.index("]")` finds first `]` — corrupts SCENARIOS list | Build | **High** |
+| T6 | `VenueAdapter` protocol missing `connect()`/`disconnect()` — `AttributeError` at runtime | Venue | **High** |
+| T7 | Shared memory writes non-atomic — visible-zero window, stale tail exposes partial state | Plane | **High** |
+| T8 | `_slot_from_payload()` reconstructs `internal_trade_id` from slot's trade_id — order-level data loss | Plane | Medium |
+| T9 | `_slot_from_payload()` duplicated in two files — double maintenance burden, schema drift risk | Plane | Medium |
+| T10 | `_build_pink_extended.py` `str.index("finally:")` finds first match — nested try/finally mismatch | Build | Medium |
+| T11 | No workspace-root `.gitignore` — `__pycache__`, backup dirs, debris untracked | Repo | Low |
+| T12 | `projection.py` lazy import failure silently swallowed — `writer=None` drops all Hazelcast writes | Bridge | Medium |
+| T13 | `Codex_CONTEXT_RESTORE__*.txt` AI context files in workspace root — debris | Repo | Low |
+| T14 | `_backup_20260530/` is a valid Python package — accidental old-code import risk | Repo | Medium |
+
+### Pass 17 Severity
+
+| Severity | Count |
+|----------|-------|
+| **High** | 5 (T1, T3, T4, T5, T6, T7) |
+| Medium | 5 (T8, T9, T10, T12, T14) |
+| Low | 4 (T2, T11, T13) |
+
+### Combined Catalog (All 17 Passes)
+
+| Pass | Focus | Count | Critical | High | Medium | Low | Info |
+|------|-------|-------|----------|------|--------|-----|------|
+| A | Architectural | 15 | 0 | 2 | 0 | 2 | 11 |
+| T | Threading/Atomicity | 9 | 1 | 3 | 3 | 2 | 0 |
+| E | E2E Trace (Pass 1) | 26 | 0 | 4 | 10 | 11 | 1 |
+| F | Deep E2E (Pass 3) | 30 | 0 | 1 | 8 | 17 | 4 |
+| G | Domain Scans (Pass 4) | 36 | 4 | 11 | 11 | 8 | 2 |
+| H | Edge Domains (Pass 5) | 22 | 3 | 9 | 5 | 4 | 1 |
+| I | Pass 6 (Math/Tests/Recovery/Security) | 22 | 3 | 11 | 4 | 2 | 2 |
+| J | Pass 7 (Test Infra/Data/Rust/Env/Conn) | 16 | 0 | 7 | 7 | 2 | 0 |
+| K | Pass 8 (Observability/Memory/Time/DeadCode) | 23 | 2 | 7 | 7 | 1 | 6 |
+| L | Pass 9 (Contracts/Events/Network/FFI/Diffs) | 16 | 0 | 4 | 8 | 4 | 0 |
+| M | Pass 10 (Runtime/TestBugs/FSM/Persistence/Metrics) | 18 | 3 | 7 | 5 | 3 | 0 |
+| N | Pass 11 (Async/Sync Seams/Locks/Threading) | 10 | 4 | 1 | 3 | 1 | 1 |
+| O | Pass 12 (Sync/Async Wider Scope) | 11 | 0 | 3 | 7 | 1 | 0 |
+| P | Pass 13 (FFI Safety/Dangling Pointers/Coverage) | 9 | 1 | 3 | 3 | 1 | 1 |
+| Q | Pass 14 (Serde Edges/Backup Diffs/Market Data) | 12 | 0 | 4 | 3 | 2 | 3 |
+| R | Pass 15 (Resource Leaks/Trust Boundaries/Security) | 14 | 2 | 6 | 3 | 2 | 1 |
+| S | Pass 16 (Error Handling/Arithmetic/Test Infra) | 16 | 4 | 7 | 5 | 0 | 0 |
+| T | Pass 17 (Unsafe Review/Dead Code/Build/Protocols) | 14 | 0 | 5 | 5 | 4 | 0 |
+| **Total** | | **319** | **27** | **95** | **92** | **64** | **37** |