Merge pull request #42 from FluffyAIcode/AgentMemory/v030-pr-a3-remove-adr-0007-deadcode-8e7f

Author: fluffy314

docs/adr/0008-session-bound-runtime-and-grpc-protocol.md

@@ -694,13 +694,40 @@ parallelize.
   `SessionStore` with create/append/close, in-memory only, no
   scheduler binding yet. INV-1, INV-2 enforced inside. Pure Python,
   no gRPC. 100% unit coverage.
-- **PR-A3**: Refactor the verifier (CPU + MLX) so its KV cache state
-  is constructed and owned by `SessionStore` rather than by the
-  scheduler / pool. Slab-pool integration (ADR 0003) becomes "slab
-  per session" instead of "slab per scheduler slot". Internal-only
-  refactor; the existing HTTP shim still works against the new
-  internal shape. 100% unit coverage; no behavior change to the
-  HTTP surface.
+- **PR-A3** *(scope split, recorded 2026-06-01 during implementation
+  of PR-A3)*: This phase originally proposed two coupled changes —
+  (a) remove the ADR 0007 `path_select` / `prefill_incremental`
+  machinery from both verifiers, and (b) refactor slab ownership so
+  the KV cache state is constructed and owned by `SessionStore`
+  rather than by the scheduler / pool. (a) requires only Linux unit
+  tests; (b) crosses into MLX-runtime hardware paths and pulls in
+  scheduler / pool API redesign. Atomic merge of (a) is essential
+  for coherence: the moment ADR 0007's `path_select` is removed
+  from the verifier, every caller has to be removed too, or the
+  speculative-decoding loop breaks. (b) does not have that
+  coherence pressure — it can land independently after (a).
+  Therefore PR-A3 is split:
+  - **PR-A3** (this PR): pure removal of ADR 0007 dead code.
+    Deletes `kv_cache_proposer/path_plan.py`,
+    `tests/core/test_path_plan.py`,
+    `tests/core/test_determinism_gate.py` (depends on
+    `path_select`); strips `path_select` /
+    `prefill_incremental` / `_cached_global_positions` /
+    `_prompt_matches_cached_positions` from both verifiers; reverts
+    `kv_cache_proposer/speculative.py`'s `generate()` dispatch to a
+    single always-prefill path. Inert defaults are retained on
+    `SpeculativeRunResult.path_selection` / `tokens_skipped` /
+    `prefill_duration_seconds` so the server-side observability
+    surface (which §6.6 rows server/* scope to PR-D1) keeps reading
+    valid values without code change. 100% Linux unit coverage.
+  - **PR-A3b** (next, queued after this PR): the slab-ownership
+    refactor proper — verifier KV state is constructed and owned by
+    `SessionStore`. Slab-pool integration becomes "slab per
+    session" instead of "slab per scheduler slot". This is the PR
+    where the verifier becomes a `CacheInspector` (PR-A2) for
+    `SessionStore`. First PR with a mandatory Mac M4 integration
+    test report (cf. §9), since it touches MLX runtime paths in a
+    non-deletion way.
 
 ### 6.2 Phase B — gRPC server + Python SDK
 
@@ -767,10 +794,10 @@ through `0a31ee9` PR 7-6):
 | ------------------------------------------------------- | -------- | ------------------------------------------------------------------------------------------------------------ |
 | `kv_cache_proposer/path_plan.py`                        | added    | Deleted in PR-A3 (verifier session-store rewiring) — `PathPlan` is replaced by `SessionStore` lookup.        |
 | `tests/core/test_path_plan.py`                          | added    | Deleted alongside the implementation in PR-A3.                                                               |
-| `tests/core/test_determinism_gate.py`                   | added    | Replaced by `tests/integration/test_inv3_session_determinism_gate.py` in PR-E1; old file deleted in PR-E1.   |
+| `tests/core/test_determinism_gate.py`                   | added    | Deleted in **PR-A3** (the test depends on `path_select`, which PR-A3 removes; cannot wait for PR-E1). Replaced by `tests/integration/test_inv3_session_determinism_gate.py` in PR-E1, which is created from scratch rather than refactored from the deleted file. |
 | `kv_cache_proposer/verifier.py`                         | modified | `path_select` / `prefill_incremental` removed in PR-A3; `cached_token_sequence` retained (still useful for INV-1 inside `SessionStore`). |
 | `inference_engine/backends/mlx/verifier.py`             | modified | Same as above.                                                                                               |
-| `kv_cache_proposer/speculative.py`                      | modified | `path_select` dispatch removed in PR-B2 (`AppendTokens` handler subsumes the role).                          |
+| `kv_cache_proposer/speculative.py`                      | modified | `path_select` dispatch removed in **PR-A3** (atomic with the verifier-side removal — leaving the dispatch but removing `verifier.path_select` would break the speculative loop). `generate()` reverts to single always-prefill. PR-B2 is now scoped to "the gRPC `AppendTokens` handler subsumes the *role* that `generate`'s dispatch used to play, this time at the protocol layer not the speculative-decoder layer." |
 | `inference_engine/server/app.py`                        | modified | `_emit_path_selection_metric` and `_session_acceptance_rate` paths removed in PR-D1 (deprecated-shim refactor).|
 | `inference_engine/server/engine.py`                     | modified | `EngineResult` `path_selection` / `tokens_skipped` / `prefill_duration_seconds` fields removed in PR-D1.     |
 | `inference_engine/server/metrics.py`                    | modified | `path_selection_total`, `continuation_tokens_skipped_total`, `verifier_prefill_duration_seconds`, `cache_invariant_violations_total` are removed in PR-D1; replaced by §2.9's `session_*` metrics in PR-B1/B3. |

inference_engine/backends/mlx/verifier.py

@@ -199,70 +199,6 @@ def commit_or_truncate(self, forwarded: int, accepted: int) -> None:
         self._record_peak_kv()
         self._assert_cache_invariant_1()
 
-    def path_select(self, prompt: List[int]) -> "PathPlan":
-        """Select between continuation and new-session paths for ``prompt``.
-
-        Same contract as the CPU verifier; see
-        :meth:`kv_cache_proposer.verifier.SinkWindowVerifier.path_select`
-        for full semantics. Implements ADR 0007 §2.4 and asserts
-        INV-2 (§2.9).
-        """
-        from kv_cache_proposer.path_plan import (  # avoid circular
-            ContinuationPlan,
-            NewSession,
-        )
-
-        if not prompt:
-            raise ValueError("prompt must be non-empty")
-        prompt_list = list(prompt)
-
-        if self.cache is None or self.cache_logical_size == 0:
-            return NewSession(prompt=prompt_list)
-
-        cache_end = self.next_global_position
-        if len(prompt_list) < cache_end:
-            return NewSession(prompt=prompt_list)
-        if not self._prompt_matches_cached_positions(prompt_list):
-            return NewSession(prompt=prompt_list)
-
-        skip_n = cache_end
-        new_tokens = prompt_list[skip_n:]
-
-        if skip_n != self.next_global_position:
-            raise AssertionError(
-                f"INV-2 violated (position monotonicity): planned "
-                f"skip_n={skip_n} but next_global_position="
-                f"{self.next_global_position}. Continuation must "
-                f"extend exactly from the cache's logical end. This "
-                f"is a bug in path_select; ADR 0007 §2.9 forbids "
-                f"silent recovery. cache_logical_size="
-                f"{self.cache_logical_size}, "
-                f"cached_token_sequence_len="
-                f"{len(self.cached_token_sequence)}."
-            )
-
-        return ContinuationPlan(skip_n=skip_n, new_tokens=new_tokens)
-
-    def prefill_incremental(self, new_tokens: List[int]) -> None:
-        """Run incremental prefill on ``new_tokens``, reusing cached state.
-
-        Same contract as the CPU verifier; see
-        :meth:`kv_cache_proposer.verifier.SinkWindowVerifier.prefill_incremental`.
-        """
-        if self.cache is None:
-            raise RuntimeError(
-                "prefill_incremental called before any prefill; cache "
-                "is None. Call path_select first and route NewSession "
-                "to prefill() instead."
-            )
-        if not new_tokens:
-            return
-        block_logits = self.forward_block(list(new_tokens))
-        self.commit_or_truncate(
-            forwarded=len(new_tokens), accepted=len(new_tokens)
-        )
-        self.next_token_logits = block_logits[-1].clone()
-
     def append_token(self, token_id: int) -> torch.Tensor:
         logits = self.forward_block([token_id])
         self.commit_or_truncate(forwarded=1, accepted=1)
@@ -277,42 +213,6 @@ def _cache_buffer_size(self) -> int:
             return 0
         return cache_ops.cache_seq_length(self.cache)
 
-    def _cached_global_positions(self) -> List[int]:
-        """Global token positions currently held in the cache.
-
-        See :meth:`kv_cache_proposer.verifier.SinkWindowVerifier._cached_global_positions`
-        for semantics.
-        """
-        n = self.next_global_position
-        if n == 0:
-            return []
-        budget = self.config.sink_size + self.config.window_size
-        if n <= budget:
-            return list(range(n))
-        sink_positions = list(range(self.config.sink_size))
-        window_start = n - self.config.window_size
-        window_positions = list(range(window_start, n))
-        return sink_positions + window_positions
-
-    def _prompt_matches_cached_positions(self, prompt: List[int]) -> bool:
-        """Token-id-level check for ADR 0007 §2.4.a.2."""
-        positions = self._cached_global_positions()
-        if len(positions) != len(self.cached_token_sequence):
-            raise AssertionError(
-                f"_prompt_matches_cached_positions: position list of "
-                f"length {len(positions)} disagrees with parallel "
-                f"sequence of length {len(self.cached_token_sequence)}; "
-                f"INV-1 should have caught this earlier"
-            )
-        for cache_idx, global_pos in enumerate(positions):
-            if global_pos >= len(prompt):
-                return False
-            if int(prompt[global_pos]) != int(
-                self.cached_token_sequence[cache_idx]
-            ):
-                return False
-        return True
-
     def _sink_window_slice(self, sequence: List[int]) -> List[int]:
         """Return ``sequence`` after the sink+window trim that the K/V
         cache applies.

kv_cache_proposer/path_plan.py

@@ -73,10 +73,13 @@ class SpeculativeRunResult:
     verifier_weight_bytes: int
     verifier_final_kv_token_count: int
     wall_time_seconds: float
-    # ADR 0007 §2.10 path-selection observability. Populated by
-    # SpeculativeDecoder.generate based on the path it dispatched to.
-    path_selection: str = "new_session"  # "continuation" | "new_session"
-    tokens_skipped: int = 0  # ContinuationPlan.skip_n if continuation, 0 else
+    # Historically ADR 0007 §2.10 path-selection observability fields.
+    # After ADR 0008 PR-A3 the path-select dispatch was removed; these
+    # are kept as inert defaults so server/engine.py and server/app.py
+    # (which still surface them on /metrics) keep working unchanged
+    # until ADR 0008 PR-D1 deletes them at the deprecated-shim refactor.
+    path_selection: str = "new_session"
+    tokens_skipped: int = 0
     prefill_duration_seconds: float = 0.0
 
     @property
@@ -143,28 +146,18 @@ def _emit(tokens: List[int]) -> bool:
         self.verifier.stats.peak_kv_bytes = 0
         self.verifier.stats.peak_activation_bytes = 0
 
-        # ADR 0007 §2.4: dispatch on path-selection. ContinuationPlan
-        # reuses cached prefix; NewSession runs full prefill (the
-        # v0.3.0-rc1 behavior). Output is bit-identical between the
-        # two paths for the same input (§2.7); the only difference
-        # is the prefill cost. We record the decision + the prefill
-        # wall time on the result so the route handler can populate
-        # the §2.10 observability metrics.
-        from .path_plan import ContinuationPlan, NewSession
-        plan = self.verifier.path_select(prompt_ids)
+        # ADR 0008 PR-A3 (supersedes ADR 0007 §2.4): the path_select /
+        # ContinuationPlan / NewSession dispatch was removed when ADR
+        # 0008 replaced ADR 0007's automatic-prefix-matching design with
+        # an explicit session_id protocol. The verifier always does a
+        # full prefill here; cross-request cache reuse is now the SDK's
+        # responsibility (it calls Generate against an existing
+        # session_id with only the new tokens). prefill_duration_seconds
+        # is still measured for observability; path_selection is left
+        # at its default of "new_session" since this entry point is
+        # always the new-session path.
         prefill_t0 = time.perf_counter()
-        if isinstance(plan, ContinuationPlan):
-            self.verifier.prefill_incremental(plan.new_tokens)
-            path_selection = "continuation"
-            tokens_skipped = int(plan.skip_n)
-        else:
-            assert isinstance(plan, NewSession), (
-                f"path_select must return ContinuationPlan or NewSession, "
-                f"got {type(plan).__name__}"
-            )
-            self.verifier.prefill(plan.prompt)
-            path_selection = "new_session"
-            tokens_skipped = 0
+        self.verifier.prefill(prompt_ids)
         prefill_duration_seconds = time.perf_counter() - prefill_t0
         committed: List[int] = list(prompt_ids)
         generated: List[int] = []
@@ -269,9 +262,10 @@ def _emit(tokens: List[int]) -> bool:
             verifier_weight_bytes=self.verifier.stats.weight_bytes,
             verifier_final_kv_token_count=self.verifier.cache_logical_size,
             wall_time_seconds=elapsed,
-            path_selection=path_selection,
-            tokens_skipped=tokens_skipped,
             prefill_duration_seconds=prefill_duration_seconds,
+            # path_selection / tokens_skipped use SpeculativeRunResult's
+            # dataclass defaults ("new_session" / 0). After ADR 0008
+            # PR-A3 there is only one path through this entry point.
         )
 
     @staticmethod