phase: 114_embed_question_length
All verification passes complete — the phase was already fully implemented in the working tree, and every gate is green. No defects found; no code changes were needed.
**Final verification pass — Phase 114 (embed question length):**
- Verified truncation: `chat.py:459` embeds `request.message[:settings.embed_question_max_chars]` (default 1200, `BOR_EMBED_QUESTION_MAX_CHARS`, `>0` validator); full question still reaches the LLM prompt/log.
- Verified error mapping: `EmbeddingInputTooLargeError(EmbeddingError)` (byte-identical message) caught **before** `EmbeddingError` → no retry, terminal frame `detail="Question too long — trim it and re-ask."` + reachability-fine hint; `ChatErrorEvent.hint` additive.
- Verified frontend chain: frame `hint` → `err.hint` → `setUiState(error, …, {hint})` → `showErrorBanner(…, {retryable: true})` — hint replaces default `ERROR_HINT`, phase-111 `#banner-retry` button revealed. 4,000-char clamp untouched (A2).
- `uv run pytest tests/unit/test_embed_question_length.py -v --no-cov` → 21 passed
- `uv run pytest tests/e2e/test_embed_question_length.py -v --no-cov` (isolation, DB up) → 1 passed (4,000-char question → done, no banner)
- Regression: `test_llm_retry.py` 4 passed · `test_oneshot_llm_retry.py` 2 passed · `test_chip_sizing_question_cap.py` 6 passed
- `uv run pytest --cov=app --cov-report=term-missing` → 2444 passed, TOTAL **99%** (>90% gate)
- `uv run ruff check .` → All checks passed; `uv run pyright` → 0 errors, 0 warnings
**Completion criteria:** (1) 4,000-char question embeds prefix + full prompt ✅ · (2) too-large → accurate frame + hint + Retry button ✅ · (3) reachability failure byte-identical (retries + old copy) ✅ · (4) all gates green ✅ · (5) commit/phase-move → left to the harness per instructions (no `git add`/`commit` run).
**Deviations:** none. **Next pending phase:** `115_doc_draft_discard`.
This commit is contained in:
@@ -0,0 +1,45 @@
|
||||
# Phase 114 — Embed question length: truncation + accurate error (TODO L6)
|
||||
|
||||
**Source:** `TODO.md` L149–181 — "L6 — 4,000-char question clamp exceeds the embed model's input cap → misleading 'couldn't reach the embedding model' error (2026-09-15, brain-of-reese interactive test)"
|
||||
**Story:** n/a (interactive-test follow-up fix; extends the phase-67 LLM-retry and phase-06 loading-feedback assets).
|
||||
**Context:** The composer clamps at 4,000 chars; `app/api/chat.py:423` embeds the **full** question via `llm.embed_one`; aipi's litellm rejects the ~903-token input with **HTTP 500**: "input (903 tokens) is too large to process. increase the physical batch size (current batch size: 512)". The single-text path in `app/rag/llm.py` (`_embed_batch` → `_TooLarge`, ~L279–284) turns that into `EmbeddingError("a single …-char chunk exceeded the endpoint's per-request input token cap — lower BOR_CHUNK_TARGET_CHARS and re-import")` — an **import-oriented** message — and the chat endpoint's catch-all (~L428–444) maps EVERY `EmbeddingError` to "I couldn't reach the embedding model — please try again." Both diagnoses are wrong (reachability is fine; the chunker constant is irrelevant to a question). The chunker's own `HARD_MAX_CHARS = 1200` (`app/rag/chunker.py:51`, ~1024 tokens at ~1.4 chars/token) shows the question path never got the same treatment.
|
||||
|
||||
## Objective
|
||||
Every legal question (≤ the UI clamp) is embeddable: the embed step gets a bounded prefix of the question (the chunker's 1200-char budget) while the full question still reaches the LLM prompt; and if the input is still too large (a smaller-cap model, a misconfiguration), the turn fails with an accurate "question too long" error — no false reachability diagnosis, no wasted retries — and the banner carries the phase-111 Retry button.
|
||||
|
||||
## Dependencies
|
||||
- `111_chat_banner_retry` (todo) — L6's acceptance: "the L1 'Try again' button fix should also apply to this banner" — the too-long error flows through the same turn-error state machine, so the phase-111 Retry button is offered on it.
|
||||
|
||||
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||
- **Truncation (task 01):** new setting `embed_question_max_chars: int = 1200` (env `BOR_EMBED_QUESTION_MAX_CHARS`, default = the chunker's `HARD_MAX_CHARS` budget, validated `> 0`). The chat embed step (chat.py:423) embeds `request.message[:settings.embed_question_max_chars]`; the LLM prompt build is unchanged (the full question still reaches the model). Questions shorter than the budget are byte-identical to today.
|
||||
- **Error mapping (task 02):** `app/rag/llm.py` — new `EmbeddingInputTooLargeError(EmbeddingError)` subclass; the single-text `_TooLarge` branch of `_embed_batch` raises it (same message text — the importer path is byte-identical, it still catches `EmbeddingError`). The chat endpoint catches `EmbeddingInputTooLargeError` **before** `EmbeddingError` inside the phase-67 retry loop → no retry (a deterministic failure — locked A3) → terminal `ChatErrorEvent` with `detail="Question too long — trim it and re-ask."` and a new optional `hint` field: `hint="The app reached the embedding model fine — only the question length is the problem."` `ChatErrorEvent` gains `hint: str | None = None` (additive; PLAN §4 old-client ignore contract). The frontend's phase-111 reworked `showErrorBanner(detail, opts)` shows `opts.hint` when the frame carries one, else the default `ERROR_HINT`.
|
||||
- **Retry:** the too-long frame flows through the turn-error state machine → the phase-111 banner Retry button is offered (re-asking is the user's call after trimming; the composer clamp still applies).
|
||||
- **NOT touched:** the importer's embed path and its batch-halving `_TooLarge` behavior/error copy, the 4,000-char composer clamp (locked A2 — truncation, not a lower clamp), the reachability-failure retry semantics (phase 67 — byte-identical).
|
||||
|
||||
## Tasks
|
||||
1. `01_embed_truncation.md` — the bounded-prefix embed + the setting.
|
||||
2. `02_too_long_error_mapping.md` — `EmbeddingInputTooLargeError`, the chat-path mapping, `ChatErrorEvent.hint`, the frontend hint support.
|
||||
3. `03_embed_length_tests.md` — the unit pins + the 4,000-char E2E.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_embed_question_length.py` (new, task 03) — truncation (long → prefix embedded, LLM prompt carries the full text; short → byte-identical), error mapping (too-large failure → exact detail + hint, no retry frame, one attempt; transport failure → legacy reachability path with retries — the regression pin), the config validator.
|
||||
- E2E: `tests/e2e/test_embed_question_length.py` (new, task 03; run in isolation: `uv run pytest tests/e2e/test_embed_question_length.py -v --no-cov`) — a 4,000-char question (the composer clamp) streams to done (mock LLM), no error banner.
|
||||
- Regression: `tests/e2e/test_llm_retry.py`, `test_oneshot_llm_retry.py`, `test_chip_sizing_question_cap.py` (the 4,000-char counter) stay green.
|
||||
- Coverage: **>90%** on `app/` (validate.sh gate).
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A 4,000-char question embeds (bounded prefix) and the turn succeeds; the LLM prompt carries the full question.
|
||||
- [ ] A too-large embed failure (forced in a unit test) → the accurate "Question too long" frame + the reachability-fine hint; the banner offers the phase-111 Retry button.
|
||||
- [ ] A reachability embed failure behaves byte-identically to pre-phase (retries + old copy).
|
||||
- [ ] `uv run pytest` green; coverage >90%; the e2e file green in isolation; `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] One `--no-gpg-sign` commit; phase dir moved to `complete/` by the pipeline gate.
|
||||
|
||||
## Locked decisions
|
||||
- **A1 — both fixes combined: 1200-char embed truncation (default = the chunker budget, env-tunable) + the precise too-long error mapping (owner-confirmed 2026-09-14, roadmap confirmation).**
|
||||
- **A2 — the 4,000-char composer clamp stays (owner-confirmed 2026-09-14) — truncation, not a lower clamp.**
|
||||
- **A3 — a too-large embed failure is NOT retried (deterministic failure) — it short-circuits the phase-67 retry loop (owner-confirmed 2026-09-14).**
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add app/ tests/ frontend/ .agents/phases/ && git commit --no-gpg-sign -m "fix(rag): embed a bounded question prefix (1200-char budget) and map the embed too-large failure to an accurate too-long error with a reachability-fine hint"
|
||||
```
|
||||
@@ -0,0 +1,21 @@
|
||||
# Task 01 — Embed a bounded question prefix
|
||||
|
||||
**Phase:** `114_embed_question_length` · **Source:** `TODO.md:151–164, 168–170` — "Repro: type/paste a question to the UI maximum (the composer clamps at 4,000 chars — char counter shows '4000/4000 — character limit') and send. Result, **100% reproducible**: the turn dies pre-token with the banner 'I couldn't reach the embedding model — please try again.' … a short question embeds fine (HTTP 200), but the 4,000-char question (~903 tokens) gets **HTTP 500** from aipi … So the maximum legal question length exceeds the embed model's maximum legal input — and the chunker's own 1200-char cap (set to stay under the ~1024-token per-request cap) shows the question path never got the same treatment." + "**Truncate for embedding** — embed a bounded prefix of the question (e.g. the same 1200-char budget as chunks) while the full question still reaches the LLM prompt."
|
||||
|
||||
## Objective
|
||||
The chat embed step embeds at most `embed_question_max_chars` (default 1200 — the chunker's `HARD_MAX_CHARS` budget) of the question; the full question still reaches the LLM prompt.
|
||||
|
||||
## Work
|
||||
1. `app/config.py` — add `embed_question_max_chars: int = Field(default=1200)` (env `BOR_EMBED_QUESTION_MAX_CHARS`), validator `> 0`; `.env.example` entry with a comment citing the chunker rationale (`app/rag/chunker.py:30–51` — ~1.4 chars/token, stays under the ~1024-token per-request cap).
|
||||
2. `app/api/chat.py` — the embed step (~L423): `question_vec = await llm.embed_one(request.message[: settings.embed_question_max_chars])`. Everything downstream is unchanged: retrieval runs on the prefix vector (intended — the prefix is the question's head); the LLM prompt build (`hist` + the full `request.message`) is untouched; the per-turn log line is untouched (`question=%r` logs the full text).
|
||||
3. One-line comment at the call site: the prefix is bounded to the embed model's input cap (the chunker budget); the full question still reaches the LLM prompt (TODO L6).
|
||||
4. ASSUMPTION: the budget is a setting (env-tunable), default 1200 — not a hard-coded constant — so a model with a larger/smaller cap is accommodated without a code change (locked A1).
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_embed_question_length.py` (new, task 03) — a question > the budget → `embed_one` receives exactly the prefix (mock LLM client); the LLM request messages carry the full question; a question ≤ the budget → byte-identical call.
|
||||
- Coverage: **>90%** on `app/`.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A 4,000-char question → `embed_one` called with the 1200-char prefix; the LLM request carries the full 4,000-char message.
|
||||
- [ ] A short question → no behavior change (byte-identical call).
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,31 @@
|
||||
# Task 02 — Map the too-large embed failure to an accurate error
|
||||
|
||||
**Phase:** `114_embed_question_length` · **Source:** `TODO.md:171–173` — "**Map the 500 to a precise error** — detect the 'too large' embed failure and surface 'question too long — trim it' (and fix the ERROR_HINT for this case: reachability is fine)." + `TODO.md:179–181` — "Acceptance: a 4,000-char question either succeeds (truncated embedding) or fails with an accurate too-long error; unit test pins the error mapping; the L1 'Try again' button fix should also apply to this banner."
|
||||
|
||||
## Objective
|
||||
A deterministic "input too large" embed failure surfaces as a precise "question too long" terminal error with a hint that reachability is fine — no false reachability diagnosis, no wasted retries; the error banner carries the phase-111 Retry button (the turn-error path).
|
||||
|
||||
## Work
|
||||
1. `app/rag/llm.py` — add `class EmbeddingInputTooLargeError(EmbeddingError)` (near `EmbeddingError`, L42), with a docstring: the single-text input exceeded the endpoint's token cap — deterministic, not a reachability failure. In the single-text `_TooLarge` branch of `_embed_batch` (~L279–284): raise `EmbeddingInputTooLargeError(<the existing import-oriented message>)` instead of plain `EmbeddingError` — the message text is **identical** (the importer path is byte-identical; it still catches `EmbeddingError`, and the subclass is a drop-in).
|
||||
2. `app/schemas.py` — `ChatErrorEvent` gains `hint: str | None = None` (additive; docstring: the client shows the hint in place of its default reachability hint when present; old clients ignore the field — PLAN §4).
|
||||
3. `app/api/chat.py` — the embed-failure handling (~L428–444, inside the phase-67 retry `while` loop): catch `EmbeddingInputTooLargeError` **before** `EmbeddingError` → do NOT restart (locked A3 — deterministic) → `settled = True`, log an error line (the existing format plus a `too-large` marker), and yield:
|
||||
```python
|
||||
ChatErrorEvent(
|
||||
detail="Question too long — trim it and re-ask.",
|
||||
hint="The app reached the embedding model fine — only the question length is the problem.",
|
||||
).model_dump()
|
||||
```
|
||||
The existing `EmbeddingError` branch (reachability) is unchanged, including the retry semantics and the old copy.
|
||||
4. `frontend/assets/app.js` — the phase-111 reworked `showErrorBanner(detail, opts)`: honor `opts.hint` — `bannerText.textContent = detail ? \`${detail} ${opts.hint ?? ERROR_HINT}\` : (opts.hint ?? ERROR_HINT)`. The SSE error-frame handler in the stream state machine (~L1281): pass `{ retryable: true, hint: ev.hint }` when the frame carries a hint.
|
||||
5. ASSUMPTION: detail copy "Question too long — trim it and re-ask." (the TODO's "question too long — trim it", phrased as a banner sentence); hint copy as in work item 3.
|
||||
6. ASSUMPTION: no retry on too-large (locked A3) — the phase-67 retry loop is for transient failures; a size failure is guaranteed to repeat.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: `tests/unit/test_embed_question_length.py` (new, task 03) — force the `_TooLarge` branch (a fake httpx response: HTTP 500 + a "too large to process" body) → the chat SSE stream yields exactly one error frame with the precise detail + hint and **no** retry frame; a transport failure (no "too large" signature) → the legacy reachability path with the retry loop and old copy (the regression pin).
|
||||
- Coverage: **>90%** on `app/` including the new exception class and branch.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A too-large embed failure → the frame `{type: "error", detail: "Question too long — trim it and re-ask.", hint: "…fine…"}` — no "couldn't reach" copy, no retry frame.
|
||||
- [ ] A reachability embed failure → byte-identical to pre-phase (retries + old copy).
|
||||
- [ ] The frontend shows the frame's hint when present; the banner offers the phase-111 Retry button on this error.
|
||||
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean.
|
||||
@@ -0,0 +1,25 @@
|
||||
# Task 03 — Unit + E2E pins for the 4,000-char question
|
||||
|
||||
**Phase:** `114_embed_question_length` · **Source:** `TODO.md:179–181` — "Acceptance: a 4,000-char question either succeeds (truncated embedding) or fails with an accurate too-long error; unit test pins the error mapping; the L1 'Try again' button fix should also apply to this banner."
|
||||
|
||||
## Objective
|
||||
The acceptance is pinned: a full-clamp (4,000-char) question succeeds end-to-end (truncated embed), and the too-long mapping is unit-pinned.
|
||||
|
||||
## Work
|
||||
1. `tests/unit/test_embed_question_length.py` (new):
|
||||
- **truncation:** a mock `LLMClient` records the `embed_one` input; a 4,000-char question → exactly the prefix (default budget); the chat request to the LLM carries the full question; a 100-char question → byte-identical call.
|
||||
- **error mapping:** a fake embed transport returning HTTP 500 + "too large to process" body for the input → the chat SSE stream yields exactly one error frame with the precise detail + the reachability-fine hint and no retry frame; short input + a 500 WITHOUT the "too large" signature → the legacy reachability path (retry frames + old copy) — the regression pin.
|
||||
- **config:** the `embed_question_max_chars` default (1200) and validator.
|
||||
2. `tests/e2e/test_embed_question_length.py` (new; the `tests/e2e/` conftest + mock-LLM pattern): type a 4,000-char question into the composer (the counter shows "4000/4000") → send → the turn streams to done (mock LLM) — no error banner.
|
||||
3. Run in isolation: `uv run pytest tests/e2e/test_embed_question_length.py -v --no-cov` (DB up).
|
||||
4. Regression: `tests/e2e/test_llm_retry.py`, `test_oneshot_llm_retry.py`, `test_chip_sizing_question_cap.py` (the 4,000-char counter) stay green.
|
||||
|
||||
## Testing & Quality
|
||||
- Unit: as above (the acceptance pin: the error mapping).
|
||||
- E2E: the 4,000-char success path (the acceptance pin: the truncated embed).
|
||||
- Coverage: **>90%** on `app/`.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] A 4,000-char question → a successful turn (E2E); the embed input was the prefix (unit).
|
||||
- [ ] The too-long mapping is unit-pinned (exact frame, no retry).
|
||||
- [ ] `uv run pytest` green; coverage >90%; the e2e file green in isolation; `uv run ruff check . && uv run pyright` clean.
|
||||
Reference in New Issue
Block a user