Skip to content

feat(channels): show tool execution progress in channel replies#2792

Merged
houko merged 1 commit into
mainfrom
feat/channel-progress-events
Apr 20, 2026
Merged

feat(channels): show tool execution progress in channel replies#2792
houko merged 1 commit into
mainfrom
feat/channel-progress-events

Conversation

@houko

@houko houko commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Surface tool calls (🔧 web_search) and failures (⚠️ tool failed) in the user's channel reply so they see what the agent is doing instead of waiting silently for the final answer
  • Works on all ~50 channel adapters with zero per-adapter code changes — the injection happens at the shared bridge layer
  • Telegram (the only supports_streaming=true adapter) shows progress via in-place message edits as it happens; Discord/Slack/Matrix/QQ/Feishu/etc. show the same content as one consolidated message after the agent completes
  • New trait method send_message_streaming_with_sender_status exposes the kernel's terminal Result to the bridge, fixing four collateral regressions an earlier draft had: wrong record_delivery success metric, missing Error lifecycle reaction, lost try_reresolution, and accidental error leakage on suppress_error_responses adapters (Mastodon)

Behavior

Before (Telegram, agent calls 2 tools):

(silent 8s)
Based on the search and weather data, Paris is 12°C.

After (Telegram, edits in place):

🔧 web_search

🔧 web_search

🔧 get_weather

🔧 web_search

🔧 get_weather

Based on the search and weather data, Paris is 12°C.

Discord/Slack/Matrix/etc.: same content, but delivered as one consolidated message after the agent finishes (still single message, not multiple).

Tool failures surface as ⚠️ \tool_name` failed`. Successful tools stay silent — the model's next prose iteration is signal enough, and emitting one line per tool gets noisy fast for agents that chain many calls.

Design notes

  • start_stream_text_bridge (api/channel_bridge.rs) is the shared event→text translator for all channels. It now injects formatted progress lines for ToolUseStart and failed ToolExecutionResult. PhaseChange is intentionally not surfaced — fires every iteration, too noisy.
  • last_progress_tool guard dedupes back-to-back duplicate ToolUseStart events some drivers emit.
  • A new helper start_stream_text_bridge_with_status returns the same text receiver plus a oneshot::Receiver<Result<(), String>> carrying the kernel's terminal status.
  • dispatch_message's non-streaming-adapter branch now routes through the streaming kernel call (via _status variant) so progress events reach those adapters too. Accumulates deltas, sends once via send_response so output_format and thread_id are still honored.
  • The status oneshot lets the bridge:
    • Emit AgentPhase::Done vs Error reaction correctly
    • record_delivery(success=...) with the real outcome
    • Journal status Completed vs Failed with the error string
    • Honor adapter.suppress_error_responses() so Mastodon (public-feed) does not leak sanitized agent errors to its timeline

Trade-offs

  • Telegram's buffered_text fallback path (used when send_streaming HTTP fails mid-stream) was not migrated to the new _status variant — same regression class as the non-streaming path had, but Telegram is not in the suppress_error_responses opt-in list, so impact is bounded. Future PR.
  • No agent.toml show_progress config — progress is on for all agents by default. Add later if anyone wants opt-out.

Test plan

  • cargo build --lib -p librefang-api -p librefang-channels green
  • cargo clippy --lib -p librefang-api -p librefang-channels -- -D warnings zero warnings
  • cargo clippy --tests --lib -p librefang-api -- -D warnings zero warnings
  • 18 channel_bridge tests pass (including 4 new progress tests + 3 new status tests)
  • 84 channels bridge tests pass (no regression)
  • Live integration test on Telegram (per CLAUDE.md): start daemon, send a real message that triggers tool use, verify progress lines appear in the live-edited Telegram message — not done in this PR, requires the user's daemon environment
  • Live integration test on a non-streaming adapter (e.g. Discord) — same caveat

Surface tool invocations and failures in the user's channel reply so they
see what the agent is doing — not a silent 8-second wait followed by a
final answer.

Implementation lives in two places:

1. start_stream_text_bridge (api/channel_bridge.rs) — translates
   StreamEvent::ToolUseStart into a "🔧 \`tool_name\`" line and
   StreamEvent::ToolExecutionResult { is_error: true } into a
   "⚠️ \`tool_name\` failed" line, injected into the text delta stream.
   Successful tools stay quiet (the model's next prose iteration is
   signal enough). PhaseChange events are not surfaced — they fire on
   every iteration and are too noisy. A last_progress_tool guard
   dedupes back-to-back duplicate ToolUseStart events.

2. dispatch_message non-streaming-adapter branch (channels/bridge.rs)
   — now routes through the streaming kernel call so progress events
   reach Discord/Slack/Matrix/QQ/Feishu/etc. too. Accumulates deltas
   and sends once via send_response so output_format and thread_id
   are still honored.

