Skip to content

OLS-3442 Add reasoning token support spec #2985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
thoraxe wants to merge 1 commit into
base: main
Choose a base branch
from
Open

OLS-3442 Add reasoning token support spec #2985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 25 additions & 9 deletions .ai/spec/how/llm-providers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ The LLM provider subsystem translates a (provider name, model name) pair from co

| File | Class | Decorator key | LangChain class | Notes |
|---|---|---|---|---|
| `openai.py` | `OpenAI` | `"openai"` | `ChatOpenAI` | Reasoning params for o-series/gpt-5 models |
| `azure_openai.py` | `AzureOpenAI` | `"azure_openai"` | `AzureChatOpenAI` | Entra ID token caching; see below |
| `openai.py` | `OpenAI` | `"openai"` | `ChatOpenAI` | Reasoning via `reasoning_config`; [PLANNED: OLS-3442 — replace model-name detection] |
| `azure_openai.py` | `AzureOpenAI` | `"azure_openai"` | `AzureChatOpenAI` | Entra ID token caching; reasoning via `reasoning_config` [PLANNED: OLS-3442] |
| `watsonx.py` | `Watsonx` | `"watsonx"` | `ChatWatsonx` | IBM-specific parameter names; see below |
| `rhoai_vllm.py` | `RHOAIVLLM` | `"rhoai_vllm"` | `ChatOpenAI` | OpenAI-compatible, no default URL |
| `rhelai_vllm.py` | `RHELAIVLLM` | `"rhelai_vllm"` | `ChatOpenAI` | OpenAI-compatible, no default URL |
| `google_vertex.py` | `GoogleVertex` | `"google_vertex"` | `ChatGoogleGenerativeAI` | Gemini / publisher models; credentials via `load_vertex_credentials` |
| `google_vertex.py` | `GoogleVertexAnthropic` | `"google_vertex_anthropic"` | `ChatAnthropicVertex` | Claude on Vertex Model Garden; same module |
| `rhoai_vllm.py` | `RHOAIVLLM` | `"rhoai_vllm"` | `ChatOpenAI` or `ChatVLLMReasoning` | OpenAI-compatible, no default URL. [PLANNED: OLS-3442 — `ChatVLLMReasoning` when `reasoning_config.enabled`] |
| `rhelai_vllm.py` | `RHELAIVLLM` | `"rhelai_vllm"` | `ChatOpenAI` or `ChatVLLMReasoning` | OpenAI-compatible, no default URL. [PLANNED: OLS-3442 — `ChatVLLMReasoning` when `reasoning_config.enabled`] |
| `google_vertex.py` | `GoogleVertex` | `"google_vertex"` | `ChatGoogleGenerativeAI` | Gemini / publisher models; credentials via `load_vertex_credentials`. [PLANNED: OLS-3442 — thinking config via `reasoning_config`] |
| `google_vertex.py` | `GoogleVertexAnthropic` | `"google_vertex_anthropic"` | `ChatAnthropicVertex` | Claude on Vertex Model Garden; same module. [PLANNED: OLS-3442 — thinking config via `reasoning_config`] |
| `bedrock.py` | `Bedrock` | `"bedrock"` | `ChatBedrockConverse` or `ChatOpenAI` | AWS Bedrock via Mantle gateway; routes by model prefix (`anthropic.*` → Converse, `openai.*` → Responses API). [PLANNED: OLS-3442 — reasoning config] |
| `vllm_reasoning.py` | `ChatVLLMReasoning` | (not registered) | N/A | [PLANNED: OLS-3442] `BaseChatOpenAI` subclass that captures `reasoning_content`/`reasoning` from vLLM responses. Not a provider — used by vLLM providers when reasoning is enabled |
| `fake_provider.py` | `FakeProvider` | `"fake_provider"` | `FakeListLLM` / `FakeStreamingListLLM` | Testing only; monkey-patches `bind_tools` |

