From 31ed52500a9733e58668a80cd25272183492fa91 Mon Sep 17 00:00:00 2001 From: Nyan Lin Paing <117423022+LinPaing21@users.noreply.github.com> Date: Tue, 1 Sep 2026 00:16:17 +0700 Subject: [PATCH] add suggestion management --- .ai/rules/pages.md | 5 + .claude/settings.local.json | 6 + .../pages/manage-suggestion-misses.blade.php | 3 + .../pages/manage-suggestions.blade.php | 15 + .../Concerns/HandlesBnfexpressErrors.php | 17 + .../Concerns/PaginatesBnfexpressLists.php | 33 ++ .../Filament/Pages/ManageSuggestionMisses.php | 150 +++++++ .../src/Filament/Pages/ManageSuggestions.php | 414 ++++++++++++++++++ .../Feature/ManageSuggestionMissesTest.php | 89 ++++ .../tests/Feature/ManageSuggestionsTest.php | 187 ++++++++ .../src/Bnfexpress/BnfexpressAdminClient.php | 191 +++++++- .../tests/Unit/BnfexpressAdminClientTest.php | 121 +++++ 12 files changed, 1226 insertions(+), 5 deletions(-) create mode 100644 .claude/settings.local.json create mode 100644 app-modules/ai-agent/resources/views/filament/pages/manage-suggestion-misses.blade.php create mode 100644 app-modules/ai-agent/resources/views/filament/pages/manage-suggestions.blade.php create mode 100644 app-modules/ai-agent/src/Filament/Concerns/PaginatesBnfexpressLists.php create mode 100644 app-modules/ai-agent/src/Filament/Pages/ManageSuggestionMisses.php create mode 100644 app-modules/ai-agent/src/Filament/Pages/ManageSuggestions.php create mode 100644 app-modules/ai-agent/tests/Feature/ManageSuggestionMissesTest.php create mode 100644 app-modules/ai-agent/tests/Feature/ManageSuggestionsTest.php diff --git a/.ai/rules/pages.md b/.ai/rules/pages.md index c6d36aa..be1918f 100644 --- a/.ai/rules/pages.md +++ b/.ai/rules/pages.md @@ -11,3 +11,8 @@ A `Filament\Pages\Page implements HasTable` (not a Resource) does NOT render its Filament v4 tables default to deferred loading. In Pest/Livewire tests, call `->loadTable()` before any `assertSee()`/`assertCanSeeTableRecords()` on a freshly-mounted component, or the table body won't be in the rendered HTML yet. For custom-data (`->records()`-backed, non-Eloquent) tables: use `->callTableAction($name, $record, data: [...])` / `->mountTableAction(...)` (not the generic `->callAction()`, which targets page-level actions and misses table header/record actions), and use `->assertMountedActionModalSee(...)` to check `->modalContent()` output — modal content is lazily rendered and won't appear in a plain `->html()`/`->assertSee()` snapshot even after mounting the action. See `[[project_internachi_modular]]`-style module layout in `app-modules/ai-agent`. + +## Custom-data table bulk actions: fetchSelectedRecords(false) still hydrates full rows +On a `Table::records()`-backed (non-Eloquent) page, `BulkAction::make(...)->fetchSelectedRecords(false)` does NOT skip hydration the way it does for an Eloquent table — the `Collection $records` passed to `->action()` still contains full row arrays (keyed by the record key), not bare ids. Use `$records->keys()->all()` to get just the selected ids; `$records->all()`/`$records->values()` gives you full row data instead. See `ManageSuggestions::deleteSelectedBulkAction()` / `ManageSuggestionMisses::promoteBulkAction()`. + +Also: `BnfexpressAdminClient`'s non-2xx handling (`errorMessage()`) must handle `detail` being a list of `{msg, ...}` objects, not just a string — FastAPI's own request-validation failures (422s) return `detail` in that shape, and casting it straight to `(string)` silently produces the literal "Array". diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 0000000..78c8763 --- /dev/null +++ b/.claude/settings.local.json @@ -0,0 +1,6 @@ +{ + "enabledMcpjsonServers": [ + "laravel-boost" + ], + "enableAllProjectMcpServers": true +} diff --git a/app-modules/ai-agent/resources/views/filament/pages/manage-suggestion-misses.blade.php b/app-modules/ai-agent/resources/views/filament/pages/manage-suggestion-misses.blade.php new file mode 100644 index 0000000..ce096a2 --- /dev/null +++ b/app-modules/ai-agent/resources/views/filament/pages/manage-suggestion-misses.blade.php @@ -0,0 +1,3 @@ + + {{ $this->table }} + diff --git a/app-modules/ai-agent/resources/views/filament/pages/manage-suggestions.blade.php b/app-modules/ai-agent/resources/views/filament/pages/manage-suggestions.blade.php new file mode 100644 index 0000000..31378b7 --- /dev/null +++ b/app-modules/ai-agent/resources/views/filament/pages/manage-suggestions.blade.php @@ -0,0 +1,15 @@ + + + @if ($syncJobId) +
+ Job {{ $syncJobId }}: {{ $syncStatus }}... +
+ @elseif ($syncResult) + 
{{ json_encode($syncResult, JSON_PRETTY_PRINT) }}
+ @else +

No sync running.