To do this safely, a new trait method send_message_streaming_with_sender_status
returns the kernel's terminal Result alongside the text receiver. The
new branch uses that status to:
  - emit AgentPhase::Done vs Error reaction correctly
  - record_delivery(success=...) with real outcome
  - journal status Completed vs Failed with error string
  - honor adapter.suppress_error_responses() so Mastodon (public-feed)
    does not leak sanitized agent errors to its timeline

Telegram (only supports_streaming=true adapter) keeps its existing
in-place edit behavior — the progress lines just become part of the
live-edited message. All ~49 other adapters get the progress for free
without any per-adapter code change because the injection happens at
the shared bridge layer.

Tests:
  - test_stream_bridge_surfaces_tool_use_progress: ToolUseStart
    becomes a visible "🔧" line followed by post-tool prose
  - test_stream_bridge_surfaces_tool_failure: failed tools emit
    "⚠️ name failed"
  - test_stream_bridge_quiet_on_tool_success: successful tools stay
    silent (no "✓" spam for chained tools)
  - test_stream_bridge_dedupes_consecutive_tool_progress: same tool
    name back-to-back emits one line, not three
  - test_stream_bridge_status_success / _status_error: oneshot
    correctly resolves to Ok/Err
  - test_stream_bridge_group_error_suppresses_text_but_reports_err:
    is_group=true keeps text out of the channel but the status
    oneshot still surfaces the error so metrics and reactions stay
    accurate
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions github-actions Bot added ready-for-review PR is ready for maintainer review area/channels Messaging channel adapters size/L 250-999 lines changed and removed ready-for-review PR is ready for maintainer review labels Apr 20, 2026
@houko
houko merged commit 64735f5 into main Apr 20, 2026
18 of 21 checks passed
@houko
houko deleted the feat/channel-progress-events branch April 20, 2026 05:36
houko added a commit that referenced this pull request Apr 20, 2026
Surface tool invocations and failures in the user's channel reply so they
see what the agent is doing — not a silent 8-second wait followed by a
final answer.

Implementation lives in two places:

1. start_stream_text_bridge (api/channel_bridge.rs) — translates
   StreamEvent::ToolUseStart into a "🔧 \`tool_name\`" line and
   StreamEvent::ToolExecutionResult { is_error: true } into a
   "⚠️ \`tool_name\` failed" line, injected into the text delta stream.
   Successful tools stay quiet (the model's next prose iteration is
   signal enough). PhaseChange events are not surfaced — they fire on
   every iteration and are too noisy. A last_progress_tool guard
   dedupes back-to-back duplicate ToolUseStart events.

2. dispatch_message non-streaming-adapter branch (channels/bridge.rs)
   — now routes through the streaming kernel call so progress events
   reach Discord/Slack/Matrix/QQ/Feishu/etc. too. Accumulates deltas
   and sends once via send_response so output_format and thread_id
   are still honored.

To do this safely, a new trait method send_message_streaming_with_sender_status
returns the kernel's terminal Result alongside the text receiver. The
new branch uses that status to:
  - emit AgentPhase::Done vs Error reaction correctly
  - record_delivery(success=...) with real outcome
  - journal status Completed vs Failed with error string
  - honor adapter.suppress_error_responses() so Mastodon (public-feed)
    does not leak sanitized agent errors to its timeline

Telegram (only supports_streaming=true adapter) keeps its existing
in-place edit behavior — the progress lines just become part of the
live-edited message. All ~49 other adapters get the progress for free
without any per-adapter code change because the injection happens at
the shared bridge layer.

Tests:
  - test_stream_bridge_surfaces_tool_use_progress: ToolUseStart
    becomes a visible "🔧" line followed by post-tool prose
  - test_stream_bridge_surfaces_tool_failure: failed tools emit
    "⚠️ name failed"
  - test_stream_bridge_quiet_on_tool_success: successful tools stay
    silent (no "✓" spam for chained tools)
  - test_stream_bridge_dedupes_consecutive_tool_progress: same tool
    name back-to-back emits one line, not three
  - test_stream_bridge_status_success / _status_error: oneshot
    correctly resolves to Ok/Err
  - test_stream_bridge_group_error_suppresses_text_but_reports_err:
    is_group=true keeps text out of the channel but the status
    oneshot still surfaces the error so metrics and reactions stay
    accurate
houko added a commit that referenced this pull request Apr 20, 2026
…gress config, integration test

Three follow-ups to #2792 (channel progress markers):

1. Telegram (streaming-adapter) buffered_text fallback now uses the
   `_status` variant. The same regression class fixed for non-streaming
   adapters in V1 was still latent on the Telegram path:
     - send_streaming Ok + kernel Err → Done reaction + success=true
     - send_streaming Err + buffered → Done reaction + success=true
   Both now correctly emit Error reaction, record_delivery(false), and
   journal Failed when the kernel actually errored. The fallback path
   also honors `suppress_error_responses` when the buffered text is a
   sanitized error (defense-in-depth — Telegram is not in the opt-in
   list today, but the path is now uniform across all adapters).

2. New `agent.toml show_progress` field (default true) plumbed through
   the streaming bridge. When false, the bridge skips both the
   `🔧 tool_name` and `⚠️ tool_name failed` injections — useful for
   agents whose output is parsed downstream, or for pristine-output
   scenarios where status markers would leak into the response. The
   trait-impl side looks up the agent's manifest from the kernel
   registry once per dispatch and passes it through to
   `start_stream_text_bridge[_with_status]`.

3. New end-to-end integration test in
   `crates/librefang-channels/tests/bridge_integration_test.rs` that
   wires a real `BridgeManager` + `MockAdapter` (non-streaming) +
   `MockProgressHandle` (override of
   `send_message_streaming_with_sender_status` that synthesises a
   delta stream with progress markers). Verifies the V2 dispatch
   pipeline actually surfaces progress to non-streaming adapters
   end-to-end via the consolidated `send_response` call.

Tests:
  - test_stream_bridge_show_progress_false_suppresses_all_markers:
    show_progress=false produces no 🔧/⚠️ markers but still flows the
    actual model prose through
  - test_bridge_non_streaming_adapter_sees_progress_markers: full
    BridgeManager → MockAdapter pipeline; verifies progress markers
    end up in the captured `send()` call

Notes:
  - True "live daemon" integration test (per CLAUDE.md) requires
    LLM API keys + a configured channel adapter (Telegram bot token,
    etc.) which the daemon environment does not currently have. The
    in-process integration test exercises the full dispatch wiring
    using real tokio tasks/channels and a mock kernel handle.
houko added a commit that referenced this pull request Apr 20, 2026
* feat(embedding): add native Cohere driver

Cohere was advertised in the embedding warning text and `detect_embedding_provider`
list, but `create_embedding_driver` had no matching branch — setting
`embedding_provider = "cohere"` fell through to the generic OpenAI-compatible
driver, which built a broken `https://cohere/v1` URL and failed DNS. Users
following the on-screen instructions to set `COHERE_API_KEY` silently got no
embeddings.

Add a dedicated `CohereEmbeddingDriver` that speaks Cohere's native API:
- Endpoint `/v1/embed` (not `/v1/embeddings`)
- Request body `{ texts, model, input_type }` instead of OpenAI's `{ input, model }`
- `input_type` hardcoded to `search_document` — the right choice for indexing
  memory/documents, which is the primary embedding path today
- Batch limit of 96 texts per request, failing fast with a clear error

Wire it into `create_embedding_driver` before the OpenAI fall-through, mirroring
the Bedrock special-case. When the caller hands us an OpenAI-style model name
(common when auto-detect fires on a config left at the default
`text-embedding-3-small`), transparently remap to `embed-english-v3.0` with a
warning so users notice and can pin a specific Cohere model in config.

Dimensions are inferred from the official Cohere model list (1024 for v3 full,
384 for v3 light, matching v2 variants).

* refactor(embedding): add InvalidInput variant and make Cohere input_type overridable

Two small cleanups flagged in review of the Cohere driver:

1. Batch-over-limit was returning `EmbeddingError::Unsupported`, which is
   semantically meant for "driver doesn't implement this feature" (e.g.
   image embeddings). A caller sending 200 texts isn't asking for an
   unsupported feature — they're just wrong about the size. Add an
   `InvalidInput(String)` variant and route the batch check through it.

2. `input_type` was hardcoded to `search_document`. That's the right
   default for indexing-dominant workloads (memory/document ingest), but
   query-time and clustering callers would silently get the wrong
   semantic bucket. Add a `LIBREFANG_COHERE_INPUT_TYPE` env var override
   with whitelist validation against the four valid v3 values
   (`search_document`, `search_query`, `classification`, `clustering`).
   Invalid values log a warning and fall back to the default rather than
   pass through — otherwise Cohere returns a cryptic 400 with no
   context.

Env var (rather than a plumbed-through config field) is intentional: it
avoids adding a Cohere-specific knob to the shared `EmbeddingConfig` or
`create_embedding_driver` signature, and it's the kind of setting that
typically gets flipped per-deployment, not per-user.

* refactor(embedding): migrate Cohere driver to v2 API and honor catalog base_url

Two related fixes, both prompted by reconciling this PR with the existing
`librefang-registry` entry for Cohere:

1. **Migrate CohereEmbeddingDriver from `/v1/embed` to `/v2/embed`.**
   `librefang-llm-drivers` already targets `api.cohere.com/v2` for chat,
   and the registry ships `base_url = "https://api.cohere.com/v2"`, so the
   v1 default in this driver was a version split waiting to bite. v2 also
   changes the response shape — `embeddings` becomes a dict keyed by
   `embedding_types` entry (`{ "embeddings": { "float": [[...]] } }`)
   rather than a flat array. Request now sends `embedding_types: ["float"]`
   (the only representation we consume), response is deserialized via a
   new `CohereEmbeddingsByType` intermediate struct.

2. **Plumb catalog-derived `base_url` to `create_embedding_driver`.**
   Both embedding-driver construction paths in `kernel::boot_with_config`
   (explicit `[memory].embedding_provider` and auto-detect) were reading
   `config.provider_urls` directly, which only contains user overrides
   from `config.toml` and ignores the registry-synced catalog. That meant
   a provider like Cohere whose registry entry pins `v2` silently lost
   that URL and fell back to the hardcoded default in
   `create_embedding_driver`. Now we read from `model_catalog` first
   (which already has `provider_urls` overrides applied earlier during
   boot), fall through to raw `provider_urls`, and only as a last resort
   hit the per-provider hardcoded default. Net effect: registry edits to
   any provider's `base_url` now actually reach every embedding driver,
   not just OpenAI-compatible ones.

Test fixtures updated to v2 URLs.

* refactor(embedding): remove cloud provider URL defaults, require catalog plumbing

Hardcoded base_url defaults for cloud providers (openai, openrouter, groq,
together, fireworks, mistral, cohere, plus the OpenAI-compatible "other"
fallback) are the exact bug class this PR is trying to eliminate: a stale
baked-in URL silently overrides whatever the registry ships, and when the
registry bumps a version (Cohere v1 → v2 being the concrete example that
prompted this PR) nothing in the code breaks — the wrong URL just keeps
getting used.

Now that `kernel::boot_with_config` plumbs `model_catalog`'s base_url
through to `create_embedding_driver`, these defaults serve no real purpose
and just re-introduce the drift risk. Remove them. If a cloud provider
shows up at `create_embedding_driver` without a caller-supplied base_url,
return `EmbeddingError::InvalidInput` pointing the operator at either the
catalog TOML or `config.toml`'s `provider_urls` — far better than
fabricating a URL that might be wrong.

Local providers (`ollama`, `vllm`, `lmstudio`) keep their hardcoded
defaults: their `localhost:PORT` URLs aren't registry-tracked, the ports
are stable conventions, and the fallback is useful for direct/test
callers that construct drivers without a kernel.

Tests updated:
- Cohere-with-key and Cohere-model-remap now pass an explicit v2 base_url
  (they previously relied on the deleted Cohere fallback).
- New test `test_create_embedding_driver_cloud_provider_requires_base_url`
  pins the "cloud without URL errors out" behavior so it can't regress.

* chore(embedding): stop advertising Groq as an embedding provider

Groq's API has no `/v1/embeddings` endpoint — `api.groq.com/openai/v1/models`
returns only chat completions and Whisper, and a real POST with any embedding
model name 404s with `model_not_found`. But the codebase kept listing Groq
as a valid embedding provider in three places, which meant a user setting
only `GROQ_API_KEY` would hit the auto-detect happy path, build an
`OpenAIEmbeddingDriver`, and then get silent 404s at the first memory
operation — with no clear indication that Groq was the wrong pick.

Fix the three misdirections:

1. Remove `("GROQ_API_KEY", "groq")` from `detect_embedding_provider`'s
   priority list. Auto-detect now skips Groq; users who set only
   `GROQ_API_KEY` correctly see "no embedding provider available" with
   actionable next steps.
2. Update the "No embedding provider" error/warn text in both
   `embedding.rs` and `kernel/mod.rs` to drop `GROQ_API_KEY` and add an
   explicit parenthetical ("Groq does not expose an embeddings endpoint")
   so users who were hoping Groq would work don't go in circles.
3. Update the module-level docstring and `OpenAIEmbeddingDriver` doc to
   remove Groq from the "works with" list and point to
   `CohereEmbeddingDriver` for Cohere's non-OpenAI-compatible path.

The `kernel::boot_with_config` auto-detect `match detected` arm for `"groq"`
is removed in the same change — it's now unreachable since detect never
returns `"groq"`.

Unrelated lookups that still mention Groq (e.g. `provider_default_key_env`
and the `needs_v1` URL-normalization match) are intentionally left alone:
they fire only when a user explicitly sets `embedding_provider = "groq"`,
which is user-error territory and will surface as a legitimate 404 from
Groq's API with its real error body — far more discoverable than the
previous silent auto-pick path.

* test(embedding): pin `detect_embedding_provider` priority, especially Groq exclusion

`detect_embedding_provider` had zero test coverage. The previous commit
removed Groq from its priority list because Groq has no embeddings
endpoint, but there was nothing stopping a future "completeness" PR from
adding it back and silently reintroducing the 404 bug.

Add four tests that lock the behavior in:

1. `test_detect_embedding_provider_ignores_groq` — with only GROQ_API_KEY
   set, detect returns None. This is the regression guard for the Groq
   removal.
