Skip to content

Commit 1e00ae9

Browse files
fede-kamelclaude
andcommitted
Move integration test to correct folder structure
- Relocated test_parallel_tool_calling_integration.py to libs/oci/tests/integration_tests/chat_models/ - Follows repository convention for integration test organization - Addresses PR review feedback 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 97af4c1 commit 1e00ae9

File tree

2 files changed

+281
-0
lines changed

2 files changed

+281
-0
lines changed

libs/oci/OCI_API_GAP_ANALYSIS.md

Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
# OCI Generative AI API vs langchain-oci SDK Gap Analysis
2+
3+
## Executive Summary
4+
5+
This document provides a comprehensive comparison between the OCI Generative AI API specifications and the current langchain-oci implementation to identify missing functionality and parameters.
6+
7+
**Last Updated:** 2025-10-30
8+
**OCI SDK Version Analyzed:** 2.162.0
9+
**langchain-oci Version:** Current main branch
10+
11+
---
12+
13+
## Current Architecture
14+
15+
The langchain-oci implementation passes parameters through this flow:
16+
1. **Class-level:** `model_kwargs` dict in `ChatOCIGenAI.__init__()`
17+
2. **Method-level:** `kwargs` passed to `invoke()`, `stream()`, or `bind_tools()`
18+
3. **Merged:** Combined into `chat_params` at line 1186
19+
4. **Passed:** Directly to `oci_chat_request(**chat_params)` at line 1198
20+
21+
This means **most API parameters can technically be used via `model_kwargs`**, but they lack:
22+
- Type validation
23+
- Documentation
24+
- IDE autocomplete
25+
- First-class API support
26+
- User awareness
27+
28+
---
29+
30+
## Gap Categories
31+
32+
### **Fully Implemented**
33+
Parameters with first-class support and documentation
34+
35+
### ⚠️ **Partially Implemented**
36+
Parameters that work via `model_kwargs` but lack documentation/validation
37+
38+
### **Not Implemented**
39+
Parameters that don't work or have bugs
40+
41+
### 🔧 **Needs Fix**
42+
Features with known issues reported by users
43+
44+
---
45+
46+
## GenericChatRequest Parameters (Meta, Llama, Grok, OpenAI, Mistral)
47+
48+
### Core Parameters
49+
50+
| Parameter | OCI API | langchain-oci | Status | Notes |
51+
|-----------|---------|---------------|--------|-------|
52+
| `messages` | ✅ Required | ✅ Implemented | ✅ Fully Implemented | Via LangChain message types |
53+
| `api_format` | ✅ Required | ✅ Implemented | ✅ Fully Implemented | Auto-set to "GENERIC" |
54+
| `is_stream` | ✅ Optional | ✅ Implemented | ✅ Fully Implemented | Via `stream()` method |
55+
| `tools` | ✅ Optional | ✅ Implemented | ✅ Fully Implemented | Via `bind_tools()` |
56+
| `tool_choice` | ✅ Optional | ✅ Implemented | ✅ Fully Implemented | Via `bind_tools(tool_choice=...)` |
57+
58+
### Generation Control Parameters
59+
60+
| Parameter | OCI API | langchain-oci | Status | Notes |
61+
|-----------|---------|---------------|--------|-------|
62+
| `temperature` | ✅ Optional (float) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | Works but not documented as first-class param |
63+
| `max_tokens` | ✅ Optional (int) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | Works but not validated |
64+
| `max_completion_tokens` | ✅ Optional (int) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | Recommended for OpenAI models |
65+
| `top_k` | ✅ Optional (int) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | No validation |
66+
| `top_p` | ✅ Optional (float) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | No validation |
67+
| `stop` | ✅ Optional (list[str]) | ✅ Implemented | ✅ Fully Implemented | Via `invoke(stop=...)` |
68+
| `num_generations` | ✅ Optional (int) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | Not documented |
69+
70+
### Quality & Control Parameters
71+
72+
| Parameter | OCI API | langchain-oci | Status | Priority | Notes |
73+
|-----------|---------|---------------|--------|----------|-------|
74+
| `seed` | ✅ Optional (int) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **HIGH** | Critical for reproducibility |
75+
| `frequency_penalty` | ✅ Optional (float) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **HIGH** | Common quality control |
76+
| `presence_penalty` | ✅ Optional (float) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **HIGH** | Common quality control |
77+
| `reasoning_effort` | ✅ Optional (enum) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **MEDIUM** | For reasoning models |
78+
| `verbosity` | ✅ Optional (enum) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **LOW** | Response length control |
79+
80+
### Advanced Parameters
81+
82+
| Parameter | OCI API | langchain-oci | Status | Priority | Notes |
83+
|-----------|---------|---------------|--------|----------|-------|
84+
| `response_format` | ✅ Optional (object) | 🔧 **Broken** | 🔧 Needs Fix | **HIGH** | Issue #33 - Users need structured output |
85+
| `logit_bias` | ✅ Optional (dict) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **LOW** | Advanced use case |
86+
| `log_probs` | ✅ Optional (int) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **LOW** | Debugging/analysis |
87+
| `is_echo` | ✅ Optional (bool) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **LOW** | Include prompt in response |
88+
| `metadata` | ✅ Optional (object) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **LOW** | Custom metadata |
89+
| `prediction` | ✅ Optional (object) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **LOW** | Prediction configuration |
90+
91+
### Streaming Parameters
92+
93+
| Parameter | OCI API | langchain-oci | Status | Priority | Notes |
94+
|-----------|---------|---------------|--------|----------|-------|
95+
| `stream_options` | ✅ Optional (object) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **MEDIUM** | Streaming configuration |
96+
97+
### Specialized Features
98+
99+
| Parameter | OCI API | langchain-oci | Status | Priority | Notes |
100+
|-----------|---------|---------------|--------|----------|-------|
101+
| `web_search_options` | ✅ Optional (object) | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **MEDIUM** | Web search integration |
102+
| `is_parallel_tool_calls` | ✅ Optional (bool) |**Just Added!** | ✅ Fully Implemented | N/A | PR #59 - Complete |
103+
104+
---
105+
106+
## CohereChatRequest Parameters (Cohere Models)
107+
108+
### Core Parameters
109+
110+
| Parameter | OCI API | langchain-oci | Status | Notes |
111+
|-----------|---------|---------------|--------|-------|
112+
| `message` | ✅ Required (str) | ✅ Implemented | ✅ Fully Implemented | Converted from LangChain messages |
113+
| `chat_history` | ✅ Optional | ✅ Implemented | ✅ Fully Implemented | Via message history |
114+
| `api_format` | ✅ Required | ✅ Implemented | ✅ Fully Implemented | Auto-set to "COHERE" |
115+
| `is_stream` | ✅ Optional | ✅ Implemented | ✅ Fully Implemented | Via `stream()` method |
116+
| `tools` | ✅ Optional | ✅ Implemented | ✅ Fully Implemented | Via `bind_tools()` |
117+
118+
### Generation Control
119+
120+
| Parameter | OCI API | langchain-oci | Status | Notes |
121+
|-----------|---------|---------------|--------|-------|
122+
| `temperature` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | Works but undocumented |
123+
| `max_tokens` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | Works but undocumented |
124+
| `max_input_tokens` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | Cohere-specific |
125+
| `top_k` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | Works but undocumented |
126+
| `top_p` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | Works but undocumented |
127+
| `stop_sequences` | ✅ Optional | ✅ Implemented | ✅ Fully Implemented | Via `invoke(stop=...)` |
128+
129+
### Quality & Control
130+
131+
| Parameter | OCI API | langchain-oci | Status | Priority | Notes |
132+
|-----------|---------|---------------|--------|----------|-------|
133+
| `seed` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **HIGH** | Reproducibility |
134+
| `frequency_penalty` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **HIGH** | Quality control |
135+
| `presence_penalty` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **HIGH** | Quality control |
136+
137+
### Cohere-Specific Features
138+
139+
| Parameter | OCI API | langchain-oci | Status | Priority | Notes |
140+
|-----------|---------|---------------|--------|----------|-------|
141+
| `documents` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **MEDIUM** | RAG support |
142+
| `response_format` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **HIGH** | Cohere structured output |
143+
| `is_search_queries_only` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **LOW** | Search query generation |
144+
| `preamble_override` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **MEDIUM** | System prompt override |
145+
| `prompt_truncation` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **LOW** | Prompt handling |
146+
| `tool_results` | ✅ Optional | ✅ Implemented | ✅ Fully Implemented | N/A | Via ToolMessage |
147+
| `is_force_single_step` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **MEDIUM** | Multi-step control |
148+
| `is_raw_prompting` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **LOW** | Raw prompt mode |
149+
| `is_echo` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **LOW** | Echo prompt |
150+
| `citation_quality` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **LOW** | Citation mode |
151+
| `safety_mode` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **MEDIUM** | Safety controls |
152+
| `stream_options` | ✅ Optional | ⚠️ Via model_kwargs | ⚠️ Partially Implemented | **MEDIUM** | Streaming config |
153+
154+
---
155+
156+
## Known Issues (GitHub)
157+
158+
### 🔧 Critical Bugs
159+
160+
1. **Issue #52: Tool Parameter Parsing**
161+
- **Problem:** Llama models return escaped JSON strings `"{\\"key\\": \\"value\\"}"` instead of JSON objects
162+
- **Impact:** Multi-turn tool conversations fail
163+
- **Status:** Open, needs robust JSON unescaping
164+
- **Priority:** **HIGH**
165+
166+
2. **Issue #33: response_format Not Supported**
167+
- **Problem:** `TypeError: Unrecognized keyword argument: response_format`
168+
- **Impact:** Can't use `json_mode` with `with_structured_output()`
169+
- **Status:** Open, parameter exists in API but not exposed
170+
- **Priority:** **HIGH**
171+
172+
3. **Issue #28: Llama Tool Response Integration**
173+
- **Problem:** Llama calls tools but doesn't integrate results into final answer
174+
- **Impact:** Tool calling workflows broken for Llama
175+
- **Status:** Open, works with Cohere, fails with Llama
176+
- **Priority:** **HIGH**
177+
178+
### ⚠️ Medium Priority Issues
179+
180+
4. **Issue #40: Gemini Structured Output**
181+
- **Problem:** `with_structured_output()` doesn't work with Gemini 2.5
182+
- **Status:** Open, provider-specific handling needed
183+
- **Priority:** **MEDIUM**
184+
185+
5. **Issue #37: OpenAI Tool Schema**
186+
- **Problem:** Incomplete tool schema for OpenAI models, array arguments fail
187+
- **Status:** Open
188+
- **Priority:** **MEDIUM**
189+
190+
6. **Issue #45: Invalid Function Schema**
191+
- **Problem:** Tool schema validation failures
192+
- **Status:** Open
193+
- **Priority:** **MEDIUM**
194+
195+
### 📦 Feature Requests
196+
197+
7. **Issue #55: LangChain 1.0 Support**
198+
- **Status:** Open
199+
- **Priority:** **MEDIUM**
200+
201+
8. **Issue #5: Tool Description Required**
202+
- **Status:** Open
203+
- **Priority:** **LOW**
204+
205+
9. **Issue #4: InMemorySaver Checkpointer**
206+
- **Status:** Open
207+
- **Priority:** **LOW**
208+
209+
---
210+
211+
## Recommendations
212+
213+
### Immediate Priorities (High Impact, Low Effort)
214+
215+
1. **✅ DONE: `is_parallel_tool_calls`** - PR #59 merged
216+
2. **Fix Issue #33: `response_format` Support**
217+
- Expose as first-class parameter
218+
- Add validation for GenericChatRequest and CohereChatRequest
219+
- Document usage examples
220+
3. **Fix Issue #52: Robust JSON Parsing**
221+
- Add JSON unescape logic for tool arguments
222+
- Handle both `'{"key": "value"}'` and `'"{\\"key\\": \\"value\\"}"'`
223+
4. **Add First-Class Parameters:**
224+
- `seed` (reproducibility)
225+
- `frequency_penalty` (quality)
226+
- `presence_penalty` (quality)
227+
228+
### Medium-Term Improvements
229+
230+
5. **Fix Issue #28: Llama Tool Integration**
231+
- Debug message history handling
232+
- Ensure tool results properly incorporated
233+
6. **Documentation Enhancement**
234+
- Document all `model_kwargs` parameters
235+
- Add examples for each provider
236+
- Create migration guide from undocumented to first-class params
237+
238+
### Long-Term Enhancements
239+
240+
7. **Provider-Specific Features**
241+
- Cohere: `documents`, `preamble_override`, `safety_mode`
242+
- Generic: `reasoning_effort`, `web_search_options`
243+
8. **Advanced Features**
244+
- `logit_bias` support
245+
- `stream_options` configuration
246+
- `prediction` support
247+
248+
---
249+
250+
## Implementation Status Summary
251+
252+
| Category | Total Params | Fully Implemented | Partially Implemented | Not Implemented | Broken |
253+
|----------|--------------|-------------------|----------------------|----------------|--------|
254+
| **GenericChatRequest** | 26 | 6 (23%) | 19 (73%) | 0 (0%) | 1 (4%) |
255+
| **CohereChatRequest** | 25 | 7 (28%) | 18 (72%) | 0 (0%) | 0 (0%) |
256+
| **Total** | 51 | 13 (25%) | 37 (73%) | 0 (0%) | 1 (2%) |
257+
258+
### Key Insight
259+
260+
**73% of parameters work via `model_kwargs` but lack documentation!**
261+
262+
Most parameters are technically usable but users don't know about them because:
263+
- Not in function signatures
264+
- Not in documentation
265+
- No type validation
266+
- No examples
267+
268+
---
269+
270+
## Next Steps
271+
272+
1.**Completed:** Add `is_parallel_tool_calls` support (PR #59)
273+
2. **Fix:** `response_format` parameter (Issue #33)
274+
3. **Fix:** Tool argument parsing bug (Issue #52)
275+
4. **Add:** First-class support for `seed`, `frequency_penalty`, `presence_penalty`
276+
5. **Document:** All working `model_kwargs` parameters
277+
6. **Fix:** Llama tool response integration (Issue #28)
278+
279+
---
280+
281+
*This gap analysis will be updated as features are implemented and new issues are discovered.*

0 commit comments

Comments
 (0)