Skip to content

Commit 80fba31

Browse files
authored
Merge pull request #40 from ksylvan/0618-run-pattern-parameters
Pattern Runner: Model and Parameter Controls
2 parents 40437cf + e6fcba1 commit 80fba31

File tree

11 files changed

+1355
-183
lines changed

11 files changed

+1355
-183
lines changed

README.md

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ Imagine seamlessly using Fabric's specialized prompts for code explanation, refa
6161

6262
This project is currently in the **implementation** phase.
6363

64-
The core architecture and proposed tools are outlined in the [High-Level Design Document][design_doc].
64+
The core architecture and proposed tools are outlined in the [High-Level Architecture Document][architecture_doc].
6565

6666
```bash
6767
# you can also use pnpm if you prefer
@@ -177,8 +177,6 @@ For more details on transport configuration, see the [Infrastructure and Deploym
177177
178178
## Contributing
179179
180-
Feedback on the [design document][design_doc] is highly welcome! Please open an issue to share your thoughts or suggestions.
181-
182180
Read the [contribution document here](./docs/contributing.md) and please follow the guidelines for this repository.
183181
184182
Also refer to the [cheat-sheet for contributors](./docs/contributing-cheatsheet.md) which contains a micro-summary of the
@@ -190,7 +188,7 @@ Copyright (c) 2025, [Kayvan Sylvan](kayvan@sylvan.com) Licensed under the [MIT L
190188
191189
[fabricGithubLink]: https://github.com/danielmiessler/fabric
192190
[MCP]: https://modelcontextprotocol.io/
193-
[design_doc]: ./docs/design.md
191+
[architecture_doc]: ./docs/architecture/index.md
194192
[develop_publish_link]: https://github.com/ksylvan/fabric-mcp/actions/workflows/publish.yml?branch=develop
195193
[develop_publish]: https://github.com/ksylvan/fabric-mcp/actions/workflows/publish.yml/badge.svg?branch=develop
196194
[develop_tests_link]: https://github.com/ksylvan/fabric-mcp/actions/workflows/tests.yml?branch=develop

docs/stories/3.3.story.md

Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,230 @@
1+
# Story 3.3: Enhance `fabric_run_pattern` with Execution Control Parameters (Model, LLM Params, Strategy)
2+
3+
## Status: Review
4+
5+
## Story
6+
7+
- As an MCP Client Developer
8+
- I want to enhance the `fabric_run_pattern` tool to allow specifying optional `model_name`, LLM tuning parameters (`temperature`, `top_p`, `presence_penalty`, `frequency_penalty`), and an optional `strategy_name` (selected from those available via `fabric_list_strategies`)
9+
- so that I can have finer control over the pattern execution by the Fabric instance.
10+
11+
## Acceptance Criteria (ACs)
12+
13+
### AC1: Tool Parameter Extension
14+
15+
1. `fabric_run_pattern` definition via `list_tools()` updated for optional `model_name` (str), `temperature` (float), `top_p` (float), `presence_penalty` (float), `frequency_penalty` (float), `strategy_name` (str)
16+
2. Tool signature maintains backward compatibility with existing parameters: `pattern_name` (required), `input_text` (optional), `stream` (optional)
17+
3. All new parameters are optional and have appropriate default values or None handling
18+
4. Tool docstring accurately describes all execution control parameters and their effects
19+
20+
### AC2: Request Construction with Parameters
21+
22+
1. Tool parses these optional params; if provided, includes them in Fabric API request (e.g., `PromptRequest.Model`, `ChatOptions`, `PromptRequest.StrategyName`)
23+
2. When `model_name` provided, sets both `model` and `vendor` fields in PromptRequest appropriately
24+
3. LLM tuning parameters (`temperature`, `top_p`, `presence_penalty`, `frequency_penalty`) included at ChatRequest root level when provided
25+
4. `strategy_name` included in PromptRequest when provided
26+
5. Omits parameters from request if not provided (maintains existing defaults)
27+
28+
### AC3: Parameter Validation
29+
30+
1. Validates `model_name` format and availability (integration with `fabric_list_models` data if possible)
31+
2. Validates `strategy_name` against available strategies (integration with `fabric_list_strategies` data if possible)
32+
3. Validates LLM parameter ranges (temperature: 0.0-2.0, top_p: 0.0-1.0, penalties: -2.0-2.0)
33+
4. Returns structured MCP error for invalid parameter values with descriptive messages
34+
35+
### AC4: Error Handling Enhancement
36+
37+
1. Returns structured MCP error if Fabric API rejects these parameters with specific parameter error details
38+
2. Graceful handling when Fabric instance doesn't support specified model or strategy
39+
3. Maintains existing error handling for connection failures and malformed responses
40+
4. Error messages clearly indicate which parameter caused the failure
41+
42+
### AC5: Backward Compatibility
43+
44+
1. Existing `fabric_run_pattern` calls without new parameters continue to work unchanged
45+
2. Default behavior remains identical to Story 3.1 implementation when no new parameters provided
46+
3. Response format unchanged: `{"output_format": "string", "output_text": "string"}`
47+
4. Non-streaming and streaming modes both support new parameters
48+
49+
### AC6: Unit Tests
50+
51+
1. Mock `FabricApiClient` for request construction with these params (individual, mixed, omitted)
52+
2. Test parameter validation for valid and invalid values
53+
3. Test backward compatibility with existing parameter combinations
54+
4. Test API errors for invalid params return appropriate MCP errors
55+
5. Test request formatting matches expected Fabric API format with new parameters
56+
6. Achieve ≥95% test coverage for enhanced implementation
57+
58+
### AC7: Integration Tests
59+
60+
1. Test `model_name` override affects output when using different models
61+
2. Test `strategy_name` application (e.g., "cot") affects pattern execution output
62+
3. Test LLM tuning parameters affect output behavior when changed
63+
4. Test invalid model/strategy names return appropriate MCP errors from live Fabric API
64+
5. Test parameter combinations work correctly through full MCP flow
65+
6. Ensure integration tests run in CI/CD without external dependencies
66+
67+
## Tasks / Subtasks
68+
69+
- [x] **Task 1: Extend Tool Definition and Parameter Handling** (AC: 1, 5)
70+
- [x] Subtask 1.1: Update `fabric_run_pattern` method signature with new optional parameters
71+
- [x] Subtask 1.2: Update `list_tools()` registration to include new parameter definitions
72+
- [x] Subtask 1.3: Update method docstring with comprehensive parameter descriptions
73+
- [x] Subtask 1.4: Verify backward compatibility with existing parameter handling
74+
75+
- [x] **Task 2: Implement Parameter Validation** (AC: 3)
76+
- [x] Subtask 2.1: Add validation functions for LLM parameters (temperature, top_p, penalties)
77+
- [x] Subtask 2.2: Add model_name format validation and availability checking
78+
- [x] Subtask 2.3: Add strategy_name validation against available strategies
79+
- [x] Subtask 2.4: Create structured MCP errors for validation failures
80+
81+
- [x] **Task 3: Enhance Request Construction** (AC: 2)
82+
- [x] Subtask 3.1: Update ChatRequest construction to include LLM parameters when provided
83+
- [x] Subtask 3.2: Update PromptRequest construction to include model and strategy when provided
84+
- [x] Subtask 3.3: Implement model_name to vendor/model field mapping logic
85+
- [x] Subtask 3.4: Ensure parameter omission maintains existing API defaults
86+
87+
- [x] **Task 4: Enhance Error Handling** (AC: 4)
88+
- [x] Subtask 4.1: Handle Fabric API parameter rejection errors specifically
89+
- [x] Subtask 4.2: Add parameter-specific error messages for debugging
90+
- [x] Subtask 4.3: Maintain existing error handling patterns for compatibility
91+
- [x] Subtask 4.4: Test error scenarios with invalid parameter combinations
92+
93+
- [x] **Task 5: Update Mock Server for Enhanced Testing** (AC: 7)
94+
- [x] Subtask 5.1: Extend mock server `/chat` endpoint to handle new parameters
95+
- [x] Subtask 5.2: Add test data for different models and strategies
96+
- [x] Subtask 5.3: Add endpoints for testing parameter validation errors
97+
- [x] Subtask 5.4: Ensure mock server matches enhanced Fabric API request format
98+
99+
- [x] **Task 6: Implement Comprehensive Unit Tests** (AC: 6)
100+
- [x] Subtask 6.1: Create tests for parameter validation (valid/invalid ranges)
101+
- [x] Subtask 6.2: Test request construction with various parameter combinations
102+
- [x] Subtask 6.3: Test backward compatibility scenarios
103+
- [x] Subtask 6.4: Test error handling for parameter-related failures
104+
- [x] Subtask 6.5: Verify ≥95% code coverage for enhanced functionality
105+
106+
- [x] **Task 7: Implement Enhanced Integration Tests** (AC: 7)
107+
- [x] Subtask 7.1: Test model_name override with different available models
108+
- [x] Subtask 7.2: Test strategy_name application with available strategies
109+
- [x] Subtask 7.3: Test LLM parameter effects on pattern execution
110+
- [x] Subtask 7.4: Test parameter validation through complete MCP flow
111+
- [x] Subtask 7.5: Test error scenarios with invalid parameters
112+
113+
## Dev Notes
114+
115+
### Relevant Source Files
116+
117+
- **Implementation**: `src/fabric_mcp/core.py` - `fabric_run_pattern()` method (enhance existing implementation from Story 3.1)
118+
- **API Client**: `src/fabric_mcp/api_client.py` - `FabricApiClient.post()` method for HTTP calls
119+
- **Mock Server**: `tests/shared/fabric_api/server.py` - Enhance `/chat` endpoint for new parameters
120+
- **Unit Tests**: Enhance existing `tests/unit/test_fabric_run_pattern.py`
121+
- **Integration Tests**: `tests/integration/test_transport_integration.py`
122+
123+
### Previous Story Implementation Context
124+
125+
From Story 3.1 completion notes:
126+
127+
- Basic `fabric_run_pattern` implementation completed with SSE streaming support
128+
- Request format established: ChatRequest with prompts array containing PromptRequest objects
129+
- Response parsing handles SSE stream: content chunks accumulated until "complete" type received
130+
- Error handling established for Fabric API errors and connection failures
131+
132+
[Source: docs/stories/3.1.story.md#completion-notes]
133+
134+
### Technical Implementation Details
135+
136+
**Enhanced ChatRequest Format** [Source: architecture/api-reference.md#fabric-rest-api]:
137+
138+
```json
139+
{
140+
"prompts": [
141+
{
142+
"userInput": "string",
143+
"patternName": "string",
144+
"model": "string", // NEW: from model_name parameter
145+
"vendor": "string", // NEW: derived from model_name
146+
"strategyName": "string" // NEW: from strategy_name parameter
147+
}
148+
],
149+
"temperature": "float", // NEW: from temperature parameter
150+
"topP": "float", // NEW: from top_p parameter
151+
"frequencyPenalty": "float", // NEW: from frequency_penalty parameter
152+
"presencePenalty": "float" // NEW: from presence_penalty parameter
153+
}
154+
```
155+
156+
**Parameter Validation Ranges**:
157+
158+
- temperature: 0.0 to 2.0 (typical LLM range)
159+
- top_p: 0.0 to 1.0 (probability range)
160+
- presence_penalty: -2.0 to 2.0 (OpenAI-style range)
161+
- frequency_penalty: -2.0 to 2.0 (OpenAI-style range)
162+
163+
**Model Name Handling**:
164+
165+
- Use existing `fabric_list_models` pattern to validate available models
166+
- Parse model_name to extract vendor and model components if formatted as "vendor/model"
167+
- Default vendor handling when model_name provided without vendor prefix
168+
169+
[Source: architecture/api-reference.md#internal-apis-provided]
170+
171+
### File Locations and Structure
172+
173+
**Project Structure** [Source: architecture/project-structure.md]:
174+
175+
- Core implementation: `src/fabric_mcp/core.py`
176+
- Unit tests: `tests/unit/test_fabric_run_pattern.py`
177+
- Integration tests: `tests/integration/test_transport_integration.py`
178+
- Mock server: `tests/shared/fabric_api/server.py`
179+
180+
**Coding Standards** [Source: architecture/coding-standards.md]:
181+
182+
- Type hints mandatory for all new parameters
183+
- Use `snake_case` for parameter names
184+
- Comprehensive docstrings with parameter descriptions
185+
- Error handling with custom exception hierarchy
186+
187+
### Testing
188+
189+
Dev Note: Story Requires the following tests:
190+
191+
- [ ] **pytest** Unit Tests: (nextToFile: true), coverage requirement: ≥95%
192+
- [ ] **pytest** Integration Test (Test Location): location: `tests/integration/test_transport_integration.py`
193+
- [ ] **Manual** E2E: location: Manual testing steps provided below
194+
195+
**Manual Test Steps:**
196+
197+
After implementation completion, verify enhanced functionality:
198+
199+
1. Start local Fabric server: `fabric --serve`
200+
2. Verify available models: Use MCP Inspector to call `fabric_list_models`
201+
3. Verify available strategies: Use MCP Inspector to call `fabric_list_strategies`
202+
4. Test model override: Use MCP Inspector to call `fabric_run_pattern` with different `model_name` values
203+
5. Test strategy application: Use MCP Inspector to call `fabric_run_pattern` with `strategy_name` (e.g., "cot")
204+
6. Test LLM parameters: Use MCP Inspector to call `fabric_run_pattern` with different `temperature` values (0.1, 0.7, 1.5)
205+
7. Test parameter validation: Try invalid values and verify appropriate error responses
206+
8. Test backward compatibility: Verify existing calls without new parameters still work
207+
208+
## Dev Agent Record
209+
210+
### Agent Model Used: Claude 3.5 Sonnet (via Warp AI Terminal)
211+
212+
### Debug Log References
213+
214+
No debug log entries required - implementation proceeded smoothly without requiring temporary fixes or reverts.
215+
216+
### Completion Notes List
217+
218+
- **Story 3.3 Implementation Completed Successfully**: All 7 tasks completed with comprehensive parameter validation, request construction, and testing
219+
- **Enhanced Parameter Support**: Added 6 new optional parameters (model_name, temperature, top_p, presence_penalty, frequency_penalty, strategy_name) with full validation
220+
- **Backward Compatibility Maintained**: All existing functionality preserved - no breaking changes introduced
221+
- **Comprehensive Test Coverage**: Added 12 new unit tests and 6 new integration tests covering all parameter scenarios
222+
- **Mock Server Enhanced**: Updated mock server to handle new parameters with realistic responses for testing
223+
- **Code Quality Maintained**: All linting, formatting, and type checking requirements met
224+
- **Total Test Coverage**: 177 tests passing (including all existing + new Story 3.3 tests)
225+
226+
### Change Log
227+
228+
| Date | Version | Description | Author |
229+
| :--- | :------ | :---------- | :----- |
230+
| 2025-06-18 | 1.0 | Initial Story 3.3 implementation with all tasks complete | James (Dev Agent) |

src/fabric_mcp/__about__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
"Version information for fabric_mcp."
22

3-
__version__ = "0.17.0"
3+
__version__ = "0.18.0"

0 commit comments

Comments
 (0)