2. `test_detect_embedding_provider_picks_cohere_when_only_cohere_set` —
   exercises the Cohere arm added earlier in this PR.
3. `test_detect_embedding_provider_priority_openai_beats_cohere` — pins
   OpenAI at the top of the priority list so reordering can't quietly
   change auto-detect behavior for anyone who has multiple keys set.
4. `test_detect_embedding_provider_none_when_nothing_set` — baseline:
   empty env returns None.

Each test uses `clear_detect_env`/`restore_detect_env` helpers to avoid
leaking host env state between tests or with other tests that set these
vars (the existing Cohere tests do).

* fix(embedding): drop `.unwrap_err()` on results whose Ok type lacks Debug

Ubuntu/macOS/Windows CI all failed to compile with:
- `CohereEmbeddingDriver` doesn't implement `Debug`
- `dyn EmbeddingDriver + Send + Sync` doesn't implement `Debug`

Both come from `.unwrap_err()` — which requires the Ok type to implement
Debug so it can panic-print on an unexpected Ok. The drivers deliberately
don't derive Debug (would leak api_key through the struct field), and
`Box<dyn EmbeddingDriver>` can't derive Debug unless the trait gains a
Debug supertrait.

Rewrite the two affected tests to match on the `Result` directly via
`matches!(&result, Err(...))`, printing `result.as_ref().err()` on failure
so only `EmbeddingError: Debug` is required.

* chore(embedding): address self-review nits — broader cloud-URL test, Cohere input_type docstring

Follow-up polish from reviewing the Cohere driver additions against the
rest of this PR:

1. **Broaden `test_create_embedding_driver_cloud_provider_requires_base_url`.**
   The original test was named for "cloud providers" but only exercised
   Cohere. Rename to the plural form and loop across every cloud provider
   that should refuse construction without a base_url (openai, openrouter,
   mistral, together, fireworks, cohere). Cohere has an earlier api_key
   check so we seed COHERE_API_KEY to force the URL branch to fire.

2. **Clarify `CohereEmbeddingDriver.input_type` docstring.** The previous
   "v3 models" wording could be read as a Cohere v3 API version when it
   meant the `embed-*` v3 model family, and it didn't flag the asymmetric-
   retrieval quality implication at all. Rewrite to spell out:
   - Cohere's asymmetric retrieval contract (`search_document` at index
     time, `search_query` at query time, quality measurably better),
   - why librefang pins a single value per driver (the
     `EmbeddingDriver` trait has no indexing/querying distinction today),
   - why `search_document` is the right single-value default here (memory
     ingest dominates),
   - how to override for query/clustering-heavy deployments
     (`LIBREFANG_COHERE_INPUT_TYPE` env var),
   - that a per-call override needs a trait change and is tracked as
     follow-up.

