feat(rag): pass chat history with prior thinking to the LLM
Phase 74 (TODO.md L4): a follow-up question now reaches the model WITH the conversation so far — every prior user/brain turn and the prior thinking blocks on brain turns (preserve-thinking) — while POST /api/chat stays stateless (A10): the client provides the history in the request body and the server stores nothing new. Server (task 01): - ChatRequest.history: optional list[HistoryTurn] (who: user|brain, text, optional thinking) — absent/empty keeps the request byte-identical to pre-phase-74 (the two-message [system, user] request; the kill-switch semantics are pinned in the integration suite). - app.rag.prompts.history_to_messages: pure mapper — walks the turns newest-first against the settings budgets (history_max_turns=40 / history_max_chars=24000, BOR_HISTORY_MAX_TURNS / BOR_HISTORY_MAX_CHARS); a capped turn is dropped WHOLE (never cut mid-answer); the kept window is returned oldest-first; brain turns carry their thinking as reasoning_content (A4) only when non-empty. - Both branches feed it: the deflected path splices it between the system prompt and the current user message (the phase-71 recovery still rebuilds from messages[1:]), the grounded agent receives run_agent(..., history=hist); llm.py's message params widen to list[dict[str, Any]] (string-only messages stay byte-identical on the wire — the SDK passes message dicts through verbatim). - The per-turn log line (PLAN §9) gains history_msgs=N after kb_chars=N. - Pins: tests/unit/test_history.py (mapper: mapping, reasoning gating, both budgets, drop-whole, ordering, empty default), tests/unit/test_config.py (the two settings + env overrides), tests/unit/test_agent.py (the history splice + the default), tests/integration/test_chat_api.py (deflected AND grounded forward the history incl. reasoning_content, no-history byte-identity, 422 pins, the log field). Client (task 02): - runTurn — the single funnel for fresh send / phase-49 retry / phase-53 stale-regen — sends history = the conversation record minus the current question, with thinking only on brain records that streamed one (undefined drops the key from the JSON, the record's convention); the question is never duplicated into the history. Wire proof (task 03): - The mock's echo my history marker (HISTORY_TRIGGER) answers with the deterministic history echo — history: N prior messages; last answer tail: <last 24 chars>; thinking: yes|no — checked BEFORE the DEFLECT_MODE branch (like TABLE_TRIGGER), so it fires on both turn branches whatever the gate says; the module docstring records the user/assistant-only history invariant that keeps every existing (tool-result-classified) marker flow unaffected. - tests/e2e/test_llm_history.py (isolated): a grounded follow-up and a deflected follow-up both receive history: 2 prior messages + thinking: yes + the byte-exact tail of turn 1's answer (derived from the persisted bor.chat.v1 record — the same array the client maps into the body); a cold start receives history: 0 prior messages / last answer tail: none / thinking: no. - Regressions green in isolation: chat_rag, chat_history (phase 50), agent_document_tools, harness_aligned_tools, stop_generation, retry_answer, response_to_docs.
This commit is contained in:
@@ -0,0 +1,28 @@
|
||||
# Phase 73 — Hidden tab never stops a generating answer
|
||||
|
||||
**Source:** `TODO.md` L3 — "Clicking on another tab while an answer is generating stops that answer from being generated. Reponses should continue to generate unless you outright close the tab."
|
||||
**Story:** n/a (TODO-derived)
|
||||
**Context:** `frontend/assets/app.js` (the SSE turn machine: `runTurn` ~L1859, the `pagehide` partial-persist handler ~L2162, the 120s pre-token guard `TURN_TIMEOUT_MS` ~L287 / `armTurnTimeout` ~L982, `readSSE` ~L1044, the settle paths `done` ~L2011 / stop ~L2079), `app/api/chat.py` (the `finally` "turn cancelled" log line — the server-side signal that the SSE consumer really went away), phase-48 teardown contract (a REAL consumer departure — tab closed, navigation, Stop — still cancels the fetch and stops the model: that behavior is correct and must survive this phase).
|
||||
|
||||
## Objective
|
||||
An in-flight answer keeps generating while the browser tab is merely hidden (switched away from) and completes when the user returns; only closing the tab, navigating away, or clicking Stop aborts the turn. Also fixes the latent record-corruption on that path (a `pagehide` partial persist can leave a duplicated brain turn in the saved conversation, which makes the answer *look* truncated on restore).
|
||||
|
||||
## Dependencies
|
||||
— (none)
|
||||
|
||||
## Tasks
|
||||
1. `01_repro_root_cause.md` — bounded repro with an instrumentation decision tree; pin down WHICH mechanism stops the answer on tab switch (no permanent code changes).
|
||||
2. `02_fix_hidden_tab.md` — the fix: correlate the pagehide partial with the turn's settle so `done`/stop *replaces* it (never appends a second brain turn); if the repro implicates the 120s guard, make hidden time not count toward it; keep phase-48 teardown for real departures.
|
||||
3. `03_e2e_hidden_tab_stream.md` — Playwright regression: synthetic `pagehide` mid-stream → the answer completes exactly once, the record has one brain turn, reload restores it; regressions + commit.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit/integration: frontend-only phase — no `app/` changes expected (coverage floor unaffected, must stay **>90%** on `app/`).
|
||||
- E2E: new story suite `tests/e2e/test_hidden_tab_stream.py`, run in isolation (`uv run pytest tests/e2e/test_hidden_tab_stream.py -v --no-cov`) against the deterministic mock LLM (long/slow deterministic streams give a guaranteed mid-stream window).
|
||||
- Regression runs in isolation: `test_chat_rag.py`, `test_chat_persistence.py`, `test_chat_history.py` (phase 50), `test_stop_generation.py` (phase 48 contract: real Stop/cancel still tears down), `test_retry_answer.py`.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] The repro's root cause is named in the phase-73 commit message body (one line: which candidate from task 01 fired, or "none of C1–C3 — <finding>").
|
||||
- [ ] `uv run pytest tests/e2e/test_hidden_tab_stream.py -v --no-cov` green in isolation: a tab switch (synthetic `pagehide`) mid-turn never stops the answer, and the persisted conversation holds exactly one brain turn for that question.
|
||||
- [ ] Real departures unchanged: Stop button, tab close, and navigation still cancel the fetch (phase-48 `test_stop_generation.py` + `test_chat_persistence.py` green).
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] One atomic `--no-gpg-sign` Conventional-Commits commit (e.g. `fix(chat): keep generating while the tab is hidden`); phase dir moved to `.agents/phases/complete/`.
|
||||
@@ -0,0 +1,35 @@
|
||||
# Task 03 — E2E: synthetic pagehide mid-stream → answer completes exactly once + regressions + commit
|
||||
|
||||
**Phase:** `73_hidden_tab_stream` · **Source:** `TODO.md:3` — "Clicking on another tab while an answer is generating stops that answer from being generated. Reponses should continue to generate unless you outright close the tab."
|
||||
**Story:** n/a (TODO-derived)
|
||||
|
||||
## Objective
|
||||
Pin the phase-73 behavior in a deterministic Playwright suite (real tab-switching is browser-environment-specific; a dispatched `pagehide` + hidden `visibilityState` exercises exactly the code path the browsers that fire it on tab switch take), run the regression suites, and commit the phase.
|
||||
|
||||
## Work
|
||||
1. `tests/e2e/test_hidden_tab_stream.py` (Playwright; the app-boot + mock-LLM pattern from `tests/e2e/conftest.py` / `tests/e2e/test_response_to_docs.py`):
|
||||
- Slow deterministic stream for a guaranteed mid-stream window: ask the mock's `write a long answer` question (on-topic phrasing so the gate is HIGH — ~8s stream, ends in `LONG-ANSWER-END`) — or `think out loud then hesitate` (4s pre-content pause) for the pre-token variant.
|
||||
- `test_hidden_tab_does_not_stop_the_answer` — send the question; wait until answer text is visibly streaming (a few delta frames rendered); then, mid-stream, from the page:
|
||||
```js
|
||||
Object.defineProperty(document, "visibilityState", { configurable: true, get: () => "hidden" });
|
||||
window.dispatchEvent(new PageTransitionEvent("pagehide", { persisted: false }));
|
||||
Object.defineProperty(document, "visibilityState", { configurable: true, get: () => "visible" });
|
||||
window.dispatchEvent(new PageTransitionEvent("pageshow", { persisted: false }));
|
||||
```
|
||||
(the exact event sequence the tab-switching browsers deliver — and, if task 01's C2 fix landed, this also covers the visible-return re-arm); wait for `done`. Assert: the bubble contains the FULL mock answer including `LONG-ANSWER-END`; no error banner ("The stream ended before my answer finished…", "That's taking a long time…"); the `bor.chat.v1` localStorage record has EXACTLY ONE brain turn for the question (no duplicated partial) and its `text` equals the full answer; the auto-saved row (admin context, `persistConversation` path) carries the same single brain turn if reachable — otherwise assert on the localStorage record (the shared shape).
|
||||
- `test_reload_after_hidden_tab_restores_one_bubble` — same setup, but after `done`, reload the page: the restored conversation renders exactly one brain bubble for the question (the restore path re-renders the `bor.chat.v1` record).
|
||||
- `test_baseline_no_pagehide_still_completes` — the same long question with NO dispatched events completes identically (guards against an over-eager fix changing the normal path).
|
||||
- If task 01 identified C2 and the re-arm landed: `test_pre_token_guard_survives_hidden_window` — `think out loud then hesitate` question (pure pre-token pause), dispatch the same hidden/`pagehide`/visible sequence DURING the pause, assert the turn still settles with the answer (no "taking a long time" error). If C2 did NOT fire, omit this test with a one-line header comment saying so.
|
||||
2. Regression runs (isolation, per AGENTS.md rule 9): `uv run pytest tests/e2e/test_chat_rag.py -v --no-cov`, `test_chat_persistence.py` (pagehide partial-persist on real navigation — the unchanged behavior), `test_chat_history.py` (phase-50 saved chats), `test_stop_generation.py` (phase-48 Stop/teardown contract), `test_retry_answer.py`.
|
||||
3. `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
4. Commit (Conventional Commits, `--no-gpg-sign`) — e.g. `fix(chat): keep generating while the tab is hidden` with the task-01 root-cause line (C1–C4 finding) in the message body; move `.agents/phases/todo/73_hidden_tab_stream/` → `.agents/phases/complete/`.
|
||||
|
||||
## Testing & Quality
|
||||
- E2E: `uv run pytest tests/e2e/test_hidden_tab_stream.py -v --no-cov` green in isolation (DB up; mock LLM).
|
||||
- Coverage: **>90%** on `app/` (no `app/` changes expected — the floor holds by the full suite).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] The three (or four) E2E tests above pass in isolation; the double-brain-turn corruption is pinned (exactly-one-brain-turn assertion).
|
||||
- [ ] Regression suites green in isolation.
|
||||
- [ ] Full suite + coverage >90% + ruff + pyright clean.
|
||||
- [ ] One atomic `--no-gpg-sign` commit carrying the root-cause line; phase dir moved to `.agents/phases/complete/`.
|
||||
@@ -0,0 +1,48 @@
|
||||
# Task 01 — Server: optional client-provided history, trimmed + mapped (incl. `reasoning_content`)
|
||||
|
||||
**Phase:** `74_llm_chat_history` · **Source:** `TODO.md:4` — "Chat history isn't being passed to the LLM. When the LLM responds and you ask a follow-up question the previous question/answer isn't passed to the model. Since my models support preserve thinking, make sure to pass previous thinking blocks as well."
|
||||
**Story:** n/a (TODO-derived)
|
||||
|
||||
## Objective
|
||||
`POST /api/chat` accepts an optional `history` (the client's prior turns) and feeds it to the model on BOTH turn branches — the deflected path and the grounded agent — with prior thinking blocks preserved via the endpoint's existing `reasoning_content` wire convention; oversized histories are trimmed oldest-first by configurable caps.
|
||||
|
||||
## Work
|
||||
1. `app/config.py` — two new `Settings` fields (existing docstring style, `BOR_`-overridable):
|
||||
- `history_max_turns: int = 40` (`BOR_HISTORY_MAX_TURNS`) — newest history turns kept.
|
||||
- `history_max_chars: int = 24000` (`BOR_HISTORY_MAX_CHARS`) — total kept history chars (text + thinking).
|
||||
2. `app/schemas.py` — next to `ChatRequest`:
|
||||
- `class HistoryTurn(BaseModel)`: `who: Literal["user", "brain"]`, `text: str = Field(min_length=1, max_length=4000)`, `thinking: str | None = Field(default=None, max_length=32000)`.
|
||||
- `ChatRequest.history: list[HistoryTurn] = Field(default_factory=list, max_length=100)` — schema-level hard ceiling (DoS sanity); the config budgets below do the real trimming.
|
||||
3. `app/rag/prompts.py` — new pure helper (unit-testable, no I/O):
|
||||
```python
|
||||
def history_to_messages(history: Sequence[HistoryTurn], settings: Settings) -> list[dict[str, Any]]
|
||||
```
|
||||
- Maps newest-first until BOTH budgets are hit (turn count ≤ `history_max_turns`; cumulative chars — `len(text) + len(thinking or "")` — ≤ `history_max_chars`), then returns the kept turns in chronological (oldest→newest) order.
|
||||
- A budget-exceeding turn is DROPPED WHOLE — never cut mid-answer.
|
||||
- `who="user"` → `{"role": "user", "content": text}`.
|
||||
- `who="brain"` → `{"role": "assistant", "content": text}` plus `"reasoning_content": thinking` ONLY when `thinking` is non-empty (the preserve-thinking wire convention `app/rag/llm.py` already reads on the response side — `delta.reasoning_content`).
|
||||
4. `app/api/chat.py`:
|
||||
- After `settings = get_settings()` in `stream()`: `hist = history_to_messages(request.history, settings)` (once per turn; the deflected recovery below reuses it via `messages`).
|
||||
- Deflected branch: build `messages = [{"role": "system", "content": plan.system_prompt}, *hist, {"role": "user", "content": request.message}]` (replaces the two-entry list at ~L381). The phase-71 recovery already rebuilds from `messages[1:]` — unchanged.
|
||||
- Grounded branch: pass `history=hist` into the `run_agent(...)` call (~L414).
|
||||
- Per-turn log line (PLAN §9, the `logger.info` at ~L636): add `history_msgs={len(hist)}` to the existing line (append it near the other context-size fields, e.g. after `kb_chars`).
|
||||
5. `app/rag/agent.py` — `run_agent(..., history: Sequence[dict[str, Any]] = (), ...)`: build `messages = [{"role": "system", "content": system_prompt}, *history, {"role": "user", "content": user_message}]` (~L863). Everything downstream (tool rounds, phase-71 recovery rebuilding from `messages`, retry restarts) already operates on `messages` — unchanged.
|
||||
6. `app/rag/llm.py` — widen the message parameter type of `chat`, `chat_stream`, and `chat_stream_retried` from `list[dict[str, str]]` to `list[dict[str, Any]]` (an assistant message may now carry `reasoning_content`; the openai SDK passes message dicts through to the request body verbatim, so no transport change — string-only messages stay byte-identical on the wire).
|
||||
7. Tests:
|
||||
- `tests/unit/` (extend the existing prompts/chat test files, e.g. `tests/unit/test_chat_gate.py` or a new `tests/unit/test_history.py`): `history_to_messages` — empty default → `[]`; user/brain mapping; `reasoning_content` present only when thinking non-empty; turn-cap trim (newest kept, oldest dropped whole); char-cap trim (text+thinking accounted, drop-whole semantics); chronological order of the result.
|
||||
- `tests/unit/test_config.py`: the two new settings + `BOR_` env overrides (existing pattern).
|
||||
- `tests/unit/test_agent.py`: `run_agent` with a non-empty `history` places it between system and the current user message (mock `LLMClient` capturing the sent messages); default `history=()` keeps the two-message request.
|
||||
- `tests/integration/test_chat_api.py`: `POST /api/chat` with `history` — deflected turn (off-topic message) and grounded turn (on-topic message) both forward the prior turns to the mock LLM client (assert on the captured request messages, including `reasoning_content`); a request without `history` sends exactly `[system, user]` (byte-identical behavior pin); schema rejection: `who="alien"` → 422; `history` > 100 entries → 422; the per-turn log line carries `history_msgs=N` (caplog).
|
||||
- - ASSUMPTION A2 (owner-confirmed 2026-09-08): `/api/chat` stays STATELESS per owner-locked A10 — the history is client-provided in the request body; the server stores nothing new (no `saved_chats`/`query_log` change beyond the log field).
|
||||
- - ASSUMPTION A3 (owner-confirmed 2026-09-08): history is sent for BOTH grounded and deflected turns; caps = newest 40 turns / 24 000 chars (new `BOR_HISTORY_MAX_TURNS` / `BOR_HISTORY_MAX_CHARS`); a capped-out turn is dropped whole, never truncated.
|
||||
- - ASSUMPTION A4 (owner-confirmed 2026-09-08): prior thinking travels as `reasoning_content` on the assistant message (the convention `app/rag/llm.py` already documents for the response side), only when non-empty — this is what makes the owner's preserve-thinking models keep the reasoning chain.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit/integration as listed in Work item 7 — every branch of the mapper and both API branches is covered.
|
||||
- Coverage: **>90%** on `app/` for this task's new/modified code (the mapper is pure — easy to pin).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `history_to_messages` unit tests green (mapping, `reasoning_content` gating, both budgets, drop-whole, ordering).
|
||||
- [ ] Integration: deflected AND grounded turns forward history to the mock LLM; no-`history` requests are unchanged; 422 pins; log line gains `history_msgs=N`.
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` >90%; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] No commit in this task (task 03 commits the phase) — but `uv run .agents/validate.sh`-equivalent gates must pass.
|
||||
@@ -0,0 +1,34 @@
|
||||
# Task 02 — Client: `runTurn` sends the conversation history (with thinking) in the request body
|
||||
|
||||
**Phase:** `74_llm_chat_history` · **Source:** `TODO.md:4` — "Chat history isn't being passed to the LLM. When the LLM responds and you ask a follow-up question the previous question/answer isn't passed to the model. Since my models support preserve thinking, make sure to pass previous thinking blocks as well."
|
||||
**Story:** n/a (TODO-derived)
|
||||
|
||||
## Objective
|
||||
The browser sends the prior conversation on every turn — user turns, brain answers, and the brain turns' `thinking` blocks — as the new `history` field of the `POST /api/chat` body, so follow-up questions are answered WITH the conversation the user has already seen.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/app.js` — in `runTurn` (~L1859), the single funnel for ALL turns (fresh send, phase-49 retry via `runTurn(text, { reask: true })`, phase-53 stale-regen — verify the invariant while editing: in every one of those paths the question being sent is the LAST entry of `conversation` at fetch time — a fresh send just pushed it; `retryLastTurn` ~L1798 pops the old brain record and keeps the question as the last entry; `regenerateStaleChat` ~L1617 follows the same pop-then-reask pattern):
|
||||
```js
|
||||
const history = conversation.slice(0, -1).map((m) => ({
|
||||
who: m.who,
|
||||
text: m.text,
|
||||
thinking: m.who === "brain" ? m.thinking || undefined : undefined,
|
||||
}));
|
||||
```
|
||||
and send `body: JSON.stringify({ message: text, history })` in the existing `fetch("/api/chat", …)` (~L1905, replacing `{ message: text }`).
|
||||
- `thinking` is only present on brain records that actually streamed one (phase-17+ optional key) — `undefined` drops the key from the JSON (the record's existing convention).
|
||||
- Old/restored records without `thinking` send none — the server maps that to a plain assistant message.
|
||||
- The current question itself is NEVER in `history` (the `slice(0, -1)`) — it is the request's `message`, exactly as the server expects.
|
||||
- No UI change: nothing renders differently; the request body just carries the record the user already sees.
|
||||
2. - ASSUMPTION A5 (owner-confirmed 2026-09-08): stopped/partial brain turns (the `stopped: true` records) are included in the history like any other prior answer — the persisted record is the source of truth, and what the user saw is what the model should treat as the prior answer.
|
||||
3. Sanity check (no code): the server-side schema (task 01) accepts exactly `{who, text, thinking}` — no other record keys (`sources`, `tools`, `deflected`, `suggestions`) travel in the body; `tools` metadata is display-only and was never part of the LLM wire.
|
||||
|
||||
## Testing & Quality
|
||||
- No JS unit-test infra in this repo — the wire-level pin is task 03's E2E (the mock sees the exact messages the client sent).
|
||||
- Manual smoke while running the dev server: DevTools → Network → the second `/api/chat` request's body carries the first Q/A pair (and `thinking` when the first turn streamed one).
|
||||
- `uv run pytest` stays green (frontend-only change).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Every `/api/chat` request carries `history` = the `conversation` record minus the current question, with `thinking` on brain turns that have it (verified in DevTools for a 2-turn conversation).
|
||||
- [ ] Retry (phase 49) and stale-regen (phase 53) send the correct history for their re-asked question (the popped/replaced answer is NOT in the history; the question is NOT duplicated).
|
||||
- [ ] No UI change; `uv run pytest` green; ruff + pyright clean.
|
||||
Reference in New Issue
Block a user