Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,6 @@ storageState.json
/.playwright-browsers/
**/.vitest-attachments/
/blocksuite/framework/std/src/__tests__/gfx/__screenshots__/

# Claude Code local state (worktrees, session data, regenerable opsx tooling)
.claude/
2 changes: 2 additions & 0 deletions openspec/changes/fix-requesty-reasoning-leak/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-07-14
56 changes: 56 additions & 0 deletions openspec/changes/fix-requesty-reasoning-leak/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
## Context

The copilot streaming pipeline turns provider SSE into a union of stream parts. Reasoning has a first-class channel already: the native `llm_adapter` emits `LlmToolLoopStreamEvent`s (`text_delta` / `reasoning_delta`, `native.ts:708-712`), which `native-adapter.ts` maps to `text-delta` / `reasoning-delta` (streamText ~213-238) and `{type:'text-delta'}` / `{type:'reasoning'}` (streamObject ~343-354). `TextStreamParser` styles reasoning as callouts (`utils.ts:275-277`), and `StreamObjectParser.mergeContent` already keeps only `text-delta` for the UI object path (`utils.ts:365-372`).

This machinery only works when the provider delivers reasoning as a **separate field**. GLM via Requesty (`RequestyProvider extends OpenAIProvider`, `resolveModelBackendKind() = 'openai_chat'`) instead embeds reasoning inline as `<think>…</think>` inside `content`, so it arrives as `text_delta` and is treated as content. A repo-wide search for `<think`, `think>`, `reasoning_content` returns zero matches — nothing separates inline reasoning today. Separately, the non-streaming text path deliberately folds reasoning into content: `extractTextResponse` filters `type === 'text' || type === 'reasoning'` (`native-execution-engine.ts:59-64`) and `adapter.text()` concatenates every chunk including reasoning (`native-adapter.ts:164-174`). Both feed `code-artifact.ts:37-55`, which consumes the raw string.

Constraint: the SSE decode itself lives in the external compiled Rust `llm_adapter` crate and is out of reach. The fix must live in the in-repo TypeScript adapter/runtime layer.

## Goals / Non-Goals

**Goals:**

- Strip inline `<think>…</think>` from the content stream and re-route it to the reasoning channel, in both streamText and streamObject paths, robust to tags split across chunks.
- Ensure non-streaming tool/artifact text (`extractTextResponse`, `adapter.text()`) contains content only, never reasoning.
- Keep reasoning available on the reasoning channel for UI display.

**Non-Goals:**

- Modifying the external Rust `affine_doc_loader` / `llm_adapter` crates.
- Changing how natively-separated reasoning (Anthropic/Gemini/OpenAI) is produced.
- Broad prompt-output sanitization beyond reasoning isolation.

## Decisions

**Decision 1 — Split inline `<think>` in the TS adapter layer, not at the SSE decode.**
The SSE decode is compiled and unmodifiable. Implement a small stateful tag-splitter applied to `text_delta` text as it is turned into stream parts in `native-adapter.ts`. It maintains an `insideThink` flag and a small carry buffer for a possibly-partial boundary tag, emitting text outside think as `text-delta`/`{type:'text-delta'}` and text inside think as `reasoning-delta`/`{type:'reasoning'}`.

- _Alternative considered:_ strip in `TextStreamParser.parse` `'text-delta'` case (`utils.ts:145-158`). Rejected as the single point because the parser is content-formatting-oriented and the object path (`StreamObjectParser`) would still need the split; doing it once at the adapter boundary covers both `streamText` and `streamObject`.
- _Alternative considered:_ strip only in `code-artifact.ts`. Rejected — a band-aid that leaves raw `<think>` leaking into chat and other tools.

**Decision 2 — Stateful splitter tolerant of chunk boundaries.**
Tags can split across chunks (`<th|ink>`, or body across many deltas). The splitter buffers a trailing partial that could be the start of `<think>`/`</think>` and only commits text once it is known not to be a tag boundary. This satisfies the "split across chunks" scenario.

