Skip to content

feat(chat): support PDF and text/code file attachments end-to-end#3094

Merged
houko merged 5 commits into
mainfrom
feat/multi-format-chat-attachments
Apr 25, 2026
Merged

feat(chat): support PDF and text/code file attachments end-to-end#3094
houko merged 5 commits into
mainfrom
feat/multi-format-chat-attachments

Conversation

@houko

@houko houko commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Wire PDF + text/code file uploads all the way to the LLM. Before this PR, dashboard's attach picker only accepted images; uploading a .md or .pdf returned 200 OK but resolve_attachments() silently dropped it, so the model replied "please paste the content you want me to summarize."
  • Add librefang_runtime::pdf_text (panic-isolated pdf-extract wrapper, 200K-char cap) and a new text-like branch in resolve_attachments() covering text/*, application/{json,xml,yaml,toml,…}, plus extension fallback for code files browsers tag with empty / application/octet-stream content_type (.rs, .py, .go, .ts, …).
  • Cross-driver fix in session_repair: after merging same-role messages, run a new coalesce_adjacent_text_blocks pass so the resulting Blocks contain at most one Text run plus image / tool / thinking blocks. Without this, small chat-tuned models behind Ollama / llama.cpp / vLLM / LM Studio attended only to the first or last Text part of a multi-part user message and dropped the attachment. Doing it at the repair layer means every driver downstream gets a normalized shape — no per-driver special case.
  • Diagnostics that made the bug obvious: WebSocket conn_id + client_ip + explicit_session on connect, reason + duration_secs + close_code on disconnect; messages_before / messages_after on the session-repair WARN; per-merge DEBUG with role + char counts; INFO at every successful attachment resolve / inject.
  • Dashboard: ATTACHMENT_ACCEPT widened to the full MIME + extension set, isSupportedFile() mirrors backend's is_text_like_attachment() so drag/paste cannot bypass with an unsupported type, attach + mic + send buttons switched to items-start so they pin to the textarea top when it grows multi-line.
  • Drive-by tsc cleanup (separate commit): PageHeader badge prop and SchemaForm's boolean | undefined coercion — both block any dashboard build today.

Why now

Reported by a user uploading a Chinese-named markdown to gemma4 via Ollama: 200 OK upload + 4096 in / 100 out LLM call + "please paste the content" reply. Three independent bugs stacked: backend silently dropped non-image, is_allowed_content_type rejected most code MIMEs anyway, multi-part user content broke local small models. All three are addressed.

Test plan

  • cargo check -p librefang-runtime -p librefang-llm-drivers -p librefang-api --lib
  • cargo test -p librefang-runtime --lib pdf_text — empty / non-PDF garbage panic-resistance
  • cargo test -p librefang-runtime --lib coalesce — 4 cases (consecutive merge, image boundary, single block, empty)
  • cargo test -p librefang-runtime --lib validate_and_repair_attachment — end-to-end attach + prompt → single coalesced Text in source order
  • Live dashboard test (just dev + ollama gemma4): upload .md + "总结一下" → real summary instead of "please paste the content"
  • CI green on all platforms
  • Manual: upload .pdf to a frontier model (anthropic / openai) — verify the extracted text appears in the assistant response and image-vision flows still work

houko added 2 commits April 25, 2026 14:38
Dashboard's attachment picker only allowed image/png|jpeg|webp|gif and
the agent loop's resolve_attachments() silently dropped anything else,
so uploading a .md or .pdf produced a 200 OK upload that the LLM never
saw — the model would respond "please paste the content you want me to
summarize" because the attachment text never reached the prompt.

This change wires the missing types end-to-end:

- librefang-runtime/pdf_text: new module wrapping
  pdf_extract::extract_text_from_mem with panic isolation (lopdf can
  panic on malformed/encrypted PDFs) and a 200K-char truncation cap
  so a 200-page report does not blow the context window.
- routes/agents::resolve_attachments: branches on image/* (existing),
  application/pdf (new — extract_text_from_pdf), and text-like files
  (new — covers text/*, application/{json,xml,yaml,toml,…}, plus
  extension fallback for code files browsers tag with empty or
  octet-stream content_type: .rs, .py, .go, .ts, .json, .toml, etc.).
  Unknown types now warn so future drops are diagnosable instead of
  silent. Adds INFO logs at every successful resolve / inject so it is
  obvious from daemon.log whether an upload made it into the LLM.
- routes/agents::is_allowed_content_type: extended exact-match list
  with json / yaml / toml / x-ipynb+json / sql / graphql / javascript
  / typescript, plus a text/* allow rule that explicitly excludes
  text/html and text/xml (XSS / XXE per the existing security
  rationale). All audio/image curated lists unchanged.
- session_repair: after merging consecutive same-role messages, run a
  new coalesce_adjacent_text_blocks pass per message so the resulting
  Blocks contain at most one Text run plus image/tool/thinking blocks.
  This matters because the inject_attachments_into_session +
  agent_loop user-text flow naturally produces Blocks([Text(attach),
  Text(prompt)]); small chat-tuned local models behind Ollama /
  llama.cpp / vLLM / LM Studio frequently attend only to the first or
  last Text part of a multi-part user message and drop the rest.
  Doing this at the repair layer means every driver downstream
  (Anthropic, OpenAI, Gemini, Groq, ChatGPT, …) gets a normalized
  shape for free — no per-driver special case.
- llm-drivers/openai: drop the obsolete is_ollama_like() flatten
  branch added during initial debugging; session_repair handles it
  generically now. Keep the "single Text part → plain string" fast
  path because some OAI-compatible backends mishandle 1-element
  arrays too.
- dashboard/ChatPage: ATTACHMENT_ACCEPT now lists the wider MIME +
  extension set; isSupportedFile mirrors backend's is_text_like
  decision so drag/paste cannot bypass with an unsupported type;
  attach + mic + send buttons switched to items-start so they stay
  pinned to the top of the textarea when it grows multi-line.
- ws.rs: WebSocket lifecycle gets per-connection conn_id (uuid) +
  client_ip + explicit_session on connect, and reason / duration_secs
  / close_code on disconnect, so concurrent reconnects (same
  agent_id, different conn_id) and idle-vs-explicit-close can be
  distinguished from the daemon log.
- agent_loop / session_repair: per-merge DEBUG with target_idx, role,
  char counts; WARN now carries messages_before / messages_after so
  you can see exactly how a 21-message session collapsed to 17.

Tests: pdf_text panic-resistance + truncation; session_repair coalesce
across single-block / image-boundary / text-only / empty cases; an
end-to-end test that simulates inject_attachments_into_session +
user-text-message + repair and asserts the LLM-bound Blocks contain a
single coalesced Text in source order.

Verified locally with just dev + ollama gemma4: uploading a Chinese
markdown file plus the prompt "总结一下" now produces a real summary
instead of "please paste the content".
The dashboard's tsc has been rejecting the build for a while because:

- PageHeader doesn't declare a `badge` prop, but ten pages
  (A2APage, AgentsPage, AnalyticsPage, ApprovalsPage, ChannelsPage,
  CommsPage, GoalsPage, HandsPage, RuntimePage, SchedulerPage,
  SessionsPage, SettingsPage, SkillsPage, TelemetryPage,
  WorkflowsPage) pass one. Add the optional prop and render it as a
  small label next to the title — matches the existing visual
  treatment in those pages.
- SchemaForm.validateSectionRequired / validateRequired pass
  `f.required` (boolean | undefined) to `hasInvalidNumberValue`,
  which expects boolean. Coerce with `?? false`.

Surfaced while building this PR; not strictly part of the
attachment work but the typecheck blocks any dashboard build.
@houko
houko force-pushed the feat/multi-format-chat-attachments branch from c077667 to 51ec03e Compare April 25, 2026 05:40

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c077667e90

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1065 to +1066
if existing_meta.is_none() {
*existing_meta = provider_metadata;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve per-part metadata when coalescing text blocks

coalesce_adjacent_text_blocks merges adjacent ContentBlock::Text entries but drops later provider_metadata whenever both sides are populated, because only the first metadata value is retained. Gemini uses per-part thought_signature in this field and expects those signatures to be echoed on subsequent turns, so a history containing adjacent Gemini text parts can lose required signatures during repair and cause follow-up requests to fail. This coalescing should either be limited to metadata-free attachment/user text or preserve metadata for every merged part.

Useful? React with 👍 / 👎.

}
Err(e) => {
tracing::warn!(file_id = %att.file_id, error = %e, "Failed to read upload for attachment");
} else if content_type == "application/pdf" {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Normalize PDF MIME before attachment type dispatch

resolve_attachments checks PDFs with an exact content_type == "application/pdf" match, but uploads are accepted after mime_base normalization and can legally include parameters (for example application/pdf; charset=binary). Those parameterized PDFs pass upload validation but miss this branch and are skipped, so the attachment text is silently dropped from model context. Use normalized MIME for dispatch here as well.

Useful? React with 👍 / 👎.

@github-actions github-actions Bot added size/L 250-999 lines changed ready-for-review PR is ready for maintainer review area/runtime Agent loop, LLM drivers, WASM sandbox labels Apr 25, 2026
github-actions Bot and others added 2 commits April 25, 2026 07:00
Two CI failures on this PR:

1. crates/librefang-runtime/src/pdf_text.rs:58 — clippy
   `explicit_counter_loop` lint (denied via `-D warnings`). Replace the
   manual `count` counter with `.enumerate()`. Behavior unchanged: the
   loop still pushes `MAX_PDF_TEXT_CHARS` chars and then appends the
   truncation marker.

2. crates/librefang-api/src/routes/agents.rs:4929 — unit test
   `test_upload_mime_allowlist_rejects_previously_accepted_types` listed
   `application/javascript` as an MIME that must be rejected, but this
   PR explicitly added it (and its siblings) to `EXTRA_ALLOWED_UPLOAD_TYPES`
   to support code-file attachments — the stated feature of the PR. The
   server stores uploads, never executes them, and the agent loop reads
   them as plain UTF-8 (per the doc-comment on `is_allowed_content_type`),
   so JS is safe to inline as a code attachment. Drop it from the rejected
   list. `text/html`, `text/xml`, `application/octet-stream` and the
   dangerous image subtypes remain blocked.
@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.

resolve_attachments() compared raw header values against exact MIMEs
("application/pdf", "image/..."). A request with charset parameters or
mixed case ("Application/PDF; charset=binary") would pass the upload
allowlist (which uses mime_base) but skip every branch in the resolver
and silently drop the attachment. Normalize via mime_base before
matching to keep the two paths consistent.
@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.

@houko
houko merged commit 8e8fe53 into main Apr 25, 2026
20 checks passed
@houko
houko deleted the feat/multi-format-chat-attachments branch April 25, 2026 08:03
@github-actions github-actions Bot removed the ready-for-review PR is ready for maintainer review label Apr 25, 2026
houko added a commit that referenced this pull request Apr 25, 2026
…g / sdk additions

Round 2 of the zh translation pass — picks up the seven mid-priority
files that the first commit didn't cover. All sections mirror the EN
content from #3189 but read naturally as Chinese (terms like agent /
session / fallback / NoEntry left in English where they're API
surface, prose translated to 中文).

- agent/templates: Live Context section, .identity/ vs root fallback,
  cache_context = true opt-out.
- api/agents: attachment-resolution table + trajectory export endpoint
  (#3025 / #3094 / #3097).
- api/system: POST /api/tools/{name}/invoke endpoint + fail-closed
  allowlist explanation.
- cli/commands: 'Slash Commands in Chat' section listing CLI registry
  commands and pointing at channel commands.
- providers/tools: 'CLI logins as first-class default providers'
  detection priority and override.
- troubleshooting: 'Audit checks' subsection + table.
- sdk: 'Auto-generated from openapi.json' section.
houko added a commit that referenced this pull request Apr 26, 2026
…pi / observability (#3189)

* docs(providers): add Novita / SearXHG / Bedrock entries

* docs(config): document [parallel_tools] section + max_history_messages

* docs(scheduler): cron multi-delivery + pre_script + silent_marker

* docs: live context.md, chat attachments, CLI slash commands

Salvages three of the six items from the (quota-killed) misc-features
agent run. Each is a focused append, no rewrite of existing content.

- agent/templates: live Live Context section explains the per-turn
  re-read of `context.md` (#3115), the .identity/ vs root path lookup,
  the 32 KB cap with stale-cache fallback, and the cache_context opt-out.
- integrations/api/agents: attachment-resolution table (image / PDF /
  text+code) covering the supported MIME / extension list and the
  200 KB truncation, plus a note about adjacent-text coalescing for
  small chat-tuned local models (#3094).
- integrations/cli/commands: 'Slash Commands in Chat' section listing
  the registry-backed CLI commands (/model /status /help /clear /kill
  /exit) and pointing at channel-side commands for the broader set
  (#3122 #3123).

* docs: tool invoke API, trajectory export, doctor audit, CLI default-pick, SDK codegen

Picks up the items the second misc-features agent didn't get to before
hitting the org quota.

- integrations/api/system: POST /api/tools/{name}/invoke (#3025) — fail-
  closed allowlist, agent_id requirement for approval-gated tools.
- integrations/api/agents: GET /sessions/{id}/trajectory (#3097) —
  json/jsonl formats, redactor pipeline, audit use case.
- operations/troubleshooting: 'Audit checks' subsection enumerating the
  three registered AuditChecks (#3082) — VaultKey, ApiListenAddr,
  ConfigTomlSchema — with what each asserts and how to extend.
- configuration/providers/tools: 'CLI logins as first-class default
  providers' section (#3061) — detection priority claude-code > codex-
  cli > gemini-cli > qwen-code, override via [default_model] in
  config.toml.
- integrations/sdk: 'Auto-generated from openapi.json' section (#3046) —
  scripts/codegen-sdks.py, pre-commit hook, openai-tag skip rule.

* docs(zh): mirror providers / config-core / cron / features-index sections

* docs(zh): mirror agent / api / cli / providers-tools / troubleshooting / sdk additions

Round 2 of the zh translation pass — picks up the seven mid-priority
files that the first commit didn't cover. All sections mirror the EN
content from #3189 but read naturally as Chinese (terms like agent /
session / fallback / NoEntry left in English where they're API
surface, prose translated to 中文).

- agent/templates: Live Context section, .identity/ vs root fallback,
  cache_context = true opt-out.
- api/agents: attachment-resolution table + trajectory export endpoint
  (#3025 / #3094 / #3097).
- api/system: POST /api/tools/{name}/invoke endpoint + fail-closed
  allowlist explanation.
- cli/commands: 'Slash Commands in Chat' section listing CLI registry
  commands and pointing at channel commands.
- providers/tools: 'CLI logins as first-class default providers'
  detection priority and override.
- troubleshooting: 'Audit checks' subsection + table.
- sdk: 'Auto-generated from openapi.json' section.

* docs: refresh /new slash-command semantics (#3071 review fix)

PR review on #3189 (houko) caught two stale references describing the
pre-#3071 behavior of /new ('wipe the session' / 'clear history'). The
new semantics fork off a fresh session while the previous one stays
resumable on the same channel — phrasing updated in 4 places (en + zh
× 2 files), plus a one-line clarifier in the slash-command roster
section so the cross-reference doesn't silently inherit the old
mental model:

- configuration/channels: 'wipe the session' → 'fork off into a new
  session (the prior conversation stays resumable on the channel)'
- integrations/api/realtime: '(clear history)' → 'forks into a fresh
  session — the prior session is preserved and remains resumable'
- integrations/cli/commands roster paragraph: explicit note that /new
  is a fork, not a wipe.

Same wording mirrored to docs/src/app/zh/.

* docs(channels): 9 channel-related Apr 22-25 PRs (en + zh)

Covers the channel + bridge feature wave:
- #3020 per-agent ChannelOverrides — fixes the contradicting 'without
  modifying the agent manifest' phrasing in both en and zh.
- #3077 prefix_agent_name + Signal plain-text default
- #3009 Slack reactions_enabled + #3005 Feishu @mention preservation
  consolidated into a 'Reactions and Processing State' section
- #3008 Signal media attachments
- #3012 WhatsApp send_voice + dm_policy / group_policy
- #3011 Webhook deliver_only mode
- #2972 channel file downloads (file_download_dir / max_bytes config)

* docs(config-core): provider timeout / browser CDP / cron session caps (en + zh)

Three new `config.toml` sections that landed Apr 22-23:
- #3004 `[provider_request_timeout_secs]` map
- #2993 / #2991 `[browser].cdp_endpoint` + `cdp_auth_token_env`
- #2994 `[kernel].cron_session_max_tokens` / `cron_session_max_messages`

* docs(api): GET /attach + Owner Notice envelope (en + zh)

- #3078 SSE attach endpoint for multi-client co-watching — describes
  the per-session SessionStreamHub fan-out, attacher behaviour, and
  the curl example.
- #2965 ReplyEnvelope.owner_notice + notify_owner tool — message body
  field, StreamEvent variant, WhatsApp OWNER_JIDS fan-out.

* docs(observability + ops): Tempo stack, cache_hit_ratio metric, daemon log tee (en + zh)

- #3064 + #3149 — bundled observability stack (otel-collector / Tempo
  / Grafana), bring-up commands, [telemetry] config, business spans,
  cache_hit_ratio metric formula and where it surfaces
- #3022 — `librefang start --foreground` now tees logs to
  ~/.librefang/logs/daemon-YYYY-MM-DD.log; same-day append; 7-day
  rotation pruned on daemon start

* docs: hooks transform / lazy tools / mcp taint / capability detection / providers extras

Final batch of catch-up docs covering the remaining Apr 22-25 user-facing
features:

- agent/hooks: transform_tool_result hook contract (#3003)
- agent/templates: tool_search / tool_load lazy loading + lazy_tools
  opt-out (#3047)
- integrations/mcp-a2a: TaintRuleId enum + per-tool skip_rules policy
  (#2999)
- configuration/features: compaction summaries in user's conversation
  language (#3007)
- configuration/providers/local: embedding auto-detection priority
  list (#3099) + Ollama capability detection (Modality / metadata
  pipeline, #3074 / #3133 / #3134 / #3140)
- configuration/providers/management: custom image-gen provider via
  generic OpenAI-compat driver (#2998) + Moonshot file-upload helper
  via /v1/files (#2966)

All sections mirrored to docs/src/app/zh/.

* docs: address houko fix-PR audit (8 items, en + zh)

PR review caught 1 stale-doc contradiction + 7 undocumented behaviour
changes from the fix-PR cohort.

- #3114 — configuration/core: 'api_key empty = unauthenticated' was
  stale; now documents the loopback-only policy (non-loopback bypass
  was closed)
- #3170 — features/observability: auto_start is opt-in; home_dir-
  scoped Docker labels; RAII cleanup
- #2989 — api/agents POST /message: per-request session_id override
- #3048 — Interrupting a Running Agent Turn: /stop cascades into
  agent_send subagents (recursive)
- #3056 + #3000 — desktop: dedicated Connection screen via custom
  URI scheme with retry / open-in-browser / uninstall
- #2952 — triggers: response routes to agent's home channel
- #2955 — triggers: task_posted assignee_match = 'self' | 'any'
- #3069 — providers/local: 60-second reprobe + manual 'Test
  connection' refresh

All sections mirrored in docs/src/app/zh/.
@houko houko mentioned this pull request Apr 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/runtime Agent loop, LLM drivers, WASM sandbox size/L 250-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant