Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

toolkit: add file content viewer #825

Merged
merged 27 commits into from
Nov 12, 2024

Conversation

danylo-boiko
Copy link
Contributor

@danylo-boiko danylo-boiko commented Oct 30, 2024

Description

  • Added viewer for imported files

AI Description

This PR introduces a new feature for retrieving files by ID, with a focus on agent and conversation files. The changes include:

  • Adding a new function, get_file, in src/backend/crud/file.py, which retrieves a file by ID. The function now accepts an optional user_id parameter, allowing for more flexible file retrieval.
  • Creating new classes, ConversationFileFull and AgentFileFull, in src/backend/schemas/file.py, to represent conversation and agent files with additional attributes like file_content.
  • Modifying the get_files_by_agent_id function in src/backend/services/file.py to return a list of file IDs instead of files. A new function, get_file_ids_by_agent_id, is introduced to retrieve file IDs associated with a specific agent ID.
  • Adding new routes in src/backend/routers/agent.py and src/backend/routers/conversation.py to handle file retrieval by ID for agents and conversations, respectively. These routes use the get_file function from the file_crud module.
  • Enhancing the src/interfaces/assistants_web/src/cohere-client/client.ts file by adding new methods, getConversationFile and getAgentFile, to retrieve conversation and agent files by ID.
  • Introducing new types, AgentFileFull and ConversationFileFull, in src/interfaces/assistants_web/src/cohere-client/generated/types.gen.ts, to represent conversation and agent files with additional attributes.
  • Adding new functions, getAgentFileV1AgentsAgentIdFilesFileIdGet and getFileV1ConversationsConversationIdFilesFileIdGet, in src/interfaces/assistants_web/src/cohere-client/generated/services.gen.ts, to retrieve agent and conversation files by ID.
  • Creating a new component, FileViewer, in src/interfaces/assistants_web/src/components/UI/FileViewer.tsx, to display file content.
  • Enhancing the ConversationPanel component in src/interfaces/assistants_web/src/components/Conversation/ConversationPanel.tsx to handle file viewing and deletion.
  • Adding a new hook, useFile, in src/interfaces/assistants_web/src/hooks/use-files.ts, to retrieve agent and conversation files by ID.
  • Introducing new tests in src/backend/tests/unit/routers/test_conversation.py to ensure the proper functioning of file retrieval by ID for conversations.

@tianjing-li
Copy link
Collaborator

@danylo-boiko Always happy to see more contributions from you.Will review when bandwidth permits

Copy link
Collaborator

@tianjing-li tianjing-li left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Backend portion looks great to me. Let me get someone with some more frontend expertise to look at the rest

src/backend/services/file.py Show resolved Hide resolved
@tianjing-li
Copy link
Collaborator

All checks are good too, asked @malexw to take a look at the frontend part

@danylo-boiko
Copy link
Contributor Author

All checks are good too, asked @malexw to take a look at the frontend part

Nice, thanks!

@tianjing-li
Copy link
Collaborator

@danylo-boiko On an aside - and perhaps this chat can happen in a different channel, but I've been collecting internal/external feedback on the toolkit and I'm open to any suggestions you might have to build out a future roadmap, mainly look at features/dev experience improvements

@danylo-boiko
Copy link
Contributor Author

@danylo-boiko On an aside - and perhaps this chat can happen in a different channel, but I've been collecting internal/external feedback on the toolkit and I'm open to any suggestions you might have to build out a future roadmap, mainly look at features/dev experience improvements

@tianjing-li I'd be happy to help with building a future roadmap. What platform do you use for communication?

@tianjing-li
Copy link
Collaborator

@danylo-boiko Mostly Slack, what's your email?

@danylo-boiko
Copy link
Contributor Author

@danylo-boiko Mostly Slack, what's your email?

@tianjing-li danielboyko02@gmail.com

Copy link
Collaborator

@malexw malexw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very helpful change - thank you! It's certainly a feature that I've wished we had in the past.

Overall, I'd prefer that the file previews not be shown in a modal dialog. However, I think that's a much more involved change, and I'm happy to use the dialog for now until we have an opportunity to build a different design.

Assuming that the dialog-based preview is temporary, then I think that the changes here to pass the dialogPaddingClassName value through the ModalContext and Modal components make the API for those components less clean than I'd prefer to see. The smaller padding does make the preview look better, but I'm not sure it's better enough to justify those changes to the components.

@danylo-boiko danylo-boiko requested a review from malexw November 12, 2024 20:14
@malexw malexw merged commit 6bf5847 into cohere-ai:main Nov 12, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants