diff --git a/app/Actions/UpdateCommitHeadlines.php b/app/Actions/UpdateCommitHeadlines.php index 5f9269b8ac..888f2f72ed 100644 --- a/app/Actions/UpdateCommitHeadlines.php +++ b/app/Actions/UpdateCommitHeadlines.php @@ -15,9 +15,9 @@ * @param list $names * @return int|null Newly created headlines, or null when the result is stale or already applied. */ - public function handle(Commit $commit, array $names, string $bodyHash): ?int + public function handle(Commit $commit, array $names, string $bodyHash, bool $reclassify = false): ?int { - return DB::transaction(function () use ($commit, $names, $bodyHash): ?int { + return DB::transaction(function () use ($commit, $names, $bodyHash, $reclassify): ?int { $locked = Commit::withTrashed()->whereKey([$commit->id, $commit->quoted_commit_id]) ->orderBy('id')->lockForUpdate()->get()->keyBy('id'); $current = $locked->get($commit->id); @@ -26,10 +26,17 @@ public function handle(Commit $commit, array $names, string $bodyHash): ?int $current->setRelation('quotedCommit', $locked->get($current->quoted_commit_id ?? '')); } - if (! $current instanceof Commit || $current->trashed() || $current->parent_id !== null || $current->status !== CommitStatus::Approved || $current->body === '' || $current->quoted_commit_id !== $commit->quoted_commit_id || hash('sha256', $current->headlineBody()) !== $bodyHash || $current->headlines_classified_at !== null || $current->headlines()->exists()) { + if (! $current instanceof Commit || $current->trashed() || $current->parent_id !== null || $current->status !== CommitStatus::Approved || $current->body === '' || $current->quoted_commit_id !== $commit->quoted_commit_id || hash('sha256', $current->headlineBody()) !== $bodyHash || (! $reclassify && ($current->headlines_classified_at !== null || $current->headlines()->exists()))) { return null; } + if ($reclassify) { + $previousNames = $current->headlines()->pluck('name'); + Headline::query()->whereIn('name', $previousNames->diff($names)->merge(collect($names)->diff($previousNames))) + ->update(['summary' => null, 'summary_created_at' => null]); + $current->headlines()->detach(); + } + $created = 0; foreach ($names as $name) { diff --git a/app/Console/Commands/LinkCommitHeadlinesCommand.php b/app/Console/Commands/LinkCommitHeadlinesCommand.php index 35f69a7546..c2358fd4bd 100644 --- a/app/Console/Commands/LinkCommitHeadlinesCommand.php +++ b/app/Console/Commands/LinkCommitHeadlinesCommand.php @@ -9,8 +9,9 @@ use Illuminate\Console\Attributes\Description; use Illuminate\Console\Attributes\Signature; use Illuminate\Console\Command; +use Illuminate\Database\Eloquent\Builder; -#[Signature('commits:link-headlines {--batch-size='.ProcessCommitHeadlines::BATCH_SIZE.' : Commits per batch (1–100)}')] +#[Signature('commits:link-headlines {--batch-size='.ProcessCommitHeadlines::BATCH_SIZE.' : Commits per batch (1–100)} {--reclassify : Include already classified commits and replace their headline assignments}')] #[Description('Classify all unprocessed approved root commits in lazy batches and link shared headlines')] final class LinkCommitHeadlinesCommand extends Command { @@ -25,12 +26,14 @@ public function handle(): int } $processed = 0; + $reclassify = (bool) $this->option('reclassify'); $commits = Commit::query()->approved()->whereNull('parent_id')->where('body', '!=', '') - ->whereNull('headlines_classified_at')->whereDoesntHave('headlines')->select('id')->lazyById($batchSize); + ->when(! $reclassify, fn (Builder $query) => $query->whereNull('headlines_classified_at')->whereDoesntHave('headlines')) + ->select('id')->lazyById($batchSize); foreach ($commits->chunk($batchSize) as $batch) { - dispatch_sync(new ProcessCommitHeadlines(array_values($batch->map(fn (Commit $commit): string => $commit->id)->all()))); + dispatch_sync(new ProcessCommitHeadlines(array_values($batch->map(fn (Commit $commit): string => $commit->id)->all()), $reclassify)); $processed += $batch->count(); } diff --git a/app/Jobs/LinkCommitHeadlines.php b/app/Jobs/LinkCommitHeadlines.php index e930f34f29..c309ede597 100644 --- a/app/Jobs/LinkCommitHeadlines.php +++ b/app/Jobs/LinkCommitHeadlines.php @@ -23,16 +23,20 @@ final class LinkCommitHeadlines implements ShouldQueue { use Queueable; + public bool $reclassify = false; + /** @param list $names */ - public function __construct(public readonly Commit $commit, public readonly array $names, public readonly string $bodyHash) + public function __construct(public readonly Commit $commit, public readonly array $names, public readonly string $bodyHash, bool $reclassify = false) { - // + if ($reclassify) { + $this->reclassify = true; + } } public function handle(UpdateCommitHeadlines $action): ?int { try { - return $action->handle($this->commit, $this->names, $this->bodyHash); + return $action->handle($this->commit, $this->names, $this->bodyHash, $this->reclassify); } catch (Throwable $throwable) { throw new CommitProcessingFailed('Headline assignment', $this->commit->id, $throwable); } diff --git a/app/Jobs/ProcessCommitHeadlines.php b/app/Jobs/ProcessCommitHeadlines.php index 901a52dd95..9806cdb207 100644 --- a/app/Jobs/ProcessCommitHeadlines.php +++ b/app/Jobs/ProcessCommitHeadlines.php @@ -13,6 +13,7 @@ use Illuminate\Cache\Lock; use Illuminate\Contracts\Cache\LockTimeoutException; use Illuminate\Contracts\Queue\ShouldQueue; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Foundation\Queue\Queueable; use Illuminate\Queue\Attributes\Backoff; use Illuminate\Queue\Attributes\Timeout; @@ -31,10 +32,14 @@ final class ProcessCommitHeadlines implements ShouldQueue public const int BATCH_SIZE = 20; + public bool $reclassify = false; + /** @param list $commitIds */ - public function __construct(public readonly array $commitIds) + public function __construct(public readonly array $commitIds, bool $reclassify = false) { - // + if ($reclassify) { + $this->reclassify = true; + } } public static function dispatchFor(Commit ...$commits): void @@ -50,7 +55,8 @@ public function handle(HeadlineProvider $provider): void { $commits = Commit::query()->whereKey($this->commitIds)->whereNull('parent_id') ->where('status', CommitStatus::Approved)->where('body', '!=', '') - ->whereNull('headlines_classified_at')->whereDoesntHave('headlines')->with('quotedCommit')->orderBy('id')->get()->toBase(); + ->unless($this->reclassify, fn (Builder $query) => $query->whereNull('headlines_classified_at')->whereDoesntHave('headlines')) + ->with('quotedCommit')->orderBy('id')->get()->toBase(); if ($commits->isEmpty()) { return; @@ -129,7 +135,7 @@ private function linkCommits(Collection $commits, array $headlines, Lock $lock): throw_unless($lock->refresh(), LockTimeoutException::class); // Finish cheap link jobs before the next stage or backfill batch reads the headline pool. - $created = new LinkCommitHeadlines($commit, $headlines[$index], hash('sha256', $commit->headlineBody())) + $created = new LinkCommitHeadlines($commit, $headlines[$index], hash('sha256', $commit->headlineBody()), $this->reclassify) ->handle(resolve(UpdateCommitHeadlines::class)); if ($created !== null) { diff --git a/tests/Feature/Commands/LinkCommitHeadlinesTest.php b/tests/Feature/Commands/LinkCommitHeadlinesTest.php index 635d9347b8..cfa20f6fa2 100644 --- a/tests/Feature/Commands/LinkCommitHeadlinesTest.php +++ b/tests/Feature/Commands/LinkCommitHeadlinesTest.php @@ -3,6 +3,7 @@ declare(strict_types=1); use App\Contracts\Services\HeadlineProvider; +use App\Exceptions\CommitProcessingFailed; use App\Models\Commit; use App\Models\Headline; use Illuminate\Support\Facades\Log; @@ -81,3 +82,57 @@ $this->artisan('commits:link-headlines', ['--batch-size' => $batchSize]) ->expectsOutputToContain('The batch size must be an integer between 1 and 100.')->assertFailed(); })->with(['0', '-1', '1.5', 'abc', '101']); + +it('reclassifies eligible commits across batches and replaces rather than appends headlines', function (): void { + $this->travelTo(now()->startOfSecond()); + $old = Headline::factory()->create(['name' => 'Old headline', 'summary' => 'Old recap', 'summary_created_at' => now()]); + $changed = Commit::factory()->create(['body' => 'A new release', 'headlines_classified_at' => now()->subDay()]); + $unsuitable = Commit::factory()->create(['body' => 'Sunny day', 'headlines_classified_at' => now()->subDay()]); + $previouslyUnsuitable = Commit::factory()->create(['body' => 'More about the release', 'headlines_classified_at' => now()->subDay()]); + $changed->headlines()->attach($old); + $unsuitable->headlines()->attach($old); + Commit::factory()->for($changed, 'parent')->create(); + Commit::factory()->pending()->create(); + Commit::factory()->rejected()->create(); + Commit::factory()->create(['body' => '']); + Commit::factory()->create()->delete(); + $provider = $this->mock(HeadlineProvider::class); + $provider->shouldReceive('candidates')->with(['A new release'])->once()->andReturn([0]); + $provider->shouldReceive('candidates')->with(['Sunny day'])->once()->andReturn([]); + $provider->shouldReceive('candidates')->with(['More about the release'])->once()->andReturn([0]); + $provider->shouldReceive('matches')->with(['A new release'], ['Old headline'])->once()->andReturn([]); + $provider->shouldReceive('matches')->with([], ['Old headline', 'New release'])->once()->andReturn([]); + $provider->shouldReceive('matches')->with(['More about the release'], ['Old headline', 'New release'])->once()->andReturn([0 => ['New release']]); + $provider->shouldReceive('generate')->with(['A new release'])->once()->andReturn(['New release']); + $provider->shouldReceive('deduplicate')->with(['New release'])->once()->andReturn(['New release']); + $provider->shouldReceive('generate')->with([])->twice()->andReturn([]); + $provider->shouldReceive('deduplicate')->with([])->twice()->andReturn([]); + + $this->artisan('commits:link-headlines --reclassify --batch-size=1') + ->expectsOutputToContain('Processed 3 commits for headlines.')->assertSuccessful(); + $this->artisan('commits:link-headlines')->expectsOutputToContain('Processed 0 commits for headlines.')->assertSuccessful(); + + expect($changed->headlines()->pluck('name')->all())->toBe(['New release']) + ->and($changed->refresh()->headlines_classified_at?->equalTo(now()))->toBeTrue() + ->and($unsuitable->headlines()->count())->toBe(0) + ->and($unsuitable->refresh()->headlines_classified_at?->equalTo(now()))->toBeTrue() + ->and($previouslyUnsuitable->headlines()->sole()->name)->toBe('New release') + ->and($old->refresh()->summary)->toBeNull() + ->and($old->summary_created_at)->toBeNull(); +}); + +it('preserves the previous classification when reclassification fails', function (): void { + $this->travelTo(now()->startOfSecond()); + $commit = Commit::factory()->create(['headlines_classified_at' => now()->subDay()]); + $old = Headline::factory()->create(['summary' => 'Existing recap', 'summary_created_at' => now()]); + $commit->headlines()->attach($old); + $provider = $this->mock(HeadlineProvider::class); + $provider->shouldReceive('candidates')->once()->andReturn([0]); + $provider->shouldReceive('matches')->once()->andThrow(new RuntimeException('Provider unavailable')); + + expect(fn () => $this->artisan('commits:link-headlines --reclassify')->run())->toThrow(CommitProcessingFailed::class); + + expect($commit->headlines()->sole()->id)->toBe($old->id) + ->and($commit->refresh()->headlines_classified_at?->equalTo(now()->subDay()))->toBeTrue() + ->and($old->refresh()->summary)->toBe('Existing recap'); +}); diff --git a/tests/Unit/Actions/UpdateCommitHeadlinesTest.php b/tests/Unit/Actions/UpdateCommitHeadlinesTest.php index 351f4d999d..cfd0cb8a7b 100644 --- a/tests/Unit/Actions/UpdateCommitHeadlinesTest.php +++ b/tests/Unit/Actions/UpdateCommitHeadlinesTest.php @@ -35,6 +35,26 @@ ->and($headlines[2]->commits()->count())->toBe(0); }); +it('invalidates recaps only for headlines gaining or losing a commit during reclassification', function (): void { + $commit = Commit::factory()->create(['headlines_classified_at' => now()->subDay()]); + $headlines = Headline::factory()->count(4)->sequence( + ['name' => 'Previous topic'], ['name' => 'Destination topic'], ['name' => 'Retained topic'], ['name' => 'Unrelated topic'], + )->create(['summary' => 'Existing recap', 'summary_created_at' => now()]); + $commit->headlines()->attach([$headlines[0]->id, $headlines[2]->id]); + + resolve(UpdateCommitHeadlines::class)->handle($commit, ['Destination topic', 'Retained topic'], hash('sha256', $commit->body), reclassify: true); + + expect($commit->headlines()->pluck('name')->all())->toEqualCanonicalizing(['Destination topic', 'Retained topic']) + ->and($headlines[0]->refresh()->summary)->toBeNull() + ->and($headlines[0]->summary_created_at)->toBeNull() + ->and($headlines[1]->refresh()->summary)->toBeNull() + ->and($headlines[1]->summary_created_at)->toBeNull() + ->and($headlines[2]->refresh()->summary)->toBe('Existing recap') + ->and($headlines[2]->summary_created_at?->toDateTimeString())->toBe(now()->toDateTimeString()) + ->and($headlines[3]->refresh()->summary)->toBe('Existing recap') + ->and($headlines[3]->summary_created_at?->toDateTimeString())->toBe(now()->toDateTimeString()); +}); + it('creates returned headlines or reuses an identical name', function (bool $exists): void { $commit = Commit::factory()->create(['body' => 'What database are you using for vector search?']); if ($exists) { diff --git a/tests/Unit/Jobs/LinkCommitHeadlinesTest.php b/tests/Unit/Jobs/LinkCommitHeadlinesTest.php index 3722de39d9..347e3dc2b8 100644 --- a/tests/Unit/Jobs/LinkCommitHeadlinesTest.php +++ b/tests/Unit/Jobs/LinkCommitHeadlinesTest.php @@ -38,10 +38,17 @@ Queue::assertPushed(ProcessCommitHeadlines::class, fn (ProcessCommitHeadlines $job): bool => $job->commitIds === [$pending->id]); }); -it('assigns a headline once even when the job runs twice', function (): void { +it('assigns a headline once even when the job runs twice', function (bool $legacy): void { $commit = Commit::factory()->create(['body' => 'Trying pgvector.']); $job = new LinkCommitHeadlines($commit, ['Vector databases'], hash('sha256', $commit->body)); + if ($legacy) { + $payload = $job->__serialize(); + unset($payload['reclassify']); + $job = new ReflectionClass(LinkCommitHeadlines::class)->newInstanceWithoutConstructor(); + $job->__unserialize($payload); + } + $job->handle(resolve(UpdateCommitHeadlines::class)); $this->travel(1)->days(); $job->handle(resolve(UpdateCommitHeadlines::class)); @@ -49,7 +56,7 @@ expect($commit->headlines()->sole()->name)->toBe('Vector databases') ->and($commit->headlines()->sole()->getRelation('pivot')->created_at->toDateTimeString())->toBe(now()->subDay()->toDateTimeString()) ->and(Headline::query()->count())->toBe(1); -}); +})->with(['current payload' => false, 'legacy payload' => true]); it('retries failures without logging commit or provider content', function (): void { $commit = Commit::factory()->create(['body' => 'Private test body']); diff --git a/tests/Unit/Jobs/ProcessCommitHeadlinesTest.php b/tests/Unit/Jobs/ProcessCommitHeadlinesTest.php index 5e78b1f39f..717505420d 100644 --- a/tests/Unit/Jobs/ProcessCommitHeadlinesTest.php +++ b/tests/Unit/Jobs/ProcessCommitHeadlinesTest.php @@ -16,6 +16,24 @@ use Laravel\Ai\Classification; use Laravel\Ai\Responses\Data\BooleanAnswer; +it('processes pre-reclassification queued batches without reclassifying existing results', function (): void { + $commit = Commit::factory()->create(['body' => 'New release']); + $classified = Commit::factory()->create(['headlines_classified_at' => now()]); + $job = new ReflectionClass(ProcessCommitHeadlines::class)->newInstanceWithoutConstructor(); + $job->__unserialize(['commitIds' => [$commit->id, $classified->id]]); + + $provider = $this->mock(HeadlineProvider::class); + $provider->shouldReceive('candidates')->once()->with(['New release'])->andReturn([0]); + $provider->shouldReceive('matches')->once()->andReturn([]); + $provider->shouldReceive('generate')->once()->andReturn(['New release']); + $provider->shouldReceive('deduplicate')->once()->andReturn(['New release']); + + $job->handle($provider); + + expect($commit->headlines()->sole()->name)->toBe('New release') + ->and($classified->headlines()->count())->toBe(0); +}); + it('filters a batch, links matches and shares the first of similar generated headlines', function (): void { $commits = Commit::factory()->count(4)->sequence( ['body' => 'Sunny day'], ['body' => 'Trying pgvector'], ['body' => 'New Laravel AI features'], ['body' => 'Laravel SDK agents'],