diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 42be04bd9..60096caeb 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1344,6 +1344,10 @@ - Fixed Ruff LSP auto-detection for Windows Python virtualenvs by checking `.venv/Scripts`, `venv/Scripts`, and `.env/Scripts` before falling back to PATH. ([#3916](https://github.com/can1357/oh-my-pi/issues/3916)) - Added the opt-in `read.renderMarkdown` setting for formatted Markdown read previews, disabled by default. +### Changed + +- All Markdown flavors (`.markdown`, `.mdx`, `.mdc`, `.mkd`, `.mdown`) now follow the `read.summarize.prose` setting like `.md`, so they read verbatim instead of being code-block summarized when prose summaries are off. + ### Fixed - Fixed Markdown file read metadata so the opt-in Markdown preview renderer can recognize local and URI-backed Markdown files consistently. diff --git a/packages/coding-agent/src/tools/read.ts b/packages/coding-agent/src/tools/read.ts index 7addd1b3b..6519602c7 100644 --- a/packages/coding-agent/src/tools/read.ts +++ b/packages/coding-agent/src/tools/read.ts @@ -25,7 +25,6 @@ import { } from "@oh-my-pi/pi-utils"; import { type } from "arktype"; import { LRUCache } from "lru-cache/raw"; -import { isSettingsInitialized, settings } from "../config/settings"; import { canonicalSnapshotKey, getFileSnapshotStore, @@ -156,19 +155,11 @@ const MAX_SUMMARY_BYTES = 2 * 1024 * 1024; const MAX_SUMMARY_LINES = 20_000; const MAX_ARTIFACT_RAW_INLINE_BYTES = DEFAULT_MAX_BYTES; /** - * Per-line column cap for file reads. Lines wider than the value of - * `tools.outputMaxColumns` are ellipsis-truncated at display time; the file - * on disk is unchanged. Shared with the streaming sink path so one setting - * covers `bash`/`ssh`/`python`/`js eval` and `read` uniformly. + * Prose files (Markdown flavors and plain text) skip code-block summarization + * unless `read.summarize.prose` opts them in. */ -const PROSE_SUMMARY_EXTENSIONS = new Set([".txt"]); - -function isMarkdownContentPath(filePath: string): boolean { - return isMarkdownPath(filePath); -} - function isProseSummaryPath(filePath: string): boolean { - return isMarkdownContentPath(filePath) || PROSE_SUMMARY_EXTENSIONS.has(path.extname(filePath).toLowerCase()); + return isMarkdownPath(filePath) || path.extname(filePath).toLowerCase() === ".txt"; } // Remote mount path prefix (sshfs mounts) - skip fuzzy matching to avoid hangs @@ -789,14 +780,6 @@ export interface ReadToolDetails { /** Paths recovered from a delimited read argument; used only by the TUI to render one call as multiple read rows. */ displayReadTargets?: string[]; } - -function markMarkdownContentType(details: ReadToolDetails, filePath: string): ReadToolDetails { - if (!details.contentType && isMarkdownContentPath(filePath)) { - details.contentType = "text/markdown"; - } - return details; -} - type ReadParams = ReadToolInput; /** Parsed representation of a path-embedded selector. */ @@ -1615,7 +1598,7 @@ export class ReadTool implements AgentTool { try { const bridgeText = await bridgePromise; const bridgeResult = this.#buildInMemoryMultiRangeResult(bridgeText, ranges, { - details: markMarkdownContentType({ resolvedPath: absolutePath, suffixResolution }, absolutePath), + details: this.#markMarkdownContentType({ resolvedPath: absolutePath, suffixResolution }, absolutePath), sourcePath: absolutePath, entityLabel: "file", raw: rawSelector, @@ -1798,7 +1781,7 @@ export class ReadTool implements AgentTool { const archive = await openArchive(resolvedArchivePath.absolutePath); throwIfAborted(signal); - const details: ReadToolDetails = markMarkdownContentType( + const details: ReadToolDetails = this.#markMarkdownContentType( { resolvedPath: resolvedArchivePath.absolutePath, suffixResolution: resolvedArchivePath.suffixResolution, @@ -2015,6 +1998,20 @@ export class ReadTool implements AgentTool { return bridge.readTextFile({ path: absolutePath, ...options }); } + /** + * Tag Markdown reads for the TUI's formatted preview, gated on the opt-in + * `read.renderMarkdown` setting. Off by default; when disabled, no local + * read is tagged `text/markdown`, so the renderer output is identical to + * the pre-setting behavior. Internal-URL reads keep their protocol-supplied + * `contentType` and render as Markdown regardless of the setting. + */ + #markMarkdownContentType(details: ReadToolDetails, filePath: string): ReadToolDetails { + if (!details.contentType && this.session.settings.get("read.renderMarkdown") && isMarkdownPath(filePath)) { + details.contentType = "text/markdown"; + } + return details; + } + async #trySummarize(absolutePath: string, fileSize: number, signal?: AbortSignal): Promise { if (fileSize > MAX_SUMMARY_BYTES) return null; @@ -2439,14 +2436,20 @@ export class ReadTool implements AgentTool { // because only `truncateHead` was being applied. if (isMultiRange(parsed) && parsed.kind === "lines") { return this.#buildInMemoryMultiRangeResult(renderedContent, parsed.ranges, { - details: { resolvedPath: absolutePath, contentType: "text/markdown" }, + details: { + resolvedPath: absolutePath, + contentType: this.session.settings.get("read.renderMarkdown") ? "text/markdown" : undefined, + }, sourcePath: absolutePath, entityLabel: "document", }); } const { offset, limit } = selToOffsetLimit(parsed); return this.#buildInMemoryTextResult(renderedContent, offset, limit, { - details: { resolvedPath: absolutePath, contentType: "text/markdown" }, + details: { + resolvedPath: absolutePath, + contentType: this.session.settings.get("read.renderMarkdown") ? "text/markdown" : undefined, + }, sourcePath: absolutePath, entityLabel: "document", raw: isRawSelector(parsed), @@ -2539,7 +2542,7 @@ export class ReadTool implements AgentTool { try { const bridgeText = await bridgePromise; const bridgeResult = this.#buildInMemoryTextResult(bridgeText, offset, limit, { - details: markMarkdownContentType( + details: this.#markMarkdownContentType( { resolvedPath: absolutePath, suffixResolution }, absolutePath, ), @@ -2841,7 +2844,7 @@ export class ReadTool implements AgentTool { } } - markMarkdownContentType(details, absolutePath); + this.#markMarkdownContentType(details, absolutePath); if (suffixResolution) { details.suffixResolution = suffixResolution; // Inline resolution notice into first text block so the model sees the actual path @@ -3575,8 +3578,7 @@ export const readToolRenderer = { title += ` ${uiTheme.fg("warning", `(⚠ ${n} conflict${n === 1 ? "" : "s"})`)}`; } const rawRequested = args?.raw === true || isRawSelector(parseSel(renderPath.sel)); - const markdownPreviewEnabled = isSettingsInitialized() && settings.get("read.renderMarkdown"); - const isMarkdown = markdownPreviewEnabled && details?.contentType === "text/markdown" && !rawRequested; + const isMarkdown = details?.contentType === "text/markdown" && !rawRequested; let cachedWidth: number | undefined; let cachedExpanded: boolean | undefined; let cachedLines: string[] | undefined; diff --git a/packages/coding-agent/test/read-summary.test.ts b/packages/coding-agent/test/read-summary.test.ts index 128c68c15..b20a46ae4 100644 --- a/packages/coding-agent/test/read-summary.test.ts +++ b/packages/coding-agent/test/read-summary.test.ts @@ -100,16 +100,24 @@ describe("read summary", () => { expect(proseResult.details?.summary?.elidedSpans).toBe(1); }); - it("marks local Markdown-like extensions as markdown while preserving model-facing source text", async () => { + it("marks local Markdown-like extensions as markdown only when previews are enabled", async () => { const markdown = "# Heading\n\nSome **bold** text.\n"; const extensions = ["md", "markdown", "mdx", "mdc", "mkd", "mdown"] as const; - const tool = new ReadTool(createSession(tmpDir)); + const defaultTool = new ReadTool(createSession(tmpDir)); + const previewTool = new ReadTool(createSession(tmpDir, { "read.renderMarkdown": true })); for (const extension of extensions) { const fixture = path.join(tmpDir, `fixture.${extension}`); await fs.writeFile(fixture, markdown); - const result = await tool.execute(`read-summary-markdown-${extension}`, { path: fixture }); + // Default (setting off): no markdown tagging, byte-identical to the pre-setting behavior. + const defaultResult = await defaultTool.execute(`read-summary-markdown-default-${extension}`, { + path: fixture, + }); + expect(defaultResult.details?.contentType).toBeUndefined(); + + // Opt-in: tagged for the TUI preview while the model-facing text stays verbatim. + const result = await previewTool.execute(`read-summary-markdown-${extension}`, { path: fixture }); const text = textOutput(result); expect(result.details?.contentType).toBe("text/markdown"); @@ -120,6 +128,19 @@ describe("read summary", () => { } }); + it("keeps non-.md markdown flavors verbatim when prose summaries are disabled", async () => { + const fixture = path.join(tmpDir, "fixture.mdx"); + await fs.writeFile( + fixture, + "# Heading\n\nIntro line.\n\n```ts\nexport function alpha(): string {\n\tconst clean = 'alpha';\n\treturn clean;\n}\n```\n\nMore prose.\n", + ); + + const tool = new ReadTool(createSession(tmpDir)); + const result = await tool.execute("read-summary-mdx-default", { path: fixture }); + expect(textOutput(result)).toContain("const clean = 'alpha';"); + expect(result.details?.summary).toBeUndefined(); + }); + it("does not truncate summarized output", async () => { const fixture = path.join(tmpDir, "many.ts"); const source = Array.from( diff --git a/packages/coding-agent/test/tools/read-renderer.test.ts b/packages/coding-agent/test/tools/read-renderer.test.ts index ccd40dca0..2148882fd 100644 --- a/packages/coding-agent/test/tools/read-renderer.test.ts +++ b/packages/coding-agent/test/tools/read-renderer.test.ts @@ -25,7 +25,6 @@ beforeAll(async () => { afterEach(() => { settings.clearOverride("tui.hyperlinks"); - settings.clearOverride("read.renderMarkdown"); }); afterAll(() => { @@ -114,33 +113,7 @@ describe("readToolRenderer hyperlinks", () => { }); describe("readToolRenderer markdown content", () => { - it("keeps text/markdown details raw unless markdown rendering is enabled", async () => { - const theme = await getThemeByName("dark"); - expect(theme).toBeDefined(); - - const component = readToolRenderer.renderResult( - { - content: [{ type: "text", text: "[notes.md#ABCD]\n1:# Heading\n2:\n3:This is **bold** text." }], - details: { - displayContent: { text: "# Heading\n\nThis is **bold** text.", startLine: 1 }, - contentType: "text/markdown", - }, - }, - { expanded: true, isPartial: false }, - theme!, - { path: "notes.md" }, - ); - - const stripped = component - .render(100) - .map(line => Bun.stripANSI(line)) - .join("\n"); - expect(stripped).toContain("# Heading"); - expect(stripped).toContain("**bold**"); - }); - it("renders text/markdown details through the markdown renderer", async () => { - settings.override("read.renderMarkdown", true); const theme = await getThemeByName("dark"); expect(theme).toBeDefined(); @@ -167,8 +140,31 @@ describe("readToolRenderer markdown content", () => { expect(stripped).not.toContain("**bold**"); }); + it("keeps untagged markdown source in the code renderer", async () => { + const theme = await getThemeByName("dark"); + expect(theme).toBeDefined(); + + const component = readToolRenderer.renderResult( + { + content: [{ type: "text", text: "[notes.md#ABCD]\n1:# Heading\n2:\n3:This is **bold** text." }], + details: { + displayContent: { text: "# Heading\n\nThis is **bold** text.", startLine: 1 }, + }, + }, + { expanded: true, isPartial: false }, + theme!, + { path: "notes.md" }, + ); + + const stripped = component + .render(100) + .map(line => Bun.stripANSI(line)) + .join("\n"); + expect(stripped).toContain("# Heading"); + expect(stripped).toContain("**bold**"); + }); + it("keeps raw markdown selector reads in the code renderer", async () => { - settings.override("read.renderMarkdown", true); const theme = await getThemeByName("dark"); expect(theme).toBeDefined();