**Decision 3 — Fix the non-streaming text path independently.**
Even with Decision 1, `adapter.text()` accumulates reasoning-delta chunks and `extractTextResponse` keeps reasoning parts. Change `extractTextResponse` to filter `type === 'text'` only, and make `text()` skip reasoning-delta chunks. Both are required; Decision 1 alone does not stop the artifact leak for models that separate reasoning natively.

**Decision 4 — Requesty GLM reasoning behavior flag is optional and secondary.**
Adding a `reasoning_supported` behavior flag to the GLM variant in `model_registry.rs` could make Requesty return separated reasoning where supported, but requires a native rebuild and depends on provider behavior. The `<think>` splitter must exist regardless as the safety net, so the flag is deferred/optional and evaluated after the splitter lands.

## Risks / Trade-offs

- **False positives — legitimate `<think>` in user content (e.g. a doc about HTML/XML).** → Scope the splitter to the reasoning-prone path and only treat `<think>` at the start of a reasoning segment; keep it conservative (exact tag match, not arbitrary angle-bracket content). Covered by the "content without think tags is unchanged" scenario.
- **Chunk-boundary bugs dropping or duplicating characters.** → Unit tests that feed the same content split at every offset and assert content + reasoning reassemble exactly.
- **Nested or malformed tags (`<think>` with no close).** → On stream end, flush any open think buffer to the reasoning channel (never to content).
- **Other tools relying on reasoning-in-content.** → Grep confirms none do; `mergeContent` already excludes reasoning, so aligning `text()`/`extractTextResponse` matches existing intent.

## Migration Plan

- Backend-only, no schema/data migration. Ships via the existing CI image build → redeploy loop used for prior copilot changes.
- If Decision 4's model-registry flag is included, it requires a native rebuild (same as prior tool-wiring changes).
- Rollback: revert the adapter/runtime changes; no persisted state is affected.

## Resolved Decisions

- **Splitter scope:** applied **universally** but conservatively — it reacts only to literal `<think>`/`</think>` tags, so it is a no-op for providers that separate reasoning natively (Claude/Gemini/OpenAI). No per-provider gating; it serves as a general safety net.
- **Tag set:** handle **`<think>` only** (the confirmed GLM convention). Other conventions (`<thinking>`, `<reasoning>`) are deliberately not stripped, to avoid eating legitimate content that references those tags; expand only if another model is shown to leak.
31 changes: 31 additions & 0 deletions openspec/changes/fix-requesty-reasoning-leak/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
## Why

Models served through the Requesty router (notably GLM 5.2 / `sference/glm-5.2`) emit their chain-of-thought **inline** as `<think>…</think>` tags inside the normal content stream rather than in a separate reasoning field. The copilot layer has no mechanism to detect or separate inline reasoning, so the thinking text is treated as ordinary content — accumulated verbatim and baked into generated HTML code artifacts, where it is plainly visible in the preview. A second, latent defect means even _correctly_ separated reasoning (e.g. Claude) is folded back into the content string that the code-artifact tool consumes. The result is leaked reasoning in user-facing output.

## What Changes

- Inline `<think>…</think>` segments in the assistant content stream are detected in the TypeScript adapter layer and re-routed onto the existing **reasoning** channel (`reasoning-delta` / `{ type: 'reasoning' }`) instead of being emitted as content, for both the `streamText` and `streamObject` paths.
- The non-streaming text/tool path stops folding reasoning into returned content: `extractTextResponse` drops `type === 'reasoning'` parts, and `adapter.text()` skips reasoning-delta chunks. Tool-driven prompts (e.g. Code Artifact) receive content only.
- As a result, `code_artifact` output no longer contains `<think>` tags or reasoning text, regardless of whether the model separates reasoning natively or emits it inline.
- (Optional, evaluated in design) Tag the Requesty GLM model variant with a reasoning behavior flag so the request asks for separated reasoning where the provider supports it — but inline `<think>` stripping remains as an always-on safety net.

## Capabilities

### New Capabilities

- `copilot-reasoning-separation`: Reasoning/"thinking" output — whether delivered natively as a separate field or inline as `<think>…</think>` tags — is isolated onto the reasoning channel and excluded from assistant content and from tool/artifact text.