+ @endif + + + {{ $this->table }} + diff --git a/app-modules/ai-agent/src/Filament/Concerns/HandlesBnfexpressErrors.php b/app-modules/ai-agent/src/Filament/Concerns/HandlesBnfexpressErrors.php index b293eba..d7d4579 100644 --- a/app-modules/ai-agent/src/Filament/Concerns/HandlesBnfexpressErrors.php +++ b/app-modules/ai-agent/src/Filament/Concerns/HandlesBnfexpressErrors.php @@ -25,4 +25,21 @@ trait HandlesBnfexpressErrors Notification::make()->title($failureTitle)->body($exception->getMessage())->danger()->send(); } } + + /** + * Same shape as callBnfexpress(), but for calls whose success notification + * needs the response (e.g. a "{created} created, {skipped} skipped" body) — + * $onSuccess builds/sends its own Notification from $callback()'s return value. + * + * @param callable(): mixed $callback + * @param callable(mixed): void $onSuccess + */ + protected function callBnfexpressForResult(callable $callback, callable $onSuccess, string $failureTitle): void + { + try { + $onSuccess($callback()); + } catch (BnfexpressApiException $exception) { + Notification::make()->title($failureTitle)->body($exception->getMessage())->danger()->send(); + } + } } diff --git a/app-modules/ai-agent/src/Filament/Concerns/PaginatesBnfexpressLists.php b/app-modules/ai-agent/src/Filament/Concerns/PaginatesBnfexpressLists.php new file mode 100644 index 0000000..6446ae8 --- /dev/null +++ b/app-modules/ai-agent/src/Filament/Concerns/PaginatesBnfexpressLists.php @@ -0,0 +1,33 @@ +} envelope in case that ever changes. + */ +trait PaginatesBnfexpressLists +{ + /** + * @param array $result + */ + private function paginateBareList(array $result, string $itemsKey, string $recordKey, int $page, int $recordsPerPage): LengthAwarePaginator + { + $items = $result[$itemsKey] ?? (array_is_list($result) ? $result : []); + + $total = $result['total'] ?? (($page - 1) * $recordsPerPage) + count($items) + (count($items) === $recordsPerPage ? 1 : 0); + + return new LengthAwarePaginator( + items: collect($items)->mapWithKeys(fn (array $item): array => [$item[$recordKey] => $item]), + total: $total, + perPage: $recordsPerPage, + currentPage: $page, + ); + } +} diff --git a/app-modules/ai-agent/src/Filament/Pages/ManageSuggestionMisses.php b/app-modules/ai-agent/src/Filament/Pages/ManageSuggestionMisses.php new file mode 100644 index 0000000..4ffec1c --- /dev/null +++ b/app-modules/ai-agent/src/Filament/Pages/ManageSuggestionMisses.php @@ -0,0 +1,150 @@ +user()?->can('manage_ai_agent') ?? false; + } + + public function table(Table $table): Table + { + return $table + ->records(function (array $filters, int $page, int $recordsPerPage): LengthAwarePaginator { + $wasUsed = $filters['was_used']['value'] ?? null; + + $result = app(BnfexpressAdminClient::class)->listSuggestionMisses( + wasUsed: $wasUsed === null || $wasUsed === '' ? null : (bool) $wasUsed, + limit: $recordsPerPage, + offset: ($page - 1) * $recordsPerPage, + ); + + return $this->paginateBareList($result, 'misses', 'id', $page, $recordsPerPage); + }) + ->columns([ + TextColumn::make('id'), + TextColumn::make('text_norm') + ->limit(80) + ->wrap(), + TextColumn::make('lang') + ->placeholder('—'), + TextColumn::make('syllables') + ->placeholder('—'), + IconColumn::make('was_used') + ->boolean(), + TextColumn::make('created_at') + ->dateTime() + ->sortable(), + ]) + ->defaultSort('created_at', 'desc') + // SelectFilter (not TernaryFilter) so the value lands in + // $filters['was_used']['value'] predictably — TernaryFilter's + // internal field key isn't documented for the custom-data path. + ->filters([ + SelectFilter::make('was_used') + ->label('Used?') + ->options(['1' => 'Used', '0' => 'Not used']), + ]) + ->recordActions([ + $this->dismissAction(), + ]) + ->toolbarActions([ + $this->promoteBulkAction(), + ]); + } + + protected function dismissAction(): Action + { + return Action::make('dismiss') + ->color('danger') + ->icon(Heroicon::OutlinedTrash) + ->requiresConfirmation() + ->action(function (array $record): void { + $this->callBnfexpress( + fn () => app(BnfexpressAdminClient::class)->dismissSuggestionMiss($record['id']), + successTitle: 'Miss dismissed', + failureTitle: 'Failed to dismiss miss', + ); + + $this->resetTable(); + }); + } + + protected function promoteBulkAction(): BulkAction + { + return BulkAction::make('promote') + ->label('Promote Selected') + ->icon(Heroicon::OutlinedArrowUp) + ->fetchSelectedRecords(false) + ->schema([ + TextInput::make('lang') + ->label('Language override') + ->helperText("Applied to every selected miss; leave blank to keep each one's own language.") + ->maxLength(10), + TextInput::make('intent'), + ]) + ->deselectRecordsAfterCompletion() + ->action(function (array $data, Collection $records): void { + // Same caveat as ManageSuggestions' deleteSelected — the collection + // holds full row arrays, not just keys, for a custom-data table. + $this->callBnfexpressForResult( + fn () => app(BnfexpressAdminClient::class)->promoteSuggestionMisses( + $records->keys()->all(), + $data['lang'] ?: null, + $data['intent'] ?: null, + ), + function (array $result): void { + Notification::make() + ->title("{$result['created']} promoted, {$result['skipped']} skipped") + ->success() + ->send(); + + $this->resetTable(); + }, + failureTitle: 'Failed to promote suggestion misses', + ); + }); + } +} diff --git a/app-modules/ai-agent/src/Filament/Pages/ManageSuggestions.php b/app-modules/ai-agent/src/Filament/Pages/ManageSuggestions.php new file mode 100644 index 0000000..4353e94 --- /dev/null +++ b/app-modules/ai-agent/src/Filament/Pages/ManageSuggestions.php @@ -0,0 +1,414 @@ +user()?->can('manage_ai_agent') ?? false; + } + + public function table(Table $table): Table + { + return $table + ->records(function (?string $search, int $page, int $recordsPerPage): LengthAwarePaginator { + $result = app(BnfexpressAdminClient::class)->listSuggestions( + q: $search, + limit: $recordsPerPage, + offset: ($page - 1) * $recordsPerPage, + ); + + return $this->paginateBareList($result, 'suggestions', 'id', $page, $recordsPerPage); + }) + ->columns([ + TextColumn::make('id'), + TextColumn::make('text_display') + ->limit(80) + ->wrap(), + TextColumn::make('lang'), + TextColumn::make('intent') + ->placeholder('—'), + TextColumn::make('weight') + ->sortable(false), + TextColumn::make('source'), + TextColumn::make('synced_at') + ->dateTime() + ->placeholder('Never') + ->toggleable(isToggledHiddenByDefault: true), + TextColumn::make('updated_at') + ->dateTime(), + ]) + ->searchable() + ->recordActions([ + $this->editAction(), + $this->deleteAction(), + $this->syncEmbeddingAction(), + $this->deleteEmbeddingAction(), + ]) + ->toolbarActions([ + $this->deleteSelectedBulkAction(), + ]) + ->headerActions([ + $this->createAction(), + $this->createManyAction(), + $this->syncAction(), + $this->reloadIndexAction(), + ]); + } + + protected function createAction(): Action + { + return Action::make('create') + ->label('New Suggestion') + ->icon(Heroicon::OutlinedPlus) + ->schema($this->formSchema()) + ->action(function (array $data): void { + $this->callBnfexpress( + fn () => app(BnfexpressAdminClient::class)->createSuggestion( + $data['text_display'], + $data['lang'], + $data['intent'] ?: null, + (int) ($data['weight'] ?? 0), + $data['source'] ?: 'admin', + ), + successTitle: 'Suggestion created', + failureTitle: 'Failed to create suggestion', + ); + + $this->resetTable(); + }); + } + + protected function createManyAction(): Action + { + return Action::make('createMany') + ->label('Create Many') + ->icon(Heroicon::OutlinedQueueList) + ->schema([ + // No source field here — bnfexpress's batch endpoint always tags + // these rows source: "mined" server-side (it's a thin wrapper + // around the same promote() misses-promotion uses), regardless + // of what's sent, so exposing a picker would be misleading. + Text::make('Created rows are tagged source: mined by bnfexpress.') + ->color('gray'), + Textarea::make('items_raw') + ->label('Phrases (one per line)') + ->required() + ->rows(8), + TextInput::make('lang') + ->required() + ->maxLength(10), + TextInput::make('intent'), + ]) + ->action(function (array $data): void { + $items = collect(preg_split('/\r\n|\r|\n/', (string) $data['items_raw'])) + ->map(fn (string $line): string => trim($line)) + ->filter() + ->map(fn (string $text): array => array_filter([ + 'text' => $text, + 'lang' => $data['lang'], + 'intent' => $data['intent'] ?: null, + ], fn (mixed $value): bool => $value !== null)) + ->values() + ->all(); + + $this->callBnfexpressForResult( + fn () => app(BnfexpressAdminClient::class)->batchCreateSuggestions($items), + function (array $result): void { + Notification::make() + ->title("{$result['created']} created, {$result['skipped']} skipped") + ->success() + ->send(); + + $this->resetTable(); + }, + failureTitle: 'Failed to create suggestions', + ); + }); + } + + protected function editAction(): Action + { + return Action::make('edit') + ->icon(Heroicon::OutlinedPencilSquare) + ->fillForm(fn (array $record): array => $record) + ->schema($this->formSchema()) + ->action(function (array $data, array $record): void { + $this->callBnfexpress( + fn () => app(BnfexpressAdminClient::class)->updateSuggestion( + $record['id'], + $data['text_display'], + $data['lang'], + $data['intent'] ?: null, + (int) ($data['weight'] ?? 0), + $data['source'] ?: null, + ), + successTitle: 'Suggestion updated', + failureTitle: 'Failed to update suggestion', + ); + + $this->resetTable(); + }); + } + + protected function deleteAction(): Action + { + return Action::make('delete') + ->color('danger') + ->icon(Heroicon::OutlinedTrash) + ->requiresConfirmation() + ->action(function (array $record): void { + $this->callBnfexpress( + fn () => app(BnfexpressAdminClient::class)->deleteSuggestion($record['id']), + successTitle: 'Suggestion deleted', + failureTitle: 'Failed to delete suggestion', + ); + + $this->resetTable(); + }); + } + + protected function deleteSelectedBulkAction(): BulkAction + { + return BulkAction::make('deleteSelected') + ->label('Delete Selected') + ->color('danger') + ->icon(Heroicon::OutlinedTrash) + ->requiresConfirmation() + ->fetchSelectedRecords(false) + ->deselectRecordsAfterCompletion() + ->action(function (Collection $records): void { + // fetchSelectedRecords(false) still resolves full row arrays for a + // custom-data table (there's no cheap ID-only path like an Eloquent + // query) — the record keys (our suggestion ids) are what's wanted here. + $this->callBnfexpressForResult( + fn () => app(BnfexpressAdminClient::class)->batchDeleteSuggestions($records->keys()->all()), + function (array $result): void { + Notification::make() + ->title("{$result['deleted']} deleted, {$result['skipped']} skipped") + ->success() + ->send(); + + $this->resetTable(); + }, + failureTitle: 'Failed to delete suggestions', + ); + }); + } + + protected function syncEmbeddingAction(): Action + { + return Action::make('syncEmbedding') + ->label('Sync Embedding') + ->icon(Heroicon::OutlinedArrowPath) + ->action(function (array $record): void { + $this->callBnfexpress( + fn () => app(BnfexpressAdminClient::class)->syncOneSuggestion($record['id']), + successTitle: 'Embedding synced', + failureTitle: 'Failed to sync embedding', + ); + + $this->resetTable(); + }); + } + + protected function deleteEmbeddingAction(): Action + { + return Action::make('deleteEmbedding') + ->label('Delete Embedding') + ->color('danger') + ->icon(Heroicon::OutlinedXCircle) + ->requiresConfirmation() + ->action(function (array $record): void { + $this->callBnfexpress( + fn () => app(BnfexpressAdminClient::class)->deleteSuggestionEmbedding($record['id']), + successTitle: 'Embedding deleted', + failureTitle: 'Failed to delete embedding', + ); + + $this->resetTable(); + }); + } + + /** + * Kicks off a full re-embed job and starts polling for it (pollSyncStatus(), + * driven by wire:poll in the view) rather than notifying immediately — + * the real outcome only lands once the job finishes. + */ + protected function syncAction(): Action + { + return Action::make('sync') + ->label('Sync to Chroma') + ->icon(Heroicon::OutlinedArrowPath) + ->action(function (): void { + try { + $result = app(BnfexpressAdminClient::class)->syncSuggestions(); + $this->syncJobId = $result['job_id'] ?? null; + $this->syncStatus = 'queued'; + $this->syncResult = null; + } catch (BnfexpressApiException $exception) { + Notification::make() + ->title('Failed to start sync') + ->body($exception->getMessage()) + ->danger() + ->send(); + } + }); + } + + protected function reloadIndexAction(): Action + { + return Action::make('reloadIndex') + ->label('Reload Index') + ->icon(Heroicon::OutlinedArrowPath) + ->requiresConfirmation() + ->action(function (): void { + $this->callBnfexpress( + fn () => app(BnfexpressAdminClient::class)->reloadSuggestionIndex(), + successTitle: 'Index reloaded', + failureTitle: 'Failed to reload index', + ); + }); + } + + /** + * Polls a sync job's status (wire:poll.2s, see the view). On "finished", + * chains a reload-index call — same two-step flow shweai_backend's admin + * JS does (POST sync-chroma → poll → POST reload-index) — and refreshes + * the table. On "failed", notifies and stops polling. + */ + public function pollSyncStatus(): void + { + if ($this->syncJobId === null) { + return; + } + + try { + $status = app(BnfexpressAdminClient::class)->getSuggestionSyncStatus($this->syncJobId); + } catch (BnfexpressApiException $exception) { + $this->syncJobId = null; + Notification::make() + ->title('Failed to check sync status') + ->body($exception->getMessage()) + ->danger() + ->send(); + + return; + } + + $this->syncStatus = $status['status'] ?? null; + + if ($this->syncStatus === 'finished') { + $this->syncResult = $status['result'] ?? null; + $this->syncJobId = null; + + $this->callBnfexpress( + fn () => app(BnfexpressAdminClient::class)->reloadSuggestionIndex(), + successTitle: 'Sync completed and index reloaded', + failureTitle: 'Sync finished but reloading the index failed', + ); + + $this->resetTable(); + } elseif ($this->syncStatus === 'failed') { + $this->syncJobId = null; + + Notification::make() + ->title('Sync failed') + ->body(is_string($status['result'] ?? null) ? $status['result'] : 'Unknown error.') + ->danger() + ->send(); + } + // else: still queued/running — the view keeps polling. + } + + /** + * @return array + */ + protected function formSchema(): array + { + return [ + Textarea::make('text_display') + ->required() + ->rows(3), + TextInput::make('lang') + ->required() + ->maxLength(10), + TextInput::make('intent'), + TextInput::make('weight') + ->numeric() + ->default(0), + // Fixed choices rather than free text (matching shweai_backend's + // seed/mined dropdown) — 'admin' is bnfexpress's own default for + // a CRUD-created row (SuggestionAdminCreate.source), 'mined' is + // what promote() tags a row with. 'seed' isn't used by any + // bnfexpress code path today (unlike the other two, which are + // hardcoded/defaulted server-side) — source is just a free string + // there, so this is offered for admins bootstrapping initial + // phrases who want that distinct from an ad-hoc manual entry, + // same convention as shweai_backend's own seed/mined dropdown. + Select::make('source') + ->options([ + 'admin' => 'Admin (manual entry)', + 'seed' => 'Seed (initial/bootstrap data)', + 'mined' => 'Mined (promoted from a miss)', + ]) + ->default('admin') + ->native(false) + ->required(), + ]; + } +} diff --git a/app-modules/ai-agent/tests/Feature/ManageSuggestionMissesTest.php b/app-modules/ai-agent/tests/Feature/ManageSuggestionMissesTest.php new file mode 100644 index 0000000..5643181 --- /dev/null +++ b/app-modules/ai-agent/tests/Feature/ManageSuggestionMissesTest.php @@ -0,0 +1,89 @@ + 'https://bnfexpress.test', + 'services.bnfexpress.client_id' => 'ev_admin', + 'services.bnfexpress.client_secret' => 'test-secret', + ]); + + Permission::findOrCreate('manage_ai_agent', 'web'); + + $this->admin = User::factory()->create()->givePermissionTo(['manage_ai_agent']); + $this->actingAs($this->admin); +}); + +test('a user without manage_ai_agent cannot access it', function () { + $this->actingAs(User::factory()->create()); + + expect(ManageSuggestionMisses::canAccess())->toBeFalse(); +}); + +test('it renders and lists misses from the client', function () { + Http::fake(['bnfexpress.test/*' => Http::response([ + ['id' => 1, 'text_norm' => 'ev charging cost', 'lang' => 'en', 'syllables' => 3, 'was_used' => false, 'created_at' => now()->toIso8601String()], + ])]); + + Livewire::test(ManageSuggestionMisses::class) + ->assertOk() + ->loadTable() + ->assertSee('ev charging cost'); + + Http::assertSent(fn (Request $request) => str_contains($request->url(), '/admin/suggestion-misses') && $request->method() === 'GET'); +}); + +test('dismiss calls dismissSuggestionMiss and shows a success notification', function () { + Http::fake(['bnfexpress.test/*' => fn (Request $request) => match ($request->method()) { + 'DELETE' => Http::response([]), + default => Http::response([ + ['id' => 1, 'text_norm' => 'to dismiss', 'lang' => 'en', 'syllables' => 2, 'was_used' => false, 'created_at' => now()->toIso8601String()], + ]), + }]); + + Livewire::test(ManageSuggestionMisses::class) + ->loadTable() + ->callTableAction('dismiss', 1) + ->assertNotified('Miss dismissed'); + + Http::assertSent(fn (Request $request) => $request->method() === 'DELETE' && str_contains($request->url(), '/admin/suggestion-misses/1')); +}); + +test('a failed dismiss surfaces the gateway detail message', function () { + Http::fake(['bnfexpress.test/*' => fn (Request $request) => match ($request->method()) { + 'DELETE' => Http::response(['detail' => 'Suggestion miss not found.'], 404), + default => Http::response([ + ['id' => 1, 'text_norm' => 'to dismiss', 'lang' => 'en', 'syllables' => 2, 'was_used' => false, 'created_at' => now()->toIso8601String()], + ]), + }]); + + Livewire::test(ManageSuggestionMisses::class) + ->loadTable() + ->callTableAction('dismiss', 1) + ->assertNotified('Failed to dismiss miss'); +}); + +test('promote bulk action calls promoteSuggestionMisses with the selected ids and shows the result', function () { + Http::fake(['bnfexpress.test/*' => fn (Request $request) => match (true) { + str_contains($request->url(), '/admin/suggestion-misses/promote') => Http::response(['created' => 2, 'skipped' => 0, 'trie_rebuilt' => true]), + default => Http::response([ + ['id' => 1, 'text_norm' => 'a', 'lang' => 'en', 'syllables' => 1, 'was_used' => false, 'created_at' => now()->toIso8601String()], + ['id' => 2, 'text_norm' => 'b', 'lang' => 'en', 'syllables' => 1, 'was_used' => false, 'created_at' => now()->toIso8601String()], + ]), + }]); + + Livewire::test(ManageSuggestionMisses::class) + ->loadTable() + ->callTableBulkAction('promote', [1, 2], data: ['lang' => 'my', 'intent' => null]) + ->assertNotified('2 promoted, 0 skipped'); + + Http::assertSent(fn (Request $request) => str_contains($request->url(), '/admin/suggestion-misses/promote') + && $request['miss_ids'] === [1, 2] + && $request['lang'] === 'my'); +}); diff --git a/app-modules/ai-agent/tests/Feature/ManageSuggestionsTest.php b/app-modules/ai-agent/tests/Feature/ManageSuggestionsTest.php new file mode 100644 index 0000000..e610034 --- /dev/null +++ b/app-modules/ai-agent/tests/Feature/ManageSuggestionsTest.php @@ -0,0 +1,187 @@ + 'https://bnfexpress.test', + 'services.bnfexpress.client_id' => 'ev_admin', + 'services.bnfexpress.client_secret' => 'test-secret', + ]); + + Permission::findOrCreate('manage_ai_agent', 'web'); + + $this->admin = User::factory()->create()->givePermissionTo(['manage_ai_agent']); + $this->actingAs($this->admin); +}); + +test('a user without manage_ai_agent cannot access it', function () { + $this->actingAs(User::factory()->create()); + + expect(ManageSuggestions::canAccess())->toBeFalse(); +}); + +test('it renders and lists suggestions from the client', function () { + Http::fake(['bnfexpress.test/*' => Http::response([ + ['id' => 1, 'text_display' => 'How do I charge my EV?', 'lang' => 'en', 'intent' => null, 'weight' => 0, 'source' => 'manual', 'updated_at' => now()->toIso8601String()], + ])]); + + Livewire::test(ManageSuggestions::class) + ->assertOk() + ->loadTable() + ->assertSee('How do I charge my EV?'); + + Http::assertSent(fn (Request $request) => str_contains($request->url(), '/admin/suggestions') && $request->method() === 'GET'); +}); + +test('creating a suggestion calls createSuggestion and shows a success notification', function () { + Http::fake(['bnfexpress.test/*' => fn (Request $request) => match ($request->method()) { + 'POST' => Http::response(['id' => 1], 201), + default => Http::response([]), + }]); + + Livewire::test(ManageSuggestions::class) + ->loadTable() + ->callTableAction('create', data: ['text_display' => 'New phrase', 'lang' => 'en', 'intent' => null, 'weight' => 0, 'source' => 'admin']) + ->assertNotified('Suggestion created'); + + Http::assertSent(fn (Request $request) => $request->method() === 'POST' + && $request->url() === 'https://bnfexpress.test/admin/suggestions' + && $request['text_display'] === 'New phrase'); +}); + +test('a failed create surfaces the gateway detail message', function () { + Http::fake(['bnfexpress.test/*' => fn (Request $request) => match ($request->method()) { + 'POST' => Http::response(['detail' => 'text_display is required.'], 422), + default => Http::response([]), + }]); + + Livewire::test(ManageSuggestions::class) + ->loadTable() + ->callTableAction('create', data: ['text_display' => 'New phrase', 'lang' => 'en', 'intent' => null, 'weight' => 0, 'source' => 'admin']) + ->assertNotified('Failed to create suggestion'); +}); + +test('createMany splits pasted lines into batch items and shows the created/skipped result', function () { + Http::fake(['bnfexpress.test/*' => fn (Request $request) => match (true) { + str_contains($request->url(), '/admin/suggestions/batch') => Http::response(['created' => 2, 'skipped' => 0, 'trie_rebuilt' => true]), + default => Http::response([]), + }]); + + Livewire::test(ManageSuggestions::class) + ->loadTable() + ->callTableAction('createMany', data: ['items_raw' => "First phrase\nSecond phrase\n\n", 'lang' => 'en', 'intent' => null]) + ->assertNotified('2 created, 0 skipped'); + + Http::assertSent(fn (Request $request) => str_contains($request->url(), '/admin/suggestions/batch') + && $request['items'] === [ + ['text' => 'First phrase', 'lang' => 'en'], + ['text' => 'Second phrase', 'lang' => 'en'], + ]); +}); + +test('deleting a suggestion calls deleteSuggestion and shows a success notification', function () { + Http::fake(['bnfexpress.test/*' => fn (Request $request) => match ($request->method()) { + 'DELETE' => Http::response([]), + default => Http::response([ + ['id' => 1, 'text_display' => 'To delete', 'lang' => 'en', 'intent' => null, 'weight' => 0, 'source' => 'manual', 'updated_at' => now()->toIso8601String()], + ]), + }]); + + Livewire::test(ManageSuggestions::class) + ->loadTable() + ->callTableAction('delete', 1) + ->assertNotified('Suggestion deleted'); + + Http::assertSent(fn (Request $request) => $request->method() === 'DELETE' && str_contains($request->url(), '/admin/suggestions/1')); +}); + +test('deleteSelected bulk action calls batchDeleteSuggestions with just the selected ids', function () { + Http::fake(['bnfexpress.test/*' => fn (Request $request) => match (true) { + $request->method() === 'DELETE' && str_contains($request->url(), '/admin/suggestions/batch') => Http::response(['deleted' => 2, 'skipped' => 0]), + default => Http::response([ + ['id' => 1, 'text_display' => 'A', 'lang' => 'en', 'intent' => null, 'weight' => 0, 'source' => 'manual', 'updated_at' => now()->toIso8601String()], + ['id' => 2, 'text_display' => 'B', 'lang' => 'en', 'intent' => null, 'weight' => 0, 'source' => 'manual', 'updated_at' => now()->toIso8601String()], + ]), + }]); + + Livewire::test(ManageSuggestions::class) + ->loadTable() + ->callTableBulkAction('deleteSelected', [1, 2]) + ->assertNotified('2 deleted, 0 skipped'); + + Http::assertSent(fn (Request $request) => $request->method() === 'DELETE' + && str_contains($request->url(), '/admin/suggestions/batch') + && $request['ids'] === [1, 2]); +}); + +test('sync sets job state and polling chains a reload-index call on finished', function () { + Http::fake(['bnfexpress.test/*' => fn (Request $request) => match (true) { + str_contains($request->url(), '/sync-chroma/job-1') => Http::response(['status' => 'finished', 'result' => ['synced' => 3]]), + str_contains($request->url(), '/sync-chroma') => Http::response(['job_id' => 'job-1'], 202), + str_contains($request->url(), '/reload-index') => Http::response(['trie_rebuilt' => true]), + default => Http::response([]), + }]); + + $test = Livewire::test(ManageSuggestions::class) + ->loadTable() + ->callTableAction('sync') + ->assertSet('syncJobId', 'job-1') + ->assertSet('syncStatus', 'queued'); + + $test->call('pollSyncStatus') + ->assertSet('syncJobId', null) + ->assertSet('syncStatus', 'finished') + ->assertNotified('Sync completed and index reloaded'); + + Http::assertSent(fn (Request $request) => str_contains($request->url(), '/admin/suggestions/reload-index')); +}); + +test('a failed sync job notifies danger and stops polling', function () { + Http::fake(['bnfexpress.test/*' => fn (Request $request) => match (true) { + str_contains($request->url(), '/sync-chroma/job-1') => Http::response(['status' => 'failed', 'result' => null]), + str_contains($request->url(), '/sync-chroma') => Http::response(['job_id' => 'job-1'], 202), + default => Http::response([]), + }]); + + Livewire::test(ManageSuggestions::class) + ->loadTable() + ->callTableAction('sync') + ->call('pollSyncStatus') + ->assertSet('syncJobId', null) + ->assertNotified('Sync failed'); +}); + +test('reloadIndex calls reloadSuggestionIndex directly', function () { + Http::fake(['bnfexpress.test/*' => Http::response(['trie_rebuilt' => true])]); + + Livewire::test(ManageSuggestions::class) + ->loadTable() + ->callTableAction('reloadIndex') + ->assertNotified('Index reloaded'); + + Http::assertSent(fn (Request $request) => str_contains($request->url(), '/admin/suggestions/reload-index')); +}); + +test('syncEmbedding and deleteEmbedding row actions call the right per-id endpoint', function () { + Http::fake(['bnfexpress.test/*' => fn (Request $request) => match (true) { + str_contains($request->url(), '/1/sync-chroma') => Http::response(['synced' => true]), + str_contains($request->url(), '/1/chroma') => Http::response([]), + default => Http::response([ + ['id' => 1, 'text_display' => 'A', 'lang' => 'en', 'intent' => null, 'weight' => 0, 'source' => 'manual', 'updated_at' => now()->toIso8601String()], + ]), + }]); + + $test = Livewire::test(ManageSuggestions::class)->loadTable(); + + $test->callTableAction('syncEmbedding', 1)->assertNotified('Embedding synced'); + $test->callTableAction('deleteEmbedding', 1)->assertNotified('Embedding deleted'); + + Http::assertSent(fn (Request $request) => str_contains($request->url(), '/admin/suggestions/1/sync-chroma') && $request->method() === 'POST'); + Http::assertSent(fn (Request $request) => str_contains($request->url(), '/admin/suggestions/1/chroma') && $request->method() === 'DELETE'); +}); diff --git a/app-modules/shared/src/Bnfexpress/BnfexpressAdminClient.php b/app-modules/shared/src/Bnfexpress/BnfexpressAdminClient.php index 0f7e24b..f7b3873 100644 --- a/app-modules/shared/src/Bnfexpress/BnfexpressAdminClient.php +++ b/app-modules/shared/src/Bnfexpress/BnfexpressAdminClient.php @@ -3,6 +3,7 @@ namespace Modules\Shared\Bnfexpress; use Illuminate\Http\Client\ConnectionException; +use Illuminate\Http\Client\Response; use Illuminate\Support\Facades\Http; use Modules\Shared\Bnfexpress\Exceptions\BnfexpressApiException; use Modules\Shared\Bnfexpress\Support\BnfexpressSignature; @@ -163,6 +164,159 @@ class BnfexpressAdminClient return $this->request('GET', "/admin/ev/history/{$userId}/{$sessionId}"); } + // --- Suggestions ------------------------------------------------------ + + /** + * @return array + */ + public function listSuggestions(?string $q = null, ?int $limit = null, ?int $offset = null): array + { + return $this->request('GET', '/admin/suggestions', query: array_filter([ + 'q' => $q, + 'limit' => $limit, + 'offset' => $offset, + ], fn (mixed $value): bool => $value !== null)); + } + + /** + * @return array + */ + public function getSuggestion(int|string $id): array + { + return $this->request('GET', "/admin/suggestions/{$id}"); + } + + /** + * @return array + */ + public function createSuggestion(string $textDisplay, string $lang, ?string $intent = null, int $weight = 0, string $source = 'admin'): array + { + return $this->request('POST', '/admin/suggestions', body: [ + 'text_display' => $textDisplay, + 'lang' => $lang, + 'intent' => $intent, + 'weight' => $weight, + 'source' => $source, + ]); + } + + /** + * @return array + */ + public function updateSuggestion(int|string $id, ?string $textDisplay = null, ?string $lang = null, ?string $intent = null, ?int $weight = null, ?string $source = null): array + { + return $this->request('PATCH', "/admin/suggestions/{$id}", body: array_filter([ + 'text_display' => $textDisplay, + 'lang' => $lang, + 'intent' => $intent, + 'weight' => $weight, + 'source' => $source, + ], fn (mixed $value): bool => $value !== null)); + } + + /** + * @return array + */ + public function deleteSuggestion(int|string $id): array + { + return $this->request('DELETE', "/admin/suggestions/{$id}"); + } + + /** + * @param list $items + * @return array{created: int, skipped: int, trie_rebuilt: bool} + */ + public function batchCreateSuggestions(array $items): array + { + return $this->request('POST', '/admin/suggestions/batch', body: ['items' => $items]); + } + + /** + * @param list $ids + * @return array{deleted: int, skipped: int} + */ + public function batchDeleteSuggestions(array $ids): array + { + return $this->request('DELETE', '/admin/suggestions/batch', body: ['ids' => $ids]); + } + + // --- Suggestion misses -------------------------------------------------- + + /** + * @return array> + */ + public function listSuggestionMisses(?bool $wasUsed = null, ?int $limit = null, ?int $offset = null): array + { + return $this->request('GET', '/admin/suggestion-misses', query: array_filter([ + 'was_used' => $wasUsed, + 'limit' => $limit, + 'offset' => $offset, + ], fn (mixed $value): bool => $value !== null)); + } + + /** + * @return array + */ + public function dismissSuggestionMiss(int|string $id): array + { + return $this->request('DELETE', "/admin/suggestion-misses/{$id}"); + } + + /** + * @param list $missIds + * @return array{created: int, skipped: int, trie_rebuilt: bool} + */ + public function promoteSuggestionMisses(array $missIds, ?string $lang = null, ?string $intent = null): array + { + return $this->request('POST', '/admin/suggestion-misses/promote', body: array_filter([ + 'miss_ids' => $missIds, + 'lang' => $lang, + 'intent' => $intent, + ], fn (mixed $value): bool => $value !== null)); + } + + // --- Suggestion sync/embeddings ------------------------------------------ + + /** + * @return array{job_id: string} + */ + public function syncSuggestions(): array + { + return $this->request('POST', '/admin/suggestions/sync-chroma'); + } + + /** + * @return array{status: string, result: mixed} + */ + public function getSuggestionSyncStatus(string $jobId): array + { + return $this->request('GET', "/admin/suggestions/sync-chroma/{$jobId}"); + } + + /** + * @return array + */ + public function syncOneSuggestion(int|string $id): array + { + return $this->request('POST', "/admin/suggestions/{$id}/sync-chroma"); + } + + /** + * @return array + */ + public function reloadSuggestionIndex(): array + { + return $this->request('POST', '/admin/suggestions/reload-index'); + } + + /** + * @return array + */ + public function deleteSuggestionEmbedding(int|string $id): array + { + return $this->request('DELETE', "/admin/suggestions/{$id}/chroma"); + } + // --- Request plumbing ------------------------------------------------ /** @@ -183,7 +337,12 @@ class BnfexpressAdminClient try { $response = match ($method) { 'GET' => $pending->get($path, $query), - 'DELETE' => $pending->delete($path, $query), + // DELETE with a body (e.g. batchDeleteSuggestions) must send it the same + // way POST/PATCH do — $query is never used as delete()'s $data here, that + // param means something else (a JSON body) than what its name implies. + 'DELETE' => $body !== null + ? $pending->withBody($rawBody, 'application/json')->delete($path) + : $pending->delete($path), 'POST' => $pending->withBody($rawBody, 'application/json')->post($path), 'PATCH' => $pending->withBody($rawBody, 'application/json')->patch($path), default => throw new \InvalidArgumentException("Unsupported HTTP method [{$method}]."), @@ -193,12 +352,34 @@ class BnfexpressAdminClient } if (! $response->successful()) { - throw new BnfexpressApiException( - (string) ($response->json('detail') ?? "bnfexpress request failed with status {$response->status()}."), - $response->status(), - ); + throw new BnfexpressApiException($this->errorMessage($response), $response->status()); } return (array) $response->json(); } + + /** + * bnfexpress's {"detail": "..."} is usually a plain string, but FastAPI's + * own request-validation failures (422s) return `detail` as a list of + * {loc, msg, type} objects instead — casting that straight to string + * produces the literal, useless "Array" (with a PHP warning). Handle + * both shapes. + */ + private function errorMessage(Response $response): string + { + $detail = $response->json('detail'); + + if (is_string($detail)) { + return $detail; + } + + if (is_array($detail)) { + return implode(' ', array_map( + fn (mixed $item): string => is_array($item) ? (string) ($item['msg'] ?? json_encode($item)) : (string) $item, + $detail, + )); + } + + return "bnfexpress request failed with status {$response->status()}."; + } } diff --git a/app-modules/shared/tests/Unit/BnfexpressAdminClientTest.php b/app-modules/shared/tests/Unit/BnfexpressAdminClientTest.php index 0318f0a..9ac1ec3 100644 --- a/app-modules/shared/tests/Unit/BnfexpressAdminClientTest.php +++ b/app-modules/shared/tests/Unit/BnfexpressAdminClientTest.php @@ -74,6 +74,16 @@ test('a non-2xx response surfaces the gateway detail message', function () use ( (new BnfexpressAdminClient($config))->getFaq(999); })->throws(BnfexpressApiException::class, 'FAQ not found.'); +test('a FastAPI validation error (detail as a list of objects) is flattened into a readable message', function () use ($config) { + Http::fake(['bnfexpress.test/*' => Http::response([ + 'detail' => [ + ['type' => 'int_parsing', 'loc' => ['path', 'suggestion_id'], 'msg' => 'Input should be a valid integer, unable to parse string as an integer', 'input' => 'batch'], + ], + ], 422)]); + + (new BnfexpressAdminClient($config))->getSuggestion('batch'); +})->throws(BnfexpressApiException::class, 'Input should be a valid integer, unable to parse string as an integer'); + test('deleteFaq signs a bodyless DELETE request', function () use ($config) { Http::fake(['bnfexpress.test/*' => Http::response(['deleted' => true])]); @@ -85,3 +95,114 @@ test('deleteFaq signs a bodyless DELETE request', function () use ($config) { && assertSignedCorrectly($request, 'DELETE', '/admin/faqs/5', '', $config['client_secret']); }); }); + +test('createSuggestion sends text_display/lang/intent/weight/source', function () use ($config) { + Http::fake(['bnfexpress.test/*' => Http::response(['id' => 1], 201)]); + + (new BnfexpressAdminClient($config))->createSuggestion('How do I charge?', 'en', 'charging', 5, 'manual'); + + $rawBody = json_encode([ + 'text_display' => 'How do I charge?', + 'lang' => 'en', + 'intent' => 'charging', + 'weight' => 5, + 'source' => 'manual', + ]); + + Http::assertSent(fn ($request) => $request->url() === 'https://bnfexpress.test/admin/suggestions' + && $request->method() === 'POST' + && $request->body() === $rawBody); +}); + +test('updateSuggestion only sends the given fields', function () use ($config) { + Http::fake(['bnfexpress.test/*' => Http::response(['id' => 1])]); + + (new BnfexpressAdminClient($config))->updateSuggestion(1, weight: 10); + + Http::assertSent(fn ($request) => $request->body() === json_encode(['weight' => 10])); +}); + +test('batchCreateSuggestions posts items and returns the created/skipped/trie_rebuilt result', function () use ($config) { + Http::fake(['bnfexpress.test/*' => Http::response(['created' => 2, 'skipped' => 1, 'trie_rebuilt' => true])]); + + $result = (new BnfexpressAdminClient($config))->batchCreateSuggestions([ + ['text' => 'a', 'lang' => 'en'], + ['text' => 'b', 'lang' => 'en', 'intent' => 'x'], + ]); + + expect($result)->toBe(['created' => 2, 'skipped' => 1, 'trie_rebuilt' => true]); + Http::assertSent(fn ($request) => $request->url() === 'https://bnfexpress.test/admin/suggestions/batch' + && $request->method() === 'POST' + && $request['items'] === [ + ['text' => 'a', 'lang' => 'en'], + ['text' => 'b', 'lang' => 'en', 'intent' => 'x'], + ]); +}); + +test('batchDeleteSuggestions sends a JSON body on DELETE and signs it', function () use ($config) { + Http::fake(['bnfexpress.test/*' => Http::response(['deleted' => 2, 'skipped' => 0])]); + + $result = (new BnfexpressAdminClient($config))->batchDeleteSuggestions([1, 2]); + + $rawBody = json_encode(['ids' => [1, 2]]); + + expect($result)->toBe(['deleted' => 2, 'skipped' => 0]); + Http::assertSent(function ($request) use ($config, $rawBody) { + return $request->url() === 'https://bnfexpress.test/admin/suggestions/batch' + && $request->method() === 'DELETE' + && $request->body() === $rawBody + && assertSignedCorrectly($request, 'DELETE', '/admin/suggestions/batch', $rawBody, $config['client_secret']); + }); +}); + +test('listSuggestionMisses filters by was_used', function () use ($config) { + Http::fake(['bnfexpress.test/*' => Http::response([])]); + + (new BnfexpressAdminClient($config))->listSuggestionMisses(wasUsed: false, limit: 25); + + Http::assertSent(fn ($request) => $request->url() === 'https://bnfexpress.test/admin/suggestion-misses?was_used=0&limit=25' + || $request->url() === 'https://bnfexpress.test/admin/suggestion-misses?was_used=&limit=25'); +}); + +test('promoteSuggestionMisses posts miss_ids with an optional lang/intent override', function () use ($config) { + Http::fake(['bnfexpress.test/*' => Http::response(['created' => 1, 'skipped' => 0, 'trie_rebuilt' => true])]); + + $result = (new BnfexpressAdminClient($config))->promoteSuggestionMisses([1, 2], lang: 'my'); + + expect($result)->toBe(['created' => 1, 'skipped' => 0, 'trie_rebuilt' => true]); + Http::assertSent(fn ($request) => $request->url() === 'https://bnfexpress.test/admin/suggestion-misses/promote' + && $request->method() === 'POST' + && $request->body() === json_encode(['miss_ids' => [1, 2], 'lang' => 'my'])); +}); + +test('syncSuggestions triggers a sync-chroma job and returns the job id', function () use ($config) { + Http::fake(['bnfexpress.test/*' => Http::response(['job_id' => 'abc-123'], 202)]); + + $result = (new BnfexpressAdminClient($config))->syncSuggestions(); + + expect($result)->toBe(['job_id' => 'abc-123']); + Http::assertSent(fn ($request) => $request->url() === 'https://bnfexpress.test/admin/suggestions/sync-chroma' + && $request->method() === 'POST'); +}); + +test('getSuggestionSyncStatus polls the job status endpoint', function () use ($config) { + Http::fake(['bnfexpress.test/*' => Http::response(['status' => 'finished', 'result' => ['synced' => 3]])]); + + $result = (new BnfexpressAdminClient($config))->getSuggestionSyncStatus('abc-123'); + + expect($result)->toBe(['status' => 'finished', 'result' => ['synced' => 3]]); + Http::assertSent(fn ($request) => $request->url() === 'https://bnfexpress.test/admin/suggestions/sync-chroma/abc-123' + && $request->method() === 'GET'); +}); + +test('deleteSuggestionEmbedding signs a bodyless DELETE request', function () use ($config) { + Http::fake(['bnfexpress.test/*' => Http::response([])]); + + (new BnfexpressAdminClient($config))->deleteSuggestionEmbedding(5); + + Http::assertSent(function ($request) use ($config) { + return $request->url() === 'https://bnfexpress.test/admin/suggestions/5/chroma' + && $request->method() === 'DELETE' + && assertSignedCorrectly($request, 'DELETE', '/admin/suggestions/5/chroma', '', $config['client_secret']); + }); +});