Commit graph

5 commits

Author SHA1 Message Date
Daniel Han
187144d4e7
Reduce and tighten code comments and docstrings repo-wide (#6095)
Trim and tighten code comments and docstrings across the repository. Comment-only: every changed file verified code-identical to main via AST/token comparison.
2026-06-08 23:09:51 -07:00
Daniel Han
8292e699e4
Studio: make code comments and docstrings more succinct (#6029)
Trim and tighten code comments and docstrings across studio/ Python. Comment-only: every changed file verified code-identical to main via AST/token comparison.
2026-06-08 23:07:28 -07:00
Daniel Han
3ce187da02
Formatting: ruff line-length 100, kwarg-spacing passes, drop blank after short local imports (#6079)
Raise ruff line-length to 100 and extend the local pre-commit format pipeline (def-signature magic-comma normalization, short multi-line assert collapse, kwarg '=' spacing, blank-line-after-short-import removal, adjacent string-literal / f-string+plain merge, redundant-pass pruning). Every transform re-checks the file AST and is dropped if it would differ; the whole-repo reformat is verified AST-identical per file and idempotent.
2026-06-08 04:24:13 -07:00
Daniel Han
4854d4579f
Studio: surface Anthropic document citations inline + in Sources panel (#5718)
* Studio: surface Anthropic document citations inline + in Sources panel

Anthropic's Messages API streams ``citations_delta`` events on
``content_block_delta`` when the request enables
``citations: {enabled: true}`` on document blocks. Each event carries
one citation pointing at the source document; previously they were
silently dropped, so reader-visible references never reached the chat
UI even when the model was citing properly.

The proxy now:
- dedupes by the type-specific anchor (char_location / page_location /
  content_block_location / search_result_location) so re-cites of the
  same span collapse onto a single footnote;
- injects ``[N]`` inline right after the matching text run;
- forwards the full list as a synthetic ``document_citations``
  tool_event at ``message_stop`` so the Sources panel can render
  per-document footnotes next to web_search / web_fetch citations.

Streams that never emit ``citations_delta`` stay byte-identical.

References:
- https://platform.claude.com/docs/en/build-with-claude/citations
- https://platform.claude.com/docs/en/build-with-claude/search-results

Tests (5 in test_anthropic_citations.py): passthrough, single
char_location, dedup of repeat citations, distinct sources get
distinct numbers, search_result_location supported.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: surface Anthropic document_citations in the Sources panel

The PR added a backend _toolEvent.type='document_citations' on
message_stop and an inline [N] marker in the assistant text, but the
chat-adapter only handles container_*/tool_*/sources from
web_search and web_fetch tool calls. Reviewers flagged that the
inline [N] markers had no matching footnote entries in the Sources
panel.

Capture the new event into a documentCitationParts buffer, convert
each citation dict into a Sources-panel source entry (using
document_title or search-result source URL plus cited_text as the
snippet), dedupe by id, and append to the final yield alongside
the existing web_search/web_fetch sourceParts.

* Studio: dedupe search_result_location citations by search_result_index

Anthropic's documented search_result_location citation shape carries
search_result_index, source, title, and start/end_block_index --
NOT document_index/document_title. The previous key keyed on
document_index + document_title + source + start_block_index, so
two distinct search results from the same source collapsed onto the
same footnote and the second [N] marker was lost.

Switch the search_result_location branch to key on the documented
fields, and pin the behaviour with a regression test asserting that
two citations sharing source/title but with different
search_result_index get distinct [1] [2] markers.

* Studio: keep each citation distinct across the end-anchor

Codex follow-ups on the citations PR:

  * Backend _anthropic_citation_key now includes the end anchor for
    every variant (end_char_index, end_page_number,
    end_block_index). Anthropic ranges are start-AND-end pairs, so
    a same-start / different-end pair is two distinct citations
    that previously collapsed onto one footnote.

  * Frontend documentCitationToSource ids include the position
    fields (search_result_index, start/end char/page/block) instead
    of being keyed on URL alone. Two citations from the same
    document or two search_result_locations with the same source
    now produce distinct Sources-panel entries, matching the
    inline [N] numbering.

* Studio: key Sources list by per-citation id instead of url

Codex flagged that the Sources renderer keys badges on source.url,
so two Anthropic document citations sharing the same source URL
collide as React keys and one badge gets dropped (or duplicated).

The chat-adapter already mints a per-citation id that folds the
position fields (search_result_index, start/end char/page/block)
into the URL, so the two citations have distinct ids even when
their URL matches. Plumb that id through SourceData and use it as
the React key for both the measurement badges and the visible
SourceBadge list. Falls back to the URL when no id is supplied
(web_search and web_fetch source parts).

* Studio: enable Anthropic doc citations on input_document blocks

Plumb citations: {enabled: true} onto the translated Anthropic document
block (both base64 and URL source branches) so the upstream actually
emits citations_delta events. Without this opt-in the inline [N] +
Sources panel plumbing added in this PR is a no-op for real user
PDF / doc uploads.

Refs https://platform.claude.com/docs/en/build-with-claude/citations

Also add edge-case coverage for the citations_delta path:
malformed citations, mixed types per document, reversed indices,
missing document_index, non-int block indices, unknown citation
type, internal _key never leaking, footnote numbering across
content blocks, and the input_document wire-through itself.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Reject unsafe citation sources, bound cited_text payload

Three follow-ups on top of #5718 surfaced by a deeper review pass:

1) javascript: / data: / vbscript: in citation source is XSS-able.
   ``documentCitationToSource`` was assigning ``cit.source`` straight
   into ``Source.url`` and rendering it as an <a href>. A hostile
   model emitting ``cit.source = "javascript:alert(document.domain)"``
   would execute on click (openLink only intercepts URLs that contain
   "://" or start with "mailto:", which both miss the javascript:
   scheme). Restrict the navigable path to http(s):// only; anything
   else falls back to the existing #anthropic-doc anchor and the
   source title still renders the raw identifier for context. Also
   reject CR/LF inside the URL string.

2) Frontend sources collapse distinct backend footnotes when the
   citation type differs but positions match. char_location(0,5) and
   page_location(0,5) over the same source previously deduped into
   one entry because the id only carried position. Fold citation
   type into the id anchor so the 1:1 mapping with inline [N]
   markers is preserved across every citation shape.

3) ``cited_text`` was forwarded unbounded inside the synthetic
   document_citations tool_event. The Sources panel trims to 240
   chars for display anyway; for large RAG / search_result spans
   (~10kB cited_text is plausible) this inflates SSE bytes 40x
   for no UI benefit. Truncate server-side at 512 chars with an
   ellipsis so the description-trim downstream still has room to
   work and the wire stays bounded.

Tests grow from 21 to 22; existing 7 + edge 15 still green. Frontend
typecheck clean.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: apply http(s) URL guard to all Sources-panel link sources

The previous round only filtered ``cit.source`` inside
``documentCitationToSource``. Two parallel code paths still copied
provider/tool-controlled ``URL:`` text directly into clickable
``<a href>`` Sources-panel links:

  * ``parseSourcesFromResult`` in chat-adapter.ts (legacy web_search /
    web_fetch tool result parser)
  * ``parseSearchResults`` in tool-ui-web-search.tsx (inline tool card)

A hostile tool response like ``URL: javascript:alert(1)`` or
``URL: data:text/html,...`` was therefore still rendered as a
navigable badge in the Sources panel.

Centralise the safe-URL test (``isSafeNavigableSourceUrl``,
``isSafeHttpUrl``) using ``new URL()`` + protocol allowlist + CR/LF
rejection, and apply it to both parsers. Unsafe blocks are dropped
rather than rewritten to a hash anchor because the web_search /
web_fetch parsers have no document-index fallback.

Citation conversion now uses the same helper so the in-place
http(s) regex and CR/LF check stay in one place.

* Shorten citation comments for PR #5718

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2026-05-25 23:39:02 -07:00
Daniel Han
ebe504b558
Studio: PDF / document attachments for Anthropic + OpenAI (#5689)
* Studio: PDF / document attachments for Anthropic + OpenAI

Studio's local-GGUF chat already supports image attachments via the
`image_url` content part shape. PDFs and other documents had no
plumbing for the external-provider path: there was no normalised
content type the frontend could send that translated to Anthropic's
native `document` block or OpenAI's `input_file`.

Add a Studio-side `input_document` content part on assistant /
user messages with three shapes:

  {type: "input_document",
   file_data: "data:application/pdf;base64,<DATA>",
   filename?: "name.pdf",
   media_type?: "application/pdf"}

  {type: "input_document",
   file_url: "https://example.com/doc.pdf",
   filename?: "doc.pdf"}

Translation:

- Anthropic Messages API: emits a `document` block with
  `{source: {type:"base64", media_type, data}}` or
  `{source: {type:"url", url}}`, plus an optional `title` from
  `filename`. PDFs are extracted server-side by Anthropic per their
  vision/document docs and counted toward input tokens.
- OpenAI Responses API: emits `{type:"input_file", file_data |
  file_url, filename?}`. PDFs are extracted server-side.

Empty / unparseable `input_document` parts are silently dropped so
a malformed frontend payload can't blow up the request.

Tests:

- New `test_multimodal_document.py` with 6 cases pinning the
  outbound body shape for base64 + URL inputs on both providers,
  and the empty-part drop behavior on both.
- The Anthropic assertions strip the prompt-cache wrapper
  (`cache_control:{type:ephemeral}` that the tail-message caching
  layer adds) before comparing the document core fields, so this
  test stays focused on the translation, not the caching layer.

Live verified end-to-end against both providers: a 363-byte
single-page "HELLO" PDF, base64-encoded, attached as a `document`
block to Opus 4.7 and as an `input_file` to gpt-5.5. Both models
correctly extracted the word "HELLO" from the PDF.

Follow-up (out of scope):

- Pydantic schema entry on ChatMessage.content for `input_document`
  (today it rides through because ChatCompletionRequest uses
  extra=allow). Will tighten when the frontend attach button lands.
- Frontend file-picker UX for non-image attachments on the external
  provider path.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Address review: gate empty-content msg + skip empty data-URI payload

Gemini High + Codex P2 on PR #5689:

1. Anthropic translation appended an empty `anthropic_parts` array
   when every part was dropped (e.g. user sent only an unparseable
   input_document). Anthropic 400s on "messages.N.content: at least
   one block is required". Skip the whole-message append when no
   parts survived. The OpenAI Responses path already had the
   equivalent guard, so this brings the two providers into parity.

2. `data:application/pdf;base64,` with no payload (or whitespace-only)
   parses to an empty `source.data` string. Anthropic rejects that
   with 400 as well. Skip the document block before constructing it.

Plus 2 new test cases pinning both behaviors:

- `test_anthropic_empty_only_document_drops_whole_message`: confirms
  a turn whose only content is an unparseable input_document does
  NOT make it onto the outbound `messages` array.
- `test_anthropic_empty_data_uri_payload_is_dropped`: confirms an
  empty-payload data-URI is filtered out at translation time.

(Note re: gemini's other High note about adding `input_document` to
the Pydantic ContentPart union -- ChatCompletionRequest is configured
with `extra=allow` so the part rides through today. Tightening the
union belongs with the frontend attach-button PR that surfaces the
field; called out as follow-up in the PR description.)

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Address review: register input_document in ContentPart + builder

Reviewer caught that the translation code on the external_provider
side was unreachable from a real ChatCompletionRequest:

- ContentPart is a discriminated Union of (text, image_url) only, so
  any `{"type": "input_document", ...}` part was rejected by Pydantic
  at request parsing with a discriminator error before the helper
  could see it.
- _build_external_messages in routes/inference.py only walked text
  and image_url parts, so even with a permissive schema the document
  parts would have been silently dropped instead of forwarded to
  the per-provider translator.

Fixes:

- Add InputDocumentContentPart with optional file_data / file_url /
  filename / media_type and Tag("input_document") on the Union.
- Extend _build_external_messages to pass input_document through as
  a plain dict for vision-capable providers (so external_provider's
  existing Anthropic `document` and OpenAI Responses `input_file`
  mappers actually run) and strip them on non-vision providers.

Tests added: schema accepts input_document, builder passes it to
vision providers, builder strips it on non-vision providers.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Address review: validate file_data before preferring over file_url

Codex P2 caught that the OpenAI input_document translator treats any
truthy file_data as valid and never falls back to file_url. That
means a malformed `data:application/pdf;base64,` (empty payload) or
a whitespace-only data URI gets forwarded as `file_data=""` and
400s the whole turn, AND silently discards a perfectly recoverable
file_url on the same part.

Mirror the Anthropic-side guard onto the OpenAI Responses path:
treat any "data:" URI with no actual base64 payload as missing and
fall through to file_url. Standalone-empty data URIs (no fallback)
are dropped entirely instead of being sent to the wire.

Tests added: empty data URI + valid file_url -> file_url wins,
whitespace-only data URI + valid file_url -> file_url wins,
empty data URI without fallback -> part is dropped.

* Address review: Anthropic side also falls back to file_url on empty data URI

Codex P2 follow-up to my earlier fix: I added the empty-data-URI ->
file_url fallback to the OpenAI Responses translator but missed
the Anthropic translator, which still `continue`d on empty payloads
and discarded an otherwise valid file_url on the same part. Result:
when the frontend supplied both file_data (placeholder / broken)
AND a working file_url, Anthropic silently lost the attachment;
when the message contained only that part, the whole message could
be dropped before reaching the wire.

Mirrored the OpenAI guard: any "data:" URI with no actual base64
payload (`data:application/pdf;base64,` or whitespace-only) is
treated as missing, and the file_url branch takes over. The
all-parts-dropped guard further down already handles the
no-fallback case.

Tests added: empty data URI + valid file_url -> URL source on the
wire with the filename preserved; whitespace-only data URI + valid
file_url -> URL source on the wire.

* Address review: gate input_document passthrough to anthropic + openai

Codex P1: only `_stream_anthropic` and `_stream_openai_responses`
have explicit translation logic for input_document parts (the former
maps to {type:"document", source:...}, the latter to
{type:"input_file", file_data|file_url}). Every other provider
(gemini / mistral / kimi / openrouter / deepseek / qwen / custom)
goes through the generic /chat/completions passthrough that forwards
`messages` verbatim, so any input_document part on a non-vision
route on those providers would 400 with an unknown content_part
type.

Added `_INPUT_DOCUMENT_PROVIDERS = frozenset({"anthropic", "openai"})`
constant and gated the pass-through branch on `provider_type in
_INPUT_DOCUMENT_PROVIDERS`. Every other provider strips the part
(text content survives). Threaded provider_type through from
_proxy_to_external_provider's call site.

Tests updated: vision + provider in {anthropic, openai} still
forwards; six unmapped providers (gemini/mistral/kimi/openrouter/
deepseek/qwen) strip the part; missing provider_type strips
defensively. The existing non-vision drop test still passes.

* Fix stale web_fetch tool-version assertion after merging main

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2026-05-22 06:22:57 -07:00