chore(agent): phase roadmap from TODO.md — 3 phases (138–140)
This commit is contained in:
@@ -0,0 +1,45 @@
|
|||||||
|
# Phase 138 — Markdown blockquotes render: `>` quotes become styled `<blockquote>`s
|
||||||
|
|
||||||
|
**Source:** `TODO.md` L3 — "Markdown formatted ">" quotes aren't rendering"
|
||||||
|
**Story:** n/a (TODO-derived phase, Protocol B append).
|
||||||
|
**Context:** `frontend/assets/markdown.js` — the shared escape-first renderer (loaded by all three pages: `index.html` chat, `document.html` viewer, `shared.html`): step 1 fence protection (`\u0000CODEn\u0000`), step 1b table protection (`\u0000TABLEn\u0000`, phase 44), step 2 `inline(escapeHtml(text))` (the inline chain: code → strong → em → h3/h4 → ul → ol), step 3 the paragraph pass (split on `\n{2,}`; the block-element guard `/^<(h\d|ul|ol|pre|li)/`), step 4 restore. Today a `>` line is escaped to `>` and renders as literal text in a `<p>` (verified: `renderMarkdown("> quoted")` → `<p>> quoted</p>`). `frontend/assets/styles.css` — the `:root` palette (`--ink-soft` 8.2:1 on `--surface`, `--line`, the `--brand-*` family; the `:root`-drift unit test parses the built-ins from this file — no second copy) and the chat-bubble / viewer typography. `tests/unit/test_markdown_tables.py` — the frontend source-pin + node-behavior unit pattern (`HAVE_NODE` skip); `tests/e2e/test_markdown_tables.py` — the phase-44 E2E pattern (mock flow with table output + the document viewer).
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
`>`-prefixed lines in any rendered markdown — a chat answer, the thinking block, a saved/shared chat (restore), the document viewer — render as a semantic `<blockquote class="md-quote">` with theme-safe styling; every non-quote input renders byte-identical to today (the phase-44 table suite and the renderer-dependent unit suites stay green unedited).
|
||||||
|
|
||||||
|
## Dependencies
|
||||||
|
- `137_pdf_image_context` (complete) — pipeline order only; the code dependencies are the shared renderer + `styles.css` on `main`.
|
||||||
|
|
||||||
|
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||||
|
- **The pass (task 01):** a new line-block pass in `markdown.js` BETWEEN step 2 (`inline(escapeHtml(text))`) and step 3 (paragraphs) — at that point the text is escaped (a quote line starts with `>`) and already inline-transformed (bold/emphasis/code work inside quotes for free — the pass never re-transforms). Group the maximal run of consecutive lines matching `^\s*>+ ?` into ONE `<blockquote class="md-quote">…</blockquote>`: strip the leading `>` run plus ONE optional space from each member (nested `>>` / `>>>` collapses to one level — ASSUMPTION A1), join members with `<br>` (the paragraph convention). A BLANK line terminates the group (a re-quoted line starts a new `<blockquote>` — simple, predictable). The step-3 guard extends to `/^<(h\d|ul|ol|pre|li|blockquote)/` so the paragraph pass leaves the block alone (no `<p>` wrap).
|
||||||
|
- **Precedence is untouched:** fences protect first (step 1 — a `>` inside a code block never reaches the pass), tables protect second (step 1b — cell content is escaped inside the `<td>` markup and restored in step 4, AFTER the pass). No other pass changes.
|
||||||
|
- **In-quote structure (documented limit):** a markdown STRUCTURE inside a quote (a `- list` line, a `# heading` line) renders as plain text inside the blockquote — the inline chain runs before the pass, so its line-start transforms never see the post-strip text. Inline (code/strong/em) still works. Acceptable — the scope is quote rendering (owner L3).
|
||||||
|
- **The CSS (task 02):** `.md-quote` in `styles.css` — theme-safe from the EXISTING `:root` palette only: left border + horizontal padding + vertical margin + a muted text color carrying a ≥4.5:1 contrast comment on the bubble surface (the house comment convention, cf. `--ink-soft`'s `/* 8.2:1 on --surface */`). ONE rule set serves all three contexts (chat `.bubble`, `.thinking-text`, the document viewer) — a scoped override only if the E2E proves a clash.
|
||||||
|
- **Byte-identity (task 03):** any input without a quote line renders byte-identical to the pre-phase renderer — pinned by the node behavior check and by the unedited `test_markdown_tables.py` / `test_thinking_scroll.py` / `test_document_viewer.py` suites.
|
||||||
|
- **E2E (task 03):** dedicated `tests/e2e/test_markdown_blockquotes.py`, run in isolation (AGENTS.md rules 4/9), mock-only.
|
||||||
|
|
||||||
|
## Tasks
|
||||||
|
1. `01_blockquote_pass.md` — the blockquote pass in `markdown.js` + the paragraph-guard extension.
|
||||||
|
2. `02_blockquote_css.md` — the `.md-quote` styling in `styles.css` (theme-safe, WCAG AA contrast).
|
||||||
|
3. `03_unit_and_e2e.md` — unit source-pins + node behavior + the dedicated Playwright E2E.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: `tests/unit/test_markdown_blockquotes.py` (new — the phase-44 pattern): source pins (pass order fences → tables → escape+inline → **blockquotes** → paragraphs → restore; the guard regex includes `blockquote`; the exact `<blockquote class="md-quote">` marker; the `.md-quote` CSS rule references palette vars only) + node behavior (single-line quote, multi-line group, nested collapse, XSS quote inert, fence-wins, a table cell containing `>` untouched, non-quote input byte-identical).
|
||||||
|
- E2E: `tests/e2e/test_markdown_blockquotes.py` (new), in isolation.
|
||||||
|
- Coverage: **>90%** on `app/` (the gate runs the full suite — this phase adds no `app/` code, so the suite staying green IS the gate).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `>` quotes render as a styled `<blockquote class="md-quote">` in the chat answer, the thinking block, saved/shared chats (restore), and the document viewer
|
||||||
|
- [ ] nested `>>` collapses to one level; a `>` inside a code fence or a table cell is untouched
|
||||||
|
- [ ] non-quote input renders byte-identical (the phase-44 / renderer-dependent suites pass unedited)
|
||||||
|
- [ ] `uv run pytest` green; `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run pytest tests/e2e/test_markdown_blockquotes.py -v --no-cov` green in isolation; `uv run ruff check . && uv run pyright` clean
|
||||||
|
- [ ] one `--no-gpg-sign` Conventional Commit; phase dir moved to `.agents/phases/complete/` by the pipeline gate
|
||||||
|
|
||||||
|
## Locked decisions
|
||||||
|
- **A1 — nested quotes (`>>`, `>>>`) collapse to a single `<blockquote>` level (owner-confirmed 2026-09-30, roadmap confirmation).**
|
||||||
|
- **A2 — the quote style is a left border + padding + muted text color (NO background fill), built from the existing `:root` palette only (owner-confirmed 2026-09-30, roadmap confirmation).**
|
||||||
|
|
||||||
|
## Commit
|
||||||
|
```bash
|
||||||
|
git add frontend/ tests/ .agents/phases/ && git commit --no-gpg-sign -m "feat(ui): render markdown blockquotes in chat, viewer, and shared pages"
|
||||||
|
```
|
||||||
@@ -0,0 +1,22 @@
|
|||||||
|
# Task 01 — The blockquote pass in the shared renderer
|
||||||
|
|
||||||
|
**Phase:** `138_markdown_blockquotes` · **Source:** `TODO.md:3` — "Markdown formatted ">" quotes aren't rendering"
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
`renderMarkdown` turns runs of `>`-prefixed lines into `<blockquote class="md-quote">` elements — after the escape + inline step, before the paragraph pass — with fences and tables keeping their precedence and non-quote input byte-identical.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `frontend/assets/markdown.js` — new pass between the step-2 `text = inline(escapeHtml(text))` and the step-3 paragraph `.map(...)`: split the text on `\n`; group the maximal run of consecutive lines matching `^\s*>+ ?` (the escaped quote marker); strip the `>` run + one optional space from each member (nested `>>`/`>>>` collapses to one level — ASSUMPTION A1); join members with `<br>`; emit `<blockquote class="md-quote">…</blockquote>`. A blank line terminates the group.
|
||||||
|
2. `frontend/assets/markdown.js` — extend the step-3 block-element guard from `/^<(h\d|ul|ol|pre|li)/` to `/^<(h\d|ul|ol|pre|li|blockquote)/` so the paragraph pass leaves the block alone (no `<p>` wrap).
|
||||||
|
3. `frontend/assets/markdown.js` — update the module docstring: pass order is now fences → tables → escape+inline → **blockquotes** → paragraphs → restore; document the group rule (blank line terminates; nested collapses) and the documented limit (a list/heading line INSIDE a quote renders as plain text — the inline chain ran first; inline code/strong/em work).
|
||||||
|
- ASSUMPTION: a `>` inside a code fence is already protected by the step-1 placeholder (no handling needed); a `>` in a table cell is escaped inside the `<td>` markup and restored in step 4 — both are pinned in task 03's behavior checks.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: covered by task 03's file (the source pin + the node behavior listed there) — the frontend house pattern is one unit file per feature.
|
||||||
|
- Coverage: n/a (frontend) — the suite + the pins are the gate.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] node repro: `renderMarkdown("> a\n> b")` → `<blockquote class="md-quote">a<br>b</blockquote>`; `renderMarkdown("> quoted")` no longer emits a literal `>`
|
||||||
|
- [ ] a non-quote input (plain paragraph, list, table, fence) renders byte-identical to the pre-task renderer
|
||||||
|
- [ ] `uv run pytest` green (the renderer-dependent unit suites unedited); `uv run ruff check .` clean
|
||||||
@@ -0,0 +1,21 @@
|
|||||||
|
# Task 02 — `.md-quote` styling (theme-safe, WCAG AA)
|
||||||
|
|
||||||
|
**Phase:** `138_markdown_blockquotes` · **Source:** `TODO.md:3` — "Markdown formatted ">" quotes aren't rendering"
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
The blockquote is visually distinct and readable in all three rendering contexts (chat bubble, thinking block, document viewer) with ONE rule set, using only the existing `:root` palette.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `frontend/assets/styles.css` — add the `.md-quote` rule next to the other markdown element styles (the phase-44 `.md-table` / `.md-table-wrap` block is the anchor): `border-left: 3px solid <palette var>` (a visible-but-restrained border — pick from the existing `:root` palette, the executor's contrast judgment), `padding: 0.25rem 0.75rem`, `margin: 0.5rem 0`, `color: <muted palette var>` with a ≥4.5:1 contrast comment on the bubble surface per the house convention (`--ink-soft`'s `/* 8.2:1 on --surface */` qualifies — verify against the actual bubble background and document it).
|
||||||
|
2. `frontend/assets/styles.css` — no background fill (ASSUMPTION A2); verify the rule renders acceptably in the chat `.bubble`, the `.thinking-text` block, and the document-viewer content region — add a scoped override only if the task-03 E2E proves a clash.
|
||||||
|
- ASSUMPTION: the built-in dark palette is the single source of truth — the `:root`-drift unit test parses built-ins from this file, so no hex literal outside `:root` and no second copy.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: `tests/unit/test_markdown_blockquotes.py` (task 03) source-pins the `.md-quote` rule's presence and its palette-var-only references (the phase-44 CSS-pin pattern).
|
||||||
|
- Coverage: n/a (frontend).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] the `.md-quote` rule exists in `styles.css`, references only `:root` palette vars, and carries a ≥4.5:1 contrast comment
|
||||||
|
- [ ] the quote is visually distinct in chat + viewer (the E2E screenshot, task 03)
|
||||||
|
- [ ] the theme no-op (B4) is unaffected — the rule uses existing vars; no new `ui_settings` column
|
||||||
@@ -0,0 +1,24 @@
|
|||||||
|
# Task 03 — Unit pins, node behavior, dedicated E2E
|
||||||
|
|
||||||
|
**Phase:** `138_markdown_blockquotes` · **Source:** `TODO.md:3` — "Markdown formatted ">" quotes aren't rendering"
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Pin the renderer contract at the source level, prove the browser behavior, and ship the dedicated Playwright file, run in isolation (AGENTS.md rules 4/9).
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `tests/unit/test_markdown_blockquotes.py` (new) — the phase-44 pattern (source pins + a `HAVE_NODE`-skipped node behavior):
|
||||||
|
- Source pins: the blockquote pass runs after `inline(escapeHtml(text))` and before the paragraph split; the paragraph guard includes `blockquote`; the exact `<blockquote class="md-quote">` marker string; the `.md-quote` CSS rule references palette vars only (the task-02 pin).
|
||||||
|
- Node behavior: single-line quote → one blockquote; a 3-line group → one blockquote with `<br>` joins; `>> nested` collapses to one level; `> <script>alert(1)</script>` renders inert (no live tag in the output); a `>` inside a ``` fence stays fence content; a table cell containing `> x` renders as table-cell text, not a blockquote; non-quote input (a paragraph + list + table) renders byte-identical to the pre-phase renderer (fixture string checked into the test, per the phase-44 shape).
|
||||||
|
2. `tests/e2e/test_markdown_blockquotes.py` (new) — dedicated, run in isolation (`--no-cov`, DB up), mock-only, following `tests/e2e/test_markdown_tables.py`'s structure:
|
||||||
|
- (a) the chat: a mock flow whose answer contains `>` quotes (the phase-44 mock-flow pattern — a new user-message marker, a deterministic canned answer) → assert the DOM: the `.bubble` contains a `blockquote.md-quote` with the quote text and no literal `>` text at the block's start;
|
||||||
|
- (b) the viewer: a KB fixture with a `.md` document containing a quote block → open the document → assert the viewer content renders the `blockquote.md-quote`;
|
||||||
|
- (c) the restore: save the chat (or reload `/?chat=<id>`) → the restored bubble keeps the blockquote (the restore path runs the same renderer — a cheap assertion if the suite structure allows it).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: as above. E2E: as above. Coverage: **>90%** on `app/` (the gate runs the full suite; frontend changes add no `app/` code — the suite must stay green).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest tests/unit/test_markdown_blockquotes.py -v` green (node available → the behavior check runs)
|
||||||
|
- [ ] `uv run pytest tests/e2e/test_markdown_blockquotes.py -v --no-cov` green in isolation
|
||||||
|
- [ ] `uv run pytest` green, coverage TOTAL >90%, `uv run ruff check . && uv run pyright` clean
|
||||||
@@ -0,0 +1,45 @@
|
|||||||
|
# Phase 139 — The Reading tool line shows which lines are being read
|
||||||
|
|
||||||
|
**Source:** `TODO.md` L4 — "The \"Reading\" tool UI output doesn't show what lines are being read, just the same file over and over which is confusing to the user"
|
||||||
|
**Story:** n/a (TODO-derived phase, Protocol B append).
|
||||||
|
**Context:** `app/api/chat.py` — the SSE `tool` frame (`ChatToolEvent`) ALREADY carries `offset` / `limit` for a range `read` (phase 131 task 04 — the fields land on the frame only when well-formed via `_whole_line_number` (L307) and are omitted otherwise, so non-range frames are byte-identical); the frame is emitted at ~L928-952. `frontend/assets/app.js` — `appendToolLine(wrap, name, argument)` (L1119 — the read branch builds `📄 Reading ` + a `<code>{argument}</code>` child; the line bytes are pinned by the phase-87 emoji guard + the phase-117 disclosure pins), the live `tool` branch (L3103-3137 — reads only `ev.name` / `ev.argument`; `toolAcc.push({ name, argument })` L3116; the status label `is reading ${argument}` L3125), `toolAcc` (L2993 — the per-turn accumulator persisted as the saved chat's `tools` array at L3229 / L3310 / L3361 / L3395), the restore path (~L2030-2045 — `m.tools` → `appendToolLine(wrap, t.name, arg)` + the phase-95 truncation marker). `frontend/assets/shared.js` — `addToolLines(wrap, tools)` (~L175+ — the shared page's parity copy of `appendToolLine` with the pinned-identical line literals). `tests/integration/test_api.py` — the frontend emoji guard strips precisely the pinned emoji literals from `app.js` / `shared.js` (no new emoji may be introduced). `tests/e2e/test_read_line_ranges.py` + the phase-131 mock flow (`read the lines` marker → grep → `read(offset, limit)`) in `tests/e2e/mock_llm.py`.
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
When the agent range-reads a document (`read(path, offset, limit)` — phase 131), the Reading tool line shows WHICH lines — `📄 Reading <code>source/path</code> (lines 42–71)` — so repeated reads of the same file are distinguishable, live, on restore, and on the shared page; a plain read renders byte-identical to today.
|
||||||
|
|
||||||
|
## Dependencies
|
||||||
|
- `138_markdown_blockquotes` (todo) — pipeline order only; no code overlap (139 touches the tool-line rendering, 138 the markdown renderer).
|
||||||
|
- Code: the phase-131 wire fields (complete) — nothing new server-side.
|
||||||
|
|
||||||
|
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||||
|
- **The suffix (task 01):** one new helper in `app.js` — `readRangeSuffix(offset, limit) → string | ""` — the single source of truth for the wording (used by both the tool line and the status label): both present → ` (lines {offset}–{offset + limit - 1})` (EN-DASH; the pinned copy — ASSUMPTION A1: the range shown is the model's REQUEST; the UI does not know the document length, so a `limit` past EOF renders the requested extent); offset only → ` (from line {offset})`; limit only → ` (first {limit} lines)`; neither / malformed → `""`. The JS well-formedness guard mirrors the server's `_whole_line_number`: `Number.isInteger(x) && x >= 1`.
|
||||||
|
- **`appendToolLine` (task 01):** the signature becomes `appendToolLine(wrap, name, argument, offset = null, limit = null)` — the read arm (`read` / `read_document`) appends, AFTER the `<code>` child, a `<span class="read-range">` with `textContent = readRangeSuffix(offset, limit)` ONLY when the suffix is non-empty (a plain read's line stays byte-identical — the phase-87/117 pins + the emoji guard stay green; no new emoji). `shared.js`'s `addToolLines` applies the identical suffix to its pinned-identical line (the parity contract — the shared page re-renders saved records that now carry the fields).
|
||||||
|
- **The live branch (task 02):** read `ev.offset` / `ev.limit` with the same guard (absent / malformed → `null`); `toolAcc.push({ name, argument, offset, limit })` with the fields included ONLY when non-null (a non-range record keeps the exact pre-phase `{name, argument}` shape — the `bor.chat.v1` save JSONB is additive; old records without the fields restore byte-identically, no migration); pass both to `appendToolLine`; the status label becomes `${brand()} is reading ${argument}${suffix}` (same helper — ASSUMPTION A2).
|
||||||
|
- **The restore path (task 02):** `t.offset` / `t.limit` (same guard) → `appendToolLine(wrap, t.name, arg, tOffset, tLimit)`; the phase-95 truncation note is untouched (it keys on the `<code>` argument — verify with the existing pins).
|
||||||
|
- **E2E (task 03):** ONE new mock flow following the phase-131 pattern (user-message marker, deterministic replies, module-header docstring entry): the model range-reads the SAME document twice with different `offset`/`limit` — the owner's exact "same file over and over" scenario.
|
||||||
|
|
||||||
|
## Tasks
|
||||||
|
1. `01_tool_line_range_suffix.md` — `readRangeSuffix` + the `appendToolLine` / `addToolLines` suffix rendering.
|
||||||
|
2. `02_live_restore_persist.md` — the live branch's `ev.offset`/`ev.limit`, the `toolAcc` persistence, the restore path, the status label.
|
||||||
|
3. `03_unit_and_e2e.md` — unit pins + node behavior + the mock flow + the dedicated Playwright E2E.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: `tests/unit/test_read_range_tool_lines.py` (new — the `test_frontend_tool_states.py` pattern): source pins (the suffix template literals, the `Number.isInteger` guard, the `ev.offset`/`ev.limit` read, the `toolAcc` push carrying the fields, the restore pass-through, the shared.js parity) + node behavior (`appendToolLine` with a range vs without — the plain line byte-identical; the three suffix shapes).
|
||||||
|
- E2E: `tests/e2e/test_read_range_tool_lines.py` (new), in isolation.
|
||||||
|
- Coverage: **>90%** on `app/` (no `app/` code changes — the suite staying green IS the gate).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] a range read renders `📄 Reading <code>path</code> (lines N–M)` live; two range reads of the same file show DIFFERENT ranges (the owner's confusion is gone)
|
||||||
|
- [ ] a plain read's line is byte-identical to pre-phase (the phase-87/117 pins + the emoji guard pass unedited)
|
||||||
|
- [ ] the suffix survives save → restore (the chat page + the shared page render it from the saved record)
|
||||||
|
- [ ] `uv run pytest` green; coverage TOTAL >90%; `uv run pytest tests/e2e/test_read_range_tool_lines.py -v --no-cov` green in isolation; `uv run ruff check . && uv run pyright` clean
|
||||||
|
- [ ] one `--no-gpg-sign` Conventional Commit; phase dir moved to `.agents/phases/complete/`
|
||||||
|
|
||||||
|
## Locked decisions
|
||||||
|
- **A1 — the suffix shows the model's REQUESTED range (`offset`…`offset+limit−1`); the UI cannot know the actual slice extent (owner-confirmed 2026-09-30, roadmap confirmation).**
|
||||||
|
- **A2 — the status label carries the same suffix (`Brain is reading source/path (lines 42–71)`) (owner-confirmed 2026-09-30, roadmap confirmation).**
|
||||||
|
|
||||||
|
## Commit
|
||||||
|
```bash
|
||||||
|
git add frontend/ tests/ .agents/phases/ && git commit --no-gpg-sign -m "feat(ui): show the read tool's line range in the Reading line"
|
||||||
|
```
|
||||||
@@ -0,0 +1,21 @@
|
|||||||
|
# Task 01 — The range suffix on the Reading line (chat + shared page)
|
||||||
|
|
||||||
|
**Phase:** `139_read_range_tool_lines` · **Source:** `TODO.md:4` — "The \"Reading\" tool UI output doesn't show what lines are being read, just the same file over and over which is confusing to the user"
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
`appendToolLine` (and the shared page's `addToolLines`) render the requested line range as a suffix on the Reading line whenever a well-formed `offset`/`limit` is passed — createElement + textContent only, plain reads byte-identical.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `frontend/assets/app.js` — add a `readRangeSuffix(offset, limit)` helper next to `appendToolLine`: the well-formedness guard `Number.isInteger(x) && x >= 1` (mirrors the server's `_whole_line_number`); both present → ` (lines ${offset}–${offset + limit - 1})` (en-dash — the pinned copy); offset only → ` (from line ${offset})`; limit only → ` (first ${limit} lines)`; otherwise → `""`.
|
||||||
|
2. `frontend/assets/app.js` — `appendToolLine(wrap, name, argument, offset = null, limit = null)`: in the read arm (`read` / `read_document`), compute `readRangeSuffix(offset, limit)`; when non-empty, append a `<span class="read-range">` (textContent = the suffix) AFTER the `<code>` child. No other arm changes; no new emoji (the guard in `tests/integration/test_api.py` strips precisely the pinned literals).
|
||||||
|
3. `frontend/assets/shared.js` — `addToolLines`: the same suffix on the shared page's pinned-identical Reading line (the saved record carries the fields from task 02; a record without them renders exactly as today).
|
||||||
|
- ASSUMPTION: the span is a SIBLING of the `<code>` child — the phase-95 truncation note is the precedent (it appends a span sibling and never rewrites the line's pinned template text).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: covered by task 03's file (source pins + node behavior).
|
||||||
|
- Coverage: n/a (frontend).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] node repro: `appendToolLine` with `offset=42, limit=30` → the line's text is `📄 Reading <path> (lines 42–71)`; without the arguments → byte-identical to pre-task
|
||||||
|
- [ ] `uv run pytest` green (the phase-87/117 pins + the emoji guard unedited); `uv run ruff check .` clean
|
||||||
@@ -0,0 +1,22 @@
|
|||||||
|
# Task 02 — Wire the range through live frames, persistence, and restore
|
||||||
|
|
||||||
|
**Phase:** `139_read_range_tool_lines` · **Source:** `TODO.md:4` — "The \"Reading\" tool UI output doesn't show what lines are being read, just the same file over and over which is confusing to the user"
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
The `offset`/`limit` already on the SSE `tool` frame (phase 131) reach the Reading line live, persist in the saved chat's `tools` record (additively), and re-render on restore — chat page + shared page.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `frontend/assets/app.js` — the live `tool` branch (~L3103): read `ev.offset` / `ev.limit` (the `Number.isInteger(x) && x >= 1` guard → `null` when absent / malformed); `toolAcc.push({ name, argument, offset, limit })` with the fields included ONLY when non-null (a non-range record keeps the exact pre-phase `{name, argument}` shape — the `bor.chat.v1` save JSONB is additive; old rows restore unchanged, no migration); pass both values to `appendToolLine`.
|
||||||
|
2. `frontend/assets/app.js` — the status label (~L3125): `${brand()} is reading ${argument}${readRangeSuffix(offset, limit)}` (the grep / ls labels are untouched).
|
||||||
|
3. `frontend/assets/app.js` — the restore path (~L2030): pass `t.offset` / `t.limit` (same guard) to `appendToolLine`; the phase-95 `appendTruncatedNote` call is untouched (it keys on the `<code>` argument).
|
||||||
|
4. `frontend/assets/app.js` — the save-payload sites (L3229 / L3310 / L3361 / L3395) need no code change (they serialize `toolAcc` as-is) — verify the phase-83 save-payload-limit tests are unaffected by the two extra fields.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: task 03's file (source pins for the `ev.offset`/`ev.limit` read, the `toolAcc` push shape, the restore pass-through).
|
||||||
|
- Coverage: n/a (frontend).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] a live range read: the line shows the suffix AND the status label shows it AND `toolAcc` carries `{name, argument, offset, limit}`
|
||||||
|
- [ ] a saved chat restored (the `/?chat=<id>` path) + the shared page re-render the suffix; a pre-phase saved record (no fields) renders byte-identical
|
||||||
|
- [ ] `uv run pytest` green (the chat-persistence / save-payload suites unedited)
|
||||||
@@ -0,0 +1,23 @@
|
|||||||
|
# Task 03 — Unit pins, node behavior, mock flow, dedicated E2E
|
||||||
|
|
||||||
|
**Phase:** `139_read_range_tool_lines` · **Source:** `TODO.md:4` — "The \"Reading\" tool UI output doesn't show what lines are being read, just the same file over and over which is confusing to the user"
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Pin the suffix contract at the source level, add the owner's "same file over and over" scenario to the deterministic mock, and prove it end-to-end in a dedicated Playwright file, run in isolation.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `tests/unit/test_read_range_tool_lines.py` (new) — the `test_frontend_tool_states.py` pattern: source pins (the ` (lines ${offset}–${offset + limit - 1})` / ` (from line …)` / ` (first … lines)` literals; the `Number.isInteger` guard; the live branch's `ev.offset` / `ev.limit` read; the `toolAcc` push carrying the fields; the restore pass-through; the shared.js identical literals) + node behavior (the three suffix shapes; the plain-read line byte-identical to pre-phase; the shared-page parity).
|
||||||
|
2. `tests/e2e/mock_llm.py` — ONE new scripted flow (house style — a user-message marker, deterministic replies, a module-header docstring entry): the marker `read the same file twice` → round 1: a `read` tool call `(path, offset=10, limit=20)`; round 2: a `read` tool call `(path, offset=40, limit=15)`; round 3: the answer echoing both results (the phase-131 read-echo shape).
|
||||||
|
3. `tests/e2e/test_read_range_tool_lines.py` (new) — dedicated, run in isolation (`--no-cov`, DB up), mock-only; a KB fixture with ONE multi-hundred-line document:
|
||||||
|
- (a) the marker question → assert TWO Reading lines for the SAME `<code>` path carrying the distinct suffixes ` (lines 10–29)` and ` (lines 40–54)`;
|
||||||
|
- (b) a plain-read flow (an existing mock flow with a `read` call and no range — e.g. the phase-131 plain-read case or the phase-37 flow) → assert the Reading line has NO `.read-range` span (byte-identical shape);
|
||||||
|
- (c) save the chat → restore (reload `/?chat=<id>`) → assert the suffixes re-render from the saved record.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: as above. E2E: as above. Coverage: **>90%** on `app/` (the gate; no `app/` changes).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest tests/unit/test_read_range_tool_lines.py -v` green
|
||||||
|
- [ ] `uv run pytest tests/e2e/test_read_range_tool_lines.py -v --no-cov` green in isolation
|
||||||
|
- [ ] `uv run pytest` green, coverage TOTAL >90%, `uv run ruff check . && uv run pyright` clean
|
||||||
@@ -0,0 +1,58 @@
|
|||||||
|
# Phase 140 — Pictorial PDFs: sparse pages are rendered and vision-described into the indexed content
|
||||||
|
|
||||||
|
**Source:** `TODO.md` L5 — "I've added a new \"ikea_instructions.tar\" to test_sources. This is an example pdf that stil isn't working. There isn't much text, it's mostly graphical, but it doesn't have pictures. Take a screenshot of the first page or pages 8 onward, it's mostly pictographic illustrations of how to assemble a piece of furniture. We need to support this kind of pdf, and in this case showing rendered pages of the pdf is more helpful than the extracted text. We need a way to determine when that's neccesary. My suggestion is to screenshot the pdf, generate a text summary, compare it to the text extraction, and if the screenshot contains more information (you can use an LLM to judge) use the screenshot or rendered page rather in addition to the extracted text. More information is better when generating the text embeddings since that increases the chance the user's search will hit something."
|
||||||
|
**Story:** n/a (TODO-derived phase, Protocol B append).
|
||||||
|
**Context:** the owner's fixture `test_sources/ikea_instructions.tar` (gitignored, on disk — the phase-133 convention: tests extract to `tmp_path`, never the repo): `besta-frame-white__AA-1272068-11-100.pdf`, 24 pages, metadata title "Assembly Instructions" (Adobe InDesign) — pages 2–5 are text-heavy (multilingual warnings, 3,091–7,288 chars), the other 20 pages are sparse (4–73 chars, `Illustration N` markers) and carry **vector-drawn** pictograms (PyMuPDF `get_image_info()` → 0 embedded raster images — the phase-122/137 embedded-image pipeline sees nothing). Today the file passes the phase-133 garbage gate (`is_usable_pdf_text` — the text is usable, just thin) and is indexed as ~24 KB of page markers → a question about the assembly steps never hits. `app/rag/pdf.py` — `PAGE_SEPARATOR = "---PAGE-{page}---"` (L57), `extract_pdf_text` (L153), `extract_pdf_page_texts` (L178 — the phase-137 `dict[int, str]` helper), `is_usable_pdf_text` (L225 — the phase-133 gate), `render_pdf_pages` (L267 — all pages at 150 dpi, the zero-byte unreadable-page placeholder), `extract_pdf_images` (L300). `app/rag/importer.py` — `_index_pdf_file` (L1140 — extract → the phase-133 gate + `_fallback_page_screenshots` → digest over the FINAL content → upsert + the unchanged early-return → chunk + embed → `lite` summary → embedded images), `ImportSummary` (L212 — stats counters + the `log()` line; stats are LOG-ONLY, the sync `detail` API shape never changes). `app/rag/summarizer.py` — the marker convention (L36-95: `IMAGE_DESCRIPTION_MODE` / `DESCRIBE_PROMPT`), `describe_image` (L208 — ONE chat-model vision call, the multimodal user message, the `context` param, fail-soft `str | None`, no app-level retry). `app/config.py` — the toggle pattern (`images: bool = False` L379 — `BOR_IMAGES`; the house kill-switch: `0` disables, a negative value fails startup loudly). `tests/e2e/mock_llm.py` — the `IMAGE_DESCRIPTION_MODE` branch (L2685) + the canned reply + the phase-137 trigger pattern (a new marker branch is additive — the existing suites stay green unedited). `tests/e2e/test_pdf_support.py` — the synthetic-PDF fixture recipe (PyMuPDF build) + the module-app pattern.
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
A PDF whose pages are mostly pictorial (the IKEA assembly-instruction class) indexes with high-quality content: every sparse page (deterministic gate: stripped text below a threshold) is rendered to a screenshot and vision-described, and the description is appended to that page's content block — so the embeddings carry what the text layer couldn't and search hits the illustrations (the owner's L5 rationale: "More information is better when generating the text embeddings"). NO LLM judge (owner decision 2026-09-30) and NO page cap (PLAN A21 — the LLM calls are free; a slower sync is acceptable, a lower-quality KB is not).
|
||||||
|
|
||||||
|
## Dependencies
|
||||||
|
- `139_read_range_tool_lines` (todo) — pipeline order only; no code overlap (140 is backend import + the mock; 139 is the frontend tool line).
|
||||||
|
- Code (all complete): phase 126 (the PDF pipeline), 133 (the garbage gate + `render_pdf_pages`), 137 (`extract_pdf_page_texts` + the vision seam).
|
||||||
|
|
||||||
|
## Design (shared by all tasks — the executor reads this, not the chat)
|
||||||
|
- **The gate (task 01):** `app/rag/pdf.py` — `pictorial_pages(page_texts: dict[int, str], *, min_chars: int) -> list[int]`: the 1-based page numbers whose `len(text.strip()) < min_chars`, ascending — pure, deterministic, NO LLM call (ASSUMPTION A1). IKEA page 1 (`BESTÅ`, 5 chars) IS a candidate — a title page; describing it is useful (the logo + product name) and never harmful.
|
||||||
|
- **The settings (task 01):** `app/config.py` — `pdf_pictorial: bool = True` (`BOR_PDF_PICTORIAL`, the house kill-switch — `0`/`false` disables the pass entirely → content byte-identical to pre-phase; the `BOR_IMAGES` naming pattern) + `pdf_pictorial_min_chars: int = 100` (`BOR_PDF_PICTORIAL_MIN_CHARS`; validator: ≥ 0 — 0 = every page is a candidate (A21), negative fails startup loudly, the house pattern).
|
||||||
|
- **The vision seam (task 02):** `app/rag/summarizer.py` — `PICTORIAL_PAGE_MODE = "PICTORIAL_PAGE_MODE"` + `PICTORIAL_INSTRUCTION` (locked wording: describe in detail what the page shows — the assembly steps, part names, arrows, tool icons, before/after states — ESPECIALLY the information the extracted text does not contain) + `PICTORIAL_PROMPT = f"{PICTORIAL_PAGE_MODE}: {PICTORIAL_INSTRUCTION}"`; `describe_pdf_page(llm, *, data, mime, page, page_text) -> str | None` — the same one-chat-call multimodal contract as `describe_image` (no system prompt, no tools, no app-level retry, fail-soft `None`); the message text part = `PICTORIAL_PROMPT` + `\n\nPage {page} of the PDF. Text extracted from this page:\n{page_text.strip() or "(none)"}`. `DESCRIBE_PROMPT` / `IMAGE_DESCRIPTION_MODE` stay BYTE-IDENTICAL (the phase-122/126/137 suites pass unedited).
|
||||||
|
- **The render selector (task 02):** `app/rag/pdf.py` — `render_pdf_pages(pdf_path, *, dpi=150, pages: Sequence[int] | None = None)`: `pages` (1-based) restricts rendering to the named pages in ascending order (a 500-page PDF with 30 sparse pages renders 30 PNGs, not 500); `None` = all pages — the phase-133 caller stays byte-identical.
|
||||||
|
- **The import pass (task 03):** `_index_pdf_file` — AFTER the phase-133 gate (the precedence is locked: an unusable text layer takes the existing full-page screenshot fallback — NO pictorial pass, no double vision), when `settings.pdf_pictorial` and the content is usable:
|
||||||
|
1. `page_texts = extract_pdf_page_texts(full_path)`; `candidates = pictorial_pages(page_texts, min_chars=settings.pdf_pictorial_min_chars)` — **ALL candidates, no cap (A21)**; zero candidates → skip entirely (no render, no vision call — a clean-text PDF's path is byte-identical to pre-phase).
|
||||||
|
2. `render_pdf_pages(full_path, pages=candidates)` — a zero-byte placeholder page (unreadable) is skipped with a warning (the phase-133 handling).
|
||||||
|
3. each candidate in order: `describe_pdf_page(...)` fail-soft (`None` → skip that page, a warning log, continue — one dead page never kills the pass).
|
||||||
|
4. content assembly: every page 1..N as its `PAGE_SEPARATOR` + the page text + (when the page has a description: `\n\n[Page {n} illustration: {description}]`). The assembly with NO descriptions is byte-identical to `extract_pdf_text`'s output (a unit pin).
|
||||||
|
5. the digest over the FINAL content (the phase-126/133 convention — an unchanged file + a stable vision reply re-syncs as `unchanged`; a non-deterministic re-describe is a cost, not a bug — the phase-133 docstring documents it, A21 endorses it).
|
||||||
|
6. counters (stats log-only): `ImportSummary.pdfs_pictorial: int = 0` (a PDF with ≥1 described page) + `pdf_pictorial_pages: int = 0` (pages described) — `log()` fields only; the sync `detail` API shape is UNTOUCHED; ONE loud log line (PLAN §9): `pdf: pictorial pass source=<s> path=<p> — N candidate pages, M described (min_chars=<threshold>)`.
|
||||||
|
- **The viewer:** the description renders as plain text in the document viewer (A4 — PDFs are served as extracted text; the `[Page N illustration: …]` line IS content). No viewer change.
|
||||||
|
|
||||||
|
## Tasks
|
||||||
|
1. `01_settings_and_gate.md` — `BOR_PDF_PICTORIAL` + `BOR_PDF_PICTORIAL_MIN_CHARS` + `pictorial_pages` + unit tests.
|
||||||
|
2. `02_pictorial_vision_seam.md` — `PICTORIAL_PAGE_MODE` + `describe_pdf_page` + the `render_pdf_pages` page selector + the mock branch + unit tests.
|
||||||
|
3. `03_importer_pictorial_pass.md` — the `_index_pdf_file` pass + counters/log + the real IKEA-fixture integration repro + the unit/integration matrix.
|
||||||
|
4. `04_e2e_pictorial_pdf.md` — the dedicated Playwright suite, run in isolation, mock-only.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: task 01 — `pictorial_pages` (threshold boundaries `<`/`=`/`>`, a whitespace-only page → a candidate, an empty dict → `[]`, ascending order); task 02 — `describe_pdf_page`'s message shape (a capturing fake LLM: `PICTORIAL_PROMPT` first, the page block after, the data URL; `None` on an empty reply / client error; the `DESCRIBE_PROMPT` byte-identity), the `render_pdf_pages` selector (subset order + count, `None` = all); task 03 — the assembly byte-identity + the branch matrix.
|
||||||
|
- Integration: task 03 — the real IKEA tar through the REAL importer (the canned vision fake, capturing style): every candidate page (computed IN the test from `extract_pdf_page_texts` + `pictorial_pages` — no hardcoded page list) carries a `[Page N illustration: …]` block, the text-heavy pages (2–5) do NOT, the chunks carry the description, the title is "Assembly Instructions"; a re-sync with the same fake → `unchanged` (digest stable, zero added vision calls); a `BOR_PDF_PICTORIAL=0` module app → the content is byte-identical to `extract_pdf_text`.
|
||||||
|
- E2E: `tests/e2e/test_pictorial_pdf.py` (new, task 04) — in isolation, mock-only.
|
||||||
|
- Coverage: **>90%** on `app/`.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] the owner's `ikea_instructions.tar` imports with described illustration blocks on every sparse page (the integration repro green — the owner's exact file)
|
||||||
|
- [ ] a chat question about the assembly content is grounded on the described pages (the E2E)
|
||||||
|
- [ ] a clean-text PDF never burns a vision call (the zero-candidates no-op is pinned) and a garbage-text PDF still takes the phase-133 fallback unchanged (the precedence pin)
|
||||||
|
- [ ] `BOR_PDF_PICTORIAL=0` → the content is byte-identical to pre-phase
|
||||||
|
- [ ] `uv run pytest --cov=app --cov-report=term-missing` green, TOTAL >90%; `uv run pytest tests/e2e/test_pictorial_pdf.py -v --no-cov` green in isolation; the pre-existing `test_pdf_support.py` / `test_garbage_pdf.py` / `test_pdf_image_context.py` suites green unedited
|
||||||
|
- [ ] `uv run ruff check . && uv run pyright` clean
|
||||||
|
- [ ] one atomic Conventional Commits commit, `--no-gpg-sign`; phase dir moved to `.agents/phases/complete/`
|
||||||
|
|
||||||
|
## Locked decisions
|
||||||
|
- **A1 — sparse-page detection is the deterministic per-page character threshold (default 100, `BOR_PDF_PICTORIAL_MIN_CHARS`) — no LLM call in the gate (owner-confirmed 2026-09-30, roadmap confirmation).**
|
||||||
|
- **A2 — NO LLM judge: every described sparse page's description appends to the content (owner-selected 2026-09-30).**
|
||||||
|
- **A3 — no page cap, no call budget (PLAN A21); `BOR_PDF_PICTORIAL` kill-switch, default ON (owner-confirmed 2026-09-30).**
|
||||||
|
- **A4 — the description appends to the page block as `[Page N illustration: …]`; the digest is over the final content (the phase-126/133 convention) (owner-confirmed 2026-09-30).**
|
||||||
|
|
||||||
|
## Commit
|
||||||
|
```bash
|
||||||
|
git add app/ tests/ .agents/phases/ && git commit --no-gpg-sign -m "feat(import): pictorial PDF support — sparse pages rendered + vision-described into the indexed content"
|
||||||
|
```
|
||||||
@@ -0,0 +1,20 @@
|
|||||||
|
# Task 01 — The kill-switch, the threshold setting, and the sparse-page gate
|
||||||
|
|
||||||
|
**Phase:** `140_pictorial_pdf_pages` · **Source:** `TODO.md:5` — "…We need a way to determine when that's neccesary. …More information is better when generating the text embeddings since that increases the chance the user's search will hit something."
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
Two settings (the feature toggle + the sparseness threshold) and the pure, deterministic function that selects a PDF's pictorial pages — the TODO's "a way to determine when that's necessary", with no LLM call.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `app/config.py` — `pdf_pictorial: bool = True` (env `BOR_PDF_PICTORIAL` — the house kill-switch: `0`/`false` disables the pictorial pass entirely; the `images` toggle's naming/docstring pattern at L379) + `pdf_pictorial_min_chars: int = 100` (env `BOR_PDF_PICTORIAL_MIN_CHARS`; a `field_validator`: ≥ 0 — 0 = every page is a candidate (PLAN A21), negative raises with the named field, the house fail-loud pattern). Document both in the `Settings` docstring convention and in `.env.example` (the comment-block pattern of the neighboring settings).
|
||||||
|
2. `app/rag/pdf.py` — `def pictorial_pages(page_texts: dict[int, str], *, min_chars: int) -> list[int]`: the 1-based page numbers whose `len(text.strip()) < min_chars`, ascending — pure; the docstring carries the contract + the IKEA example (`{"1": "BESTÅ", "2": <7 KB of warnings>, "6": "Illustration 6"}` at `min_chars=100` → `[1, 6, …]`).
|
||||||
|
- ASSUMPTION: the threshold is STRICTLY less-than — a page with exactly `min_chars` stripped chars is NOT sparse (the boundary is pinned in the tests).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: `tests/unit/test_pictorial_gate.py` (new) — the validators (0 ok, 100 ok, −1 raises with the named field, the defaults — toggle on, 100) and `pictorial_pages` (the `<`/`=`/`>` boundary, a whitespace-only page → a candidate, an empty dict → `[]`, ascending order, a large dict).
|
||||||
|
- Coverage: **>90%** on the new/modified code.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest tests/unit/test_pictorial_gate.py -v` green
|
||||||
|
- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean
|
||||||
@@ -0,0 +1,22 @@
|
|||||||
|
# Task 02 — The `PICTORIAL_PAGE_MODE` vision seam + the render selector
|
||||||
|
|
||||||
|
**Phase:** `140_pictorial_pdf_pages` · **Source:** `TODO.md:5` — "…Take a screenshot of the first page or pages 8 onward… My suggestion is to screenshot the pdf, generate a text summary… use the screenshot or rendered page rather in addition to the extracted text."
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
A dedicated, deterministic, fail-soft vision call for one rendered PDF page (the page's own text as context, so the model describes what the text does NOT) — plus the ability to render only the candidate pages of a large PDF.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `app/rag/summarizer.py` — `PICTORIAL_PAGE_MODE = "PICTORIAL_PAGE_MODE"` + `PICTORIAL_INSTRUCTION` (the locked wording from the phase overview's design) + `PICTORIAL_PROMPT = f"{PICTORIAL_PAGE_MODE}: {PICTORIAL_INSTRUCTION}"` — the marker convention next to `IMAGE_DESCRIPTION_MODE` (L75-95).
|
||||||
|
2. `app/rag/summarizer.py` — `async def describe_pdf_page(llm, *, data: bytes, mime: str, page: int, page_text: str | None, settings: Settings | None = None) -> str | None` — the same one-chat-call multimodal contract as `describe_image` (L208): the user message `[{type: "text", text: PICTORIAL_PROMPT + "\n\nPage {page} of the PDF. Text extracted from this page:\n" + (page_text.strip() or "(none)")}, {type: "image_url", image_url: {url: <data URL from data + mime>}}]` — no system prompt, no tools, no app-level retry; an empty reply → `None` (the house one-shot empty-content policy); a client failure → `None` (fail-soft — one dead page never stalls a sync).
|
||||||
|
3. `app/rag/pdf.py` — `render_pdf_pages(pdf_path, *, dpi: int = 150, pages: Sequence[int] | None = None)`: the optional 1-based selector — render only the named pages, ascending, one PNG each (an unreadable named page → the zero-byte placeholder at its position, as today); `None` = all pages — the phase-133 caller stays byte-identical (its existing tests pin it).
|
||||||
|
4. `tests/e2e/mock_llm.py` — ONE new deterministic branch (house style + a module-header docstring entry): a chat request whose text part contains `PICTORIAL_PAGE_MODE` → the fixed canned reply `PICTORIAL_PAGE_ANSWER` (a distinctive, retrieval-friendly string, e.g. "Pictorial page: the illustration shows two parts being joined with an arrow.") — additive; the `IMAGE_DESCRIPTION_MODE` branch and every existing flow are untouched.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: `tests/unit/test_pictorial_seam.py` (new) — a capturing fake LLM: the message shape (the text part starts with `PICTORIAL_PROMPT`, then the page block; the `"(none)"` case; the image part's data URL from the bytes + mime), `None` on an empty reply and on a client exception, the `DESCRIBE_PROMPT` byte-identity (regression pin — the string is untouched); `render_pdf_pages`' selector (the subset's count + order + PNG magic bytes, `None` = all — a ≥2-page fixture PDF, the phase-126/133 recipe).
|
||||||
|
- Coverage: **>90%** on the new/modified code.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest tests/unit/test_pictorial_seam.py -v` green
|
||||||
|
- [ ] `uv run pytest` green (the pre-existing vision suites — `test_image_documents.py` et al. — pass unedited)
|
||||||
|
- [ ] `uv run ruff check . && uv run pyright` clean
|
||||||
@@ -0,0 +1,30 @@
|
|||||||
|
# Task 03 — The `_index_pdf_file` pictorial pass + the IKEA repro
|
||||||
|
|
||||||
|
**Phase:** `140_pictorial_pdf_pages` · **Source:** `TODO.md:5` — "I've added a new \"ikea_instructions.tar\" to test_sources. This is an example pdf that stil isn't working… We need to support this kind of pdf, and in this case showing rendered pages of the pdf is more helpful than the extracted text… use the screenshot or rendered page rather in addition to the extracted text."
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
The import pass itself: after the phase-133 garbage gate, a usable-text PDF's sparse pages are rendered + vision-described + appended to the page content — and the owner's exact IKEA file is pinned as an integration regression.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `app/rag/importer.py` — `_index_pdf_file` (L1140): after the existing phase-133 gate block (the precedence is locked — an unusable layer takes `_fallback_page_screenshots` and the pass NEVER runs), when `settings.pdf_pictorial`: `page_texts = extract_pdf_page_texts(full_path)`; `candidates = pictorial_pages(page_texts, min_chars=settings.pdf_pictorial_min_chars)`; zero candidates → skip entirely (no render, no vision — a clean-text PDF is byte-identical to pre-phase); otherwise render (`render_pdf_pages(full_path, pages=candidates)`, the zero-byte placeholder page skipped with a warning), describe each candidate (`describe_pdf_page`, fail-soft: `None` → a warning + skip the page), and assemble the content: every page = its `PAGE_SEPARATOR` + the page text + `\n\n[Page {n} illustration: {description}]` for the described pages — the assembly with NO descriptions is byte-identical to `extract_pdf_text`'s output (a pin).
|
||||||
|
2. `app/rag/importer.py` — the digest over the FINAL content is the existing code (the pass runs before it, exactly as the phase-133 fallback does); the unchanged early-return, the chunk + embed, and the `lite` summary pipeline are untouched downstream.
|
||||||
|
3. `app/rag/importer.py` — `ImportSummary` (L212): `pdfs_pictorial: int = 0` + `pdf_pictorial_pages: int = 0` — `log()` fields only (the stats log-only convention; the sync `detail` API shape is untouched) + ONE loud log line (PLAN §9): `pdf: pictorial pass source=<s> path=<p> — N candidate pages, M described (min_chars=<threshold>)`.
|
||||||
|
4. `tests/integration/test_importer.py` (extend) — the IKEA repro (the phase-133 real-fixture convention: `test_sources/` is gitignored — extract the tar to `tmp_path`, never the repo; if the tar is absent the test fails loudly with a named message, the owner's machine always has it): run the REAL `import_sources` over the extracted PDF with a fake multimodal LLM (the canned per-page reply; a capturing fake that records every `PICTORIAL_PAGE_MODE` call):
|
||||||
|
- the candidates are computed IN the test from `extract_pdf_page_texts` + `pictorial_pages` (no hardcoded page list — the fixture is stable but the test must not rot);
|
||||||
|
- every candidate page's content block carries `[Page N illustration: <canned>]`; the text-heavy pages (2–5) carry NO block;
|
||||||
|
- the document's chunks carry the canned description (assert the string in the joined chunk text);
|
||||||
|
- the title is "Assembly Instructions" (the PDF metadata);
|
||||||
|
- a second `import_sources` run with the same fake → the file counts `unchanged` (digest stable, ZERO added vision calls);
|
||||||
|
- a `BOR_PDF_PICTORIAL=0` module app → the stored content == `extract_pdf_text`'s output (byte-identical);
|
||||||
|
- a garbage-text PDF (the phase-133 synthetic recipe) with `pdf_pictorial` ON → the phase-133 fallback fires (the content is the full-page descriptions), the pictorial pass does NOT run (the fake records zero `PICTORIAL_PAGE_MODE` calls) — the precedence pin.
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- Unit: `tests/unit/test_pictorial_import.py` (new) — the assembly byte-identity (no descriptions) + the branch matrix (feature off, zero candidates, a fail-soft `None` page, the unreadable-page placeholder skip) with a capturing fake LLM + the synthetic-PDF recipe (the `test_pdf_support.py` pattern).
|
||||||
|
- Integration: the step-4 matrix above.
|
||||||
|
- Coverage: **>90%** on the new/modified code.
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] the IKEA tar imports with a description block on every sparse page + `unchanged` on the re-sync (the repro green)
|
||||||
|
- [ ] `uv run pytest tests/unit/test_pictorial_import.py -v` green
|
||||||
|
- [ ] `uv run pytest` green, coverage >90%; `uv run ruff check . && uv run pyright` clean
|
||||||
@@ -0,0 +1,23 @@
|
|||||||
|
# Task 04 — The dedicated E2E (synthetic pictorial PDF, mock-only, in isolation)
|
||||||
|
|
||||||
|
**Phase:** `140_pictorial_pdf_pages` · **Source:** `TODO.md:5` — "…in this case showing rendered pages of the pdf is more helpful than the extracted text… More information is better when generating the text embeddings since that increases the chance the user's search will hit something."
|
||||||
|
**Story:** n/a
|
||||||
|
|
||||||
|
## Objective
|
||||||
|
One dedicated Playwright file, run in isolation (AGENTS.md rules 4/9), proves the owner's scenario end to end: upload a pictorial PDF → sync → the indexed content carries the described page → a chat question grounded on the illustration is answered.
|
||||||
|
|
||||||
|
## Work
|
||||||
|
1. `tests/e2e/test_pictorial_pdf.py` (new) — the `test_pdf_support.py` structure (synthetic-PDF module app, the admin upload flow): build a 2-page PDF with PyMuPDF (the recipe): page 1 = a 500+ char text paragraph; page 2 = an `Illustration 2` line + a drawn vector figure (a rectangle + an arrow — the sparse page); tar it → `POST /api/git-sources/upload` (202) → the Sync button → the document appears on the Sources page:
|
||||||
|
- (a) the document viewer's content shows the page-1 text WITHOUT an illustration block and page 2 WITH `[Page 2 illustration: <the mock's canned PICITORIAL_PAGE_ANSWER>]` (the task-02 `PICTORIAL_PAGE_MODE` branch — deterministic);
|
||||||
|
- (b) a chat question that ONLY the illustration answers (the canned reply names a distinctive phrase — the question: "How are the two parts joined on page two?") → the SSE completes without deflecting (the described page's embedding retrieves — the mock's deterministic token-overlap embeddings make the phrase → description hit) → the `done` event's `sources` includes the PDF and the answer echoes the phrase;
|
||||||
|
- (c) the sync response's `detail` JSON keeps its pre-phase keys (no new field — the stats log-only convention) — a cheap key-set assertion.
|
||||||
|
2. No other file changes (the mock branch shipped in task 02).
|
||||||
|
|
||||||
|
## Testing & Quality
|
||||||
|
- E2E: this file, in isolation: `uv run pytest tests/e2e/test_pictorial_pdf.py -v --no-cov` (DB up: `podman compose up -d db`).
|
||||||
|
- Coverage: **>90%** on `app/` (the phase gate runs the full suite in the final pass).
|
||||||
|
|
||||||
|
## Completion Criteria
|
||||||
|
- [ ] `uv run pytest tests/e2e/test_pictorial_pdf.py -v --no-cov` green in isolation
|
||||||
|
- [ ] the pre-existing `test_pdf_support.py` / `test_garbage_pdf.py` / `test_pdf_image_context.py` suites green unedited
|
||||||
|
- [ ] full gate: `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90%; `uv run ruff check . && uv run pyright` clean
|
||||||
Reference in New Issue
Block a user