phase: 178_split_thinking_levels
## Phase 178 — final verification pass: 100% green
**Verified (all 5 tasks complete in `complete/178_split_thinking_levels/`):**
- `config.py` rename (`llm_chat_thinking_level` default `low`) + new `llm_summary_thinking_level` (default `""`); two `CATALOG` rows; `.env.example` replaced; no stale old-key references (only intentional negative pins in tests)
- Wire split: `chat_stream()` → chat level, one-shot `chat()` → summary level (absent `extra_body` when empty); per-turn line reads chat level only
**Gates:**
- `uv run pytest -k "config and thinking" -v` → 7 passed (defaults + env overrides both knobs)
- `uv run pytest` (unit + integration) → 4170 tests, exit 0; `--cov=app` → **99%** (>90% ✓)
- `uv run ruff check .` clean; `uv run pyright` clean
- `uv run pytest tests/e2e/test_split_thinking_levels.py -v --no-cov` → 3 passed in isolation
- `test_admin_settings_panel.py` + `test_admin_settings_layout.py` → 17 passed in isolation
**Completion criteria:** all 6 test/doc criteria ✅ (admin GET lists both keys, old key gone — integration + E2E pins); commit + phase-dir move = harness's job.
**Documentation:** README.md — both new env vars correctly documented in the LLM (aipi) table with defaults/wire-mapping/off-rule, defaults match `config.py`, all `.env.example` vars covered: verified consistent, no changes needed. AGENTS.md — all referenced files/commands verified to exist, no phase-178 impact (no UI code change, A4): verified consistent, no changes needed.
**Decisions:** none required — no live changes made (E2E booted its own app subprocess against the existing DB).
**Next pending phase:** none — 178 is the last phase in `todo/`.
This commit is contained in:
+116
-33
@@ -1324,48 +1324,62 @@ def test_chat_retry_delay_is_flat_never_backoff(monkeypatch: pytest.MonkeyPatch)
|
||||
assert sleeps == [1.25, 1.25, 1.25]
|
||||
|
||||
|
||||
# ---------- thinking level passthrough (phase 172, task 03) ----------
|
||||
# ---------- thinking level passthrough (phase 172; the split, 178 A3) ----------
|
||||
# chat() (one-shot) rides the SUMMARY level; chat_stream() rides the CHAT
|
||||
# level; an empty (trimmed) level is the off switch (A2 — the parameter
|
||||
# is absent, never a literal "none").
|
||||
|
||||
|
||||
def test_chat_sends_reasoning_effort_extra_body_when_level_set() -> None:
|
||||
"""Phase 172 (A1): a non-empty level rides the one-shot request as
|
||||
the top-level ``reasoning_effort`` parameter via the SDK's
|
||||
# --- the one-shot path: the summary level (phase 178, A3) ---
|
||||
|
||||
|
||||
def test_chat_sends_thinking_level_extra_body_when_summary_level_set() -> None:
|
||||
"""Phase 178 (A3): a non-empty summary level rides the one-shot
|
||||
request as the top-level ``reasoning_effort`` parameter via the SDK's
|
||||
``extra_body``."""
|
||||
llm, completions = _make_chat_client(
|
||||
_FakeCompletion("ok"), llm_thinking_level="low"
|
||||
_FakeCompletion("ok"), llm_summary_thinking_level="high"
|
||||
)
|
||||
asyncio.run(llm.chat([{"role": "user", "content": "q"}]))
|
||||
assert completions.chat_kwargs is not None
|
||||
assert completions.chat_kwargs["extra_body"] == {"reasoning_effort": "low"}
|
||||
assert completions.chat_kwargs["extra_body"] == {"reasoning_effort": "high"}
|
||||
|
||||
|
||||
def test_chat_default_level_low_is_always_sent() -> None:
|
||||
"""The default (``low``) is always sent (A2): a client built without
|
||||
naming the setting still carries the parameter."""
|
||||
def test_chat_default_summary_thinking_off_is_pre_phase172_byte_identical() -> None:
|
||||
"""The default summary level is EMPTY (the TODO's "none", A2): a
|
||||
client built without naming either setting sends NO ``extra_body`` —
|
||||
one-shot requests are byte-identical to pre-phase-172."""
|
||||
llm, completions = _make_chat_client(_FakeCompletion("ok"))
|
||||
asyncio.run(llm.chat([{"role": "user", "content": "q"}]))
|
||||
assert completions.chat_kwargs is not None
|
||||
assert completions.chat_kwargs["extra_body"] == {"reasoning_effort": "low"}
|
||||
assert "extra_body" not in completions.chat_kwargs
|
||||
assert set(completions.chat_kwargs) == {
|
||||
"model",
|
||||
"messages",
|
||||
"temperature",
|
||||
"max_tokens",
|
||||
"stream",
|
||||
}
|
||||
|
||||
|
||||
def test_chat_trims_the_level_before_sending() -> None:
|
||||
"""TRIM: a hand-edited ``.env`` value with trailing whitespace
|
||||
(``"low "``) is sent trimmed — the raw stored string never reaches
|
||||
the wire with surrounding whitespace."""
|
||||
def test_chat_trims_the_summary_thinking_level_before_sending() -> None:
|
||||
"""TRIM: a hand-edited ``.env`` value with surrounding whitespace
|
||||
(``" high "``) is sent trimmed — the raw stored string never reaches
|
||||
the wire with whitespace."""
|
||||
llm, completions = _make_chat_client(
|
||||
_FakeCompletion("ok"), llm_thinking_level="low "
|
||||
_FakeCompletion("ok"), llm_summary_thinking_level=" high "
|
||||
)
|
||||
asyncio.run(llm.chat([{"role": "user", "content": "q"}]))
|
||||
assert completions.chat_kwargs is not None
|
||||
assert completions.chat_kwargs["extra_body"] == {"reasoning_effort": "low"}
|
||||
assert completions.chat_kwargs["extra_body"] == {"reasoning_effort": "high"}
|
||||
|
||||
|
||||
def test_chat_empty_level_absent_from_kwargs_byte_identical() -> None:
|
||||
"""Phase 172 (A2): a whitespace value is the off switch —
|
||||
``extra_body`` is ABSENT (never None, never ``"off"``) and the
|
||||
request kwargs keep the pre-phase-172 shape."""
|
||||
def test_chat_empty_summary_thinking_level_absent_from_kwargs_byte_identical() -> None:
|
||||
"""Phase 172 (A2): an empty value is the off switch —
|
||||
``extra_body`` is ABSENT (never None, never a literal ``"none"``) and
|
||||
the request kwargs keep the pre-phase-172 shape."""
|
||||
llm, completions = _make_chat_client(
|
||||
_FakeCompletion("ok"), llm_thinking_level=" "
|
||||
_FakeCompletion("ok"), llm_summary_thinking_level=""
|
||||
)
|
||||
asyncio.run(llm.chat([{"role": "user", "content": "q"}]))
|
||||
assert completions.chat_kwargs is not None
|
||||
@@ -1379,30 +1393,42 @@ def test_chat_empty_level_absent_from_kwargs_byte_identical() -> None:
|
||||
}
|
||||
|
||||
|
||||
def test_chat_stream_sends_reasoning_effort_extra_body_when_level_set() -> None:
|
||||
"""Phase 172 (A1): the chat turn rides the parameter too."""
|
||||
def test_chat_whitespace_summary_thinking_level_is_the_off_switch() -> None:
|
||||
"""Whitespace-only (``" "``) is empty for wire purposes (A2)."""
|
||||
llm, completions = _make_chat_client(
|
||||
_FakeCompletion("ok"), llm_summary_thinking_level=" "
|
||||
)
|
||||
asyncio.run(llm.chat([{"role": "user", "content": "q"}]))
|
||||
assert completions.chat_kwargs is not None
|
||||
assert "extra_body" not in completions.chat_kwargs
|
||||
|
||||
|
||||
def test_chat_stream_sends_thinking_level_extra_body_when_level_set() -> None:
|
||||
"""Phase 178 (A3): the chat turn rides the parameter from the
|
||||
**chat** level."""
|
||||
llm, completions = _make_stream_client(
|
||||
[_chunk("x")], llm_thinking_level="high"
|
||||
[_chunk("x")], llm_chat_thinking_level="high"
|
||||
)
|
||||
asyncio.run(_collect(llm, [{"role": "user", "content": "q"}]))
|
||||
assert completions.kwargs is not None
|
||||
assert completions.kwargs["extra_body"] == {"reasoning_effort": "high"}
|
||||
|
||||
|
||||
def test_chat_stream_trims_the_level_before_sending() -> None:
|
||||
def test_chat_stream_trims_the_thinking_level_before_sending() -> None:
|
||||
llm, completions = _make_stream_client(
|
||||
[_chunk("x")], llm_thinking_level=" high "
|
||||
[_chunk("x")], llm_chat_thinking_level=" high "
|
||||
)
|
||||
asyncio.run(_collect(llm, [{"role": "user", "content": "q"}]))
|
||||
assert completions.kwargs is not None
|
||||
assert completions.kwargs["extra_body"] == {"reasoning_effort": "high"}
|
||||
|
||||
|
||||
def test_chat_stream_empty_level_absent_from_kwargs_byte_identical() -> None:
|
||||
"""Phase 172 (A2): the stream's off switch — ``extra_body`` absent
|
||||
(never None) and the kwargs dict keeps the pre-phase-172 shape."""
|
||||
def test_chat_stream_empty_thinking_level_absent_from_kwargs_byte_identical() -> None:
|
||||
"""Phase 178 (A2): the stream's off switch — ``extra_body`` absent
|
||||
(never None, never a literal ``"none"``) and the kwargs dict keeps
|
||||
the pre-phase-172 shape."""
|
||||
llm, completions = _make_stream_client(
|
||||
[_chunk("x")], llm_thinking_level=""
|
||||
[_chunk("x")], llm_chat_thinking_level=""
|
||||
)
|
||||
asyncio.run(_collect(llm, [{"role": "user", "content": "q"}]))
|
||||
assert completions.kwargs is not None
|
||||
@@ -1416,22 +1442,79 @@ def test_chat_stream_empty_level_absent_from_kwargs_byte_identical() -> None:
|
||||
}
|
||||
|
||||
|
||||
def test_chat_stream_whitespace_level_is_the_off_switch() -> None:
|
||||
def test_chat_stream_whitespace_thinking_level_is_the_off_switch() -> None:
|
||||
"""Whitespace-only (``" "``) is empty for wire purposes (A2)."""
|
||||
llm, completions = _make_stream_client(
|
||||
[_chunk("x")], llm_thinking_level=" "
|
||||
[_chunk("x")], llm_chat_thinking_level=" "
|
||||
)
|
||||
asyncio.run(_collect(llm, [{"role": "user", "content": "q"}]))
|
||||
assert completions.kwargs is not None
|
||||
assert "extra_body" not in completions.kwargs
|
||||
|
||||
|
||||
# --- the cross-pins (phase 178, A3 — the whole point of the split) ---
|
||||
|
||||
|
||||
def _make_both_client(
|
||||
chunks: list,
|
||||
completion: _FakeCompletion,
|
||||
**settings_kwargs: Any,
|
||||
) -> tuple[LLMClient, _FakeCompletions]:
|
||||
"""One client for BOTH paths (a shared fake ``completions`` serves the
|
||||
streaming and the non-streaming ``create()`` alike) so the two levels
|
||||
can be observed on one client."""
|
||||
completions = _FakeCompletions(chunks, None, completion)
|
||||
fake_openai = SimpleNamespace(chat=SimpleNamespace(completions=completions))
|
||||
llm = LLMClient(_settings(**settings_kwargs))
|
||||
llm._client = fake_openai # pyright: ignore[reportAttributeAccessIssue]
|
||||
return llm, completions
|
||||
|
||||
|
||||
def test_split_thinking_levels_inverted_defaults_chat_high_summary_off() -> None:
|
||||
"""Phase 178 (A3): the defaults INVERTED — the chat level ``high``
|
||||
rides ``chat_stream`` while the (empty) summary level keeps ``chat``
|
||||
free of ``extra_body``. The two knobs are independent on the wire."""
|
||||
llm, completions = _make_both_client(
|
||||
[_chunk("x")],
|
||||
_FakeCompletion("ok"),
|
||||
llm_chat_thinking_level="high",
|
||||
llm_summary_thinking_level="",
|
||||
)
|
||||
asyncio.run(_collect(llm, [{"role": "user", "content": "q"}]))
|
||||
assert completions.kwargs is not None
|
||||
assert completions.kwargs["extra_body"] == {"reasoning_effort": "high"}
|
||||
asyncio.run(llm.chat([{"role": "user", "content": "q"}]))
|
||||
assert completions.chat_kwargs is not None
|
||||
assert "extra_body" not in completions.chat_kwargs
|
||||
|
||||
|
||||
def test_split_thinking_levels_default_client_stream_low_summary_off() -> None:
|
||||
"""The DEFAULTS (chat ``low`` + summary ``""``) — the stream carries
|
||||
``reasoning_effort="low"`` while the one-shot request is
|
||||
byte-identical to pre-phase-172 (no ``extra_body``; the TODO's
|
||||
"summary should be none")."""
|
||||
llm, completions = _make_both_client([_chunk("x")], _FakeCompletion("ok"))
|
||||
asyncio.run(_collect(llm, [{"role": "user", "content": "q"}]))
|
||||
assert completions.kwargs is not None
|
||||
assert completions.kwargs["extra_body"] == {"reasoning_effort": "low"}
|
||||
asyncio.run(llm.chat([{"role": "user", "content": "q"}]))
|
||||
assert completions.chat_kwargs is not None
|
||||
assert "extra_body" not in completions.chat_kwargs
|
||||
assert set(completions.chat_kwargs) == {
|
||||
"model",
|
||||
"messages",
|
||||
"temperature",
|
||||
"max_tokens",
|
||||
"stream",
|
||||
}
|
||||
|
||||
|
||||
def test_embeddings_body_never_carries_reasoning_effort() -> None:
|
||||
"""Phase 172 (A3): the embeddings path is untouched — with the level
|
||||
set, every POST body stays the minimal ``{model, input}`` payload;
|
||||
``reasoning_effort`` never appears in it."""
|
||||
service = _FakeEmbeddingsService()
|
||||
llm, http = _make_client(service, llm_thinking_level="high")
|
||||
llm, http = _make_client(service, llm_chat_thinking_level="high")
|
||||
asyncio.run(llm.embed(["a", "b"]))
|
||||
vec = asyncio.run(llm.embed_one("c"))
|
||||
assert len(vec) == 768
|
||||
|
||||
Reference in New Issue
Block a user