phase: 172_thinking_level
All verification complete. Final report: **Phase 172 final verification pass — all tasks (01–05) were already complete; verified the whole phase, no code fixes needed.** - **Verified implementation:** `Settings.llm_thinking_level: str = "low"` + `CATALOG` row (`LLM (aipi)` / "Thinking level", plain: not secret/restart/overridable-exempt) + `.env.example` line under `BOR_LLM_CHAT_MODEL` + README table row; `_thinking_extra_body()` → `reasoning_effort` via `extra_body` on `chat()` + `chat_stream()` when trimmed-non-empty, key absent (byte-identical) when empty, embeddings untouched; per-turn line gains `thinking=<value>` (`-` when empty) before `thinking_chars`. - **Tests/lint/coverage (all green):** `uv run pytest` → 4084 passed · `uv run pytest --cov=app --cov-report=term-missing` → 4084 passed, `app/` **99%** (>90%) · `uv run ruff check .` → All checks passed · `uv run pyright` → 0 errors, 0 warnings. - **Completion criteria:** (1) `pytest -k "config and thinking"` 4 passed + `.env.example` grep ✓ (2) live `GET /api/admin/settings` curl shows the row (str, `low`, `default`) + integration PUT/DELETE round-trip ✓ (3) 9 mocked-client unit tests assert `extra_body` present/absent ✓ (4) log-line tests assert `low`/`high`/`-`/whitespace ✓ (5) suite + coverage + lint ✓ (6) `test_thinking_level.py` 3/3 in isolation; `test_admin_settings_panel.py` 12/12; `test_admin_settings_layout.py` 5/5 ✓ (7) atomic commit + phase move = the harness's job (task files already in `complete/`; I ran no git index/history commands). - **Documentation:** README.md reviewed in full — verified consistent (env table complete vs `.env.example`, catalog restart/non-overridable claims re-verified at runtime); AGENTS.md reviewed in full — verified consistent (no rule affected, A5 = no UI change). No changes needed. **Flag (not fixable here):** PLAN.md §9's per-turn-line spec predates the phase-172 `thinking=` field; PLAN.md is off-limits per protocol. - **Next pending phase:** none — `todo/` contains only phase 172 (the harness moves it on this pass's success).
This commit is contained in:
@@ -1547,22 +1547,33 @@ def test_endpoint_log_line_records_suggested_after_summary_hits(
|
||||
``suggested=N`` immediately AFTER ``summary_hits=N`` — the field
|
||||
order of every existing field is untouched (the phase-114
|
||||
``retries=N scaffold_stripped=N`` tail stays last). The value is the
|
||||
seeded suggestion tier's size (``len(plan.suggested_docs)``)."""
|
||||
seeded suggestion tier's size (``len(plan.suggested_docs)``).
|
||||
Phase 172 (A6): the line gains ``thinking=<level>`` immediately
|
||||
BEFORE ``thinking_chars=N`` — the effective (overlay-resolved)
|
||||
``BOR_LLM_THINKING_LEVEL``, trimmed, ``-`` when empty (the off
|
||||
switch)."""
|
||||
_session, _llm = gate_env
|
||||
doc = _doc("Kubernetes Homelab Cluster", "TALOS_DOC_SENT")
|
||||
chunks = [_chunk(doc, 0.30), _chunk(doc, 0.20, is_summary=True)]
|
||||
monkeypatch.setattr(chat_api, "retrieve", _fake_retriever(chunks))
|
||||
|
||||
with caplog.at_level("INFO", logger="app.chat"):
|
||||
_ask(client, "How is my Kubernetes cluster set up?")
|
||||
def _last_log_line() -> str:
|
||||
caplog.clear()
|
||||
with caplog.at_level("INFO", logger="app.chat"):
|
||||
_ask(client, "How is my Kubernetes cluster set up?")
|
||||
log_lines = [r.getMessage() for r in caplog.records if "question=" in r.getMessage()]
|
||||
assert log_lines
|
||||
return log_lines[-1]
|
||||
|
||||
log_lines = [r.getMessage() for r in caplog.records if "question=" in r.getMessage()]
|
||||
assert log_lines
|
||||
line = log_lines[-1]
|
||||
# The default (the fixture's settings carry the field default):
|
||||
# thinking=low, immediately before thinking_chars.
|
||||
line = _last_log_line()
|
||||
assert "thinking=low thinking_chars=" in line
|
||||
# The new slot: suggested=N right after summary_hits=N (one suggested
|
||||
# doc — the single fixture doc — and one summary hit on it).
|
||||
assert "summary_hits=1 suggested=1" in line
|
||||
# The full field order (the phase-114 tail stays last).
|
||||
# The full field order (the phase-114 tail stays last; phase 172
|
||||
# inserts thinking= between sources= and thinking_chars=).
|
||||
order = (
|
||||
"question=",
|
||||
"embed_ms=",
|
||||
@@ -1576,6 +1587,7 @@ def test_endpoint_log_line_records_suggested_after_summary_hits(
|
||||
"threshold=",
|
||||
"deflected=",
|
||||
"sources=",
|
||||
"thinking=",
|
||||
"thinking_chars=",
|
||||
"tool_calls=",
|
||||
"rounds=",
|
||||
@@ -1589,3 +1601,31 @@ def test_endpoint_log_line_records_suggested_after_summary_hits(
|
||||
pos = line.find(field)
|
||||
assert pos > idx, f"{field} out of order in the per-turn log line"
|
||||
idx = pos
|
||||
|
||||
# The effective value: a non-default setting rides the line as-is.
|
||||
monkeypatch.setattr(
|
||||
chat_api,
|
||||
"get_settings",
|
||||
lambda: Settings(_env_file=None, relevance_threshold=0.30, llm_thinking_level="high"), # pyright: ignore[reportCallIssue]
|
||||
)
|
||||
line = _last_log_line()
|
||||
assert "thinking=high thinking_chars=" in line
|
||||
|
||||
# The off switch: an empty setting logs ``-`` ...
|
||||
monkeypatch.setattr(
|
||||
chat_api,
|
||||
"get_settings",
|
||||
lambda: Settings(_env_file=None, relevance_threshold=0.30, llm_thinking_level=""), # pyright: ignore[reportCallIssue]
|
||||
)
|
||||
line = _last_log_line()
|
||||
assert "thinking=- thinking_chars=" in line
|
||||
|
||||
# ... and so does a whitespace-only one (A6: the TRIMMED value
|
||||
# decides, the log field mirrors the wire's trim).
|
||||
monkeypatch.setattr(
|
||||
chat_api,
|
||||
"get_settings",
|
||||
lambda: Settings(_env_file=None, relevance_threshold=0.30, llm_thinking_level=" "), # pyright: ignore[reportCallIssue]
|
||||
)
|
||||
line = _last_log_line()
|
||||
assert "thinking=- thinking_chars=" in line
|
||||
|
||||
@@ -105,6 +105,34 @@ def test_llm_summary_model_env_override(monkeypatch) -> None:
|
||||
assert s.llm_summary_model == "mini"
|
||||
|
||||
|
||||
def test_llm_thinking_level_default_is_low(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Phase 172: the model's thinking level defaults to ``low``
|
||||
(``BOR_LLM_THINKING_LEVEL`` unset)."""
|
||||
monkeypatch.delenv("BOR_LLM_THINKING_LEVEL", raising=False)
|
||||
s = _settings()
|
||||
assert s.llm_thinking_level == "low"
|
||||
|
||||
|
||||
def test_llm_thinking_level_env_override(monkeypatch) -> None:
|
||||
"""Phase 172 (A4): free-form like ``llm_chat_model`` —
|
||||
``BOR_LLM_THINKING_LEVEL=high`` resolves to ``"high"`` (no
|
||||
allowlist, no validator)."""
|
||||
monkeypatch.setenv("BOR_LLM_THINKING_LEVEL", "high")
|
||||
s = _settings()
|
||||
assert s.llm_thinking_level == "high"
|
||||
|
||||
|
||||
def test_llm_thinking_level_empty_env_value_stays_empty(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Phase 172 (A2): the EMPTY value is the off switch — it must not
|
||||
be coerced back to the default (trimming happens only at the wire,
|
||||
task 03; no normalization at load)."""
|
||||
monkeypatch.setenv("BOR_LLM_THINKING_LEVEL", "")
|
||||
s = _settings()
|
||||
assert s.llm_thinking_level == ""
|
||||
|
||||
|
||||
def test_summary_max_chars_default_and_env_override(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Phase 30: document content sent to the ``lite`` model is capped at
|
||||
``BOR_SUMMARY_MAX_CHARS`` (default 12 000 chars per call)."""
|
||||
|
||||
@@ -1324,6 +1324,124 @@ 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) ----------
|
||||
|
||||
|
||||
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
|
||||
``extra_body``."""
|
||||
llm, completions = _make_chat_client(
|
||||
_FakeCompletion("ok"), llm_thinking_level="low"
|
||||
)
|
||||
asyncio.run(llm.chat([{"role": "user", "content": "q"}]))
|
||||
assert completions.chat_kwargs is not None
|
||||
assert completions.chat_kwargs["extra_body"] == {"reasoning_effort": "low"}
|
||||
|
||||
|
||||
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."""
|
||||
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"}
|
||||
|
||||
|
||||
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."""
|
||||
llm, completions = _make_chat_client(
|
||||
_FakeCompletion("ok"), llm_thinking_level="low "
|
||||
)
|
||||
asyncio.run(llm.chat([{"role": "user", "content": "q"}]))
|
||||
assert completions.chat_kwargs is not None
|
||||
assert completions.chat_kwargs["extra_body"] == {"reasoning_effort": "low"}
|
||||
|
||||
|
||||
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."""
|
||||
llm, completions = _make_chat_client(
|
||||
_FakeCompletion("ok"), llm_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
|
||||
assert set(completions.chat_kwargs) == {
|
||||
"model",
|
||||
"messages",
|
||||
"temperature",
|
||||
"max_tokens",
|
||||
"stream",
|
||||
}
|
||||
|
||||
|
||||
def test_chat_stream_sends_reasoning_effort_extra_body_when_level_set() -> None:
|
||||
"""Phase 172 (A1): the chat turn rides the parameter too."""
|
||||
llm, completions = _make_stream_client(
|
||||
[_chunk("x")], llm_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:
|
||||
llm, completions = _make_stream_client(
|
||||
[_chunk("x")], llm_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."""
|
||||
llm, completions = _make_stream_client(
|
||||
[_chunk("x")], llm_thinking_level=""
|
||||
)
|
||||
asyncio.run(_collect(llm, [{"role": "user", "content": "q"}]))
|
||||
assert completions.kwargs is not None
|
||||
assert "extra_body" not in completions.kwargs
|
||||
assert set(completions.kwargs) == {
|
||||
"model",
|
||||
"messages",
|
||||
"temperature",
|
||||
"max_tokens",
|
||||
"stream",
|
||||
}
|
||||
|
||||
|
||||
def test_chat_stream_whitespace_level_is_the_off_switch() -> None:
|
||||
"""Whitespace-only (``" "``) is empty for wire purposes (A2)."""
|
||||
llm, completions = _make_stream_client(
|
||||
[_chunk("x")], llm_thinking_level=" "
|
||||
)
|
||||
asyncio.run(_collect(llm, [{"role": "user", "content": "q"}]))
|
||||
assert completions.kwargs is not None
|
||||
assert "extra_body" not in completions.kwargs
|
||||
|
||||
|
||||
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")
|
||||
asyncio.run(llm.embed(["a", "b"]))
|
||||
vec = asyncio.run(llm.embed_one("c"))
|
||||
assert len(vec) == 768
|
||||
assert http.bodies # the requests actually happened
|
||||
for body in http.bodies:
|
||||
assert set(body) == {"model", "input"}
|
||||
assert "reasoning_effort" not in body
|
||||
assert "extra_body" not in body
|
||||
|
||||
|
||||
# ---------- chat_stream_retried (phase 67, task 01) ----------
|
||||
|
||||
_RETRY_MSGS: list[dict[str, str]] = [{"role": "user", "content": "q"}]
|
||||
|
||||
@@ -93,6 +93,21 @@ def test_env_var_is_bare_key_upper_prefixed() -> None:
|
||||
assert env_var(key).startswith("BOR_")
|
||||
|
||||
|
||||
def test_llm_thinking_level_is_a_plain_live_overridable_row() -> None:
|
||||
"""Phase 172 (task 01): the new ``llm_thinking_level`` pair sits in
|
||||
``LLM (aipi)`` with the ``Thinking level`` label and is plain —
|
||||
not ``secret`` (a free-form text value, like ``llm_chat_model``),
|
||||
not ``restart_required`` (the LLM client reads the setting per
|
||||
request via ``get_settings()`` — live), overridable (the overlay is
|
||||
generic over ``str`` fields)."""
|
||||
meta = CATALOG["llm_thinking_level"]
|
||||
assert meta.group == "LLM (aipi)"
|
||||
assert meta.label == "Thinking level"
|
||||
assert "llm_thinking_level" not in SECRETS
|
||||
assert "llm_thinking_level" not in RESTART_REQUIRED
|
||||
assert "llm_thinking_level" not in NON_OVERRIDABLE
|
||||
|
||||
|
||||
def test_labels_nonempty_and_unique() -> None:
|
||||
labels = [meta.label for meta in CATALOG.values()]
|
||||
assert all(label.strip() for label in labels), "empty label in the catalogue"
|
||||
|
||||
Reference in New Issue
Block a user