diff options
Diffstat (limited to 'notes')
| -rw-r--r-- | notes/archived-workspace-create-warning-plan.md | 85 | ||||
| -rw-r--r-- | notes/assumptions-log.md | 34 | ||||
| -rw-r--r-- | notes/instant-stop-2-plan.md | 86 | ||||
| -rw-r--r-- | notes/pinned-tiles-plan.md | 129 | ||||
| -rw-r--r-- | notes/search-backend-selection-plan.md | 112 | ||||
| -rw-r--r-- | notes/text-file-embeds-implementation.md | 294 | ||||
| -rw-r--r-- | notes/text-file-embeds-plan.md | 93 | ||||
| -rw-r--r-- | notes/tool-call-display-plan.md | 254 |
8 files changed, 1086 insertions, 1 deletions
diff --git a/notes/archived-workspace-create-warning-plan.md b/notes/archived-workspace-create-warning-plan.md new file mode 100644 index 0000000..537473b --- /dev/null +++ b/notes/archived-workspace-create-warning-plan.md @@ -0,0 +1,85 @@ +# Plan — Warn when creating a workspace whose slug belongs to an archived workspace + +Status: requested by user, ready to implement +Branch: `feature/archived-workspace-create-warning` +Scope: **frontend only**. No backend change (the backend already exposes everything needed). + +--- + +## 1. User requirement + +> "I want the frontend to warn me if I try to create a workspace that already +> exists and is archived (it should say you can't)." + +So: when the user submits the **New workspace** form with a slug that matches an +existing **archived** workspace, the frontend must **not** create/ensure it and +must show a message saying it **can't** be created because a workspace with that +name already exists and is archived. + +## 2. Current behavior (why it's wrong today) + +- `HomeHeader.svelte` (`src/features/workspaces/ui/HomeHeader.svelte`) — the + New-workspace popover. `submit()` validates the slug with `isValidSlug`, then + calls `onCreate(slug)` and shows `result.error` in the existing `slugError` + slot when `result.ok === false`. +- `WorkspacesHome.svelte` → `createWorkspace(slug)` calls `store.ensure(slug)`, + then `store.refresh()`, `tiles.placeFirstEmpty(slug, …)`, and enters rename + mode (`renamingId = slug`). +- `store.ensure` → `http.ensure` → backend `PUT /workspaces/:id`, which is + **idempotent**: for an existing archived workspace it just returns it + (`archived: true`) — no error. The active list (`GET /workspaces`, archived + excluded by default) doesn't contain it, yet the home grid places a tile for + it and enters rename mode. Silent and confusing. + +Backend facts (no change needed): +- `GET /workspaces/:id` → the workspace incl. `archived: boolean`, or 404. + Frontend adapter already has `http.get(id)` (`src/features/workspaces/adapter/http.ts`) + — note it currently returns `null` for **both** 404 and network/other errors. +- `GET /workspaces?archived=true` → archived-only list (the store's + `refreshArchived()` / `archivedList`, used by the Archived view). + +## 3. Required behavior + +1. On submit, **before** calling `ensure`, determine whether a workspace with + that slug exists and is archived. Use an authoritative check against the + backend (e.g. `GET /workspaces/:id`) — do **not** rely on `archivedList` + alone, since it's only loaded when the Archived view is opened. +2. If it exists and is archived: + - do **not** call `ensure`, do not refresh/place a tile, do not enter rename; + - keep the popover open and show the error in the existing `slugError` slot, + clearly saying it can't be created because an archived workspace with that + name already exists. Suggested wording: + `A workspace named "<slug>" already exists and is archived — you can't create it.` + (It's fine to append a short hint that it can be restored from Archived + workspaces, but don't add new UI controls for that.) +3. If it doesn't exist, or exists and is **not** archived → today's behavior, + unchanged. +4. If the existence check itself fails (network/server error), don't silently + swallow it in a way that creates an archived workspace's tile: either surface + an error, or proceed exactly as today — pick one, keep it simple, and justify + it in the report. (Distinguishing 404 from "request failed" may require a + small adapter/store change; that's in scope.) +5. While the check is in flight, the existing `submitting` state should cover it + (no double submits). + +Follow the frontend's architecture rules in `AGENTS.md` (where decision logic +lives, store/adapter boundaries, Svelte 5 runes conventions, styling). + +## 4. Out of scope +- Creating a slug that matches an existing **active** workspace (unchanged). +- Any backend change (e.g. making `PUT /workspaces/:id` reject archived ids). +- New UI for unarchiving from the popover. + +## 5. Tests +- Unit/store test: creating an archived slug returns `ok: false` with the + "can't create" message and does **not** call `ensure`. +- Non-archived existing slug and brand-new slug → `ensure` is called as before. +- Check-failure path per your choice in 3.4. +- Component test (`HomeHeader.test.ts` or `WorkspacesHome` level, whichever + matches existing patterns): the message renders in the popover and the + popover stays open. + +## 6. Verification gate +`bun install`, `bun run typecheck`, `bun run test`, `bun run check`, +`bun run build` — all green. Commit to +`feature/archived-workspace-create-warning` only. Do not merge or push. diff --git a/notes/assumptions-log.md b/notes/assumptions-log.md index 026cde2..3291e28 100644 --- a/notes/assumptions-log.md +++ b/notes/assumptions-log.md @@ -2,6 +2,38 @@ > Recorded while working autonomously (user away). Raise these when the user returns. +## 2026-10-24 — Close-tab cascade completion + +- Retained the original visible-group + snapshot semantics below, including one close + request per id; no backend lineage cascade and no close-all retry workaround. +- User explicitly authorized replacing the companion backend symlink with an isolated + feature worktree to fix the reproducible active-turn Close → idle finalization race. + Backend commit `fd07259` fixes this and adds regression coverage. Verification writes + for the fixed backend use fresh private runtime paths, never the user's instance. +- Native modal lifecycle (top layer/focus containment/safe initial focus) replaces the + original CSS-only overlay; stale target dialogs dismiss on workspace/remote removal. +- Final verification and cross-repo handoff: `backend-handoff.md` close-tab milestone. + +## 2026-10-16 — Close-tab cascade confirm dialog (subagent groups) + +1. **"Subagents" of a tab = its VISIBLE subagent group (open tabs only).** The confirm dialog + triggers when the closed tab has descendants in the tab grid's grouping + (`tabGroupParentId` forest, transitive). Subagent conversations that were never opened as + tabs are NOT closed — the FE has no knowledge of them (no lineage API consumed here). The + backend's `POST /conversations/:id/close` does not cascade either, so this matches the + visible mental model. + +2. **The cascade issues one `POST /close` per conversation** (via the existing `onClose` → + `store.closeTab` port, unchanged). The backend close is idempotent, so a descendant closed + by another client mid-dialog is a harmless no-op. No new cross-unit contract was needed: + the decision logic is pure (`cascadeCloseIds` in the tabs unit) and the dialog lives inside + `TabList`. + +3. **Dialog offers three outs:** "Yes, close all N" (cascade), "No, keep them open" (close + only the target tab — the pre-change behavior), and Cancel/Escape/backdrop (close nothing). + The user asked for a yes/no prompt; Cancel was added as the safe dismissal path. The + cascade-order snapshot (ids) is taken when the dialog opens. + ## 2026-06-24 — CR-10: workspaceId on conversation.open / statusChanged 1. **Contract version already at 0.19.0 in the backend repo.** The `file:` dep in `package.json` @@ -13,7 +45,7 @@ 2. **The `dist/` directory is owned by root** (likely from a previous root-run build). `bun run build` fails with `EACCES` when Vite tries to empty `dist/assets`. This is an environment issue, not a code issue — I verified the build succeeds with `--outDir dist-tmp` (a fresh directory I own, then - removed). The user may want to `sudo chown -R tradam:tradam dist/` or remove it. + removed). The user may want to `sudo chown -R "$(id -un):$(id -gn)" dist/` or remove it. 3. **Parser now rejects broadcasts missing `workspaceId`.** The WS parser (`logic.ts`) returns `null` for `conversation.open` / `conversation.statusChanged` messages that lack a string `workspaceId`. diff --git a/notes/instant-stop-2-plan.md b/notes/instant-stop-2-plan.md new file mode 100644 index 0000000..21b1db9 --- /dev/null +++ b/notes/instant-stop-2-plan.md @@ -0,0 +1,86 @@ +# Markdown Streaming Performance & UX — Implementation Plan + +**Status:** Ready for implementation — RE-SCOPED (see Scope) +**Branch:** `feature/markdown-streaming-render` (renamed from `feature/instant-stop-frontend-render`; commit to HEAD) +**Spec:** `notes/instant-stop-2-spec-ref.md` (read-only reference copy in this worktree; original at `backend/notes/instant-stop-2-frontend-render-backlog.md`) + +## Scope (IMPORTANT — this plan was re-scoped after the original dispatch) + +This feature implements ONLY the **markdown rendering performance/UX fix** — the checkpoint caching design below. The spec's other items are explicitly DEFERRED and must NOT be implemented here: + +- NO stop barrier / composer `stopping` state / stop-failure reconciliation (spec items A, B, C, G) +- NO delta coalescing / scroll frame-bounding (spec items D, F) +- No chat-store, Composer, ChatView, app-store, ws-adapter, or smart-scroll changes. + +The markdown fix stands alone: `streaming-markdown.ts` + `Markdown.svelte` + `markdown.ts`, plus tests. + +## Problem + +Every streamed delta re-renders the ENTIRE accumulating message through the full pipeline (closeOpenDelimiters regexes over the whole text → marked parse → hljs highlight of every fenced block → copy-button rewrite → DOMPurify sanitize → `{@html}` full subtree replacement). Cost per token grows with message length — ~O(n²) over the message lifetime — so long responses and growing code fences jank the UI, and whole-subtree DOM replacement churns layout and scroll. + +## Design — checkpoint caching of settled markdown blocks + +### New pure module: `src/features/markdown/logic/streaming-markdown.ts` +- `createStreamingMarkdown()`; `advance(state, textSoFar) → { prefixHtml, tailHtml }`; reset when the text shrinks or is not an append-extension of the cached source (e.g. authoritative replace). +- Internals: a block-boundary line-scanner, a per-block HTML cache, and a settled-source offset. + +### Safe-boundary rule (the core invariant) +A checkpoint may be placed ONLY at a blank-line boundary that is NOT inside: +- an open fenced code block (``` or ~~~), +- a list (any depth — blank lines inside a list do not end it), +- a blockquote, +- an unclosed raw-HTML construct (`<!-- -->`, unbalanced tag). + +When uncertain → do NOT checkpoint; extend the tail. The failure mode is degraded caching, NEVER wrong output. + +- Fallback: if new content matches a link-reference-definition pattern (`^\[.*\]:`), invalidate the cache and do one full re-render (a definition can rewrite earlier `[label]` shortrefs; rare in assistant output). +- Why blank-line + closed-context suffices: setext headings (`foo` + `---`) and GFM table delimiter rows require the triggering line to IMMEDIATELY follow the paragraph line — a blank line between makes retroactive block formation impossible. Lists/blockquotes/fences are tracked as open context by the scanner. + +### Rendering levels +- **Level 1 (mandatory):** the tail renders through TODAY'S exact pipeline (`closeOpenDelimiters` on the tail only → marked → `addCopyButtons` → DOMPurify). Output contract: `prefixHtml + tailHtml` ≡ `renderMarkdown(textSoFar)` at every step, modulo the newline marked emits between top-level blocks. +- **Level 2 (approved by the user):** hljs is SKIPPED for the open tail fence (plain escaped `<pre><code>`); a code block is highlighted EXACTLY ONCE, when it closes (settled into the cache with its highlighted HTML). Code renders uncolored while its fence is open; colors pop in when the fence closes — accepted UX. NO line-incremental highlighting — unsound (hljs state spans lines; an unterminated string/block comment recolors earlier lines). +- **Completion backstop:** when `streaming` flips false (done / turn-sealed), do ONE full authoritative render via today's `renderMarkdown(text)` and use that. Final/persisted output is always byte-identical to today's behavior; any incremental-path divergence is transient (streaming-only) by construction. + +### Component changes: `Markdown.svelte` +- Two sibling containers inside the existing `.markdown-body` wrapper: `<div>{@html prefixHtml}</div><div>{@html tailHtml}</div>`. + VERIFIED in the installed runtime: Svelte 5 skips the DOM write when the html string is unchanged (`svelte/src/internal/client/dom/blocks/html.js:75` — `if (value === (value = get_value() ?? '')) return;`), so the settled prefix's DOM subtree is never re-parsed / re-created / re-laid-out. +- The wrappers must not break block margin collapsing: no padding/border/min-height on them. +- The delegated copy-button listener stays on the outer container (unchanged). +- `streaming === false` → the single-container full-render path (today's behavior, unchanged). +- Thinking chunks (plain pre-wrap) and user text chunks (plain `<p>`) are unaffected. + +## Tests +- **Corpus equivalence property test:** a corpus of tricky markdown (setext after blank line; GFM table formation; link-reference definitions; nested lists with internal blank lines; blockquotes; ~~~ fences; inline emphasis spanning delta boundaries; raw HTML; a huge growing fence) replayed token-by-token: at EVERY prefix length, the concatenation (whitespace-normalized) equals `renderMarkdown(prefix)` where the boundary rule held, and the final render equals today's output exactly. +- hljs spy: exactly one highlight call per closed block; zero while a fence is open. +- Cache stability: `prefixHtml` string identity stable across appends; shrink/replace resets. +- All existing markdown tests remain green; sanitization guarantees unchanged for final HTML. + +## Performance validation + +Instrumented fixture (counts, not wall-clock): while streaming thousands of deltas into a long message with growing code fences, full-pipeline invocations and hljs calls remain proportional to the number of BLOCKS, not to message length × deltas. The subsystem is isolated — no full-app fixture needed. + +## Files +- New: `src/features/markdown/logic/streaming-markdown.ts` (+ test). +- Modified: `src/features/markdown/ui/Markdown.svelte`; `src/features/markdown/logic/markdown.ts` only if the tail pipeline needs a shared export (+ tests). +- NOT modified: chat store, Composer, ChatView, app store, ws adapter, smart-scroll, backend. + Note: the checkpoint fix itself removes most scroll/layout amplification, because settled content stops resizing (the ResizeObserver stops firing for the prefix) — no scroll-controller changes are needed for this feature. + +## Verification gates + +``` +bun run typecheck && bun run test && bun run check && bun run build +``` + +## Commit plan + +1. `docs(markdown): re-scoped implementation plan — streaming performance` (this file, replacing the earlier three-phase version) +2. `feat(markdown): checkpoint streaming render — settled-block cache, highlight on close` +(plus fixups as needed) + +## Out of scope (deferred, separate future work) + +- Stop barrier / stopping composer state / stop-failure reconciliation (spec items A, B, C, G). +- Delta coalescing + scroll frame-bounding (spec items D, F). +- Problem 1 (merged) / Problem 3 (backend queue semantics). +- Transcript virtualization; line-incremental highlighting (unsound). +- Backend WS event coalescing. diff --git a/notes/pinned-tiles-plan.md b/notes/pinned-tiles-plan.md new file mode 100644 index 0000000..6d50e55 --- /dev/null +++ b/notes/pinned-tiles-plan.md @@ -0,0 +1,129 @@ +# Pinned tiles + displacement reflow — plan + +Feature: home tiles can be dropped onto **specific cells** of the grid; empty cells are +real estate that persists. Dragging shows a live **FLIP** reflow preview; Escape/cancel +returns everything to the committed layout. + +## Interaction semantics (user-specified, literal) + +The grid's cells form a fixed stream (wide: column-major over 4 rows; tall: row-major +over 2 columns). Only **filled** cells flow. Insertion at a cell pushes the contiguous +run of filled cells **forward** (in stream order) until the first empty cell absorbs it: + +- `X _ X` + place Y at slot 1 → `Y X X` (X shifts into the gap; the second X never moves) +- `_ X Z _` + place Y at slot 2 → `_ Y X Z` (X and Z each shift one; leading gap kept) + +Rules that follow (and are deliberately uniform): + +1. **Gaps never collapse.** Removing a tile opens a gap at its anchor; nothing slides + backward to fill it. So an internal move = remove (gap opens) + insert (run pushes + forward): `X Y Z`, drag X to slot 3 → `_ Y X Z`. The gap at slot 1 is preserved — + consistent with the two examples above. +2. **Preview = the exact post-drop state, recomputed from the COMMITTED layout on every + hover-cell change** ("as if the user directly moved from the original to there" — no + accumulation of intermediate previews). The animation, however, runs from wherever + tiles currently are on screen (mid-flight included) to the new targets — re-entrant + FLIP. +3. **Escape / pointercancel** discards the preview: the dragged tile flies back to its + committed anchor and everything FLIPs home. Nothing persists. +4. During a drag: a **faint dot** marks the center of every unit cell (incl. one extra + pitch of slack past the content end), and the **dragged tile element itself follows + the pointer** (lifted: scale + shadow, no pointer events). A dashed ghost footprint + marks the landing anchor. + +## Data model (v2) + +```ts +TileLayoutEntry { id, colSpan, rowSpan, order, col?, row? } // col/row: pinned anchor +TileLayoutData { version: 2, tiles } // sanitize migrates v1→v2 +``` + +- Anchors are persisted in a **universal 4-row frame** (wide's frame). Tall normalizes: + render `{ c, r }` ↔ universal `{ col: 1, row: 2(r−1)+c }`. Wide is identity. + A pin outside tall's 2 columns reads as **unpinned** in tall (render-time clamp — same + philosophy as spans). Round-trips can be lossy in that ragged case; accepted. +- `col`/`row` absent = **unpinned** → flows into the first free cells at render time. +- `order` remains: arrival order for unpinned flow + first-wins tie-breaks. Sort-by-recent + now also **clears pins** (an explicit arrangement reset). +- v1 documents load as all-unpinned (render identical to today) and write back as v2 on + the next mutation. No version sniffing outside `sanitizeLayout`. + +## Pure ops (`logic/tile-layout.ts`) + +- `pack(entries, geometry)` — v2: place in-range pinned tiles first (universal→render + anchor; out-of-range/overlapping/span-unfit → deferred to unpinned), then flow unpinned + by first-fit. One pass, no solver; first-pinned-wins, strictly one-ply (a displaced pin + never bumps another pin). +- `displacePlace(placements, draggedId, cell, geometry)` — the render-frame core: + remove the dragged tile (gap opens), force-anchor it at `cell`, then walk the others in + stream order: keep if free, else re-anchor at the next fitting slot **forward** of the + conflict (never backward). Used for BOTH the live preview (on packed placements) and, + via `placeAtCell`, the commit. +- `placeAtCell(entries, id, cell, geometry)` — commit path: pack → displacePlace → + pin back to entries (universal-frame anchors; persisted spans kept raw, anchors come + from the render placements). +- `pinAtFirstEmpty(entries, id, geometry)` — the create flow: pin the tile at the first + truly-empty slot (holes now count). +- `firstEmptySlot` — unchanged contract, now hole-aware by construction. +- `sortByRecentActivity` — renumbers order AND clears pins. + +## Store (`tile-layout-store.svelte.ts`) + +- `move(id, toIndex)` → replaced by `placeAtCell(id, cell, geometry)`. +- Structural equality (`sameEntries`) gains `col`/`row`. + +## Drag (`logic/drag-controller.ts`) + +- `targetIndex: number` → `targetCell: { col, row } | null` (pure rename; semantics of + the state machine — tap/lift/drop/cancel, touch scroll-wins — unchanged). + +## Component (`HomeTileGrid.svelte`) + +- **Hit math** replaces `elementFromPoint`: cell = `floor((pointer − gridRect.origin) / + pitch) + 1` (pitch = unit + gap), clamped to ≥ 1 and to the fixed cross-axis. The gap + between cells maps to the preceding cell. The pointer always resolves to a cell during + a drag (clamped), so empty cells are drop targets. +- **Dots overlay**: while dragging, the grid gets a `radial-gradient` background + (2px dot, `base-content` at low opacity) with `background-size = pitch × pitch` and + `background-position = unit/2` so dots sit at cell centers; `padding` extends one pitch + past the content end so the append-slot has a dot. Pure CSS, no extra DOM. +- **Dragged tile follows the pointer**: the wrapper keeps its committed grid slot in the + layout; a `translate(pointer − grabOffset) scale(1.05)` transform (via gsap) lifts it + out visually; `pointer-events: none` + pointer capture keep the events flowing. +- **Ghost**: dashed outline (dragged tile's tone) at the preview anchor, footprint-sized. +- **FLIP**: GSAP `Flip` plugin. Capture in `$effect.pre` (state of all tile elements + except the dragged/settling one), mutate (preview placements / committed placements), + `Flip.from(state, …)` in `$effect` after the DOM update. Re-entrancy comes free: + `Flip.getState` records in-flight transforms, so each hover change animates from the + current visual position — while the preview itself is always recomputed from COMMITTED + (semantics vs. motion stay separate, per the user's requirement). +- **Drop**: commit `placeAtCell`, then settle the tile from its pointer transform to + identity (gsap, ~150ms) — it lands exactly where the ghost was. +- **Escape**: window `keydown` in the CAPTURE phase while dragging (beats the page-level + Escape→deselect handler, `stopPropagation`): dispatch `cancel` → preview discards → + tiles FLIP home → dragged tile gsap-animates from the pointer back to its anchor. + +## Out of scope / non-goals + +- No backward displacement (runs only push forward); no multi-tile drag; no touch + auto-scroll while dragging (existing scroll-wins semantics stay). +- The resize picker is unchanged: resizing a pinned tile may push it to unpinned flow if + the footprint no longer fits its anchor (render-time rule — persisted spans untouched). + +## Files + +1. `src/features/workspaces/logic/tile-layout.ts` (+ `tile-layout.test.ts`) — the bulk +2. `src/features/workspaces/tile-layout-store.svelte.ts` (+ test) +3. `src/features/workspaces/logic/drag-controller.ts` (+ test) — mechanical targetCell rename +4. `src/features/workspaces/ui/HomeTileGrid.svelte` — hit math, dots, follow-pointer, ghost, Flip +5. `src/features/workspaces/ui/WorkspacesHome.svelte` (+ test) — wire placeAtCell; sort label +6. `src/features/workspaces/ui/HomeHeader.svelte` — sort label copy + +## Verification + +- New pure tests: v2 sanitize/migration, pinned pack (incl. wide↔tall mapping, overlap + first-wins, out-of-range → flow), displacement (both user examples + internal moves + + span cases), pin round-trip, hole-aware firstEmptySlot, store equality. +- Component tests: Escape cancels a drag without persisting; drop commits `placeAtCell`. +- `bun run typecheck && bun run test && bun run build`; biome clean on touched files. +- Headless screenshots (wide + tall) of a pinned layout with gaps. diff --git a/notes/search-backend-selection-plan.md b/notes/search-backend-selection-plan.md new file mode 100644 index 0000000..b7c9623 --- /dev/null +++ b/notes/search-backend-selection-plan.md @@ -0,0 +1,112 @@ +# search-backend-selection — Frontend Plan (research + planning, NOT implemented) + +Goal: a "Search backend" dropdown under Settings that lets the user pick the active +search backend. GLOBAL setting persisted on the backend. Decided: exactly two +backend entries — "Firecrawl (self-hosted)" and "Umans (Exa)" — plus "None" as the +off state; save-on-change. Mirrors the backend plan in +`worktrees/search-backend-selection/backend/notes/`. + +## 1. Current state (verified in this worktree) + +- `src/app/views/SettingsPane.svelte` is currently FE-local only: a single + `ChatLimitField` (chat limit, persisted browser-side). It takes `ports` and calls + `store.setChatLimit` — so the pane already has a store seam for persistence. +- Server-persisted per-workspace settings today follow the heartbeat pattern: own + feature dir (`src/features/heartbeat/`) with `logic/view-model.ts` + `ui/*.svelte`, + store methods `heartbeatConfig()` / `setHeartbeatConfig()` (store.svelte.ts ~2303), + own view pane (`HeartbeatPane.svelte`), registered via `register-views.ts`. +- `ChatLimitField.svelte` is the canonical settings-field pattern: `$state` + value/saving/error/justSaved, `$effect` re-seed while untouched, dirty-guarded + save button, Enter-to-save, spinner while saving. +- Dropdown precedent: `features/chat/ui/ModelSelector.svelte`, + `ReasoningEffortSelector.svelte`, `HeartbeatView.svelte` all use native `<select>` + with DaisyUI classes. +- Each FE feature exposes a `manifest` (name/description) listed in + `ExtensionsPane.svelte`'s frontend-modules table — a new feature dir adds its + manifest there. + +## 2. Proposed UI + +New feature `src/features/search-settings/`: + +``` +features/search-settings/ +├── index.ts # public exports + manifest +├── logic/view-model.ts # pure: options list, dirty check, validation +├── logic/view-model.test.ts +└── ui/SearchBackendField.svelte + └── SearchBackendField.test.ts +``` + +**`SearchBackendField.svelte`** — modeled on `ChatLimitField`: +- Native `<select class="select select-bordered select-sm">` bound to local `$state`. +- Options: "Firecrawl (self-hosted)" and "Umans (Exa)" (disabled with a "not + configured" suffix when `!configured` — i.e. no `UMANS_API_KEY`), **plus** a + `"none"` option labeled e.g. "None (search disabled)". +- **Save on change** (decided): a select commits immediately — no separate Set + button. Saving/saved/error feedback around the change. +- States: `saving` (spinner), `saved` ("Saved: <label>."), `error` (inline + `text-error`, revert select to last persisted value on failure). +- Loading state: skeleton/disabled select while `GET` is in flight. +- Help text below: one line explaining the selection affects the `web_search` + tool's *search* mode only — scrape/crawl still route to the self-hosted + Firecrawl backend (and `map` is being removed from the tool entirely). + +**Placement**: inside `SettingsPane.svelte`, under the Chat limit section (user +asked for "a dropdown under settings"). The config is GLOBAL, so no +workspace-switch re-seeding is needed — load once on first mount (or on settings +view open) and cache in the store; the field still re-seeds from the persisted +value after a successful save (ChatLimitField's `$effect` pattern). + +## 3. Store / API wiring + +`store.svelte.ts` (port contract + implementation): + +```ts +searchBackendConfig(): Promise<SearchBackendConfigResult>; +setSearchBackend(backend: string): Promise<SearchBackendConfigResult>; +// state: searchBackendConfig | null (loaded), loading flag, last error +``` + +Calls the backend contract (see backend plan §2.4 — GLOBAL routes): + +``` +GET /search/config → { backend, available: [{id,label,configured}] } +PUT /search/config { backend } → { backend } | 400 | 503 +``` +(No capabilities field — the selection affects search mode only; scrape/crawl +always route to Firecrawl. `map` is removed from the tool entirely, per the +backend plan.) + +- Wire types live in `@dispatch/transport-contract` / `wire` (backend plan) — FE + types mirror them via the existing wire type imports. +- Load when the Settings pane mounts (lazy) or on workspace switch; cache like + heartbeatConfig does. +- 503 (extension not loaded) → render the field disabled with "search not + available" hint — graceful degradation, matching heartbeat's degrade philosophy. + +## 4. Registering the module + +- `features/search-settings/index.ts` exports `manifest` (name "Search Settings", + description) → add to the `ExtensionsPane.svelte` frontend-modules list so the + table doesn't drift from what's composed. + +## 5. Tests / verification + +- `view-model.test.ts`: options construction (available ∪ none, configured flag → + disabled option), dirty check, revert-on-error value computation. +- `SearchBackendField.test.ts`: render options from config, save-on-change fires + `save`, error state reverts selection, loading state disables. +- Store tests: `searchBackendConfig`/`setSearchBackend` wiring against a fake port, + workspace-switch re-seed. +- Full `bun run typecheck` / `bun run test` / `bun run check` per AGENTS.md. + +## 6. Decisions (shared with backend plan §4) + +1. Dropdown entries: exactly "Firecrawl (self-hosted)" + "Umans (Exa)", plus + "None (search disabled)". Umans sub-provider fixed to `exa`. +2. GLOBAL setting persisted to the backend (`GET/PUT /search/config`) — no + workspace-scoped behavior. +3. Save-on-change with saved/error feedback (no Set button). +4. Relay output decided: hits only — the tool returns parsed `SearchHit[]`; + the relay model's prose answer is discarded. No open questions remain. diff --git a/notes/text-file-embeds-implementation.md b/notes/text-file-embeds-implementation.md new file mode 100644 index 0000000..2f23a41 --- /dev/null +++ b/notes/text-file-embeds-implementation.md @@ -0,0 +1,294 @@ +# Text-file embeds — implementation report + +## Delivered behavior and syntax + +Save the document completely, then emit a short summary and a **standalone paragraph**: + +```markdown +I saved the implementation plan; the key change is the new validation boundary. + +[Implementation plan](text-file:notes/plan.md) +``` + +Absolute path and filename encoding examples: + +```markdown +[Research](text-file:/tmp/research.md) + +[Research with spaces](<text-file:notes/research%20report.md>) + +[Literal percent](text-file:notes/100%25.md) +``` + +The prefix is exactly `text-file:` (not image syntax). Destinations are POSIX +absolute paths or paths relative to the conversation's authoritative effective +working directory. Angle-bracket destinations can also contain literal spaces. +Markdown escapes are interpreted by marked, then percent escapes are decoded +**once**. `%2520` denotes a literal `%20` filename; `%23` and `%3F` denote filename +`#` and `?`. The HTTP adapter uses URLSearchParams to transport that literal +filesystem path; the backend does not perform another destination decode. + +Only a complete, single-line, top-level inline-link paragraph opts in. Ordinary +links, images, inline/fenced/indented code, reference-style links, titled links, +list/blockquote links, and links surrounded by prose retain ordinary Markdown +behavior and never trigger text-file reads. While streaming, an embed waits for +a blank-line paragraph boundary or completion of the message, avoiding speculative +fetches for incomplete links. Examples inside code never fetch. Unsupported or +malformed destinations remain inert ordinary Markdown, not browser/server URL +fetches. Raw HTML cannot forge a file-fetching block. + +## Presentation + +The document appears inside the existing **inset-well**, matching the recessed +panel style of tool-call/thinking blocks. It includes its label/path and +Refresh/Retry controls. Markdown extensions `.md`, `.markdown`, `.mdown`, `.mkd`, +`.mkdn` render through the existing marked/highlight.js/DOMPurify pipeline. +Other valid text/code files render safely as literal whitespace-preserving code, +without guessing a language or interpreting their HTML/Markdown. + +The default preview renders approximately the **first six and last six source +lines**, with an omitted-source-line count. Files of twelve lines or fewer are +not duplicated. Expand document shows the full rendered document; Collapse +document returns to the preview. The control exposes `aria-expanded`. +Top-level code-fence context is restored around preview fragments. Previews are +source-line excerpts, not a six-visual-line layout clamp: wrapping, headings, +tables and multiline constructs can change their visual height/formatting. Some +constructs crossing excerpt boundaries (lists, tables, reference definitions, +raw HTML) are necessarily approximate; expansion renders the authoritative full +Markdown. + +Both preview and full content are sanitized. Existing raw-HTML/link policy and +remote-image click consent are preserved. Omitted images are not mounted until +expansion. Relative local images resolve against the returned document's directory +through the existing local-image route. Embedded documents receive no text-file +client, so nested embed references are inert and cannot recursively fetch. + +Loading, empty-file, transport/validation failure and retry states are visible; +failures do not break surrounding chat. File text is display data, never a +conversation/history mutation. + +## File lifecycle and resource policy + +- **Live view, not snapshot.** Only the short reference remains in stored chat. + No document body or hash/snapshot is persisted with the message. +- A mounted chat fetches current disk bytes on first reference. Repeated exact + decoded paths share an in-flight/result entry while subscribed. There is no + file watcher or polling. Already displayed content does not update itself. +- Refresh (or Retry after failure) invalidates **all live files in that mounted + chat**, refetches authoritative context, and rereads their current contents. + This prevents mixing old and refreshed routing contexts. Expansion alone does + not fetch. File deletion produces a not-found state on the next read; recreation + can recover through Retry. +- When the last subscriber leaves, its entry is aborted/released. Returning to + chat/reloading/remounting reads current disk state, which can differ from the + original message. No historical immutability is implied. +- Known cwd/computer setting changes and conversation/workspace switches replace + the context-bound client, abort old requests and discard its cache. Late + responses cannot publish into a new client/path. External context edits not + signaled to the UI are detected by backend guards on subsequent reads or by + Refresh; existing displayed text is not watched for external changes. +- Limit **256 KiB (262144 bytes)** per file, strict UTF-8, allowing tab/LF/CR but + rejecting NUL and other C0/C1 binary controls. Exactly the limit is accepted; + oversize files are rejected, never silently truncated. +- At most **8 embeds per message**, **32 active distinct paths per mounted chat** + (at most 8 MiB source text), and **4 concurrent file loads**. The per-message + limit is explained inline; active-cache overflow has a visible error. +- Backend opens a single nonblocking/no-follow descriptor, checks regular-file + status and reads at most limit+1 bytes, including a final growth check. FIFOs, + devices, directories and final-component symlinks are rejected. Ancestor + directory symlinks and ordinary `..`/absolute paths are allowed: this is **not a + cwd sandbox**. Reads have a byte/work bound but no special filesystem timeout + or guarantee of a coherent snapshot while another process modifies a file. + +Unsupported: file URLs, HTTP/network destinations, tilde expansion, Windows paths, +backslashes, control characters and remote-computer embeds. Remote targets are +explicitly rejected, never read from an identically named local path. + +## Backend seam and trust boundary + +Existing `transport-http` is extended; no new extension/tier or kernel feature. +Additive `transport-contract` interfaces are `TextFileQuery`, `TextFileResponse`, +`TextFileContextResponse`; package version remains 0.26.0 with local `file:` deps, +and the frontend's tracked public contract mirror is updated. + +```text +GET /conversations/:id/text-file-context + -> { conversationId, cwd: string|null, computerId: string|null } + +GET /conversations/:id/text-file?path=...&expectedCwd=...&expectedComputerId=... + -> { path, text, format: "markdown"|"text", bytes, maxBytes } +``` + +Context preflight performs no LSP or file work. Both routes resolve persisted +conversation settings and workspace defaults server-side, not browser state or +server process cwd. Relative conversation cwd is resolved against an absolute +workspace default; an unresolvable configured relative cwd returns 422. With no +cwd configured, context is null, absolute local file references work, and relative +file references return 400. The frontend always supplies both preflight-derived +guards. Empty guard values assert unset cwd/local computer; absent guards skip +comparison. Guards never override server routing. The route rechecks context +after I/O before returning successful text. + +All endpoint responses use `Cache-Control: no-store`, including middleware errors. +Relevant statuses: 400 invalid/non-text/nonregular, 403 unreadable or rejected +Origin, 404 unknown conversation/missing file, 409 stale context, 413 oversized, +422 unsupported remote/unresolvable configured context, 500 read failure. Existing +Host rejection remains 421. + +**Authentication limitation, explicitly audited:** this application currently uses +Host/Origin checks plus loopback/Tailscale deployment restrictions, not bearer or +per-user session authentication. The new routes retain exactly that existing trust +boundary; they do not invent an authentication guarantee. A trusted client with +access to this API can read supported local files accessible to the backend UID, +including absolute paths outside cwd. Do not expose this transport publicly or to +untrusted users. No arbitrary HTTP fetch is added. + +## Initial AI context, custom prompts and persistence + +Mandatory application guidance is appended **after template variable resolution** +in `SystemPromptService.construct`. The unchanged default template and exact +user-authored custom-template bytes are preserved. An empty custom template now +constructs guidance-only context. Guidance includes actual syntax/examples, +save-before-reference, short summaries, supported paths/limits, local-only/live +semantics, preview/expand behavior and the distinction between user display and +reading a file for the model's own understanding. + +New conversations and normal prompt reconstruction (compaction, missing resolved +prompt, cwd/computer metadata invalidation) receive it, including installations +with existing custom templates. Resolved prompts remain persisted per conversation. +Existing matching resolved prompts, including legacy empty prompts, are returned +**byte-for-byte unchanged** until ordinary reconstruction; changing the global +template alone does not retrofit existing conversations. There is no migration or +read-time mutation. The pure suffix-idempotent `appendTextFileGuidance` helper is +also used for missing/stale read-only cache-warming assembly, yielding the same +provider system prompt as the next real turn without warm persistence writes. +Generic `resolveText` remains generic and does not append application guidance. + +The session-orchestrator tests capture actual provider requests using real prompt +service, conversation store, assembly and kernel turn loop: default/custom first +turns, legacy continued/warm prompts, missing/cwd/computer invalidation, warm/real +parity, compaction and later reuse. History retains the reference, not the file +body. This is verified delivery, not merely a default-string assertion. + +Explicit caller-owned per-turn `systemPrompt` overrides (e.g. heartbeat) retain +their existing **AS-IS bypass** contract; they do not receive appended guidance. +The initial instructions in already-running/persisted conversations are not +retroactively changed, and this task did not redeploy/restart the shared service. + +## Initial verification (superseded by Bun-native follow-up below) + +`bun install` completed inside both feature worktrees before build/testing; frontend +was refreshed after backend declarations were built. All work is on +`feature/text-file-embeds`, with no merge or push. After the user's stop request, +all delegated generations were stopped and the lead completed fixes, tests, +verification, documentation and commits directly; no further subagents. + +### Frontend + +- `bun run typecheck`: **0 errors / 0 warnings**. +- `bun run check`: clean, 308 files (no warnings/infos). +- `NODE_OPTIONS=--no-experimental-webstorage bun run test`: **2586 passed, + 2 opt-in live tests skipped**, twice, 130 passed files / one skipped file. +- Markdown-specific tests: **236 passed** (parser, streaming, sanitized UI, + image consent, head/tail/expand, code/plain text, request effects and limits). +- `bun run build`: succeeds; existing Svelte initial-value capture diagnostics + and large-chunk advisory remain. +- `TEXT_FILE_LIVE_PROBE=1 bunx vitest run scripts/text-file-live.test.ts`: + **2 passed** against a freshly spawned ephemeral Bun HTTP app using a real + conversation store and temp disk files. The actual Svelte component and client + render a saved plan from only summary+reference, expand the omitted middle, + refresh updated content, remount/reload current bytes, show deletion, recover on + Retry and reject authoritative remote context. The fixture process/files are + cleaned up. No deployed service is used or restarted. +- Plain `bun run test` on host **Node v26.11.1** reproduces 26 pre-existing + localStorage-related failures in untouched `tile-layout-store` and + `WorkspacesHome` suites. A temporary `git archive HEAD` baseline with the same + dependencies independently reproduced exactly those 26 failures. Disabling + Node's experimental Web Storage lets jsdom supply browser storage and makes + the whole suite pass; no unrelated application/test code was modified for this. + +### Backend + +- `bun run typecheck`: succeeds. +- `bun run test`: **4277 passed / 10 skipped**, 221 passed files / one skipped. +- `bun run test:bun`: **190 passed / 0 failed**, including two real ephemeral + loopback text-file HTTP tests in the standard Bun test target. +- `bun run check`: clean, 754 files (no warnings/infos). +- `bun build packages/host-bin/src/main.ts --compile --outfile dist/dispatch-server --minify` + and equivalent CLI compile: both succeed, worktree-local ignored outputs only. +- Standard `bin/build --no-frontend` cannot complete here: its existing sibling + `claude` symlink points at the separate main Claude checkout, where bundling + fails resolving `@dispatch/kernel`. The wrapper may have refreshed ignored compiler outputs there before failing; + no dependency install or tracked edits were made there; the standalone feature backend/CLI compile above is the verified + build. External Claude bundling is **not** claimed verified. +- Backend tests cover pure path/query/encoding policy, effective context, + strict UTF-8/binary/empty/exact-limit/oversize files, directories/devices/FIFO/ + symlink refusal, unknown conversation, stale guards, remote inheritance with + an existing local path, live changes/deletion and Host/Origin rejection. +- `git diff --check` clean in both repositories. No probe server/collector leaks. + +### Remaining manual validation + +Automated UI tests use jsdom, not a graphical browser. After deploying both +commits, visually confirm block styling/responsiveness on desktop/mobile, expand +and collapse a longer Markdown plan, and click a remote-image consent button. +No screenshots, graphical-browser layout check, AI-provider-generated live reply, +remote file support, watchers, snapshots or editor/attachment redesign are claimed. + +## Bun-native verification follow-up — 2026-10-24 + +This is the **authoritative final verification**, superseding the initial +Node-backed Vitest results above. `bun run` previously respected Vitest's +`#!/usr/bin/env node` launcher. Both repositories now explicitly execute +`bun --bun node_modules/vitest/vitest.mjs` in `test` and `test:watch`, so ordinary +`bun run test` uses Bun itself, including its workers. No `NODE_OPTIONS` is needed; +all following runs explicitly unset it. Runtime: **Bun 1.4.3**. + +Bun-native runs exposed existing test assumptions which have been corrected, not +skipped and not hidden with larger timeouts: + +- HTTP/WS listener tests now spy on **Bun.serve only** and restore it afterward, + instead of redefining the non-configurable global `Bun` object. The fallback + global stub remains only for non-Bun supplemental runners. +- The image migration test sorts a directory listing before comparing its + contents; native filesystem enumeration order is not a contract. +- The local-image home-expansion test uses a path relative to the actual OS home + into its temporary fixture; it no longer assumes mutating HOME changes Bun's + cached homedir result. No real home-directory file is created. +- The frontend cyclic tool-input fallback was genuinely slow under Bun's pretty + JSON serializer. A local ancestor-stack replacer detects cycles before native + recursion, preserving the existing fallback text. Added a regression case for + nested cycles and **shared non-cyclic references** so valid JSON isn't rejected. + The original timeout-prone test now completes promptly; no timeout was raised. +- The opt-in live UI/network test no longer depends on Node-only + `transferableAbortController`. Its scoped jsdom environment retains the + runtime's matching native AbortController/AbortSignal pair, so actual Bun fetch + and cancellation are exercised. The production adapter remains unchanged. + +Final commands/results: + +| Worktree | Command | Result | +| --- | --- | --- | +| frontend | `bun run test` | **2587 passed / 2 opt-in skipped**, repeated green (130 passed files / one skipped) | +| frontend | `bun run test:text-file-live` | **2 passed**, real Svelte + ephemeral Bun HTTP + temp disk files | +| frontend | `bun --bun run typecheck` | **0 errors / 0 warnings** | +| frontend | `bun --bun run check` | **309 files clean**, no warnings/infos | +| frontend | `bun --bun run build` | **passed**, existing initial-value/large-chunk advisories remain | +| backend | `bun run test` | **4278 passed / 9 skipped**, twice (221 passed files / one skipped) | +| backend | `bun run test:bun` | **190 passed / 0 failed** | +| backend | `bun --bun run typecheck` | **passed** | +| backend | `bun --bun run check` | **754 files clean**, no warnings/infos | +| backend | `bun build ... --compile` (server and CLI) | **both passed**, ignored worktree-local binaries | + +The nine backend skips are the existing SSH integration suite; an additional +Bun-only test previously skipped under Node now runs, hence 4278 rather than +4277 passing. The frontend count increases by one for the new shared-reference +regression test. The two frontend live tests are intentionally opt-in but were +run separately and passed. No tests were excluded to obtain green results. + +Dependencies were refreshed using `bun install` inside both feature worktrees. +The external-Claude standard-build wrapper limitation described above remains; +no external/main checkout edits, installs, service restarts, merge or push were +performed in this verification follow-up. No subagents were used. Native probe +processes and temporary fixture files were cleaned up. Browser visual validation +remains manual as documented above. diff --git a/notes/text-file-embeds-plan.md b/notes/text-file-embeds-plan.md new file mode 100644 index 0000000..e56b3ed --- /dev/null +++ b/notes/text-file-embeds-plan.md @@ -0,0 +1,93 @@ +# Text-file embeds in chat Markdown + +## User request (verbatim) + +> I would like to add a new feature to the frontend: embedding of text files into the chat markdown. This is to save on the ai having to output verbatim content, when it can instead just link to it to show it in the chat. For example when making an new plan, it can just embed the plan. Or giving research, it can just embed it to display in the chat. This feature will also require a initial context update to explain to do this to the ai. Plan the feature and then summon a sol agent to implement it + +## Goal + +An assistant can save a plan, research report, or other text document to disk and reference it concisely in its reply. The chat displays that document inline without requiring the model to output its contents again. The reference remains the stored chat text; reading the embed for display must not inject its body back into model history. + +## Observed integration points + +- Frontend: `src/features/markdown/ui/Markdown.svelte`, `logic/markdown.ts`, and `logic/streaming-markdown.ts`. Rendering currently uses marked, highlight.js and DOMPurify. Its rendering core is synchronous/pure; streaming caches settled blocks separately from the changing tail. +- Chat wiring: `src/features/chat/ui/ChatView.svelte` supplies the Markdown component with the backend base URL and conversation cwd for local images. Audit the conversation computer context as well before resolving file references. +- Backend: `packages/transport-http/src/routes/images.ts` has a `/local-image` route; it serves local raster files, not text files. Inspect existing file/remote-execution contracts before introducing a text-serving seam. +- AI guidance: `packages/system-prompt/src/service.ts` defines `DEFAULT_TEMPLATE`, including existing image-embedding instructions. Stored custom templates take precedence over that fallback, and resolved prompts are persisted per conversation. Updating only the fallback may therefore not teach agents using an existing custom template. Trace the actual initial-context assembly and choose the appropriate non-destructive delivery point. + +These are discovery pointers, not a mandate to copy the image route's filesystem policy or to restructure the renderer. + +## Implementation plan + +### 1. Define the reference contract + +Implementation contract: a standalone paragraph link `[Plan](text-file:notes/plan.md)` opts into a text-file embed. An absolute path is also supported, e.g. `[Research](text-file:/tmp/research.md)`. For spaces use an angle-bracket destination, e.g. `[Research](<text-file:notes/research%20report.md>)`; percent escapes decode once after Markdown escapes, and literal percent signs use `%25`. Destinations are POSIX filesystem paths only, never `file:` URLs, HTTP URLs, tilde expansion, backslashes, or arbitrary network targets. Remote-computer text embeds are explicitly unsupported in this version (never fall back to reading the same path locally). A short labeled file reference is enough for an assistant to emit it. Ordinary Markdown links, images, inline code, and fenced code must retain their existing behavior. Syntax shown inside code must not fetch files. + +The syntax and its supported path forms must be identical across parser, tests, and AI instructions. Support absolute paths and paths relative to the conversation cwd. Decide explicitly how file URLs, spaces/encoding, and remote-computer paths work. Reject unsupported destinations rather than accidentally reading the same path from a different computer. Do not silently add arbitrary remote HTTP fetching. + +### 2. Provide bounded, authenticated file reads + +Reuse suitable existing contracts if available; otherwise expose the smallest transport operation required by the frontend. Keep access within the existing authenticated/authorized transport boundary and validate all request inputs. Resolve against the correct conversation/computer context, not the browser URL or server process cwd. + +Return text as data, never executable HTML. Bound file size and read work; handle non-regular files, binary/invalid text, unavailable paths, and remote errors gracefully. Ensure unsupported paths/protocols cannot become browser fetches or server-side URL fetches. Use a concrete, tested text-size policy and communicate any rejection or truncation in the UI. If extending contracts, follow package boundaries and existing route/composition patterns. + +### 3. Display an inline document + +Additional user requirement (verbatim): + +> the inline text should render the markdown, but it should be inside of a block like how tool calls are inside of an expandable block. It should show ~the first and last 6 lines of text from the markdown, and have an expand button to show more. + +This supersedes the generic document-container presentation. Render Markdown documents through the existing sanitized Markdown pipeline inside an expandable, tool-call-like block. The default preview displays rendered Markdown from approximately the first six and last six source lines, with a clear omission indicator between them; short documents must not duplicate overlapping lines. Provide an accessible Expand/Collapse control to show the full rendered document. Determine concrete styling from existing tool-call UI patterns. Both preview fragments and expanded content must be sanitized, preserve image-consent policy, and never recursively fetch nested embeds. Display other supported text/code files safely, preserving whitespace and using existing highlighting where suitable. Include the reference label/path so users can tell embedded content from the assistant's own prose. + +Keep fetching in an effectful UI/adapter layer rather than making pure Markdown rendering perform I/O. Provide visible loading, empty-file, and failure states, with a practical retry path. A failure must not break the surrounding chat. + +Integrate with streaming without repeatedly fetching on every token or replacing settled document state unnecessarily. Clean up stale requests on unmount/context changes and prevent late responses from rendering under a different path/conversation/computer. Repeated references should not cause uncontrolled duplicate work. + +Embedded content is untrusted: sanitize it, preserve existing remote-image consent, and do not recursively fetch embeds found within an embedded document. Preserve the existing security policy for raw HTML and links. Keep rendering bounded for large files. + +### 4. Specify persistence semantics + +Document whether an embed displays the current on-disk file or a captured snapshot. Prefer the simplest coherent implementation using existing infrastructure; do not imply historical immutability if the file is fetched live. State what happens when a file changes or is deleted and how refresh/retry behaves. Snapshots, file watchers, editors, and broad attachment-system redesign are not required by this request. + +### 5. Teach the AI in its initial context + +Explain the actual implemented syntax with a plan/research example, supported path rules, and relevant limitations. Tell the assistant to finish saving the file before emitting its reference, to use embeds instead of repeating long saved content when useful, and to retain a short explanatory summary in its reply. Make clear that embedding displays the file to the user; it is not a substitute for reading a file the assistant itself needs to understand. + +Verify guidance reaches the intended initial-context path for existing installations/custom templates without overwriting user-authored prompts. Respect existing prompt persistence semantics; explicitly report which conversations receive the new instructions and when. Add coverage for the delivery mechanism, not just a string assertion on an unused fallback. + +### 6. Verify and commit + +Read each repo's `AGENTS.md` and applicable scoped rules. Run `bun install` in both worktrees before verification. Implement within the isolated feature worktrees only, coordinating owners if delegating package work. + +Cover at least: +- Parsing and path resolution, escaped/encoded paths, non-embed links/images/code, incomplete streaming syntax. +- Markdown and plain-text display, XSS attempts, nested embeds, existing image-consent behavior. +- Loading, missing/empty/oversized/binary files, retry, context switches, stale responses, repeated references. +- Backend validation/access behavior and correct computer routing or explicit unsupported-computer rejection. +- Initial-context guidance and custom-template compatibility. +- End-to-end scenario: save a plan file, emit only its embed plus a short summary, and see its formatted contents in chat; reload and error behavior match documented semantics. + +Run frontend typecheck, tests, formatting/lint checks and production build. Run applicable backend typecheck, tests (including Bun-specific transport coverage as appropriate), formatting/lint checks and build verification according to its architecture guidance. Report pre-existing failures distinctly; do not silently claim passing checks. + +Commit verified work in both repositories on `feature/text-file-embeds`. Include final syntax, lifecycle semantics, verification results, and any limitations in the implementation report. Do not merge or push. + +## Acceptance criteria + +1. A short file reference displays a saved plan/research document inline in assistant chat without repeating its body in the model reply, inside a tool-call-like expandable block with rendered first/last approximately six-source-line previews and Expand/Collapse. +2. Existing ordinary Markdown behavior remains intact, including streamed messages. +3. File I/O and document rendering are bounded and remain within the existing transport trust and sanitization policies. The audited existing trust boundary is Host/Origin plus trusted-network deployment, not per-user/session authentication. +4. The AI receives accurate initial-context instructions through a verified path, without destroying custom prompts. +5. Both repositories contain committed, verified work on the feature branch; `dev` remains untouched. + +## Handoff + +This plan defines user-visible goals, integration observations, and acceptance criteria. The Sol implementation agent owns technical design, including final syntax and the minimal appropriate contract/architecture. Do not treat illustrative approaches as additional user requirements. Resolve details by inspecting the codebase and record the chosen behavior in the report. + +## Implementation outcome + +Implemented and verified; authoritative final syntax, lifecycle, context delivery, +limits and verification caveats are recorded in +`notes/text-file-embeds-implementation.md`. Existing persisted prompts are not migrated; +remote computer embeds are explicitly unsupported. The user requested no subagents +during implementation; all delegated generations were stopped and remaining work +was completed directly. No merge, push or shared deployment. diff --git a/notes/tool-call-display-plan.md b/notes/tool-call-display-plan.md new file mode 100644 index 0000000..000cbb3 --- /dev/null +++ b/notes/tool-call-display-plan.md @@ -0,0 +1,254 @@ +# Tool-call display cleanup — plan + +Branch: `feature/tool-call-cards` (frontend-only; no backend change required). + +## ✅ DECISIONS (user, final — these OVERRIDE anything below that disagrees) +1. **Every tool call is its own bubble.** No batch grouping anymore — parallel calls in one step + render as consecutive independent bubbles, each one a single card (call + result). +2. **Add a status icon** to the header: spinner while pending, ✓ (success tint) on ok, ✗ (error + tint) on `isError`. Keep the wrench as the tool icon; the status icon is added alongside it. +3. **Default collapsed** (all cards, including errors). +4. **Output preview = 12 lines: the first 6 and the last 6**, with a `⋯ N more lines` toggle + between them that expands to the full output (and collapses back). Outputs with ≤ 12 lines + render in full with no toggle. +5. Per-tool summaries for Dispatch's built-in tools + a generic fallback (§2.4) — proceed with + the recommendation. + +## Goal (from the user) +1. A tool call and its response render as **one dropdown** — not a call card plus a separate + result card. +2. The collapsed header shows a **concise summary of what was called AND what came back** + (e.g. `run_shell bun run test → 128 lines`). +3. Inspiration: oh-my-pi's tool cards. + +--- + +## 1. Current state (research) + +### Data +- Wire (`.dispatch/wire.reference.md`): `ToolCallChunk { toolCallId, toolName, input: unknown, stepId? }` + and `ToolResultChunk { toolCallId, toolName, content: string, isError, stepId? }`. Results are + separate chunks (role `tool`), paired to calls by `toolCallId`. The kernel guarantees exactly one + result per call (synthesized on interrupt). +- Results are **plain strings** — no structured `details` (exit code, line counts, diff, …). Any + result summary must be derived from `content` + `isError` + the call's `input`. + +### Rendering (`src/features/chat/ui/ChatView.svelte`, `src/core/chunks/groups.ts`) +- `groupRenderedChunks` makes a `tool-batch` group **only when a step has ≥2 calls**. Those batch + entries already pair call+result in one `LazyCollapse` (title = wrench + toolName + spinner/error + badge; body = `JSON.stringify(input)` then raw `content`). +- **Single-call steps (the common case) are NOT paired**: the call and the result each become a + `single` group and render as two separate collapses with the same title (wrench + toolName). This + is the "two dropdowns" the user sees. +- Neither path shows *what* was called or *what* came back in the collapsed header — only the tool + name. +- Keys: single tool chunks are keyed `c<seq>` / `p<index>`, so a provisional single tool card + changes key at seal → its open state is lost (same bug class the thinking-ordinal keys fixed). +- `LazyCollapse.svelte` (daisyUI checkbox collapse, lazily-mounted body) is the shared primitive — + keep it. +- Consumers of `RenderGroup` beyond ChatView: `src/core/metrics/place.ts` (`groupStepId`, + `groupTurnId` — step/turn metrics placement) and tests (`groups.test.ts`, `ui.test.ts`, + `ChatView.test.ts`, `place.test.ts`, `placement-scenarios.test.ts`). +- Chat-limit trimming (`trim.ts`) unloads by chunk seq, so a call can be unloaded while its result + stays loaded → an **orphan result** is possible and must still render. + +### Dispatch tool result shapes (backend, read-only) +| tool | input | result `content` | +|---|---|---| +| `run_shell` | `command`, `timeout?` | raw output; `isError` on non-zero exit / abort; `…\n\n[Command timed out]`; `[Output truncated: …]` | +| `read_file` | `path`, `offset?`, `limit?` | numbered lines / dir listing / `(empty file: …)` | +| `edit_file` | `path`, `oldString`, `newString`, `replaceAll?` | `Replaced N occurrence(s) in "…".` (+ appended LSP diagnostics) | +| `write_file` | `path`, `content`, `overwrite?` | `Created/Overwrote "…" (N bytes).` | +| `web_search` | `query` / `url`, `mode?` | markdown text | +| `lsp` | `operation`, `path`, `line?`, `character?` | text | +| `todo_write` | `todos[]` | text | +| `load_skill` | `name` | skill text | +| `consult_vision` | `question`, `imageIds?`/`path?` | conv id + answer | +| agent tools (`summon_agent`, `send_agent_message`, `read_agent`, `list_agents`, …) | `task`/`agent`/`title`, `conversationId`, `message` | text | +| MCP tools | arbitrary | arbitrary → generic fallback | + +Note: the exit code of `run_shell` is not in the result — only `isError`. + +### oh-my-pi (`../../oh-my-pi/packages/collab-web/src/tool-render/`) +- **One card per call, result folded in** (`ToolView.tsx`): the transcript pairs `toolResult` + messages to calls by id and passes `result` into the call's card; standalone results are never + rendered. +- **Header (one line, always visible)**: status glyph (spinner running / green ok / red err / + hollow pending) · tool name · per-tool **`Summary`** (truncated with ellipsis) · chevron. +- **Body (on expand)**: per-tool **`Body`** — args shown in a tool-appropriate way (e.g. `$ cmd` + prompt line for bash, path for read, colored diff for edit) then output in an `Output` block that + shows the first ~10–14 lines with a `⋯ N more lines` expander. Errors tinted red. +- **Registry** (`registry.ts`): `toolName → { Summary, Body }` with a **generic fallback** + (`argsDigest` = compact one-line JSON of args; body = args JSON + result text). +- Summaries mostly describe the *call* (command / path / pattern / query) and append result facts as + small badges when known (`+12 −3`, `failed`, `N matches`). + +--- + +## 2. Design + +### 2.1 One card per tool call (call + result) +Every tool call renders as exactly one collapse containing both call and response. A tool result is +never rendered on its own, except as an orphan (its call is unloaded) — then it renders as the same +card with the call side unknown. + +**Decision: no batch grouping.** Calls a model emitted together in one step render as consecutive +independent bubbles (one card each), in stream order. One code path for every tool call. + +### 2.2 Collapsed header = call summary → result summary +``` +[status] tool_name <call summary, truncated…> → <result summary> [chevron] +``` +- **status**: spinner while the result is pending; ✓ (success tint) on ok; ✗ (error tint) on + `isError`. Added alongside the existing wrench (decision 2). +- **call summary**: the most meaningful argument, single-line, whitespace-collapsed, truncated + (CSS `truncate` + a char cap). +- **result summary**: a short fact about the response (`128 lines`, `no output`, `1 replacement`, + `3.2 KB`, `failed`, `timed out`, first line of an error…). Omitted while pending. + +Examples: +``` +✓ run_shell bun run test → 42 lines +✗ run_shell bun run typecheck → failed · 12 lines +✓ read_file src/core/chunks/groups.ts:1-80 → 80 lines +✓ edit_file src/features/chat/ui/ChatView.svelte +3 −1 → 1 replacement +✓ write_file notes/plan.md → created · 4.1 KB +✓ web_search "svelte 5 runes snippets" → 37 lines +✓ lsp references src/app/store.svelte.ts:12 → 9 lines +⟳ summon_agent "tool-call-display: research" (running) +✓ mcp__foo {"a":1,"b":"x"} → first line of output… +``` + +### 2.3 Expanded body +1. **Call**: a tool-appropriate view of the input — `$ <command>` for shell, path line for + file tools, old/new for edit (as a simple −/+ block), otherwise a key/value list of scalar args with + pretty JSON for nested values (fallback = current `JSON.stringify(input, null, 2)`). +2. **Response**: the `content` in a `<pre>`, error-tinted when `isError`. If it has > 12 lines, + show the **first 6 lines, a `⋯ N more lines` toggle, then the last 6 lines** (N = total − 12); + the toggle expands to the full output and collapses back. ≤ 12 lines → full output, no toggle. + (Replaces today's `max-h-96` scroll boxes; still lazily mounted by `LazyCollapse`.) +3. Pending → "running…" placeholder instead of the response. + +### 2.4 Where the per-tool knowledge lives +A pure, data-driven **summarizer table** keyed by `toolName` with a **generic fallback**, mirroring +oh-my-pi's registry. It only produces strings/view-models (no components), so ChatView stays one +generic template. Unknown/MCP tools → fallback: +- call summary: first present of `command | path | file_path | query | url | pattern | name | + question | title | task | message | conversationId`, else compact one-line JSON of the input; +- result summary: `isError` ? `failed · <first line>` : `N lines` / `no output` / first line if + single-line and short. + +This is chat transcript rendering, not the surface interpreter, so the "never switch on a surface +id" rule doesn't apply; the table is optional per-tool polish with a guaranteed generic path +(GLOSSARY currently says tools are "rendered generically … by toolName" — this stays true for any +tool without an entry). + +--- + +## 3. Implementation steps (dependency-ordered) + +### Step 1 — Grouping: pair every call with its result (`src/core/chunks/groups.ts`) +- Replace the `single`-tool path AND the `tool-batch` group (decision 1: no batching) with ONE + group kind per tool call: + ```ts + export interface ToolEntry { + readonly toolCallId: string; + readonly toolName: string; // from the call, or from an orphan result + readonly call: ToolCallChunk | null; // null = orphan result (call unloaded by chat limit) + readonly result: ToolResultChunk | null; // null = pending + } + export type RenderGroup = + | { kind: "single"; chunk: RenderedChunk } // non-tool chunks only + | { kind: "tool"; entry: ToolEntry; provisional: boolean; stepId?: string; turnId?: string }; + ``` + (Name check vs GLOSSARY before coining; `ToolBatchEntry` / `tool-batch` go away — remove them + from `core/chunks/index.ts` and `features/chat/index.ts` exports.) +- Each `tool-call` chunk emits one `tool` group at its position; its `tool-result` (matched by + `toolCallId`, regardless of `stepId` — legacy rows without `stepId` pair too) is absorbed and + never emitted on its own. `provisional` = call OR result provisional; `stepId`/`turnId` from the + call (fall back to the result). +- Orphan results (no call in the rendered window) → a `tool` group at the result's position with + `call: null`. +- Keep the existing duplicate-`toolCallId` dedupe (first call occurrence wins; never emit two + groups with the same `toolCallId` — duplicate keys abort Svelte's keyed each). +- Stable render key `t<toolCallId>` — survives the provisional→committed seal (fixes the + lost-open-state bug for single tool cards). +- Update `src/core/metrics/place.ts` `groupStepId` / `groupTurnId` for the new shape (step-metrics + rows must still land after the LAST tool card of their step). +- Tests: rewrite `groups.test.ts` (every call → its own paired group; parallel calls → consecutive + groups; legacy no-stepId pairing; orphan result; pending result; dedupe), keep + `place.test.ts` / `placement-scenarios.test.ts` green (update fixtures for the new shape). + +### Step 2 — Pure summary view-model (new `src/features/chat/logic/tool-summary.ts`) +```ts +export interface ToolCardView { + readonly name: string; + readonly status: "pending" | "ok" | "error"; + readonly callSummary: string; // one line, truncated + readonly callBadges: readonly string[]; // e.g. "+3 −1", ":1-80" + readonly resultSummary: string | null; // null while pending + readonly callDetail: // how to render the input when expanded + | { kind: "command"; text: string } + | { kind: "edit"; path: string; oldText: string; newText: string } + | { kind: "fields"; fields: readonly { key: string; value: string; multiline: boolean }[] }; + readonly output: ToolOutputView | null; // null while pending +} +/** Decision 4: > 12 lines → head (first 6) + hiddenCount + tail (last 6); else full. */ +export type ToolOutputView = + | { kind: "full"; lines: readonly string[]; isError: boolean } + | { kind: "elided"; head: readonly string[]; hiddenCount: number; tail: readonly string[]; + all: readonly string[]; isError: boolean }; +export function viewToolEntry(entry: ToolEntry): ToolCardView; +``` +- Input normalization: `input` is `unknown` — accept objects; if a string, try `JSON.parse`; never + throw. +- Per-tool table for: `run_shell`, `read_file`, `edit_file`, `write_file`, `web_search`, `lsp`, + `todo_write`, `load_skill`, `consult_vision`, `youtube_transcript`, agent tools. Generic fallback + for everything else. +- Result facts derived from `content`: line count, `no output` / `(empty file…)`, `[Command timed + out]` → `timed out`, `[Output truncated…]` → `truncated`, edit `Replaced N occurrence(s)` → + `N replacement(s)` (+ `diagnostics` badge when diagnostics are appended), write `(N bytes)` → + `created/overwrote · size`. +- Exhaustive vitest coverage (pure, no mocks): each tool + fallback, malformed input, error, + pending, orphan, and the output elision boundaries (12 lines → full; 13 → 6 + 1 hidden + 6; + trailing newline handling). + +### Step 3 — Component (`src/features/chat/ui/ToolCard.svelte`, thin) +- Props: `entry: ToolEntry`. Calls `viewToolEntry`, renders `LazyCollapse` (collapsed by default) + with the header (status icon · wrench · name · `truncate` call summary · badges · `→ result + summary`) and the lazily-mounted body (call detail + output: first 6 lines, `⋯ N more lines` + toggle, last 6 lines; toggle expands to full output and back). +- Header layout must stay one line: `min-w-0 truncate` on the summary, `shrink-0` on name/status/ + result summary. `LazyCollapse` may need a `min-w-0` on its title flex row. +- Accessible label: `Toggle tool call <name>`. + +### Step 4 — Wire into `ChatView.svelte` +- Delete the `tool-call` / `tool-result` branches of `chunkRow` (they can no longer reach it). +- Delete the batched-tool branch. A `tool` group renders ONE bubble (same + `chat chat-start [&>.chat-bubble]:max-w-5xl [&>.chat-bubble]:p-0` shim) containing one + `<ToolCard {entry}/>`. +- Key update in `keyedRows`: `t<toolCallId>` per Step 1. +- Update `ui.test.ts` / `ChatView.test.ts`: one bubble + one card per call (parallel calls → + separate bubbles); header contains call + result summary; no standalone result card; pending + spinner; ✓/✗ status icon; error tint; orphan result card; collapsed by default. + +### Step 5 — Verify + live check +`bun install`, `bun run typecheck`, `bun run test` (×2), `bun run check`, `bun run build`. Then a +human visual pass via `bin/up` from `worktrees/tool-call-cards/` (stop the main `bin/up` first — +same ports): a turn with a single shell call, parallel calls (separate bubbles), an error, an edit, +a long output (6 + toggle + 6), a streaming (pending) call, and a reload mid-turn (seal keeps open +state). + +No wire/contract change → no `backend-handoff.md` ask. Update `GLOSSARY.md` only if a new term is +coined. + +--- + +## 4. Optional backend follow-up (not required) +Result summaries are heuristics over strings. If richer headers are wanted later (real exit code, +match counts, diff stats), the backend could add an optional structured `details` object to +`ToolResultChunk` (as oh-my-pi does). That would be a wire change via `backend-handoff.md` — out of +scope for this feature. + +## 5. Open questions +All resolved — see **DECISIONS** at the top. |