From b97aa5a1791337b9617e50339eb193a3721ad365 Mon Sep 17 00:00:00 2001 From: Ace Data Cloud Dev Date: Thu, 6 Aug 2026 08:20:40 +0800 Subject: [PATCH] fix(claude): preserve model selector provenance Co-Authored-By: Claude Opus 5 (1M context) --- coding_bridge/connection.py | 45 ++++++++-------------------- coding_bridge/providers/claude.py | 15 ++++++++-- coding_bridge/session.py | 6 +++- coding_bridge/session_meta.py | 14 +++++++-- tests/test_connection.py | 50 +++++++++++++++++++++---------- tests/test_history.py | 6 ++-- tests/test_session.py | 3 +- tests/test_session_meta.py | 48 ++++++++++++++++++++++++----- 8 files changed, 122 insertions(+), 65 deletions(-) diff --git a/coding_bridge/connection.py b/coding_bridge/connection.py index 1a17ca4..89b6733 100644 --- a/coding_bridge/connection.py +++ b/coding_bridge/connection.py @@ -549,11 +549,10 @@ async def _send_history_detail(self, payload: dict) -> None: ) return detail = await asyncio.to_thread(history.read_session, provider, session_id) - # The transcript carries cwd/model but never effort/permission_mode — fold - # in the sidecar we saved while the session ran so a resume restores all of - # them. cwd stays transcript-authoritative; model does too, except that the - # transcript records the RESOLVED id (`claude-opus-5`) and drops the `[1m]` - # context suffix, so resuming from it would silently downgrade a 1M session. + # A transcript records the provider-resolved model, not the exact selector + # that launched it. Keep those fields separate: only a versioned sidecar can + # supply `model_selector`; legacy `model` values have ambiguous provenance. + resolved_model = detail.pop("model", None) meta = session_meta.load(self.settings.config_dir, session_id) if meta.get("permission_mode"): detail["permission_mode"] = meta["permission_mode"] @@ -561,8 +560,14 @@ async def _send_history_detail(self, payload: dict) -> None: detail["effort"] = meta["effort"] if not detail.get("cwd") and meta.get("cwd"): detail["cwd"] = meta["cwd"] - if meta.get("model") and _prefer_sidecar_model(detail.get("model"), meta["model"]): - detail["model"] = meta["model"] + selector = meta.get("model_selector") + if isinstance(selector, str): + detail["model_selector"] = selector + # Compatibility for older browser clients: `model` is selector-only. + detail["model"] = selector + if isinstance(resolved_model, str): + detail["resolved_model"] = resolved_model + detail["model_contract"] = 2 await self.send_payload(event_payload(Event.HISTORY_DETAIL, session_id, **detail)) async def _send_fs_list(self, payload: dict) -> None: @@ -588,32 +593,6 @@ def _session_overrides(payload: dict) -> dict: return {key: payload[key] for key in ("model", "effort", "permission_mode") if key in payload} -def _prefer_sidecar_model(transcript: object, sidecar: str) -> bool: - """Should the sidecar's model win over the one read from the transcript? - - Yes when the transcript has none, and yes when the sidecar only adds a - context suffix the transcript can't record (`opus[1m]` is logged as - `claude-opus-5`) — otherwise resuming a 1M session silently drops it to - 200k. A genuinely different model still wins from the transcript, which is - the authority on what the session last actually ran. - """ - if not transcript or not isinstance(transcript, str): - return True - if not sidecar.endswith("]") or "[" not in sidecar: - return False - return _model_family(sidecar) in _model_family(transcript) - - -def _model_family(model: str) -> str: - """Strip the `[1m]`-style context suffix and any version tail: opus[1m] -> opus.""" - base = model.split("[", 1)[0].strip().lower() - base = base.removeprefix("claude-") - for tier in ("opus", "sonnet", "haiku"): - if base.startswith(tier): - return tier - return base - - def _is_auth_error(exc: Exception) -> bool: code = getattr(exc, "status_code", None) or getattr(exc, "status", None) if code in (401, 403): diff --git a/coding_bridge/providers/claude.py b/coding_bridge/providers/claude.py index 1e7f60b..435513a 100644 --- a/coding_bridge/providers/claude.py +++ b/coding_bridge/providers/claude.py @@ -239,6 +239,12 @@ async def _ensure_client( "claude-agent-sdk is not installed; run `pip install claude-agent-sdk`" ) from exc resume_id = claude_transcript.prepare_resume(resume) if resume else None + logger.info( + "claude launch session=%s resume=%s model_selector=%r", + resume or self._sdk_session_id or self._session_id, + bool(resume), + model, + ) options = ClaudeAgentOptions( cwd=cwd or None, # Empty means "no --model flag", which lets the CLI apply the user's @@ -401,9 +407,14 @@ def _note_system(self, message: Any) -> None: return data = getattr(message, "data", None) version = data.get("claude_code_version") if isinstance(data, dict) else None - if version: + resolved_model = data.get("model") if isinstance(data, dict) else None + if version or resolved_model: logger.info( - "claude resume init version=%s session=%s", version, self._session_id + "claude init version=%s session=%s model_selector=%r resolved_model=%r", + version, + self._session_id, + self._model, + resolved_model, ) def _with_attachments( diff --git a/coding_bridge/session.py b/coding_bridge/session.py index a72a62b..d7e895e 100644 --- a/coding_bridge/session.py +++ b/coding_bridge/session.py @@ -128,7 +128,7 @@ def _remember_settings(self, sdk_session_id: str) -> None: sdk_session_id, provider=self.provider, cwd=self.cwd, - model=self.model, + model_selector=self.model, permission_mode=self.permission_mode, effort=self.effort, ) @@ -171,6 +171,8 @@ async def start( self.session_id, cwd=self.cwd, model=self.model, + model_selector=self.model, + model_contract=2, provider=self.provider, permission_mode=self.permission_mode, effort=self.effort, @@ -356,6 +358,8 @@ def info(self) -> dict[str, Any]: "provider": self.provider, "cwd": self.cwd, "model": self.model, + "model_selector": self.model, + "model_contract": 2, "permission_mode": self.permission_mode, "effort": self.effort, } diff --git a/coding_bridge/session_meta.py b/coding_bridge/session_meta.py index 85911d8..0e03750 100644 --- a/coding_bridge/session_meta.py +++ b/coding_bridge/session_meta.py @@ -15,8 +15,11 @@ from . import store -# Only settings that aren't reliably recoverable from the transcript alone. -_FIELDS = ("cwd", "model", "permission_mode", "effort", "provider") +# Version 2 distinguishes the exact launch selector from the provider-resolved +# model recorded in transcripts. A legacy `model` value has ambiguous provenance +# and must never be promoted to a selector. +_SCHEMA_VERSION = 2 +_FIELDS = ("version", "cwd", "model_selector", "permission_mode", "effort", "provider") _SAFE_ID = re.compile(r"^[A-Za-z0-9._-]+$") @@ -33,11 +36,16 @@ def save(config_dir: Path | str, sid: str, **fields: Any) -> None: path = _path(config_dir, sid) if path is None: return + selector_supplied = "model_selector" in fields data = {k: v for k, v in fields.items() if k in _FIELDS and v is not None} - if not data: + if not data and not selector_supplied: return existing = store.load(path) or {} + existing.pop("model", None) + if selector_supplied and fields["model_selector"] is None: + existing.pop("model_selector", None) existing.update(data) + existing["version"] = _SCHEMA_VERSION store.save(path, existing) diff --git a/tests/test_connection.py b/tests/test_connection.py index 013de3c..69fbe29 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -659,38 +659,56 @@ def _detail(conn): ) -async def _detail_for(conn, monkeypatch, transcript_model, sidecar_model): - """Run history.get with a stubbed transcript + sidecar, return the reply.""" - from coding_bridge import history, session_meta +async def _detail_for(conn, monkeypatch, transcript_model, sidecar=None): + """Run history.get with stubbed transcript + sidecar, return the reply.""" + from coding_bridge import history, store monkeypatch.setattr( history, "read_session", lambda provider, sid: {"provider": provider, "model": transcript_model, "events": []}, ) - if sidecar_model: - session_meta.save(conn.settings.config_dir, "s1", provider="claude", model=sidecar_model) + if sidecar: + store.save(conn.settings.config_dir / "sessions" / "s1.json", sidecar) await conn._dispatch( {"action": Action.HISTORY_GET, "provider": "claude", "session_id": "s1"} ) return _detail(conn) -async def test_history_detail_restores_the_1m_context_suffix(tmp_path, monkeypatch): - """The transcript logs the resolved id and drops `[1m]`; resuming must not downgrade.""" +async def test_history_detail_separates_selector_from_resolved_model(tmp_path, monkeypatch): conn = _reads_conn(tmp_path) - detail = await _detail_for(conn, monkeypatch, "claude-opus-5", "opus[1m]") - assert detail["model"] == "opus[1m]" + detail = await _detail_for( + conn, + monkeypatch, + "claude-opus-5", + {"version": 2, "provider": "claude", "model_selector": "opus[1m]"}, + ) + assert detail["model_selector"] == "opus[1m]" + assert detail["model"] == "opus[1m]" # compatibility: selector-only + assert detail["resolved_model"] == "claude-opus-5" -async def test_history_detail_keeps_the_transcript_model_when_it_differs(tmp_path, monkeypatch): - """A genuine mid-session model switch is transcript-authoritative, not sidecar.""" +async def test_history_detail_does_not_resume_from_polluted_legacy_model(tmp_path, monkeypatch): conn = _reads_conn(tmp_path) - detail = await _detail_for(conn, monkeypatch, "claude-sonnet-4-5", "opus[1m]") - assert detail["model"] == "claude-sonnet-4-5" + detail = await _detail_for( + conn, + monkeypatch, + "claude-opus-5", + {"provider": "claude", "model": "claude-opus-5"}, + ) + assert "model_selector" not in detail + assert "model" not in detail + assert detail["resolved_model"] == "claude-opus-5" -async def test_history_detail_still_backfills_a_missing_model(tmp_path, monkeypatch): +async def test_history_detail_roundtrips_explicit_bare_selector(tmp_path, monkeypatch): conn = _reads_conn(tmp_path) - detail = await _detail_for(conn, monkeypatch, None, "opus") - assert detail["model"] == "opus" + detail = await _detail_for( + conn, + monkeypatch, + "claude-opus-5", + {"version": 2, "provider": "claude", "model_selector": "opus"}, + ) + assert detail["model_selector"] == "opus" + assert detail["resolved_model"] == "claude-opus-5" diff --git a/tests/test_history.py b/tests/test_history.py index b2b8729..44f3cea 100644 --- a/tests/test_history.py +++ b/tests/test_history.py @@ -800,7 +800,7 @@ async def test_dispatch_history_get_emits_detail(monkeypatch): async def test_dispatch_history_get_folds_in_sidecar(monkeypatch, tmp_path): from coding_bridge import session_meta - # Transcript carries cwd/model but not effort/permission_mode. + # Transcript carries cwd/resolved model but not effort/permission mode or selector. monkeypatch.setattr( history, "read_session", @@ -819,7 +819,9 @@ async def test_dispatch_history_get_folds_in_sidecar(monkeypatch, tmp_path): assert detail["permission_mode"] == "plan" assert detail["effort"] == "high" assert detail["cwd"] == "/repo" # transcript stays authoritative - assert detail["model"] == "opus" + assert detail["resolved_model"] == "opus" + assert "model_selector" not in detail + assert "model" not in detail async def test_dispatch_history_get_requires_params(): diff --git a/tests/test_session.py b/tests/test_session.py index 2e9eae5..37ee302 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -175,8 +175,9 @@ async def ask(*_a): await sess._task saved = session_meta.load(tmp_path, "disk-9") assert saved == { + "version": 2, "cwd": "/repo", - "model": "opus", + "model_selector": "opus", "permission_mode": "acceptEdits", "effort": "high", "provider": "claude", diff --git a/tests/test_session_meta.py b/tests/test_session_meta.py index c4c0170..8b4da3f 100644 --- a/tests/test_session_meta.py +++ b/tests/test_session_meta.py @@ -4,28 +4,41 @@ def test_save_then_load_roundtrip(tmp_path): session_meta.save( - tmp_path, "sid-1", cwd="/repo", model="opus", permission_mode="plan", effort="high" + tmp_path, + "sid-1", + cwd="/repo", + model_selector="opus[1m]", + permission_mode="plan", + effort="high", ) loaded = session_meta.load(tmp_path, "sid-1") assert loaded == { + "version": 2, "cwd": "/repo", - "model": "opus", + "model_selector": "opus[1m]", "permission_mode": "plan", "effort": "high", } def test_save_merges_and_drops_none(tmp_path): - session_meta.save(tmp_path, "sid-1", cwd="/repo", model="opus") - # A later turn only changes effort/mode; cwd/model must survive the merge. - session_meta.save(tmp_path, "sid-1", permission_mode="acceptEdits", effort=None, model=None) + session_meta.save(tmp_path, "sid-1", cwd="/repo", model_selector="opus") + # A later turn only changes effort/mode; cwd/selector must survive the merge. + session_meta.save(tmp_path, "sid-1", permission_mode="acceptEdits", effort=None) loaded = session_meta.load(tmp_path, "sid-1") assert loaded["cwd"] == "/repo" - assert loaded["model"] == "opus" + assert loaded["model_selector"] == "opus" assert loaded["permission_mode"] == "acceptEdits" assert "effort" not in loaded +def test_explicit_default_clears_a_saved_selector(tmp_path): + session_meta.save(tmp_path, "sid-1", model_selector="opus[1m]") + session_meta.save(tmp_path, "sid-1", model_selector=None) + loaded = session_meta.load(tmp_path, "sid-1") + assert loaded == {"version": 2} + + def test_load_missing_is_empty(tmp_path): assert session_meta.load(tmp_path, "nope") == {} @@ -39,4 +52,25 @@ def test_unsafe_id_is_ignored(tmp_path): def test_ignores_unknown_fields(tmp_path): session_meta.save(tmp_path, "sid-1", cwd="/repo", secret="x") - assert session_meta.load(tmp_path, "sid-1") == {"cwd": "/repo"} + assert session_meta.load(tmp_path, "sid-1") == {"version": 2, "cwd": "/repo"} + + +def test_legacy_resolved_model_is_not_promoted_to_selector(tmp_path): + from coding_bridge import store + + store.save( + tmp_path / "sessions" / "sid-1.json", + {"cwd": "/repo", "model": "claude-opus-5", "permission_mode": "plan"}, + ) + legacy = session_meta.load(tmp_path, "sid-1") + assert legacy["model"] == "claude-opus-5" + assert "model_selector" not in legacy + + session_meta.save(tmp_path, "sid-1", effort="high") + migrated = session_meta.load(tmp_path, "sid-1") + assert migrated == { + "version": 2, + "cwd": "/repo", + "permission_mode": "plan", + "effort": "high", + }