* fix(api): route comms_task through kernel wrapper; surface task system events (#2789)

* fix(api): route comms_task through kernel wrapper; surface task system events in comms feed

Two related fixes for the task event bus integration (#2788):

1. `POST /api/comms/task` called `memory_substrate().task_post()` directly,
   bypassing `Kernel::task_post` which publishes `SystemEvent::TaskPosted`.
   Tasks created via HTTP never hit the event bus, so triggers with
   `pattern: "TaskPosted"` didn't fire for UI-posted tasks. Route through
   the `KernelHandle::task_post` wrapper instead.

2. Extend `filter_to_comms_event` to map `SystemEvent::TaskPosted`,
   `TaskClaimed`, and `TaskCompleted` to their corresponding
   `CommsEventKind` variants (which already existed) so they appear in
   `GET /api/comms/events` alongside messages and lifecycle events.

* fix(api,kernel): thread completer identity through task_complete

Addresses houko's review on #2789: SystemEvent::TaskCompleted was
constructed with Event::new(AgentId::new(), …), so event.source was a
freshly-minted random UUID. The comms feed rendered that UUID as the
completer, and resolve_name fell back to printing it verbatim.

Proper fix (parallel to task_claim/TaskClaimed):
- Add completed_by: String to SystemEvent::TaskCompleted payload.
- KernelHandle::task_complete gains an agent_id parameter; kernel impl
  resolves it via AgentId::from_str → registry.find_by_name, matching
  task_claim's resolution path.
- tool_task_complete now requires caller_agent_id and forwards it.
- filter_to_comms_event populates source_id/source_name from
  completed_by instead of the bogus event.source UUID.
- triggers.rs describe-path renders "completed by {agent}" too.

Test doubles and the triggers match arm updated for the new field.
Workspace build + cargo test -p librefang-api -p librefang-runtime
(1035 passed) clean; clippy -D warnings clean.

* feat(dashboard): add stop button to interrupt in-flight agent streams (#2787)

* feat(dashboard): add stop button to interrupt in-flight agent streams

The backend already exposes POST /api/agents/{id}/stop (wired to the
kernel's AbortHandle in running_tasks), and the client helper
stopAgent() existed in api.ts, but no UI surfaced it — users had no
way to cancel a long-running agent loop from the chat view.

While a response is streaming, the Send button now morphs into a red
Stop button (Square icon, same position). Clicking it calls stopAgent
for the current agent and optimistically finalizes the streaming
message + releases the per-agent loading flag, so the textarea, voice
button, and Send button re-enable immediately without waiting for the
WebSocket to emit a terminal event. Late response/error frames from
the backend still reconcile the message content via the existing
handlers.

Adds chat.stop_hint translations for en and zh.

* fix(dashboard): prevent stop button from triggering HTTP retry + route via mutation hook

Lift per-turn lifecycle (responded, fallbackTimer, cleanup) into an
activeTurnsRef keyed by agent id so stopMessage can cancel the 180s/30s
watchdog that would otherwise re-send the user's message via HTTP.

Add useStopAgent mutation hook and route ChatPage through it per
dashboard CLAUDE.md convention.

Addresses review on PR #2787.

* feat(channels): show tool execution progress in channel replies (#2792)

Surface tool invocations and failures in the user's channel reply so they
see what the agent is doing — not a silent 8-second wait followed by a
final answer.

Implementation lives in two places:

1. start_stream_text_bridge (api/channel_bridge.rs) — translates
   StreamEvent::ToolUseStart into a "🔧 \`tool_name\`" line and
   StreamEvent::ToolExecutionResult { is_error: true } into a
   "⚠️ \`tool_name\` failed" line, injected into the text delta stream.
   Successful tools stay quiet (the model's next prose iteration is
   signal enough). PhaseChange events are not surfaced — they fire on
   every iteration and are too noisy. A last_progress_tool guard
   dedupes back-to-back duplicate ToolUseStart events.

2. dispatch_message non-streaming-adapter branch (channels/bridge.rs)
   — now routes through the streaming kernel call so progress events
   reach Discord/Slack/Matrix/QQ/Feishu/etc. too. Accumulates deltas
   and sends once via send_response so output_format and thread_id
   are still honored.

To do this safely, a new trait method send_message_streaming_with_sender_status
returns the kernel's terminal Result alongside the text receiver. The
new branch uses that status to:
  - emit AgentPhase::Done vs Error reaction correctly
  - record_delivery(success=...) with real outcome
  - journal status Completed vs Failed with error string
  - honor adapter.suppress_error_responses() so Mastodon (public-feed)
    does not leak sanitized agent errors to its timeline

Telegram (only supports_streaming=true adapter) keeps its existing
in-place edit behavior — the progress lines just become part of the
live-edited message. All ~49 other adapters get the progress for free
without any per-adapter code change because the injection happens at
the shared bridge layer.

Tests:
  - test_stream_bridge_surfaces_tool_use_progress: ToolUseStart
    becomes a visible "🔧" line followed by post-tool prose
  - test_stream_bridge_surfaces_tool_failure: failed tools emit
    "⚠️ name failed"
  - test_stream_bridge_quiet_on_tool_success: successful tools stay
    silent (no "✓" spam for chained tools)
  - test_stream_bridge_dedupes_consecutive_tool_progress: same tool
    name back-to-back emits one line, not three
  - test_stream_bridge_status_success / _status_error: oneshot
    correctly resolves to Ok/Err
  - test_stream_bridge_group_error_suppresses_text_but_reports_err:
    is_group=true keeps text out of the channel but the status
    oneshot still surfaces the error so metrics and reactions stay
    accurate

* fix(embedding): default Cohere auto-fallback to multilingual v3

The auto-fallback that kicks in when `[memory].embedding_model` isn't a
`embed-*` name was hardcoded to `embed-english-v3.0`. For non-English
deployments (librefang is used in Chinese/Japanese/Korean workflows)
that silently embeds multilingual corpora with an English-only model,
which degrades retrieval quality in ways the user can't see until they
notice worse search results.

`embed-multilingual-v3.0` is the right default:
- Same 1024 dims as `embed-english-v3.0` (infer_cohere_dimensions
  already handles both; no code path changes)
- Same per-token price
- Supports 100+ languages including English (English quality is only
  marginally lower than the English-specialized model)

The warning text is updated to point users at `embed-english-v3.0` and
the `*-light-v3.0` variants as deliberate alternatives, so English-only
deployments can opt back in explicitly rather than drift into the
multilingual default.

* chore(embedding): sync test comment with multilingual-v3 fallback

Previous commit changed the Cohere auto-fallback from
`embed-english-v3.0` to `embed-multilingual-v3.0` but left the test's
doc-comment describing the old behavior. The assertion itself still
passes (both models are 1024 dims), so CI was silent — but the comment
was now actively lying to future readers. Update the comment and
explicitly note that the assertion checks dimensions only, with the
actual fallback target pinned by the warn!() log line rather than the
test.

---------

Co-authored-by: Vignesh Jagadeesh <[email protected]>
houko added a commit that referenced this pull request Apr 20, 2026
…m fix, show_progress, i18n, prettify, dashboard parity (#2793)

* feat(channels): channel-progress v2 — Telegram fallback fix, show_progress config, integration test

Three follow-ups to #2792 (channel progress markers):

1. Telegram (streaming-adapter) buffered_text fallback now uses the
   `_status` variant. The same regression class fixed for non-streaming
   adapters in V1 was still latent on the Telegram path:
     - send_streaming Ok + kernel Err → Done reaction + success=true
     - send_streaming Err + buffered → Done reaction + success=true
   Both now correctly emit Error reaction, record_delivery(false), and
   journal Failed when the kernel actually errored. The fallback path
   also honors `suppress_error_responses` when the buffered text is a
   sanitized error (defense-in-depth — Telegram is not in the opt-in
   list today, but the path is now uniform across all adapters).

2. New `agent.toml show_progress` field (default true) plumbed through
   the streaming bridge. When false, the bridge skips both the
   `🔧 tool_name` and `⚠️ tool_name failed` injections — useful for
   agents whose output is parsed downstream, or for pristine-output
   scenarios where status markers would leak into the response. The
   trait-impl side looks up the agent's manifest from the kernel
   registry once per dispatch and passes it through to
   `start_stream_text_bridge[_with_status]`.

3. New end-to-end integration test in
   `crates/librefang-channels/tests/bridge_integration_test.rs` that
   wires a real `BridgeManager` + `MockAdapter` (non-streaming) +
   `MockProgressHandle` (override of
   `send_message_streaming_with_sender_status` that synthesises a
   delta stream with progress markers). Verifies the V2 dispatch
   pipeline actually surfaces progress to non-streaming adapters
   end-to-end via the consolidated `send_response` call.

Tests:
  - test_stream_bridge_show_progress_false_suppresses_all_markers:
    show_progress=false produces no 🔧/⚠️ markers but still flows the
    actual model prose through
  - test_bridge_non_streaming_adapter_sees_progress_markers: full
    BridgeManager → MockAdapter pipeline; verifies progress markers
    end up in the captured `send()` call

Notes:
  - True "live daemon" integration test (per CLAUDE.md) requires
    LLM API keys + a configured channel adapter (Telegram bot token,
    etc.) which the daemon environment does not currently have. The
    in-process integration test exercises the full dispatch wiring
    using real tokio tasks/channels and a mock kernel handle.

* fix(kernel/wizard): add show_progress to AgentManifest struct literal

CI compile error: wizard.rs constructed AgentManifest with an explicit
struct literal listing every field, so adding show_progress in V2 broke
that one site (the wizard agent created via setup intent).

All other AgentManifest constructors use ..Default::default() and pick
up show_progress=true automatically; only this one was a full-literal.

* feat(channels): channel-progress v3 — collapse, i18n, prettify, dashboard parity, smoke script

Six follow-ups bundled per request to keep the channel-progress effort in
one PR. Done from #2 onward; #1 (per-OutputFormat backtick adaptation)
was investigated and dropped — backtick renders correctly in TelegramHtml
(<code>), SlackMrkdwn, Discord/Matrix Markdown (inline code), and the
only adapter that strips backticks (PlainText for Mastodon) is already
opted out of the progress path via suppress_error_responses.

#2 buffered_text fallback test: new integration test
   test_bridge_streaming_adapter_kernel_and_transport_both_fail covers
   the V2 4th outcome (send_streaming Err + kernel Err).

#3 surface context_warning PhaseChange so the user sees when the agent's
   context window was trimmed/overflowed. Other phases stay SSE-only.

#4 collapse repeated tool calls within an iteration — replace
   last_progress_tool with iter_tools_seen HashSet cleared at every
   ContentComplete. Batch agents no longer spam "🔧 web_search" per
   parallel call; retries in a later iteration still get a fresh line.

#5 i18n for the "failed" suffix — supports en/zh-CN/es/ja/de/fr (matches
   librefang_types::i18n). Language threaded from
   kernel.config_snapshot().language through start_stream_text_bridge.

#6 prettify tool names — web_search → Web Search; MCP_call → MCP Call
   (preserves internal caps). Backticks dropped from progress lines
   since the prettified form is now the identity.

#8 dashboard ToolCallCard parity — mirror prettifyToolName() in
   dashboard/src/lib/string.ts and apply it in the card header so
   chat reply and dashboard show the same rendering.

#7 live-integration smoke script — scripts/tests/channel_progress_smoke.sh
   automates the daemon-side flow once the user supplies an LLM API key
   and a configured channel adapter. Documents the channel-delivery gap
   (needs an external webhook receiver). No driver injection hook added
   to the kernel — that would have been scope creep.

Tests added/updated:
  - test_prettify_tool_name_snake_to_title / _kebab_and_dotted /
    _preserves_internal_caps
  - test_tr_progress_failed_languages
  - test_bridge_streaming_adapter_kernel_and_transport_both_fail
  - Updated existing assertions to expect prettified names

* fix(channels): correct success/err pairing + treat timeout as soft success + clippy collapsible_if

Three review-driven fixes for the V3 PR:

Bug 1 — Telegram path outcome 3 (send_streaming Err + kernel Ok) was
recording delivery as success=true with err=Some(stream_error). The
fallback send_response had already delivered the buffered text and the
kernel succeeded, so the transport-side stream error is irrelevant to
delivery accounting — keeping it in the err field produced a
contradictory metric (success AND err). Now: err is Some only when
kernel actually failed.

Bug 2 — TIMEOUT_PARTIAL_OUTPUT_MARKER was being mapped to status=Err,
which flipped the lifecycle reaction to Error and record_delivery to
success=false. Pre-V2 the bridge had no status channel and treated
these turns as Done because the model emitted useful partial output
before the inactivity timer fired. Restore that semantics: status=Ok
for timeouts. The user still sees the "[Task timed out…]" tail
appended to their reply.

Clippy collapsible_if — flatten the inner if show_progress && ... into
match guards on the ToolExecutionResult and PhaseChange arms (CI
clippy -D warnings caught this in run 24651228831).

* chore(channels): drop dead 'let _ = e' annotation

The variable 'e' (stream transport error) is already used at warn! line 2886
and the empty-buffer fallback at line 2953, so the explicit underscore-binding
introduced by the Bug 1 fix is noise — clean it up.

* test(channels): regression coverage for Bug 1 (success/err pairing) + Bug 2 (timeout-as-success)

Both bugs were caught by review, not tests — add the missing coverage so
they cannot silently regress.

Bug 2 unit test: test_stream_bridge_timeout_partial_output_reports_ok_status
Constructs a kernel handle that fails with a string containing
TIMEOUT_PARTIAL_OUTPUT_MARKER. Asserts:
  - The user-facing text channel still receives the "[Task timed out…]"
    tail so the user knows the reply may be incomplete.
  - The status oneshot resolves to Ok(()) — NOT Err — so bridge.rs
    drives the lifecycle reaction to Done and record_delivery to
    success=true. Pre-V2 semantics preserved.

Bug 1 integration test: test_bridge_streaming_adapter_kernel_ok_transport_fail_records_clean_success
Adds MockKernelOkHandle (overrides send_message_streaming_with_sender_status
to emit clean text + status=Ok, AND overrides record_delivery to capture
every (success, err) pair). Combined with the existing
MockFailingStreamingAdapter (always Err on send_streaming), the test
exercises Telegram outcome 3:
  - Fallback send_response delivers the buffered text  ✓
  - record_delivery is called with success=true AND err=None — proving
    the transport-side stream error does NOT leak into the err field
    when the kernel itself succeeded. (Pre-fix this would have been
    success=true, err=Some(stream_e) — a contradictory metric.)

* polish(channels): unify progress-line spacing, codepoint-safe dashboard prettify, smoke script auto-spawn

Three minor follow-ups from review:

#3 — Symmetric blank lines around progress markers.
  ToolUseStart used \n\n…\n; ToolExecutionResult and PhaseChange used
  \n…\n. Adjacent markers (e.g. 🔧 X right before ⚠️ X failed) ended up
  on consecutive lines without a blank separator and many markdown
  renderers collapsed them into one paragraph. All three now use
  \n\n…\n\n so every renderer that respects markdown blank-line
  semantics shows them as separate blocks.

#4 — prettifyToolName() in dashboard/src/lib/string.ts now iterates by
  Unicode codepoint (via spread) instead of UTF-16 unit. A tool name
  starting with a non-BMP character (e.g. emoji) no longer drops its
  surrogate half when uppercased. Mirrors the Rust-side prettifier in
  channel_bridge.rs which uses chars().next() (also codepoint-correct).
  Tool names are usually ASCII so this is forward-looking, not a fix.

#5 — channel_progress_smoke.sh now auto-spawns a temporary agent when
  the daemon has none. Picks the first available LLM key
  (GROQ/OPENAI/ANTHROPIC/MINIMAX) for the provider, sends a minimal
  manifest_toml inline (the SpawnRequest schema requires either
  manifest_toml or template — name alone isn't valid), and despawns
  the agent on exit. Pre-existing agents are still reused untouched.

* fix(channels/tests): extract DeliveryLog type alias to satisfy clippy::type_complexity

CI clippy -D warnings rejected Arc<Mutex<Vec<(bool, Option<String>)>>>
as too complex. Hoist into a named alias — same shape, named contract.
@houko houko mentioned this pull request Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/channels Messaging channel adapters size/L 250-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant