diff --git a/lib/Listener/FileActionTaskFailedListener.php b/lib/Listener/FileActionTaskFailedListener.php index 0e8bfa29..68303f01 100644 --- a/lib/Listener/FileActionTaskFailedListener.php +++ b/lib/Listener/FileActionTaskFailedListener.php @@ -14,6 +14,7 @@ use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; use OCP\Files\IRootFolder; +use OCP\Files\Node; use OCP\TaskProcessing\Events\TaskFailedEvent; use Psr\Log\LoggerInterface; @@ -52,6 +53,13 @@ public function handle(Event $event): void { $this->logger->debug('FileActionTaskListener', ['source_file_id' => $sourceFileId]); $userFolder = $this->rootFolder->getUserFolder($task->getUserId()); $sourceFile = $userFolder->getFirstNodeById($sourceFileId); + if (!$sourceFile instanceof Node) { + // the source file was removed while the task was running, there is nothing to point the user at + $this->logger->warning('FileActionTaskListener task failed and the source file is gone', [ + 'source_file_id' => $sourceFileId, + ]); + return; + } $this->notificationService->sendFileActionNotification( $task->getUserId(), $taskTypeId, $task->getId(), $sourceFileId, $sourceFile->getName(), $userFolder->getRelativePath($sourceFile->getPath()), diff --git a/lib/Listener/FileActionTaskSuccessfulListener.php b/lib/Listener/FileActionTaskSuccessfulListener.php index 3ee5c314..3482169b 100644 --- a/lib/Listener/FileActionTaskSuccessfulListener.php +++ b/lib/Listener/FileActionTaskSuccessfulListener.php @@ -15,6 +15,7 @@ use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; use OCP\Files\IRootFolder; +use OCP\Files\Node; use OCP\SystemTag\TagNotFoundException; use OCP\TaskProcessing\Events\TaskSuccessfulEvent; use OCP\TaskProcessing\TaskTypes\TextToTextSummary; @@ -57,6 +58,13 @@ public function handle(Event $event): void { $userFolder = $this->rootFolder->getUserFolder($task->getUserId()); $sourceFile = $userFolder->getFirstNodeById($sourceFileId); $this->logger->debug('FileActionTaskListener', ['source file id' => $sourceFileId]); + if (!$sourceFile instanceof Node) { + // the source file was removed while the task was running, there is nowhere to write the result + $this->logger->warning('FileActionTaskListener task succeeded but the source file is gone', [ + 'source file id' => $sourceFileId, + ]); + return; + } try { $sourceFileParent = $sourceFile->getParent(); $this->logger->debug('FileActionTaskListener', ['source file PARENT id' => $sourceFileParent->getId()]); diff --git a/lib/Listener/LoadAdditionalScriptsListener.php b/lib/Listener/LoadAdditionalScriptsListener.php index 5043d483..2bf74e17 100644 --- a/lib/Listener/LoadAdditionalScriptsListener.php +++ b/lib/Listener/LoadAdditionalScriptsListener.php @@ -8,6 +8,7 @@ namespace OCA\Assistant\Listener; use OCA\Assistant\AppInfo\Application; +use OCA\Assistant\Service\AttachmentService; use OCA\Files\Event\LoadAdditionalScriptsEvent; use OCP\AppFramework\Services\IInitialState; use OCP\EventDispatcher\Event; @@ -28,6 +29,7 @@ public function __construct( private IAppConfig $appConfig, private IInitialState $initialStateService, private ITaskProcessingManager $taskProcessingManager, + private AttachmentService $attachmentService, ) { } @@ -46,6 +48,12 @@ public function handle(Event $event): void { $this->initialStateService->provideInitialState('stt-available', $sttAvailable); $this->initialStateService->provideInitialState('tts-available', $ttsAvailable); $this->initialStateService->provideInitialState('summarize-available', $summarizeAvailable); + // extra mime types the summarize action can handle by sending the file to the provider, + // on top of the ones we can extract text from ourselves + $this->initialStateService->provideInitialState( + 'summarize-attachment-mime-types', + $summarizeAvailable ? $this->attachmentService->getAttachableMimeTypes(TextToTextSummary::ID) : [], + ); Util::addInitScript(Application::APP_ID, Application::APP_ID . '-fileActions'); // New file menu to generate images diff --git a/lib/Service/AttachmentService.php b/lib/Service/AttachmentService.php new file mode 100644 index 00000000..13afa132 --- /dev/null +++ b/lib/Service/AttachmentService.php @@ -0,0 +1,143 @@ +taskProcessingManager->getAvailableTaskTypes(); + $shape = $taskTypes[$taskTypeId]['optionalInputShape']['input_attachments'] ?? null; + return $shape instanceof ShapeDescriptor + && $shape->getShapeType() === EShapeType::ListOfFiles; + } catch (\Throwable $e) { + // this is also called while rendering templates, never let it bubble up + $this->logger->debug('Could not determine attachment support for ' . $taskTypeId, ['exception' => $e]); + return false; + } + } + + /** + * Whether $file is one we would rather hand to the model directly. + */ + public function isAttachableFile(File $file): bool { + if ($file->getSize() > $this->getMaxAttachmentSize()) { + return false; + } + $mimeType = $file->getMimeType(); + if (in_array($mimeType, self::ATTACHABLE_MIME_TYPES, true)) { + return true; + } + return in_array($mimeType, self::EXTRACTABLE_TEXT_MIME_TYPES, true) + && $file->getSize() > $this->getTextInlineThreshold(); + } + + /** + * Whether the file behind $fileId should be attached to a $taskTypeId task + * rather than parsed into text first. + */ + public function shouldAttachFile(File $file, string $taskTypeId): bool { + return $this->providerSupportsInputAttachments($taskTypeId) + && $this->isAttachableFile($file); + } + + /** + * The mime types the summarize file action can handle over and above the + * ones we can extract text from. + * + * These end up in a file action `enabled()` check that compares mime types + * exactly, so only concrete types belong here - no wildcards. + * + * @return list + */ + public function getAttachableMimeTypes(string $taskTypeId): array { + if (!$this->providerSupportsInputAttachments($taskTypeId)) { + return []; + } + return self::ATTACHABLE_MIME_TYPES; + } + + private function getMaxAttachmentSize(): int { + return $this->appConfig->getValueInt( + Application::APP_ID, + 'max_attachment_size', + self::DEFAULT_MAX_ATTACHMENT_SIZE, + lazy: true, + ); + } + + private function getTextInlineThreshold(): int { + return $this->appConfig->getValueInt( + Application::APP_ID, + 'text_inline_threshold', + self::DEFAULT_TEXT_INLINE_THRESHOLD, + lazy: true, + ); + } +} diff --git a/lib/Service/TaskProcessingService.php b/lib/Service/TaskProcessingService.php index 0db8b286..cf228a7a 100644 --- a/lib/Service/TaskProcessingService.php +++ b/lib/Service/TaskProcessingService.php @@ -33,6 +33,7 @@ public function __construct( private IRootFolder $rootFolder, private LoggerInterface $logger, private AssistantService $assistantService, + private AttachmentService $attachmentService, ) { } @@ -88,6 +89,15 @@ public function getOutputFileContent(int $fileId): string { return $file->getContent(); } + /** + * Task types that take the input file itself rather than its text content. + */ + private function isAudioInputTaskType(string $taskTypeId): bool { + return $taskTypeId === AudioToText::ID + || (class_exists('OCP\\TaskProcessing\\TaskTypes\\AudioToTextSubtitles') + && $taskTypeId === \OCP\TaskProcessing\TaskTypes\AudioToTextSubtitles::ID); + } + public function isFileActionTaskTypeSupported(string $taskTypeId): bool { $authorizedTaskTypes = [AudioToText::ID, TextToTextSummary::ID]; if (class_exists('OCP\\TaskProcessing\\TaskTypes\\TextToSpeech')) { @@ -114,9 +124,7 @@ public function runFileAction(string $userId, int $fileId, string $taskTypeId): throw new Exception('Invalid task type for file action'); } try { - $input = ($taskTypeId === AudioToText::ID) || (class_exists('OCP\\TaskProcessing\\TaskTypes\\AudioToTextSubtitles') && $taskTypeId === \OCP\TaskProcessing\TaskTypes\AudioToTextSubtitles::ID) - ? ['input' => $fileId] - : ['input' => $this->assistantService->parseTextFromFile($userId, fileId: $fileId)]; + $input = $this->buildFileActionInput($userId, $fileId, $taskTypeId); } catch (NotPermittedException|GenericFileException|LockedException|\OCP\Files\NotFoundException|Exception $e) { $this->logger->warning('Assistant runFileAction, impossible to read the file action input file', ['exception' => $e]); throw new Exception('Impossible to read the file action input file'); @@ -140,4 +148,38 @@ public function runFileAction(string $userId, int $fileId, string $taskTypeId): } return $taskId; } + + /** + * Build the task input for a file action. + * + * Audio task types consume the file directly. For the others we normally + * extract the text ourselves, but when the provider can take files and the + * file is one our parsers handle badly, we hand it over untouched instead. + * + * The mandatory text input stays empty in that case: the provider builds + * its own instructions around the attachment, and anything we put there + * would be untranslated and fight that prompt. + * + * @return array + * @throws GenericFileException + * @throws LockedException + * @throws NotPermittedException + * @throws \OCP\Files\NotFoundException + */ + private function buildFileActionInput(string $userId, int $fileId, string $taskTypeId): array { + if ($this->isAudioInputTaskType($taskTypeId)) { + return ['input' => $fileId]; + } + + $file = $this->rootFolder->getUserFolder($userId)->getFirstNodeById($fileId); + if ($file instanceof File && $this->attachmentService->shouldAttachFile($file, $taskTypeId)) { + $this->logger->debug('Assistant file action: sending the file to the provider as an attachment', [ + 'fileId' => $fileId, + 'taskTypeId' => $taskTypeId, + ]); + return ['input' => '', 'input_attachments' => [$fileId]]; + } + + return ['input' => $this->assistantService->parseTextFromFile($userId, fileId: $fileId)]; + } } diff --git a/src/files/fileActions.js b/src/files/fileActions.js index f58b87d1..0f4369dc 100644 --- a/src/files/fileActions.js +++ b/src/files/fileActions.js @@ -39,7 +39,7 @@ function registerGroupAction(mimeTypes) { registerFileAction(groupAction) } -function registerSummarizeAction() { +function registerSummarizeAction(mimeTypes) { const summarizeAction = { id: 'assistant-summarize', parent: 'assistant-group', @@ -51,7 +51,7 @@ function registerSummarizeAction() { && nodes.length === 1 && !nodes.some(({ permissions }) => (permissions & Permission.READ) === 0) && nodes.every(({ type }) => type === FileType.File) - && nodes.every(({ mime }) => VALID_TEXT_MIME_TYPES.includes(mime)) + && nodes.every(({ mime }) => mimeTypes.includes(mime)) }, iconSvgInline: () => SummarizeSymbol, order: 0, @@ -199,24 +199,30 @@ const assistantEnabled = loadState('assistant', 'assistant-enabled', false) const summarizeAvailable = loadState('assistant', 'summarize-available', false) const sttAvailable = loadState('assistant', 'stt-available', false) const ttsAvailable = loadState('assistant', 'tts-available', false) +// files the AI provider can read itself, so summarizing them does not depend on our text extraction +const summarizeAttachmentMimeTypes = loadState('assistant', 'summarize-attachment-mime-types', []) +const summarizeMimeTypes = [...new Set([...VALID_TEXT_MIME_TYPES, ...summarizeAttachmentMimeTypes])] if (assistantEnabled) { if (summarizeAvailable || sttAvailable || ttsAvailable) { - const groupMimeTypes = [] - if (summarizeAvailable || ttsAvailable) { - groupMimeTypes.push(...VALID_TEXT_MIME_TYPES) + const groupMimeTypes = new Set() + if (summarizeAvailable) { + summarizeMimeTypes.forEach((mime) => groupMimeTypes.add(mime)) + } + if (ttsAvailable) { + VALID_TEXT_MIME_TYPES.forEach((mime) => groupMimeTypes.add(mime)) } if (sttAvailable) { - groupMimeTypes.push(...VALID_AUDIO_MIME_TYPES) + VALID_AUDIO_MIME_TYPES.forEach((mime) => groupMimeTypes.add(mime)) } - registerGroupAction(groupMimeTypes) + registerGroupAction([...groupMimeTypes]) } if (sttAvailable) { registerSttAction() registerSttSubtitlesAction() } if (summarizeAvailable) { - registerSummarizeAction() + registerSummarizeAction(summarizeMimeTypes) } if (ttsAvailable) { registerTtsAction() diff --git a/tests/unit/Service/AttachmentServiceTest.php b/tests/unit/Service/AttachmentServiceTest.php new file mode 100644 index 00000000..274b9795 --- /dev/null +++ b/tests/unit/Service/AttachmentServiceTest.php @@ -0,0 +1,151 @@ +taskProcessingManager = $this->createMock(IManager::class); + + $appConfig = $this->createMock(IAppConfig::class); + // no admin overrides, always hand back the default + $appConfig->method('getValueInt')->willReturnCallback( + static fn (string $app, string $key, int $default = 0, bool $lazy = false): int => $default, + ); + + return new AttachmentService( + $this->taskProcessingManager, + $appConfig, + $this->createMock(LoggerInterface::class), + ); + } + + private function mockFile(string $mimeType, int $size): File&MockObject { + $file = $this->createMock(File::class); + $file->method('getMimeType')->willReturn($mimeType); + $file->method('getSize')->willReturn($size); + return $file; + } + + /** + * @param ?ShapeDescriptor $shape the input_attachments descriptor the provider advertises, if any + */ + private function mockAvailableTaskTypes(?ShapeDescriptor $shape): void { + $this->taskProcessingManager->method('getAvailableTaskTypes')->willReturn([ + self::TASK_TYPE_ID => [ + 'optionalInputShape' => $shape === null ? [] : ['input_attachments' => $shape], + ], + ]); + } + + public function testProviderWithoutAttachmentSlotIsNotSupported(): void { + $service = $this->buildService(); + $this->mockAvailableTaskTypes(null); + self::assertFalse($service->providerSupportsInputAttachments(self::TASK_TYPE_ID)); + } + + public function testProviderAdvertisingListOfFilesIsSupported(): void { + $service = $this->buildService(); + $this->mockAvailableTaskTypes(new ShapeDescriptor('Attachments', 'Files', EShapeType::ListOfFiles)); + self::assertTrue($service->providerSupportsInputAttachments(self::TASK_TYPE_ID)); + } + + public function testSlotWithTheWrongShapeTypeIsNotSupported(): void { + $service = $this->buildService(); + // a provider using the same key for something else must not switch the feature on + $this->mockAvailableTaskTypes(new ShapeDescriptor('Attachments', 'Files', EShapeType::Text)); + self::assertFalse($service->providerSupportsInputAttachments(self::TASK_TYPE_ID)); + } + + public function testUnknownTaskTypeIsNotSupported(): void { + $service = $this->buildService(); + $this->mockAvailableTaskTypes(new ShapeDescriptor('Attachments', 'Files', EShapeType::ListOfFiles)); + self::assertFalse($service->providerSupportsInputAttachments('core:text2text:chat')); + } + + public function testManagerFailureDoesNotBubbleUp(): void { + $service = $this->buildService(); + // this runs while rendering templates, a throw here would break the page + $this->taskProcessingManager->method('getAvailableTaskTypes') + ->willThrowException(new \RuntimeException('boom')); + self::assertFalse($service->providerSupportsInputAttachments(self::TASK_TYPE_ID)); + } + + /** + * @dataProvider attachableFileDataProvider + */ + public function testIsAttachableFile(string $mimeType, int $size, bool $expected): void { + $service = $this->buildService(); + self::assertSame($expected, $service->isAttachableFile($this->mockFile($mimeType, $size))); + } + + public function attachableFileDataProvider(): array { + return [ + // PDFs always go to the provider, our parser is the unreliable part + 'small pdf' => ['application/pdf', 12_000, true], + 'large pdf' => ['application/pdf', 20_000_000, true], + 'oversized pdf' => ['application/pdf', 80_000_000, false], + // text extracts fine and cheaply until it no longer fits in a Text input + 'small markdown' => ['text/markdown', 10_000, false], + 'large markdown' => ['text/markdown', 1_000_000, true], + 'small plain text' => ['text/plain', 1_000, false], + 'large plain text' => ['text/plain', 500_000, true], + 'large csv' => ['text/csv', 400_000, true], + // office formats are rejected provider side, they must keep using text extraction + 'docx' => ['application/vnd.openxmlformats-officedocument.wordprocessingml.document', 5_000_000, false], + 'odt' => ['application/vnd.oasis.opendocument.text', 900_000, false], + 'rtf' => ['text/rtf', 900_000, false], + 'image' => ['image/jpeg', 900_000, false], + ]; + } + + public function testShouldAttachFileRequiresBothProviderSupportAndAnEligibleFile(): void { + $service = $this->buildService(); + $this->mockAvailableTaskTypes(new ShapeDescriptor('Attachments', 'Files', EShapeType::ListOfFiles)); + + self::assertTrue($service->shouldAttachFile($this->mockFile('application/pdf', 12_000), self::TASK_TYPE_ID)); + self::assertFalse($service->shouldAttachFile($this->mockFile('text/markdown', 10_000), self::TASK_TYPE_ID)); + } + + public function testShouldNotAttachFileWhenTheProviderCannotTakeAttachments(): void { + $service = $this->buildService(); + $this->mockAvailableTaskTypes(null); + self::assertFalse($service->shouldAttachFile($this->mockFile('application/pdf', 12_000), self::TASK_TYPE_ID)); + } + + public function testAttachableMimeTypesAreOnlyAdvertisedWhenTheProviderSupportsThem(): void { + $service = $this->buildService(); + $this->mockAvailableTaskTypes(new ShapeDescriptor('Attachments', 'Files', EShapeType::ListOfFiles)); + $mimeTypes = $service->getAttachableMimeTypes(self::TASK_TYPE_ID); + + self::assertContains('application/pdf', $mimeTypes); + // the file action compares mime types exactly, wildcards would never match + foreach ($mimeTypes as $mimeType) { + self::assertStringNotContainsString('*', $mimeType); + } + } + + public function testNoAttachableMimeTypesWithoutProviderSupport(): void { + $service = $this->buildService(); + $this->mockAvailableTaskTypes(null); + self::assertSame([], $service->getAttachableMimeTypes(self::TASK_TYPE_ID)); + } +}