Skip to content
Closed
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
32 changes: 16 additions & 16 deletions src/core/mentions/__tests__/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe("parseMentions - URL error handling", () => {

expect(consoleErrorSpy).toHaveBeenCalledWith("Error fetching URL https://example.com:", timeoutError)
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content: Navigation timeout of 30000 ms exceeded")
expect(result.text).toContain("Error fetching content: Navigation timeout of 30000 ms exceeded")
})

it("should handle DNS resolution errors", async () => {
Expand All @@ -51,7 +51,7 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://nonexistent.example", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content: net::ERR_NAME_NOT_RESOLVED")
expect(result.text).toContain("Error fetching content: net::ERR_NAME_NOT_RESOLVED")
})

it("should handle network disconnection errors", async () => {
Expand All @@ -61,7 +61,7 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://example.com", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content: net::ERR_INTERNET_DISCONNECTED")
expect(result.text).toContain("Error fetching content: net::ERR_INTERNET_DISCONNECTED")
})

it("should handle 403 Forbidden errors", async () => {
Expand All @@ -71,7 +71,7 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://example.com", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content: 403 Forbidden")
expect(result.text).toContain("Error fetching content: 403 Forbidden")
})

it("should handle 404 Not Found errors", async () => {
Expand All @@ -81,7 +81,7 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://example.com/missing", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content: 404 Not Found")
expect(result.text).toContain("Error fetching content: 404 Not Found")
})

it("should handle generic errors with fallback message", async () => {
Expand All @@ -91,7 +91,7 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://example.com", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content: Some unexpected error")
expect(result.text).toContain("Error fetching content: Some unexpected error")
})

it("should handle non-Error objects thrown", async () => {
Expand All @@ -101,7 +101,7 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://example.com", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.url_fetch_error_with_url")
expect(result).toContain("Error fetching content:")
expect(result.text).toContain("Error fetching content:")
})

it("should handle browser launch errors correctly", async () => {
Expand All @@ -113,7 +113,7 @@ describe("parseMentions - URL error handling", () => {
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(
"Error fetching content for https://example.com: Failed to launch browser",
)
expect(result).toContain("Error fetching content: Failed to launch browser")
expect(result.text).toContain("Error fetching content: Failed to launch browser")
// Should not attempt to fetch URL if browser launch failed
expect(mockUrlContentFetcher.urlToMarkdown).not.toHaveBeenCalled()
})
Expand All @@ -127,7 +127,7 @@ describe("parseMentions - URL error handling", () => {
expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(
"Error fetching content for https://example.com: String error",
)
expect(result).toContain("Error fetching content: String error")
expect(result.text).toContain("Error fetching content: String error")
})

it("should successfully fetch URL content when no errors occur", async () => {
Expand All @@ -136,9 +136,9 @@ describe("parseMentions - URL error handling", () => {
const result = await parseMentions("Check @https://example.com", "/test", mockUrlContentFetcher)

expect(vscode.window.showErrorMessage).not.toHaveBeenCalled()
expect(result).toContain('<url_content url="https://example.com">')
expect(result).toContain("# Example Content\n\nThis is the content.")
expect(result).toContain("</url_content>")
expect(result.text).toContain('<url_content url="https://example.com">')
expect(result.text).toContain("# Example Content\n\nThis is the content.")
expect(result.text).toContain("</url_content>")
})

it("should handle multiple URLs with mixed success and failure", async () => {
Expand All @@ -152,9 +152,9 @@ describe("parseMentions - URL error handling", () => {
mockUrlContentFetcher,
)

expect(result).toContain('<url_content url="https://example1.com">')
expect(result).toContain("# First Site")
expect(result).toContain('<url_content url="https://example2.com">')
expect(result).toContain("Error fetching content: timeout")
expect(result.text).toContain('<url_content url="https://example1.com">')
expect(result.text).toContain("# First Site")
expect(result.text).toContain('<url_content url="https://example2.com">')
expect(result.text).toContain("Error fetching content: timeout")
})
})
27 changes: 16 additions & 11 deletions src/core/mentions/__tests__/processUserContentMentions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ describe("processUserContentMentions", () => {
mockFileContextTracker = {} as FileContextTracker
mockRooIgnoreController = {}

// Default mock implementation
vi.mocked(parseMentions).mockImplementation(async (text) => `parsed: ${text}`)
// Default mock implementation - parseMentions now returns an object with text and optional pdfAttachments
vi.mocked(parseMentions).mockImplementation(async (text) => ({ text: `parsed: ${text}` }))
})

describe("maxReadFileLine parameter", () => {
Expand Down Expand Up @@ -55,6 +55,7 @@ describe("processUserContentMentions", () => {
true, // includeDiagnosticMessages
50, // maxDiagnosticMessages
100,
true, // enablePdfMultimodal
)
})

Expand Down Expand Up @@ -84,6 +85,7 @@ describe("processUserContentMentions", () => {
true, // includeDiagnosticMessages
50, // maxDiagnosticMessages
undefined,
true, // enablePdfMultimodal
)
})

Expand Down Expand Up @@ -114,6 +116,7 @@ describe("processUserContentMentions", () => {
true, // includeDiagnosticMessages
50, // maxDiagnosticMessages
-1,
true, // enablePdfMultimodal
)
})
})
Expand All @@ -135,7 +138,7 @@ describe("processUserContentMentions", () => {
})