### `ols/src/llms/providers/utils.py` -- Shared utilities
Expand Down Expand Up @@ -126,7 +128,7 @@ Each provider follows the same pattern: read generic fields from `ProviderConfig

### Constants

- Provider type strings: `PROVIDER_OPENAI`, `PROVIDER_AZURE_OPENAI`, `PROVIDER_WATSONX`, `PROVIDER_RHOAI_VLLM`, `PROVIDER_RHELAI_VLLM`, `PROVIDER_FAKE`, `PROVIDER_GOOGLE_VERTEX`, `PROVIDER_GOOGLE_VERTEX_ANTHROPIC` in `ols/constants.py`.
- Provider type strings: `PROVIDER_OPENAI`, `PROVIDER_AZURE_OPENAI`, `PROVIDER_WATSONX`, `PROVIDER_RHOAI_VLLM`, `PROVIDER_RHELAI_VLLM`, `PROVIDER_FAKE`, `PROVIDER_GOOGLE_VERTEX`, `PROVIDER_GOOGLE_VERTEX_ANTHROPIC`, `PROVIDER_BEDROCK` in `ols/constants.py`.
- `SUPPORTED_PROVIDER_TYPES` frozenset is checked during config validation (not by the registry).

### TLS and proxy
Expand Down Expand Up @@ -162,9 +164,23 @@ WatsonX uses IBM's `GenTextParamsMetaNames` constants for parameter keys instead

WatsonX also passes `params=self.params` to `ChatWatsonx` (as a nested dict), unlike OpenAI-compatible providers that spread params as `**self.params`.

### OpenAI reasoning model handling
### Reasoning model handling

Both `openai.py` and `azure_openai.py` detect o-series and gpt-5 models by name pattern (`self.model.startswith("o")` or `"gpt-5" in self.model`). For these models, temperature/top_p/frequency_penalty are omitted and reasoning parameters (effort, summary) are set instead. The `ModelParameters` config class provides `reasoning_effort`, `reasoning_summary`, and `verbosity` fields.
[PLANNED: OLS-3442] Currently, `openai.py` and `azure_openai.py` detect o-series and gpt-5 models by name pattern (`self.model.startswith("o")` or `"gpt-5" in self.model`). This model-name detection will be replaced by config-driven enablement via `reasoning_config`.

When `reasoning_config` is present in a model's `ModelParameters`, the provider reads provider-specific keys from it and passes them to the LangChain adapter. Standard sampling parameters (`temperature`, `top_p`, `frequency_penalty`) are skipped. When `reasoning_config` is absent, the provider applies standard sampling defaults.

The existing `ModelParameters` fields `reasoning_effort`, `reasoning_summary`, and `verbosity` will be removed and replaced by the freeform `reasoning_config` dict.

### vLLM reasoning subclass (`ChatVLLMReasoning`)

[PLANNED: OLS-3442] `ChatVLLMReasoning` in `vllm_reasoning.py` subclasses `BaseChatOpenAI` (same base as `ChatDeepSeek` from `langchain-deepseek`) and overrides two methods:

1. **`_create_chat_result()`** — non-streaming. Calls `super()`, extracts `reasoning_content` or `reasoning` from the raw `openai.BaseModel` response (which preserves unknown fields via Pydantic `extra="allow"`), stores in `message.additional_kwargs["reasoning_content"]`.

2. **`_convert_chunk_to_generation_chunk()`** — streaming. Calls `super()`, extracts reasoning from `chunk["choices"][0]["delta"]` dict, stores in `message.additional_kwargs["reasoning_content"]`.

Both vLLM field names (`reasoning_content` and `reasoning`) are checked to handle the vLLM rename (RFC #27755). Output is normalized to `additional_kwargs["reasoning_content"]` regardless of source field. No new Python dependencies required.

### HTTP client setup

Expand Down
6 changes: 4 additions & 2 deletions .ai/spec/how/query-pipeline.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ All pipeline output flows through `StreamedChunk(type, text, data)`. The `Stream
| Type | When emitted | Payload |
|---|---|---|
| `TEXT` | LLM text tokens | `text` field |
| `REASONING` | Model reasoning tokens (e.g. OpenAI o-series) | `text` field |
| `REASONING` | Model reasoning tokens. Source varies by provider: LangChain `content_blocks` (OpenAI, Gemini, Anthropic) or `additional_kwargs["reasoning_content"]` (vLLM via `ChatVLLMReasoning`). [PLANNED: OLS-3442] | `text` field |
| `TOOL_CALL` | Before tool execution | `data`: tool name, args, id, server metadata |
| `APPROVAL_REQUIRED` | Tool requires user approval | `data`: approval request details |
| `TOOL_RESULT` | After tool execution | `data`: id, name, status, content, round, structured_content |
Expand Down Expand Up @@ -297,7 +297,9 @@ The budget is calculated once in `DocsSummarizer.__init__` via `TokenBudgetTrack

### How streaming chunks interleave during tool calling

In non-final rounds, `LLMExecutionAgent._invoke_llm()` binds tools to the LLM (no `tool_choice` constraint). `_collect_round_llm_chunks()` accumulates all chunks from one LLM invocation, separating tool-call chunks from text/reasoning chunks. Text chunks are yielded immediately for streaming. After the LLM stream ends, `_process_tool_calls_for_round()` emits `TOOL_CALL` events, executes the tools, then emits `TOOL_RESULT` events. The AI message (including any reasoning content) and all tool messages are appended to the prompt template for the next round.
In non-final rounds, `LLMExecutionAgent._invoke_llm()` binds tools to the LLM (no `tool_choice` constraint). `_collect_round_llm_chunks()` accumulates all chunks from one LLM invocation, separating tool-call chunks from text/reasoning chunks. Text and reasoning chunks are yielded immediately for streaming. [PLANNED: OLS-3442] For vLLM providers, reasoning arrives via `chunk.additional_kwargs["reasoning_content"]` (captured by `ChatVLLMReasoning`) rather than `content_blocks` — a check for this source is added after the existing `match chunk.content` block. After the LLM stream ends, `_process_tool_calls_for_round()` emits `TOOL_CALL` events, executes the tools, then emits `TOOL_RESULT` events. The AI message (including any reasoning content) and all tool messages are appended to the prompt template for the next round.

[PLANNED: OLS-3442] In `streaming_ols.py`, both `StreamChunkType.TEXT` and `StreamChunkType.REASONING` chunks are accumulated into the `response` string. Previously only TEXT chunks were accumulated, causing reasoning to be lost from the stored conversation history.
Comment on lines +300 to +302

@coderabbitai coderabbitai Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== files ==\n'
git ls-files | rg '(^|/)(streaming_ols\.py|query-pipeline\.md|StreamChunkType|stream_chunk|streaming_ols)' || true

printf '\n== search for StreamChunkType handling ==\n'
rg -n "StreamChunkType|reasoning_content|response\s*\+=" -S . || true

printf '\n== inspect relevant docs snippet ==\n'
if [ -f .ai/spec/how/query-pipeline.md ]; then
  nl -ba .ai/spec/how/query-pipeline.md | sed -n '285,310p'
fi

printf '\n== inspect streaming_ols.py if present ==\n'
if [ -f streaming_ols.py ]; then
  nl -ba streaming_ols.py | sed -n '1,260p'
fi

printf '\n== inspect matching path(s) if elsewhere ==\n'
fd -a 'streaming_ols.py|.*streaming.*ols.*\.py' . || true

Repository: openshift/lightspeed-service

Length of output: 11183

🏁 Script executed:

python3 - <<'PY'
from pathlib import Path

for path, start, end in [
    ("ols/app/endpoints/streaming_ols.py", 435, 460),
    (".ai/spec/how/query-pipeline.md", 295, 305),
]:
    print(f"\n== {path} ==")
    lines = Path(path).read_text().splitlines()
    for i in range(start, min(end, len(lines)) + 1):
        print(f"{i:4d}: {lines[i-1]}")
PY

Repository: openshift/lightspeed-service

Length of output: 3675

Update the spec to match streaming_ols.py.
REASONING is streamed, but only TEXT is appended to response today, so this reads like a shipped fix that isn’t in place yet. Either soften the spec to a planned change or add the missing append in code.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.ai/spec/how/query-pipeline.md around lines 300 - 302, The query-pipeline
spec is describing behavior that is not yet implemented in streaming_ols.py:
StreamChunkType.REASONING is streamed, but only StreamChunkType.TEXT is
currently appended to the stored response. Update the wording around
LLMExecutionAgent._invoke_llm(), _collect_round_llm_chunks(), and the [PLANNED:
OLS-3442] note so it reflects the actual shipped behavior, or clearly mark
reasoning accumulation as a future change rather than completed.


On the final round (round == max_rounds or no tools available), the LLM is invoked with `tool_choice="none"` and `strict=False` to force a text-only response. The Granite model family requires special handling: the first 6 chunks of a tool call (`"", "<", "tool", "_", "call", ">"`) are suppressed via `skip_special_chunk()` to avoid leaking tool-call markup to the user.

Expand Down
11 changes: 6 additions & 5 deletions .ai/spec/what/config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,18 @@ Each provider entry under `llm_providers` supports:
| Field Path | Type | Default | Purpose |
|------------|------|---------|---------|
| `llm_providers[].name` | string | (required) | Provider name |
| `llm_providers[].type` | string | =name | Provider type (openai, azure_openai, watsonx, rhoai_vllm, rhelai_vllm, google_vertex, google_vertex_anthropic, fake_provider) |
| `llm_providers[].type` | string | =name | Provider type (openai, azure_openai, watsonx, rhoai_vllm, rhelai_vllm, google_vertex, google_vertex_anthropic, bedrock, fake_provider) |
| `llm_providers[].url` | URL | none | Provider endpoint URL |
| `llm_providers[].credentials_path` | string | none | Path to credential file or directory |
| `llm_providers[].models[]` | list | (required, >= 1) | Model definitions |
| `llm_providers[].models[].name` | string | (required) | Model identifier |
| `llm_providers[].models[].context_window_size` | int | 128000 | Context window size in tokens |
| `llm_providers[].models[].parameters.max_tokens_for_response` | int | 4096 | Tokens reserved for response |
| `llm_providers[].models[].parameters.tool_budget_ratio` | float | 0.25 | Fraction of context window for tool outputs (0.1--0.6) |
| `llm_providers[].models[].parameters.reasoning_effort` | enum | low | Reasoning effort level (low, medium, high) |
| `llm_providers[].models[].parameters.reasoning_summary` | enum | concise | Reasoning summary style (auto, concise, detailed) |
| `llm_providers[].models[].parameters.verbosity` | enum | low | General verbosity level (low, medium, high) |
| `llm_providers[].models[].parameters.reasoning_config` | dict | none | [PLANNED: OLS-3442] Freeform dict of provider-specific reasoning/thinking parameters. See `what/llm-providers.md` rule 13 for valid keys per provider |
| `llm_providers[].models[].parameters.reasoning_effort` | enum | low | [DEPRECATED: OLS-3442] Reasoning effort level (low, medium, high). Replaced by `reasoning_config` |
| `llm_providers[].models[].parameters.reasoning_summary` | enum | concise | [DEPRECATED: OLS-3442] Reasoning summary style (auto, concise, detailed). Replaced by `reasoning_config` |
| `llm_providers[].models[].parameters.verbosity` | enum | low | [DEPRECATED: OLS-3442] General verbosity level (low, medium, high). Replaced by `reasoning_config` |
Comment on lines +112 to +123

@coderabbitai coderabbitai Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the cited spec sections with line numbers.
file=".ai/spec/what/config.md"
if [ -f "$file" ]; then
  echo "== $file (lines 100-170) =="
  sed -n '100,170p' "$file" | nl -ba
else
  echo "Missing $file"
fi

echo
echo "== Search for llm_providers[].type / allowlist / validation references =="
rg -n --no-heading "llm_providers\\[\\]\\.type|allowlist|registered provider|type.*name|type = name|provider instance name|aliases|alias" .ai/spec . 2>/dev/null || true

echo
echo "== Nearby references to line 153 in config.md =="
if [ -f "$file" ]; then
  sed -n '145,160p' "$file" | nl -ba
fi

Repository: openshift/lightspeed-service

Length of output: 252

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file=".ai/spec/what/config.md"

echo "== $file (lines 100-170) =="
awk 'NR>=100 && NR<=170 { printf "%d:%s\n", NR, $0 }' "$file"

echo
echo "== Search for type/allowlist references =="
rg -n --no-heading "llm_providers\\[\\]\\.type|allowlist|registered provider|type.*name|type = name|provider instance name|aliases|alias" .ai/spec . 2>/dev/null || true

echo
echo "== Lines around 153 =="
awk 'NR>=145 && NR<=160 { printf "%d:%s\n", NR, $0 }' "$file"

Repository: openshift/lightspeed-service

Length of output: 19131

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="ols/app/models/config.py"

echo "== provider type logic =="
awk 'NR>=430 && NR<=490 { printf "%d:%s\n", NR, $0 }' "$file"

echo
echo "== related tests =="
awk 'NR>=1500 && NR<=1545 { printf "%d:%s\n", NR, $0 }' tests/unit/app/models/test_config.py

echo
echo "== search for type validation / unsupported provider references =="
rg -n --no-heading "unsupported provider|supported provider types|Provider type must be one of|set_provider_type|type.*lower\\(\\)|defaulted from provider name|allowlist" ols tests .ai/spec/what/llm-providers.md .ai/spec/how/llm-providers.md

Repository: openshift/lightspeed-service

Length of output: 8555

Clarify llm_providers[].type defaulting The default only works when name is already one of the supported provider types; aliased provider names must set type explicitly or startup fails.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.ai/spec/what/config.md around lines 112 - 123, Clarify the defaulting
behavior for llm_providers[].type in the config spec: the current
default-to-name rule only works when the provider name already matches a
supported provider type, while aliased names require an explicit type. Update
the documentation in the llm_providers schema section to state this constraint
clearly, and note that aliases must set type explicitly to avoid startup
failures.

| `llm_providers[].models[].options` | dict | none | Arbitrary key-value model options |
| `llm_providers[].<type>_config` | object | none | Provider-specific config (at most one per provider) |
| `llm_providers[].tlsSecurityProfile` | object | none | TLS security profile for provider connection |
Expand Down Expand Up @@ -149,7 +150,7 @@ Each provider entry under `llm_providers` supports:
13. `default_model` must name a model that exists within the default provider's model list.
14. `conversation_cache.type` must be either `memory` or `postgres`. The corresponding sub-section (`memory` or `postgres`) must be present when the type is specified.
15. `authentication_config.module` must be one of: `k8s`, `noop`, `noop-with-token`.
16. Provider `type` must be one of the supported provider types: openai, azure_openai, watsonx, rhoai_vllm, rhelai_vllm, google_vertex, google_vertex_anthropic, fake_provider.
16. Provider `type` must be one of the supported provider types: openai, azure_openai, watsonx, rhoai_vllm, rhelai_vllm, google_vertex, google_vertex_anthropic, bedrock, fake_provider.
17. `project_id` is required when provider type is `watsonx`.

### Tool Budget Computation
Expand Down
2 changes: 2 additions & 0 deletions .ai/spec/what/conversation-history.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ The conversation history subsystem preserves prior exchanges within a conversati

2. When a query-response exchange is stored, the system must either append it to an existing conversation (if one exists for the given user_id + conversation_id) or create a new conversation. Each stored exchange contains the user query (HumanMessage), the AI response (AIMessage), any attachments submitted with the query, any tool calls requested by the model, and any tool results returned from tool execution.

3. [PLANNED: OLS-3442] The AI response stored in the cache must include both text and reasoning content accumulated during streaming. Reasoning chunks (`StreamChunkType.REASONING`) must be accumulated into the response string alongside text chunks, so the model has access to its own reasoning within the current conversation turn. The response is stored as a plain-string `AIMessage` — no structured reasoning blocks or provider-specific signatures are preserved. Cache schema changes for structured reasoning storage are deferred until testing and evals show they improve multi-turn quality.

3. Retrieving history for a user_id + conversation_id must return all stored exchanges ordered oldest to newest. If no conversation exists for the given key, the system must return an empty result.

4. Listing conversations for a user must return metadata for every conversation belonging to that user, ordered by last message timestamp (most recent first). Each entry includes: conversation_id, topic_summary, last_message_timestamp, and message_count.
Expand Down
Loading