### Modified Capabilities

<!-- None: there are no existing specs under openspec/specs/. -->

## Impact

- **Code (backend copilot layer only):**
- `packages/backend/server/src/plugins/copilot/runtime/tool/native-adapter.ts` — `streamText` (~213-238), `streamObject` (~343-354), and `text()` accumulation (~164-174).
- `packages/backend/server/src/plugins/copilot/providers/utils.ts` — `TextStreamParser` `'text-delta'` handling (~145-158); reference behavior in `StreamObjectParser.mergeContent` (~365-372).
- `packages/backend/server/src/plugins/copilot/runtime/native-execution-engine.ts` — `extractTextResponse` (59-64).
- (Optional) `packages/backend/native/src/llm/core/model_registry.rs` — Requesty GLM variant behavior flag (requires native rebuild).
- **No changes** to the external Rust `affine_doc_loader` or `llm_adapter` crates (the SSE decode is compiled and out of scope).
- **Tests:** server-side ava suite under `packages/backend/server/src/__tests__/copilot/` (CI Node 22).
- **Behavior:** user-facing output (chat + artifacts) no longer leaks reasoning; reasoning remains available on its own channel for display as callouts.
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
## ADDED Requirements

### Requirement: Inline reasoning tags are separated from content

The system SHALL detect reasoning delivered inline as `<think>…</think>` segments within the assistant content stream and route that reasoning onto the reasoning channel, excluding it from assistant content. This SHALL apply to both the streaming text path and the streaming object path.

#### Scenario: Inline think block in a streamed text response

- **WHEN** a provider (e.g. GLM via Requesty) streams content containing `<think>my reasoning</think>visible answer`
- **THEN** `my reasoning` is emitted on the reasoning channel (as `reasoning-delta`)
- **AND** only `visible answer` is emitted as content
- **AND** the accumulated content string contains no `<think>` or `</think>` markers and no reasoning text

#### Scenario: Think block split across multiple stream chunks

- **WHEN** the `<think>` open tag, reasoning body, and `</think>` close tag arrive in separate stream chunks
- **THEN** the reasoning body is still routed to the reasoning channel in full
- **AND** no partial tag fragment leaks into content

#### Scenario: Content without think tags is unchanged

- **WHEN** a provider streams content that contains no `<think>` tags
- **THEN** all content is emitted unchanged as content
- **AND** no reasoning-delta is produced from the content

### Requirement: Tool and artifact text excludes reasoning

The system SHALL exclude reasoning parts from the text returned to tool-driven, non-streaming prompt executions, so that generated artifacts contain only content.

#### Scenario: Code artifact from a reasoning model

- **WHEN** a `code_artifact` prompt is executed against a model whose response includes reasoning (either separated natively or inline `<think>` tags)
- **THEN** the returned artifact HTML contains no reasoning text and no `<think>`/`</think>` markers

#### Scenario: Non-streaming text extraction drops reasoning parts

- **WHEN** `extractTextResponse` processes a message whose `content` parts include both `text` and `reasoning` parts
- **THEN** only `text` parts are concatenated into the returned string
- **AND** `reasoning` parts are omitted

#### Scenario: Accumulated text() skips reasoning chunks

- **WHEN** `adapter.text()` accumulates a stream that yields both text-delta and reasoning-delta chunks
- **THEN** the returned string contains only the text-delta content
- **AND** reasoning-delta content is omitted

### Requirement: Reasoning remains available on its own channel

The system SHALL continue to surface reasoning on the reasoning channel for display purposes; separation MUST NOT discard reasoning entirely.

#### Scenario: Reasoning still emitted for display

- **WHEN** a response contains reasoning (native or inline)
- **THEN** the reasoning is available as reasoning-delta / `{ type: 'reasoning' }` events for the UI to render (e.g. as a callout)
- **AND** it is not present in the content channel
35 changes: 35 additions & 0 deletions openspec/changes/fix-requesty-reasoning-leak/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
## 1. Inline reasoning splitter (TDD)

