diff --git a/tests/test_session.py b/tests/test_session.py index f41addd5..97384496 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -997,6 +997,120 @@ class TestTitleRetry: # Flag stays True after successful generation assert session._title_generated is True + def test_title_sanitizes_thinking_model_output(self, tmp_db): + """A reasoning model's answer can arrive wrapped in an unparsed + ```` span (lanes that don't split it into reasoning_content) + plus markdown / quotes. There is no portable switch to disable thinking, + so the title pass gives reasoning room (raised max_tokens), reuses + ``_strip_reasoning``, and peels wrapping decoration — keeping INTERNAL + punctuation (the hyphen survives).""" + from turnstone.core.providers._protocol import ModelCapabilities + from turnstone.core.session import _TITLE_MAX_TOKENS + + session = _make_session() + session._title_generated = True + session.messages = turns_from_dicts([{"role": "user", "content": "hi"}]) + result = MagicMock() + result.content = ( + "The user greets me; a fitting title would be...\n\n" + '**"Cluster Routing Deep-Dive"**' + ) + session._provider = MagicMock() + session._provider.get_capabilities.return_value = ModelCapabilities() + session._provider.create_completion.return_value = result + + captured: dict[str, str] = {} + with patch( + "turnstone.core.session.update_workstream_title", + side_effect=lambda ws_id, title: captured.update(title=title), + ): + session._generate_title() + + assert captured["title"] == "Cluster Routing Deep-Dive" + # Reasoning gets room to finish rather than a 200-token squeeze that + # the think pass swallows whole (the empty-content regression); and the + # title call forces no temperature — it defers to the session value. + _, kw = session._provider.create_completion.call_args + assert kw["max_tokens"] == _TITLE_MAX_TOKENS + assert kw["temperature"] == session.temperature + + def test_title_skipped_when_reasoning_consumes_whole_budget(self, tmp_db): + """If the budget is spent inside an unclosed ```` (the empty/ + cut-off content that broke titling), the cleaner yields no words — so + nothing is persisted rather than a fragment of reasoning becoming the + title.""" + from turnstone.core.providers._protocol import ModelCapabilities + + session = _make_session() + session._title_generated = True + session.messages = turns_from_dicts([{"role": "user", "content": "hi"}]) + result = MagicMock() + result.content = "still reasoning, never closed before the cap" + session._provider = MagicMock() + session._provider.get_capabilities.return_value = ModelCapabilities() + session._provider.create_completion.return_value = result + + with patch("turnstone.core.session.update_workstream_title") as upd: + session._generate_title() + + upd.assert_not_called() + + def test_title_strips_reasoning_variants(self, tmp_db): + """Reasoning reaches ``content`` in several shapes the title pass must + survive: an opener-absent ``…`` (templates that pre-inject the + opening tag), a paired ```` block, and a trailing + explanation after the title (only the first non-empty line is kept).""" + from turnstone.core.providers._protocol import ModelCapabilities + + cases = [ + ("I should weigh the options here\n\nRendezvous Routing", "Rendezvous Routing"), + ( + "pondering the ask\nCluster Health Digest", + "Cluster Health Digest", + ), + ("Auth Layer Refactor\n\nThis title captures the request well.", "Auth Layer Refactor"), + ] + for content, expected in cases: + session = _make_session() + session._title_generated = True + session.messages = turns_from_dicts([{"role": "user", "content": "hi"}]) + result = MagicMock() + result.content = content + session._provider = MagicMock() + session._provider.get_capabilities.return_value = ModelCapabilities() + session._provider.create_completion.return_value = result + + captured: dict[str, str] = {} + with patch( + "turnstone.core.session.update_workstream_title", + side_effect=lambda ws_id, title, _c=captured: _c.update(title=title), + ): + session._generate_title() + assert captured.get("title") == expected, (content, captured) + + def test_title_truncates_to_max_chars(self, tmp_db): + """The ``[:_TITLE_MAX_CHARS]`` slice is the only length guard now that + the persist-time ``title[:80]`` is gone — a long title is bounded.""" + from turnstone.core.providers._protocol import ModelCapabilities + from turnstone.core.session import _TITLE_MAX_CHARS + + session = _make_session() + session._title_generated = True + session.messages = turns_from_dicts([{"role": "user", "content": "hi"}]) + result = MagicMock() + result.content = "Story " * 40 # 240 chars on one line + session._provider = MagicMock() + session._provider.get_capabilities.return_value = ModelCapabilities() + session._provider.create_completion.return_value = result + + captured: dict[str, str] = {} + with patch( + "turnstone.core.session.update_workstream_title", + side_effect=lambda ws_id, title: captured.update(title=title), + ): + session._generate_title() + assert len(captured["title"]) == _TITLE_MAX_CHARS + def test_title_skipped_after_resume_changes_ws_id(self, tmp_db): """If ws_id changes (via resume) during title generation, discard the result.""" from turnstone.core.providers._protocol import ModelCapabilities @@ -5433,6 +5547,29 @@ def test_utility_completion_records_aux_usage(): assert rec["model"] == "test-model" +def test_utility_completion_defers_temperature_to_session(): + """Utility calls (title, compaction, web-fetch extraction) must NOT force a + temperature: an unset temperature resolves to the session/registry value, so + one operator-set ``[models.*]`` temperature governs every lane and code never + fights a thinking/no-temp model by hard-coding a constant. An explicit + override still wins for any caller that genuinely needs one.""" + from turnstone.core.providers._protocol import CompletionResult, ModelCapabilities + + session = _make_session() + session.temperature = 0.42 + session._provider = MagicMock() + session._provider.get_capabilities.return_value = ModelCapabilities() + session._provider.create_completion.return_value = CompletionResult(content="x") + + session._utility_completion([{"role": "user", "content": "hi"}]) + _, kw = session._provider.create_completion.call_args + assert kw["temperature"] == 0.42 # deferred to the session/registry value + + session._utility_completion([{"role": "user", "content": "hi"}], temperature=0.9) + _, kw2 = session._provider.create_completion.call_args + assert kw2["temperature"] == 0.9 # explicit override still honored + + def test_record_aux_usage_skips_when_usage_missing(): """A provider that reports no usage object must not emit a phantom zero-token row.""" diff --git a/turnstone/core/session.py b/turnstone/core/session.py index cb352fe7..5b0a8a31 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -616,6 +616,27 @@ _SKILL_ARG_NAME_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") # excludes the ``$ARGUMENTS[N]`` form which is a different placeholder. _SPEC_ARGUMENTS_LITERAL_RE = re.compile(r"\$ARGUMENTS\b(?!\[)") +# Auto-title generation runs on the session's own (often thinking-capable) +# utility model, and there is no portable way to turn reasoning off: +# ``enable_thinking`` is Qwen-only and commercial providers each differ. A +# small ``max_tokens`` lets the reasoning pass swallow the entire budget so the +# title text never lands (``finish_reason=length``, empty ``content`` → skip). +# This hits auto-title and refresh alike (shared path) on any thinking model. +# So give the think pass room (``_TITLE_MAX_TOKENS``), then recover the title +# from ``content``: reuse :meth:`ChatSession._strip_reasoning` (the canonical +# ````/```` remover, for lanes that leave reasoning inline +# rather than in ``reasoning_content``), take the first non-empty line (a model +# that appends an explanation shouldn't fold prose into the title), then peel a +# ``Title:`` label and wrapping markdown/quote decoration. Internal punctuation +# is preserved so ``.NET``, ``CI/CD``, ``v1.6.0`` survive. +_TITLE_MAX_TOKENS = 2048 +# Match the manual-rename (alias) cap so generated and hand-set titles share +# one length bound. +_TITLE_MAX_CHARS = 80 +_TITLE_LABEL_RE = re.compile(r"(?i)^\s*title\s*[:\-—]\s*") +# Wrapping decoration peeled off both ends of a generated title. +_TITLE_WRAP_CHARS = "*`\"' " + # Soft cap on ``"watch_triggered"`` entries in the per-session NudgeQueue. # The pull-model path batches N watch fires into ONE envelope splice on @@ -2533,8 +2554,11 @@ class ChatSession: snippet += "\n\nTitle:" log.info("ws.title.llm_call_start", ws_id=ws_id[:8]) - # Use slightly higher temperature for refreshes to encourage variety - temp = 0.7 if current_title else 0.3 + # No temperature override here: defer to the session/registry + # temperature (``_utility_completion`` default). A manual refresh + # leans on the prompt's "generate a DIFFERENT title" instruction and + # the changing ``current_title`` it feeds in for variety, rather than + # forcing a hotter sample on top of the operator's chosen model. result = self._utility_completion( [ @@ -2551,17 +2575,28 @@ class ChatSession: }, {"role": "user", "content": snippet}, ], - max_tokens=200, - temperature=temp, + max_tokens=_TITLE_MAX_TOKENS, ) - raw = (result.content or "").strip() + raw = result.content or "" log.info("ws.title.llm_response", ws_id=ws_id[:8], raw=raw[:200]) - # Take first line, strip quotes - title = raw.split("\n")[0].strip().strip('"').strip("'") + # Take the assistant's answer (``content``), never its reasoning. + # Reuse the canonical reasoning stripper, then drop a leftover close + # tag from lanes that pre-inject the opening ```` into the + # prompt (only ```` reaches ``content``). See ``_TITLE_*``. + stripped = self._strip_reasoning(raw) + for _close in ("", ""): + _pos = stripped.rfind(_close) + if _pos != -1: + stripped = stripped[_pos + len(_close) :] + # First non-empty line, with a ``Title:`` label and wrapping + # markdown/quote decoration peeled (internal punctuation kept). + line = next((ln for ln in stripped.splitlines() if ln.strip()), "") + line = _TITLE_LABEL_RE.sub("", line.strip(_TITLE_WRAP_CHARS)) + title = line.strip(_TITLE_WRAP_CHARS)[:_TITLE_MAX_CHARS] if title and self._ws_id == ws_id: log.info("ws.title.updating", ws_id=ws_id[:8], title=title) - update_workstream_title(ws_id, title[:80]) - self.ui.on_rename(title[:80]) + update_workstream_title(ws_id, title) + self.ui.on_rename(title) log.info("ws.title.success", ws_id=ws_id[:8], title=title) else: log.info( @@ -3548,7 +3583,7 @@ class ChatSession: messages: list[dict[str, Any]], *, max_tokens: int = 4096, - temperature: float = 0.3, + temperature: float | None = None, reasoning_effort: str = "low", ) -> CompletionResult: """Run a lightweight internal completion (title gen, compaction, extraction). @@ -3557,6 +3592,13 @@ class ChatSession: commercial providers) and ``extra_params`` (for local model servers) so callers don't need to duplicate it. ``max_tokens`` is clamped to the model's advertised output limit so small models don't error. + + ``temperature`` defaults to the session temperature (``self.temperature``) + — the same operator/registry-resolved value the main turn uses — rather + than a hard-coded constant: utility calls should not silently override an + explicit ``[models.*]`` temperature. The provider still drops it for + models that forbid temperature (GPT-5 base, O-series) or pins it (Claude + with thinking), so this only governs models that genuinely accept one. """ caps = self._get_capabilities() clamped = min(max_tokens, caps.max_output_tokens) if caps.max_output_tokens else max_tokens @@ -3566,7 +3608,7 @@ class ChatSession: model=self.model, messages=messages, max_tokens=clamped, - temperature=temperature, + temperature=self.temperature if temperature is None else temperature, reasoning_effort=reasoning_effort, extra_params=self._provider_extra_params(), capabilities=caps, @@ -13033,7 +13075,8 @@ class ChatSession: # Phase 3: summarization API call. # Use a generous max_tokens so thinking models don't starve the # visible answer, and pass reasoning_effort="low" to avoid wasting - # budget on deep reasoning for a simple extraction task. + # budget on deep reasoning for a simple extraction task. Temperature + # is left to the session/registry default rather than overridden here. try: result = self._utility_completion( [ @@ -13057,7 +13100,6 @@ class ChatSession: }, ], max_tokens=8192, - temperature=0.2, ) answer = result.content or "" if not answer: