fix(vbgui): code-review hardening of JSON-RPC + cache + widget

Acts on the F-A..F-H code review findings. Each fix is pinned by a
regression test in tests/v4/test_jsonrpc_review_fixes.py (13 cases).

CRITICAL
- C1 LRUCache.get/set now deep-copy via _isolate() (Pydantic
  model_copy(deep=True) for BaseModel, copy.deepcopy otherwise) so a
  cache hit cannot leak a shared mutable object to two callers. Adds
  ~150 µs per hit; verify cold still 1.6 ms, cached 0.2 ms.
- C2 data.preview_parquet now distinguishes channels=None (emit all
  available side-channels) from channels=[] (emit none); both branches
  use different cache keys and produce different payloads.

HIGH
- H1 tokenizer _load() guarded by a module-level Lock so two
  concurrent first-loads on the same source share one Tokenizer
  instance.
- H4 EncodeVisualizeParams._cache_key now uses
  sha256(text.encode()) instead of Python hash() — deterministic
  across processes, survives server restart, matches the cache.py
  docstring contract.
- H8 FlowCanvas drop handler uses e.currentTarget (the canvas wrapper)
  for getBoundingClientRect instead of e.target (often a child node) —
  drop coordinates are now correct when the user lands on an existing
  node.

MEDIUM/LOW
- L3 widget.py _on_msg now also writes the dispatched outcome to the
  last_result traitlet (with method + id) so notebook Python code can
  observe RPC results without snooping on the wire. Error envelopes
  do NOT overwrite last_result.
- M7 DataInspector resets channel toggles when the schema (token
  column OR available channels list) changes across loads, so a new
  parquet file's columns auto-enable instead of inheriting stale
  off-toggles.

Test count delta: +13 review-fix tests + 1 existing test rewired
(test_verify_cache_hit_short_circuits now asserts equality not
identity, since the cache returns defensive copies).

Full v4 regression: 2103 passed / 5 skipped / 15 xfailed / 0 failed.
Vbgui: 77 tests passed (no churn).

Author: apstenku123

cppmega_v4/jsonrpc/cache.py

@@ -8,12 +8,25 @@
 
 from __future__ import annotations
 
+import copy
 import hashlib
 import json
 from collections import OrderedDict
 from threading import Lock
 from typing import Any, Mapping
 
+try:
+    from pydantic import BaseModel
+except ImportError:  # pragma: no cover - pydantic is part of the gui extra
+    BaseModel = None  # type: ignore[assignment]
+
+
+def _isolate(value: Any) -> Any:
+    """Return a copy of ``value`` safe for an independent consumer."""
+    if BaseModel is not None and isinstance(value, BaseModel):
+        return value.model_copy(deep=True)
+    return copy.deepcopy(value)
+
 
 DEFAULT_CAPACITY: int = 50
 
@@ -81,21 +94,31 @@ def __len__(self) -> int:
             return len(self._data)
 
     def get(self, key: str) -> Any | None:
+        """Return a fresh deep-copy of the cached value (or None on miss).
+
+        Hits MUST return a distinct object so that a downstream consumer
+        mutating the result cannot corrupt the next reader. Pydantic
+        models pay one model_copy(deep=True); plain dicts/lists pay one
+        copy.deepcopy.
+        """
         with self._lock:
             if key not in self._data:
                 self._misses += 1
                 return None
             self._data.move_to_end(key)
             self._hits += 1
-            return self._data[key]
+            stored = self._data[key]
+        return _isolate(stored)
 
     def set(self, key: str, value: Any) -> None:
+        """Store a deep-copy so the caller can mutate the original safely."""
+        snapshot = _isolate(value)
         with self._lock:
             if key in self._data:
                 self._data.move_to_end(key)
-                self._data[key] = value
+                self._data[key] = snapshot
                 return
-            self._data[key] = value
+            self._data[key] = snapshot
             if len(self._data) > self._capacity:
                 self._data.popitem(last=False)
 

cppmega_v4/jsonrpc/data_methods.py

@@ -72,8 +72,11 @@ def preview_parquet(
     if params.offset < 0:
         raise ValueError(f"offset must be ≥ 0, got {params.offset}")
 
-    cache_key = f"preview::{params.path}::{params.offset}::{params.limit}::" \
-                f"{','.join(sorted(params.channels or []))}"
+    # Distinguish "no filter" (None → all channels) from "explicit empty
+    # filter" ([] → drop all channels) in both cache key and selection.
+    filter_tag = "ALL" if params.channels is None else \
+                 f"FILTER:{','.join(sorted(params.channels))}"
+    cache_key = f"preview::{params.path}::{params.offset}::{params.limit}::{filter_tag}"
     if cache is not None:
         hit = cache.get(cache_key)
         if hit is not None:
@@ -93,8 +96,11 @@ def preview_parquet(
     # bookkeeping. Caller may further restrict via ``params.channels``.
     caps = introspect_parquet(params.path, sample_rows=min(params.limit, 64))
     available = sorted(c for c in caps.side_channels if c != token_col)
-    selected = [c for c in available if not params.channels
-                or c in params.channels]
+    if params.channels is None:
+        selected = available
+    else:
+        wanted = set(params.channels)
+        selected = [c for c in available if c in wanted]
     cols_to_read = [token_col, *selected]
 
     table = pf.read(columns=cols_to_read)

cppmega_v4/jsonrpc/tokenizer_methods.py

@@ -13,6 +13,8 @@
 
 from __future__ import annotations
 
+import hashlib
+import threading
 import time
 from dataclasses import dataclass
 from pathlib import Path
@@ -85,6 +87,7 @@ class _LoadedTokenizer:
 
 
 _TOKENIZER_CACHE: dict[str, _LoadedTokenizer] = {}
+_TOKENIZER_CACHE_LOCK = threading.Lock()
 
 
 def _load(source: str) -> _LoadedTokenizer:
@@ -94,11 +97,15 @@ def _load(source: str) -> _LoadedTokenizer:
     path = Path(source)
     if not path.is_file():
         raise FileNotFoundError(f"tokenizer source not found: {source}")
-    tok = Tokenizer.from_file(str(path))
-    caps = introspect_tokenizer(path)
-    loaded = _LoadedTokenizer(tokenizer=tok, capabilities=caps)
-    _TOKENIZER_CACHE[source] = loaded
-    return loaded
+    with _TOKENIZER_CACHE_LOCK:
+        hit = _TOKENIZER_CACHE.get(source)
+        if hit is not None:
+            return hit
+        tok = Tokenizer.from_file(str(path))
+        caps = introspect_tokenizer(path)
+        loaded = _LoadedTokenizer(tokenizer=tok, capabilities=caps)
+        _TOKENIZER_CACHE[source] = loaded
+        return loaded
 
 
 # ---------------------------------------------------------------------------
@@ -168,8 +175,11 @@ def list_presets() -> ListPresetsResult:
 
 
 def _cache_key(p: EncodeVisualizeParams) -> str:
-    # No layout fields here, so canonical JSON via sorted-keys is fine.
-    return f"tokenize::{p.tokenizer_source}::{p.add_special_tokens}::{hash(p.text)}"
+    # SHA-256 over text so the key is stable across processes — Python's
+    # built-in hash() is salted by PYTHONHASHSEED and would not survive a
+    # server restart.
+    text_digest = hashlib.sha256(p.text.encode("utf-8")).hexdigest()
+    return f"tokenize::{p.tokenizer_source}::{p.add_special_tokens}::{text_digest}"
 
 
 def _caps_to_dict(c: TokenizerCapabilities) -> dict[str, Any]:

cppmega_v4/widget/widget.py

@@ -87,12 +87,24 @@ def __init__(
         self.on_msg(self._on_msg)
 
     def _on_msg(self, _: object, content: Any, _buffers: list[bytes]) -> None:
-        """Route a kernel-bound JSON-RPC envelope to the dispatcher."""
+        """Route a kernel-bound JSON-RPC envelope to the dispatcher.
+
+        Side effect: every successful dispatch also lands in the
+        ``last_result`` traitlet so Python notebook code can observe RPC
+        results without snooping on the wire.
+        """
         if not isinstance(content, dict):
             _log.warning("VisualBuilderWidget: dropped non-dict msg %r", content)
             return
         response = dispatch(content, cache=self._cache)
-        self.send(response.model_dump(mode="json", exclude_none=True))
+        payload = response.model_dump(mode="json", exclude_none=True)
+        self.send(payload)
+        if response.result is not None:
+            self.last_result = {
+                "method": content.get("method"),
+                "id": response.id,
+                "result": response.result,
+            }
 
     @property
     def cache_stats(self) -> dict[str, int]:

tests/v4/test_jsonrpc_methods.py

@@ -108,7 +108,10 @@ def test_verify_cache_hit_short_circuits():
     params = _simple_verify_params()
     r1 = verify(params, cache=cache)
     r2 = verify(params, cache=cache)
-    assert r1 is r2
+    # Cache returns a defensive deep-copy on hit (C1 from review) so
+    # consumers can mutate freely — assert equality, not identity.
+    assert r1 == r2
+    assert r1 is not r2
     stats = cache.stats()
     assert stats["hits"] == 1 and stats["misses"] == 1