perf: replace N+1 find() with whereIn batch lookup in checkDependencies()
Resolves #23 - Replace per-dependency `AgentPhase::find()` loop with a single `AgentPhase::whereIn('id', $dependencies)->get()` call, reducing query count from N to 1 for any number of dependencies - Short-circuit early when dependencies list is empty to avoid unnecessary query at all - Add tests: empty deps, skipped-dep passthrough, single-query assertion, blocker shape validation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
fcdeace290
commit
909c2da6df
2 changed files with 81 additions and 3 deletions
|
|
@ -317,11 +317,17 @@ class AgentPhase extends Model
|
|||
public function checkDependencies(): array
|
||||
{
|
||||
$dependencies = $this->dependencies ?? [];
|
||||
|
||||
if (empty($dependencies)) {
|
||||
return [];
|
||||
}
|
||||
|
||||
$blockers = [];
|
||||
|
||||
foreach ($dependencies as $depId) {
|
||||
$dep = AgentPhase::find($depId);
|
||||
if ($dep && ! $dep->isCompleted() && ! $dep->isSkipped()) {
|
||||
$deps = AgentPhase::whereIn('id', $dependencies)->get();
|
||||
|
||||
foreach ($deps as $dep) {
|
||||
if (! $dep->isCompleted() && ! $dep->isSkipped()) {
|
||||
$blockers[] = [
|
||||
'phase_id' => $dep->id,
|
||||
'phase_order' => $dep->order,
|
||||
|
|
|
|||
|
|
@ -286,6 +286,78 @@ class AgentPhaseTest extends TestCase
|
|||
$this->assertEquals($dep2->id, $blockers[0]['phase_id']);
|
||||
}
|
||||
|
||||
public function test_check_dependencies_returns_empty_when_no_dependencies(): void
|
||||
{
|
||||
$phase = AgentPhase::factory()->pending()->create([
|
||||
'agent_plan_id' => $this->plan->id,
|
||||
'dependencies' => null,
|
||||
]);
|
||||
|
||||
$this->assertSame([], $phase->checkDependencies());
|
||||
}
|
||||
|
||||
public function test_check_dependencies_not_blocked_by_skipped_phase(): void
|
||||
{
|
||||
$dep = AgentPhase::factory()->skipped()->create([
|
||||
'agent_plan_id' => $this->plan->id,
|
||||
'order' => 1,
|
||||
]);
|
||||
|
||||
$phase = AgentPhase::factory()->pending()->create([
|
||||
'agent_plan_id' => $this->plan->id,
|
||||
'order' => 2,
|
||||
'dependencies' => [$dep->id],
|
||||
]);
|
||||
|
||||
$this->assertSame([], $phase->checkDependencies());
|
||||
$this->assertTrue($phase->canStart());
|
||||
}
|
||||
|
||||
public function test_check_dependencies_uses_single_query_for_multiple_deps(): void
|
||||
{
|
||||
$deps = AgentPhase::factory()->pending()->count(5)->create([
|
||||
'agent_plan_id' => $this->plan->id,
|
||||
]);
|
||||
|
||||
$phase = AgentPhase::factory()->pending()->create([
|
||||
'agent_plan_id' => $this->plan->id,
|
||||
'dependencies' => $deps->pluck('id')->toArray(),
|
||||
]);
|
||||
|
||||
$queryCount = 0;
|
||||
\DB::listen(function () use (&$queryCount) {
|
||||
$queryCount++;
|
||||
});
|
||||
|
||||
$blockers = $phase->checkDependencies();
|
||||
|
||||
$this->assertCount(5, $blockers);
|
||||
$this->assertSame(1, $queryCount, 'checkDependencies() should issue exactly one query');
|
||||
}
|
||||
|
||||
public function test_check_dependencies_blocker_contains_expected_keys(): void
|
||||
{
|
||||
$dep = AgentPhase::factory()->inProgress()->create([
|
||||
'agent_plan_id' => $this->plan->id,
|
||||
'order' => 1,
|
||||
'name' => 'Blocker Phase',
|
||||
]);
|
||||
|
||||
$phase = AgentPhase::factory()->pending()->create([
|
||||
'agent_plan_id' => $this->plan->id,
|
||||
'order' => 2,
|
||||
'dependencies' => [$dep->id],
|
||||
]);
|
||||
|
||||
$blockers = $phase->checkDependencies();
|
||||
|
||||
$this->assertCount(1, $blockers);
|
||||
$this->assertEquals($dep->id, $blockers[0]['phase_id']);
|
||||
$this->assertEquals(1, $blockers[0]['phase_order']);
|
||||
$this->assertEquals('Blocker Phase', $blockers[0]['phase_name']);
|
||||
$this->assertEquals(AgentPhase::STATUS_IN_PROGRESS, $blockers[0]['status']);
|
||||
}
|
||||
|
||||
public function test_can_start_checks_dependencies(): void
|
||||
{
|
||||
$dep = AgentPhase::factory()->pending()->create([
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue