mirror of
https://github.com/logancyang/obsidian-copilot.git
synced 2026-07-22 07:50:24 +00:00
fix: prevent </div> from being consumed by indented code blocks in think sections (#2343)
When thinking content ends with a 4-space-indented line (e.g. Gemma's bullet-point reasoning), markdown's indented code block rule would consume the immediately-following </div> closing tag and render it as literal "</div>" text. Fix by always trimming section content and appending \n before the closing tag so it lands on its own unindented line. Also adds integration tests covering non-streaming, streaming-complete, and streaming-unclosed think block paths. Co-authored-by: H <trulyshelton@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
bfc7a4e017
commit
baa2b954dc
2 changed files with 148 additions and 14 deletions
|
|
@ -67,6 +67,135 @@ const { __renderMarkdownMock: renderMarkdownMock } = jest.requireMock("obsidian"
|
|||
__renderMarkdownMock: jest.Mock;
|
||||
};
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Verifies that the HTML string passed to MarkdownRenderer.renderMarkdown
|
||||
// never has </div> or </details> on the same line as a 4-space-indented
|
||||
// line. This was the root cause of the Gemma rendering bug: google/gemma-4-31b-it
|
||||
// thinking output ends with 4-space-indented bullet points, and without a
|
||||
// trailing \n the closing </div> was consumed by markdown's indented code
|
||||
// block rule and rendered as literal "</div>" text.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("think block rendering — closing tags are not consumed by indented code blocks", () => {
|
||||
const createAppStub = (): App =>
|
||||
({
|
||||
workspace: { getActiveFile: jest.fn(() => null) },
|
||||
metadataCache: { getFirstLinkpathDest: jest.fn(() => null) },
|
||||
}) as unknown as App;
|
||||
|
||||
const baseAiMessage: ChatMessage = {
|
||||
id: "ai-1",
|
||||
sender: "AI",
|
||||
message: "",
|
||||
isVisible: true,
|
||||
timestamp: null,
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
renderMarkdownMock.mockReset();
|
||||
});
|
||||
|
||||
beforeAll(() => {
|
||||
(globalThis as any).activeDocument = document;
|
||||
});
|
||||
|
||||
/**
|
||||
* Asserts that no line in the rendered markdown matches:
|
||||
* <4+ spaces><any content></div> or <4+ spaces><any content></details>
|
||||
* Such a pattern means the closing tag is inside a code block and will be
|
||||
* escaped by the markdown renderer.
|
||||
*/
|
||||
function assertNoClosingTagOnIndentedLine(capturedMarkdown: string[]) {
|
||||
for (const md of capturedMarkdown) {
|
||||
for (const line of md.split("\n")) {
|
||||
if (/^ {4}/.test(line)) {
|
||||
expect(line).not.toContain("</div>");
|
||||
expect(line).not.toContain("</details>");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
it("does not place </div> on a 4-space-indented line (non-streaming, think block)", async () => {
|
||||
const thinkContent =
|
||||
"Planning my response:\n * Be helpful and direct.\n * Answer clearly.";
|
||||
const messageText = `<think>${thinkContent}</think>Here is my answer.`;
|
||||
|
||||
const capturedMarkdown: string[] = [];
|
||||
renderMarkdownMock.mockImplementation((md: string, el: HTMLElement) => {
|
||||
capturedMarkdown.push(md);
|
||||
el.innerHTML = "<p>rendered</p>";
|
||||
});
|
||||
|
||||
render(
|
||||
<TooltipProvider>
|
||||
<ChatSingleMessage
|
||||
message={{ ...baseAiMessage, message: messageText }}
|
||||
app={createAppStub()}
|
||||
isStreaming={false}
|
||||
onDelete={() => {}}
|
||||
/>
|
||||
</TooltipProvider>
|
||||
);
|
||||
|
||||
await waitFor(() => expect(renderMarkdownMock).toHaveBeenCalled());
|
||||
|
||||
assertNoClosingTagOnIndentedLine(capturedMarkdown);
|
||||
});
|
||||
|
||||
it("does not place </div> on a 4-space-indented line (streaming, complete think block)", async () => {
|
||||
const thinkContent = "Thinking:\n 1. First step.\n 2. Second step.";
|
||||
const messageText = `<think>${thinkContent}</think>Response text.`;
|
||||
|
||||
const capturedMarkdown: string[] = [];
|
||||
renderMarkdownMock.mockImplementation((md: string, el: HTMLElement) => {
|
||||
capturedMarkdown.push(md);
|
||||
el.innerHTML = "<p>rendered</p>";
|
||||
});
|
||||
|
||||
render(
|
||||
<TooltipProvider>
|
||||
<ChatSingleMessage
|
||||
message={{ ...baseAiMessage, message: messageText }}
|
||||
app={createAppStub()}
|
||||
isStreaming={true}
|
||||
onDelete={() => {}}
|
||||
/>
|
||||
</TooltipProvider>
|
||||
);
|
||||
|
||||
await waitFor(() => expect(renderMarkdownMock).toHaveBeenCalled());
|
||||
|
||||
assertNoClosingTagOnIndentedLine(capturedMarkdown);
|
||||
});
|
||||
|
||||
it("does not place </div> on a 4-space-indented line (streaming, unclosed think block)", async () => {
|
||||
// Simulates mid-stream: the </think> closing tag has not arrived yet.
|
||||
const messageText = "<think>Thinking:\n * Still streaming.";
|
||||
|
||||
const capturedMarkdown: string[] = [];
|
||||
renderMarkdownMock.mockImplementation((md: string, el: HTMLElement) => {
|
||||
capturedMarkdown.push(md);
|
||||
el.innerHTML = "<p>rendered</p>";
|
||||
});
|
||||
|
||||
render(
|
||||
<TooltipProvider>
|
||||
<ChatSingleMessage
|
||||
message={{ ...baseAiMessage, message: messageText }}
|
||||
app={createAppStub()}
|
||||
isStreaming={true}
|
||||
onDelete={() => {}}
|
||||
/>
|
||||
</TooltipProvider>
|
||||
);
|
||||
|
||||
await waitFor(() => expect(renderMarkdownMock).toHaveBeenCalled());
|
||||
|
||||
assertNoClosingTagOnIndentedLine(capturedMarkdown);
|
||||
});
|
||||
});
|
||||
|
||||
describe("normalizeFootnoteRendering", () => {
|
||||
beforeEach(() => {
|
||||
renderMarkdownMock.mockReset();
|
||||
|
|
|
|||
|
|
@ -373,6 +373,18 @@ const ChatSingleMessage: React.FC<ChatSingleMessageProps> = ({
|
|||
const openTag = `<${tagName}>`;
|
||||
let sectionIndex = 0;
|
||||
|
||||
// Trims content and appends \n so the closing </div> always lands on
|
||||
// its own unindented line. Without this, content ending with a
|
||||
// 4-space-indented line (a markdown code block) would cause </div> to
|
||||
// be consumed by the code block and rendered as escaped literal text.
|
||||
const ensureClosingTagOnNewLine = (text: string) => text.trim() + "\n";
|
||||
|
||||
const buildDetails = (sectionContent: string, openAttr: string, domId: string) =>
|
||||
`<details id="${domId}"${openAttr} style="${detailsStyle}">` +
|
||||
`<summary style="${summaryStyle}">${summaryText}</summary>` +
|
||||
`<div class="tw-text-muted" style="${contentStyle}">${ensureClosingTagOnNewLine(sectionContent)}</div>` +
|
||||
`</details>\n\n`;
|
||||
|
||||
// During streaming, if we find any tag that's either unclosed or being processed
|
||||
if (isStreaming && content.includes(openTag)) {
|
||||
// Replace any complete sections first
|
||||
|
|
@ -383,21 +395,18 @@ const ChatSingleMessage: React.FC<ChatSingleMessageProps> = ({
|
|||
const domId = buildCopilotCollapsibleDomId(messageId.current, sectionKey);
|
||||
// Check if user has explicitly set a state; if not, default to collapsed (original behavior)
|
||||
const openAttribute = collapsibleOpenStateMap.get(domId) ? " open" : "";
|
||||
|
||||
return `<details id="${domId}"${openAttribute} style="${detailsStyle}">
|
||||
<summary style="${summaryStyle}">${summaryText}</summary>
|
||||
<div class="tw-text-muted" style="${contentStyle}">${sectionContent.trim()}</div>
|
||||
</details>\n\n`;
|
||||
return buildDetails(sectionContent, openAttribute, domId);
|
||||
});
|
||||
|
||||
// Then handle any unclosed tag, but preserve the streamed content
|
||||
const unClosedRegex = new RegExp(`<${tagName}>([\\s\\S]*)$`);
|
||||
content = content.replace(
|
||||
unClosedRegex,
|
||||
(_match, partialContent) => `<div style="${detailsStyle}">
|
||||
<div style="${summaryStyle}">${streamingSummaryText}</div>
|
||||
<div class="tw-text-muted" style="${contentStyle}">${partialContent.trim()}</div>
|
||||
</div>`
|
||||
(_match, partialContent) =>
|
||||
`<div style="${detailsStyle}">` +
|
||||
`<div style="${summaryStyle}">${streamingSummaryText}</div>` +
|
||||
`<div class="tw-text-muted" style="${contentStyle}">${ensureClosingTagOnNewLine(partialContent)}</div>` +
|
||||
`</div>`
|
||||
);
|
||||
return content;
|
||||
}
|
||||
|
|
@ -410,11 +419,7 @@ const ChatSingleMessage: React.FC<ChatSingleMessageProps> = ({
|
|||
const domId = buildCopilotCollapsibleDomId(messageId.current, sectionKey);
|
||||
// Restore open state from previous render
|
||||
const openAttribute = collapsibleOpenStateMap.get(domId) ? " open" : "";
|
||||
|
||||
return `<details id="${domId}"${openAttribute} style="${detailsStyle}">
|
||||
<summary style="${summaryStyle}">${summaryText}</summary>
|
||||
<div class="tw-text-muted" style="${contentStyle}">${sectionContent.trim()}</div>
|
||||
</details>\n\n`;
|
||||
return buildDetails(sectionContent, openAttribute, domId);
|
||||
});
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue