chore(agent): phase roadmap from TODO.md (178 split thinking levels)
LLM-Generated: true
This commit is contained in:
@@ -0,0 +1,49 @@
|
|||||||
|
# Phase 178 — Split thinking levels: a chat level and a summary level, separately tunable
|
||||||
|
|
||||||
|
**Source:** `TODO.md` L3 — "Need a separate summary thinking level and chat thinking level. Default thinking level for summary should be none. Chat should stay at low"
|
||||||
|
**Story:** n/a (TODO-derived, owner roadmap confirmation 2026-10-11)
|
||||||
|
**Context:** `app/config.py` L108–115 — `llm_thinking_level: str = "low"` (the phase-172 shared knob, directly after `llm_summary_model` L103–107, before `stream_thinking` L116), `app/core/settings_catalog.py` L154–155 — the single `llm_thinking_level` `SettingMeta` row (label "Thinking level", group `LLM (aipi)`; the completeness pin in `tests/unit/test_settings_catalog.py` — `set(CATALOG) == set(Settings.model_fields)` — makes every `Settings` field a build failure until it has a `SettingMeta`), `.env.example` L61 — `BOR_LLM_THINKING_LEVEL=low`, `app/rag/llm.py` — `_thinking_extra_body(settings)` (L49–57, reads `settings.llm_thinking_level`), `LLMClient.chat()` (L470, the one-shot `chat.completions.create` L430–452 — `extra = _thinking_extra_body(self.settings)` L432, `**extra_kwargs` L444; callers: `app/rag/summarizer.py` L242 + L359 document summaries, `app/rag/folder_summaries.py` L347, `app/rag/overview.py` L254 KB overview, `check_models` L805 the pre-sync probe), `LLMClient.chat_stream()` (L511, the kwargs dict L615–622 + `kwargs["extra_body"] = extra` L625–628), `app/api/chat.py` — the per-turn log line (L1272–1291, `thinking=%s` L1275, `settings.llm_thinking_level.strip() or "-"` L1289), the module docstring L14–19, `frontend/assets/settings.js` (the phase-168 generated panel — **no code change here**: `str` catalogue entries render as free-form text rows automatically), `tests/unit/test_config.py` L108–133 (default / env override / empty-stays-empty), `tests/unit/test_settings_catalog.py` L99–111, `tests/unit/test_llm_client.py` L1330–1440 (the mocked-client `extra_body` family), `tests/unit/test_chat_gate.py` L1550–1630 (the per-turn log-line family), `tests/integration/test_admin_settings_api.py` L521–607 + `test_admin_settings_live.py` L171–204, `tests/e2e/test_thinking_level.py` (the phase-172 Playwright suite — the pattern to extend).
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
The operator tunes the chat turns' thinking level and the one-shot summaries' thinking level **independently**: `llm_chat_thinking_level` (renamed from the phase-172 `llm_thinking_level`, default `low` — chat stays at low, TODO L3) rides `chat_stream()`; `llm_summary_thinking_level` (default empty = **off** — the TODO's "none", the phase-172 locked A2 off-switch: the parameter is ABSENT from the request) rides the one-shot `chat()` (document summaries, folder summaries, KB overview, the `check_models` probe). Both are free-form text, both live in `.env` (`BOR_LLM_CHAT_THINKING_LEVEL` / `BOR_LLM_SUMMARY_THINKING_LEVEL`) and the admin Settings panel, both live-apply without a restart.
|
||||||
|
|
||||||
|
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||||
|
- **D1 — the two fields (task 01).** `app/config.py` LLM section: `llm_thinking_level` is **renamed** to `llm_chat_thinking_level` (default stays `"low"`) and `llm_summary_thinking_level: str = ""` is added next to it (both before `stream_thinking`). Plain `str`, no allowlist, no validator (phase-172 A4, kept). House docstrings name the env var, the default, the wire mapping and the off rule. `.env.example` L61: `BOR_LLM_THINKING_LEVEL=low` is **replaced** by `BOR_LLM_CHAT_THINKING_LEVEL=low` + `BOR_LLM_SUMMARY_THINKING_LEVEL=` (empty value, one-line comment each).
|
||||||
|
- **D2 — the consumers rename with the field (task 01, behavior-identical).** Every consumer of the old field is mechanically renamed to `llm_chat_thinking_level` in task 01 — `app/rag/llm.py` (module docstring, `_thinking_extra_body`), `app/api/chat.py` (module docstring, per-turn log line L1289) — and every test pin is renamed in place (unit `test_config.py` / `test_settings_catalog.py` / `test_llm_client.py` / `test_chat_gate.py`, integration `test_admin_settings_api.py` / `test_admin_settings_live.py`, e2e `test_thinking_level.py`). After task 01 the suite is green and behavior is **byte-identical** to pre-phase: both `chat()` and `chat_stream()` ride the (renamed) chat level, default `low`; the new summary field exists but is inert until task 03.
|
||||||
|
- **D3 — the wire split (task 03).** `_thinking_extra_body` takes the **level string** (pure function, no `Settings` read): `chat_stream()` passes `self.settings.llm_chat_thinking_level`; `chat()` passes `self.settings.llm_summary_thinking_level`. Trimmed non-empty → `{"reasoning_effort": <value>}`; empty/whitespace → `None` (the parameter is ABSENT — never a literal `"none"` or `"off"`, phase-172 A2). The `check_models` probe (L805) goes through `chat()` → summary level → off by default; no special-casing (it is a reachability ping). Embeddings path untouched.
|
||||||
|
- **D4 — the admin rows (task 02).** The single `SettingMeta("llm_thinking_level", …, "Thinking level")` becomes TWO rows: `llm_chat_thinking_level` → "Chat thinking level", `llm_summary_thinking_level` → "Summary thinking level" (both group `LLM (aipi)`, placed where the old row sat — after `llm_summary_model`, before `stream_thinking`). The panel (`settings.js`), the `GET` catalogue, the `PUT` / `DELETE` routes and the DB-over-env overlay are generic over the catalogue — the two free-form text rows, their save/reset and live-apply fall out with zero extra code. The completeness pin is the enforcement.
|
||||||
|
- **D5 — the per-turn line (task 04).** The per-turn log line's `thinking=<value>` (or `-` when empty) reads the **chat** level — the level the turn actually ran at; the summary level is a one-shot concern and gains no log field (the summary log lines have no knob field today). No `query_log` schema change (phase-172 A6, kept).
|
||||||
|
- **NOT touched:** `frontend/assets/settings.js`, `frontend/assets/styles.css`, the `app_settings` table shape, `app/core/settings_overlay.py` (generic), the embeddings path, `stream_thinking` (the phase-17 kill-switch for `thinking` SSE events), every other setting.
|
||||||
|
|
||||||
|
## Dependencies
|
||||||
|
— (none: the app is fully functional at the head of `complete/`; this phase refines the committed phase-172 thinking-level machinery)
|
||||||
|
|
||||||
|
## Tasks
|
||||||
|
1. `01_settings_catalog_env.md` — the rename + the new field in `app/config.py` + the two `CATALOG` rows + `.env.example` + the mechanical consumer/test rename (suite green, behavior byte-identical).
|
||||||
|
2. `02_admin_row_contract.md` — the integration set / reset / live-apply contract for BOTH keys (stored raw, live without restart, empty storable as the summary off-switch).
|
||||||
|
3. `03_llm_extra_body_split.md` — `chat_stream()` → chat level, `chat()` → summary level; the mocked-client unit tests per method.
|
||||||
|
4. `04_per_turn_log_field.md` — `thinking=<chat level>` (or `-`) pinned: the line follows the chat knob, never the summary knob.
|
||||||
|
5. `05_e2e_admin_settings_rows.md` — `tests/e2e/test_split_thinking_levels.py` (the phase's dedicated Playwright suite, AGENTS rule 4), the regression set, the full gates, the atomic commit.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- **Unit:** `tests/unit/test_config.py` — chat default `low` / env override / empty-stays-empty; summary default `""` / env override / empty-stays-empty (no coercion, no allowlist — D3/A2). `tests/unit/test_settings_catalog.py` — the completeness pin stays green (both fields have their `SettingMeta`; group/label/flags correct). `tests/unit/test_llm_client.py` — `chat_stream()` sends `extra_body={"reasoning_effort": <chat level>}` when the chat level is trimmed-non-empty and no `extra_body` when it is empty; `chat()` sends it from the **summary** level with the same absent-when-empty rule; an empty summary level (the default) keeps one-shot requests byte-identical to pre-phase-172; the embeddings path is unaffected. `tests/unit/test_chat_gate.py` — the per-turn line shows the chat level (with the summary level deliberately different — it must not leak into the line).
|
||||||
|
- **Integration:** `tests/integration/test_admin_settings_api.py` / `test_admin_settings_live.py` — BOTH rows appear in `GET /api/admin/settings` (`str`, correct labels, effective defaults `low` / ``); `PUT` stores the raw string on each, the empty value is storable (the summary off-switch), the overlay resolves live without a restart; `DELETE` resets each to env.
|
||||||
|
- **E2E (AGENTS rule 4, task 05):** `tests/e2e/test_split_thinking_levels.py`, run **in isolation** — both rows visible in the `LLM (aipi)` group (labels "Chat thinking level" / "Summary thinking level"), chat default `low`, summary default empty, both free-form text inputs (type, save, survives reload), both reset-to-env restore the defaults, the env-var display pins `BOR_LLM_CHAT_THINKING_LEVEL` / `BOR_LLM_SUMMARY_THINKING_LEVEL`; the `test_admin_settings_panel.py` + `test_admin_settings_layout.py` regression set stays green.
|
||||||
|
- **Coverage:** **>90%** on `app/` (`uv run pytest --cov=app --cov-report=term-missing`).
|
||||||
|
- **Lint/types:** `uv run ruff check . && uv run pyright`.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest -k "config and thinking" -v` shows chat default `low` + summary default `""` (env overrides for both).
|
||||||
|
- [ ] `GET /api/admin/settings` (admin) lists `llm_chat_thinking_level` and `llm_summary_thinking_level` in the `LLM (aipi)` group (`str`, defaults `low` / empty); `PUT` / `DELETE` round-trip each per the overlay contract; the old `llm_thinking_level` key no longer appears.
|
||||||
|
- [ ] With the levels set, `chat_stream()` carries `reasoning_effort` from the chat level and `chat()` from the summary level (unit-asserted via the mocked client); with a level empty, that method's request kwargs contain no `extra_body` (byte-identical to pre-phase-172 for the default one-shot path).
|
||||||
|
- [ ] The per-turn log line carries `thinking=<chat level>` (`-` when empty) — never the summary level.
|
||||||
|
- [ ] `uv run pytest` green, `app/` coverage >90%, ruff + pyright clean.
|
||||||
|
- [ ] `uv run pytest tests/e2e/test_split_thinking_levels.py -v --no-cov` green **in isolation**; `test_admin_settings_panel.py` + `test_admin_settings_layout.py` (phase 171) green in isolation.
|
||||||
|
- [ ] One atomic `--no-gpg-sign` Conventional Commit (e.g. `feat(llm): split the thinking level into a chat level (default low) and a summary level (default off)`); the phase dir moves to `.agents/phases/complete/`.
|
||||||
|
|
||||||
|
## Locked decisions (owner-confirmed 2026-10-11, roadmap confirmation)
|
||||||
|
- **A1 — rename, not add-on:** `llm_thinking_level` is **renamed** to `llm_chat_thinking_level` (`BOR_LLM_THINKING_LEVEL` → `BOR_LLM_CHAT_THINKING_LEVEL`). Any DB-stored override under the old key becomes inert (it is no longer a catalogue row) and chat falls back to the env default `low` — chat behavior is preserved exactly as the TODO wants; no value migration.
|
||||||
|
- **A2 — the TODO's "none" = the phase-172 off-switch:** `llm_summary_thinking_level` defaults to empty (`""`) → `reasoning_effort` is ABSENT from one-shot `chat()` requests. Per phase-172's locked A2 ("nothing is sent as `off` or `null` — absence is the off"), no literal `"none"` string is ever sent.
|
||||||
|
- **A3 — scope:** `chat_stream()` (the chat turn) → chat level; every one-shot `chat()` (document summaries, folder summaries, KB overview, the `check_models` probe) → summary level; the embeddings path is untouched.
|
||||||
|
- **A4 — no frontend code change:** the panel is generated from the catalogue; the two free-form text rows appear automatically (phase-172 A5 pattern).
|
||||||
|
- **A5 — the per-turn line** keeps its single `thinking=<chat level>` field (`-` when empty), reads the chat level, and gains no summary field; no `query_log` schema change.
|
||||||
@@ -0,0 +1,69 @@
|
|||||||
|
# Task 01 — The two settings + the two catalogue rows + `.env.example` + the mechanical rename
|
||||||
|
|
||||||
|
**Phase:** `178_split_thinking_levels` · **Source:** `TODO.md:3` — "Need a separate summary thinking level and chat thinking level. Default thinking level for summary should be none. Chat should stay at low"
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
The two settings exist in `app/config.py` (chat default `low`, summary default empty = off, both free-form), in the admin catalogue (the completeness pin stays green), and in `.env.example` — and every existing consumer/test of the old single `llm_thinking_level` is renamed so the suite is green and behavior is **byte-identical** to pre-phase (both LLM paths still ride the chat level, default `low`).
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `app/config.py` — in the LLM section, where the phase-172 field sits (L108–115, directly after the `llm_summary_model` block, before `stream_thinking` L116): **rename** `llm_thinking_level` → `llm_chat_thinking_level` (default stays `"low"`) and add the summary field next to it. House docstring style (env var named, default, wire mapping, off rule):
|
||||||
|
```python
|
||||||
|
#: The chat turn's thinking level (phase 178,
|
||||||
|
#: ``BOR_LLM_CHAT_THINKING_LEVEL``; the phase-172 knob, renamed) —
|
||||||
|
#: pi-style reasoning effort, free-form text exactly like
|
||||||
|
#: ``llm_chat_model`` (no allowlist — the endpoint is the source of
|
||||||
|
#: truth). Default ``low``. Rides the streaming chat-completions
|
||||||
|
#: requests to aipi as the top-level ``reasoning_effort`` parameter
|
||||||
|
#: (``app/rag/llm.py`` ``chat_stream``); the EMPTY value is the off
|
||||||
|
#: switch — the parameter is not sent at all.
|
||||||
|
llm_chat_thinking_level: str = "low"
|
||||||
|
#: The one-shot summary thinking level (phase 178,
|
||||||
|
#: ``BOR_LLM_SUMMARY_THINKING_LEVEL``) — rides the one-shot
|
||||||
|
#: ``chat()`` calls (document summaries, folder summaries, KB
|
||||||
|
#: overview, the ``check_models`` probe) as ``reasoning_effort``.
|
||||||
|
#: Default EMPTY = the off switch (phase-172 A2): the parameter is
|
||||||
|
#: not sent at all; the TODO's "none" means absence, never a
|
||||||
|
#: literal "none" string.
|
||||||
|
llm_summary_thinking_level: str = ""
|
||||||
|
```
|
||||||
|
No validator, no coercion — plain `str` (phase-172 A4, kept). Verify by reading: `llm_summary_model` (L103–107) and `stream_thinking` (L116–120) keep their shape byte-identical.
|
||||||
|
2. `app/core/settings_catalog.py` — replace the single row (L154–155) with TWO rows, placed exactly where the old row sat (after `llm_summary_model` L151–152, before `stream_thinking` L157):
|
||||||
|
```python
|
||||||
|
"llm_chat_thinking_level": SettingMeta(
|
||||||
|
"llm_chat_thinking_level", "LLM (aipi)", "Chat thinking level"
|
||||||
|
),
|
||||||
|
"llm_summary_thinking_level": SettingMeta(
|
||||||
|
"llm_summary_thinking_level", "LLM (aipi)", "Summary thinking level"
|
||||||
|
),
|
||||||
|
```
|
||||||
|
Nothing else: the panel (`settings.js`), the `GET` catalogue, the `PUT` / `DELETE` routes and the overlay are generic over the catalogue (D4).
|
||||||
|
3. `.env.example` — in the LLM block (L57–65), replace `BOR_LLM_THINKING_LEVEL=low` (L61) with:
|
||||||
|
```
|
||||||
|
BOR_LLM_CHAT_THINKING_LEVEL=low # the chat turn's thinking level (pi-style; free-form, rides reasoning_effort); empty = the parameter is not sent
|
||||||
|
BOR_LLM_SUMMARY_THINKING_LEVEL= # the one-shot summary thinking level (document/folder summaries, KB overview); empty (default) = the parameter is not sent
|
||||||
|
```
|
||||||
|
directly under `BOR_LLM_CHAT_MODEL=turbo` (L60), house comment style.
|
||||||
|
4. `app/rag/llm.py` — **mechanical rename, behavior-identical** (D2): the module docstring (L14–19 — rename `BOR_LLM_THINKING_LEVEL` / `default low` to `BOR_LLM_CHAT_THINKING_LEVEL`; the "rides every chat-completions request (one-shot `chat` and streaming `chat_stream`)" sentence stays TRUE after this task because both paths still ride the chat level; task 03 finalizes the wording) and `_thinking_extra_body` (L49–57, `settings.llm_thinking_level` → `settings.llm_chat_thinking_level`; docstring names the chat level). `chat()` (L432) and `chat_stream()` (L625) keep calling the same helper with the chat level — no split yet (that is task 03). The embeddings path is untouched.
|
||||||
|
5. `app/api/chat.py` — **mechanical rename, behavior-identical** (D2): the module docstring (L14–19, `BOR_LLM_THINKING_LEVEL` → `BOR_LLM_CHAT_THINKING_LEVEL`, "the effective chat level") and the per-turn log line argument (L1289, `settings.llm_thinking_level` → `settings.llm_chat_thinking_level`; the format string L1275 keeps `thinking=%s`).
|
||||||
|
6. `tests/unit/test_config.py` — L108–133: the three phase-172 tests become SIX: `llm_chat_thinking_level` (default `low` with `BOR_LLM_CHAT_THINKING_LEVEL` unset; env override `high`; empty env value stays empty) and `llm_summary_thinking_level` (default `""` with `BOR_LLM_SUMMARY_THINKING_LEVEL` unset; env override `high`; empty env value stays empty). Same monkeypatch pattern, no coercion asserted.
|
||||||
|
7. `tests/unit/test_settings_catalog.py` — L99–111: the single-row test becomes a BOTH-rows test (each `SettingMeta` in the `LLM (aipi)` group with the correct label, `str`-ish flags — not in `SECRETS`, not in `RESTART_REQUIRED`, not in `NON_OVERRIDABLE`); the completeness pin (`set(CATALOG) == set(Settings.model_fields)`) must stay green.
|
||||||
|
8. `tests/unit/test_llm_client.py` — L1330–1440: rename the kwarg `llm_thinking_level=` → `llm_chat_thinking_level=` in the existing family (behavior assertions unchanged — the helper still reads the chat level); the `chat()`-path tests keep passing as-is.
|
||||||
|
9. `tests/unit/test_chat_gate.py` — L1550–1630: rename the `Settings(..., llm_thinking_level=…)` pins → `llm_chat_thinking_level=…`; the log-line assertions (L1553, `BOR_LLM_THINKING_LEVEL` docstring) name `BOR_LLM_CHAT_THINKING_LEVEL` / the chat level.
|
||||||
|
10. `tests/integration/test_admin_settings_api.py` — L521–607: the `llm_thinking_level` row contract is renamed to `llm_chat_thinking_level` (env var `BOR_LLM_CHAT_THINKING_LEVEL`, default `low`; PUT/DELETE/anonymous-401 pins unchanged in shape). Task 02 extends it for the summary row.
|
||||||
|
11. `tests/integration/test_admin_settings_live.py` — L171–204: the live-apply test is renamed to `llm_chat_thinking_level` (same save/empty/delete pattern against the new key).
|
||||||
|
12. `tests/e2e/test_thinking_level.py` — rename the pins in place: `tr[data-key="llm_thinking_level"]` → `tr[data-key="llm_chat_thinking_level"]`, `BOR_LLM_THINKING_LEVEL` → `BOR_LLM_CHAT_THINKING_LEVEL`, the `Settings.model_fields["llm_thinking_level"]` env lookup → `llm_chat_thinking_level`. E2E runs only in isolation (the per-task gate covers unit + integration), but the file must not rot: task 05 turns it into `test_split_thinking_levels.py`.
|
||||||
|
- ASSUMPTION: A1 (phase-locked) — the rename orphans any stored `app_settings` row under `llm_thinking_level` (it becomes inert; chat falls back to env default `low`); no value migration is performed.
|
||||||
|
- ASSUMPTION: A2 (phase-locked) — the summary default is the empty string (the phase-172 off-switch: `reasoning_effort` ABSENT), never a literal `"none"`.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: `tests/unit/test_config.py` (six thinking-level tests), `tests/unit/test_settings_catalog.py` (the completeness pin + both rows), `tests/unit/test_llm_client.py` (renamed family, unchanged assertions), `tests/unit/test_chat_gate.py` (renamed pins).
|
||||||
|
- Integration: `tests/integration/test_admin_settings_api.py` + `test_admin_settings_live.py` (renamed row contract, green).
|
||||||
|
- Coverage: **>90%** on `app/` (`uv run pytest --cov=app --cov-report=term-missing`).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `grep -rn "llm_thinking_level\b\|BOR_LLM_THINKING_LEVEL\b" app/ tests/ .env.example` returns **nothing** (the old name is gone everywhere; `llm_chat_thinking_level` / `BOR_LLM_CHAT_THINKING_LEVEL` in its place).
|
||||||
|
- [ ] `uv run pytest -k "config and thinking" -v` — chat default `low` + summary default `""` + env overrides for both.
|
||||||
|
- [ ] `uv run pytest` green (unit + integration), `app/` coverage >90%, `uv run ruff check . && uv run pyright` clean.
|
||||||
|
- [ ] Behavior check (read, not run): `chat()` and `chat_stream()` both still ride the chat level; with both defaults (`low` / empty) and the old `.env` value, streaming requests are byte-identical to pre-phase-172.
|
||||||
|
- [ ] No behavior change in completed work.
|
||||||
@@ -0,0 +1,28 @@
|
|||||||
|
# Task 02 — The admin row contract: both keys, set / reset / live-apply
|
||||||
|
|
||||||
|
**Phase:** `178_split_thinking_levels` · **Source:** `TODO.md:3` — "Need a separate summary thinking level and chat thinking level. Default thinking level for summary should be none. Chat should stay at low"
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
The integration contract proves both rows behave per the overlay: they appear in the catalogue with the right group/label/default, `PUT` stores the raw string (empty included — the summary off-switch), the value resolves live without a restart, and `DELETE` resets to env — for the chat row AND the summary row, independently.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `tests/integration/test_admin_settings_api.py` — the phase-172 row contract (now `test_llm_chat_thinking_level_row_contract`, from task 01) gains a **summary twin** `test_llm_summary_thinking_level_row_contract` following the same shape, with the summary specifics:
|
||||||
|
- `GET /api/admin/settings` lists `llm_summary_thinking_level` in the `LLM (aipi)` group, `str`, label "Summary thinking level", `env_var` `BOR_LLM_SUMMARY_THINKING_LEVEL`, effective default `""` (with `BOR_LLM_SUMMARY_THINKING_LEVEL` deleted from the env for the test — the house `monkeypatch.delenv(…, raising=False)` pattern).
|
||||||
|
- `PUT {API}/llm_summary_thinking_level` with `{"value": "high"}` → 200, the raw string stored (`SELECT value FROM app_settings WHERE key = 'llm_summary_thinking_level'` → `"high"`), `get_settings().llm_summary_thinking_level == "high"` **without a restart**.
|
||||||
|
- `PUT` with `{"value": ""}` → 200 (the off-switch is storable — an empty stored value must not be rejected as "no value"), `get_settings().llm_summary_thinking_level == ""`.
|
||||||
|
- `DELETE` → 200, the row gone from `app_settings`, `get_settings().llm_summary_thinking_level == ""` (the env default).
|
||||||
|
- An anonymous (non-admin) `PUT` → the 401/403 pin, same as the chat twin.
|
||||||
|
- Independence pin: after `PUT`-ing the summary row `"high"`, the chat row's effective value is still `"low"` (the two overlays resolve independently — the whole point of the split).
|
||||||
|
2. `tests/integration/test_admin_settings_live.py` — the chat live-apply test (from task 01) gains a **summary twin** `test_llm_summary_thinking_level_live_apply_no_restart`: `delenv` `BOR_LLM_SUMMARY_THINKING_LEVEL` → default `""`; `_save(db, "llm_summary_thinking_level", "high")` → `get_settings().llm_summary_thinking_level == "high"` **NO restart**; `_save(…, "")` → `""`; `DELETE FROM app_settings WHERE key = 'llm_summary_thinking_level'` → back to `""`.
|
||||||
|
3. Verify by running: `uv run pytest tests/integration/test_admin_settings_api.py tests/integration/test_admin_settings_live.py -v` — both families green, and the old single-row test is gone (no `llm_thinking_level` key referenced anywhere: `grep -rn "llm_thinking_level\b" tests/` empty).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Integration: the two extended families above (row contract + live-apply, chat AND summary).
|
||||||
|
- Coverage: **>90%** on `app/` (this task is test-only — `app/` code is untouched, so the task-01 coverage must simply stay green).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest tests/integration/test_admin_settings_api.py -v -k "thinking" -v` — the chat + summary row contracts green (group/label/env-var/default, PUT raw storage incl. empty, DELETE reset, anonymous rejected, independence).
|
||||||
|
- [ ] `uv run pytest tests/integration/test_admin_settings_live.py -v -k "thinking"` — both live-apply twins green (no restart, empty storable, delete resets).
|
||||||
|
- [ ] `uv run pytest` green, `app/` coverage >90%.
|
||||||
|
- [ ] No behavior change in completed work.
|
||||||
@@ -0,0 +1,41 @@
|
|||||||
|
# Task 03 — The wire split: `chat_stream()` → chat level, `chat()` → summary level
|
||||||
|
|
||||||
|
**Phase:** `178_split_thinking_levels` · **Source:** `TODO.md:3` — "Need a separate summary thinking level and chat thinking level. Default thinking level for summary should be none. Chat should stay at low"
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
The one-shot `chat()` path (document summaries, folder summaries, KB overview, the `check_models` probe) sends `reasoning_effort` from the **summary** level — absent by default (A2) — while `chat_stream()` keeps sending it from the **chat** level (default `low`), so the two surfaces are independently tunable on the wire.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `app/rag/llm.py` — `_thinking_extra_body` (L49–57) becomes a pure function of the level string (drop the `Settings` read):
|
||||||
|
```python
|
||||||
|
def _thinking_extra_body(level: str) -> dict[str, str] | None:
|
||||||
|
"""The thinking level as the chat-completions extra body (phase 178).
|
||||||
|
|
||||||
|
Non-empty (trimmed) value → ``{"reasoning_effort": <value>}`` (the
|
||||||
|
OpenAI-standard top-level parameter, phase-172 A1); empty /
|
||||||
|
whitespace → ``None`` (the off switch — the parameter is ABSENT
|
||||||
|
from the request, phase-172 A2; never a literal ``"none"`` or
|
||||||
|
``"off"``).
|
||||||
|
"""
|
||||||
|
trimmed = level.strip()
|
||||||
|
return {"reasoning_effort": trimmed} if trimmed else None
|
||||||
|
```
|
||||||
|
2. `app/rag/llm.py` — `chat()`'s single-attempt body (L432): `extra = _thinking_extra_body(self.settings.llm_summary_thinking_level)`; the comment (L443–444) becomes "Phase 178 (A3/A2): the **summary** level rides as `reasoning_effort` (document summaries, folder summaries, KB overview, the `check_models` probe); the key is absent (never None) when off — the default (empty) keeps one-shot requests byte-identical to pre-phase-172."
|
||||||
|
3. `app/rag/llm.py` — `chat_stream()` (L625–628): `if (extra := _thinking_extra_body(self.settings.llm_chat_thinking_level)) is not None: …` (the comment L626–627 becomes "Phase 178 (A3/A2): the **chat** level rides as `reasoning_effort`; the key is absent (never None) when off.").
|
||||||
|
4. `app/rag/llm.py` — finalize the module docstring (L1–25, rewritten in task 01): replace the single-level paragraph with the split wording — the chat turn's level (`BOR_LLM_CHAT_THINKING_LEVEL`, default `low`) rides streaming `chat_stream`; the one-shot level (`BOR_LLM_SUMMARY_THINKING_LEVEL`, default empty = off) rides `chat` (document summaries, folder summaries, KB overview, the `check_models` probe); an empty value is the off switch (parameter absent, byte-identical to pre-phase-172); the embeddings path never carries it. Note the probe's consequence: `check_models` (L805) goes through `chat()` → off by default — it is a reachability ping, no special-casing.
|
||||||
|
5. `tests/unit/test_llm_client.py` — the mocked-client family (L1330–1440, renamed in task 01):
|
||||||
|
- The `chat()`-path tests (currently `llm_chat_thinking_level="low"` / `"low "` / `" "` at L1335–1368) are re-pointed at the **summary** kwarg: `llm_summary_thinking_level="high"` → the `chat.completions.create` call carries `extra_body={"reasoning_effort": "high"}`; `" high "` → trimmed `"high"`; `""` / `" "` → the call kwargs contain **no** `extra_body` key (never `None`, never a literal "none").
|
||||||
|
- The `chat_stream()`-path tests (L1385–1422) keep their shape: `llm_chat_thinking_level="high"` / `" high "` → `extra_body={"reasoning_effort": "high"}`; `""` / `" "` → no `extra_body` key.
|
||||||
|
- Add the cross-pins (the whole point of the split): client with `llm_chat_thinking_level="high"` + `llm_summary_thinking_level=""` (the defaults inverted) → `chat_stream()` carries `reasoning_effort="high"` while `chat()` carries no `extra_body`; the default client (chat `"low"` + summary `""`) → `chat()` is byte-identical to pre-phase-172 (no `extra_body`) while `chat_stream()` carries `reasoning_effort="low"`.
|
||||||
|
- The embeddings-path test (no `extra_body` ever) stays green untouched.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: `tests/unit/test_llm_client.py` (the re-pointed + extended family above; the mocked-SDK-client pattern).
|
||||||
|
- Coverage: **>90%** on `app/` (`uv run pytest --cov=app --cov-report=term-missing`).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest tests/unit/test_llm_client.py -v -k "thinking" -v` — per-method `extra_body` behavior green (chat level → `chat_stream` only; summary level → `chat` only; absent-when-empty; cross-pins).
|
||||||
|
- [ ] Default-config read check: with chat `low` + summary `""`, a `chat()` request carries no `extra_body` and a `chat_stream()` request carries `reasoning_effort="low"` (the test pins above).
|
||||||
|
- [ ] `uv run pytest` green, `app/` coverage >90%, `uv run ruff check . && uv run pyright` clean.
|
||||||
|
- [ ] No behavior change in completed work (the streaming path is byte-identical at the defaults; the one-shot path is off by default — the TODO's "summary should be none").
|
||||||
@@ -0,0 +1,25 @@
|
|||||||
|
# Task 04 — The per-turn line: `thinking=<chat level>`, never the summary knob
|
||||||
|
|
||||||
|
**Phase:** `178_split_thinking_levels` · **Source:** `TODO.md:3` — "Need a separate summary thinking level and chat thinking level. Default thinking level for summary should be none. Chat should stay at low"
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
The per-turn log line (PLAN §9, AGENTS rule 10) is pinned to the **chat** level — the level the turn actually ran at: `thinking=<trimmed chat level>` (`-` when empty) immediately before `thinking_chars=N`, with a cross-pin proving the summary level (set to a different value) never leaks into the line. No `query_log` schema change (phase-172 A6 / phase-178 A5, kept).
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `app/api/chat.py` — the per-turn log line (L1272–1291) already reads the chat level after task 01's mechanical rename (`settings.llm_chat_thinking_level.strip() or "-"` L1289, format `thinking=%s` L1275): verify by reading, no functional change. Finalize the inline comment (L1289) to "phase 178 (A5): the effective **chat** thinking level, trimmed, `-` when empty — the level this turn ran at; the summary level is a one-shot concern and gains no field here."
|
||||||
|
2. `app/api/chat.py` — the module docstring (L14–19, renamed in task 01): finalize the wording — "The per-turn log line records `thinking=<level>` (the effective `BOR_LLM_CHAT_THINKING_LEVEL`, trimmed, `-` when empty) before `thinking_chars` (phase 172, A6; phase 178, A5 — the **chat** level; the one-shot `BOR_LLM_SUMMARY_THINKING_LEVEL` never appears on the line)."
|
||||||
|
3. `tests/unit/test_chat_gate.py` — the log-line family (L1545–1630, pins renamed in task 01: the default `thinking=low`, the `"high"` override, the `""` → `-` off-switch, the `" "` → `-` trim pin, the full field-order pin):
|
||||||
|
- Update the test docstring (L1550–1557) to name `BOR_LLM_CHAT_THINKING_LEVEL` / the chat level (phase 178, A5).
|
||||||
|
- Add the **cross-pin** (the split's proof on the log line): `Settings(_env_file=None, relevance_threshold=0.30, llm_chat_thinking_level="low", llm_summary_thinking_level="high")` → the line is `thinking=low thinking_chars=…` — the summary's `high` does NOT appear as the line's `thinking=` value.
|
||||||
|
- Add the inverse off-pin: `llm_chat_thinking_level=""` + `llm_summary_thinking_level="high"` → `thinking=- thinking_chars=…` (the chat off-switch logs `-` even though the summary knob is set).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: `tests/unit/test_chat_gate.py` (the extended log-line family above).
|
||||||
|
- Coverage: **>90%** on `app/` (this task touches only the docstring/comment + tests — coverage must stay green).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest tests/unit/test_chat_gate.py -v -k "log" -v` — the full log-line family green: default `thinking=low`, override `high`, off-switch `-`, trim `-`, field order, AND the two cross-pins (summary level never leaks into `thinking=`).
|
||||||
|
- [ ] `grep -n "BOR_LLM_SUMMARY_THINKING_LEVEL" app/api/chat.py` returns nothing (the one-shot knob has no presence on the chat turn's log line, A5).
|
||||||
|
- [ ] `uv run pytest` green, `app/` coverage >90%, `uv run ruff check . && uv run pyright` clean.
|
||||||
|
- [ ] No behavior change in completed work.
|
||||||
@@ -0,0 +1,30 @@
|
|||||||
|
# Task 05 — The E2E suite: `test_split_thinking_levels.py` + regression + gates + commit
|
||||||
|
|
||||||
|
**Phase:** `178_split_thinking_levels` · **Source:** `TODO.md:3` — "Need a separate summary thinking level and chat thinking level. Default thinking level for summary should be none. Chat should stay at low"
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
The phase's dedicated Playwright suite (AGENTS rule 4 — one test file for the phase, run in isolation) proves the operator's acceptance in a real browser: TWO free-form thinking-level rows in the admin Settings panel (chat default `low`, summary default empty = the TODO's "none"), independently saveable/resettable with live-apply, plus the full phase gates and the atomic commit.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `tests/e2e/test_thinking_level.py` → `tests/e2e/test_split_thinking_levels.py` — `git mv` the phase-172 suite (the module scaffolding is the house pattern, kept: `_boot_app` / `_stop_app` / `_clean_settings_state` / the autouse `_clean` (truncates `app_settings` + `ui_settings`, deletes `e2e-`-labeled tokens, module teardown pins `app_settings` empty) / the `.env` mtime guard / the `_cookies` / `_admin_client` / `_catalog_item` helpers) and rewrite the module docstring (Source → `TODO.md` L3 this item; the test → contract mapping below; the env pins: the module app opts BACK IN over the conftest's global `BOR_SETTINGS_DB=0` pin (`env["BOR_SETTINGS_DB"] = "1"`), and pins `BOR_LLM_CHAT_THINKING_LEVEL=low` AND `BOR_LLM_SUMMARY_THINKING_LEVEL=` (empty string SET — so both badges read `env`, not `default`, and an operator's local `.env` cannot leak a different level into the suite; the house leak-guard pattern) and `BOR_LLM_CHAT_MODEL` (the "like chat model" reference)).
|
||||||
|
2. The three tests (renamed/extended from the phase-172 trio):
|
||||||
|
- `test_both_rows_render_free_form_with_defaults` — the `LLM (aipi)` group band is visible (the phase-171 band contract); a row labelled **Chat thinking level** with the `BOR_LLM_CHAT_THINKING_LEVEL` env-var display shows `low` with the `env` badge; a row labelled **Summary thinking level** with the `BOR_LLM_SUMMARY_THINKING_LEVEL` display shows an **empty** input with the `env` badge (the TODO's "none" = the off switch, A2); BOTH inputs are `type="text"` (free-form — NOT `number`/`select`, the A4 contract); the `llm_chat_model` row's input is the same control (the "like chat model" reference, one assertion pair); `tr[data-key="llm_thinking_level"]` is **absent** (the old key is gone, A1).
|
||||||
|
- `test_saved_levels_persist_without_restart` — the phase-168 §7.4 Save lifecycle per row: type `think-soft` (no allowlist — A4) into the summary row and `high` into the chat row (dirty hints appear), save both; the API catalogue (`_catalog_item`) AND the raw `app_settings` rows carry the saved values for EACH key independently; a reload of `/settings.html` still shows `think-soft` / `high` with the `DB` badges (no restart — the phase-168 contract); the independence pin: the two rows' controls are distinct and the saved values do not cross.
|
||||||
|
- `test_reset_returns_to_env_defaults` — the per-row Reset (the per-row `DELETE`): after the saves, both Resets are live; clicking the summary Reset lands the value at **empty** with the `env` badge, clicking the chat Reset lands `low` with the `env` badge (the overlay reset, task 02's contract, in the browser); the status lines show the house "… is back to the environment value." toast for each.
|
||||||
|
3. Regression set (in isolation, per the house E2E protocol): `uv run pytest tests/e2e/test_admin_settings_panel.py -v --no-cov` (phase 168) and `uv run pytest tests/e2e/test_admin_settings_layout.py -v --no-cov` (phase 171) — both green (they do not pin the thinking rows — verified: no `thinking` references — but the two new rows must not break the panel/layout contracts).
|
||||||
|
4. The full gates: `uv run pytest` (unit + integration) green; `uv run pytest --cov=app --cov-report=term-missing` — `app/` coverage **>90%**; `uv run ruff check . && uv run pyright` clean; `uv run pytest tests/e2e/test_split_thinking_levels.py -v --no-cov` green **in isolation** (DB up: `podman compose up -d db`).
|
||||||
|
5. Commit (AGENTS rule 8): ONE atomic Conventional Commit, `--no-gpg-sign` — e.g. `feat(llm): split the thinking level into a chat level (default low) and a summary level (default off)` — body: the TODO L3 source, the rename (A1), the wire split (A3), the log line (A5), the two admin rows; then move the phase dir: `git mv .agents/phases/todo/178_split_thinking_levels .agents/phases/complete/178_split_thinking_levels` and include it in the commit (the `.agents/` tree is tracked — only `.agents/phase-sessions/` + `.agents/pipeline.log` are gitignored).
|
||||||
|
6. `TODO.md` — ALREADY CLEARED by the conversion (the file holds only the `# TODO` title; verify by reading — do not re-add the item).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- E2E (AGENTS rule 4): `tests/e2e/test_split_thinking_levels.py` run **in isolation** (the three tests above).
|
||||||
|
- Coverage: **>90%** on `app/`.
|
||||||
|
- Lint/types: ruff + pyright clean.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest tests/e2e/test_split_thinking_levels.py -v --no-cov` green **in isolation** (both rows, defaults `low`/empty, free-form, save + reload persistence per key, reset to env defaults, old key absent).
|
||||||
|
- [ ] `uv run pytest tests/e2e/test_admin_settings_panel.py -v --no-cov` and `uv run pytest tests/e2e/test_admin_settings_layout.py -v --no-cov` green in isolation (regression).
|
||||||
|
- [ ] `uv run pytest` green, `app/` coverage >90%, ruff + pyright clean.
|
||||||
|
- [ ] One atomic `--no-gpg-sign` Conventional Commit including the `.agents/` phase move; `TODO.md` holds only the `# TODO` title.
|
||||||
|
- [ ] No behavior change in completed work.
|
||||||
Reference in New Issue
Block a user