expect(parseMentions).toHaveBeenCalled()
expect(result[0]).toEqual({
expect(result.content[0]).toEqual({
type: "text",
text: "parsed: <task>Do something</task>",
})
Expand All @@ -157,7 +160,7 @@ describe("processUserContentMentions", () => {
})

expect(parseMentions).toHaveBeenCalled()
expect(result[0]).toEqual({
expect(result.content[0]).toEqual({
type: "text",
text: "parsed: <feedback>Fix this issue</feedback>",
})
Expand All @@ -179,7 +182,7 @@ describe("processUserContentMentions", () => {
})

expect(parseMentions).not.toHaveBeenCalled()
expect(result[0]).toEqual(userContent[0])
expect(result.content[0]).toEqual(userContent[0])
})

it("should process tool_result blocks with string content", async () => {
Expand All @@ -199,7 +202,7 @@ describe("processUserContentMentions", () => {
})

expect(parseMentions).toHaveBeenCalled()
expect(result[0]).toEqual({
expect(result.content[0]).toEqual({
type: "tool_result",
tool_use_id: "123",
content: "parsed: <feedback>Tool feedback</feedback>",
Expand Down Expand Up @@ -232,7 +235,7 @@ describe("processUserContentMentions", () => {
})

expect(parseMentions).toHaveBeenCalledTimes(1)
expect(result[0]).toEqual({
expect(result.content[0]).toEqual({
type: "tool_result",
tool_use_id: "123",
content: [
Expand Down Expand Up @@ -278,13 +281,13 @@ describe("processUserContentMentions", () => {
})

expect(parseMentions).toHaveBeenCalledTimes(2)
expect(result).toHaveLength(3)
expect(result[0]).toEqual({
expect(result.content).toHaveLength(3)
expect(result.content[0]).toEqual({
type: "text",
text: "parsed: <task>First task</task>",
})
expect(result[1]).toEqual(userContent[1]) // Image block unchanged
expect(result[2]).toEqual({
expect(result.content[1]).toEqual(userContent[1]) // Image block unchanged
expect(result.content[2]).toEqual({
type: "tool_result",
tool_use_id: "456",
content: "parsed: <feedback>Feedback</feedback>",
Expand Down Expand Up @@ -318,6 +321,7 @@ describe("processUserContentMentions", () => {
true, // includeDiagnosticMessages
50, // maxDiagnosticMessages
undefined,
true, // enablePdfMultimodal
)
})

Expand Down Expand Up @@ -347,6 +351,7 @@ describe("processUserContentMentions", () => {
true, // includeDiagnosticMessages
50, // maxDiagnosticMessages
undefined,
true, // enablePdfMultimodal
)
})
})
Expand Down
59 changes: 42 additions & 17 deletions src/core/mentions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { getCommitInfo, getWorkingState } from "../../utils/git"
import { getWorkspacePath } from "../../utils/path"

import { openFile } from "../../integrations/misc/open-file"
import { extractTextFromFile } from "../../integrations/misc/extract-text"
import { extractTextFromFile, supportsMultimodalAnalysis } from "../../integrations/misc/extract-text"
import { diagnosticsToProblemsString } from "../../integrations/diagnostics"

import { UrlContentFetcher } from "../../services/browser/UrlContentFetcher"
Expand Down Expand Up @@ -87,9 +87,11 @@ export async function parseMentions(
includeDiagnosticMessages: boolean = true,
maxDiagnosticMessages: number = 50,
maxReadFileLine?: number,
): Promise<string> {
enablePdfMultimodal: boolean = true,
): Promise<{ text: string; pdfAttachments?: Array<{ path: string; data: any }> }> {
const mentions: Set<string> = new Set()
const validCommands: Map<string, Command> = new Map()
const pdfAttachments: Array<{ path: string; data: any }> = []

// First pass: check which command mentions exist and cache the results
const commandMatches = Array.from(text.matchAll(commandRegexGlobal))
Expand Down Expand Up @@ -188,20 +190,30 @@ export async function parseMentions(
} else if (mention.startsWith("/")) {
const mentionPath = mention.slice(1)
try {
const content = await getFileOrFolderContent(
const result = await getFileOrFolderContent(
mentionPath,
cwd,
rooIgnoreController,
showRooIgnoredFiles,
maxReadFileLine,
enablePdfMultimodal,
)
if (mention.endsWith("/")) {
parsedText += `\n\n<folder_content path="${mentionPath}">\n${content}\n</folder_content>`

// Check if this is a PDF with multimodal content
if (result.pdfData) {
pdfAttachments.push({
path: mentionPath,
data: result.pdfData,
})
parsedText += `\n\n<file_content path="${mentionPath}">\n[PDF file attached for multimodal analysis - contains visual elements like charts, diagrams, and tables]\n${result.content}\n</file_content>`
} else if (mention.endsWith("/")) {
parsedText += `\n\n<folder_content path="${mentionPath}">\n${result.content}\n</folder_content>`
} else {
parsedText += `\n\n<file_content path="${mentionPath}">\n${content}\n</file_content>`
if (fileContextTracker) {
await fileContextTracker.trackFileContext(mentionPath, "file_mentioned")
}
parsedText += `\n\n<file_content path="${mentionPath}">\n${result.content}\n</file_content>`
}

if (fileContextTracker && !mention.endsWith("/")) {
await fileContextTracker.trackFileContext(mentionPath, "file_mentioned")
}
} catch (error) {
if (mention.endsWith("/")) {
Expand Down Expand Up @@ -263,7 +275,7 @@ export async function parseMentions(
}
}

return parsedText
return { text: parsedText, pdfAttachments: pdfAttachments.length > 0 ? pdfAttachments : undefined }
}

async function getFileOrFolderContent(
Expand All @@ -272,7 +284,8 @@ async function getFileOrFolderContent(
rooIgnoreController?: any,
showRooIgnoredFiles: boolean = true,
maxReadFileLine?: number,
): Promise<string> {
enablePdfMultimodal: boolean = true,
): Promise<{ content: string; pdfData?: any }> {
const unescapedPath = unescapeSpaces(mentionPath)
const absPath = path.resolve(cwd, unescapedPath)

Expand All @@ -281,13 +294,25 @@ async function getFileOrFolderContent(

if (stats.isFile()) {
if (rooIgnoreController && !rooIgnoreController.validateAccess(absPath)) {
return `(File ${mentionPath} is ignored by .rooignore)`
return { content: `(File ${mentionPath} is ignored by .rooignore)` }
}
try {
const content = await extractTextFromFile(absPath, maxReadFileLine)
return content
// Check if this is a PDF and multimodal is enabled
const fileExtension = path.extname(absPath).toLowerCase()
if (enablePdfMultimodal && supportsMultimodalAnalysis(fileExtension)) {
// Get both text and multimodal content for PDFs
const textContent = (await extractTextFromFile(absPath, maxReadFileLine, false)) as string
const pdfData = await extractTextFromFile(absPath, maxReadFileLine, true)
return {
content: textContent,
pdfData: pdfData,
}
} else {
const content = (await extractTextFromFile(absPath, maxReadFileLine, false)) as string
return { content }
}
} catch (error) {
return `(Failed to read contents of ${mentionPath}): ${error.message}`
return { content: `(Failed to read contents of ${mentionPath}): ${error.message}` }
}
} else if (stats.isDirectory()) {
const entries = await fs.readdir(absPath, { withFileTypes: true })
Expand Down Expand Up @@ -339,9 +364,9 @@ async function getFileOrFolderContent(
}
}
const fileContents = (await Promise.all(fileContentPromises)).filter((content) => content)
return `${folderContent}\n${fileContents.join("\n\n")}`.trim()
return { content: `${folderContent}\n${fileContents.join("\n\n")}`.trim() }
} else {
return `(Failed to read contents of ${mentionPath})`
return { content: `(Failed to read contents of ${mentionPath})` }
}
} catch (error) {
throw new Error(`Failed to access path "${mentionPath}": ${error.message}`)
Expand Down
Loading
Loading