fix: address code review feedback on frame-based architecture

- Important Issue 1: Add guard in ALLOC_SHARED to prevent self-referential act_id (lane leak)
- Added check before parent lookup to reject tokens with act_id already in tag_store
- Added test_alloc_shared_self_referential_guard() to verify TokenRejected is emitted

- Important Issue 2: Add codegen initial_tag_store tuple assertion
- Added assertions in test_alloc_tokens_per_activation() to verify all tuple values
- Prevents regression to int values in future codegen changes

- Important Issue 3: Add REPL tag_store formatting assertion
- Added assertion in test_pe_after_load() to verify 'lane' in output or empty marker
- Ensures formatting includes lane information

- Minor Issue 1: Extract _smart_free helper to deduplicate FREE logic
- New method _smart_free() handles shared FREE logic for all three paths
- Reduces code duplication in _handle_frame_control() FREE/FREE_LANE and _process_token() FREE_FRAME
- Helper correctly handles frame-in-use case and emits appropriate FrameFreed events

- Minor Issue 2: Guard ALLOC_REMOTE against None slot values
- Added check for None before reading target_pe and target_act
- Logs warning and returns early if slots contain None

- Minor Issue 3: Add warnings for FREE_LANE/FREE/FREE_FRAME with invalid act_id
- Added else clauses with logger.warning for unknown act_ids in all FREE paths
- Consistent with other error handling in PE

emu/pe.py

···
             target_act = self.frames[frame_id][inst.fref + 1] if inst.fref + 1 < len(self.frames[frame_id]) else 0
             parent_act = self.frames[frame_id][inst.fref + 2] if inst.fref + 2 < len(self.frames[frame_id]) else 0
 
+            # Guard against None slot values
+            if target_pe is None or target_act is None:
+                logger.warning(f"PE {self.pe_id}: ALLOC_REMOTE has None at fref slots, skipping")
+                return
+
             if parent_act:
                 alloc_op = FrameOp.ALLOC_SHARED
                 payload = parent_act
···
             yield self.env.timeout(1)  # EMIT cycle (no output token)
             # Frame deallocation happens during EMIT cycle with smart FREE logic
             if token.act_id in self.tag_store:
-                freed_frame, lane = self.tag_store.pop(token.act_id)
-                # Clear this lane's match state
-                for i in range(self.matchable_offsets):
-                    self.match_data[freed_frame][i][lane] = None
-                    self.presence[freed_frame][i][lane] = False
-                    self.port_store[freed_frame][i][lane] = None
-                # Check if any other activations use this frame
-                frame_in_use = any(fid == freed_frame for fid, _ in self.tag_store.values())
-                if frame_in_use:
-                    self.lane_free[freed_frame].add(lane)
-                    self._on_event(FrameFreed(
-                        time=self.env.now, component=self._component,
-                        act_id=token.act_id, frame_id=freed_frame,
-                        lane=lane, frame_freed=False,
-                    ))
-                else:
-                    self.free_frames.append(freed_frame)
-                    if freed_frame in self.lane_free:
-                        del self.lane_free[freed_frame]
-                    for i in range(self.frame_slots):
-                        self.frames[freed_frame][i] = None
-                    self._on_event(FrameFreed(
-                        time=self.env.now, component=self._component,
-                        act_id=token.act_id, frame_id=freed_frame,
-                        lane=lane, frame_freed=True,
-                    ))
+                self._smart_free(token.act_id)
+            else:
+                logger.warning(f"PE {self.pe_id}: FREE_FRAME for unknown act_id {token.act_id}")
         else:
             # Normal ALU execute
             # MINOR FIX: Restructure const_val handling to avoid dead code
···
             yield self.env.timeout(1)  # EMIT cycle
             self._do_emit_new(inst, result, bool_out, token.act_id, frame_id, left=left)
 
