smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(coding-agent): preserved default markdown rendering for internal-URL reads

Browse Source 
Gated the read.renderMarkdown opt-in at read time (details tagging) instead
of inside the renderer. The renderer gate silently flipped every
protocol-supplied text/markdown read (skill://, pr://, issue://, history://,
rule://, omp://, agent://, vault://, local://, memory://, ssh://) from the
formatted markdown cell to the raw code cell when the setting was off, which
regressed default TUI behavior. Local file tagging now happens only when the
setting is enabled, so the default render path is byte-identical to the
pre-setting behavior while opt-in previews still work end-to-end.

Also inlined the tautological isMarkdownContentPath wrapper, pinned the
widened prose-summary bypass (.mdx stays verbatim when prose summaries are
off) with a test, and documented it under Changed in the changelog.
This commit is contained in:
can1357
2026-07-23 17:24:12 +02:00
parent 42a3da21de
commit 9d5f158e23
4 changed files with 82 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+4
View File
@@ -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.
packages/coding-agent/src/tools/read.ts
+30 -28
View File
@@ -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<typeof readSchema, ReadToolDetails> {
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<typeof readSchema, ReadToolDetails> {
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<typeof readSchema, ReadToolDetails> {
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<SummaryResult | null> {
if (fileSize > MAX_SUMMARY_BYTES) return null;
@@ -2439,14 +2436,20 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
// 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<typeof readSchema, ReadToolDetails> {
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<typeof readSchema, ReadToolDetails> {
}
}
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;
packages/coding-agent/test/read-summary.test.ts
+24 -3
View File
@@ -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(
packages/coding-agent/test/tools/read-renderer.test.ts
+24 -28
View File
@@ -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();