- [x] 1.1 Add a failing unit test: a helper that, given a sequence of content chunks containing `<think>…</think>`, yields separated `{ content, reasoning }` streams — assert reasoning routed out, content clean, no tag markers left
- [x] 1.2 Add a failing test for tags split across chunk boundaries (feed the same input split at every offset; content + reasoning must reassemble exactly)
- [x] 1.3 Add a failing test for an unterminated `<think>` (flush open buffer to reasoning on stream end, never to content)
- [x] 1.4 Add a failing test that content with no `<think>` tags passes through unchanged
- [x] 1.5 Implement the stateful `<think>` splitter (insideThink flag + partial-boundary carry buffer) to make 1.1–1.4 pass

## 2. Wire the splitter into the adapter paths

- [x] 2.1 Apply the splitter in `native-adapter.ts` `streamText` (~213-238): inside-think text → `reasoning-delta`, outside → `text-delta`
- [x] 2.2 Apply the splitter in `native-adapter.ts` `streamObject` (~343-354): inside-think → `{type:'reasoning'}`, outside → `{type:'text-delta'}`
- [x] 2.3 Add a test proving a GLM-style inline `<think>` stream produces reasoning on its channel and clean content on both stream paths

## 3. Purge reasoning from the non-streaming text/tool path

- [x] 3.1 Add a failing test: `extractTextResponse` (`native-execution-engine.ts:59-64`) drops `type === 'reasoning'` parts, keeps only `text`
- [x] 3.2 Change `extractTextResponse` filter to `type === 'text'` only
- [x] 3.3 Add a failing test: `adapter.text()` (`native-adapter.ts:164-174`) skips reasoning-delta chunks
- [x] 3.4 Update `adapter.text()` accumulation to skip reasoning-delta chunks

## 4. End-to-end artifact assertion

- [x] 4.1 Add a test that a `code_artifact` prompt over a stream containing inline `<think>` tags returns HTML with no `<think>`/`</think>` markers and no reasoning text
- [x] 4.2 Add a test for the same over a natively-separated-reasoning response (reasoning parts present) → artifact still content-only

## 5. Optional: Requesty GLM reasoning flag

- [x] 5.1 Evaluate adding a `reasoning_supported`/behavior flag to the Requesty GLM variant in `model_registry.rs` — **DEFERRED**. The inline `<think>` splitter already fully fixes the symptom and is provider-agnostic. Adding the flag requires a native (Rust) rebuild and depends on whether Requesty forwards a reasoning-separation param for GLM and whether GLM honors it — an optimization that needs testing against the live Requesty API, not required for the fix. The splitter stays as the always-on safety net regardless.
- [x] 5.2 N/A — flag deferred (see 5.1); no native rebuild performed.

## 6. Verify

- [~] 6.1 Copilot ava suite: **cannot run locally** — ava fails to bootstrap its prelude under local Node 24 (`ERR_MODULE_NOT_FOUND` resolving `src/prelude.ts`), the exact "runs in CI on Node 22, not locally" constraint noted in design. Verified equivalent behavior via standalone `tsx` execution: **58 splitter assertions + 13 integration assertions (streamText/streamObject/text/extractTextResponse/code_artifact) all green**, plus a clean `tsc --noEmit` on all changed files and the spec. The ava specs themselves run in CI on Node 22.
- [x] 6.2 Manual GLM 5.2-via-Requesty artifact check — confirmed on the live deployment: HTML artifact preview shows no thinking blocks.
20 changes: 20 additions & 0 deletions openspec/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
schema: spec-driven

# Project context (optional)
# This is shown to AI when creating artifacts.
# Add your tech stack, conventions, style guides, domain knowledge, etc.
# Example:
# context: |
# Tech stack: TypeScript, React, Node.js
# We use conventional commits
# Domain: e-commerce platform

# Per-artifact rules (optional)
# Add custom rules for specific artifacts.
# Example:
# rules:
# proposal:
# - Keep proposals under 500 words
# - Always include a "Non-goals" section
# tasks:
# - Break tasks into chunks of max 2 hours
Loading
Loading