Skip to content

Commit a41ef60

Browse files
felixarntzfelixarntz
authored
Merge pull request #20 from WordPress/fix-builder-chaining
Fix Prompt_Builder method chaining
2 parents d5a0251 + d196f39 commit a41ef60

File tree

4 files changed

+614
-2
lines changed

4 files changed

+614
-2
lines changed

‎includes/Builders/Prompt_Builder.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,14 @@ public function __construct( ProviderRegistry $registry, $prompt = null ) {
124124
*/
125125
public function __call( string $name, array $arguments ) {
126126
$callable = $this->get_builder_callable( $name );
127-
return $callable( ...$arguments );
127+
$result = $callable( ...$arguments );
128+
129+
// If the result is a PromptBuilder, return the current instance to allow method chaining.
130+
if ( $result instanceof PromptBuilder ) {
131+
return $this;
132+
}
133+
134+
return $result;
128135
}
129136

130137
/**

‎includes/Builders/Prompt_Builder_With_WP_Error.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,14 @@ public function __call( string $name, array $arguments ) {
144144
}
145145

146146
try {
147-
return $callable( ...$arguments );
147+
$result = $callable( ...$arguments );
148+
149+
// If the result is a PromptBuilder, return the current instance to allow method chaining.
150+
if ( $result instanceof PromptBuilder ) {
151+
return $this;
152+
}
153+
154+
return $result;
148155
} catch ( Exception $e ) {
149156
$this->error = new WP_Error(
150157
'prompt_builder_error',

‎tests/phpunit/tests/Builders/Prompt_Builder_Tests.php‎

Lines changed: 294 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,294 @@
1+
<?php
2+
/**
3+
* Tests for WordPress\AI_Client\Builders\Prompt_Builder
4+
*
5+
* @package WordPress\AI_Client
6+
*/
7+
8+
namespace WordPress\AI_Client\PHPUnit\Tests\Builders;
9+
10+
use BadMethodCallException;
11+
use ReflectionClass;
12+
use WordPress\AI_Client\Builders\Prompt_Builder;
13+
use WordPress\AI_Client\PHPUnit\Includes\Test_Case;
14+
use WordPress\AiClient\AiClient;
15+
use WordPress\AiClient\Builders\PromptBuilder;
16+
use WordPress\AiClient\Messages\DTO\Message;
17+
use WordPress\AiClient\Providers\Models\DTO\ModelConfig;
18+
19+
class Prompt_Builder_Tests extends Test_Case {
20+
21+
/**
22+
* Test that Prompt_Builder can be instantiated.
23+
*
24+
* @since n.e.x.t
25+
*/
26+
public function test_instantiation(): void {
27+
$registry = AiClient::defaultRegistry();
28+
$prompt_builder = new Prompt_Builder( $registry );
29+
30+
$this->assertInstanceOf( Prompt_Builder::class, $prompt_builder );
31+
32+
// Verify the wrapped builder is a PromptBuilder instance.
33+
$reflection_class = new ReflectionClass( Prompt_Builder::class );
34+
$builder_property = $reflection_class->getProperty( 'builder' );
35+
$builder_property->setAccessible( true );
36+
$wrapped_builder = $builder_property->getValue( $prompt_builder );
37+
38+
$this->assertInstanceOf( PromptBuilder::class, $wrapped_builder );
39+
}
40+
41+
/**
42+
* Test that Prompt_Builder can be instantiated with initial prompt content.
43+
*
44+
* @since n.e.x.t
45+
*/
46+
public function test_instantiation_with_prompt(): void {
47+
$registry = AiClient::defaultRegistry();
48+
$prompt_builder = new Prompt_Builder( $registry, 'Initial prompt text' );
49+
50+
$this->assertInstanceOf( Prompt_Builder::class, $prompt_builder );
51+
}
52+
53+
/**
54+
* Test method chaining with fluent methods.
55+
*
56+
* This tests the bug fix where methods that return the PromptBuilder instance
57+
* should instead return the Prompt_Builder decorator to allow proper chaining.
58+
*
59+
* @since n.e.x.t
60+
*/
61+
public function test_method_chaining_returns_decorator(): void {
62+
$registry = AiClient::defaultRegistry();
63+
$prompt_builder = new Prompt_Builder( $registry );
64+
65+
// Test chaining with_text which should return the decorator.
66+
$result = $prompt_builder->with_text( 'Test text' );
67+
$this->assertSame( $prompt_builder, $result, 'with_text should return the Prompt_Builder decorator instance' );
68+
$this->assertInstanceOf( Prompt_Builder::class, $result );
69+
70+
// Test chaining using_system_instruction.
71+
$result = $prompt_builder->using_system_instruction( 'System instruction' );
72+
$this->assertSame( $prompt_builder, $result, 'using_system_instruction should return the Prompt_Builder decorator instance' );
73+
$this->assertInstanceOf( Prompt_Builder::class, $result );
74+
75+
// Test chaining using_max_tokens.
76+
$result = $prompt_builder->using_max_tokens( 100 );
77+
$this->assertSame( $prompt_builder, $result, 'using_max_tokens should return the Prompt_Builder decorator instance' );
78+
$this->assertInstanceOf( Prompt_Builder::class, $result );
79+
80+
// Test chaining using_temperature.
81+
$result = $prompt_builder->using_temperature( 0.7 );
82+
$this->assertSame( $prompt_builder, $result, 'using_temperature should return the Prompt_Builder decorator instance' );
83+
$this->assertInstanceOf( Prompt_Builder::class, $result );
84+
85+
// Test chaining using_top_p.
86+
$result = $prompt_builder->using_top_p( 0.9 );
87+
$this->assertSame( $prompt_builder, $result, 'using_top_p should return the Prompt_Builder decorator instance' );
88+
$this->assertInstanceOf( Prompt_Builder::class, $result );
89+
90+
// Test chaining using_top_k.
91+
$result = $prompt_builder->using_top_k( 50 );
92+
$this->assertSame( $prompt_builder, $result, 'using_top_k should return the Prompt_Builder decorator instance' );
93+
$this->assertInstanceOf( Prompt_Builder::class, $result );
94+
95+
// Test chaining using_presence_penalty.
96+
$result = $prompt_builder->using_presence_penalty( 0.5 );
97+
$this->assertSame( $prompt_builder, $result, 'using_presence_penalty should return the Prompt_Builder decorator instance' );
98+
$this->assertInstanceOf( Prompt_Builder::class, $result );
99+
100+
// Test chaining using_frequency_penalty.
101+
$result = $prompt_builder->using_frequency_penalty( 0.5 );
102+
$this->assertSame( $prompt_builder, $result, 'using_frequency_penalty should return the Prompt_Builder decorator instance' );
103+
$this->assertInstanceOf( Prompt_Builder::class, $result );
104+
105+
// Test chaining as_output_mime_type.
106+
$result = $prompt_builder->as_output_mime_type( 'application/json' );
107+
$this->assertSame( $prompt_builder, $result, 'as_output_mime_type should return the Prompt_Builder decorator instance' );
108+
$this->assertInstanceOf( Prompt_Builder::class, $result );
109+
}
110+
111+
/**
112+
* Test complex method chaining scenario.
113+
*
114+
* This tests that multiple methods can be chained together fluently.
115+
*
116+
* @since n.e.x.t
117+
*/
118+
public function test_complex_method_chaining(): void {
119+
$registry = AiClient::defaultRegistry();
120+
$prompt_builder = new Prompt_Builder( $registry );
121+
122+
// Chain multiple methods together.
123+
$result = $prompt_builder
124+
->with_text( 'Test prompt' )
125+
->using_system_instruction( 'You are a helpful assistant' )
126+
->using_max_tokens( 500 )
127+
->using_temperature( 0.7 )
128+
->using_top_p( 0.9 );
129+
130+
// The final result should still be the same Prompt_Builder instance.
131+
$this->assertSame( $prompt_builder, $result, 'Chained methods should return the same Prompt_Builder decorator instance' );
132+
$this->assertInstanceOf( Prompt_Builder::class, $result );
133+
}
134+
135+
/**
136+
* Test that boolean-returning methods do not return the decorator.
137+
*
138+
* @since n.e.x.t
139+
*/
140+
public function test_boolean_methods_return_boolean(): void {
141+
$registry = AiClient::defaultRegistry();
142+
$prompt_builder = new Prompt_Builder( $registry, 'Test text' );
143+
144+
// Boolean methods should return boolean, not the decorator.
145+
$result = $prompt_builder->is_supported_for_text_generation();
146+
$this->assertIsBool( $result, 'is_supported_for_text_generation should return a boolean' );
147+
$this->assertNotSame( $prompt_builder, $result, 'is_supported_for_text_generation should not return the decorator' );
148+
}
149+
150+
/**
151+
* Test snake_case to camelCase conversion.
152+
*
153+
* This tests that snake_case method names are properly converted to camelCase
154+
* when proxying to the underlying PromptBuilder.
155+
*
156+
* @since n.e.x.t
157+
*/
158+
public function test_snake_case_to_camel_case_conversion(): void {
159+
$registry = AiClient::defaultRegistry();
160+
$prompt_builder = new Prompt_Builder( $registry );
161+
162+
// Test various snake_case patterns.
163+
$test_cases = array(
164+
'with_text' => 'withText',
165+
'using_system_instruction' => 'usingSystemInstruction',
166+
'using_max_tokens' => 'usingMaxTokens',
167+
'as_output_mime_type' => 'asOutputMimeType',
168+
'using_model_config' => 'usingModelConfig',
169+
'with_message_parts' => 'withMessageParts',
170+
'using_stop_sequences' => 'usingStopSequences',
171+
'using_candidate_count' => 'usingCandidateCount',
172+
'using_function_declarations' => 'usingFunctionDeclarations',
173+
);
174+
175+
$reflection_class = new ReflectionClass( Prompt_Builder::class );
176+
$conversion_method = $reflection_class->getMethod( 'snake_to_camel_case' );
177+
$conversion_method->setAccessible( true );
178+
179+
foreach ( $test_cases as $snake_case => $expected_camel_case ) {
180+
$actual_camel_case = $conversion_method->invoke( $prompt_builder, $snake_case );
181+
$this->assertSame( $expected_camel_case, $actual_camel_case, "Failed converting {$snake_case} to {$expected_camel_case}" );
182+
}
183+
}
184+
185+
/**
186+
* Test that calling a non-existent method throws an exception.
187+
*
188+
* @since n.e.x.t
189+
*/
190+
public function test_invalid_method_throws_exception(): void {
191+
$registry = AiClient::defaultRegistry();
192+
$prompt_builder = new Prompt_Builder( $registry );
193+
194+
$this->expectException( BadMethodCallException::class );
195+
$this->expectExceptionMessage( 'Method non_existent_method does not exist' );
196+
197+
$prompt_builder->non_existent_method();
198+
}
199+
200+
/**
201+
* Test that get_builder_callable returns a valid callable.
202+
*
203+
* @since n.e.x.t
204+
*/
205+
public function test_get_builder_callable(): void {
206+
$registry = AiClient::defaultRegistry();
207+
$prompt_builder = new Prompt_Builder( $registry );
208+
209+
$reflection_class = new ReflectionClass( Prompt_Builder::class );
210+
$callable_method = $reflection_class->getMethod( 'get_builder_callable' );
211+
$callable_method->setAccessible( true );
212+
213+
$callable = $callable_method->invoke( $prompt_builder, 'with_text' );
214+
$this->assertTrue( is_callable( $callable ), 'get_builder_callable should return a valid callable' );
215+
216+
// Verify the callable is an array with the wrapped builder and the camelCase method name.
217+
$this->assertIsArray( $callable );
218+
$this->assertCount( 2, $callable );
219+
$this->assertInstanceOf( PromptBuilder::class, $callable[0] );
220+
$this->assertSame( 'withText', $callable[1] );
221+
}
222+
223+
/**
224+
* Test that the wrapped builder is properly configured with the registry.
225+
*
226+
* @since n.e.x.t
227+
*/
228+
public function test_wrapped_builder_has_correct_registry(): void {
229+
$registry = AiClient::defaultRegistry();
230+
$prompt_builder = new Prompt_Builder( $registry );
231+
232+
$reflection_class = new ReflectionClass( Prompt_Builder::class );
233+
$builder_property = $reflection_class->getProperty( 'builder' );
234+
$builder_property->setAccessible( true );
235+
$wrapped_builder = $builder_property->getValue( $prompt_builder );
236+
237+
$wrapped_builder_reflection = new ReflectionClass( get_class( $wrapped_builder ) );
238+
$registry_property = $wrapped_builder_reflection->getProperty( 'registry' );
239+
$registry_property->setAccessible( true );
240+
241+
$this->assertSame( $registry, $registry_property->getValue( $wrapped_builder ), 'Wrapped builder should have the same registry' );
242+
}
243+
244+
/**
245+
* Test method chaining with with_history.
246+
*
247+
* @since n.e.x.t
248+
*/
249+
public function test_method_chaining_with_history(): void {
250+
$registry = AiClient::defaultRegistry();
251+
$prompt_builder = new Prompt_Builder( $registry );
252+
253+
$message1 = Message::fromArray(
254+
array(
255+
'role' => 'user',
256+
'parts' => array(
257+
array(
258+
'text' => 'Hello',
259+
),
260+
),
261+
)
262+
);
263+
$message2 = Message::fromArray(
264+
array(
265+
'role' => 'user',
266+
'parts' => array(
267+
array(
268+
'text' => 'How are you?',
269+
),
270+
),
271+
)
272+
);
273+
274+
$result = $prompt_builder->with_history( $message1, $message2 );
275+
$this->assertSame( $prompt_builder, $result, 'with_history should return the Prompt_Builder decorator instance' );
276+
$this->assertInstanceOf( Prompt_Builder::class, $result );
277+
}
278+
279+
/**
280+
* Test method chaining with using_model_config.
281+
*
282+
* @since n.e.x.t
283+
*/
284+
public function test_method_chaining_with_model_config(): void {
285+
$registry = AiClient::defaultRegistry();
286+
$prompt_builder = new Prompt_Builder( $registry );
287+
288+
$config = new ModelConfig( array( 'maxTokens' => 100 ) );
289+
290+
$result = $prompt_builder->using_model_config( $config );
291+
$this->assertSame( $prompt_builder, $result, 'using_model_config should return the Prompt_Builder decorator instance' );
292+
$this->assertInstanceOf( Prompt_Builder::class, $result );
293+
}
294+
}

0 commit comments

Comments
 (0)