Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions app/Actions/UpdateCommitHeadlines.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
* @param list<string> $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);
Expand All @@ -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();
Comment thread
paulinevos marked this conversation as resolved.
Resolved
Comment thread
paulinevos marked this conversation as resolved.
Resolved
}

$created = 0;

foreach ($names as $name) {
Expand Down
9 changes: 6 additions & 3 deletions app/Console/Commands/LinkCommitHeadlinesCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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();
}

Expand Down
10 changes: 7 additions & 3 deletions app/Jobs/LinkCommitHeadlines.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,20 @@ final class LinkCommitHeadlines implements ShouldQueue
{
use Queueable;

public bool $reclassify = false;

/** @param list<string> $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);
}
Expand Down
14 changes: 10 additions & 4 deletions app/Jobs/ProcessCommitHeadlines.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,10 +32,14 @@ final class ProcessCommitHeadlines implements ShouldQueue

public const int BATCH_SIZE = 20;

public bool $reclassify = false;

/** @param list<string> $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
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
55 changes: 55 additions & 0 deletions tests/Feature/Commands/LinkCommitHeadlinesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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');
});
20 changes: 20 additions & 0 deletions tests/Unit/Actions/UpdateCommitHeadlinesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
11 changes: 9 additions & 2 deletions tests/Unit/Jobs/LinkCommitHeadlinesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,25 @@
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));

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']);
Expand Down
18 changes: 18 additions & 0 deletions tests/Unit/Jobs/ProcessCommitHeadlinesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down
Loading