mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-08 22:21:23 +00:00
Fix log message parsing inside code blocks within tool parameters
- Enhanced DirectiveStreamingParser to track code block state within tool parameters - Fixed FallbackParser to respect code blocks and properly parse nested XML structures - Added comprehensive tests for both streaming and fallback scenarios - Prevents log messages inside code blocks from being processed as actual directives Fixes issue where <log_message> tags inside code blocks within tool parameters were being parsed as separate log directives instead of plain text content.
This commit is contained in:
parent
cc1a4df15c
commit
5ef186300e
5 changed files with 288 additions and 12 deletions
|
|
@ -26,8 +26,15 @@ export class DirectiveStreamingParser {
|
|||
let activeHandler: any = null
|
||||
|
||||
parser.onopentag = (node: sax.Tag) => {
|
||||
// Check if we're inside a code block (either global or within tool parameters)
|
||||
const insideCodeBlock =
|
||||
context.codeBlockState === CodeBlockState.INSIDE ||
|
||||
(activeHandler &&
|
||||
"isInsideParameterCodeBlock" in activeHandler &&
|
||||
(activeHandler as any).isInsideParameterCodeBlock())
|
||||
|
||||
// Only process XML tags if NOT inside code block
|
||||
if (context.codeBlockState !== CodeBlockState.INSIDE) {
|
||||
if (!insideCodeBlock) {
|
||||
context.hasXmlTags = true
|
||||
tagStack.push(node.name)
|
||||
const handler = this.registry.getHandler(node.name)
|
||||
|
|
@ -42,12 +49,23 @@ export class DirectiveStreamingParser {
|
|||
} else {
|
||||
// Inside code block - treat as plain text
|
||||
const tagText = `<${node.name}${this.attributesToString(node.attributes)}>`
|
||||
this.registry.getTextHandler().onText(tagText, context)
|
||||
if (activeHandler) {
|
||||
activeHandler.onText(tagText, context)
|
||||
} else {
|
||||
this.registry.getTextHandler().onText(tagText, context)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
parser.onclosetag = (tagName: string) => {
|
||||
if (context.codeBlockState !== CodeBlockState.INSIDE) {
|
||||
// Check if we're inside a code block (either global or within tool parameters)
|
||||
const insideCodeBlock =
|
||||
context.codeBlockState === CodeBlockState.INSIDE ||
|
||||
(activeHandler &&
|
||||
"isInsideParameterCodeBlock" in activeHandler &&
|
||||
(activeHandler as any).isInsideParameterCodeBlock())
|
||||
|
||||
if (!insideCodeBlock) {
|
||||
// Normal XML processing
|
||||
if (activeHandler) {
|
||||
activeHandler.onCloseTag(tagName, context)
|
||||
|
|
@ -59,7 +77,11 @@ export class DirectiveStreamingParser {
|
|||
tagStack.pop()
|
||||
} else {
|
||||
// Inside code block - treat as plain text
|
||||
this.registry.getTextHandler().onText(`</${tagName}>`, context)
|
||||
if (activeHandler) {
|
||||
activeHandler.onText(`</${tagName}>`, context)
|
||||
} else {
|
||||
this.registry.getTextHandler().onText(`</${tagName}>`, context)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -6,12 +6,34 @@ export class FallbackParser {
|
|||
static parse(assistantMessage: string): Directive[] {
|
||||
const contentBlocks: Directive[] = []
|
||||
|
||||
// Check if we're inside code blocks before parsing log messages
|
||||
const codeBlockRegex = /```[\s\S]*?```/g
|
||||
const codeBlocks: Array<{ start: number; end: number }> = []
|
||||
let codeBlockMatch
|
||||
|
||||
// Find all code block ranges
|
||||
while ((codeBlockMatch = codeBlockRegex.exec(assistantMessage)) !== null) {
|
||||
codeBlocks.push({
|
||||
start: codeBlockMatch.index,
|
||||
end: codeBlockMatch.index + codeBlockMatch[0].length,
|
||||
})
|
||||
}
|
||||
|
||||
// Helper function to check if a position is inside a code block
|
||||
const isInsideCodeBlock = (position: number): boolean => {
|
||||
return codeBlocks.some((block) => position >= block.start && position < block.end)
|
||||
}
|
||||
|
||||
// Handle multiple log messages
|
||||
const logMessageRegex = /<log_message>([\s\S]*?)(?:<\/log_message>|$)/g
|
||||
let lastIndex = 0
|
||||
let match
|
||||
|
||||
while ((match = logMessageRegex.exec(assistantMessage)) !== null) {
|
||||
// Skip log messages that are inside code blocks
|
||||
if (isInsideCodeBlock(match.index)) {
|
||||
continue
|
||||
}
|
||||
// Add any text before this log message
|
||||
if (match.index > lastIndex) {
|
||||
const textBefore = assistantMessage.substring(lastIndex, match.index).trim()
|
||||
|
|
@ -65,14 +87,69 @@ export class FallbackParser {
|
|||
const toolContent = toolMatch[0]
|
||||
const params: Record<string, string> = {}
|
||||
|
||||
// Extract parameters
|
||||
const paramRegex = /<(\w+)>(.*?)(?:<\/\1>|$)/g
|
||||
let paramMatch
|
||||
while ((paramMatch = paramRegex.exec(toolContent)) !== null) {
|
||||
const [, paramName, paramValue] = paramMatch
|
||||
if (paramName !== toolName) {
|
||||
// Extract parameters - need to be more careful about nested structures
|
||||
// Find direct child parameters of the tool, not nested ones
|
||||
const toolInnerContent = toolContent
|
||||
.replace(new RegExp(`^<${toolName}>`), "")
|
||||
.replace(new RegExp(`</${toolName}>$`), "")
|
||||
|
||||
// Use a more sophisticated approach to find top-level parameters
|
||||
let currentIndex = 0
|
||||
while (currentIndex < toolInnerContent.length) {
|
||||
// Find the next opening tag
|
||||
const tagMatch = toolInnerContent.substring(currentIndex).match(/<(\w+)>/)
|
||||
if (!tagMatch) break
|
||||
|
||||
const paramName = tagMatch[1]
|
||||
const tagStart = currentIndex + tagMatch.index!
|
||||
const contentStart = tagStart + tagMatch[0].length
|
||||
|
||||
// Find the matching closing tag, accounting for nested tags
|
||||
let depth = 1
|
||||
let searchIndex = contentStart
|
||||
let paramValue = ""
|
||||
|
||||
while (depth > 0 && searchIndex < toolInnerContent.length) {
|
||||
const nextTag = toolInnerContent.substring(searchIndex).match(/<\/?(\w+)>/)
|
||||
if (!nextTag) {
|
||||
// No more tags, take the rest as content
|
||||
paramValue = toolInnerContent.substring(contentStart)
|
||||
break
|
||||
}
|
||||
|
||||
const tagName = nextTag[1]
|
||||
const isClosing = nextTag[0].startsWith("</")
|
||||
|
||||
if (tagName === paramName) {
|
||||
if (isClosing) {
|
||||
depth--
|
||||
if (depth === 0) {
|
||||
// Found the matching closing tag
|
||||
paramValue = toolInnerContent.substring(
|
||||
contentStart,
|
||||
searchIndex + nextTag.index!,
|
||||
)
|
||||
currentIndex = searchIndex + nextTag.index! + nextTag[0].length
|
||||
break
|
||||
}
|
||||
} else {
|
||||
depth++
|
||||
}
|
||||
}
|
||||
|
||||
searchIndex += nextTag.index! + nextTag[0].length
|
||||
}
|
||||
|
||||
if (paramName !== toolName && paramValue !== undefined) {
|
||||
params[paramName] = paramValue
|
||||
}
|
||||
|
||||
if (depth > 0) {
|
||||
// Unclosed tag, take the rest
|
||||
paramValue = toolInnerContent.substring(contentStart)
|
||||
params[paramName] = paramValue
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
const ToolDirective: ToolDirective = {
|
||||
|
|
|
|||
|
|
@ -137,4 +137,76 @@ suite("DirectiveStreamingParser", () => {
|
|||
} as TextDirective,
|
||||
])
|
||||
})
|
||||
|
||||
test("should not parse directives inside code blocks within tool directive parameters", () => {
|
||||
const input =
|
||||
"<attempt_completion><result>Here's the format:\n\n```xml\n<log_message>\n<message>This should be plain text</message>\n<level>debug</level>\n</log_message>\n```\n\nThat's the format.</result></attempt_completion>"
|
||||
const result = DirectiveStreamingParser.parse(input)
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].type).toBe("tool_use")
|
||||
expect((result[0] as any).name).toBe("attempt_completion")
|
||||
expect((result[0] as any).params.result).toContain("```xml")
|
||||
expect((result[0] as any).params.result).toContain("<log_message>")
|
||||
expect((result[0] as any).params.result).toContain("This should be plain text")
|
||||
// The key test: ensure it's treated as one text block, not parsed as separate directives
|
||||
expect((result[0] as any).params.result).toBe(
|
||||
"Here's the format:\n\n```xml\n<log_message>\n<message>This should be plain text</message>\n<level>debug</level>\n</log_message>\n```\n\nThat's the format.",
|
||||
)
|
||||
})
|
||||
|
||||
test("should not parse directives inside code blocks within tool directive parameters during streaming", () => {
|
||||
// Simulate streaming chunks
|
||||
const chunks = [
|
||||
"<attempt_completion>",
|
||||
"<result>Here's the format:\n\n```xml\n",
|
||||
"<log_message>\n<message>This should be plain text</message>\n<level>debug</level>\n</log_message>\n",
|
||||
"```\n\nThat's the format.</result>",
|
||||
"</attempt_completion>",
|
||||
]
|
||||
|
||||
let accumulatedMessage = ""
|
||||
let finalResult: any[] = []
|
||||
|
||||
// Test each streaming chunk
|
||||
for (const chunk of chunks) {
|
||||
accumulatedMessage += chunk
|
||||
const result = DirectiveStreamingParser.parse(accumulatedMessage)
|
||||
finalResult = result
|
||||
}
|
||||
|
||||
// Final result should have only one directive (attempt_completion)
|
||||
expect(finalResult).toHaveLength(1)
|
||||
expect(finalResult[0].type).toBe("tool_use")
|
||||
expect(finalResult[0].name).toBe("attempt_completion")
|
||||
|
||||
// The result parameter should contain the log_message as plain text
|
||||
expect(finalResult[0].params.result).toContain("<log_message>")
|
||||
expect(finalResult[0].params.result).toContain("This should be plain text")
|
||||
|
||||
// Most importantly: there should be NO separate log_message directive
|
||||
const logMessages = finalResult.filter((r: any) => r.type === "log_message")
|
||||
expect(logMessages).toHaveLength(0)
|
||||
})
|
||||
|
||||
test("should handle malformed XML that might trigger FallbackParser", () => {
|
||||
// Test a scenario that might cause parse errors and trigger FallbackParser
|
||||
const input =
|
||||
"<attempt_completion><result>Here's the format:\n\n```xml\n<log_message>\n<message>This should be plain text</message>\n<level>debug</level>\n</log_message>\n```\n\nThat's the format.</result></attempt_completion>"
|
||||
|
||||
// Add some malformed XML to potentially trigger fallback
|
||||
const malformedInput = input + "<unclosed_tag>"
|
||||
|
||||
const result = DirectiveStreamingParser.parse(malformedInput)
|
||||
|
||||
// Should still not parse log_message as separate directive
|
||||
const logMessages = result.filter((r: any) => r.type === "log_message")
|
||||
expect(logMessages).toHaveLength(0)
|
||||
|
||||
// Should have attempt_completion with log_message preserved as text
|
||||
const attemptCompletion = result.find((r: any) => r.type === "tool_use" && r.name === "attempt_completion")
|
||||
expect(attemptCompletion).toBeDefined()
|
||||
if (attemptCompletion && attemptCompletion.type === "tool_use") {
|
||||
expect((attemptCompletion as any).params.result).toContain("<log_message>")
|
||||
}
|
||||
})
|
||||
})
|
||||
|
|
|
|||
81
src/core/message-parsing/__tests__/fallback-parser.spec.ts
Normal file
81
src/core/message-parsing/__tests__/fallback-parser.spec.ts
Normal file
|
|
@ -0,0 +1,81 @@
|
|||
import { FallbackParser } from "../FallbackParser"
|
||||
import { LogDirective, TextDirective } from "../directives"
|
||||
|
||||
describe("FallbackParser", () => {
|
||||
test("should not parse log messages inside code blocks", () => {
|
||||
const input = `Here's the format:
|
||||
|
||||
\`\`\`xml
|
||||
<log_message>
|
||||
<message>This should be plain text</message>
|
||||
<level>debug</level>
|
||||
</log_message>
|
||||
\`\`\`
|
||||
|
||||
That should be treated as code.`
|
||||
|
||||
const result = FallbackParser.parse(input)
|
||||
|
||||
// Should only have text directive, no log message directive
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].type).toBe("text")
|
||||
expect((result[0] as TextDirective).content).toContain("<log_message>")
|
||||
expect((result[0] as TextDirective).content).toContain("This should be plain text")
|
||||
})
|
||||
|
||||
test("should parse log messages outside code blocks", () => {
|
||||
const input = `Some text
|
||||
|
||||
<log_message>
|
||||
<message>This is a real log message</message>
|
||||
<level>info</level>
|
||||
</log_message>
|
||||
|
||||
More text with code:
|
||||
|
||||
\`\`\`xml
|
||||
<log_message>
|
||||
<message>This should be ignored</message>
|
||||
<level>debug</level>
|
||||
</log_message>
|
||||
\`\`\`
|
||||
|
||||
End text.`
|
||||
|
||||
const result = FallbackParser.parse(input)
|
||||
|
||||
// Should have text + log message + text
|
||||
expect(result).toHaveLength(3)
|
||||
expect(result[0].type).toBe("text")
|
||||
expect(result[1].type).toBe("log_message")
|
||||
expect((result[1] as LogDirective).message).toBe("This is a real log message")
|
||||
expect(result[2].type).toBe("text")
|
||||
expect((result[2] as TextDirective).content).toContain("This should be ignored")
|
||||
})
|
||||
|
||||
test("should handle attempt_completion with log messages in code blocks", () => {
|
||||
const input = `<attempt_completion><result>Here's the format:
|
||||
|
||||
\`\`\`xml
|
||||
<log_message>
|
||||
<message>This should be plain text</message>
|
||||
<level>debug</level>
|
||||
</log_message>
|
||||
\`\`\`
|
||||
|
||||
That's the format.</result></attempt_completion>`
|
||||
|
||||
const result = FallbackParser.parse(input)
|
||||
|
||||
// Should have tool directive, no separate log message
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].type).toBe("tool_use")
|
||||
expect((result[0] as any).name).toBe("attempt_completion")
|
||||
expect((result[0] as any).params.result).toContain("<log_message>")
|
||||
expect((result[0] as any).params.result).toContain("This should be plain text")
|
||||
|
||||
// No separate log message directive
|
||||
const logMessages = result.filter((r) => r.type === "log_message")
|
||||
expect(logMessages).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
|
@ -1,7 +1,8 @@
|
|||
import * as sax from "sax"
|
||||
import { BaseDirectiveHandler } from "./BaseDirectiveHandler"
|
||||
import { ParseContext } from "../ParseContext"
|
||||
import { ParseContext, CodeBlockState } from "../ParseContext"
|
||||
import { ToolDirective, ToolParamName } from "../directives"
|
||||
import { CodeBlockStateMachine } from "../CodeBlockStateMachine"
|
||||
|
||||
export class ToolDirectiveHandler extends BaseDirectiveHandler {
|
||||
readonly tagName: string
|
||||
|
|
@ -9,6 +10,8 @@ export class ToolDirectiveHandler extends BaseDirectiveHandler {
|
|||
private currentParamName?: ToolParamName
|
||||
private currentParamValue = ""
|
||||
private currentContext: "param" | "none" = "none"
|
||||
private stateMachine = new CodeBlockStateMachine()
|
||||
private paramCodeBlockState: CodeBlockState = CodeBlockState.OUTSIDE
|
||||
|
||||
constructor(toolName: string) {
|
||||
super()
|
||||
|
|
@ -29,6 +32,8 @@ export class ToolDirectiveHandler extends BaseDirectiveHandler {
|
|||
this.currentParamName = node.name as ToolParamName
|
||||
this.currentParamValue = ""
|
||||
this.currentContext = "param"
|
||||
// Reset code block state for new parameter
|
||||
this.paramCodeBlockState = CodeBlockState.OUTSIDE
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -49,7 +54,19 @@ export class ToolDirectiveHandler extends BaseDirectiveHandler {
|
|||
|
||||
override onText(text: string, context: ParseContext): void {
|
||||
if (this.currentContext === "param" && this.currentParamName && this.currentToolDirective) {
|
||||
this.currentParamValue += text
|
||||
// Create a temporary context to track code block state within this parameter
|
||||
const tempContext = {
|
||||
...context,
|
||||
codeBlockState: this.paramCodeBlockState,
|
||||
}
|
||||
|
||||
// Process text through the code block state machine
|
||||
const result = this.stateMachine.processText(text, tempContext)
|
||||
|
||||
// Update our parameter-specific code block state
|
||||
this.paramCodeBlockState = tempContext.codeBlockState
|
||||
|
||||
this.currentParamValue += result.processedText
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -63,4 +80,11 @@ export class ToolDirectiveHandler extends BaseDirectiveHandler {
|
|||
context.contentBlocks.push(this.currentToolDirective)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if we're currently inside a code block within a tool parameter
|
||||
*/
|
||||
isInsideParameterCodeBlock(): boolean {
|
||||
return this.currentContext === "param" && this.paramCodeBlockState === CodeBlockState.INSIDE
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue