docs(quick-1): add execution summary for internal-error bug fix
This commit is contained in:
1 parent
44875aa03e
commit
abb9db0b77
1 file changed
+81
@@ -0,0 +1,81 @@
|
||||
---
|
||||
phase: quick
|
||||
plan: 1
|
||||
subsystem: inference-core, inference-server, chat-ui
|
||||
tags: [bug-fix, kv-cache, streaming, chat-history]
|
||||
dependency_graph:
|
||||
requires: []
|
||||
provides: [stable-multi-turn-chat, clean-error-propagation]
|
||||
affects: [inference-core, inference-server, ChatView]
|
||||
tech_stack:
|
||||
added: []
|
||||
patterns: [fail-fast-on-null, empty-content-guard, history-filter]
|
||||
key_files:
|
||||
modified:
|
||||
- crates/inference-core/src/engine.rs
|
||||
- crates/inference-core/src/session.rs
|
||||
- crates/inference-server/src/routes.rs
|
||||
- tauri-app/src/components/ChatView.tsx
|
||||
decisions:
|
||||
- "Return Err from new_session() when llama_get_memory is NULL rather than silently continuing — prevents decode corruption on second request"
|
||||
- "StreamItem::Error emits content:None so fullContent stays empty and is not persisted as a valid assistant message"
|
||||
- "Assistant INSERT is guarded against empty content to avoid empty DB rows when stream fails silently"
|
||||
- "buildApiMessages filters out empty and error-prefixed messages so corrupted history rows do not poison future context"
|
||||
metrics:
|
||||
duration: ~10min
|
||||
completed_date: "2026-03-30"
|
||||
tasks: 2
|
||||
files: 4
|
||||
---
|
||||
|
||||
# Quick Task 1: Fix "internal error" on Second Chat Message
|
||||
|
||||
**One-liner:** Three-layer fix for KV cache corruption causing "internal error" accumulation and DB pollution on the second chat turn.
|
||||
|
||||
## What Was Built
|
||||
|
||||
### Root Cause
|
||||
|
||||
On the second request, `llama_get_memory` returns NULL on some llama.cpp builds. The previous code issued a warning and continued, leaving the KV cache from the prior session in place. The new prefill then tried to write tokens at positions already occupied, causing `llama_decode` to return non-zero. The engine propagated `StreamItem::Error`, which the frontend accumulated as real content (`"internal error"`) and saved to the DB — which then corrupted the history sent on the third request.
|
||||
|
||||
### Fix Layer 1 — engine.rs: Fail fast on NULL KV cache
|
||||
|
||||
`new_session()` now returns `Err` immediately when `llama_get_memory` returns NULL instead of logging a warning and continuing. This surfaces the failure as an HTTP 500 rather than a silent decode corruption.
|
||||
|
||||
### Fix Layer 2 — session.rs: Improve prefill error diagnostic
|
||||
|
||||
`decode_prefill` error message now includes the total token count so operators can distinguish KV cache failures from context window overflows.
|
||||
|
||||
### Fix Layer 3 — routes.rs: StreamItem::Error emits no content
|
||||
|
||||
`StreamItem::Error` now sends a final chunk with `content: None` instead of `content: Some("internal error")`. The frontend's `fullContent` accumulator stays empty, so nothing gets persisted to the DB.
|
||||
|
||||
### Fix Layer 4 — ChatView.tsx: Guard assistant INSERT and filter history
|
||||
|
||||
Two frontend fixes:
|
||||
1. `buildApiMessages` SQL excludes empty content, `⚠️ Error:` prefixed messages, and `"internal error"` rows — so any pre-existing corrupted rows do not reach the backend.
|
||||
2. The assistant `db.run(INSERT ...)` is wrapped in a guard: if `fullContent` is empty after stream completion, no row is written.
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
None — plan executed exactly as written. The plan prescribed four specific fixes across three files; all were applied verbatim.
|
||||
|
||||
## Self-Check
|
||||
|
||||
**Files exist:**
|
||||
- `crates/inference-core/src/engine.rs` — FOUND
|
||||
- `crates/inference-core/src/session.rs` — FOUND
|
||||
- `crates/inference-server/src/routes.rs` — FOUND
|
||||
- `tauri-app/src/components/ChatView.tsx` — FOUND
|
||||
|
||||
**Commits exist:**
|
||||
- `d919879` — FOUND (Task 1: engine.rs + session.rs)
|
||||
- `44875aa` — FOUND (Task 2: routes.rs + ChatView.tsx)
|
||||
|
||||
**Compilation:**
|
||||
- `cargo check -p inference-core -p inference-server` — PASSED
|
||||
- `npx tsc --noEmit` — PASSED
|
||||
|
||||
**Note on test runner:** `vitest run` fails with `ERR_PACKAGE_PATH_NOT_EXPORTED` for `vite/module-runner` under Node 24 + vitest 4.1.2 + vite 5.4.x. This is a pre-existing environment incompatibility (vitest 4.x requires vite 6.x) unrelated to these changes. The test file contracts are compatible with the SQL change (`buildApiMessages` mock matches on `FROM messages WHERE chat_id = ?` which is still present in the updated query).
|
||||
|
||||
## Self-Check: PASSED
|
||||
Reference in new issue
Block a user