+    def _smart_free(self, act_id: int) -> None:
+        """Smart FREE helper: deallocate lane, possibly returning frame to free list.
+
+        Does NOT yield. Caller handles timing. Emits FrameFreed event.
+        """
+        if act_id not in self.tag_store:
+            return  # Caller should have checked, but skip silently
+
+        frame_id, lane = self.tag_store.pop(act_id)
+        # Clear this lane's match state
+        for i in range(self.matchable_offsets):
+            self.match_data[frame_id][i][lane] = None
+            self.presence[frame_id][i][lane] = False
+            self.port_store[frame_id][i][lane] = None
+        # Check if any other activations use this frame
+        frame_in_use = any(fid == frame_id for fid, _ in self.tag_store.values())
+        if frame_in_use:
+            # Return lane to pool, keep frame
+            self.lane_free[frame_id].add(lane)
+            self._on_event(FrameFreed(
+                time=self.env.now, component=self._component,
+                act_id=act_id, frame_id=frame_id,
+                lane=lane, frame_freed=False,
+            ))
+        else:
+            # Last lane — return frame to free list
+            self.free_frames.append(frame_id)
+            if frame_id in self.lane_free:
+                del self.lane_free[frame_id]
+            # Clear frame slots
+            for i in range(self.frame_slots):
+                self.frames[frame_id][i] = None
+            self._on_event(FrameFreed(
+                time=self.env.now, component=self._component,
+                act_id=act_id, frame_id=frame_id,
+                lane=lane, frame_freed=True,
+            ))
+
     def _handle_frame_control(self, token: FrameControlToken) -> None:
         """Handle ALLOC, FREE, ALLOC_SHARED, and FREE_LANE operations."""
         if token.op == FrameOp.ALLOC:
···
                 logger.warning(f"PE {self.pe_id}: no free frames available")
         elif token.op == FrameOp.FREE:
             if token.act_id in self.tag_store:
-                frame_id, lane = self.tag_store.pop(token.act_id)
-                # Clear this lane's match state
-                for i in range(self.matchable_offsets):
-                    self.match_data[frame_id][i][lane] = None
-                    self.presence[frame_id][i][lane] = False
-                    self.port_store[frame_id][i][lane] = None
-                # Check if any other activations use this frame
-                frame_in_use = any(fid == frame_id for fid, _ in self.tag_store.values())
-                if frame_in_use:
-                    # Return lane to pool, keep frame
-                    self.lane_free[frame_id].add(lane)
-                    self._on_event(FrameFreed(
-                        time=self.env.now, component=self._component,
-                        act_id=token.act_id, frame_id=frame_id,
-                        lane=lane, frame_freed=False,
-                    ))
-                else:
-                    # Last lane — return frame to free list
-                    self.free_frames.append(frame_id)
-                    if frame_id in self.lane_free:
-                        del self.lane_free[frame_id]
-                    # Clear frame slots
-                    for i in range(self.frame_slots):
-                        self.frames[frame_id][i] = None
-                    self._on_event(FrameFreed(
-                        time=self.env.now, component=self._component,
-                        act_id=token.act_id, frame_id=frame_id,
-                        lane=lane, frame_freed=True,
-                    ))
+                self._smart_free(token.act_id)
+            else:
+                logger.warning(f"PE {self.pe_id}: FREE for unknown act_id {token.act_id}")
         elif token.op == FrameOp.ALLOC_SHARED:
             # Shared allocation: find parent's frame, assign next free lane
+            # Guard against self-referential act_id (would leak old lane)
+            if token.act_id in self.tag_store:
+                self._on_event(TokenRejected(
+                    time=self.env.now, component=self._component,
+                    token=token, reason=f"act_id {token.act_id} already in tag store",
+                ))
+                return
             parent_act_id = token.payload
             if parent_act_id not in self.tag_store:
                 self._on_event(TokenRejected(
···
                 act_id=token.act_id, frame_id=parent_frame_id, lane=lane,
             ))
         elif token.op == FrameOp.FREE_LANE:
-            # Free lane only — never returns frame to free list
+            # Free lane only — never returns frame to free list (smart FREE always does the right thing)
             if token.act_id in self.tag_store:
