Skip to content

Add support for images and pdf files #306

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.

Sign up for GitHub

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

Merged
brichet merged 14 commits into from
Apr 15, 2026
+327 −171
Merged

Add support for images and pdf files #306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
34480f8
Add support for images and pdf files
Yahiewi Apr 2, 2026
6acf401
Merge remote-tracking branch 'upstream/main' into image
Yahiewi Apr 7, 2026
b531350
Use IAttachment mimetype for reading attachments
Yahiewi Apr 7, 2026
030280b
Use IAttachment mimetype for reading attachments
Yahiewi Apr 7, 2026
ad6aa39
Refactor AIChatModel methods to Private namespace
Yahiewi Apr 9, 2026
d68774c
Lint extension
Yahiewi Apr 9, 2026
bd6e8d8
Fix conflicts
Yahiewi Apr 9, 2026
4a45d63
Improve error handling and messaging
Yahiewi Apr 10, 2026
ed34f4f
Improve error handling
Yahiewi Apr 13, 2026
041e088
Resolve conflicts
Yahiewi Apr 13, 2026
8694ae0
Lint
Yahiewi Apr 13, 2026
c07978c
Fix generateResponse
Yahiewi Apr 14, 2026
9c8aed3
Resolve conflicts
Yahiewi Apr 14, 2026
bedc7dc
Improve error logging
Yahiewi Apr 14, 2026
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
58 changes: 48 additions & 10 deletions src/agent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ import {
type TypedToolError,
type TypedToolOutputDenied,
type TypedToolResult,
type AssistantModelMessage
type UserContent,
type AssistantModelMessage,
APICallError
} from 'ai';
import { ISecretsManager } from 'jupyter-secrets-manager';

Expand Down Expand Up @@ -571,10 +573,17 @@ export class AgentManager implements IAgentManager {
* Handles the complete execution cycle including tool calls.
* @param message The user message to respond to (may include processed attachment content)
*/
async generateResponse(message: string): Promise<void> {
async generateResponse(message: UserContent): Promise<void> {
this._streaming = new PromiseDelegate();
this._controller = new AbortController();
const responseHistory: ModelMessage[] = [];

// Add user message to history
responseHistory.push({
role: 'user',
content: message
});

try {
// Ensure we have an agent
if (!this._agent) {
Expand All @@ -585,12 +594,6 @@ export class AgentManager implements IAgentManager {
throw new Error('Failed to initialize agent');
}

// Add user message to history
responseHistory.push({
role: 'user',
content: message
});

let continueLoop = true;
while (continueLoop) {
const result = await this._agent.stream({
Expand Down Expand Up @@ -647,9 +650,41 @@ export class AgentManager implements IAgentManager {
this._history.push(...Private.sanitizeModelMessages(responseHistory));
} catch (error) {
if ((error as Error).name !== 'AbortError') {
let helpMessage = `${(error as Error).message}`;

// Remove attachments from history on payload rejection errors
if (
APICallError.isInstance(error) &&
(error.statusCode === 400 ||
error.statusCode === 404 ||
error.statusCode === 413 ||
error.statusCode === 415 ||
error.statusCode === 422)
) {
for (const msg of [...this._history, ...responseHistory]) {
Copy link
Copy Markdown
Collaborator

@brichet brichet Apr 14, 2026

Choose a reason for hiding this comment

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

The error message should only appear in the responseHistory object; we probably don't need to go through the entire history.
Actually, if I'm not mistaken, the user message should always be the first message of the responseHistory, we can probably get it with responseHistory[0].

Copy link
Copy Markdown
Contributor Author

@Yahiewi Yahiewi Apr 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This is how I did it at first too (by only looking at the last sent user message).
However in the current version, I look at the entire history because of the following case:
If a user uses a vision-capable model at first and sends it attachment(s), they then later for some reason (say they no longer have any credits) decide to switch to a non vision-capable model, upon sending a message, the conversation would enter the infinite error loop because of the attachment(s) in the history.
Here's what it looks like:

Screencast.from.2026-04-14.17-03-50.mp4

As you can see we avoid the infinite error loop because the attachment is deleted when we catch the error. However, this is still not ideal I think because you need to send a message to trigger the error and then send the prompt. I'm currently thinking about how I can remove this friction.

EDIT: I tried encapsulating the function body in a while loop and make it retry sending the message once automatically after the first error of this kind and this works fine. But I think it overcomplicates the code a bit and makes it less readable so in the end I just improved the error logging:

image

Copy link
Copy Markdown
Collaborator

@brichet brichet Apr 14, 2026

Choose a reason for hiding this comment

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

Thanks for the clarification, indeed it would break when switching the model.

Let's keep it that way for now, but we should open an issue to handle it.

Copy link
Copy Markdown
Collaborator

@brichet brichet Apr 14, 2026

Choose a reason for hiding this comment

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

I opened #321

if (msg.role === 'user' && Array.isArray(msg.content)) {
const hasMedia = msg.content.some(p => p.type !== 'text');
if (hasMedia) {
const textContent = msg.content
.filter(p => p.type === 'text')
.map(p => (p as { text: string }).text)
.join('\n');
msg.content =
textContent || '_Attachment removed due to error_';
}
}
}
helpMessage +=
'\n\nAttachments have been removed from history. Please send your prompt again.';
}
this._agentEvent.emit({
type: 'error',
data: { error: error as Error }
data: { error: new Error(helpMessage) }
});
this._history.push(...Private.sanitizeModelMessages(responseHistory));
this._history.push({
role: 'assistant',
content: helpMessage
});
}
} finally {
Expand Down Expand Up @@ -932,6 +967,9 @@ ${richOutputWorkflowInstruction}`;
await this._handleApprovalRequest(part, processResult);
break;

case 'error':
throw part.error;

case 'finish-step':
this._updateTokenUsage(part.usage, part.usage.inputTokens);
break;
Expand All @@ -940,7 +978,7 @@ ${richOutputWorkflowInstruction}`;
processResult.aborted = true;
break;

// Ignore: text-start, text-end, finish, error, and others
// Ignore: text-start, text-end, finish, and others
default:
break;
}
Expand Down
Loading
Loading