-                frame_id, lane = self.tag_store.pop(token.act_id)
-                for i in range(self.matchable_offsets):
-                    self.match_data[frame_id][i][lane] = None
-                    self.presence[frame_id][i][lane] = False
-                    self.port_store[frame_id][i][lane] = None
-                self.lane_free[frame_id].add(lane)
-                self._on_event(FrameFreed(
-                    time=self.env.now, component=self._component,
-                    act_id=token.act_id, frame_id=frame_id,
-                    lane=lane, frame_freed=False,
-                ))
+                # Note: _smart_free() handles the frame still being in-use case correctly
+                self._smart_free(token.act_id)
+            else:
+                logger.warning(f"PE {self.pe_id}: FREE_LANE for unknown act_id {token.act_id}")
 
     def _handle_local_write(self, token: PELocalWriteToken) -> None:
         """Handle IRAM write and frame write."""

tests/test_codegen_frames.py

···
         assert len(alloc_tokens) == 2
         assert all(t.op == FrameOp.ALLOC for t in alloc_tokens)
 
+        # Verify that PE configs have initial_tag_store with tuple values
+        assert len(result.pe_configs) == 1
+        pe_cfg = result.pe_configs[0]
+        if pe_cfg.initial_tag_store:
+            for act_id, val in pe_cfg.initial_tag_store.items():
+                assert isinstance(val, tuple) and len(val) == 2, \
+                    f"initial_tag_store[{act_id}] should be (frame_id, lane) tuple, got {val}"
+                frame_id, lane = val
+                assert isinstance(frame_id, int), f"frame_id should be int, got {type(frame_id)}"
+                assert isinstance(lane, int), f"lane should be int, got {type(lane)}"
+
 
 class TestTask3SeedTokens:
     """Task 3: Seed token generation with act_id."""

tests/test_pe_lanes.py

···
         # Parent should not be in tag_store
         assert 999 not in pe.tag_store
 
+    def test_alloc_shared_self_referential_guard(self):
+        """ALLOC_SHARED with act_id already in tag_store emits TokenRejected."""
+        env = simpy.Environment()
+        events = []
+        config = PEConfig(frame_count=4, lane_count=4, on_event=events.append)
+        pe = ProcessingElement(env=env, pe_id=0, config=config)
+
+        # First ALLOC to establish act_id=0 in tag_store
+        fct_alloc = FrameControlToken(
+            target=0, act_id=0, op=FrameOp.ALLOC, payload=0
+        )
+        inject_and_run(env, pe, fct_alloc)
+        assert 0 in pe.tag_store, "act_id=0 should be in tag_store after ALLOC"
+        frame_id_0, lane_0 = pe.tag_store[0]
+
+        # Now try ALLOC_SHARED with act_id=0 and payload=1 (parent_act_id=1)
+        # This should be rejected because act_id=0 already exists
+        fct_alloc_parent = FrameControlToken(
+            target=0, act_id=1, op=FrameOp.ALLOC, payload=0
+        )
+        inject_and_run(env, pe, fct_alloc_parent)
+        assert 1 in pe.tag_store, "act_id=1 should be in tag_store after ALLOC"
+
+        events.clear()
+        fct_shared = FrameControlToken(
+            target=0, act_id=0, op=FrameOp.ALLOC_SHARED, payload=1
+        )
+        inject_and_run(env, pe, fct_shared)
+
+        rejected = [e for e in events if isinstance(e, TokenRejected)]
+        assert len(rejected) > 0, "Should have TokenRejected event"
+        assert "already in tag store" in rejected[0].reason, "Reason should mention already in tag store"
+
+        # Frame and lane should be unchanged
+        assert pe.tag_store[0] == (frame_id_0, lane_0), "act_id=0 state should be unchanged"
+
 
 class TestLaneExhaustion:
     """AC3.6, AC8.2: Lane exhaustion and TokenRejected."""

tests/test_repl.py

···
         out = output.getvalue()
         # Should show PE state or not found message
         assert len(out) > 0
+        # Verify formatting includes lane information or empty tag store marker
+        assert "lane" in out or "Tag store: (empty)" in out
 
     def test_pe_invalid_id(self, repl, temp_dfasm_file):
         """pe with non-integer ID should error."""